feat: add config to recover from previously failed attempt

Signed-off-by: Lukas Wöhrl <[email protected]>

charts/aws-pca-issuer/templates/deployment.yaml

@@ -45,6 +45,9 @@ spec:
             {{- if .Values.disableApprovedCheck }}
             - -disable-approved-check
             {{- end }}
+            {{- if .Values.maxRetryDuration }}
+            - -max-retry-duration={{ .Values.maxRetryDuration }}
+            {{- end }}
           ports:
             - containerPort: 8080
               name: http

charts/aws-pca-issuer/values.yaml

@@ -10,6 +10,9 @@ image:
 # Disable waiting for CertificateRequests to be Approved before signing
 disableApprovedCheck: false
 
+# Maxium duration to retry fullfilling a CertificateRequest
+maxRetryDuration: 0
+
 imagePullSecrets: []
 nameOverride: ""
 fullnameOverride: ""
@@ -33,7 +36,6 @@ service:
   type: ClusterIP
   port: 8080
 
-
 # Options for configuring a target ServiceAccount with the role to approve
 # all awspca.cert-manager.io requests.
 approverRole:

config/crd/bases/awspca.cert-manager.io_awspcaclusterissuers.yaml

@@ -67,11 +67,11 @@ spec:
                     - key
                     type: object
                   name:
-                    description: Name is unique within a namespace to reference a
+                    description: name is unique within a namespace to reference a
                       secret resource.
                     type: string
                   namespace:
-                    description: Namespace defines the space within which the secret
+                    description: namespace defines the space within which the secret
                       name must be unique.
                     type: string
                   secretAccessKeySelector:
@@ -103,13 +103,14 @@ spec:
                   description: "Condition contains details for one aspect of the current
                     state of this API Resource. --- This struct is intended for direct
                     use as an array at the field path .status.conditions.  For example,
