Merge pull request #18 from JoshVanL/cert-manager-v1.3

Author: maruina

.golangci.yml

@@ -74,3 +74,7 @@ linters:
     - unused
     - varcheck
     - whitespace
+issues:
+  # Exclude scope checks in tests: "Using the variable on range scope `test` in function literal"
+  exclude:
+     - Using the variable on range scope `test` in function literal

README.md

@@ -170,6 +170,14 @@ A KMSIssuer resource configures a new [Cert-Manager external issuer](https://cer
 | spec.duration    | duration                                                                                                                 | Certificate default Duration. (optional, default=26280h aka 3 years)                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                          |
 | spec.renewBefore | duration                                                                                                                 | The amount of time before the certificate’s notAfter time that the issuer will begin to attempt to renew the certificate. If this value is greater than the total duration of the certificate (i.e. notAfter - notBefore), it will be automatically renewed 2/3rds of the way through the certificate’s duration. <br> <br> The `NotBefore` field on the certificate is set to the current time rounded down by the renewal interval. For example, if the certificate is renewed every hour, the `NotBefore` field is set to the beggining of the hour. If the certificate is renewed every day, the `NotBefore` field is set to the beggining of the day. This allows the generation of consistent certificates regardless of when it has been generated during the renewal period, or recreate the same certificate after a backup/restore of your kubernetes cluster. For more details on the computation, check the [time.Truncate](https://golang.org/pkg/time/#Time.Truncate) function. |
 
+## Disable Approval Check
+
+The KMS Issuer will wait for CertificateRequests to have an [approved condition
+set](https://cert-manager.io/docs/concepts/certificaterequest/#approval) before
+signing. If using an older version of cert-manager (pre v1.3), you can disable
+this check by supplying the command line flag `-enable-approved-check=false` to
+the Issuer Deployment.
+
 ## Contributing
 
 Kms-Issuer is built using the [Kubebuilder](https://book.kubebuilder.io/) framework. See the [official documentation](https://book.kubebuilder.io/quick-start.html) to get started and check [CONTRIBUTING.md](CONTRIBUTING.md) for more details.

api/v1alpha1/zz_generated.deepcopy.go

@@ -0,0 +1,14 @@
+# permissions to approve all cert-manager.skyscanner.net requests
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: cert-manager-controller-approve:cert-manager-skyscanner-net
+rules:
+- apiGroups:
+  - cert-manager.io
+  resources:
+  - signers
+  verbs:
+  - approve
+  resourceNames:
+  - kmsissuers.cert-manager.skyscanner.net/*

config/crd/bases/cert-manager.io_certificaterequests.yaml

@@ -0,0 +1,14 @@
+# bind the cert-manager internal approver to approve
+# cert-manager.skyscanner.net CertificateRequests
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: cert-manager-controller-approve:cert-manager-skyscanner-net
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: cert-manager-controller-approve:cert-manager-skyscanner-net
+subjects:
+- kind: ServiceAccount
+  name: cert-manager
+  namespace: cert-manager

config/rbac/cert_manager_controller_approver_clusterrole.yaml

@@ -10,3 +10,7 @@ resources:
 - auth_proxy_role.yaml
 - auth_proxy_role_binding.yaml
 - auth_proxy_client_clusterrole.yaml
+# Comment the following 2 lines if you don't wish for the internal cert-manager
+# approver to approve all cert-manager.skyscanner.net CertificateRequests by default.
+- cert_manager_controller_approver_clusterrole.yaml
+- cert_manager_controller_approver_clusterrolebinding.yaml

config/rbac/cert_manager_controller_approver_clusterrolebinding.yaml

@@ -26,23 +26,32 @@ import (
 	kmsca "github.com/Skyscanner/kms-issuer/pkg/kmsca"
 	"github.com/go-logr/logr"
 	apiutil "github.com/jetstack/cert-manager/pkg/api/util"
-	cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha2"
+	cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
 	cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
 	pkiutil "github.com/jetstack/cert-manager/pkg/util/pki"
 	core "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/utils/clock"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const (
+	levelDebug = 4
+)
+
 // CertificateRequestReconciler reconciles a StepIssuer object.
 type CertificateRequestReconciler struct {
 	client.Client
 	Log      logr.Logger
 	Recorder record.EventRecorder
 	KMSCA    *kmsca.KMSCA
+
+	Clock                  clock.Clock
+	CheckApprovedCondition bool
 }
 
 // Annotation for generating RBAC role for writing Events
@@ -54,8 +63,7 @@ type CertificateRequestReconciler struct {
 // Reconcile will read and validate a KMSIssuer resource associated to the
 // CertificateRequest resource, and it will sign the CertificateRequest with the
 // provisioner in the KMSIssuer.
-func (r *CertificateRequestReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
-	ctx := context.Background()
+func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	log := r.Log.WithValues("certificaterequests", req.NamespacedName)
 
 	// Fetch the CertificateRequest resource being reconciled.
@@ -77,6 +85,11 @@ func (r *CertificateRequestReconciler) Reconcile(req ctrl.Request) (ctrl.Result,
 		return ctrl.Result{}, nil
 	}
 
+	shouldProcess, err := r.requestShouldBeProcessed(ctx, log, cr)
+	if err != nil || !shouldProcess {
+		return ctrl.Result{}, err
+	}
+
 	// If the certificate data is already set then we skip this request as it
 	// has already been completed in the past.
 	if len(cr.Status.Certificate) > 0 {
@@ -96,15 +109,15 @@ func (r *CertificateRequestReconciler) Reconcile(req ctrl.Request) (ctrl.Result,
 		Namespace: req.Namespace,
 		Name:      cr.Spec.IssuerRef.Name,
 	}
-	if err := r.Client.Get(ctx, issNamespaceName, &issuer); err != nil {
+	if err = r.Client.Get(ctx, issNamespaceName, &issuer); err != nil {
 		log.Error(err, "failed to retrieve KMSIssuer resource", "namespace", req.Namespace, "name", cr.Spec.IssuerRef.Name)
 		_ = r.setStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Failed to retrieve KMSIssuer resource %s: %v", issNamespaceName, err)
 		return ctrl.Result{}, err
 	}
 
 	// Check if the KMSIssuer resource has been marked Ready
 	if !issuer.Status.IsReady() {
-		err := fmt.Errorf("resource %s is not ready", issNamespaceName)
+		err = fmt.Errorf("resource %s is not ready", issNamespaceName)
 		log.Error(err, "failed to retrieve StepIssuer resource", "namespace", req.Namespace, "name", cr.Spec.IssuerRef.Name)
 		_ = r.setStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "StepIssuer resource %s is not Ready", issNamespaceName)
 		return ctrl.Result{}, err
@@ -161,6 +174,64 @@ func (r *CertificateRequestReconciler) SetupWithManager(mgr ctrl.Manager) error
 // 	return false
 // }
 
+// requestShouldBeProcessed will return false if the conditions on the request
+// mean that it should not be processed. If the request has been denied, it
+// will set the request failure time and add a Ready=False condition.
+func (r *CertificateRequestReconciler) requestShouldBeProcessed(ctx context.Context, log logr.Logger, cr *cmapi.CertificateRequest) (bool, error) {
+	dbg := log.V(levelDebug)
+
+	// Ignore CertificateRequest if it is already Ready
+	if apiutil.CertificateRequestHasCondition(cr, cmapi.CertificateRequestCondition{
+		Type:   cmapi.CertificateRequestConditionReady,
+		Status: cmmeta.ConditionTrue,
+	}) {
+		dbg.Info("CertificateRequest is Ready. Ignoring.")
+		return false, nil
+	}
+	// Ignore CertificateRequest if it is already Failed
+	if apiutil.CertificateRequestHasCondition(cr, cmapi.CertificateRequestCondition{
+		Type:   cmapi.CertificateRequestConditionReady,
+		Status: cmmeta.ConditionFalse,
+		Reason: cmapi.CertificateRequestReasonFailed,
+	}) {
+		dbg.Info("CertificateRequest is Failed. Ignoring.")
+		return false, nil
+	}
+	// Ignore CertificateRequest if it already has a Denied Ready Reason
+	if apiutil.CertificateRequestHasCondition(cr, cmapi.CertificateRequestCondition{
+		Type:   cmapi.CertificateRequestConditionReady,
+		Status: cmmeta.ConditionFalse,
+		Reason: cmapi.CertificateRequestReasonDenied,
+	}) {
+		dbg.Info("CertificateRequest already has a Ready condition with Denied Reason. Ignoring.")
+		return false, nil
+	}
+
+	// If CertificateRequest has been denied, mark the CertificateRequest as
+	// Ready=Denied and set FailureTime if not already.
+	if apiutil.CertificateRequestIsDenied(cr) {
+		dbg.Info("CertificateRequest has been denied. Marking as failed.")
+
+		if cr.Status.FailureTime == nil {
+			nowTime := metav1.NewTime(r.Clock.Now())
+			cr.Status.FailureTime = &nowTime
+		}
+
+		message := "The CertificateRequest was denied by an approval controller"
+		return false, r.setStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonDenied, message)
+	}
+
+	if r.CheckApprovedCondition {
+		// If CertificateRequest has not been approved, exit early.
+		if !apiutil.CertificateRequestIsApproved(cr) {
+			dbg.Info("certificate request has not been approved")
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
+
 func (r *CertificateRequestReconciler) setStatus(ctx context.Context, cr *cmapi.CertificateRequest, status cmmeta.ConditionStatus, reason, message string, args ...interface{}) error {
 	completeMessage := fmt.Sprintf(message, args...)
 	apiutil.SetCertificateRequestCondition(cr, cmapi.CertificateRequestConditionReady, status, reason, completeMessage)