From 7e33256b6868d02e5e08e0281c5d4fc1b3089e32 Mon Sep 17 00:00:00 2001 From: Gus Parvin Date: Tue, 13 Nov 2018 16:16:29 +0000 Subject: [PATCH 1/5] changes to add a NotAfter field to the cert status Signed-off-by: Gus Parvin --- .../reference/output/reference/api-docs/index.html | 4 ++++ pkg/apis/certmanager/v1alpha1/types_certificate.go | 1 + pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go | 9 +++++++++ pkg/controller/certificates/sync.go | 3 +++ pkg/issuer/acme/issue.go | 3 +++ pkg/issuer/vault/BUILD.bazel | 1 + pkg/issuer/vault/issue.go | 4 ++++ pkg/util/pki/BUILD.bazel | 1 + pkg/util/pki/csr.go | 7 ++++++- test/util/util.go | 5 +++++ 10 files changed, 37 insertions(+), 1 deletion(-) diff --git a/docs/generated/reference/output/reference/api-docs/index.html b/docs/generated/reference/output/reference/api-docs/index.html index 90ac3cd49..5a4ff9791 100755 --- a/docs/generated/reference/output/reference/api-docs/index.html +++ b/docs/generated/reference/output/reference/api-docs/index.html @@ -146,6 +146,10 @@ Appears In: lastFailureTime
Time + +notAfter
Time + +
diff --git a/pkg/apis/certmanager/v1alpha1/types_certificate.go b/pkg/apis/certmanager/v1alpha1/types_certificate.go index 67e5fb057..f07b2c454 100644 --- a/pkg/apis/certmanager/v1alpha1/types_certificate.go +++ b/pkg/apis/certmanager/v1alpha1/types_certificate.go @@ -103,6 +103,7 @@ type ACMECertificateConfig struct { type CertificateStatus struct { Conditions []CertificateCondition `json:"conditions,omitempty"` LastFailureTime *metav1.Time `json:"lastFailureTime,omitempty"` + NotAfter *metav1.Time `json:"notAfter,omitempty"` } // CertificateCondition contains condition information for an Certificate. diff --git a/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go index 4a26fc512..40fc48dd3 100644 --- a/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go @@ -515,6 +515,15 @@ func (in *CertificateStatus) DeepCopyInto(out *CertificateStatus) { (*in).DeepCopyInto(*out) } } + if in.NotAfter != nil { + in, out := &in.NotAfter, &out.NotAfter + if *in == nil { + *out = nil + } else { + *out = new(v1.Time) + (*in).DeepCopyInto(*out) + } + } return } diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index b23f86694..3eba6e259 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -191,6 +191,9 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (reque // end checking if the TLS certificate is valid/needs a re-issue or renew + metaNotAfter := metav1.NewTime(cert.NotAfter) + crtCopy.Status.NotAfter = &metaNotAfter + return false, nil } diff --git a/pkg/issuer/acme/issue.go b/pkg/issuer/acme/issue.go index dd2c25a6f..04250aa8a 100644 --- a/pkg/issuer/acme/issue.go +++ b/pkg/issuer/acme/issue.go @@ -182,6 +182,9 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss return a.retryOrder(crt, existingOrder) } + metaExpireTime := metav1.NewTime(x509Cert.NotAfter) + crt.Status.NotAfter = &metaExpireTime + if a.Context.IssuerOptions.CertificateNeedsRenew(x509Cert) { // existing order's certificate is near expiry return a.retryOrder(crt, existingOrder) diff --git a/pkg/issuer/vault/BUILD.bazel b/pkg/issuer/vault/BUILD.bazel index 3922554b1..703ee4aec 100644 --- a/pkg/issuer/vault/BUILD.bazel +++ b/pkg/issuer/vault/BUILD.bazel @@ -20,6 +20,7 @@ go_library( "//vendor/github.com/hashicorp/vault/api:go_default_library", "//vendor/github.com/hashicorp/vault/helper/certutil:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", ], ) diff --git a/pkg/issuer/vault/issue.go b/pkg/issuer/vault/issue.go index f15c2352e..afe5fd18b 100644 --- a/pkg/issuer/vault/issue.go +++ b/pkg/issuer/vault/issue.go @@ -36,6 +36,7 @@ import ( "github.com/jetstack/cert-manager/pkg/util/kube" "github.com/jetstack/cert-manager/pkg/util/pki" k8sErrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -103,6 +104,9 @@ func (v *Vault) obtainCertificate(ctx context.Context, crt *v1alpha1.Certificate return nil, nil, nil, err } + metaExpireTime := metav1.NewTime(time.Now().Add(defaultCertificateDuration)) + crt.Status.NotAfter = &metaExpireTime + keyBytes, err := pki.EncodePrivateKey(signeeKey) if err != nil { return nil, nil, nil, err diff --git a/pkg/util/pki/BUILD.bazel b/pkg/util/pki/BUILD.bazel index 5936b0526..abb4f7c39 100644 --- a/pkg/util/pki/BUILD.bazel +++ b/pkg/util/pki/BUILD.bazel @@ -12,6 +12,7 @@ go_library( deps = [ "//pkg/apis/certmanager/v1alpha1:go_default_library", "//pkg/util/errors:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", ], ) diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index 0c059df5b..ee4442da5 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -28,6 +28,7 @@ import ( "time" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // CommonNameForCertificate returns the common name that should be used for the @@ -149,6 +150,10 @@ func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) keyUsages |= x509.KeyUsageCertSign } + expireTime := time.Now().Add(defaultNotAfter) + metaExpireTime := metav1.NewTime(expireTime) + crt.Status.NotAfter = &metaExpireTime + return &x509.Certificate{ Version: 3, BasicConstraintsValid: true, @@ -160,7 +165,7 @@ func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) CommonName: commonName, }, NotBefore: time.Now(), - NotAfter: time.Now().Add(defaultNotAfter), + NotAfter: expireTime, // see http://golang.org/pkg/crypto/x509/#KeyUsage KeyUsage: keyUsages, DNSNames: dnsNames, diff --git a/test/util/util.go b/test/util/util.go index e70b9857c..5b75d86ed 100644 --- a/test/util/util.go +++ b/test/util/util.go @@ -283,6 +283,11 @@ func WaitCertificateIssuedValid(certClient clientset.CertificateInterface, secre return false, nil } + if !cert.NotAfter.Equal(certificate.Status.NotAfter.Time) { + glog.Info("Expected certificate expire date to be %v, but got %v", certificate.Status.NotAfter, cert.NotAfter) + return false, nil + } + label, ok := secret.Labels[v1alpha1.CertificateNameKey] if !ok { return false, fmt.Errorf("Expected secret to have certificate-name label, but had none") From ff1a8534fa8ce8da281cdb7b19bb5180b17e64e3 Mon Sep 17 00:00:00 2001 From: Gus Parvin Date: Wed, 14 Nov 2018 15:11:56 +0000 Subject: [PATCH 2/5] remove changes in issuers that seems to not be needed Signed-off-by: Gus Parvin --- .../reference/output/reference/api-docs/index.html | 2 +- pkg/apis/certmanager/v1alpha1/types_certificate.go | 5 ++++- pkg/issuer/acme/issue.go | 3 --- pkg/issuer/vault/BUILD.bazel | 1 - pkg/issuer/vault/issue.go | 4 ---- pkg/util/pki/BUILD.bazel | 1 - pkg/util/pki/csr.go | 7 +------ 7 files changed, 6 insertions(+), 17 deletions(-) diff --git a/docs/generated/reference/output/reference/api-docs/index.html b/docs/generated/reference/output/reference/api-docs/index.html index 5a4ff9791..c988f0a01 100755 --- a/docs/generated/reference/output/reference/api-docs/index.html +++ b/docs/generated/reference/output/reference/api-docs/index.html @@ -148,7 +148,7 @@ Appears In: notAfter
Time - +The expiration time of the certificate stored in the secret named by this resource in spec.secretName. diff --git a/pkg/apis/certmanager/v1alpha1/types_certificate.go b/pkg/apis/certmanager/v1alpha1/types_certificate.go index f07b2c454..13fa14364 100644 --- a/pkg/apis/certmanager/v1alpha1/types_certificate.go +++ b/pkg/apis/certmanager/v1alpha1/types_certificate.go @@ -103,7 +103,10 @@ type ACMECertificateConfig struct { type CertificateStatus struct { Conditions []CertificateCondition `json:"conditions,omitempty"` LastFailureTime *metav1.Time `json:"lastFailureTime,omitempty"` - NotAfter *metav1.Time `json:"notAfter,omitempty"` + + // The expiration time of the certificate stored in the secret named + // by this resource in spec.secretName. + NotAfter *metav1.Time `json:"notAfter,omitempty"` } // CertificateCondition contains condition information for an Certificate. diff --git a/pkg/issuer/acme/issue.go b/pkg/issuer/acme/issue.go index 04250aa8a..dd2c25a6f 100644 --- a/pkg/issuer/acme/issue.go +++ b/pkg/issuer/acme/issue.go @@ -182,9 +182,6 @@ func (a *Acme) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Iss return a.retryOrder(crt, existingOrder) } - metaExpireTime := metav1.NewTime(x509Cert.NotAfter) - crt.Status.NotAfter = &metaExpireTime - if a.Context.IssuerOptions.CertificateNeedsRenew(x509Cert) { // existing order's certificate is near expiry return a.retryOrder(crt, existingOrder) diff --git a/pkg/issuer/vault/BUILD.bazel b/pkg/issuer/vault/BUILD.bazel index 703ee4aec..3922554b1 100644 --- a/pkg/issuer/vault/BUILD.bazel +++ b/pkg/issuer/vault/BUILD.bazel @@ -20,7 +20,6 @@ go_library( "//vendor/github.com/hashicorp/vault/api:go_default_library", "//vendor/github.com/hashicorp/vault/helper/certutil:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", ], ) diff --git a/pkg/issuer/vault/issue.go b/pkg/issuer/vault/issue.go index afe5fd18b..f15c2352e 100644 --- a/pkg/issuer/vault/issue.go +++ b/pkg/issuer/vault/issue.go @@ -36,7 +36,6 @@ import ( "github.com/jetstack/cert-manager/pkg/util/kube" "github.com/jetstack/cert-manager/pkg/util/pki" k8sErrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -104,9 +103,6 @@ func (v *Vault) obtainCertificate(ctx context.Context, crt *v1alpha1.Certificate return nil, nil, nil, err } - metaExpireTime := metav1.NewTime(time.Now().Add(defaultCertificateDuration)) - crt.Status.NotAfter = &metaExpireTime - keyBytes, err := pki.EncodePrivateKey(signeeKey) if err != nil { return nil, nil, nil, err diff --git a/pkg/util/pki/BUILD.bazel b/pkg/util/pki/BUILD.bazel index abb4f7c39..5936b0526 100644 --- a/pkg/util/pki/BUILD.bazel +++ b/pkg/util/pki/BUILD.bazel @@ -12,7 +12,6 @@ go_library( deps = [ "//pkg/apis/certmanager/v1alpha1:go_default_library", "//pkg/util/errors:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", ], ) diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index ee4442da5..0c059df5b 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -28,7 +28,6 @@ import ( "time" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // CommonNameForCertificate returns the common name that should be used for the @@ -150,10 +149,6 @@ func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) keyUsages |= x509.KeyUsageCertSign } - expireTime := time.Now().Add(defaultNotAfter) - metaExpireTime := metav1.NewTime(expireTime) - crt.Status.NotAfter = &metaExpireTime - return &x509.Certificate{ Version: 3, BasicConstraintsValid: true, @@ -165,7 +160,7 @@ func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) CommonName: commonName, }, NotBefore: time.Now(), - NotAfter: expireTime, + NotAfter: time.Now().Add(defaultNotAfter), // see http://golang.org/pkg/crypto/x509/#KeyUsage KeyUsage: keyUsages, DNSNames: dnsNames, From aceb9970329b347e82eecc2781863b293156a7f0 Mon Sep 17 00:00:00 2001 From: Gus Parvin Date: Wed, 14 Nov 2018 16:34:52 +0000 Subject: [PATCH 3/5] possible timing issue with the e2e test and the NotAfter status field Signed-off-by: Gus Parvin --- test/util/util.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/util/util.go b/test/util/util.go index 5b75d86ed..8d3318924 100644 --- a/test/util/util.go +++ b/test/util/util.go @@ -283,6 +283,10 @@ func WaitCertificateIssuedValid(certClient clientset.CertificateInterface, secre return false, nil } + if certificate.Status.NotAfter == nil { + glog.Infof("No certificate expiration found for Certificate %q", name) + return false, nil + } if !cert.NotAfter.Equal(certificate.Status.NotAfter.Time) { glog.Info("Expected certificate expire date to be %v, but got %v", certificate.Status.NotAfter, cert.NotAfter) return false, nil From ff3f198eca1918d1310dee043ebb32bde1206086 Mon Sep 17 00:00:00 2001 From: Gus Parvin Date: Thu, 15 Nov 2018 14:07:34 +0000 Subject: [PATCH 4/5] make sure the expire time is set when the certificate is in an infinite renewal loop Signed-off-by: Gus Parvin --- pkg/controller/certificates/sync.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 3eba6e259..17b93c4bc 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -179,6 +179,9 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (reque return c.issue(ctx, i, crtCopy) } + metaNotAfter := metav1.NewTime(cert.NotAfter) + crtCopy.Status.NotAfter = &metaNotAfter + // check if the certificate needs renewal needsRenew := c.Context.IssuerOptions.CertificateNeedsRenew(cert) if needsRenew { @@ -191,9 +194,6 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (reque // end checking if the TLS certificate is valid/needs a re-issue or renew - metaNotAfter := metav1.NewTime(cert.NotAfter) - crtCopy.Status.NotAfter = &metaNotAfter - return false, nil } From acc0fa887b99a173106e5e0128f99ee47accbd27 Mon Sep 17 00:00:00 2001 From: Gus Parvin Date: Thu, 15 Nov 2018 14:30:31 +0000 Subject: [PATCH 5/5] set the NotAfter time as soon as the cert is parsed successfully Signed-off-by: Gus Parvin --- pkg/controller/certificates/sync.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 17b93c4bc..20885f0c3 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -156,6 +156,9 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (reque return false, err } + metaNotAfter := metav1.NewTime(cert.NotAfter) + crtCopy.Status.NotAfter = &metaNotAfter + // begin checking if the TLS certificate is valid/needs a re-issue or renew // check if the private key is the corresponding pair to the certificate @@ -179,9 +182,6 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (reque return c.issue(ctx, i, crtCopy) } - metaNotAfter := metav1.NewTime(cert.NotAfter) - crtCopy.Status.NotAfter = &metaNotAfter - // check if the certificate needs renewal needsRenew := c.Context.IssuerOptions.CertificateNeedsRenew(cert) if needsRenew {