Merge pull request #4399 from irbekrm/fix_renewal_issue

Fix renewalTime skew issue

pkg/controller/certificates/util.go

@@ -307,6 +307,16 @@ func RenewalTime(notBefore, notAfter time.Time, renewBeforeOverride *metav1.Dura
 
 	// 2. Calculate when a cert should be renewed
 
-	rt := metav1.NewTime(notAfter.Add(-1 * renewBefore))
+	// Truncate the renewal time to nearest second. This is important
+	// because the renewal time also gets stored on Certificate's status
+	// where it is truncated to the nearest second. We use the renewal time
+	// from Certificate's status to determine when the Certificate will be
+	// added to the queue to be renewed, but then re-calculate whether it
+	// needs to be renewed _now_ using this function- so returning a
+	// non-truncated value here would potentially cause Certificates to be
+	// re-queued for renewal earlier than the calculated renewal time thus
+	// causing Certificates to not be automatically renewed. See
+	// https://github.com/jetstack/cert-manager/pull/4399.
+	rt := metav1.NewTime(notAfter.Add(-1 * renewBefore).Truncate(time.Second))
 	return &rt
 }

pkg/controller/certificates/util_test.go

@@ -302,7 +302,7 @@ func TestRenewalTime(t *testing.T) {
 		renewBeforeOverride *metav1.Duration
 		expectedRenewalTime *metav1.Time
 	}
-	now := time.Now()
+	now := time.Now().Truncate(time.Second)
 	tests := map[string]scenario{
 		"short lived cert, spec.renewBefore is not set": {
 			notBefore:           now,
@@ -343,6 +343,15 @@ func TestRenewalTime(t *testing.T) {
 			renewBeforeOverride: &metav1.Duration{Duration: time.Hour * 24},
 			expectedRenewalTime: &metav1.Time{Time: now.Add(time.Minute * 3)}, // renew in 3 minutes
 		},
+		// This test case is here to guard against an earlier bug where
+		// a non-truncated renewal time returned from this function
+		// caused certs to not be renewed.
+		// See https://github.com/jetstack/cert-manager/pull/4399
+		"certificate's duration is skewed by a second": {
+			notBefore:           now,
+			notAfter:            now.Add(time.Hour * 24).Add(time.Second * -1),
+			expectedRenewalTime: &metav1.Time{Time: now.Add(time.Hour * 16).Add(time.Second * -1)},
+		},
 	}
 	for n, s := range tests {
 		t.Run(n, func(t *testing.T) {