From fd8e81eba4117f7e252b17f64a215582944cbec4 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 2 Mar 2021 12:12:27 -0500 Subject: [PATCH] remove csr v1beta1 usage --- .../certificates/admission_approval_test.go | 16 ++++---- .../certificates/admission_sign_test.go | 39 ++++++++++++++++--- .../admission_subjectrestriction_test.go | 17 ++++---- .../certificates/controller_approval_test.go | 39 ++++++++++--------- ...aulting_test.go => deprecated_api_test.go} | 17 ++++---- .../certificates/field_selector_test.go | 22 ++++++----- 6 files changed, 92 insertions(+), 58 deletions(-) rename test/integration/certificates/{defaulting_test.go => deprecated_api_test.go} (80%) diff --git a/test/integration/certificates/admission_approval_test.go b/test/integration/certificates/admission_approval_test.go index 57d566833d8..d281abbc62e 100644 --- a/test/integration/certificates/admission_approval_test.go +++ b/test/integration/certificates/admission_approval_test.go @@ -20,7 +20,8 @@ import ( "context" "testing" - certv1beta1 "k8s.io/api/certificates/v1beta1" + certv1 "k8s.io/api/certificates/v1" + v1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -64,7 +65,7 @@ func TestCSRSignerNameApprovalPlugin(t *testing.T) { const username = "test-user" grantUserPermissionToApproveFor(t, client, username, test.allowedSignerName) // Create a CSR to attempt to approve. - csr := createTestingCSR(t, client.CertificatesV1beta1().CertificateSigningRequests(), "csr", test.signerName, "") + csr := createTestingCSR(t, client.CertificatesV1().CertificateSigningRequests(), "csr", test.signerName, "") // Create a second client, impersonating the 'test-user' for us to test with. testuserConfig := restclient.CopyConfig(s.ClientConfig) @@ -72,12 +73,13 @@ func TestCSRSignerNameApprovalPlugin(t *testing.T) { testuserClient := clientset.NewForConfigOrDie(testuserConfig) // Attempt to update the Approved condition. - csr.Status.Conditions = append(csr.Status.Conditions, certv1beta1.CertificateSigningRequestCondition{ - Type: certv1beta1.CertificateApproved, + csr.Status.Conditions = append(csr.Status.Conditions, certv1.CertificateSigningRequestCondition{ + Type: certv1.CertificateApproved, + Status: v1.ConditionTrue, Reason: "AutoApproved", Message: "Approved during integration test", }) - _, err := testuserClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(context.TODO(), csr, metav1.UpdateOptions{}) + _, err := testuserClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(context.TODO(), csr.Name, csr, metav1.UpdateOptions{}) if err != nil && test.error != err.Error() { t.Errorf("expected error %q but got: %v", test.error, err) } @@ -114,13 +116,13 @@ func buildApprovalClusterRoleForSigners(name string, signerNames ...string) *rba // 'signerName' to approve CSRs with the given signerName. { Verbs: []string{"approve"}, - APIGroups: []string{certv1beta1.SchemeGroupVersion.Group}, + APIGroups: []string{certv1.SchemeGroupVersion.Group}, Resources: []string{"signers"}, ResourceNames: signerNames, }, { Verbs: []string{"update"}, - APIGroups: []string{certv1beta1.SchemeGroupVersion.Group}, + APIGroups: []string{certv1.SchemeGroupVersion.Group}, Resources: []string{"certificatesigningrequests/approval"}, }, }, diff --git a/test/integration/certificates/admission_sign_test.go b/test/integration/certificates/admission_sign_test.go index 514114a0536..ffd803aa289 100644 --- a/test/integration/certificates/admission_sign_test.go +++ b/test/integration/certificates/admission_sign_test.go @@ -20,7 +20,7 @@ import ( "context" "testing" - certv1beta1 "k8s.io/api/certificates/v1beta1" + certv1 "k8s.io/api/certificates/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -64,7 +64,7 @@ func TestCSRSignerNameSigningPlugin(t *testing.T) { const username = "test-user" grantUserPermissionToSignFor(t, client, username, test.allowedSignerName) // Create a CSR to attempt to sign. - csr := createTestingCSR(t, client.CertificatesV1beta1().CertificateSigningRequests(), "csr", test.signerName, "") + csr := createTestingCSR(t, client.CertificatesV1().CertificateSigningRequests(), "csr", test.signerName, "") // Create a second client, impersonating the 'test-user' for us to test with. testuserConfig := restclient.CopyConfig(s.ClientConfig) @@ -72,8 +72,35 @@ func TestCSRSignerNameSigningPlugin(t *testing.T) { testuserClient := clientset.NewForConfigOrDie(testuserConfig) // Attempt to 'sign' the certificate. - csr.Status.Certificate = []byte("dummy data") - _, err := testuserClient.CertificatesV1beta1().CertificateSigningRequests().UpdateStatus(context.TODO(), csr, metav1.UpdateOptions{}) + // random valid pem + csr.Status.Certificate = []byte(` +Leading non-PEM content +-----BEGIN CERTIFICATE----- +MIIBqDCCAU2gAwIBAgIUfbqeieihh/oERbfvRm38XvS/xHAwCgYIKoZIzj0EAwIw +GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMCAXDTE2MTAxMTA1MDYwMFoYDzIx +MTYwOTE3MDUwNjAwWjAUMRIwEAYDVQQDEwlNeSBDbGllbnQwWTATBgcqhkjOPQIB +BggqhkjOPQMBBwNCAARv6N4R/sjMR65iMFGNLN1GC/vd7WhDW6J4X/iAjkRLLnNb +KbRG/AtOUZ+7upJ3BWIRKYbOabbQGQe2BbKFiap4o3UwczAOBgNVHQ8BAf8EBAMC +BaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU +K/pZOWpNcYai6eHFpmJEeFpeQlEwHwYDVR0jBBgwFoAUX6nQlxjfWnP6aM1meO/Q +a6b3a9kwCgYIKoZIzj0EAwIDSQAwRgIhAIWTKw/sjJITqeuNzJDAKU4xo1zL+xJ5 +MnVCuBwfwDXCAiEAw/1TA+CjPq9JC5ek1ifR0FybTURjeQqYkKpve1dveps= +-----END CERTIFICATE----- +Intermediate non-PEM content +-----BEGIN CERTIFICATE----- +MIIBqDCCAU6gAwIBAgIUfqZtjoFgczZ+oQZbEC/BDSS2J6wwCgYIKoZIzj0EAwIw +EjEQMA4GA1UEAxMHUm9vdC1DQTAgFw0xNjEwMTEwNTA2MDBaGA8yMTE2MDkxNzA1 +MDYwMFowGjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMFkwEwYHKoZIzj0CAQYI +KoZIzj0DAQcDQgAEyWHEMMCctJg8Xa5YWLqaCPbk3MjB+uvXac42JM9pj4k9jedD +kpUJRkWIPzgJI8Zk/3cSzluUTixP6JBSDKtwwaN4MHYwDgYDVR0PAQH/BAQDAgGm +MBMGA1UdJQQMMAoGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYE +FF+p0JcY31pz+mjNZnjv0Gum92vZMB8GA1UdIwQYMBaAFB7P6+i4/pfNjqZgJv/b +dgA7Fe4tMAoGCCqGSM49BAMCA0gAMEUCIQCTT1YWQZaAqfQ2oBxzOkJE2BqLFxhz +3smQlrZ5gCHddwIgcvT7puhYOzAgcvMn9+SZ1JOyZ7edODjshCVCRnuHK2c= +-----END CERTIFICATE----- +Trailing non-PEM content +`) + _, err := testuserClient.CertificatesV1().CertificateSigningRequests().UpdateStatus(context.TODO(), csr, metav1.UpdateOptions{}) if err != nil && test.error != err.Error() { t.Errorf("expected error %q but got: %v", test.error, err) } @@ -110,13 +137,13 @@ func buildSigningClusterRoleForSigners(name string, signerNames ...string) *rbac // 'signerName' to approve CSRs with the given signerName. { Verbs: []string{"sign"}, - APIGroups: []string{certv1beta1.SchemeGroupVersion.Group}, + APIGroups: []string{certv1.SchemeGroupVersion.Group}, Resources: []string{"signers"}, ResourceNames: signerNames, }, { Verbs: []string{"update"}, - APIGroups: []string{certv1beta1.SchemeGroupVersion.Group}, + APIGroups: []string{certv1.SchemeGroupVersion.Group}, Resources: []string{"certificatesigningrequests/status"}, }, }, diff --git a/test/integration/certificates/admission_subjectrestriction_test.go b/test/integration/certificates/admission_subjectrestriction_test.go index 9231693cfcc..2bf19b027c8 100644 --- a/test/integration/certificates/admission_subjectrestriction_test.go +++ b/test/integration/certificates/admission_subjectrestriction_test.go @@ -20,7 +20,7 @@ import ( "context" "testing" - certv1beta1 "k8s.io/api/certificates/v1beta1" + certv1 "k8s.io/api/certificates/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" @@ -36,16 +36,17 @@ func TestCertificateSubjectRestrictionPlugin(t *testing.T) { error string }{ "should reject a request if signerName is kube-apiserver-client and group is system:masters": { - signerName: certv1beta1.KubeAPIServerClientSignerName, + signerName: certv1.KubeAPIServerClientSignerName, group: "system:masters", error: `certificatesigningrequests.certificates.k8s.io "csr" is forbidden: use of kubernetes.io/kube-apiserver-client signer with system:masters group is not allowed`, }, - "should admit a request if signerName is NOT kube-apiserver-client and org is system:masters": { - signerName: certv1beta1.LegacyUnknownSignerName, - group: "system:masters", - }, + // legacy unknown signer is no longer allowed in v1. + //"should admit a request if signerName is NOT kube-apiserver-client and org is system:masters": { + // signerName: certv1.LegacyUnknownSignerName, + // group: "system:masters", + //}, "should admit a request if signerName is kube-apiserver-client and group is NOT system:masters": { - signerName: certv1beta1.KubeAPIServerClientSignerName, + signerName: certv1.KubeAPIServerClientSignerName, group: "system:notmasters", }, } @@ -58,7 +59,7 @@ func TestCertificateSubjectRestrictionPlugin(t *testing.T) { // Attempt to create the CSR resource. csr := buildTestingCSR("csr", test.signerName, test.group) - _, err := client.CertificatesV1beta1().CertificateSigningRequests().Create(context.TODO(), csr, metav1.CreateOptions{}) + _, err := client.CertificatesV1().CertificateSigningRequests().Create(context.TODO(), csr, metav1.CreateOptions{}) if err != nil && test.error != err.Error() { t.Errorf("expected error %q but got: %v", test.error, err) } diff --git a/test/integration/certificates/controller_approval_test.go b/test/integration/certificates/controller_approval_test.go index 228b463c999..07fd135c755 100644 --- a/test/integration/certificates/controller_approval_test.go +++ b/test/integration/certificates/controller_approval_test.go @@ -24,7 +24,7 @@ import ( "testing" "time" - certv1beta1 "k8s.io/api/certificates/v1beta1" + certv1 "k8s.io/api/certificates/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -49,22 +49,22 @@ func TestController_AutoApproval(t *testing.T) { Organization: []string{"system:nodes"}, }, }) - validKubeAPIServerClientKubeletUsages := []certv1beta1.KeyUsage{ - certv1beta1.UsageDigitalSignature, - certv1beta1.UsageKeyEncipherment, - certv1beta1.UsageClientAuth, + validKubeAPIServerClientKubeletUsages := []certv1.KeyUsage{ + certv1.UsageDigitalSignature, + certv1.UsageKeyEncipherment, + certv1.UsageClientAuth, } tests := map[string]struct { signerName string request []byte - usages []certv1beta1.KeyUsage + usages []certv1.KeyUsage username string autoApproved bool grantNodeClient bool grantSelfNodeClient bool }{ "should auto-approve CSR that has kube-apiserver-client-kubelet signerName and matches requirements": { - signerName: certv1beta1.KubeAPIServerClientKubeletSignerName, + signerName: certv1.KubeAPIServerClientKubeletSignerName, request: validKubeAPIServerClientKubeletCSR, usages: validKubeAPIServerClientKubeletUsages, username: validKubeAPIServerClientKubeletUsername, @@ -72,20 +72,21 @@ func TestController_AutoApproval(t *testing.T) { autoApproved: true, }, "should auto-approve CSR that has kube-apiserver-client-kubelet signerName and matches requirements despite missing username if nodeclient permissions are granted": { - signerName: certv1beta1.KubeAPIServerClientKubeletSignerName, + signerName: certv1.KubeAPIServerClientKubeletSignerName, request: validKubeAPIServerClientKubeletCSR, usages: validKubeAPIServerClientKubeletUsages, username: "does-not-match-cn", grantNodeClient: true, autoApproved: true, }, - "should not auto-approve CSR that has kube-apiserver-client-kubelet signerName that does not match requirements": { - signerName: certv1beta1.KubeAPIServerClientKubeletSignerName, - request: pemWithGroup("system:notnodes"), - autoApproved: false, - }, + // usages are now required in v1. + //"should not auto-approve CSR that has kube-apiserver-client-kubelet signerName that does not match requirements": { + // signerName: certv1.KubeAPIServerClientKubeletSignerName, + // request: pemWithGroup("system:notnodes"), + // autoApproved: false, + //}, "should not auto-approve CSR that has kube-apiserver-client signerName that DOES match kubelet CSR requirements": { - signerName: certv1beta1.KubeAPIServerClientSignerName, + signerName: certv1.KubeAPIServerClientSignerName, request: validKubeAPIServerClientKubeletCSR, usages: validKubeAPIServerClientKubeletUsages, username: validKubeAPIServerClientKubeletUsername, @@ -124,17 +125,17 @@ func TestController_AutoApproval(t *testing.T) { if err != nil { t.Fatalf("Error in create clientset: %v", err) } - csr := &certv1beta1.CertificateSigningRequest{ + csr := &certv1.CertificateSigningRequest{ ObjectMeta: metav1.ObjectMeta{ Name: "csr", }, - Spec: certv1beta1.CertificateSigningRequestSpec{ + Spec: certv1.CertificateSigningRequestSpec{ Request: test.request, Usages: test.usages, - SignerName: &test.signerName, + SignerName: test.signerName, }, } - _, err = impersonationClient.CertificatesV1beta1().CertificateSigningRequests().Create(context.TODO(), csr, metav1.CreateOptions{}) + _, err = impersonationClient.CertificatesV1().CertificateSigningRequests().Create(context.TODO(), csr, metav1.CreateOptions{}) if err != nil { t.Fatalf("failed to create testing CSR: %v", err) } @@ -212,7 +213,7 @@ func buildNodeClientRoleForUser(name string, resourceType string) *rbacv1.Cluste Rules: []rbacv1.PolicyRule{ { Verbs: []string{"create"}, - APIGroups: []string{certv1beta1.SchemeGroupVersion.Group}, + APIGroups: []string{certv1.SchemeGroupVersion.Group}, Resources: []string{resourceType}, }, }, diff --git a/test/integration/certificates/defaulting_test.go b/test/integration/certificates/deprecated_api_test.go similarity index 80% rename from test/integration/certificates/defaulting_test.go rename to test/integration/certificates/deprecated_api_test.go index 9376703c494..36aa3ef5ba6 100644 --- a/test/integration/certificates/defaulting_test.go +++ b/test/integration/certificates/deprecated_api_test.go @@ -20,15 +20,16 @@ import ( "context" "testing" - capi "k8s.io/api/certificates/v1beta1" + certv1beta1 "k8s.io/api/certificates/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" clientset "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" - + capi "k8s.io/kubernetes/pkg/apis/certificates" "k8s.io/kubernetes/test/integration/framework" ) +// TODO this test can be removed once we get to 1.22 and v1beta1 is no longer allowed // Verifies that the signerName field defaulting is wired up correctly. // An exhaustive set of test cases for all permutations of the possible // defaulting cases is written as a unit tests in the @@ -38,20 +39,20 @@ import ( func TestCSRSignerNameDefaulting(t *testing.T) { strPtr := func(s string) *string { return &s } tests := map[string]struct { - csr capi.CertificateSigningRequestSpec + csr certv1beta1.CertificateSigningRequestSpec expectedSignerName string }{ "defaults to legacy-unknown if not recognised": { - csr: capi.CertificateSigningRequestSpec{ + csr: certv1beta1.CertificateSigningRequestSpec{ Request: pemWithGroup(""), - Usages: []capi.KeyUsage{capi.UsageKeyEncipherment, capi.UsageDigitalSignature}, + Usages: []certv1beta1.KeyUsage{certv1beta1.UsageKeyEncipherment, certv1beta1.UsageDigitalSignature}, }, expectedSignerName: capi.LegacyUnknownSignerName, }, "does not default signerName if an explicit value is provided": { - csr: capi.CertificateSigningRequestSpec{ + csr: certv1beta1.CertificateSigningRequestSpec{ Request: pemWithGroup(""), - Usages: []capi.KeyUsage{capi.UsageKeyEncipherment, capi.UsageDigitalSignature}, + Usages: []certv1beta1.KeyUsage{certv1beta1.UsageKeyEncipherment, certv1beta1.UsageDigitalSignature}, SignerName: strPtr("example.com/my-custom-signer"), }, expectedSignerName: "example.com/my-custom-signer", @@ -63,7 +64,7 @@ func TestCSRSignerNameDefaulting(t *testing.T) { defer closeFn() client := clientset.NewForConfigOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}) csrClient := client.CertificatesV1beta1().CertificateSigningRequests() - csr := &capi.CertificateSigningRequest{ + csr := &certv1beta1.CertificateSigningRequest{ ObjectMeta: metav1.ObjectMeta{Name: "testcsr"}, Spec: test.csr, } diff --git a/test/integration/certificates/field_selector_test.go b/test/integration/certificates/field_selector_test.go index 9f1b4eb5854..6209f05b861 100644 --- a/test/integration/certificates/field_selector_test.go +++ b/test/integration/certificates/field_selector_test.go @@ -25,11 +25,11 @@ import ( "encoding/pem" "testing" - capi "k8s.io/api/certificates/v1beta1" + certv1 "k8s.io/api/certificates/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" clientset "k8s.io/client-go/kubernetes" - certclientset "k8s.io/client-go/kubernetes/typed/certificates/v1beta1" + certclientset "k8s.io/client-go/kubernetes/typed/certificates/v1" restclient "k8s.io/client-go/rest" "k8s.io/kubernetes/test/integration/framework" @@ -41,14 +41,14 @@ func TestCSRSignerNameFieldSelector(t *testing.T) { defer closeFn() client := clientset.NewForConfigOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}) - csrClient := client.CertificatesV1beta1().CertificateSigningRequests() + csrClient := client.CertificatesV1().CertificateSigningRequests() csr1 := createTestingCSR(t, csrClient, "csr-1", "example.com/signer-name-1", "") csr2 := createTestingCSR(t, csrClient, "csr-2", "example.com/signer-name-2", "") // csr3 has the same signerName as csr2 so we can ensure multiple items are returned when running a filtered // LIST call. csr3 := createTestingCSR(t, csrClient, "csr-3", "example.com/signer-name-2", "") - signerOneList, err := client.CertificatesV1beta1().CertificateSigningRequests().List(context.TODO(), metav1.ListOptions{FieldSelector: "spec.signerName=example.com/signer-name-1"}) + signerOneList, err := client.CertificatesV1().CertificateSigningRequests().List(context.TODO(), metav1.ListOptions{FieldSelector: "spec.signerName=example.com/signer-name-1"}) if err != nil { t.Errorf("unable to list CSRs with spec.signerName=example.com/signer-name-1") return @@ -59,7 +59,7 @@ func TestCSRSignerNameFieldSelector(t *testing.T) { t.Errorf("expected CSR named 'csr-1' to be returned but got %q", signerOneList.Items[0].Name) } - signerTwoList, err := client.CertificatesV1beta1().CertificateSigningRequests().List(context.TODO(), metav1.ListOptions{FieldSelector: "spec.signerName=example.com/signer-name-2"}) + signerTwoList, err := client.CertificatesV1().CertificateSigningRequests().List(context.TODO(), metav1.ListOptions{FieldSelector: "spec.signerName=example.com/signer-name-2"}) if err != nil { t.Errorf("unable to list CSRs with spec.signerName=example.com/signer-name-2") return @@ -73,7 +73,7 @@ func TestCSRSignerNameFieldSelector(t *testing.T) { } } -func createTestingCSR(t *testing.T, certClient certclientset.CertificateSigningRequestInterface, name, signerName, groupName string) *capi.CertificateSigningRequest { +func createTestingCSR(t *testing.T, certClient certclientset.CertificateSigningRequestInterface, name, signerName, groupName string) *certv1.CertificateSigningRequest { csr, err := certClient.Create(context.TODO(), buildTestingCSR(name, signerName, groupName), metav1.CreateOptions{}) if err != nil { t.Fatalf("failed to create testing CSR: %v", err) @@ -81,14 +81,16 @@ func createTestingCSR(t *testing.T, certClient certclientset.CertificateSigningR return csr } -func buildTestingCSR(name, signerName, groupName string) *capi.CertificateSigningRequest { - return &capi.CertificateSigningRequest{ +func buildTestingCSR(name, signerName, groupName string) *certv1.CertificateSigningRequest { + return &certv1.CertificateSigningRequest{ ObjectMeta: metav1.ObjectMeta{ Name: name, }, - Spec: capi.CertificateSigningRequestSpec{ - SignerName: &signerName, + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: signerName, Request: pemWithGroup(groupName), + // this is the old defaulting for usages + Usages: []certv1.KeyUsage{certv1.UsageDigitalSignature, certv1.UsageKeyEncipherment}, }, } }