Adds a test case for renewal time skew and a comment

Signed-off-by: irbekrm <irbekrm@gmail.com>

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
 
+	// 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

@@ -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) {