From 4e98c1bbdbf9a2b63126063946dca5e51b18ce16 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Mon, 20 Sep 2021 17:14:58 -0700 Subject: [PATCH] Tests use CertificatesV1 when available, otherwise use CertificatesV1beta1 CertificatesV1beta1 was removed in Kube 1.22, so the tests cannot blindly rely on it anymore. Use CertificatesV1 whenever the server reports that is available, and otherwise use the old CertificatesV1beta1. Note that CertificatesV1 was introduced in Kube 1.19. --- .../testutil/kube_server_compatibility.go | 30 ++++++ .../concierge_impersonation_proxy_test.go | 96 +++++++++++++------ test/integration/whoami_test.go | 60 ++++++++---- 3 files changed, 142 insertions(+), 44 deletions(-) create mode 100644 internal/testutil/kube_server_compatibility.go diff --git a/internal/testutil/kube_server_compatibility.go b/internal/testutil/kube_server_compatibility.go new file mode 100644 index 00000000..89cf15b4 --- /dev/null +++ b/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 +} diff --git a/test/integration/concierge_impersonation_proxy_test.go b/test/integration/concierge_impersonation_proxy_test.go index 5c3b0994..ce451a65 100644 --- a/test/integration/concierge_impersonation_proxy_test.go +++ b/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), - []string{"system:serviceaccounts", "system:serviceaccounts:" + namespaceName, "system:authenticated"}, + expectedUsername, + 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{}) - 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, serviceaccount.MakeUsername(namespaceName, saName), saCSR.Spec.Username) - require.Equal(t, string(saUID), saCSR.Spec.UID) - require.Equal(t, []string{"system:serviceaccounts", "system:serviceaccounts:" + namespaceName, "system:authenticated"}, 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) + if testutil.KubeServerSupportsCertificatesV1API(t, adminClient.Discovery()) { + saCSR, err := impersonationProxySAClient.Kubernetes.CertificatesV1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{}) + require.NoError(t, err) + err = adminClient.CertificatesV1().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]certificatesv1.ExtraValue{ + "authentication.kubernetes.io/pod-name": {pod.Name}, + "authentication.kubernetes.io/pod-uid": {string(pod.UID)}, + }, 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 { - outUID = csReq.Spec.UID + extrasAsStrings := map[string][]string{} + + 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) { diff --git a/test/integration/whoami_test.go b/test/integration/whoami_test.go index 17bae321..f4aef611 100644 --- a/test/integration/whoami_test.go +++ b/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(). - Delete(context.Background(), csrName, metav1.DeleteOptions{})) + if useCertificatesV1API { + 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 - // use the beta CSR API to support older clusters - _, 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", + if useCertificatesV1API { + _, err = kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csrName, &certificatesv1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: csrName, + }, + Status: certificatesv1.CertificateSigningRequestStatus{ + Conditions: []certificatesv1.CertificateSigningRequestCondition{ + { + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + Reason: "WhoAmICSRTest", + }, }, }, - }, - }, metav1.UpdateOptions{}) - require.NoError(t, err) + }, metav1.UpdateOptions{}) + 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)