diff --git a/pkg/controller/certificatesigningrequests/sync.go b/pkg/controller/certificatesigningrequests/sync.go index 292dfe0e4..91a08d43c 100644 --- a/pkg/controller/certificatesigningrequests/sync.go +++ b/pkg/controller/certificatesigningrequests/sync.go @@ -38,6 +38,8 @@ func (c *Controller) Sync(ctx context.Context, csr *certificatesv1.CertificateSi log := logf.WithResource(logf.FromContext(ctx), csr).WithValues("signerName", csr.Spec.SignerName) dbg := log.V(logf.DebugLevel) + csr = csr.DeepCopy() + ref, ok := util.SignerIssuerRefFromSignerName(csr.Spec.SignerName) if !ok { dbg.Info("certificate signing request has malformed signer name,", "signerName", csr.Spec.SignerName) diff --git a/pkg/controller/certificatesigningrequests/venafi/venafi.go b/pkg/controller/certificatesigningrequests/venafi/venafi.go index 4a2df7ef5..24fdd6baa 100644 --- a/pkg/controller/certificatesigningrequests/venafi/venafi.go +++ b/pkg/controller/certificatesigningrequests/venafi/venafi.go @@ -112,8 +112,8 @@ func (v *Venafi) Sign(ctx context.Context, csr *certificatesv1.CertificateSignin message := fmt.Sprintf("Failed to parse %q annotation: %s", experimentalapi.CertificateSigningRequestVenafiCustomFieldsAnnotationKey, err) v.recorder.Event(csr, corev1.EventTypeWarning, "ErrorCustomFields", message) util.CertificateSigningRequestSetFailed(csr, "ErrorCustomFields", message) - _, err = v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) - return err + _, userr := v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + return userr } } @@ -123,8 +123,8 @@ func (v *Venafi) Sign(ctx context.Context, csr *certificatesv1.CertificateSignin log.Error(err, message) v.recorder.Event(csr, corev1.EventTypeWarning, "ErrorParseDuration", message) util.CertificateSigningRequestSetFailed(csr, "ErrorParseDuration", message) - _, err := v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) - return err + _, userr := v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + return userr } // The signing process with Venafi is slow. The "pickupID" allows us to track @@ -143,16 +143,16 @@ func (v *Venafi) Sign(ctx context.Context, csr *certificatesv1.CertificateSignin log.Error(err, "") v.recorder.Event(csr, corev1.EventTypeWarning, "ErrorCustomFields", err.Error()) util.CertificateSigningRequestSetFailed(csr, "ErrorCustomFields", err.Error()) - _, err := v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) - return err + _, userr := v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + return userr default: message := fmt.Sprintf("Failed to request venafi certificate: %s", err) log.Error(err, message) v.recorder.Event(csr, corev1.EventTypeWarning, "ErrorRequest", message) util.CertificateSigningRequestSetFailed(csr, "ErrorRequest", message) - _, err := v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) - return err + _, userr := v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + return userr } } @@ -160,19 +160,25 @@ func (v *Venafi) Sign(ctx context.Context, csr *certificatesv1.CertificateSignin csr.Annotations = make(map[string]string) } csr.Annotations[experimentalapi.CertificateSigningRequestVenafiPickupIDAnnotationKey] = pickupID - _, err = v.certClient.Update(ctx, csr, metav1.UpdateOptions{}) - return err + _, uerr := v.certClient.Update(ctx, csr, metav1.UpdateOptions{}) + return uerr } certPem, err := client.RetrieveCertificate(pickupID, csr.Spec.Request, duration, customFields) if err != nil { switch err.(type) { - case endpoint.ErrCertificatePending, endpoint.ErrRetrieveCertificateTimeout: + case endpoint.ErrCertificatePending: message := "Venafi certificate still in a pending state, waiting" - log.Error(err, message) + log.V(2).Info(message, "error", err.Error()) v.recorder.Event(csr, corev1.EventTypeNormal, "IssuancePending", message) return err + case endpoint.ErrRetrieveCertificateTimeout: + message := "Venafi retrieve certificate timeout, retrying" + log.Error(err, message) + v.recorder.Event(csr, corev1.EventTypeWarning, "RetrieveCertificateTimeout", message) + return err + default: message := fmt.Sprintf("Failed to obtain venafi certificate: %s", err) log.Error(err, message) @@ -187,8 +193,8 @@ func (v *Venafi) Sign(ctx context.Context, csr *certificatesv1.CertificateSignin log.Error(err, message) v.recorder.Event(csr, corev1.EventTypeWarning, "ErrorParse", message) util.CertificateSigningRequestSetFailed(csr, "ErrorParse", message) - _, err := v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) - return err + _, userr := v.certClient.UpdateStatus(ctx, csr, metav1.UpdateOptions{}) + return userr } csr.Status.Certificate = bundle.ChainPEM diff --git a/pkg/controller/certificatesigningrequests/venafi/venafi_test.go b/pkg/controller/certificatesigningrequests/venafi/venafi_test.go index 066f47434..7a04fc5f3 100644 --- a/pkg/controller/certificatesigningrequests/venafi/venafi_test.go +++ b/pkg/controller/certificatesigningrequests/venafi/venafi_test.go @@ -638,7 +638,7 @@ func TestProcessItem(t *testing.T) { builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{baseIssuer.DeepCopy()}, ExpectedEvents: []string{ - "Normal IssuancePending Venafi certificate still in a pending state, waiting", + "Warning RetrieveCertificateTimeout Venafi retrieve certificate timeout, retrying", }, ExpectedActions: []testpkg.Action{ testpkg.NewAction(coretesting.NewCreateAction( diff --git a/pkg/issuer/venafi/client/venaficlient.go b/pkg/issuer/venafi/client/venaficlient.go index 1352b4839..a140a6ef0 100644 --- a/pkg/issuer/venafi/client/venaficlient.go +++ b/pkg/issuer/venafi/client/venaficlient.go @@ -70,6 +70,8 @@ type connector interface { RenewCertificate(req *certificate.RenewalRequest) (requestID string, err error) } +// New constructs a Venafi client Interface. Errors may be network errors and +// should be considered for retrying. func New(namespace string, secretsLister corelisters.SecretLister, issuer cmapi.GenericIssuer) (Interface, error) { cfg, err := configForIssuer(issuer, secretsLister, namespace) if err != nil { diff --git a/test/e2e/suite/conformance/certificatesigningrequests/venafi/cloud.go b/test/e2e/suite/conformance/certificatesigningrequests/venafi/cloud.go index ae753a40c..c56fc6352 100644 --- a/test/e2e/suite/conformance/certificatesigningrequests/venafi/cloud.go +++ b/test/e2e/suite/conformance/certificatesigningrequests/venafi/cloud.go @@ -107,6 +107,9 @@ func (c *cloud) createIssuer(f *framework.Framework) string { return fmt.Sprintf("issuers.cert-manager.io/%s.%s", issuer.Namespace, issuer.Name) } +// createClusterIssuer creates and returns name of a Venafi Cloud +// ClusterIssuer. The name is of the form +// "clusterissuers.cert-manager.io/issuer-ab3de1". func (c *cloud) createClusterIssuer(f *framework.Framework) string { By("Creating a Venafi Cloud ClusterIssuer")