Merge pull request #4092 from irbekrm/renew_override

Remove the default renewBefore value

deploy/crds/crd-certificates.yaml

@@ -199,7 +199,7 @@ spec:
                       description: RotationPolicy controls how private keys should be regenerated when a re-issuance is being processed. If set to Never, a private key will only be generated if one does not already exist in the target `spec.secretName`. If one does exists but it does not have the correct algorithm or size, a warning will be raised to await user intervention. If set to Always, a private key matching the specified requirements will be generated whenever a re-issuance occurs. Default is 'Never' for backward compatibility.
                       type: string
                 renewBefore:
-                  description: The amount of time before the currently issued certificate's `notAfter` time that cert-manager will begin to attempt to renew the certificate. If unset this defaults to 30 days. Certificate will be renewed either 2/3 through its duration or `renewBefore` period before its expiry, whichever is later. Minimum accepted value is 5 minutes. Value must be in units accepted by Go time.ParseDuration https://golang.org/pkg/time/#ParseDuration
+                  description: How long before the currently issued certificate's expiry cert-manager should renew the certificate. The default is 2/3 of the issued certificate's duration. Minimum accepted value is 5 minutes. Value must be in units accepted by Go time.ParseDuration https://golang.org/pkg/time/#ParseDuration
                   type: string
                 revisionHistoryLimit:
                   description: revisionHistoryLimit is the maximum number of CertificateRequest revisions that are maintained in the Certificate's history. Each revision represents a single `CertificateRequest` created by this Certificate, either when it was created, renewed, or Spec was changed. Revisions will be removed by oldest first if the number of revisions exceeds this number. If set, revisionHistoryLimit must be a value of `1` or greater. If unset (`nil`), revisions will not be garbage collected. Default value is `nil`.
@@ -497,7 +497,7 @@ spec:
                       description: RotationPolicy controls how private keys should be regenerated when a re-issuance is being processed. If set to Never, a private key will only be generated if one does not already exist in the target `spec.secretName`. If one does exists but it does not have the correct algorithm or size, a warning will be raised to await user intervention. If set to Always, a private key matching the specified requirements will be generated whenever a re-issuance occurs. Default is 'Never' for backward compatibility.
                       type: string
                 renewBefore:
-                  description: The amount of time before the currently issued certificate's `notAfter` time that cert-manager will begin to attempt to renew the certificate. If unset this defaults to 30 days. Certificate will be renewed either 2/3 through its duration or `renewBefore` period before its expiry, whichever is later. Minimum accepted value is 5 minutes. Value must be in units accepted by Go time.ParseDuration https://golang.org/pkg/time/#ParseDuration
+                  description: How long before the currently issued certificate's expiry cert-manager should renew the certificate. The default is 2/3 of the issued certificate's duration. Minimum accepted value is 5 minutes. Value must be in units accepted by Go time.ParseDuration https://golang.org/pkg/time/#ParseDuration
                   type: string
                 revisionHistoryLimit:
                   description: revisionHistoryLimit is the maximum number of CertificateRequest revisions that are maintained in the Certificate's history. Each revision represents a single `CertificateRequest` created by this Certificate, either when it was created, renewed, or Spec was changed. Revisions will be removed by oldest first if the number of revisions exceeds this number. If set, revisionHistoryLimit must be a value of `1` or greater. If unset (`nil`), revisions will not be garbage collected. Default value is `nil`.
@@ -802,7 +802,7 @@ spec:
                       description: Size is the key bit size of the corresponding private key for this certificate. If `algorithm` is set to `RSA`, valid values are `2048`, `4096` or `8192`, and will default to `2048` if not specified. If `algorithm` is set to `ECDSA`, valid values are `256`, `384` or `521`, and will default to `256` if not specified. No other values are allowed.
                       type: integer
                 renewBefore:
-                  description: The amount of time before the currently issued certificate's `notAfter` time that cert-manager will begin to attempt to renew the certificate. If unset this defaults to 30 days. Certificate will be renewed either 2/3 through its duration or `renewBefore` period before its expiry, whichever is later. Minimum accepted value is 5 minutes. Value must be in units accepted by Go time.ParseDuration https://golang.org/pkg/time/#ParseDuration
+                  description: How long before the currently issued certificate's expiry cert-manager should renew the certificate. The default is 2/3 of the issued certificate's duration. Minimum accepted value is 5 minutes. Value must be in units accepted by Go time.ParseDuration https://golang.org/pkg/time/#ParseDuration
                   type: string
                 revisionHistoryLimit:
                   description: revisionHistoryLimit is the maximum number of CertificateRequest revisions that are maintained in the Certificate's history. Each revision represents a single `CertificateRequest` created by this Certificate, either when it was created, renewed, or Spec was changed. Revisions will be removed by oldest first if the number of revisions exceeds this number. If set, revisionHistoryLimit must be a value of `1` or greater. If unset (`nil`), revisions will not be garbage collected. Default value is `nil`.
