Adds comments to some func's and changes return err names to be more

clear

Signed-off-by: joshvanl <vleeuwenjoshua@gmail.com>

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)

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

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(

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 {

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")