diff --git a/pkg/controller/certificaterequests/BUILD.bazel b/pkg/controller/certificaterequests/BUILD.bazel index b7d9b94a1..7842aa6ff 100644 --- a/pkg/controller/certificaterequests/BUILD.bazel +++ b/pkg/controller/certificaterequests/BUILD.bazel @@ -13,6 +13,7 @@ go_library( "//pkg/api/util:go_default_library", "//pkg/apis/certmanager:go_default_library", "//pkg/apis/certmanager/v1alpha2:go_default_library", + "//pkg/apis/meta/v1:go_default_library", "//pkg/client/clientset/versioned:go_default_library", "//pkg/client/listers/certmanager/v1alpha2:go_default_library", "//pkg/controller:go_default_library", diff --git a/pkg/controller/certificaterequests/acme/acme_test.go b/pkg/controller/certificaterequests/acme/acme_test.go index d265394f0..bd66de28a 100644 --- a/pkg/controller/certificaterequests/acme/acme_test.go +++ b/pkg/controller/certificaterequests/acme/acme_test.go @@ -75,6 +75,10 @@ func generateCSR(t *testing.T, secretKey crypto.Signer, commonName string, dnsNa func TestSign(t *testing.T) { baseIssuer := gen.Issuer("test-issuer", gen.SetIssuerACME(cmacme.ACMEIssuer{}), + gen.AddIssuerCondition(cmapi.IssuerCondition{ + Type: cmapi.IssuerConditionReady, + Status: cmmeta.ConditionTrue, + }), ) sk, err := pki.GenerateRSAPrivateKey(2048) @@ -216,6 +220,35 @@ func TestSign(t *testing.T) { }, }, + "should exit nil and set status pending if referenced issuer is not ready": { + certificateRequest: baseCR.DeepCopy(), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), + gen.Issuer(baseIssuer.DeepCopy().Name, + gen.SetIssuerACME(cmacme.ACMEIssuer{}), + )}, + ExpectedEvents: []string{ + "Normal IssuerNotReady Referenced issuer does not have a Ready status condition", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: "Pending", + Message: "Referenced issuer does not have a Ready status condition", + LastTransitionTime: &metaFixedClockStart, + }), + ), + )), + }, + }, + }, + "if we fail to get the order resource due to a transient error then we should report pending and return error to re-sync": { certificateRequest: baseCR.DeepCopy(), builder: &testpkg.Builder{ diff --git a/pkg/controller/certificaterequests/ca/ca_test.go b/pkg/controller/certificaterequests/ca/ca_test.go index 584049701..1a36ac674 100644 --- a/pkg/controller/certificaterequests/ca/ca_test.go +++ b/pkg/controller/certificaterequests/ca/ca_test.go @@ -98,6 +98,10 @@ func generateSelfSignedCertFromCR(t *testing.T, cr *cmapi.CertificateRequest, ke func TestSign(t *testing.T) { baseIssuer := gen.Issuer("test-issuer", gen.SetIssuerCA(cmapi.CAIssuer{SecretName: "root-ca-secret"}), + gen.AddIssuerCondition(cmapi.IssuerCondition{ + Type: cmapi.IssuerConditionReady, + Status: cmmeta.ConditionTrue, + }), ) // Build root RSA CA @@ -114,7 +118,7 @@ func TestSign(t *testing.T) { gen.SetCertificateRequestIsCA(true), gen.SetCertificateRequestCSR(rsaCSR), gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ - Name: baseIssuer.Name, + Name: baseIssuer.DeepCopy().Name, Group: certmanager.GroupName, Kind: "Issuer", }), @@ -154,7 +158,7 @@ func TestSign(t *testing.T) { certificateRequest: baseCR.DeepCopy(), builder: &testpkg.Builder{ KubeObjects: []runtime.Object{}, - CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer.DeepCopy()}, ExpectedEvents: []string{ `Normal SecretMissing Referenced secret default-unit-test-ns/root-ca-secret not found: secret "root-ca-secret" not found`, }, @@ -176,11 +180,15 @@ func TestSign(t *testing.T) { }, }, }, - "a secret with invlaid datashould set condition to pending and wait for re-sync": { + "a secret with invlaid data should set condition to pending and wait for re-sync": { certificateRequest: baseCR.DeepCopy(), builder: &testpkg.Builder{ - KubeObjects: []runtime.Object{badDataSecret}, - CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, + KubeObjects: []runtime.Object{badDataSecret}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), + gen.IssuerFrom(baseIssuer.DeepCopy(), + gen.SetIssuerCA(cmapi.CAIssuer{SecretName: badDataSecret.Name}), + ), + }, ExpectedEvents: []string{ "Normal SecretInvalidData Failed to parse signing CA keypair from secret default-unit-test-ns/root-ca-secret: error decoding private key PEM block", }, @@ -206,7 +214,7 @@ func TestSign(t *testing.T) { certificateRequest: baseCR.DeepCopy(), builder: &testpkg.Builder{ KubeObjects: []runtime.Object{rsaCASecret}, - CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer.DeepCopy()}, ExpectedEvents: []string{ `Normal SecretGetError Failed to get certificate key pair from secret default-unit-test-ns/root-ca-secret: this is a network error`, }, @@ -238,6 +246,35 @@ func TestSign(t *testing.T) { }, expectedErr: true, }, + "should exit nil and set status pending if referenced issuer is not ready": { + certificateRequest: baseCR.DeepCopy(), + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{rsaCASecret}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), + gen.Issuer(baseIssuer.DeepCopy().Name, + gen.SetIssuerCA(cmapi.CAIssuer{}), + )}, + ExpectedEvents: []string{ + "Normal IssuerNotReady Referenced issuer does not have a Ready status condition", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: "Pending", + Message: "Referenced issuer does not have a Ready status condition", + LastTransitionTime: &metaFixedClockStart, + }), + ), + )), + }, + }, + }, "a secret that fails to sign should set condition to failed": { certificateRequest: baseCR.DeepCopy(), templateGenerator: func(*cmapi.CertificateRequest) (*x509.Certificate, error) { @@ -245,7 +282,7 @@ func TestSign(t *testing.T) { }, builder: &testpkg.Builder{ KubeObjects: []runtime.Object{rsaCASecret}, - CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer.DeepCopy()}, ExpectedEvents: []string{ "Warning SigningError Error generating certificate template: this is a sign error", }, @@ -280,7 +317,7 @@ func TestSign(t *testing.T) { }, builder: &testpkg.Builder{ KubeObjects: []runtime.Object{rsaCASecret}, - CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer.DeepCopy()}, ExpectedEvents: []string{ "Normal CertificateIssued Certificate fetched from issuer successfully", }, diff --git a/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go b/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go index 2c7890594..2cd28632f 100644 --- a/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go +++ b/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go @@ -76,6 +76,10 @@ func TestSign(t *testing.T) { baseIssuer := gen.Issuer("test-issuer", gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + gen.AddIssuerCondition(cmapi.IssuerCondition{ + Type: cmapi.IssuerConditionReady, + Status: cmmeta.ConditionTrue, + }), ) skRSA, err := pki.GenerateRSAPrivateKey(2048) @@ -285,6 +289,35 @@ func TestSign(t *testing.T) { }, }, }, + "should exit nil and set status pending if referenced issuer is not ready": { + certificateRequest: baseCR.DeepCopy(), + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), + gen.Issuer(baseIssuer.DeepCopy().Name, + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + )}, + ExpectedEvents: []string{ + "Normal IssuerNotReady Referenced issuer does not have a Ready status condition", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: "Pending", + Message: "Referenced issuer does not have a Ready status condition", + LastTransitionTime: &metaFixedClockStart, + }), + ), + )), + }, + }, + }, "a CertificateRequest that transiently fails a secret lookup should backoff error to retry": { certificateRequest: baseCR.DeepCopy(), builder: &testpkg.Builder{ diff --git a/pkg/controller/certificaterequests/sync.go b/pkg/controller/certificaterequests/sync.go index 32cfc2380..90ba48b7e 100644 --- a/pkg/controller/certificaterequests/sync.go +++ b/pkg/controller/certificaterequests/sync.go @@ -29,6 +29,7 @@ import ( apiutil "github.com/jetstack/cert-manager/pkg/api/util" "github.com/jetstack/cert-manager/pkg/apis/certmanager" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha2" + cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager/validation" logf "github.com/jetstack/cert-manager/pkg/logs" "github.com/jetstack/cert-manager/pkg/util/pki" @@ -99,6 +100,16 @@ func (c *Controller) Sync(ctx context.Context, cr *v1alpha2.CertificateRequest) return nil } + // check ready condition + if !apiutil.IssuerHasCondition(issuerObj, v1alpha2.IssuerCondition{ + Type: v1alpha2.IssuerConditionReady, + Status: cmmeta.ConditionTrue, + }) { + c.reporter.Pending(crCopy, nil, "IssuerNotReady", + "Referenced issuer does not have a Ready status condition") + return nil + } + dbg.Info("validating CertificateRequest resource object") el := validation.ValidateCertificateRequest(crCopy) diff --git a/pkg/controller/certificaterequests/sync_test.go b/pkg/controller/certificaterequests/sync_test.go index 956c364a9..6ba8d6226 100644 --- a/pkg/controller/certificaterequests/sync_test.go +++ b/pkg/controller/certificaterequests/sync_test.go @@ -118,6 +118,10 @@ func TestSync(t *testing.T) { baseIssuer := gen.Issuer("test-issuer", gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + gen.AddIssuerCondition(cmapi.IssuerCondition{ + Type: cmapi.IssuerConditionReady, + Status: cmmeta.ConditionTrue, + }), ) baseCR := gen.CertificateRequest("test-cr", @@ -223,13 +227,8 @@ func TestSync(t *testing.T) { certificateRequest: baseCR.DeepCopy(), builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{baseCR, - gen.Issuer(baseIssuer.Name, - gen.AddIssuerCondition(cmapi.IssuerCondition{ - Type: cmapi.IssuerConditionReady, - Status: cmmeta.ConditionTrue, - }), - // no type set - ), + // no type set + gen.Issuer(baseIssuer.Name), }, ExpectedActions: []testpkg.Action{ testpkg.NewAction(coretesting.NewUpdateSubresourceAction( @@ -252,6 +251,35 @@ func TestSync(t *testing.T) { }, }, }, + "should exit nil and set status pending if referenced issuer is not ready": { + certificateRequest: baseCR.DeepCopy(), + builder: &testpkg.Builder{ + CertManagerObjects: []runtime.Object{baseCR, + gen.Issuer(baseIssuer.Name, + gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}), + ), + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: "Pending", + Message: "Referenced issuer does not have a Ready status condition", + LastTransitionTime: &nowMetaTime, + }), + ), + )), + }, + ExpectedEvents: []string{ + "Normal IssuerNotReady Referenced issuer does not have a Ready status condition", + }, + }, + }, "exit nil and no action if the issuer type does not match ours (its not meant for us)": { certificateRequest: baseCR.DeepCopy(), builder: &testpkg.Builder{ diff --git a/pkg/controller/certificaterequests/vault/vault_test.go b/pkg/controller/certificaterequests/vault/vault_test.go index 6220da087..ca5ca8226 100644 --- a/pkg/controller/certificaterequests/vault/vault_test.go +++ b/pkg/controller/certificaterequests/vault/vault_test.go @@ -99,6 +99,10 @@ func TestSign(t *testing.T) { metaFixedClockStart := metav1.NewTime(fixedClockStart) baseIssuer := gen.Issuer("vault-issuer", gen.SetIssuerVault(cmapi.VaultIssuer{}), + gen.AddIssuerCondition(cmapi.IssuerCondition{ + Type: cmapi.IssuerConditionReady, + Status: cmmeta.ConditionTrue, + }), ) rsaSK, err := pki.GenerateRSAPrivateKey(2048) @@ -250,6 +254,35 @@ func TestSign(t *testing.T) { }, }, }, + "should exit nil and set status pending if referenced issuer is not ready": { + certificateRequest: baseCR.DeepCopy(), + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), + gen.Issuer(baseIssuer.DeepCopy().Name, + gen.SetIssuerVault(cmapi.VaultIssuer{}), + )}, + ExpectedEvents: []string{ + "Normal IssuerNotReady Referenced issuer does not have a Ready status condition", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(baseCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: "Pending", + Message: "Referenced issuer does not have a Ready status condition", + LastTransitionTime: &metaFixedClockStart, + }), + ), + )), + }, + }, + }, "a client with a token secret referenced with token but failed to sign should report fail": { certificateRequest: baseCR.DeepCopy(), builder: &testpkg.Builder{ diff --git a/pkg/controller/certificaterequests/venafi/venafi_test.go b/pkg/controller/certificaterequests/venafi/venafi_test.go index 7415b5518..db990d089 100644 --- a/pkg/controller/certificaterequests/venafi/venafi_test.go +++ b/pkg/controller/certificaterequests/venafi/venafi_test.go @@ -106,6 +106,10 @@ func TestSign(t *testing.T) { baseIssuer := gen.Issuer("test-issuer", gen.SetIssuerVenafi(cmapi.VenafiIssuer{}), + gen.AddIssuerCondition(cmapi.IssuerCondition{ + Type: cmapi.IssuerConditionReady, + Status: cmmeta.ConditionTrue, + }), ) tppIssuer := gen.IssuerFrom(baseIssuer, @@ -306,6 +310,35 @@ func TestSign(t *testing.T) { fakeSecretLister: failGetSecretLister, expectedErr: true, }, + "should exit nil and set status pending if referenced issuer is not ready": { + certificateRequest: cloudCR.DeepCopy(), + builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{cloudCR.DeepCopy(), + gen.Issuer(cloudIssuer.DeepCopy().Name, + gen.SetIssuerVenafi(cmapi.VenafiIssuer{}), + )}, + ExpectedEvents: []string{ + "Normal IssuerNotReady Referenced issuer does not have a Ready status condition", + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(cloudCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: "Pending", + Message: "Referenced issuer does not have a Ready status condition", + LastTransitionTime: &metaFixedClockStart, + }), + ), + )), + }, + }, + }, "tpp: if sign returns pending error then set pending and return err": { certificateRequest: tppCR.DeepCopy(), builder: &controllertest.Builder{