weiguoqiang/cert-manager
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Ignore failed CRs for previous issuance in certificates-issuing controller

Browse Source 
Issuing controller should only look at 'current' CertificateRequests

Signed-off-by: irbekrm <irbekrm@gmail.com>
This commit is contained in:
irbekrm 2021-12-22 14:51:25 +00:00
parent 52bba1dcdb
commit ff67b2a9a0
2 changed files with 56 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
pkg/controller/certificates/issuing/issuing_controller.go
View File

@ -236,6 +236,24 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
return nil
}
certIssuingCond := apiutil.GetCertificateCondition(crt, cmapi.CertificateConditionIssuing)
crReadyCond := apiutil.GetCertificateRequestCondition(req, cmapi.CertificateRequestConditionReady)
if certIssuingCond == nil {
// This should never happen
log.V(logf.ErrorLevel).Info("Certificate does not have an issuing condition")
return nil
}
// If the CertificateRequest for this revision has failed, but before the
// Issuing condition was set on the Certificate, then it must be a
// failed CertificateRequest from previous issuance for the same
// revision. Leave it to certificate-requests controller to delete the
// CertificateRequest and create a new one.
if req.Status.FailureTime != nil &&
req.Status.FailureTime.Before(certIssuingCond.LastTransitionTime) && crReadyCond.Reason == cmapi.CertificateRequestReasonFailed {
log.V(logf.InfoLevel).Info("Found a failed CertificateRequest from previous issuance, waiting for it to be deleted...")
return nil
}
// Some issuers won't honor the "Denied=True" condition, and we don't want
// to break these issuers. To avoid breaking these issuers, we skip bubbling
// up the "Denied=True" condition from the certificate request object to the
@ -246,8 +264,7 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
// certificate request is "Denied=True" and that the issuer still proceeds
// to adding the "Ready" condition (to either true or false), then we
// consider that this issuer has ignored the "Denied" state.
cond := apiutil.GetCertificateRequestCondition(req, cmapi.CertificateRequestConditionReady)
if cond == nil {
if crReadyCond == nil {
if apiutil.CertificateRequestIsDenied(req) {
return c.failIssueCertificate(ctx, log, crt, apiutil.GetCertificateRequestCondition(req, cmapi.CertificateRequestConditionDenied))
}
@ -258,7 +275,7 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
// If the certificate request has failed, set the last failure time to now,
// and set the Issuing status condition to False with reason.
if cond.Reason == cmapi.CertificateRequestReasonFailed {
if crReadyCond.Reason == cmapi.CertificateRequestReasonFailed {
return c.failIssueCertificate(ctx, log, crt, apiutil.GetCertificateRequestCondition(req, cmapi.CertificateRequestConditionReady))
}
@ -278,7 +295,7 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
// If the CertificateRequest is valid and ready, verify its status and issue
// accordingly.
if cond.Reason == cmapi.CertificateRequestReasonIssued {
if crReadyCond.Reason == cmapi.CertificateRequestReasonIssued {
return c.issueCertificate(ctx, nextRevision, crt, req, pk)
}
@ -290,7 +307,7 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
}
// CertificateRequest is not in a final state so do nothing.
log.V(logf.DebugLevel).Info("CertificateRequest not in final state, waiting...", "reason", cond.Reason)
log.V(logf.DebugLevel).Info("CertificateRequest not in final state, waiting...", "reason", crReadyCond.Reason)
return nil
}

37
pkg/controller/certificates/issuing/issuing_controller_test.go
View File

@ -71,17 +71,17 @@ func TestIssuingController(t *testing.T) {
exampleBundle := internaltest.MustCreateCryptoBundle(t, baseCert.DeepCopy(), fixedClock)
exampleBundleAlt := internaltest.MustCreateCryptoBundle(t, baseCert.DeepCopy(), fixedClock)
metaFixedClockStart := metav1.NewTime(fixedClockStart)
issuingCert := gen.CertificateFrom(baseCert.DeepCopy(),
gen.SetCertificateStatusCondition(cmapi.CertificateCondition{
Type: cmapi.CertificateConditionIssuing,
Status: cmmeta.ConditionTrue,
ObservedGeneration: 3,
LastTransitionTime: &metaFixedClockStart,
}),
)
metaFixedClockStart := metav1.NewTime(fixedClockStart)
tests := map[string]testT{
"if certificate is not in Issuing state, then do nothing": {
certificate: exampleBundle.Certificate,
@ -213,7 +213,38 @@ func TestIssuingController(t *testing.T) {
},
expectedErr: false,
},
"if certificate is in Issuing state, one CertificateRequest that has failed during previous issuance, do nothing": {
certificate: exampleBundle.Certificate,
builder: &testpkg.Builder{
CertManagerObjects: []runtime.Object{
gen.CertificateFrom(issuingCert),
gen.CertificateRequestFrom(exampleBundle.CertificateRequestFailed,
gen.AddCertificateRequestAnnotations(map[string]string{
cmapi.CertificateRequestRevisionAnnotationKey: "2", // Current Certificate revision=1
}),
gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
Type: cmapi.CertificateRequestConditionReady,
Status: cmmeta.ConditionFalse,
Reason: cmapi.CertificateRequestReasonFailed,
Message: "The certificate request failed because of reasons",
}),
gen.SetCertificateRequestFailureTime(metav1.Time{Time: metaFixedClockStart.Time.Add(time.Hour * -1)}),
)},
KubeObjects: []runtime.Object{
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: nextPrivateKeySecretName,
Namespace: exampleBundle.Certificate.Namespace,
},
Data: map[string][]byte{
corev1.TLSPrivateKeyKey: exampleBundle.PrivateKeyBytes,
},
},
},
ExpectedActions: []testpkg.Action{},
},
expectedErr: false,
},
"if certificate is in Issuing state, one CertificateRequest, but has failed, set failed state and log event": {
certificate: exampleBundle.Certificate,
builder: &testpkg.Builder{