@@ -1107,7 +1107,7 @@ spec:
                       description: Size is the key bit size of the corresponding private key for this certificate. If `algorithm` is set to `RSA`, valid values are `2048`, `4096` or `8192`, and will default to `2048` if not specified. If `algorithm` is set to `ECDSA`, valid values are `256`, `384` or `521`, and will default to `256` if not specified. No other values are allowed.
                       type: integer
                 renewBefore:
-                  description: The amount of time before the currently issued certificate's `notAfter` time that cert-manager will begin to attempt to renew the certificate. If unset this defaults to 30 days. Certificate will be renewed either 2/3 through its duration or `renewBefore` period before its expiry, whichever is later. Minimum accepted value is 5 minutes. Value must be in units accepted by Go time.ParseDuration https://golang.org/pkg/time/#ParseDuration
+                  description: How long before the currently issued certificate's expiry cert-manager should renew the certificate. The default is 2/3 of the issued certificate's duration. Minimum accepted value is 5 minutes. Value must be in units accepted by Go time.ParseDuration https://golang.org/pkg/time/#ParseDuration
                   type: string
                 revisionHistoryLimit:
                   description: revisionHistoryLimit is the maximum number of CertificateRequest revisions that are maintained in the Certificate's history. Each revision represents a single `CertificateRequest` created by this Certificate, either when it was created, renewed, or Spec was changed. Revisions will be removed by oldest first if the number of revisions exceeds this number. If set, revisionHistoryLimit must be a value of `1` or greater. If unset (`nil`), revisions will not be garbage collected. Default value is `nil`.

pkg/apis/certmanager/v1/const.go

@@ -28,7 +28,7 @@ const (
 	// minimum certificate duration before certificate expiration
 	MinimumRenewBefore = time.Minute * 5
 
-	// Default duration before certificate expiration if  Issuer.spec.renewBefore is not set
+	// Deprecated: the default is now 2/3 of Certificate's duration
 	DefaultRenewBefore = time.Hour * 24 * 30
 )
 

pkg/apis/certmanager/v1/types_certificate.go

