Merge branch 'main' into crd_printcolumns

internal/certauthority/certauthority.go

@@ -24,12 +24,10 @@ import (
 )
 
 // certBackdate is the amount of time before time.Now() that will be used to set
-// a certificate's NotBefore field.
+// a certificate's NotBefore field.  We use the same hard coded and unconfigurable
-//
+// backdate value as used by the Kubernetes controller manager certificate signer:
-// This could certainly be made configurable by an installer of pinniped, but we
+// https://github.com/kubernetes/kubernetes/blob/68d646a101005e95379d84160adf01d146bdd149/pkg/controller/certificates/signer/signer.go#L199
-// will see if we can save adding a configuration knob with a reasonable default
+const certBackdate = 5 * time.Minute
-// here.
-const certBackdate = 10 * time.Second
 
 type env struct {
 	// secure random number generators for various steps (usually crypto/rand.Reader, but broken out here for tests).

internal/certauthority/certauthority_test.go

@@ -96,7 +96,7 @@ func TestNew(t *testing.T) {
 	caCert, err := x509.ParseCertificate(ca.caCertBytes)
 	require.NoError(t, err)
 	require.Equal(t, "Test CA", caCert.Subject.CommonName)
-	require.WithinDuration(t, now.Add(-10*time.Second), caCert.NotBefore, 10*time.Second)
+	require.WithinDuration(t, now.Add(-5*time.Minute), caCert.NotBefore, 10*time.Second)
 	require.WithinDuration(t, now.Add(time.Minute), caCert.NotAfter, 10*time.Second)
 
 	require.NotNil(t, ca.privateKey)
@@ -153,7 +153,7 @@ func TestNewInternal(t *testing.T) {
 			},
 			wantCommonName: "Test CA",
 			wantNotAfter:   now.Add(time.Minute),
-			wantNotBefore:  now.Add(-10 * time.Second),
+			wantNotBefore:  now.Add(-5 * time.Minute),
 		},
 	}
 	for _, tt := range tests {

internal/controller/impersonatorconfig/impersonator_config_test.go

@@ -1056,7 +1056,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			caCert, err := x509.ParseCertificate(block.Bytes)
 			require.NoError(t, err)
 			require.Equal(t, "Pinniped Impersonation Proxy CA", caCert.Subject.CommonName)
-			require.WithinDuration(t, time.Now().Add(-10*time.Second), caCert.NotBefore, 10*time.Second)
+			require.WithinDuration(t, time.Now().Add(-5*time.Minute), caCert.NotBefore, 10*time.Second)
 			require.WithinDuration(t, time.Now().Add(100*time.Hour*24*365), caCert.NotAfter, 10*time.Second)
 			return createdCertPEM
 		}
@@ -1077,7 +1077,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			r.NotNil(createdCertPEM)
 			validCert := testutil.ValidateServerCertificate(t, string(caCert), string(createdCertPEM))
 			validCert.RequireMatchesPrivateKey(string(createdKeyPEM))
