diff --git a/controllers/certificaterequest_controller.go b/controllers/certificaterequest_controller.go index 98ed43c..eba077b 100644 --- a/controllers/certificaterequest_controller.go +++ b/controllers/certificaterequest_controller.go @@ -69,6 +69,13 @@ type CertificateRequestReconciler struct { // Clock is used to mock condition transition times in tests. Clock clock.PassiveClock + // SetCAOnCertificateRequest is used to enable setting the CA status field on + // the CertificateRequest resource. This is disabled by default. + // Deprecated: this option is for backwards compatibility only. The use of + // ca.crt is discouraged. Instead, the CA certificate should be provided + // separately using a tool such as trust-manager. + SetCAOnCertificateRequest bool + PostSetupWithManager func(context.Context, schema.GroupVersionKind, ctrl.Manager, controller.Controller) error } @@ -343,7 +350,10 @@ func (r *CertificateRequestReconciler) reconcileStatusPatch( } } - crStatusPatch.Certificate = signedCertificate + crStatusPatch.Certificate = signedCertificate.ChainPEM + if r.SetCAOnCertificateRequest { + crStatusPatch.CA = signedCertificate.CAPEM + } conditions.SetCertificateRequestStatusCondition( r.Clock, cr.Status.Conditions, diff --git a/controllers/certificaterequest_controller_integration_test.go b/controllers/certificaterequest_controller_integration_test.go index 1df99d9..78fde78 100644 --- a/controllers/certificaterequest_controller_integration_test.go +++ b/controllers/certificaterequest_controller_integration_test.go @@ -90,9 +90,11 @@ func TestCertificateRequestControllerIntegrationIssuerInitiallyNotFoundAndNotRea MaxRetryDuration: time.Minute, EventSource: kubeutil.NewEventStore(), Client: mgr.GetClient(), - Sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { + Sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { atomic.AddUint64(&counters[extractIdFromNamespace(t, cr.GetNamespace())], 1) - return []byte("ok"), nil + return signer.PEMBundle{ + ChainPEM: []byte("cert"), + }, nil }, EventRecorder: record.NewFakeRecorder(100), Clock: clock.RealClock{}, @@ -227,13 +229,13 @@ func TestCertificateRequestControllerIntegrationSetCondition(t *testing.T) { MaxRetryDuration: time.Minute, EventSource: kubeutil.NewEventStore(), Client: mgr.GetClient(), - Sign: func(ctx context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { + Sign: func(ctx context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { atomic.AddUint64(&counter, 1) select { case err := <-signResult: - return nil, err + return signer.PEMBundle{}, err case <-ctx.Done(): - return nil, ctx.Err() + return signer.PEMBundle{}, ctx.Err() } }, EventRecorder: record.NewFakeRecorder(100), diff --git a/controllers/certificaterequest_controller_test.go b/controllers/certificaterequest_controller_test.go index 8a72c10..8567f96 100644 --- a/controllers/certificaterequest_controller_test.go +++ b/controllers/certificaterequest_controller_test.go @@ -127,8 +127,10 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { ) successSigner := func(cert string) signer.Sign { - return func(_ context.Context, _ signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return []byte(cert), nil + return func(_ context.Context, _ signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{ + ChainPEM: []byte(cert), + }, nil } } @@ -392,8 +394,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // condition to Failed. { name: "timeout-permanent-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, fmt.Errorf("a specific error") + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, fmt.Errorf("a specific error") }, objects: []client.Object{ cmgen.CertificateRequestFrom(cr1, @@ -428,8 +430,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // the MaxRetryDuration has been exceeded). { name: "retry-on-pending-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.PendingError{Err: fmt.Errorf("pending error")} + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.PendingError{Err: fmt.Errorf("pending error")} }, objects: []client.Object{ cmgen.CertificateRequestFrom(cr1, @@ -472,8 +474,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // condition to *Pending*. { name: "error-set-certificate-request-condition-should-add-new-condition-and-retry", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: fmt.Errorf("test error"), ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -526,8 +528,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // condition to *Pending*. { name: "error-set-certificate-request-condition-should-update-existing-condition-and-retry", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: fmt.Errorf("test error2"), ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -589,8 +591,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // to *Failed*. { name: "error-set-certificate-request-condition-should-add-new-condition-and-timeout", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: fmt.Errorf("test error"), ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -644,8 +646,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // to *Failed*. { name: "error-set-certificate-request-condition-should-update-existing-condition-and-timeout", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: fmt.Errorf("test error2"), ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -704,8 +706,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // exceeded). { name: "error-set-certificate-request-condition-should-not-timeout-if-pending", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: signer.PendingError{Err: fmt.Errorf("test error")}, ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -757,8 +759,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // exceeded). { name: "error-set-certificate-request-condition-should-not-retry-on-permanent-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: signer.PermanentError{Err: fmt.Errorf("test error")}, ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -804,8 +806,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // Set the Ready condition to Failed if the sign function returns a permanent error. { name: "fail-on-permanent-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.PermanentError{Err: fmt.Errorf("a specific error")} + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.PermanentError{Err: fmt.Errorf("a specific error")} }, objects: []client.Object{ cmgen.CertificateRequestFrom(cr1, @@ -837,8 +839,8 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) { // to retry. { name: "retry-on-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, errors.New("waiting for approval") + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, errors.New("waiting for approval") }, objects: []client.Object{ cmgen.CertificateRequestFrom(cr1, diff --git a/controllers/certificatesigningrequest_controller.go b/controllers/certificatesigningrequest_controller.go index 9a7a3e0..1e5ebe5 100644 --- a/controllers/certificatesigningrequest_controller.go +++ b/controllers/certificatesigningrequest_controller.go @@ -255,7 +255,7 @@ func (r *CertificateSigningRequestReconciler) reconcileStatusPatch( } } - csrStatusPatch.Certificate = signedCertificate + csrStatusPatch.Certificate = signedCertificate.ChainPEM logger.V(1).Info("Successfully finished the reconciliation.") r.EventRecorder.Eventf(&csr, corev1.EventTypeNormal, "Issued", "Succeeded signing the CertificateRequest") diff --git a/controllers/certificatesigningrequest_controller_test.go b/controllers/certificatesigningrequest_controller_test.go index 0c07a08..9df1de4 100644 --- a/controllers/certificatesigningrequest_controller_test.go +++ b/controllers/certificatesigningrequest_controller_test.go @@ -117,8 +117,10 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { ) successSigner := func(cert string) signer.Sign { - return func(_ context.Context, _ signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return []byte(cert), nil + return func(_ context.Context, _ signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{ + ChainPEM: []byte(cert), + }, nil } } @@ -287,8 +289,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // condition to Failed. { name: "timeout-permanent-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, fmt.Errorf("a specific error") + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, fmt.Errorf("a specific error") }, objects: []client.Object{ cmgen.CertificateSigningRequestFrom(cr1, @@ -322,8 +324,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // the MaxRetryDuration has been exceeded). { name: "retry-on-pending-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.PendingError{Err: fmt.Errorf("pending error")} + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.PendingError{Err: fmt.Errorf("pending error")} }, objects: []client.Object{ cmgen.CertificateSigningRequestFrom(cr1, @@ -357,8 +359,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // condition to *Pending*. { name: "error-set-certificate-request-condition-should-add-new-condition-and-retry", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: fmt.Errorf("test error"), ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -404,8 +406,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // condition to *Pending*. { name: "error-set-certificate-request-condition-should-update-existing-condition-and-retry", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: fmt.Errorf("test error2"), ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -461,8 +463,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // to *Failed*. { name: "error-set-certificate-request-condition-should-add-new-condition-and-timeout", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: fmt.Errorf("test error"), ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -516,8 +518,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // to *Failed*. { name: "error-set-certificate-request-condition-should-update-existing-condition-and-timeout", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: fmt.Errorf("test error2"), ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -577,8 +579,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // exceeded). { name: "error-set-certificate-request-condition-should-not-timeout-if-pending", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: signer.PendingError{Err: fmt.Errorf("test error")}, ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -623,8 +625,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // exceeded). { name: "error-set-certificate-request-condition-should-not-retry-on-permanent-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.SetCertificateRequestConditionError{ + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.SetCertificateRequestConditionError{ Err: signer.PermanentError{Err: fmt.Errorf("test error")}, ConditionType: "[condition type]", Status: cmmeta.ConditionTrue, @@ -670,8 +672,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // Set the Ready condition to Failed if the sign function returns a permanent error. { name: "fail-on-permanent-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, signer.PermanentError{Err: fmt.Errorf("a specific error")} + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, signer.PermanentError{Err: fmt.Errorf("a specific error")} }, objects: []client.Object{ cmgen.CertificateSigningRequestFrom(cr1, @@ -702,8 +704,8 @@ func TestCertificateSigningRequestReconcilerReconcile(t *testing.T) { // to retry. { name: "retry-on-error", - sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { - return nil, errors.New("waiting for approval") + sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { + return signer.PEMBundle{}, errors.New("waiting for approval") }, objects: []client.Object{ cmgen.CertificateSigningRequestFrom(cr1, diff --git a/controllers/combined_controller.go b/controllers/combined_controller.go index f7bee46..19a553f 100644 --- a/controllers/combined_controller.go +++ b/controllers/combined_controller.go @@ -51,6 +51,13 @@ type CombinedController struct { // Clock is used to mock condition transition times in tests. Clock clock.PassiveClock + // SetCAOnCertificateRequest is used to enable setting the CA status field on + // the CertificateRequest resource. This is disabled by default. + // Deprecated: this option is for backwards compatibility only. The use of + // ca.crt is discouraged. Instead, the CA certificate should be provided + // separately using a tool such as trust-manager. + SetCAOnCertificateRequest bool + PostSetupWithManager func(context.Context, schema.GroupVersionKind, ctrl.Manager, controller.Controller) error } @@ -94,6 +101,8 @@ func (r *CombinedController) SetupWithManager(ctx context.Context, mgr ctrl.Mana EventRecorder: r.EventRecorder, Clock: r.Clock, + SetCAOnCertificateRequest: r.SetCAOnCertificateRequest, + PostSetupWithManager: r.PostSetupWithManager, }).SetupWithManager(ctx, mgr); err != nil { return fmt.Errorf("CertificateRequestReconciler: %w", err) diff --git a/controllers/combined_controller_integration_test.go b/controllers/combined_controller_integration_test.go index 0aee684..0072795 100644 --- a/controllers/combined_controller_integration_test.go +++ b/controllers/combined_controller_integration_test.go @@ -75,12 +75,12 @@ func TestCombinedControllerTemporaryFailedCertificateRequestRetrigger(t *testing return ctx.Err() } }, - Sign: func(_ context.Context, _ signer.CertificateRequestObject, _ v1alpha1.Issuer) ([]byte, error) { + Sign: func(_ context.Context, _ signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) { select { case err := <-signResult: - return nil, err + return signer.PEMBundle{}, err case <-ctx.Done(): - return nil, ctx.Err() + return signer.PEMBundle{}, ctx.Err() } }, EventRecorder: record.NewFakeRecorder(100), diff --git a/controllers/signer/interface.go b/controllers/signer/interface.go index fcd9e88..0474477 100644 --- a/controllers/signer/interface.go +++ b/controllers/signer/interface.go @@ -22,12 +22,23 @@ import ( "time" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/cert-manager/cert-manager/pkg/util/pki" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/cert-manager/issuer-lib/api/v1alpha1" ) -type Sign func(ctx context.Context, cr CertificateRequestObject, issuerObject v1alpha1.Issuer) ([]byte, error) +// PEMBundle includes the PEM encoded X.509 certificate chain and CA. +// The first certificate in the ChainPEM chain is the leaf certificate, and the +// last certificate in the chain is the highest level non-self-signed certificate. +// The CAPEM certificate is our best guess at the CA that issued the leaf. +// IMORTANT: the CAPEM certificate is only used when the SetCAOnCertificateRequest +// option is enabled in the controller. This option is for backwards compatibility +// only. The use of the CA field and the ca.crt field in the resulting Secret is +// discouraged, instead the CA should be provisioned separately (e.g. using trust-manager). +type PEMBundle pki.PEMBundle + +type Sign func(ctx context.Context, cr CertificateRequestObject, issuerObject v1alpha1.Issuer) (PEMBundle, error) type Check func(ctx context.Context, issuerObject v1alpha1.Issuer) error // CertificateRequestObject is an interface that represents either a diff --git a/internal/testsetups/simple/controller/signer.go b/internal/testsetups/simple/controller/signer.go index f93ca5e..8f79e5c 100644 --- a/internal/testsetups/simple/controller/signer.go +++ b/internal/testsetups/simple/controller/signer.go @@ -67,11 +67,11 @@ func (Signer) Check(ctx context.Context, issuerObject v1alpha1.Issuer) error { return nil } -func (Signer) Sign(ctx context.Context, cr signer.CertificateRequestObject, issuerObject v1alpha1.Issuer) ([]byte, error) { +func (Signer) Sign(ctx context.Context, cr signer.CertificateRequestObject, issuerObject v1alpha1.Issuer) (signer.PEMBundle, error) { // generate random ca private key caPrivateKey, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader) if err != nil { - return nil, err + return signer.PEMBundle{}, err } caCRT := &x509.Certificate{ @@ -90,7 +90,7 @@ func (Signer) Sign(ctx context.Context, cr signer.CertificateRequestObject, issu // load client certificate request clientCRTTemplate, _, _, err := cr.GetRequest() if err != nil { - return nil, err + return signer.PEMBundle{}, err } // create client certificate from template and CA public key @@ -100,5 +100,7 @@ func (Signer) Sign(ctx context.Context, cr signer.CertificateRequestObject, issu } clientCrt := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: clientCRTRaw}) - return clientCrt, nil + return signer.PEMBundle{ + ChainPEM: clientCrt, + }, nil }