From 58c42b7e9400cfa03a3dfc31417462f8721a5097 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 2 Mar 2021 14:40:55 -0500 Subject: [PATCH] move CSR v1beta1 behavior tests to a single file to remove for 1.22 --- .../admission_subjectrestriction_test.go | 5 - .../certificates/controller_approval_test.go | 6 - .../certificates/deprecated_api_test.go | 128 ++++++++++++++++++ 3 files changed, 128 insertions(+), 11 deletions(-) diff --git a/test/integration/certificates/admission_subjectrestriction_test.go b/test/integration/certificates/admission_subjectrestriction_test.go index 2bf19b027c8..f996744b485 100644 --- a/test/integration/certificates/admission_subjectrestriction_test.go +++ b/test/integration/certificates/admission_subjectrestriction_test.go @@ -40,11 +40,6 @@ func TestCertificateSubjectRestrictionPlugin(t *testing.T) { 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`, }, - // 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: certv1.KubeAPIServerClientSignerName, group: "system:notmasters", diff --git a/test/integration/certificates/controller_approval_test.go b/test/integration/certificates/controller_approval_test.go index 07fd135c755..1abf147c489 100644 --- a/test/integration/certificates/controller_approval_test.go +++ b/test/integration/certificates/controller_approval_test.go @@ -79,12 +79,6 @@ func TestController_AutoApproval(t *testing.T) { grantNodeClient: true, autoApproved: true, }, - // 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: certv1.KubeAPIServerClientSignerName, request: validKubeAPIServerClientKubeletCSR, diff --git a/test/integration/certificates/deprecated_api_test.go b/test/integration/certificates/deprecated_api_test.go index 36aa3ef5ba6..4d6dd8585a3 100644 --- a/test/integration/certificates/deprecated_api_test.go +++ b/test/integration/certificates/deprecated_api_test.go @@ -19,13 +19,18 @@ package certificates import ( "context" "testing" + "time" + certv1 "k8s.io/api/certificates/v1" certv1beta1 "k8s.io/api/certificates/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/informers" clientset "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" + kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" capi "k8s.io/kubernetes/pkg/apis/certificates" + "k8s.io/kubernetes/pkg/controller/certificates/approver" "k8s.io/kubernetes/test/integration/framework" ) @@ -78,3 +83,126 @@ func TestCSRSignerNameDefaulting(t *testing.T) { }) } } + +// TODO this test can be removed once we get to 1.22 and v1beta1 is no longer allowed +// Verifies that the CertificateSubjectRestriction admission controller works as expected. +func TestDeprecatedCertificateSubjectRestrictionPlugin(t *testing.T) { + tests := map[string]struct { + signerName string + group string + error string + }{ + "should admit a request if signerName is NOT kube-apiserver-client and org is system:masters": { + signerName: certv1beta1.LegacyUnknownSignerName, + group: "system:masters", + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + // Run an apiserver with the default configuration options. + s := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{""}, framework.SharedEtcd()) + defer s.TearDownFn() + client := clientset.NewForConfigOrDie(s.ClientConfig) + + // Attempt to create the CSR resource. + csr := buildDeprecatedTestingCSR("csr", test.signerName, test.group) + _, err := client.CertificatesV1beta1().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) + } + if err == nil && test.error != "" { + t.Errorf("expected to get an error %q but got none", test.error) + } + }) + } +} + +func buildDeprecatedTestingCSR(name, signerName, groupName string) *certv1beta1.CertificateSigningRequest { + return &certv1beta1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: certv1beta1.CertificateSigningRequestSpec{ + SignerName: &signerName, + Request: pemWithGroup(groupName), + }, + } +} + +// TODO this test can be removed once we get to 1.22 and v1beta1 is no longer allowed +// Integration tests that verify the behaviour of the CSR auto-approving controller. +func TestDeprecatedController_AutoApproval(t *testing.T) { + tests := map[string]struct { + signerName string + request []byte + usages []certv1beta1.KeyUsage + username string + autoApproved bool + grantNodeClient bool + grantSelfNodeClient bool + }{ + "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, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + // Run an apiserver with the default configuration options. + s := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{""}, framework.SharedEtcd()) + defer s.TearDownFn() + client := clientset.NewForConfigOrDie(s.ClientConfig) + informers := informers.NewSharedInformerFactory(clientset.NewForConfigOrDie(restclient.AddUserAgent(s.ClientConfig, "certificatesigningrequest-informers")), time.Second) + + // Register the controller + c := approver.NewCSRApprovingController(client, informers.Certificates().V1().CertificateSigningRequests()) + // Start the controller & informers + stopCh := make(chan struct{}) + defer close(stopCh) + informers.Start(stopCh) + go c.Run(1, stopCh) + + // Configure appropriate permissions + if test.grantNodeClient { + grantUserNodeClientPermissions(t, client, test.username, false) + } + if test.grantSelfNodeClient { + grantUserNodeClientPermissions(t, client, test.username, true) + } + + // Use a client that impersonates the test case 'username' to ensure the `spec.username` + // field on the CSR is set correctly. + impersonationConfig := restclient.CopyConfig(s.ClientConfig) + impersonationConfig.Impersonate.UserName = test.username + impersonationClient, err := clientset.NewForConfig(impersonationConfig) + if err != nil { + t.Fatalf("Error in create clientset: %v", err) + } + csr := &certv1beta1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: "csr", + }, + Spec: certv1beta1.CertificateSigningRequestSpec{ + Request: test.request, + Usages: test.usages, + SignerName: &test.signerName, + }, + } + _, err = impersonationClient.CertificatesV1beta1().CertificateSigningRequests().Create(context.TODO(), csr, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("failed to create testing CSR: %v", err) + } + + if test.autoApproved { + if err := waitForCertificateRequestApproved(client, csr.Name); err != nil { + t.Errorf("failed to wait for CSR to be auto-approved: %v", err) + } + } else { + if err := ensureCertificateRequestNotApproved(client, csr.Name); err != nil { + t.Errorf("failed to ensure that CSR was not auto-approved: %v", err) + } + } + }) + } +}