From 7665f15b7d8d9006e410e41f6678cfa2be3ac602 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Thu, 12 Apr 2018 07:09:12 -0700 Subject: [PATCH] sarapprover: remove self node cert The functionality to bootstrap node certificates is ready but is blocked by a seperable issue discussed in: https://github.com/kubernetes/community/pull/1982. The functionality could be useful for power users who want to write their own approvers if the feature could be promoted to beta. In it's current state this feature doesn't help anybody. I propose that we remove automated approval of node serving certificates for now and work towards getting the node functionality to beta. --- pkg/controller/certificates/approver/BUILD | 2 - .../certificates/approver/sarapprove.go | 34 --------- .../certificates/approver/sarapprove_test.go | 71 ------------------- .../authorizer/rbac/bootstrappolicy/policy.go | 10 --- 4 files changed, 117 deletions(-) diff --git a/pkg/controller/certificates/approver/BUILD b/pkg/controller/certificates/approver/BUILD index e54a142236e0f..874a6ddf3835b 100644 --- a/pkg/controller/certificates/approver/BUILD +++ b/pkg/controller/certificates/approver/BUILD @@ -28,10 +28,8 @@ go_library( deps = [ "//pkg/apis/certificates/v1beta1:go_default_library", "//pkg/controller/certificates:go_default_library", - "//pkg/features:go_default_library", "//vendor/k8s.io/api/authorization/v1beta1:go_default_library", "//vendor/k8s.io/api/certificates/v1beta1:go_default_library", - "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", "//vendor/k8s.io/client-go/informers/certificates/v1beta1:go_default_library", "//vendor/k8s.io/client-go/kubernetes:go_default_library", ], diff --git a/pkg/controller/certificates/approver/sarapprove.go b/pkg/controller/certificates/approver/sarapprove.go index 3aa6be1ce08fd..e5ca32f867ada 100644 --- a/pkg/controller/certificates/approver/sarapprove.go +++ b/pkg/controller/certificates/approver/sarapprove.go @@ -25,12 +25,10 @@ import ( authorization "k8s.io/api/authorization/v1beta1" capi "k8s.io/api/certificates/v1beta1" - utilfeature "k8s.io/apiserver/pkg/util/feature" certificatesinformers "k8s.io/client-go/informers/certificates/v1beta1" clientset "k8s.io/client-go/kubernetes" k8s_certificates_v1beta1 "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" "k8s.io/kubernetes/pkg/controller/certificates" - "k8s.io/kubernetes/pkg/features" ) type csrRecognizer struct { @@ -69,13 +67,6 @@ func recognizers() []csrRecognizer { successMessage: "Auto approving kubelet client certificate after SubjectAccessReview.", }, } - if utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletServerCertificate) { - recognizers = append(recognizers, csrRecognizer{ - recognize: isSelfNodeServerCert, - permission: authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "selfnodeserver"}, - successMessage: "Auto approving self kubelet server certificate after SubjectAccessReview.", - }) - } return recognizers } @@ -201,28 +192,3 @@ func isSelfNodeClientCert(csr *capi.CertificateSigningRequest, x509cr *x509.Cert } return true } - -var kubeletServerUsages = []capi.KeyUsage{ - capi.UsageKeyEncipherment, - capi.UsageDigitalSignature, - capi.UsageServerAuth, -} - -func isSelfNodeServerCert(csr *capi.CertificateSigningRequest, x509cr *x509.CertificateRequest) bool { - if !reflect.DeepEqual([]string{"system:nodes"}, x509cr.Subject.Organization) { - return false - } - if len(x509cr.DNSNames) == 0 || len(x509cr.IPAddresses) == 0 { - return false - } - if !hasExactUsages(csr, kubeletServerUsages) { - return false - } - if !strings.HasPrefix(x509cr.Subject.CommonName, "system:node:") { - return false - } - if csr.Spec.Username != x509cr.Subject.CommonName { - return false - } - return true -} diff --git a/pkg/controller/certificates/approver/sarapprove_test.go b/pkg/controller/certificates/approver/sarapprove_test.go index f17b9cebec464..40dcc316d4335 100644 --- a/pkg/controller/certificates/approver/sarapprove_test.go +++ b/pkg/controller/certificates/approver/sarapprove_test.go @@ -187,77 +187,6 @@ func TestHandle(t *testing.T) { } } -func TestSelfNodeServerCertRecognizer(t *testing.T) { - defaultCSR := csrBuilder{ - cn: "system:node:foo", - orgs: []string{"system:nodes"}, - requestor: "system:node:foo", - usages: []capi.KeyUsage{ - capi.UsageKeyEncipherment, - capi.UsageDigitalSignature, - capi.UsageServerAuth, - }, - dns: []string{"node"}, - ips: []net.IP{net.ParseIP("192.168.0.1")}, - } - - testCases := []struct { - description string - csrBuilder csrBuilder - expectedOutcome bool - }{ - { - description: "Success - all requirements met", - csrBuilder: defaultCSR, - expectedOutcome: true, - }, - { - description: "No organization", - csrBuilder: func(b csrBuilder) csrBuilder { - b.orgs = []string{} - return b - }(defaultCSR), - expectedOutcome: false, - }, - { - description: "Wrong organization", - csrBuilder: func(b csrBuilder) csrBuilder { - b.orgs = append(b.orgs, "new-org") - return b - }(defaultCSR), - expectedOutcome: false, - }, - { - description: "Wrong usages", - csrBuilder: func(b csrBuilder) csrBuilder { - b.usages = []capi.KeyUsage{} - return b - }(defaultCSR), - expectedOutcome: false, - }, - { - description: "Wrong common name", - csrBuilder: func(b csrBuilder) csrBuilder { - b.cn = "wrong-common-name" - return b - }(defaultCSR), - expectedOutcome: false, - }, - } - for _, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - csr := makeFancyTestCsr(tc.csrBuilder) - x509cr, err := k8s_certificates_v1beta1.ParseCSR(csr) - if err != nil { - t.Errorf("unexpected err: %v", err) - } - if isSelfNodeServerCert(csr, x509cr) != tc.expectedOutcome { - t.Errorf("expected recognized to be %v", tc.expectedOutcome) - } - }) - } -} - func TestRecognizers(t *testing.T) { goodCases := []func(b *csrBuilder){ func(b *csrBuilder) { diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go index a870ea13ade52..7f1613fc25e9b 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go @@ -501,16 +501,6 @@ func ClusterRoles() []rbac.ClusterRole { }, } - if utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletServerCertificate) { - roles = append(roles, rbac.ClusterRole{ - // a role making the csrapprover controller approve a node server CSR requested by the node itself - ObjectMeta: metav1.ObjectMeta{Name: "system:certificates.k8s.io:certificatesigningrequests:selfnodeserver"}, - Rules: []rbac.PolicyRule{ - rbac.NewRule("create").Groups(certificatesGroup).Resources("certificatesigningrequests/selfnodeserver").RuleOrDie(), - }, - }) - } - if utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { roles = append(roles, rbac.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: "system:volume-scheduler"},