Skip to content

Commit

Permalink
add support for the CA status field on CertificateRequest resources
Browse files Browse the repository at this point in the history 
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
  • Loading branch information
@inteon
inteon committed Jun 25, 2023
1 parent 2e8fe7f commit 545b152
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 59 deletions.
12 changes: 11 additions & 1 deletion controllers/certificaterequest_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
// seperately using a tool such as trust-manager.
SetCAOnCertificateRequest bool

PostSetupWithManager func(context.Context, schema.GroupVersionKind, ctrl.Manager, controller.Controller) error
}

Expand Down Expand Up @@ -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,
Expand Down
12 changes: 7 additions & 5 deletions controllers/certificaterequest_controller_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down Expand Up @@ -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),
Expand Down
46 changes: 24 additions & 22 deletions controllers/certificaterequest_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion controllers/certificatesigningrequest_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
46 changes: 24 additions & 22 deletions controllers/certificatesigningrequest_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions controllers/combined_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
// seperately using a tool such as trust-manager.
SetCAOnCertificateRequest bool

PostSetupWithManager func(context.Context, schema.GroupVersionKind, ctrl.Manager, controller.Controller) error
}

Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions controllers/combined_controller_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
13 changes: 12 additions & 1 deletion controllers/signer/interface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 seperately (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
Expand Down
Loading

0 comments on commit 545b152

Please sign in to comment.