Updates the base CertificateRequest controller to first check for the

approval condition to be present and set to true, before processing
further

Signed-off-by: joshvanl <vleeuwenjoshua@gmail.com>
This commit is contained in:
joshvanl 2021-03-04 16:28:42 +00:00
parent 2db7582586
commit 1d758a5ccf
3 changed files with 66 additions and 9 deletions

View File

@ -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",

View File

@ -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()

View File

@ -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{