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,12 @@ spec:
             {{- if .Values.disableApprovedCheck }}
             - -disable-approved-check
             {{- end }}
+            {{- if .Values.retryFailedCertificateRequests }}
+            - -retry-failed-certificate-requests
+            {{- end }}
+            {{- if .Values.controllerSyncPeriod }}
+            - -controller-sync-period={{ .Values.controllerSyncPeriod }}
+            {{- end }}
           ports:
             - containerPort: 8080
               name: http

charts/aws-pca-issuer/values.yaml

@@ -10,6 +10,11 @@ image:
 # Disable waiting for CertificateRequests to be Approved before signing
 disableApprovedCheck: false
 
+# Disable retrying failed CertificateRequests
+retryFailedCertificateRequests: false
+
+controllerSyncPeriod: 10h
+
 imagePullSecrets: []
 nameOverride: ""
 fullnameOverride: ""
@@ -33,7 +38,6 @@ service:
   type: ClusterIP
   port: 8080
 
-
 # Options for configuring a target ServiceAccount with the role to approve
 # all awspca.cert-manager.io requests.
 approverRole:

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,8 @@ func main() {
 	var enableLeaderElection bool
 	var probeAddr string
 	var disableApprovedCheck bool
+	var retryFailedCertificateRequests bool
+	var controllerSyncPeriod 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 +67,8 @@ 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.BoolVar(&retryFailedCertificateRequests, "retry-failed-certificate-requests", false, "If enabled, the controller will retry signing failed CertificateRequests.")
+	flag.DurationVar(&controllerSyncPeriod, "controller-sync-period", 10*time.Hour, "The period at which the controller will sync CertificateRequests.")
 
 	opts := zap.Options{
 		Development: false,
@@ -80,6 +85,7 @@ func main() {
 		HealthProbeBindAddress: probeAddr,
 		LeaderElection:         enableLeaderElection,
 		LeaderElectionID:       "b858308c.awspca.cert-manager.io",
+		SyncPeriod:             &controllerSyncPeriod,
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")
@@ -117,8 +123,9 @@ func main() {
 		Scheme:   mgr.GetScheme(),
 		Recorder: mgr.GetEventRecorderFor("awspcaissuer-controller"),
 
-		Clock:                  clock.RealClock{},
-		CheckApprovedCondition: !disableApprovedCheck,
+		Clock:                          clock.RealClock{},
+		CheckApprovedCondition:         !disableApprovedCheck,
+		RetryFailedCertificateRequests: retryFailedCertificateRequests,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "CertificateRequest")
 		os.Exit(1)

pkg/controllers/certificaterequest_controller.go

@@ -47,8 +47,9 @@ type CertificateRequestReconciler struct {
 	Scheme   *runtime.Scheme
 	Recorder record.EventRecorder
 
-	Clock                  clock.Clock
-	CheckApprovedCondition bool
+	Clock                          clock.Clock
+	CheckApprovedCondition         bool
+	RetryFailedCertificateRequests bool
 }
 
 // +kubebuilder:rbac:groups=cert-manager.io,resources=certificaterequests,verbs=get;list;watch;update
@@ -87,7 +88,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 		return ctrl.Result{}, nil
 	}
 	// Ignore CertificateRequest if it is already Failed
-	if cmutil.CertificateRequestHasCondition(cr, cmapi.CertificateRequestCondition{
+	if !r.RetryFailedCertificateRequests && cmutil.CertificateRequestHasCondition(cr, cmapi.CertificateRequestCondition{
 		Type:   cmapi.CertificateRequestConditionReady,
 		Status: cmmeta.ConditionFalse,
 		Reason: cmapi.CertificateRequestReasonFailed,
@@ -156,8 +157,11 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 	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")
+
+		errorMsg := fmt.Sprintf("failed to retrieve provisioner (name: %s, namespace: %s)", issuerName.Name, issuerName.Namespace)
+
+		log.Error(err, errorMsg)
+		_ = r.setStatus(ctx, cr, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, errorMsg)
 		return ctrl.Result{}, err
 	}
 

pkg/controllers/certificaterequest_controller_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 controllers
 
@@ -135,14 +137,15 @@ func TestCertificateRequestReconcile(t *testing.T) {
 			},
 		},
 		"success-cluster-issuer": {
-			name: types.NamespacedName{Name: "cr1"},
+			name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
 			objects: []client.Object{
 				cmgen.CertificateRequest(
 					"cr1",
+					cmgen.SetCertificateRequestNamespace("ns1"),
 					cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{
 						Name:  "clusterissuer1",
 						Group: issuerapi.GroupVersion.Group,
-						Kind:  "ClusterIssuer",
+						Kind:  "AWSPCAClusterIssuer",
 					}),
 					cmgen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
 						Type:   cmapi.CertificateRequestConditionReady,
@@ -190,6 +193,63 @@ func TestCertificateRequestReconcile(t *testing.T) {
 				awspca.StoreProvisioner(types.NamespacedName{Name: "clusterissuer1"}, &fakeProvisioner{caCert: []byte("cacert"), cert: []byte("cert")})
 			},
 		},
+		"success-previous-failure": {
+			name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
+			objects: []client.Object{
+				cmgen.CertificateRequest(
+					"cr1",
+					cmgen.SetCertificateRequestNamespace("ns1"),
+					cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{
+						Name:  "clusterissuer1",
+						Group: issuerapi.GroupVersion.Group,
+						Kind:  "AWSPCAClusterIssuer",
+					}),
+					cmgen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+						Type:   cmapi.CertificateRequestConditionReady,
+						Status: cmmeta.ConditionFalse,
+					}),
+				),
+				&issuerapi.AWSPCAClusterIssuer{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "clusterissuer1",
+					},
+					Spec: issuerapi.AWSPCAIssuerSpec{
+						SecretRef: issuerapi.AWSCredentialsSecretReference{
+							SecretReference: v1.SecretReference{
+								Name: "clusterissuer1-credentials",
+							},
+						},
+						Region: "us-east-1",
+						Arn:    "arn:aws:acm-pca:us-east-1:account:certificate-authority/12345678-1234-1234-1234-123456789012",
+					},
+					Status: issuerapi.AWSPCAIssuerStatus{
+						Conditions: []metav1.Condition{
+							{
+								Type:   issuerapi.ConditionTypeReady,
+								Status: metav1.ConditionTrue,
+							},
+						},
+					},
+				},
+				&v1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "clusterissuer1-credentials",
+					},
+					Data: map[string][]byte{
+						"AWS_ACCESS_KEY_ID":     []byte("ZXhhbXBsZQ=="),
+						"AWS_SECRET_ACCESS_KEY": []byte("ZXhhbXBsZQ=="),
+					},
+				},
+			},
+			expectedReadyConditionStatus: cmmeta.ConditionTrue,
+			expectedReadyConditionReason: cmapi.CertificateRequestReasonIssued,
+			expectedError:                false,
+			expectedCertificate:          []byte("cert"),
+			expectedCACertificate:        []byte("cacert"),
+			mockProvisioner: func() {
+				awspca.StoreProvisioner(types.NamespacedName{Name: "clusterissuer1"}, &fakeProvisioner{caCert: []byte("cacert"), cert: []byte("cert")})
+			},
+		},
 		"failure-issuer-not-ready": {
 			name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
 			objects: []client.Object{
@@ -404,16 +464,15 @@ func TestCertificateRequestReconcile(t *testing.T) {
 			var cr cmapi.CertificateRequest
 			err = fakeClient.Get(ctx, tc.name, &cr)
 			require.NoError(t, client.IgnoreNotFound(err), "unexpected error from fake client")
-			if err == nil {
-				if tc.expectedReadyConditionStatus != "" {
-					assertCertificateRequestHasReadyCondition(t, tc.expectedReadyConditionStatus, tc.expectedReadyConditionReason, &cr)
-				}
-				if tc.expectedCertificate != nil {
-					assert.Equal(t, tc.expectedCertificate, cr.Status.Certificate)
-				}
-				if tc.expectedCACertificate != nil {
-					assert.Equal(t, tc.expectedCACertificate, cr.Status.CA)
-				}
+			if tc.expectedReadyConditionStatus != "" {
+				assertCertificateRequestHasReadyCondition(t, tc.expectedReadyConditionStatus, tc.expectedReadyConditionReason, &cr)
+			}
+
+			if tc.expectedCertificate != nil {
+				assert.Equal(t, tc.expectedCertificate, cr.Status.Certificate)
+			}
+			if tc.expectedCACertificate != nil {
+				assert.Equal(t, tc.expectedCACertificate, cr.Status.CA)
 			}
 		})
 	}