refactor certificate controller to break it into two parts

cmd/gke-certificates-controller/app/gke_certificates_controller.go

@@ -72,16 +72,15 @@ func Run(s *GKECertificatesController) error {
 
 	sharedInformers := informers.NewSharedInformerFactory(client, time.Duration(12)*time.Hour)
 
-	signer, err := NewGKESigner(s.ClusterSigningGKEKubeconfig, s.ClusterSigningGKERetryBackoff.Duration, recorder)
+	signer, err := NewGKESigner(s.ClusterSigningGKEKubeconfig, s.ClusterSigningGKERetryBackoff.Duration, recorder, client)
 	if err != nil {
 		return err
 	}
 
 	controller, err := certificates.NewCertificateController(
 		client,
 		sharedInformers.Certificates().V1beta1().CertificateSigningRequests(),
-		signer,
-		certificates.NewGroupApprover(s.ApproveAllKubeletCSRsForGroup),
+		signer.handle,
 	)
 	if err != nil {
 		return err

cmd/gke-certificates-controller/app/gke_signer.go

@@ -29,11 +29,13 @@ import (
 	"k8s.io/client-go/tools/record"
 	"k8s.io/kubernetes/pkg/api"
 	_ "k8s.io/kubernetes/pkg/apis/certificates/install"
-	certificates "k8s.io/kubernetes/pkg/apis/certificates/v1beta1"
+	capi "k8s.io/kubernetes/pkg/apis/certificates/v1beta1"
+	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
+	"k8s.io/kubernetes/pkg/controller/certificates"
 )
 
 var (
-	groupVersions = []schema.GroupVersion{certificates.SchemeGroupVersion}
+	groupVersions = []schema.GroupVersion{capi.SchemeGroupVersion}
 )
 
 // GKESigner uses external calls to GKE in order to sign certificate signing
@@ -43,10 +45,11 @@ type GKESigner struct {
 	kubeConfigFile string
 	retryBackoff   time.Duration
 	recorder       record.EventRecorder
+	client         clientset.Interface
 }
 
 // NewGKESigner will create a new instance of a GKESigner.
-func NewGKESigner(kubeConfigFile string, retryBackoff time.Duration, recorder record.EventRecorder) (*GKESigner, error) {
+func NewGKESigner(kubeConfigFile string, retryBackoff time.Duration, recorder record.EventRecorder, client clientset.Interface) (*GKESigner, error) {
 	webhook, err := webhook.NewGenericWebhook(api.Registry, api.Codecs, kubeConfigFile, groupVersions, retryBackoff)
 	if err != nil {
 		return nil, err
@@ -57,13 +60,29 @@ func NewGKESigner(kubeConfigFile string, retryBackoff time.Duration, recorder re
 		kubeConfigFile: kubeConfigFile,
 		retryBackoff:   retryBackoff,
 		recorder:       recorder,
+		client:         client,
 	}, nil
 }
 
+func (s *GKESigner) handle(csr *capi.CertificateSigningRequest) error {
+	if !certificates.IsCertificateRequestApproved(csr) {
+		return nil
+	}
+	csr, err := s.sign(csr)
+	if err != nil {
+		return fmt.Errorf("error auto signing csr: %v", err)
+	}
+	_, err = s.client.Certificates().CertificateSigningRequests().UpdateStatus(csr)
+	if err != nil {
+		return fmt.Errorf("error updating signature for csr: %v", err)
+	}
+	return nil
+}
+
 // Sign will make an external call to GKE order to sign the given
-// *certificates.CertificateSigningRequest, using the GKESigner's
+// *capi.CertificateSigningRequest, using the GKESigner's
 // kubeConfigFile.
-func (s *GKESigner) Sign(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, error) {
+func (s *GKESigner) sign(csr *capi.CertificateSigningRequest) (*capi.CertificateSigningRequest, error) {
 	result := s.webhook.WithExponentialBackoff(func() rest.Result {
 		return s.webhook.RestClient.Post().Body(csr).Do()
 	})
@@ -80,7 +99,7 @@ func (s *GKESigner) Sign(csr *certificates.CertificateSigningRequest) (*certific
 		return nil, s.webhookError(csr, fmt.Errorf("received unsuccessful response code from webhook: %d", statusCode))
 	}
 
-	result_csr := &certificates.CertificateSigningRequest{}
+	result_csr := &capi.CertificateSigningRequest{}
 
 	if err := result.Into(result_csr); err != nil {
 		return nil, s.webhookError(result_csr, err)
@@ -91,7 +110,7 @@ func (s *GKESigner) Sign(csr *certificates.CertificateSigningRequest) (*certific
 	return csr, nil
 }
 
-func (s *GKESigner) webhookError(csr *certificates.CertificateSigningRequest, err error) error {
+func (s *GKESigner) webhookError(csr *capi.CertificateSigningRequest, err error) error {
 	glog.V(2).Infof("error contacting webhook backend: %s", err)
 	s.recorder.Eventf(csr, "Warning", "SigningError", "error while calling GKE: %v", err)
 	return err

cmd/gke-certificates-controller/app/gke_signer_test.go

@@ -104,12 +104,12 @@ func TestGKESigner(t *testing.T) {
 			t.Fatalf("error closing kubeconfig template: %v", err)
 		}
 
-		signer, err := NewGKESigner(kubeConfig.Name(), time.Duration(500)*time.Millisecond, record.NewFakeRecorder(10))
+		signer, err := NewGKESigner(kubeConfig.Name(), time.Duration(500)*time.Millisecond, record.NewFakeRecorder(10), nil)
 		if err != nil {
 			t.Fatalf("error creating GKESigner: %v", err)
 		}
 
-		cert, err := signer.Sign(&certificates.CertificateSigningRequest{})
+		cert, err := signer.sign(&certificates.CertificateSigningRequest{})
 
 		if c.wantErr {
 			if err == nil {

cmd/kube-controller-manager/app/BUILD

@@ -44,7 +44,8 @@ go_library(
         "//pkg/cloudprovider/providers/vsphere:go_default_library",
         "//pkg/controller:go_default_library",
         "//pkg/controller/bootstrap:go_default_library",
-        "//pkg/controller/certificates:go_default_library",
+        "//pkg/controller/certificates/approver:go_default_library",
+        "//pkg/controller/certificates/signer:go_default_library",
         "//pkg/controller/cronjob:go_default_library",
         "//pkg/controller/daemon:go_default_library",
         "//pkg/controller/deployment:go_default_library",

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

@@ -24,33 +24,55 @@ import (
 	"github.com/golang/glog"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	certcontroller "k8s.io/kubernetes/pkg/controller/certificates"
+	"k8s.io/kubernetes/pkg/controller/certificates/approver"
+	"k8s.io/kubernetes/pkg/controller/certificates/signer"
 )
 
-func startCSRController(ctx ControllerContext) (bool, error) {
+func startCSRSigningController(ctx ControllerContext) (bool, error) {
 	if !ctx.AvailableResources[schema.GroupVersionResource{Group: "certificates.k8s.io", Version: "v1beta1", Resource: "certificatesigningrequests"}] {
 		return false, nil
 	}
+	if ctx.Options.ClusterSigningCertFile == "" || ctx.Options.ClusterSigningKeyFile == "" {
+		return false, nil
+	}
 	c := ctx.ClientBuilder.ClientOrDie("certificate-controller")
 
-	signer, err := certcontroller.NewCFSSLSigner(ctx.Options.ClusterSigningCertFile, ctx.Options.ClusterSigningKeyFile)
+	signer, err := signer.NewCSRSigningController(
+		c,
+		ctx.InformerFactory.Certificates().V1beta1().CertificateSigningRequests(),
+		ctx.Options.ClusterSigningCertFile,
+		ctx.Options.ClusterSigningKeyFile,
+	)
 	if err != nil {
 		glog.Errorf("Failed to start certificate controller: %v", err)
 		return false, nil
 	}
+	go signer.Run(1, ctx.Stop)
+
+	return true, nil
+}
+
+func startCSRApprovingController(ctx ControllerContext) (bool, error) {
+	if !ctx.AvailableResources[schema.GroupVersionResource{Group: "certificates.k8s.io", Version: "v1beta1", Resource: "certificatesigningrequests"}] {
+		return false, nil
+	}
+	if ctx.Options.ApproveAllKubeletCSRsForGroup == "" {
+		return false, nil
+	}
+	c := ctx.ClientBuilder.ClientOrDie("certificate-controller")
 
-	certController, err := certcontroller.NewCertificateController(
+	approver, err := approver.NewCSRApprovingController(
 		c,
 		ctx.InformerFactory.Certificates().V1beta1().CertificateSigningRequests(),
-		signer,
-		certcontroller.NewGroupApprover(ctx.Options.ApproveAllKubeletCSRsForGroup),
+		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
 		// cause a crash loop
 		glog.Errorf("Failed to start certificate controller: %v", err)
 		return false, nil
 	}
-	go certController.Run(1, ctx.Stop)
+	go approver.Run(1, ctx.Stop)
+
 	return true, nil
 }

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

@@ -309,7 +309,8 @@ func NewControllerInitializers() map[string]InitFunc {
 	controllers["disruption"] = startDisruptionController
 	controllers["statefulset"] = startStatefulSetController
 	controllers["cronjob"] = startCronJobController
-	controllers["certificatesigningrequests"] = startCSRController
+	controllers["csrsigning"] = startCSRSigningController
+	controllers["csrapproving"] = startCSRApprovingController
 	controllers["ttl"] = startTTLController
 	controllers["bootstrapsigner"] = startBootstrapSignerController
 	controllers["tokencleaner"] = startTokenCleanerController

pkg/controller/certificates/BUILD

@@ -1,5 +1,3 @@
-package(default_visibility = ["//visibility:public"])
-
 licenses(["notice"])
 
 load(
@@ -13,21 +11,18 @@ go_library(
     srcs = [
         "certificate_controller.go",
         "certificate_controller_utils.go",
-        "cfssl_signer.go",
-        "doc.go",
-        "groupapprove.go",
     ],
     tags = ["automanaged"],
+    visibility = [
+        ":__subpackages__",
+        "//cmd/gke-certificates-controller:__subpackages__",
+    ],
     deps = [
         "//pkg/apis/certificates/v1beta1:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/client/informers/informers_generated/externalversions/certificates/v1beta1:go_default_library",
         "//pkg/client/listers/certificates/v1beta1:go_default_library",
         "//pkg/controller:go_default_library",
-        "//vendor/github.com/cloudflare/cfssl/config:go_default_library",
-        "//vendor/github.com/cloudflare/cfssl/helpers:go_default_library",
-        "//vendor/github.com/cloudflare/cfssl/signer:go_default_library",
-        "//vendor/github.com/cloudflare/cfssl/signer/local:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
@@ -49,22 +44,20 @@ filegroup(
 
 filegroup(
     name = "all-srcs",
-    srcs = [":package-srcs"],
+    srcs = [
+        ":package-srcs",
+        "//pkg/controller/certificates/approver:all-srcs",
+        "//pkg/controller/certificates/signer:all-srcs",
+    ],
     tags = ["automanaged"],
+    visibility = [
+        "//pkg/controller:__pkg__",
+    ],
 )
 
 go_test(
     name = "go_default_test",
-    srcs = [
-        "certificate_controller_test.go",
-        "cfssl_signer_test.go",
-        "groupapprove_test.go",
-    ],
-    data = [
-        "testdata/ca.crt",
-        "testdata/ca.key",
-        "testdata/kubelet.csr",
-    ],
+    srcs = ["certificate_controller_test.go"],
     library = ":go_default_library",
     tags = ["automanaged"],
     deps = [
@@ -73,10 +66,5 @@ go_test(
         "//pkg/client/informers/informers_generated/externalversions:go_default_library",
         "//pkg/controller:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
-        "//vendor/k8s.io/client-go/tools/cache:go_default_library",
-        "//vendor/k8s.io/client-go/util/cert:go_default_library",
-        "//vendor/k8s.io/client-go/util/cert/triple:go_default_library",
     ],
 )

pkg/controller/certificates/approver/BUILD

@@ -0,0 +1,43 @@
+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+load(
+    "@io_bazel_rules_go//go:def.bzl",
+    "go_library",
+    "go_test",
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["groupapprove_test.go"],
+    library = ":go_default_library",
+    tags = ["automanaged"],
+    deps = ["//pkg/apis/certificates/v1beta1:go_default_library"],
+)
+
+go_library(
+    name = "go_default_library",
+    srcs = ["groupapprove.go"],
+    tags = ["automanaged"],
+    deps = [
+        "//pkg/apis/certificates/v1beta1:go_default_library",
+        "//pkg/client/clientset_generated/clientset:go_default_library",
+        "//pkg/client/informers/informers_generated/externalversions/certificates/v1beta1:go_default_library",
+        "//pkg/controller/certificates:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+)