diff --git a/deploy/crds/crd-certificates.yaml b/deploy/crds/crd-certificates.yaml index 16fa096c9..80462702d 100644 --- a/deploy/crds/crd-certificates.yaml +++ b/deploy/crds/crd-certificates.yaml @@ -507,8 +507,8 @@ spec: revisions exceeds this number. If set, revisionHistoryLimit must be a value of `1` or greater. - If unset (`nil`), revisions will not be garbage collected. - Default value is `nil`. + If set to 0, revisions will not be garbage collected. + Default value is `1`. type: integer format: int32 secretName: diff --git a/internal/apis/certmanager/types_certificate.go b/internal/apis/certmanager/types_certificate.go index 82e0c9bbd..3b8423e07 100644 --- a/internal/apis/certmanager/types_certificate.go +++ b/internal/apis/certmanager/types_certificate.go @@ -252,8 +252,8 @@ type CertificateSpec struct { // revisions exceeds this number. // // If set, revisionHistoryLimit must be a value of `1` or greater. - // If unset (`nil`), revisions will not be garbage collected. - // Default value is `nil`. + // If set to 0, revisions will not be garbage collected. + // Default value is `1`. RevisionHistoryLimit *int32 // Defines extra output formats of the private key and signed certificate chain diff --git a/pkg/controller/certificates/revisionmanager/revisionmanager_controller.go b/pkg/controller/certificates/revisionmanager/revisionmanager_controller.go index c2d81e722..d68bb52ba 100644 --- a/pkg/controller/certificates/revisionmanager/revisionmanager_controller.go +++ b/pkg/controller/certificates/revisionmanager/revisionmanager_controller.go @@ -117,9 +117,15 @@ func (c *controller) ProcessItem(ctx context.Context, key types.NamespacedName) log = logf.WithResource(log, crt) - // If RevisionHistoryLimit is nil, don't attempt to garbage collect old - // CertificateRequests + // If RevisionHistoryLimit is nil, then default to 1 if crt.Spec.RevisionHistoryLimit == nil { + defaultRevisionHistoryLimit := int32(1) + crt.Spec.RevisionHistoryLimit = &defaultRevisionHistoryLimit + } + + // If RevisionHistoryLimit is 0, don't attempt to garbage collect old + // CertificateRequests + if *crt.Spec.RevisionHistoryLimit == 0 { return nil } diff --git a/pkg/controller/certificates/revisionmanager/revisionmanager_controller_test.go b/pkg/controller/certificates/revisionmanager/revisionmanager_controller_test.go index 579abd60f..8b46cbd5b 100644 --- a/pkg/controller/certificates/revisionmanager/revisionmanager_controller_test.go +++ b/pkg/controller/certificates/revisionmanager/revisionmanager_controller_test.go @@ -81,7 +81,6 @@ func TestProcessItem(t *testing.T) { "do nothing if Certificate is not in a Ready=True state": { certificate: gen.CertificateFrom(baseCrt, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{Type: cmapi.CertificateConditionIssuing, Status: cmmeta.ConditionFalse}), - gen.SetCertificateRevisionHistoryLimit(1), ), requests: []runtime.Object{ gen.CertificateRequestFrom(baseCR, @@ -96,13 +95,11 @@ func TestProcessItem(t *testing.T) { "do nothing if no requests exist": { certificate: gen.CertificateFrom(baseCrt, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue}), - gen.SetCertificateRevisionHistoryLimit(1), ), }, "do nothing if requests don't have or bad revisions set": { certificate: gen.CertificateFrom(baseCrt, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue}), - gen.SetCertificateRevisionHistoryLimit(1), ), requests: []runtime.Object{ gen.CertificateRequestFrom(baseCR, @@ -117,7 +114,6 @@ func TestProcessItem(t *testing.T) { "do nothing if requests aren't owned by this Certificate": { certificate: gen.CertificateFrom(baseCrt, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue}), - gen.SetCertificateRevisionHistoryLimit(1), ), requests: []runtime.Object{ gen.CertificateRequestFrom(baseCRNoOwner, @@ -146,9 +142,10 @@ func TestProcessItem(t *testing.T) { ), }, }, - "do nothing if revision limit is not set": { + "do nothing if revision limit is not set to 0": { certificate: gen.CertificateFrom(baseCrt, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue}), + gen.SetCertificateRevisionHistoryLimit(0), ), requests: []runtime.Object{ gen.CertificateRequestFrom(baseCR, @@ -161,10 +158,9 @@ func TestProcessItem(t *testing.T) { ), }, }, - "delete 1 request if limit is 1 and 2 requests exist": { + "delete 1 request if 2 requests exist since the default limit is 1": { certificate: gen.CertificateFrom(baseCrt, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue}), - gen.SetCertificateRevisionHistoryLimit(1), ), requests: []runtime.Object{ gen.CertificateRequestFrom(baseCR,