diff --git a/charts/aws-pca-issuer/templates/deployment.yaml b/charts/aws-pca-issuer/templates/deployment.yaml index fedd1d62..09dabe90 100644 --- a/charts/aws-pca-issuer/templates/deployment.yaml +++ b/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 diff --git a/charts/aws-pca-issuer/values.yaml b/charts/aws-pca-issuer/values.yaml index 91e6baa0..dd110153 100644 --- a/charts/aws-pca-issuer/values.yaml +++ b/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: diff --git a/main.go b/main.go index 0c4b6d8a..2a48dcad 100644 --- a/main.go +++ b/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) diff --git a/pkg/controllers/certificaterequest_controller.go b/pkg/controllers/certificaterequest_controller.go index e9459e75..d30b5be6 100644 --- a/pkg/controllers/certificaterequest_controller.go +++ b/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 } diff --git a/pkg/controllers/certificaterequest_controller_test.go b/pkg/controllers/certificaterequest_controller_test.go index 30a842df..901d0b71 100644 --- a/pkg/controllers/certificaterequest_controller_test.go +++ b/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) } }) }