diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index c5004229d..e0d2c3fb0 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -299,33 +299,33 @@ func (c *Controller) updateSecret(crt *v1alpha1.Certificate, namespace string, c Data: map[string][]byte{}, } } + secret.Data[corev1.TLSCertKey] = cert secret.Data[corev1.TLSPrivateKeyKey] = key - - if ca != nil { - secret.Data[TLSCAKey] = ca - } + secret.Data[TLSCAKey] = ca if secret.Annotations == nil { secret.Annotations = make(map[string]string) } - // Note: since this sets annotations based on certificate resource, incorrect - // annotations will be set if resource and actual certificate somehow get out - // of sync - dnsNames := pki.DNSNamesForCertificate(crt) - cn := pki.CommonNameForCertificate(crt) + // If we are updating the Certificate, we update the secret metadata to + // reflect the actual certificate it contains + if cert != nil { + x509Cert, err := pki.DecodeX509CertificateBytes(cert) + if err != nil { + return nil, fmt.Errorf("invalid certificate data: %v", err) + } - secret.Annotations[v1alpha1.AltNamesAnnotationKey] = strings.Join(dnsNames, ",") - secret.Annotations[v1alpha1.CommonNameAnnotationKey] = cn - - secret.Annotations[v1alpha1.IssuerNameAnnotationKey] = crt.Spec.IssuerRef.Name - secret.Annotations[v1alpha1.IssuerKindAnnotationKey] = issuerKind(crt) + secret.Annotations[v1alpha1.IssuerNameAnnotationKey] = crt.Spec.IssuerRef.Name + secret.Annotations[v1alpha1.IssuerKindAnnotationKey] = issuerKind(crt) + secret.Annotations[v1alpha1.CommonNameAnnotationKey] = x509Cert.Subject.CommonName + secret.Annotations[v1alpha1.AltNamesAnnotationKey] = strings.Join(x509Cert.DNSNames, ",") + } + // Always set the certificate name label on the target secret if secret.Labels == nil { secret.Labels = make(map[string]string) } - secret.Labels[v1alpha1.CertificateNameKey] = crt.Name // if it is a new resource @@ -353,7 +353,7 @@ func (c *Controller) issue(ctx context.Context, issuer issuer.Interface, crt *v1 return err } - if resp.PrivateKey == nil { + if resp == nil { return nil } diff --git a/pkg/issuer/acme/issue.go b/pkg/issuer/acme/issue.go index b9d2104f4..0a1a30333 100644 --- a/pkg/issuer/acme/issue.go +++ b/pkg/issuer/acme/issue.go @@ -49,11 +49,11 @@ var ( certificateGvk = v1alpha1.SchemeGroupVersion.WithKind("Certificate") ) -func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.IssueResponse, error) { +func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (*issuer.IssueResponse, error) { key, generated, err := a.getCertificatePrivateKey(crt) if err != nil { glog.Errorf("Error getting certificate private key: %v", err) - return issuer.IssueResponse{}, err + return nil, err } if generated { // If we have generated a new private key, we return here to ensure we @@ -63,10 +63,11 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss keyPem, err := pki.EncodePrivateKey(key) if err != nil { - return issuer.IssueResponse{}, err + return nil, err } - return issuer.IssueResponse{ + // Replace the existing secret with one containing only the new private key. + return &issuer.IssueResponse{ PrivateKey: keyPem, }, nil } @@ -78,7 +79,7 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss // exists. expectedOrder, err := buildOrder(crt, nil) if err != nil { - return issuer.IssueResponse{}, err + return nil, err } // Cleanup Order resources that are owned by this Certificate but are not @@ -89,7 +90,7 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss err = a.cleanupOwnedOrders(crt, expectedOrder.Name) if err != nil { glog.Errorf("Error cleaning up old orders: %v", err) - return issuer.IssueResponse{}, err + return nil, err } // Obtain the existing Order for this Certificate from the API server. @@ -98,10 +99,10 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss existingOrder, err := a.orderLister.Orders(expectedOrder.Namespace).Get(expectedOrder.Name) if err != nil && !apierrors.IsNotFound(err) { glog.Errorf("Error getting existing Order resource: %v", err) - return issuer.IssueResponse{}, err + return nil, err } if existingOrder == nil { - return issuer.IssueResponse{}, a.createNewOrder(crt, expectedOrder, key) + return nil, a.createNewOrder(crt, expectedOrder, key) } // if there is an existing order, we check to make sure it is up to date @@ -115,18 +116,18 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss validForKey, err := existingOrderIsValidForKey(existingOrder, key) if err != nil { - return issuer.IssueResponse{}, err + return nil, err } if !validForKey { glog.V(4).Infof("CSR on existing order resource does not match certificate %s/%s private key", crt.Namespace, crt.Name) - return a.retryOrder(crt, existingOrder) + return nil, a.retryOrder(crt, existingOrder) } // If the existing order has expired, we should create a new one // TODO: implement setting this order state in the acmeorders controller if existingOrder.Status.State == v1alpha1.Expired { a.Recorder.Eventf(crt, corev1.EventTypeNormal, "OrderExpired", "Existing certificate for Order %q expired", existingOrder.Name) - return a.retryOrder(crt, existingOrder) + return nil, a.retryOrder(crt, existingOrder) } // If the existing order has failed, we should check if the Certificate @@ -142,10 +143,10 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss } if time.Now().Sub(crt.Status.LastFailureTime.Time) < createOrderWaitDuration { - return issuer.IssueResponse{}, fmt.Errorf("applying acme order back-off for certificate %s/%s because it has failed within the last %s", crt.Namespace, crt.Name, createOrderWaitDuration) + return nil, fmt.Errorf("applying acme order back-off for certificate %s/%s because it has failed within the last %s", crt.Namespace, crt.Name, createOrderWaitDuration) } - return a.retryOrder(crt, existingOrder) + return nil, a.retryOrder(crt, existingOrder) } if existingOrder.Status.State != v1alpha1.Valid { @@ -153,7 +154,7 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss // We don't immediately requeue, as the change to the Order resource on // transition should trigger the certificate to be re-synced. - return issuer.IssueResponse{}, nil + return nil, nil } a.Recorder.Eventf(crt, corev1.EventTypeNormal, "OrderComplete", "Order %q completed successfully", existingOrder.Name) @@ -161,7 +162,7 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss // this should never happen if existingOrder.Status.Certificate == nil { a.Recorder.Eventf(crt, corev1.EventTypeWarning, "BadCertificate", "Empty certificate data retrieved from ACME server") - return issuer.IssueResponse{}, fmt.Errorf("Order in a valid state but certificate data not set") + return nil, fmt.Errorf("Order in a valid state but certificate data not set") } // TODO: replace with a call to a function that returns the whole chain @@ -169,7 +170,7 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss if err != nil { glog.Infof("Error parsing existing x509 certificate on Order resource %q: %v", existingOrder.Name, err) // if parsing the certificate fails, recreate the order - return a.retryOrder(crt, existingOrder) + return nil, a.retryOrder(crt, existingOrder) } // x509Cert := x509Certs[0] @@ -184,17 +185,17 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss a.Recorder.Eventf(crt, corev1.EventTypeNormal, "OrderExpired", "Order %q contains a certificate nearing expiry. "+ "Creating new order...") // existing order's certificate is near expiry - return a.retryOrder(crt, existingOrder) + return nil, a.retryOrder(crt, existingOrder) } // encode the private key and return keyPem, err := pki.EncodePrivateKey(key) if err != nil { // TODO: this is probably an internal error - we should fail safer here - return issuer.IssueResponse{}, err + return nil, err } - return issuer.IssueResponse{ + return &issuer.IssueResponse{ Certificate: existingOrder.Status.Certificate, PrivateKey: keyPem, }, nil @@ -308,13 +309,13 @@ func (a *Acme) createNewOrder(crt *v1alpha1.Certificate, template *v1alpha1.Orde // deletion policy. // If delete successfully (i.e. cleaned up), the order name will be // reset to empty and a resync of the resource will begin. -func (a *Acme) retryOrder(crt *v1alpha1.Certificate, existingOrder *v1alpha1.Order) (issuer.IssueResponse, error) { +func (a *Acme) retryOrder(crt *v1alpha1.Certificate, existingOrder *v1alpha1.Order) error { foregroundDeletion := metav1.DeletePropagationForeground err := a.CMClient.CertmanagerV1alpha1().Orders(existingOrder.Namespace).Delete(existingOrder.Name, &metav1.DeleteOptions{ PropagationPolicy: &foregroundDeletion, }) if err != nil { - return issuer.IssueResponse{}, err + return err } crt.Status.LastFailureTime = nil @@ -323,7 +324,7 @@ func (a *Acme) retryOrder(crt *v1alpha1.Certificate, existingOrder *v1alpha1.Ord // has been observed by the informer. // If we set Requeue: true here, we may cause a race where the lister has // not observed the updated orderRef. - return issuer.IssueResponse{}, nil + return nil } func existingOrderIsValidForKey(o *v1alpha1.Order, key crypto.Signer) (bool, error) { diff --git a/pkg/issuer/acme/issue_test.go b/pkg/issuer/acme/issue_test.go index 6a1292949..6c6d4f457 100644 --- a/pkg/issuer/acme/issue_test.go +++ b/pkg/issuer/acme/issue_test.go @@ -157,7 +157,7 @@ func TestIssueHappyPath(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { // returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) if resp.PrivateKey == nil { @@ -186,11 +186,11 @@ func TestIssueHappyPath(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) - if resp.PrivateKey != nil { - t.Errorf("unexpected PrivateKey response set") + if resp != nil { + t.Errorf("expected IssuerResponse to be nil") } if !reflect.DeepEqual(returnedCert, testCert) { t.Errorf("output was not as expected: %s", pretty.Diff(returnedCert, testCert)) @@ -208,11 +208,11 @@ func TestIssueHappyPath(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) - if resp.PrivateKey != nil { - t.Errorf("unexpected PrivateKey response set") + if resp != nil { + t.Errorf("expected IssuerResponse to be nil") } if !reflect.DeepEqual(returnedCert, testCert) { t.Errorf("output was not as expected: %s", pretty.Diff(returnedCert, testCert)) @@ -231,7 +231,7 @@ func TestIssueHappyPath(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) if !reflect.DeepEqual(returnedCert, testCert) { @@ -266,14 +266,11 @@ func TestIssueHappyPath(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) - if resp.Certificate != nil { - t.Errorf("unexpected certificate data") - } - if resp.PrivateKey != nil { - t.Errorf("unexpected private key data") + if resp != nil { + t.Errorf("expected IssuerResponse to be nil") } if !reflect.DeepEqual(returnedCert, testCert) { t.Errorf("output was not as expected: %s", pretty.Diff(returnedCert, testCert)) @@ -409,16 +406,12 @@ func TestIssueRetryCases(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) - if resp.PrivateKey != nil { - t.Errorf("unexpected PrivateKey response set") + if resp != nil { + t.Errorf("expected IssuerResponse to be nil") } - if resp.Certificate != nil { - t.Errorf("unexpected Certificate response set") - } - // the orderRef field should be set to nil if !reflect.DeepEqual(returnedCert, testCert) { t.Errorf("expected certificate order ref to be nil: %s", pretty.Diff(returnedCert, testCert)) } @@ -441,16 +434,12 @@ func TestIssueRetryCases(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) - if resp.PrivateKey != nil { - t.Errorf("unexpected PrivateKey response set") + if resp != nil { + t.Errorf("expected IssuerResponse to be nil") } - if resp.Certificate != nil { - t.Errorf("unexpected Certificate response set") - } - // the orderRef field should be set to nil if !reflect.DeepEqual(returnedCert, testCert) { t.Errorf("expected certificate order ref to be nil: %s", pretty.Diff(returnedCert, testCert)) } @@ -473,16 +462,12 @@ func TestIssueRetryCases(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) - if resp.PrivateKey != nil { - t.Errorf("unexpected PrivateKey response set") + if resp != nil { + t.Errorf("expected IssuerResponse to be nil") } - if resp.Certificate != nil { - t.Errorf("unexpected Certificate response set") - } - // the orderRef field should be set to nil if !reflect.DeepEqual(returnedCert, testCert) { t.Errorf("expected certificate order ref to be nil: %s", pretty.Diff(returnedCert, testCert)) } @@ -505,16 +490,12 @@ func TestIssueRetryCases(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) - if resp.PrivateKey != nil { - t.Errorf("unexpected PrivateKey response set") + if resp != nil { + t.Errorf("expected IssuerResponse to be nil") } - if resp.Certificate != nil { - t.Errorf("unexpected Certificate response set") - } - // the orderRef field should be set to nil if !reflect.DeepEqual(returnedCert, testCert) { t.Errorf("expected certificate order ref to be nil: %s", pretty.Diff(returnedCert, testCert)) } @@ -533,14 +514,11 @@ func TestIssueRetryCases(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) - if resp.PrivateKey != nil { - t.Errorf("unexpected PrivateKey response set") - } - if resp.Certificate != nil { - t.Errorf("unexpected Certificate response set") + if resp != nil { + t.Errorf("expected IssuerResponse to be nil") } // the resource should not be changed if !reflect.DeepEqual(returnedCert, recentlyFailedCertificate) { @@ -561,14 +539,11 @@ func TestIssueRetryCases(t *testing.T) { }, CheckFn: func(t *testing.T, s *acmeFixture, args ...interface{}) { returnedCert := args[0].(*v1alpha1.Certificate) - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) // err := args[2].(error) - if resp.PrivateKey != nil { - t.Errorf("unexpected PrivateKey response set") - } - if resp.Certificate != nil { - t.Errorf("unexpected Certificate response set") + if resp != nil { + t.Errorf("expected IssuerResponse to be nil") } // the resource should have the last failure time set if !reflect.DeepEqual(returnedCert, recentlyFailedCertificate) { diff --git a/pkg/issuer/ca/issue.go b/pkg/issuer/ca/issue.go index c83e6ce09..ec3276b64 100644 --- a/pkg/issuer/ca/issue.go +++ b/pkg/issuer/ca/issue.go @@ -43,7 +43,7 @@ const ( // If there are any failures, they are likely caused by missing or invalid // supporting resources, and to ensure we re-attempt issuance when these resources // are fixed, it always returns an error on any failure. -func (c *CA) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.IssueResponse, error) { +func (c *CA) Issue(ctx context.Context, crt *v1alpha1.Certificate) (*issuer.IssueResponse, error) { // get a copy of the existing/currently issued Certificate's private key signeeKey, err := kube.SecretTLSKey(c.secretsLister, crt.Namespace, crt.Spec.SecretName) if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) { @@ -54,57 +54,57 @@ func (c *CA) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Issue // don't trigger a retry. An error from this function implies some // invalid input parameters, and retrying without updating the // resource will not help. - return issuer.IssueResponse{}, nil + return nil, nil } } if err != nil { glog.Errorf("Error getting private key %q for certificate: %v", crt.Spec.SecretName, err) - return issuer.IssueResponse{}, err + return nil, err } // extract the public component of the key signeePublicKey, err := pki.PublicKeyForPrivateKey(signeeKey) if err != nil { glog.Errorf("Error getting public key from private key: %v", err) - return issuer.IssueResponse{}, err + return nil, err } // get a copy of the CA certificate named on the Issuer caCert, caKey, err := kube.SecretTLSKeyPair(c.secretsLister, c.resourceNamespace, c.issuer.GetSpec().CA.SecretName) if err != nil { glog.Errorf("Error getting signing CA for Issuer: %v", err) - return issuer.IssueResponse{}, err + return nil, err } // generate a x509 certificate template for this Certificate template, err := pki.GenerateTemplate(c.issuer, crt) if err != nil { c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) - return issuer.IssueResponse{}, err + return nil, err } // sign and encode the certificate certPem, _, err := pki.SignCertificate(template, caCert, signeePublicKey, caKey) if err != nil { c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) - return issuer.IssueResponse{}, err + return nil, err } // Encode output private key and CA cert ready for return keyPem, err := pki.EncodePrivateKey(signeeKey) if err != nil { c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorPrivateKey", "Error encoding private key: %v", err) - return issuer.IssueResponse{}, err + return nil, err } // encode the CA certificate to be bundled in the output caPem, err := pki.EncodeX509(caCert) if err != nil { c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error encoding certificate: %v", err) - return issuer.IssueResponse{}, err + return nil, err } - return issuer.IssueResponse{ + return &issuer.IssueResponse{ PrivateKey: keyPem, Certificate: certPem, CA: caPem, diff --git a/pkg/issuer/ca/issue_test.go b/pkg/issuer/ca/issue_test.go index c9bc81dbe..f67ccdf44 100644 --- a/pkg/issuer/ca/issue_test.go +++ b/pkg/issuer/ca/issue_test.go @@ -82,7 +82,7 @@ func generateSelfSignedCert(t *testing.T, crt *v1alpha1.Certificate, key crypto. func allFieldsSetCheck(expectedCA []byte) func(t *testing.T, s *caFixture, args ...interface{}) { return func(t *testing.T, s *caFixture, args ...interface{}) { - resp := args[1].(issuer.IssueResponse) + resp := args[1].(*issuer.IssueResponse) if resp.PrivateKey == nil { t.Errorf("expected new private key to be generated") diff --git a/pkg/issuer/issuer.go b/pkg/issuer/issuer.go index d8f55e536..3997bbb03 100644 --- a/pkg/issuer/issuer.go +++ b/pkg/issuer/issuer.go @@ -30,7 +30,7 @@ type Interface interface { // Issue attempts to issue a certificate as described by the certificate // resource given - Issue(context.Context, *v1alpha1.Certificate) (IssueResponse, error) + Issue(context.Context, *v1alpha1.Certificate) (*IssueResponse, error) } type IssueResponse struct { diff --git a/pkg/issuer/selfsigned/issue.go b/pkg/issuer/selfsigned/issue.go index c07ea1794..f1c71ec87 100644 --- a/pkg/issuer/selfsigned/issue.go +++ b/pkg/issuer/selfsigned/issue.go @@ -30,7 +30,7 @@ import ( "github.com/jetstack/cert-manager/pkg/util/pki" ) -func (c *SelfSigned) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.IssueResponse, error) { +func (c *SelfSigned) Issue(ctx context.Context, crt *v1alpha1.Certificate) (*issuer.IssueResponse, error) { // get a copy of the existing/currently issued Certificate's private key signeePrivateKey, err := kube.SecretTLSKey(c.secretsLister, crt.Namespace, crt.Spec.SecretName) if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) { @@ -41,43 +41,43 @@ func (c *SelfSigned) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issu // don't trigger a retry. An error from this function implies some // invalid input parameters, and retrying without updating the // resource will not help. - return issuer.IssueResponse{}, nil + return nil, nil } } if err != nil { glog.Errorf("Error getting private key %q for certificate: %v", crt.Spec.SecretName, err) - return issuer.IssueResponse{}, err + return nil, err } // extract the public component of the key signeePublicKey, err := pki.PublicKeyForPrivateKey(signeePrivateKey) if err != nil { glog.Errorf("Error getting public key from private key: %v", err) - return issuer.IssueResponse{}, err + return nil, err } // generate a x509 certificate template for this Certificate template, err := pki.GenerateTemplate(c.issuer, crt) if err != nil { c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) - return issuer.IssueResponse{}, err + return nil, err } // sign and encode the certificate certPem, _, err := pki.SignCertificate(template, template, signeePublicKey, signeePrivateKey) if err != nil { c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) - return issuer.IssueResponse{}, err + return nil, err } // Encode output private key keyPem, err := pki.EncodePrivateKey(signeePrivateKey) if err != nil { c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorPrivateKey", "Error encoding private key: %v", err) - return issuer.IssueResponse{}, err + return nil, err } - return issuer.IssueResponse{ + return &issuer.IssueResponse{ PrivateKey: keyPem, Certificate: certPem, CA: certPem, diff --git a/pkg/issuer/vault/issue.go b/pkg/issuer/vault/issue.go index 39f70a4ff..2ef4a2aee 100644 --- a/pkg/issuer/vault/issue.go +++ b/pkg/issuer/vault/issue.go @@ -51,7 +51,7 @@ const ( messageCertIssued = "Certificate issued successfully" ) -func (v *Vault) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.IssueResponse, error) { +func (v *Vault) Issue(ctx context.Context, crt *v1alpha1.Certificate) (*issuer.IssueResponse, error) { // get a copy of the existing/currently issued Certificate's private key signeePrivateKey, err := kube.SecretTLSKey(v.secretsLister, crt.Namespace, crt.Spec.SecretName) if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) { @@ -62,28 +62,28 @@ func (v *Vault) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Is // don't trigger a retry. An error from this function implies some // invalid input parameters, and retrying without updating the // resource will not help. - return issuer.IssueResponse{}, nil + return nil, nil } } if err != nil { glog.Errorf("Error getting private key %q for certificate: %v", crt.Spec.SecretName, err) - return issuer.IssueResponse{}, err + return nil, err } /// BEGIN building CSR // TODO: we should probably surface some of these errors to users template, err := pki.GenerateCSR(v.issuer, crt) if err != nil { - return issuer.IssueResponse{}, err + return nil, err } derBytes, err := pki.EncodeCSR(template, signeePrivateKey) if err != nil { - return issuer.IssueResponse{}, err + return nil, err } pemRequestBuf := &bytes.Buffer{} err = pem.Encode(pemRequestBuf, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: derBytes}) if err != nil { - return issuer.IssueResponse{}, fmt.Errorf("error encoding certificate request: %s", err.Error()) + return nil, fmt.Errorf("error encoding certificate request: %s", err.Error()) } /// END building CSR @@ -96,17 +96,17 @@ func (v *Vault) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Is certPem, caPem, err := v.requestVaultCert(template.Subject.CommonName, certDuration, template.DNSNames, pemRequestBuf.Bytes()) if err != nil { v.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Failed to request certificate: %v", err) - return issuer.IssueResponse{}, err + return nil, err } /// END requesting certificate key, err := pki.EncodePrivateKey(signeePrivateKey) if err != nil { v.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorPrivateKey", "Error encoding private key: %v", err) - return issuer.IssueResponse{}, err + return nil, err } - return issuer.IssueResponse{ + return &issuer.IssueResponse{ PrivateKey: key, Certificate: certPem, CA: caPem,