diff --git a/cmd/kube-controller-manager/app/certificates.go b/cmd/kube-controller-manager/app/certificates.go index 760d1efd91b..58d7f265304 100644 --- a/cmd/kube-controller-manager/app/certificates.go +++ b/cmd/kube-controller-manager/app/certificates.go @@ -38,7 +38,7 @@ func startCSRController(ctx ControllerContext) (bool, error) { resyncPeriod, ctx.Options.ClusterSigningCertFile, ctx.Options.ClusterSigningKeyFile, - certcontroller.NewGroupApprover(c.Certificates().CertificateSigningRequests(), ctx.Options.ApproveAllKubeletCSRsForGroup), + certcontroller.NewGroupApprover(ctx.Options.ApproveAllKubeletCSRsForGroup), ) if err != nil { // TODO this is failing consistently in test-cmd and local-up-cluster.sh. Fix them and make it consistent with all others which diff --git a/pkg/controller/certificates/BUILD b/pkg/controller/certificates/BUILD index 54df3e450ca..6113f7815b8 100644 --- a/pkg/controller/certificates/BUILD +++ b/pkg/controller/certificates/BUILD @@ -21,7 +21,6 @@ go_library( deps = [ "//pkg/apis/certificates/v1beta1:go_default_library", "//pkg/client/clientset_generated/clientset:go_default_library", - "//pkg/client/clientset_generated/clientset/typed/certificates/v1beta1:go_default_library", "//pkg/client/legacylisters:go_default_library", "//pkg/controller:go_default_library", "//vendor:github.com/cloudflare/cfssl/config", diff --git a/pkg/controller/certificates/certificate_controller.go b/pkg/controller/certificates/certificate_controller.go index 94de94f37b4..f30e3f98faf 100644 --- a/pkg/controller/certificates/certificate_controller.go +++ b/pkg/controller/certificates/certificate_controller.go @@ -37,12 +37,16 @@ import ( "github.com/golang/glog" ) +// err returned from these interfaces should indicate utter failure that +// should be retried. "Buisness logic" errors should be indicated by adding +// a condition to the CSRs status, not by returning an error. + type AutoApprover interface { AutoApprove(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, error) } type Signer interface { - Sign(csr *certificates.CertificateSigningRequest) ([]byte, error) + Sign(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, error) } type CertificateController struct { @@ -197,6 +201,10 @@ func (cc *CertificateController) maybeSignCertificate(key string) error { if err != nil { return fmt.Errorf("error auto approving csr: %v", err) } + _, err = cc.kubeClient.Certificates().CertificateSigningRequests().UpdateApproval(csr) + if err != nil { + return fmt.Errorf("error updating approval for csr: %v", err) + } } // At this point, the controller needs to: @@ -204,14 +212,16 @@ func (cc *CertificateController) maybeSignCertificate(key string) error { // 2. Generate a signed certificate // 3. Update the Status subresource - if csr.Status.Certificate == nil && IsCertificateRequestApproved(csr) { - certBytes, err := cc.signer.Sign(csr) + if cc.signer != nil && csr.Status.Certificate == nil && IsCertificateRequestApproved(csr) { + csr, err := cc.signer.Sign(csr) if err != nil { - return err + return fmt.Errorf("error auto signing csr: %v", err) + } + _, err = cc.kubeClient.Certificates().CertificateSigningRequests().UpdateStatus(csr) + if err != nil { + return fmt.Errorf("error updating signature for csr: %v", err) } - csr.Status.Certificate = certBytes } - _, err = cc.kubeClient.Certificates().CertificateSigningRequests().UpdateStatus(csr) - return err + return nil } diff --git a/pkg/controller/certificates/cfssl_signer.go b/pkg/controller/certificates/cfssl_signer.go index 1670a4cb9ac..35cf4f9d23f 100644 --- a/pkg/controller/certificates/cfssl_signer.go +++ b/pkg/controller/certificates/cfssl_signer.go @@ -77,7 +77,7 @@ func NewCFSSLSigner(caFile, caKeyFile string) (*CFSSLSigner, error) { }, nil } -func (cs *CFSSLSigner) Sign(csr *certificates.CertificateSigningRequest) ([]byte, error) { +func (cs *CFSSLSigner) Sign(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, error) { var usages []string for _, usage := range csr.Spec.Usages { usages = append(usages, string(usage)) @@ -93,7 +93,13 @@ func (cs *CFSSLSigner) Sign(csr *certificates.CertificateSigningRequest) ([]byte if err != nil { return nil, err } - return s.Sign(signer.SignRequest{ + + csr.Status.Certificate, err = s.Sign(signer.SignRequest{ Request: string(csr.Spec.Request), }) + if err != nil { + return nil, err + } + + return csr, nil } diff --git a/pkg/controller/certificates/cfssl_signer_test.go b/pkg/controller/certificates/cfssl_signer_test.go index 81d95b9edaf..56300f69e0f 100644 --- a/pkg/controller/certificates/cfssl_signer_test.go +++ b/pkg/controller/certificates/cfssl_signer_test.go @@ -49,10 +49,11 @@ func TestSigner(t *testing.T) { }, } - certData, err := s.Sign(csr) + csr, err = s.Sign(csr) if err != nil { t.Fatalf("failed to sign CSR: %v", err) } + certData := csr.Status.Certificate if len(certData) == 0 { t.Fatalf("expected a certificate after signing") } diff --git a/pkg/controller/certificates/groupapprove.go b/pkg/controller/certificates/groupapprove.go index b2a04585c6b..2dabb5a04d4 100644 --- a/pkg/controller/certificates/groupapprove.go +++ b/pkg/controller/certificates/groupapprove.go @@ -23,19 +23,16 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" certificates "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" - clientcertificates "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/typed/certificates/v1beta1" ) // groupApprover implements AutoApprover for signing Kubelet certificates. type groupApprover struct { - client clientcertificates.CertificateSigningRequestInterface approveAllKubeletCSRsForGroup string } // NewGroupApprover creates an approver that accepts any CSR requests where the subject group contains approveAllKubeletCSRsForGroup. -func NewGroupApprover(client clientcertificates.CertificateSigningRequestInterface, approveAllKubeletCSRsForGroup string) AutoApprover { +func NewGroupApprover(approveAllKubeletCSRsForGroup string) AutoApprover { return &groupApprover{ - client: client, approveAllKubeletCSRsForGroup: approveAllKubeletCSRsForGroup, } } @@ -84,7 +81,7 @@ func (cc *groupApprover) AutoApprove(csr *certificates.CertificateSigningRequest Reason: "AutoApproved", Message: "Auto approving of all kubelet CSRs is enabled on the controller manager", }) - return cc.client.UpdateApproval(csr) + return csr, nil } var kubeletClientUsages = []certificates.KeyUsage{