-			validCert.RequireLifetime(time.Now().Add(-10*time.Second), time.Now().Add(100*time.Hour*24*365), 10*time.Second)
+			validCert.RequireLifetime(time.Now().Add(-5*time.Minute), time.Now().Add(100*time.Hour*24*365), 10*time.Second)
 		}
 
 		var requireSigningCertProviderHasLoadedCerts = func(certPEM, keyPEM []byte) {

internal/registry/credentialrequest/rest.go

@@ -87,7 +87,7 @@ func (*REST) Categories() []string {
 func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
 	t := trace.FromContext(ctx).Nest("create", trace.Field{
 		Key:   "kind",
-		Value: obj.GetObjectKind().GroupVersionKind().Kind,
+		Value: "TokenCredentialRequest",
 	})
 	defer t.Log()
 

internal/testutil/kube_server_compatibility.go

@@ -0,0 +1,30 @@
+// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package testutil
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	certificatesv1 "k8s.io/api/certificates/v1"
+	"k8s.io/client-go/discovery"
+)
+
+func KubeServerSupportsCertificatesV1API(t *testing.T, discoveryClient discovery.DiscoveryInterface) bool {
+	t.Helper()
+	groupList, err := discoveryClient.ServerGroups()
+	require.NoError(t, err)
+	for _, group := range groupList.Groups {
+		if group.Name == certificatesv1.GroupName {
+			for _, version := range group.Versions {
+				if version.Version == "v1" {
+					// Note: v1 should exist in Kubernetes 1.19 and above
+					return true
+				}
+			}
+		}
+		continue
+	}
+	return false
+}

test/integration/concierge_impersonation_proxy_test.go

@@ -938,6 +938,9 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
 			namespaceName := testlib.CreateNamespace(ctx, t, "impersonation").Name
 			kubeClient := adminClient.CoreV1()
 			saName, _, saUID := createServiceAccountToken(ctx, t, adminClient, namespaceName)
+			expectedUsername := serviceaccount.MakeUsername(namespaceName, saName)
+			expectedUID := string(saUID)
+			expectedGroups := []string{"system:serviceaccounts", "system:serviceaccounts:" + namespaceName, "system:authenticated"}
 
 			_, tokenRequestProbeErr := kubeClient.ServiceAccounts(namespaceName).CreateToken(ctx, saName, &authenticationv1.TokenRequest{}, metav1.CreateOptions{})
 			if k8serrors.IsNotFound(tokenRequestProbeErr) && tokenRequestProbeErr.Error() == "the server could not find the requested resource" {
@@ -1002,8 +1005,8 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
 			// new service account tokens include the pod info in the extra fields
 			require.Equal(t,
 				expectedWhoAmIRequestResponse(
-					serviceaccount.MakeUsername(namespaceName, saName),
+					expectedUsername,
-					[]string{"system:serviceaccounts", "system:serviceaccounts:" + namespaceName, "system:authenticated"},
+					expectedGroups,
 					map[string]identityv1alpha1.ExtraValue{
 						"authentication.kubernetes.io/pod-name": {pod.Name},
 						"authentication.kubernetes.io/pod-uid":  {string(pod.UID)},
@@ -1017,7 +1020,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
 				rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: saName, Namespace: namespaceName},
 				rbacv1.RoleRef{Kind: "ClusterRole", APIGroup: rbacv1.GroupName, Name: "system:node-bootstrapper"},
 			)
-			testlib.WaitForUserToHaveAccess(t, serviceaccount.MakeUsername(namespaceName, saName), []string{}, &authorizationv1.ResourceAttributes{
+			testlib.WaitForUserToHaveAccess(t, expectedUsername, []string{}, &authorizationv1.ResourceAttributes{
 				Verb: "create", Group: certificatesv1.GroupName, Version: "*", Resource: "certificatesigningrequests",
 			})
 
@@ -1041,20 +1044,34 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
 			)
 			require.NoError(t, err)
 
-			saCSR, err := impersonationProxySAClient.Kubernetes.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
+			if testutil.KubeServerSupportsCertificatesV1API(t, adminClient.Discovery()) {
-			require.NoError(t, err)
+				saCSR, err := impersonationProxySAClient.Kubernetes.CertificatesV1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
-
+				require.NoError(t, err)
-			err = adminClient.CertificatesV1beta1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
+				err = adminClient.CertificatesV1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
-			require.NoError(t, err)
+				require.NoError(t, err)
-
+				// make sure the user info that the CSR captured matches the SA, including the UID
-			// make sure the user info that the CSR captured matches the SA, including the UID
+				require.Equal(t, expectedUsername, saCSR.Spec.Username)
-			require.Equal(t, serviceaccount.MakeUsername(namespaceName, saName), saCSR.Spec.Username)
+				require.Equal(t, expectedUID, saCSR.Spec.UID)
-			require.Equal(t, string(saUID), saCSR.Spec.UID)
+				require.Equal(t, expectedGroups, saCSR.Spec.Groups)
-			require.Equal(t, []string{"system:serviceaccounts", "system:serviceaccounts:" + namespaceName, "system:authenticated"}, saCSR.Spec.Groups)
+				require.Equal(t, map[string]certificatesv1.ExtraValue{
-			require.Equal(t, map[string]certificatesv1beta1.ExtraValue{
+					"authentication.kubernetes.io/pod-name": {pod.Name},
-				"authentication.kubernetes.io/pod-name": {pod.Name},
+					"authentication.kubernetes.io/pod-uid":  {string(pod.UID)},
-				"authentication.kubernetes.io/pod-uid":  {string(pod.UID)},
+				}, saCSR.Spec.Extra)
-			}, saCSR.Spec.Extra)
+			} else {
+				// On old Kubernetes clusters use CertificatesV1beta1
+				saCSR, err := impersonationProxySAClient.Kubernetes.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
+				require.NoError(t, err)
+				err = adminClient.CertificatesV1beta1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
+				require.NoError(t, err)
+				// make sure the user info that the CSR captured matches the SA, including the UID
+				require.Equal(t, expectedUsername, saCSR.Spec.Username)
+				require.Equal(t, expectedUID, saCSR.Spec.UID)
+				require.Equal(t, expectedGroups, saCSR.Spec.Groups)
+				require.Equal(t, map[string]certificatesv1beta1.ExtraValue{
+					"authentication.kubernetes.io/pod-name": {pod.Name},
+					"authentication.kubernetes.io/pod-uid":  {string(pod.UID)},
+				}, saCSR.Spec.Extra)
+			}
 		})
 
 		t.Run("kubectl as a client", func(t *testing.T) {
@@ -2416,7 +2433,7 @@ func getCredForConfig(t *testing.T, config *rest.Config) *loginv1alpha1.ClusterC
 	return out
 }
 
-func getUIDAndExtraViaCSR(ctx context.Context, t *testing.T, uid string, client kubernetes.Interface) (string, map[string]certificatesv1beta1.ExtraValue) {
+func getUIDAndExtraViaCSR(ctx context.Context, t *testing.T, uid string, client kubernetes.Interface) (string, map[string][]string) {
 	t.Helper()
 
 	privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -2439,18 +2456,43 @@ func getUIDAndExtraViaCSR(ctx context.Context, t *testing.T, uid string, client
 	)
 	require.NoError(t, err)
 
-	csReq, err := client.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
-	require.NoError(t, err)
-
-	err = client.CertificatesV1beta1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
-	require.NoError(t, err)
-
 	outUID := uid // in the future this may not be empty on some clusters
-	if len(outUID) == 0 {
+	extrasAsStrings := map[string][]string{}
-		outUID = csReq.Spec.UID
+
+	if testutil.KubeServerSupportsCertificatesV1API(t, client.Discovery()) {
+		csReq, err := client.CertificatesV1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
+		require.NoError(t, err)
+
+		err = client.CertificatesV1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
+		require.NoError(t, err)
+
+		if len(outUID) == 0 {
+			outUID = csReq.Spec.UID
+		}
+
+		// Convert each `ExtraValue` to `[]string` to return, so we don't have to deal with v1beta1 types versus v1 types
+		for k, v := range csReq.Spec.Extra {
+			extrasAsStrings[k] = v
+		}
+	} else {
+		// On old Kubernetes clusters use CertificatesV1beta1
+		csReq, err := client.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
+		require.NoError(t, err)
+
+		err = client.CertificatesV1beta1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
+		require.NoError(t, err)
+
+		if len(outUID) == 0 {
+			outUID = csReq.Spec.UID
+		}
+
+		// Convert each `ExtraValue` to `[]string` to return, so we don't have to deal with v1beta1 types versus v1 types
+		for k, v := range csReq.Spec.Extra {
+			extrasAsStrings[k] = v
+		}
 	}
 
-	return outUID, csReq.Spec.Extra
+	return outUID, extrasAsStrings
 }
 
 func parallelIfNotEKS(t *testing.T) {

test/integration/whoami_test.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/client-go/util/keyutil"
 
 	identityv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/identity/v1alpha1"
+	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/test/testlib"
 )
 
@@ -281,28 +282,53 @@ func TestWhoAmI_CSR_Parallel(t *testing.T) {
 	)
 	require.NoError(t, err)
 
+	useCertificatesV1API := testutil.KubeServerSupportsCertificatesV1API(t, kubeClient.Discovery())
+
 	t.Cleanup(func() {
-		require.NoError(t, kubeClient.CertificatesV1beta1().CertificateSigningRequests().
+		if useCertificatesV1API {
-			Delete(context.Background(), csrName, metav1.DeleteOptions{}))
+			require.NoError(t, kubeClient.CertificatesV1().CertificateSigningRequests().
+				Delete(context.Background(), csrName, metav1.DeleteOptions{}))
+		} else {
+			// On old clusters use v1beta1
+			require.NoError(t, kubeClient.CertificatesV1beta1().CertificateSigningRequests().
+				Delete(context.Background(), csrName, metav1.DeleteOptions{}))
+		}
 	})
 
-	// this is a blind update with no resource version checks, which is only safe during tests
+	if useCertificatesV1API {
-	// use the beta CSR API to support older clusters
+		_, err = kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csrName, &certificatesv1.CertificateSigningRequest{
-	_, err = kubeClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, &certificatesv1beta1.CertificateSigningRequest{
+			ObjectMeta: metav1.ObjectMeta{
-		ObjectMeta: metav1.ObjectMeta{
+				Name: csrName,
-			Name: csrName,
+			},
-		},
+			Status: certificatesv1.CertificateSigningRequestStatus{
-		Status: certificatesv1beta1.CertificateSigningRequestStatus{
+				Conditions: []certificatesv1.CertificateSigningRequestCondition{
-			Conditions: []certificatesv1beta1.CertificateSigningRequestCondition{
+					{
-				{
+						Type:   certificatesv1.CertificateApproved,
-					Type:   certificatesv1beta1.CertificateApproved,
+						Status: corev1.ConditionTrue,
-					Status: corev1.ConditionTrue,
+						Reason: "WhoAmICSRTest",
-					Reason: "WhoAmICSRTest",
+					},
 				},
 			},
-		},
+		}, metav1.UpdateOptions{})
-	}, metav1.UpdateOptions{})
+		require.NoError(t, err)
-	require.NoError(t, err)
+	} else {
+		// On old Kubernetes clusters use CertificatesV1beta1
+		_, err = kubeClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, &certificatesv1beta1.CertificateSigningRequest{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: csrName,
+			},
+			Status: certificatesv1beta1.CertificateSigningRequestStatus{
+				Conditions: []certificatesv1beta1.CertificateSigningRequestCondition{
+					{
+						Type:   certificatesv1beta1.CertificateApproved,
+						Status: corev1.ConditionTrue,
+						Reason: "WhoAmICSRTest",
+					},
+				},
+			},
+		}, metav1.UpdateOptions{})
+		require.NoError(t, err)
+	}
 
 	crtPEM, err := csr.WaitForCertificate(ctx, kubeClient, csrName, csrUID)
 	require.NoError(t, err)