diff --git a/pkg/controller/certificaterequests/BUILD.bazel b/pkg/controller/certificaterequests/BUILD.bazel index 53ec20aa4..b0ca735ff 100644 --- a/pkg/controller/certificaterequests/BUILD.bazel +++ b/pkg/controller/certificaterequests/BUILD.bazel @@ -68,6 +68,7 @@ filegroup( srcs = [ ":package-srcs", "//pkg/controller/certificaterequests/acme:all-srcs", + "//pkg/controller/certificaterequests/approver:all-srcs", "//pkg/controller/certificaterequests/ca:all-srcs", "//pkg/controller/certificaterequests/fake:all-srcs", "//pkg/controller/certificaterequests/selfsigned:all-srcs", diff --git a/pkg/controller/certificaterequests/sync.go b/pkg/controller/certificaterequests/sync.go index 4c55f6950..f6c6cc6e1 100644 --- a/pkg/controller/certificaterequests/sync.go +++ b/pkg/controller/certificaterequests/sync.go @@ -29,17 +29,17 @@ import ( apiutil "github.com/jetstack/cert-manager/pkg/api/util" "github.com/jetstack/cert-manager/pkg/apis/certmanager" - v1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" logf "github.com/jetstack/cert-manager/pkg/logs" "github.com/jetstack/cert-manager/pkg/util/pki" ) var ( - certificateRequestGvk = v1.SchemeGroupVersion.WithKind(v1.CertificateRequestKind) + certificateRequestGvk = cmapi.SchemeGroupVersion.WithKind(cmapi.CertificateRequestKind) ) -func (c *Controller) Sync(ctx context.Context, cr *v1.CertificateRequest) (err error) { +func (c *Controller) Sync(ctx context.Context, cr *cmapi.CertificateRequest) (err error) { log := logf.FromContext(ctx) dbg := log.V(logf.DebugLevel) @@ -48,12 +48,18 @@ func (c *Controller) Sync(ctx context.Context, cr *v1.CertificateRequest) (err e return nil } + // If CertificateRequest has not been approved, exit early. + if !apiutil.CertificateRequestHasApproved(cr) { + dbg.Info("certificate request has not been approved") + return nil + } + switch apiutil.CertificateRequestReadyReason(cr) { - case v1.CertificateRequestReasonFailed: + case cmapi.CertificateRequestReasonFailed: dbg.Info("certificate request Ready condition failed so skipping processing") return - case v1.CertificateRequestReasonIssued: + case cmapi.CertificateRequestReasonIssued: dbg.Info("certificate request Ready condition true so skipping processing") return } @@ -99,8 +105,8 @@ func (c *Controller) Sync(ctx context.Context, cr *v1.CertificateRequest) (err e } // check ready condition - if !apiutil.IssuerHasCondition(issuerObj, v1.IssuerCondition{ - Type: v1.IssuerConditionReady, + if !apiutil.IssuerHasCondition(issuerObj, cmapi.IssuerCondition{ + Type: cmapi.IssuerConditionReady, Status: cmmeta.ConditionTrue, }) { c.reporter.Pending(crCopy, nil, "IssuerNotReady", @@ -148,7 +154,7 @@ func (c *Controller) Sync(ctx context.Context, cr *v1.CertificateRequest) (err e return nil } -func (c *Controller) updateCertificateRequestStatusAndAnnotations(ctx context.Context, old, new *v1.CertificateRequest) (*v1.CertificateRequest, error) { +func (c *Controller) updateCertificateRequestStatusAndAnnotations(ctx context.Context, old, new *cmapi.CertificateRequest) (*cmapi.CertificateRequest, error) { log := logf.FromContext(ctx, "updateStatus") // if annotations changed we have to call .Update() and not .UpdateStatus() diff --git a/pkg/controller/certificaterequests/sync_test.go b/pkg/controller/certificaterequests/sync_test.go index bf9830681..686eb9ad7 100644 --- a/pkg/controller/certificaterequests/sync_test.go +++ b/pkg/controller/certificaterequests/sync_test.go @@ -122,7 +122,7 @@ func TestSync(t *testing.T) { }), ) - baseCR := gen.CertificateRequest("test-cr", + baseCRNotApproved := gen.CertificateRequest("test-cr", gen.SetCertificateRequestIsCA(false), gen.SetCertificateRequestCSR(csrRSAPEM), gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ @@ -131,6 +131,16 @@ func TestSync(t *testing.T) { }), ) + baseCR := gen.CertificateRequestFrom(baseCRNotApproved, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionApproved, + Status: cmmeta.ConditionTrue, + Reason: "cert-manager.io", + Message: "Certificate request has been approved by cert-manager.io", + LastTransitionTime: &nowMetaTime, + }), + ) + certRSAPEM := generateSelfSignedCert(t, baseCR, skRSA, fixedClockStart, fixedClockStart.Add(time.Hour*12)) certRSAPEMExpired := generateSelfSignedCert(t, baseCR, skRSA, fixedClockStart.Add(-time.Hour*13), fixedClockStart.Add(-time.Hour*12)) @@ -150,6 +160,46 @@ func TestSync(t *testing.T) { ExpectedActions: []testpkg.Action{}, }, }, + "should return nil (no action) if certificate request is not approved": { + certificateRequest: gen.CertificateRequestFrom(baseCRNotApproved), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{baseIssuer, baseCR}, + ExpectedEvents: []string{}, + ExpectedActions: []testpkg.Action{}, + }, + }, + "should return nil (no action) if certificate request is denied": { + certificateRequest: gen.CertificateRequestFrom(baseCRNotApproved, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionApproved, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonDenied, + Message: "Certificate request has been denied by cert-manager.io", + LastTransitionTime: &nowMetaTime, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{baseIssuer, baseCR}, + ExpectedEvents: []string{}, + ExpectedActions: []testpkg.Action{}, + }, + }, + "should return nil (no action) if certificate request approved is set to false": { + certificateRequest: gen.CertificateRequestFrom(baseCRNotApproved, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionApproved, + Status: cmmeta.ConditionFalse, + Reason: "cert-manager.io", + Message: "Certificate request has not been approved", + LastTransitionTime: &nowMetaTime, + }), + ), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{baseIssuer, baseCR}, + ExpectedEvents: []string{}, + ExpectedActions: []testpkg.Action{}, + }, + }, "should return nil (no action) if certificate request is ready and reason Issued": { certificateRequest: gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{