-                    type FooStatus struct{     // Represents the observations of a
-                    foo's current state.     // Known .status.conditions.type are:
-                    \"Available\", \"Progressing\", and \"Degraded\"     // +patchMergeKey=type
-                    \    // +patchStrategy=merge     // +listType=map     // +listMapKey=type
-                    \    Conditions []metav1.Condition `json:\"conditions,omitempty\"
-                    patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`
-                    \n     // other fields }"
+                    \n \ttype FooStatus struct{ \t    // Represents the observations
+                    of a foo's current state. \t    // Known .status.conditions.type
+                    are: \"Available\", \"Progressing\", and \"Degraded\" \t    //
+                    +patchMergeKey=type \t    // +patchStrategy=merge \t    // +listType=map
+                    \t    // +listMapKey=type \t    Conditions []metav1.Condition
+                    `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\"
+                    protobuf:\"bytes,1,rep,name=conditions\"` \n \t    // other fields
+                    \t}"
                   properties:
                     lastTransitionTime:
                       description: lastTransitionTime is the last time the condition

config/crd/bases/awspca.cert-manager.io_awspcaissuers.yaml

@@ -66,11 +66,11 @@ spec:
                     - key
                     type: object
                   name:
-                    description: Name is unique within a namespace to reference a
+                    description: name is unique within a namespace to reference a
                       secret resource.
                     type: string
                   namespace:
-                    description: Namespace defines the space within which the secret
+                    description: namespace defines the space within which the secret
                       name must be unique.
                     type: string
                   secretAccessKeySelector:
@@ -102,13 +102,14 @@ spec:
                   description: "Condition contains details for one aspect of the current
                     state of this API Resource. --- This struct is intended for direct
                     use as an array at the field path .status.conditions.  For example,
-                    type FooStatus struct{     // Represents the observations of a
-                    foo's current state.     // Known .status.conditions.type are:
-                    \"Available\", \"Progressing\", and \"Degraded\"     // +patchMergeKey=type
-                    \    // +patchStrategy=merge     // +listType=map     // +listMapKey=type
-                    \    Conditions []metav1.Condition `json:\"conditions,omitempty\"
-                    patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`
-                    \n     // other fields }"
+                    \n \ttype FooStatus struct{ \t    // Represents the observations
+                    of a foo's current state. \t    // Known .status.conditions.type
+                    are: \"Available\", \"Progressing\", and \"Degraded\" \t    //
+                    +patchMergeKey=type \t    // +patchStrategy=merge \t    // +listType=map
+                    \t    // +listMapKey=type \t    Conditions []metav1.Condition
+                    `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\"
+                    protobuf:\"bytes,1,rep,name=conditions\"` \n \t    // other fields
+                    \t}"
                   properties:
                     lastTransitionTime:
                       description: lastTransitionTime is the last time the condition

main.go

@@ -19,6 +19,7 @@ package main
 import (
 	"flag"
 	"os"
+	"time"
 
 	certmanager "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 
@@ -56,6 +57,7 @@ func main() {
 	var enableLeaderElection bool
 	var probeAddr string
 	var disableApprovedCheck bool
+	var maxRetryDuration time.Duration
 
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
@@ -64,6 +66,7 @@ func main() {
 			"Enabling this will ensure there is only one active controller manager.")
 	flag.BoolVar(&disableApprovedCheck, "disable-approved-check", false,
 		"Disables waiting for CertificateRequests to have an approved condition before signing.")
+	flag.DurationVar(&maxRetryDuration, "max-retry-duration", 0, "Maximum duration to retry failed CertificateRequests. Defaults to 0 (disable).")
 
 	opts := zap.Options{
 		Development: false,
@@ -119,6 +122,7 @@ func main() {
 
 		Clock:                  clock.RealClock{},
 		CheckApprovedCondition: !disableApprovedCheck,
+		MaxRetryDuration:       maxRetryDuration,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "CertificateRequest")
 		os.Exit(1)

pkg/api/v1beta1/awspcaissuer_types.go

@@ -39,7 +39,7 @@ type AWSPCAIssuerSpec struct {
 	SecretRef AWSCredentialsSecretReference `json:"secretRef,omitempty"`
 }
 
-//AWSCredentialsSecretReference defines the secret used by the issuer
+// AWSCredentialsSecretReference defines the secret used by the issuer
 type AWSCredentialsSecretReference struct {
 	v1.SecretReference `json:""`
 	// Specifies the secret key where the AWS Access Key ID exists

pkg/aws/pca_test.go

@@ -1,14 +1,16 @@
 /*
-  Copyright 2021.
-  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.
+Copyright 2021.
+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 aws
 

pkg/clientset/v1beta1/api.go

@@ -12,12 +12,12 @@ type Interface interface {
 	AWSPCAClusterIssuers() AWSPCAClusterIssuerInterface
 }
 
-//Client defines a client for interacting with AWS PCA issuers
+// Client defines a client for interacting with AWS PCA issuers
 type Client struct {
 	restClient rest.Interface
 }
 
-//NewForConfig is a function which lets you configure pca issuer clientset
+// NewForConfig is a function which lets you configure pca issuer clientset
 func NewForConfig(c *rest.Config) (*Client, error) {
 	err := AddToScheme(scheme.Scheme)
 	if err != nil {
@@ -38,15 +38,15 @@ func NewForConfig(c *rest.Config) (*Client, error) {
 	return &Client{restClient: client}, nil
 }
 
-//AWSPCAIssuers is a function which lets you interact with AWSPCAIssuers
+// AWSPCAIssuers is a function which lets you interact with AWSPCAIssuers
 func (c *Client) AWSPCAIssuers(namespace string) AWSPCAIssuerInterface {
 	return &awspcaIssuerClient{
 		restClient: c.restClient,
 		ns:         namespace,
 	}
 }
 
-//AWSPCAClusterIssuers is a function which lets you interact with AWSPCAClusterIssuers
+// AWSPCAClusterIssuers is a function which lets you interact with AWSPCAClusterIssuers
 func (c *Client) AWSPCAClusterIssuers() AWSPCAClusterIssuerInterface {
 	return &awspcaClusterIssuerClient{
 		restClient: c.restClient,

pkg/clientset/v1beta1/issuers.go

@@ -17,15 +17,15 @@ var (
 	awspcaclusterissuers = "awspcaclusterissuers"
 )
 
-//AWSPCAIssuerInterface is a interface for interacting with a AWSPCAIssuer
+// AWSPCAIssuerInterface is a interface for interacting with a AWSPCAIssuer
 type AWSPCAIssuerInterface interface {
 	Get(ctx context.Context, name string, opts metav1.GetOptions) (*v1beta1.AWSPCAIssuer, error)
 	Create(ctx context.Context, issuer *v1beta1.AWSPCAIssuer, opts metav1.CreateOptions) (*v1beta1.AWSPCAIssuer, error)
 	Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error
 	Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error)
 }
 
-//AWSPCAClusterIssuerInterface is a interface for interacting with a AWSPCAClusterIssuer
+// AWSPCAClusterIssuerInterface is a interface for interacting with a AWSPCAClusterIssuer
 type AWSPCAClusterIssuerInterface interface {
 	Get(ctx context.Context, name string, opts metav1.GetOptions) (*v1beta1.AWSPCAClusterIssuer, error)
 	Create(ctx context.Context, issuer *v1beta1.AWSPCAClusterIssuer, opts metav1.CreateOptions) (*v1beta1.AWSPCAClusterIssuer, error)

pkg/controllers/certificaterequest_controller.go

@@ -19,6 +19,8 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"math/rand"
+	"time"
 
 	"github.com/cert-manager/aws-privateca-issuer/pkg/aws"
 	"github.com/cert-manager/aws-privateca-issuer/pkg/util"
@@ -49,6 +51,7 @@ type CertificateRequestReconciler struct {
 
 	Clock                  clock.Clock
 	CheckApprovedCondition bool
+	MaxRetryDuration       time.Duration
 }
 
 // +kubebuilder:rbac:groups=cert-manager.io,resources=certificaterequests,verbs=get;list;watch;update
@@ -116,7 +119,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 		}
 
 		message := "The CertificateRequest was denied by an approval controller"
-		return ctrl.Result{}, r.setStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonDenied, message)
+		return ctrl.Result{}, r.setPermanentStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonDenied, message)
 	}
 
 	if r.CheckApprovedCondition {
@@ -140,36 +143,49 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 		issuerName.Namespace = ""
 	}
 
+	err := r.HandleSignRequest(ctx, log, issuerName, cr)
+
+	if err != nil {
+		now := r.Clock.Now()
+		creationTime := cr.GetCreationTimestamp()
+		pastMaxRetryDuration := now.After(creationTime.Add(r.MaxRetryDuration))
+
+		if pastMaxRetryDuration {
+			return ctrl.Result{}, r.setPermanentStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "Permanent error signing certificate: %s", err.Error())
+		}
+
+		retryAfter := 1*time.Minute + time.Duration(rand.Intn(60))*time.Second
+
+		return ctrl.Result{
+			RequeueAfter: retryAfter,
+		}, r.setTemporaryStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "Temporary error signing certificate, retry again: %s", err.Error())
+	}
+
+	return ctrl.Result{}, r.setPermanentStatus(ctx, cr, cmmeta.ConditionTrue, cmapi.CertificateRequestReasonIssued, "Certificate issued")
+}
+
+func (r *CertificateRequestReconciler) HandleSignRequest(ctx context.Context, log logr.Logger, issuerName types.NamespacedName, cr *cmapi.CertificateRequest) error {
 	iss, err := util.GetIssuer(ctx, r.Client, issuerName)
 	if err != nil {
-		log.Error(err, "failed to retrieve Issuer resource")
-		_ = r.setStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "issuer could not be found")
-		return ctrl.Result{}, err
+		return fmt.Errorf("failed to retrieve Issuer resource: %w", err)
 	}
 
 	if !isReady(iss) {
-		err := fmt.Errorf("issuer %s is not ready", iss.GetName())
-		_ = r.setStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "issuer is not ready")
-		return ctrl.Result{}, err
+		return fmt.Errorf("issuer %s is not ready", iss.GetName())
 	}
 
 	provisioner, ok := aws.GetProvisioner(issuerName)
 	if !ok {
-		err := fmt.Errorf("provisioner for %s not found", issuerName)
-		log.Error(err, "failed to retrieve provisioner")
-		_ = r.setStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "failed to retrieve provisioner")
-		return ctrl.Result{}, err
+		return fmt.Errorf("provisioner for %s not found (name: %s, namespace: %s)", issuerName, issuerName.Name, issuerName.Namespace)
 	}
 
 	pem, ca, err := provisioner.Sign(ctx, cr, log)
 	if err != nil {
-		log.Error(err, "failed to request certificate from PCA")
-		return ctrl.Result{}, r.setStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, "failed to request certificate from PCA: "+err.Error())
+		return fmt.Errorf("failed to sign certificat from PCA: %w", err)
 	}
 	cr.Status.Certificate = pem
 	cr.Status.CA = ca
-
-	return ctrl.Result{}, r.setStatus(ctx, cr, cmmeta.ConditionTrue, cmapi.CertificateRequestReasonIssued, "certificate issued")
+	return nil
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -188,15 +204,31 @@ func isReady(issuer api.GenericIssuer) bool {
 	return false
 }
 
-func (r *CertificateRequestReconciler) setStatus(ctx context.Context, cr *cmapi.CertificateRequest, status cmmeta.ConditionStatus, reason, message string, args ...interface{}) error {
+func (r *CertificateRequestReconciler) setStatusInternal(ctx context.Context, cr *cmapi.CertificateRequest, permanent bool, status cmmeta.ConditionStatus, reason, message string, args ...interface{}) error {
 	completeMessage := fmt.Sprintf(message, args...)
-	cmutil.SetCertificateRequestCondition(cr, "Ready", status, reason, completeMessage)
 
 	eventType := core.EventTypeNormal
 	if status == cmmeta.ConditionFalse {
 		eventType = core.EventTypeWarning
 	}
 	r.Recorder.Event(cr, eventType, reason, completeMessage)
 
-	return r.Client.Status().Update(ctx, cr)
+	if permanent {
+		cmutil.SetCertificateRequestCondition(cr, "Ready", status, reason, completeMessage)
+		r.Client.Status().Update(ctx, cr)
+	}
+
+	if reason == cmapi.CertificateRequestReasonFailed {
+		return fmt.Errorf(completeMessage)
+	}
+
+	return nil
+}
+
+func (r *CertificateRequestReconciler) setPermanentStatus(ctx context.Context, cr *cmapi.CertificateRequest, status cmmeta.ConditionStatus, reason, message string, args ...interface{}) error {
+	return r.setStatusInternal(ctx, cr, true, status, reason, message, args...)
+}
+
+func (r *CertificateRequestReconciler) setTemporaryStatus(ctx context.Context, cr *cmapi.CertificateRequest, status cmmeta.ConditionStatus, reason, message string, args ...interface{}) error {
+	return r.setStatusInternal(ctx, cr, false, status, reason, message, args...)
 }