@@ -105,12 +105,10 @@ type CertificateSpec struct {
 	// +optional
 	Duration *metav1.Duration `json:"duration,omitempty"`
 
-	// The amount of time before the currently issued certificate's `notAfter`
-	// time that cert-manager will begin to attempt to renew the certificate. If
-	// unset this defaults to 30 days. Certificate will be renewed either 2/3
-	// through its duration or `renewBefore` period before its expiry, whichever
-	// is later. Minimum accepted value is 5 minutes. Value must be in units
-	// accepted by Go time.ParseDuration
+	// How long before the currently issued certificate's expiry
+	// cert-manager should renew the certificate. The default is 2/3 of the
+	// issued certificate's duration. Minimum accepted value is 5 minutes.
+	// Value must be in units accepted by Go time.ParseDuration
 	// https://golang.org/pkg/time/#ParseDuration
 	// +optional
 	RenewBefore *metav1.Duration `json:"renewBefore,omitempty"`

pkg/apis/certmanager/v1alpha2/const.go

@@ -28,7 +28,7 @@ const (
 	// minimum certificate duration before certificate expiration
 	MinimumRenewBefore = time.Minute * 5
 
-	// Default duration before certificate expiration if  Issuer.spec.renewBefore is not set
+	// Deprecated: the default is now 2/3 of Certificate's duration
 	DefaultRenewBefore = time.Hour * 24 * 30
 )
 

pkg/apis/certmanager/v1alpha2/types_certificate.go

@@ -105,12 +105,10 @@ type CertificateSpec struct {
 	// +optional
 	Duration *metav1.Duration `json:"duration,omitempty"`
 
-	// The amount of time before the currently issued certificate's `notAfter`
-	// time that cert-manager will begin to attempt to renew the certificate. If
-	// unset this defaults to 30 days. Certificate will be renewed either 2/3
-	// through its duration or `renewBefore` period before its expiry, whichever
-	// is later. Minimum accepted value is 5 minutes. Value must be in units
-	// accepted by Go time.ParseDuration
+	// How long before the currently issued certificate's expiry
+	// cert-manager should renew the certificate. The default is 2/3 of the
+	// issued certificate's duration. Minimum accepted value is 5 minutes.
+	// Value must be in units accepted by Go time.ParseDuration
 	// https://golang.org/pkg/time/#ParseDuration
 	// +optional
 	RenewBefore *metav1.Duration `json:"renewBefore,omitempty"`

pkg/apis/certmanager/v1alpha3/const.go

@@ -28,7 +28,7 @@ const (
 	// minimum certificate duration before certificate expiration
 	MinimumRenewBefore = time.Minute * 5
 
-	// Default duration before certificate expiration if  Issuer.spec.renewBefore is not set
+	// Deprecated: the default is now 2/3 of Certificate's duration
 	DefaultRenewBefore = time.Hour * 24 * 30
 )
 

pkg/apis/certmanager/v1alpha3/types_certificate.go

@@ -103,12 +103,10 @@ type CertificateSpec struct {
 	// +optional
 	Duration *metav1.Duration `json:"duration,omitempty"`
 
-	// The amount of time before the currently issued certificate's `notAfter`
-	// time that cert-manager will begin to attempt to renew the certificate. If
-	// unset this defaults to 30 days. Certificate will be renewed either 2/3
-	// through its duration or `renewBefore` period before its expiry, whichever
-	// is later. Minimum accepted value is 5 minutes. Value must be in units
-	// accepted by Go time.ParseDuration
+	// How long before the currently issued certificate's expiry
+	// cert-manager should renew the certificate. The default is 2/3 of the
+	// issued certificate's duration. Minimum accepted value is 5 minutes.
+	// Value must be in units accepted by Go time.ParseDuration
 	// https://golang.org/pkg/time/#ParseDuration
 	// +optional
 	RenewBefore *metav1.Duration `json:"renewBefore,omitempty"`

pkg/apis/certmanager/v1beta1/const.go

@@ -28,7 +28,7 @@ const (
 	// minimum certificate duration before certificate expiration
 	MinimumRenewBefore = time.Minute * 5
 
-	// Default duration before certificate expiration if  Issuer.spec.renewBefore is not set
+	// Deprecated: the default is now 2/3 of Certificate's duration
 	DefaultRenewBefore = time.Hour * 24 * 30
 )
 

pkg/apis/certmanager/v1beta1/types_certificate.go

@@ -104,12 +104,10 @@ type CertificateSpec struct {
 	// +optional
 	Duration *metav1.Duration `json:"duration,omitempty"`
 
-	// The amount of time before the currently issued certificate's `notAfter`
-	// time that cert-manager will begin to attempt to renew the certificate. If
-	// unset this defaults to 30 days. Certificate will be renewed either 2/3
-	// through its duration or `renewBefore` period before its expiry, whichever
-	// is later. Minimum accepted value is 5 minutes. Value must be in units
-	// accepted by Go time.ParseDuration
+	// How long before the currently issued certificate's expiry
+	// cert-manager should renew the certificate. The default is 2/3 of the
+	// issued certificate's duration. Minimum accepted value is 5 minutes.
+	// Value must be in units accepted by Go time.ParseDuration
 	// https://golang.org/pkg/time/#ParseDuration
 	// +optional
 	RenewBefore *metav1.Duration `json:"renewBefore,omitempty"`

pkg/controller/certificates/readiness/readiness_controller.go

@@ -242,7 +242,7 @@ func (c *controllerWrapper) Register(ctx *controllerpkg.Context) (workqueue.Rate
 		ctx.KubeSharedInformerFactory,
 		ctx.SharedInformerFactory,
 		NewReadinessPolicyChain(ctx.Clock),
-		certificates.RenewalTimeWrapper(cmapi.DefaultRenewBefore),
+		certificates.RenewalTime,
 		policyEvaluator,
 	)
 	c.controller = ctrl

pkg/controller/certificates/trigger/policies/policies.go

@@ -70,15 +70,15 @@ func (c Chain) Evaluate(input Input) (string, string, bool) {
 	return "", "", false
 }
 
-func NewTriggerPolicyChain(c clock.Clock, defaultRenewBeforeExpiryDuration time.Duration) Chain {
+func NewTriggerPolicyChain(c clock.Clock) Chain {
 	return Chain{
 		SecretDoesNotExist,
 		SecretIsMissingData,
 		SecretPublicKeysDiffer,
 		SecretPrivateKeyMatchesSpec,
 		SecretIssuerAnnotationsNotUpToDate,
 		CurrentCertificateRequestNotValidForSpec,
-		CurrentCertificateNearingExpiry(c, defaultRenewBeforeExpiryDuration),
+		CurrentCertificateNearingExpiry(c),
 	}
 }
 
@@ -195,7 +195,7 @@ func currentSecretValidForSpec(input Input) (string, string, bool) {
 // CurrentCertificateNearingExpiry returns a policy function that can be used to
 // check whether an X.509 cert currently issued for a Certificate should be
 // renewed.
-func CurrentCertificateNearingExpiry(c clock.Clock, defaultRenewBeforeExpiryDuration time.Duration) Func {
+func CurrentCertificateNearingExpiry(c clock.Clock) Func {
 
 	return func(input Input) (string, string, bool) {
 
@@ -213,8 +213,7 @@ func CurrentCertificateNearingExpiry(c clock.Clock, defaultRenewBeforeExpiryDura
 		notBefore := metav1.NewTime(x509cert.NotBefore)
 		notAfter := metav1.NewTime(x509cert.NotAfter)
 		crt := input.Certificate
-		renewalTimeCalculator := certificates.RenewalTimeWrapper(defaultRenewBeforeExpiryDuration)
-		renewalTime := renewalTimeCalculator(notBefore.Time, notAfter.Time, crt.Spec.RenewBefore)
+		renewalTime := certificates.RenewalTime(notBefore.Time, notAfter.Time, crt.Spec.RenewBefore)
 
 		renewIn := renewalTime.Time.Sub(c.Now())
 		if renewIn > 0 {

pkg/controller/certificates/trigger/policies/policies_test.go

@@ -485,9 +485,7 @@ func TestDefaultPolicyChain(t *testing.T) {
 			},
 		},
 	}
-	// we don't really test default renewal time here, it's just passed through
-	someDefaultRenewalTime := time.Hour * 5
-	policyChain := NewTriggerPolicyChain(clock, someDefaultRenewalTime)
+	policyChain := NewTriggerPolicyChain(clock)
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
 			reason, message, reissue := policyChain.Evaluate(Input{

pkg/controller/certificates/trigger/trigger_controller.go

@@ -281,7 +281,7 @@ func (c *controllerWrapper) Register(ctx *controllerpkg.Context) (workqueue.Rate
 		ctx.SharedInformerFactory,
 		ctx.Recorder,
 		ctx.Clock,
-		policies.NewTriggerPolicyChain(ctx.Clock, cmapi.DefaultRenewBefore).Evaluate,
+		policies.NewTriggerPolicyChain(ctx.Clock).Evaluate,
 	)
 	c.controller = ctrl
 

pkg/controller/certificates/util.go

@@ -268,24 +268,27 @@ func GenerateLocallySignedTemporaryCertificate(crt *cmapi.Certificate, pkData []
 //RenewalTimeFunc is a custom function type for calculating renewal time of a certificate.
 type RenewalTimeFunc func(time.Time, time.Time, *metav1.Duration) *metav1.Time
 
-// RenewalTimeWrapper returns RenewalTimeFunc implementation
-func RenewalTimeWrapper(defaultRenewBeforeExpiryDuration time.Duration) RenewalTimeFunc {
-	return func(notBefore, notAfter time.Time, renewBeforeHint *metav1.Duration) *metav1.Time {
-
-		// 1. Calculate how long before expiry a cert should be renewed
-		renewBefore := defaultRenewBeforeExpiryDuration
-		if renewBeforeHint != nil {
-			renewBefore = renewBeforeHint.Duration
-		}
-		actualDuration := notAfter.Sub(notBefore)
-		// renewBefore = min(renewBefore, actualDuration/3)
-		if renewBefore >= (actualDuration / 3) {
-			renewBefore = actualDuration / 3
-		}
+// RenewalTime calculates renewal time for a certificate. Default renewal time
+// is 2/3 through certificate's lifetime. If user has configured
+// spec.renewBefore, renewal time will be renewBefore period before expiry
+// (unless that is after the expiry).
+func RenewalTime(notBefore, notAfter time.Time, renewBeforeOverride *metav1.Duration) *metav1.Time {
+
+	// 1. Calculate how long before expiry a cert should be renewed
 
-		// 2. Calculate when a cert should be renewed
-		rt := metav1.NewTime(notAfter.Add(-1 * renewBefore))
-		return &rt
+	actualDuration := notAfter.Sub(notBefore)
+
+	renewBefore := actualDuration / 3
+
+	// If spec.renewBefore was set (and is less than duration)
+	// respect that. We don't want to prevent users from renewing
+	// longer lived certs more frequently.
+	if renewBeforeOverride != nil && renewBeforeOverride.Duration < actualDuration {
+		renewBefore = renewBeforeOverride.Duration
 	}
 
+	// 2. Calculate when a cert should be renewed
+
+	rt := metav1.NewTime(notAfter.Add(-1 * renewBefore))
+	return &rt
 }