Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Status.CA support #31

Merged
merged 1 commit into from
Jul 25, 2023
Merged

Add Status.CA support #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion controllers/certificaterequest_controller.go
View file
Open in desktop
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
// separately 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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
// separately 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
Open in desktop
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
Open in desktop
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 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
Expand Down
Loading