Refactor certificate controller to make approval an interface

cmd/kube-controller-manager/app/controllermanager.go

@@ -523,12 +523,13 @@ func StartControllers(s *options.CMServer, rootClientBuilder, clientBuilder cont
 	if availableResources[schema.GroupVersionResource{Group: "certificates.k8s.io", Version: "v1alpha1", Resource: "certificatesigningrequests"}] {
 		glog.Infof("Starting certificate request controller")
 		resyncPeriod := ResyncPeriod(s)()
+		c := clientBuilder.ClientOrDie("certificate-controller")
 		certController, err := certcontroller.NewCertificateController(
-			clientBuilder.ClientOrDie("certificate-controller"),
+			c,
 			resyncPeriod,
 			s.ClusterSigningCertFile,
 			s.ClusterSigningKeyFile,
-			s.ApproveAllKubeletCSRsForGroup,
+			certcontroller.NewGroupApprover(c.Certificates().CertificateSigningRequests(), s.ApproveAllKubeletCSRsForGroup),
 		)
 		if err != nil {
 			glog.Errorf("Failed to start certificate controller: %v", err)

pkg/controller/certificates/BUILD

@@ -16,6 +16,7 @@ go_library(
         "controller.go",
         "controller_utils.go",
         "doc.go",
+        "groupapprove.go",
     ],
     tags = ["automanaged"],
     deps = [
@@ -23,6 +24,7 @@ go_library(
         "//pkg/apis/certificates/v1alpha1:go_default_library",
         "//pkg/client/cache:go_default_library",
         "//pkg/client/clientset_generated/release_1_5:go_default_library",
+        "//pkg/client/clientset_generated/release_1_5/typed/certificates/v1alpha1:go_default_library",
         "//pkg/client/clientset_generated/release_1_5/typed/core/v1:go_default_library",
         "//pkg/client/record:go_default_library",
         "//pkg/controller:go_default_library",

pkg/controller/certificates/controller.go

@@ -18,8 +18,6 @@ package certificates
 
 import (
 	"fmt"
-	"reflect"
-	"strings"
 	"time"
 
 	"k8s.io/kubernetes/pkg/api/v1"
@@ -30,7 +28,6 @@ import (
 	"k8s.io/kubernetes/pkg/client/record"
 	"k8s.io/kubernetes/pkg/controller"
 	"k8s.io/kubernetes/pkg/runtime"
-	certutil "k8s.io/kubernetes/pkg/util/cert"
 	utilruntime "k8s.io/kubernetes/pkg/util/runtime"
 	"k8s.io/kubernetes/pkg/util/wait"
 	"k8s.io/kubernetes/pkg/util/workqueue"
@@ -42,6 +39,10 @@ import (
 	"github.com/golang/glog"
 )
 
+type AutoApprover interface {
+	AutoApprove(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, error)
+}
+
 type CertificateController struct {
 	kubeClient clientset.Interface
 
@@ -51,14 +52,14 @@ type CertificateController struct {
 
 	syncHandler func(csrKey string) error
 
-	approveAllKubeletCSRsForGroup string
+	approver AutoApprover
 
 	signer *local.Signer
 
 	queue workqueue.RateLimitingInterface
 }
 
-func NewCertificateController(kubeClient clientset.Interface, syncPeriod time.Duration, caCertFile, caKeyFile string, approveAllKubeletCSRsForGroup string) (*CertificateController, error) {
+func NewCertificateController(kubeClient clientset.Interface, syncPeriod time.Duration, caCertFile, caKeyFile string, approver AutoApprover) (*CertificateController, error) {
 	// Send events to the apiserver
 	eventBroadcaster := record.NewBroadcaster()
 	eventBroadcaster.StartLogging(glog.Infof)
@@ -78,7 +79,7 @@ func NewCertificateController(kubeClient clientset.Interface, syncPeriod time.Du
 		kubeClient: kubeClient,
 		queue:      workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "certificate"),
 		signer:     ca,
-		approveAllKubeletCSRsForGroup: approveAllKubeletCSRsForGroup,
+		approver:   approver,
 	}
 
 	// Manage the addition/update of certificate requests
@@ -195,10 +196,12 @@ func (cc *CertificateController) maybeSignCertificate(key string) error {
 	}
 	csr := obj.(*certificates.CertificateSigningRequest)
 
-	csr, err = cc.maybeAutoApproveCSR(csr)
+	if cc.approver != nil {
+		csr, err = cc.approver.AutoApprove(csr)
 		if err != nil {
 			return fmt.Errorf("error auto approving csr: %v", err)
 		}
+	}
 
 	// At this point, the controller needs to:
 	// 1. Check the approval conditions
@@ -218,47 +221,3 @@ func (cc *CertificateController) maybeSignCertificate(key string) error {
 	_, err = cc.kubeClient.Certificates().CertificateSigningRequests().UpdateStatus(csr)
 	return err
 }
-
-func (cc *CertificateController) maybeAutoApproveCSR(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, error) {
-	// short-circuit if we're not auto-approving
-	if cc.approveAllKubeletCSRsForGroup == "" {
-		return csr, nil
-	}
-	// short-circuit if we're already approved or denied
-	if approved, denied := getCertApprovalCondition(&csr.Status); approved || denied {
-		return csr, nil
-	}
-
-	isKubeletBootstrapGroup := false
-	for _, g := range csr.Spec.Groups {
-		if g == cc.approveAllKubeletCSRsForGroup {
-			isKubeletBootstrapGroup = true
-			break
-		}
-	}
-	if !isKubeletBootstrapGroup {
-		return csr, nil
-	}
-
-	x509cr, err := certutil.ParseCSRV1alpha1(csr)
-	if err != nil {
-		utilruntime.HandleError(fmt.Errorf("unable to parse csr %q: %v", csr.Name, err))
-		return csr, nil
-	}
-	if !reflect.DeepEqual([]string{"system:nodes"}, x509cr.Subject.Organization) {
-		return csr, nil
-	}
-	if !strings.HasPrefix(x509cr.Subject.CommonName, "system:node:") {
-		return csr, nil
-	}
-	if len(x509cr.DNSNames)+len(x509cr.EmailAddresses)+len(x509cr.IPAddresses) != 0 {
-		return csr, nil
-	}
-
-	csr.Status.Conditions = append(csr.Status.Conditions, certificates.CertificateSigningRequestCondition{
-		Type:    certificates.CertificateApproved,
-		Reason:  "AutoApproved",
-		Message: "Auto approving of all kubelet CSRs is enabled on the controller manager",
-	})
-	return cc.kubeClient.Certificates().CertificateSigningRequests().UpdateApproval(csr)
-}

pkg/controller/certificates/groupapprove.go

@@ -0,0 +1,86 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package certificates
+
+import (
+	"fmt"
+	"reflect"
+	"strings"
+
+	certificates "k8s.io/kubernetes/pkg/apis/certificates/v1alpha1"
+	clientcertificates "k8s.io/kubernetes/pkg/client/clientset_generated/release_1_5/typed/certificates/v1alpha1"
+	certutil "k8s.io/kubernetes/pkg/util/cert"
+	utilruntime "k8s.io/kubernetes/pkg/util/runtime"
+)
+
+// 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 {
+	return &groupApprover{
+		client: client,
+		approveAllKubeletCSRsForGroup: approveAllKubeletCSRsForGroup,
+	}
+}
+
+func (cc *groupApprover) AutoApprove(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, error) {
+	// short-circuit if we're not auto-approving
+	if cc.approveAllKubeletCSRsForGroup == "" {
+		return csr, nil
+	}
+	// short-circuit if we're already approved or denied
+	if approved, denied := getCertApprovalCondition(&csr.Status); approved || denied {
+		return csr, nil
+	}
+
+	isKubeletBootstrapGroup := false
+	for _, g := range csr.Spec.Groups {
+		if g == cc.approveAllKubeletCSRsForGroup {
+			isKubeletBootstrapGroup = true
+			break
+		}
+	}
+	if !isKubeletBootstrapGroup {
+		return csr, nil
+	}
+
+	x509cr, err := certutil.ParseCSRV1alpha1(csr)
+	if err != nil {
+		utilruntime.HandleError(fmt.Errorf("unable to parse csr %q: %v", csr.Name, err))
+		return csr, nil
+	}
+	if !reflect.DeepEqual([]string{"system:nodes"}, x509cr.Subject.Organization) {
+		return csr, nil
+	}
+	if !strings.HasPrefix(x509cr.Subject.CommonName, "system:node:") {
+		return csr, nil
+	}
+	if len(x509cr.DNSNames)+len(x509cr.EmailAddresses)+len(x509cr.IPAddresses) != 0 {
+		return csr, nil
+	}
+
+	csr.Status.Conditions = append(csr.Status.Conditions, certificates.CertificateSigningRequestCondition{
+		Type:    certificates.CertificateApproved,
+		Reason:  "AutoApproved",
+		Message: "Auto approving of all kubelet CSRs is enabled on the controller manager",
+	})
+	return cc.client.UpdateApproval(csr)
+}