Merge pull request #34 from jetstack/approval-condition

Add support for cert-manager v1.3 Approval conditions

cmd/root.go

@@ -50,6 +50,7 @@ func init() {
 	rootCmd.PersistentFlags().String("metrics-addr", ":8080", "The address the metric endpoint binds to.")
 	rootCmd.PersistentFlags().Bool("enable-leader-election", false, "Enable leader election for controller manager.")
 	rootCmd.PersistentFlags().String("cluster-resource-namespace", "cert-manager", "The namespace for secrets in which cluster-scoped resources are found.")
+	rootCmd.PersistentFlags().Bool("disable-approval-check", false, "Don't check whether a CertificateRequest is approved before signing. For compatibility with cert-manager <v1.3.0.")
 
 	// Zap flags
 	rootCmd.PersistentFlags().Bool("zap-devel", false, "Zap Development Mode defaults(encoder=consoleEncoder,logLevel=Debug,stackTraceLevel=Warn).")

config/rbac/cert_manager_controller_approver_clusterrole.yaml

@@ -0,0 +1,14 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: cert-manager-controller-approve:casissuer
+rules:
+- apiGroups:
+  - cert-manager.io
+  resources:
+  - signers
+  verbs:
+  - approve
+  resourceNames:
+  - googlecasclusterissuers.cas-issuer.jetstack.io/*
+  - googlecasissuers.cas-issuer.jetstack.io/*

config/rbac/cert_manager_controller_approver_clusterrolebinding.yaml

@@ -0,0 +1,12 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: cert-manager-controller-approve:casissuer
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: cert-manager-controller-approve:casissuer
+subjects:
+- kind: ServiceAccount
+  name: cert-manager
+  namespace: cert-manager

config/rbac/kustomization.yaml

@@ -4,7 +4,14 @@ resources:
 - leader_election_role.yaml
 - leader_election_role_binding.yaml
 - serviceaccount.yaml
-# Comment the following 4 lines if you want to disable
+
+# Comment the following 2 lines if you don't wish for the internal cert-manager
+# approver to approve all Google CAS Issuer CertificateRequests by default.
+
+- cert_manager_controller_approver_clusterrole.yaml
+- cert_manager_controller_approver_clusterrolebinding.yaml
+
+# Uncomment the following 4 lines if you want to disable
 # the auth proxy (https://github.com/brancz/kube-rbac-proxy)
 # which protects your /metrics endpoint.
 #- auth_proxy_service.yaml

go.mod

@@ -1,18 +1,18 @@
 module github.com/jetstack/google-cas-issuer
 
-go 1.13
+go 1.16
 
 require (
 	cloud.google.com/go v0.71.0
 	github.com/go-logr/logr v0.3.0
 	github.com/golang/protobuf v1.4.3
 	github.com/google/uuid v1.1.2
-	github.com/jetstack/cert-manager v1.0.4
+	github.com/jetstack/cert-manager v1.3.1
 	github.com/onsi/ginkgo v1.14.1
 	github.com/onsi/gomega v1.10.2
 	github.com/spf13/cobra v1.1.1
 	github.com/spf13/viper v1.7.0
-	github.com/stretchr/testify v1.6.1 // indirect
+	github.com/stretchr/testify v1.6.1
 	google.golang.org/api v0.34.0
 	google.golang.org/genproto v0.0.0-20201110150050-8816d57aaa9a
 	k8s.io/api v0.20.1

pkg/controller/certificaterequest/certificaterequest_controller.go

@@ -45,6 +45,7 @@ const (
 	reasonSignerNotReady = "SignerNotReady"
 	reasonCRInvalid      = "CRInvalid"
 	reasonCertIssued     = "CertificateIssued"
+	reasonCRNotApproved  = "CRNotApproved"
 )
 
 // CertificateRequestReconciler reconciles CRs
@@ -78,12 +79,28 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 		return ctrl.Result{}, nil
 	}
 
-	// Ignore already Ready CRs
+	// Ignore CRs that have reached a terminal state
 	if cmutil.CertificateRequestHasCondition(&certificateRequest, cmapi.CertificateRequestCondition{
 		Type:   cmapi.CertificateRequestConditionReady,
 		Status: cmmeta.ConditionTrue,
 	}) {
-		log.Info("CertificateRequest is Ready, Ignoring.", "certificaterequest", req.NamespacedName)
+		log.Info("CertificateRequest is Ready, ignoring.", "cr", req.NamespacedName)
+		return ctrl.Result{}, nil
+	}
+	if cmutil.CertificateRequestHasCondition(&certificateRequest, cmapi.CertificateRequestCondition{
+		Type:   cmapi.CertificateRequestConditionReady,
+		Status: cmmeta.ConditionFalse,
+		Reason: cmapi.CertificateRequestReasonDenied,
+	}) {
+		log.Info("CertificateRequest has been denied, ignoring.", "cr", req.NamespacedName)
+		return ctrl.Result{}, nil
+	}
+	if cmutil.CertificateRequestHasCondition(&certificateRequest, cmapi.CertificateRequestCondition{
+		Type:   cmapi.CertificateRequestConditionReady,
+		Status: cmmeta.ConditionFalse,
+		Reason: cmapi.CertificateRequestReasonFailed,
+	}) {
+		log.Info("CertificateRequest has failed, ignoring.", "cr", req.NamespacedName)
 		return ctrl.Result{}, nil
 	}
 
@@ -111,6 +128,25 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 		}
 	}()
 
+	// Explicitly fail if the certificate request has been denied
+	if cmutil.CertificateRequestIsDenied(&certificateRequest) {
+		msg := "certificate request has been denied, not signing"
+		log.Info(msg, "cr", req.NamespacedName)
+		setReadyCondition(cmmeta.ConditionFalse, cmapi.CertificateRequestReasonDenied, msg)
+		return ctrl.Result{}, nil
+	}
+
+	// From cert-manager v1.3 onwards, CertificateRequests must be approved before they are signed.
+	if !viper.GetBool("disable-approval-check") {
+		log.Info("Checking whether CR has been approved", "cr", req.NamespacedName)
+		if !cmutil.CertificateRequestIsApproved(&certificateRequest){
+			msg := "certificate request is not approved yet"
+			log.Info(msg, "cr", req.NamespacedName)
+			r.Recorder.Event(&certificateRequest, eventTypeWarning, reasonCRNotApproved, msg)
+			return ctrl.Result{}, nil
+		}
+	}
+
 	// Add a Ready condition if one does not already exist
 	if ready := cmutil.GetCertificateRequestCondition(&certificateRequest, cmapi.CertificateRequestConditionReady); ready == nil {
 		log.Info("Initialising Ready condition")
@@ -180,7 +216,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 	}
 
-	// Check for obvious errors, e.g. missing duration, malformed certificte request
+	// Check for obvious errors, e.g. missing duration, malformed certificate request
 	if err := sanitiseCertificateRequestSpec(&certificateRequest.Spec); err != nil {
 		log.Error(err, "certificate request has issues", "cr", req.NamespacedName)
 		msg := "certificate request has issues: " + err.Error()