diff --git a/charts/aws-pca-issuer/templates/deployment.yaml b/charts/aws-pca-issuer/templates/deployment.yaml index fedd1d62..c42499d6 100644 --- a/charts/aws-pca-issuer/templates/deployment.yaml +++ b/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 diff --git a/charts/aws-pca-issuer/values.yaml b/charts/aws-pca-issuer/values.yaml index 91e6baa0..f422bb1c 100644 --- a/charts/aws-pca-issuer/values.yaml +++ b/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: diff --git a/config/crd/bases/awspca.cert-manager.io_awspcaclusterissuers.yaml b/config/crd/bases/awspca.cert-manager.io_awspcaclusterissuers.yaml index 3e78428b..83ce9504 100644 --- a/config/crd/bases/awspca.cert-manager.io_awspcaclusterissuers.yaml +++ b/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 diff --git a/config/crd/bases/awspca.cert-manager.io_awspcaissuers.yaml b/config/crd/bases/awspca.cert-manager.io_awspcaissuers.yaml index d4eccf7a..cefc975d 100644 --- a/config/crd/bases/awspca.cert-manager.io_awspcaissuers.yaml +++ b/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 diff --git a/main.go b/main.go index 0c4b6d8a..f107a4ee 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,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) diff --git a/pkg/api/v1beta1/awspcaissuer_types.go b/pkg/api/v1beta1/awspcaissuer_types.go index 765fcffd..22a14a03 100644 --- a/pkg/api/v1beta1/awspcaissuer_types.go +++ b/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 diff --git a/pkg/aws/pca_test.go b/pkg/aws/pca_test.go index a67cb98a..3ef0f88c 100644 --- a/pkg/aws/pca_test.go +++ b/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 diff --git a/pkg/clientset/v1beta1/api.go b/pkg/clientset/v1beta1/api.go index 2998aeac..64631dae 100644 --- a/pkg/clientset/v1beta1/api.go +++ b/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,7 +38,7 @@ 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, @@ -46,7 +46,7 @@ func (c *Client) AWSPCAIssuers(namespace string) AWSPCAIssuerInterface { } } -//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, diff --git a/pkg/clientset/v1beta1/issuers.go b/pkg/clientset/v1beta1/issuers.go index 847663db..a6d47f39 100644 --- a/pkg/clientset/v1beta1/issuers.go +++ b/pkg/clientset/v1beta1/issuers.go @@ -17,7 +17,7 @@ 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) @@ -25,7 +25,7 @@ type AWSPCAIssuerInterface interface { 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) diff --git a/pkg/controllers/certificaterequest_controller.go b/pkg/controllers/certificaterequest_controller.go index e9459e75..7c7b946f 100644 --- a/pkg/controllers/certificaterequest_controller.go +++ b/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,9 +204,8 @@ 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 { @@ -198,5 +213,22 @@ func (r *CertificateRequestReconciler) setStatus(ctx context.Context, cr *cmapi. } 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...) } diff --git a/pkg/controllers/certificaterequest_controller_test.go b/pkg/controllers/certificaterequest_controller_test.go index 30a842df..5788c223 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 @@ -16,6 +18,7 @@ import ( "context" "errors" "testing" + "time" "github.com/aws/aws-sdk-go-v2/aws" cmutil "github.com/cert-manager/cert-manager/pkg/api/util" @@ -32,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" + "k8s.io/utils/clock" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -71,6 +75,7 @@ func TestCertificateRequestReconcile(t *testing.T) { expectedReadyConditionReason string expectedCertificate []byte expectedCACertificate []byte + retryDuration time.Duration mockProvisioner createMockProvisioner } tests := map[string]testCase{ @@ -80,6 +85,7 @@ func TestCertificateRequestReconcile(t *testing.T) { cmgen.CertificateRequest( "cr1", cmgen.SetCertificateRequestNamespace("ns1"), + SetCreationTime(time.Now()), cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ Name: "issuer1", Group: issuerapi.GroupVersion.Group, @@ -135,14 +141,16 @@ 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"), + SetCreationTime(time.Now()), cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ Name: "clusterissuer1", Group: issuerapi.GroupVersion.Group, - Kind: "ClusterIssuer", + Kind: "AWSPCAClusterIssuer", }), cmgen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ Type: cmapi.CertificateRequestConditionReady, @@ -190,12 +198,71 @@ 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"), + SetCreationTime(time.Now()), + 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{ cmgen.CertificateRequest( "cr1", cmgen.SetCertificateRequestNamespace("ns1"), + SetCreationTime(time.Now()), cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ Name: "issuer1", Group: issuerapi.GroupVersion.Group, @@ -254,6 +321,7 @@ func TestCertificateRequestReconcile(t *testing.T) { cmgen.CertificateRequest( "cr1", cmgen.SetCertificateRequestNamespace("ns1"), + SetCreationTime(time.Now()), cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ Name: "issuer1", Group: issuerapi.GroupVersion.Group, @@ -285,6 +353,7 @@ func TestCertificateRequestReconcile(t *testing.T) { cmgen.CertificateRequest( "cr1", cmgen.SetCertificateRequestNamespace("ns1"), + SetCreationTime(time.Now()), cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ Name: "issuer1", Group: issuerapi.GroupVersion.Group, @@ -310,12 +379,46 @@ func TestCertificateRequestReconcile(t *testing.T) { expectedReadyConditionReason: cmapi.CertificateRequestReasonFailed, expectedError: true, }, + "failure-provisioner-not-found-temporary": { + name: types.NamespacedName{Namespace: "ns1", Name: "cr1"}, + objects: []client.Object{ + cmgen.CertificateRequest( + "cr1", + cmgen.SetCertificateRequestNamespace("ns1"), + SetCreationTime(time.Now()), + cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ + Name: "issuer1", + Group: issuerapi.GroupVersion.Group, + Kind: "Issuer", + }), + cmgen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionUnknown, + }), + ), + &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "issuer1-credentials", + Namespace: "ns1", + }, + Data: map[string][]byte{ + "AWS_ACCESS_KEY_ID": []byte("ZXhhbXBsZQ=="), + "AWS_SECRET_ACCESS_KEY": []byte("ZXhhbXBsZQ=="), + }, + }, + }, + expectedReadyConditionStatus: cmmeta.ConditionUnknown, + expectedReadyConditionReason: "", + expectedError: true, + retryDuration: 1 * time.Hour, + }, "failure-sign-failure": { name: types.NamespacedName{Namespace: "ns1", Name: "cr1"}, objects: []client.Object{ cmgen.CertificateRequest( "cr1", cmgen.SetCertificateRequestNamespace("ns1"), + SetCreationTime(time.Now()), cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ Name: "issuer1", Group: issuerapi.GroupVersion.Group, @@ -382,10 +485,12 @@ func TestCertificateRequestReconcile(t *testing.T) { WithObjects(tc.objects...). Build() controller := CertificateRequestReconciler{ - Client: fakeClient, - Log: logrtesting.NewTestLogger(t), - Scheme: scheme, - Recorder: record.NewFakeRecorder(10), + Client: fakeClient, + Log: logrtesting.NewTestLogger(t), + Scheme: scheme, + Recorder: record.NewFakeRecorder(10), + Clock: clock.RealClock{}, + MaxRetryDuration: tc.retryDuration, } ctx := context.TODO() @@ -404,16 +509,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) } }) } @@ -428,7 +532,14 @@ func assertCertificateRequestHasReadyCondition(t *testing.T, status cmmeta.Condi validReasons := sets.NewString( cmapi.CertificateRequestReasonFailed, cmapi.CertificateRequestReasonIssued, + "", ) assert.Contains(t, validReasons, reason, "unexpected condition reason") assert.Equal(t, reason, condition.Reason, "unexpected condition reason") } + +func SetCreationTime(time time.Time) cmgen.CertificateRequestModifier { + return func(c *cmapi.CertificateRequest) { + c.SetCreationTimestamp(metav1.NewTime(time)) + } +} diff --git a/pkg/controllers/genericissuer_controller_test.go b/pkg/controllers/genericissuer_controller_test.go index 7569e230..01bba85a 100644 --- a/pkg/controllers/genericissuer_controller_test.go +++ b/pkg/controllers/genericissuer_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