Merge pull request #7441 from cert-manager-bot/cherry-pick-7421-to-release-1.16

cert-manager-prow[bot] · web-flow · commit c3ed8135c8e1 · 2024-11-25T15:40:29.000Z
[release-1.16] Fix renewBeforePercentage to comply with its spec
diff --git a/pkg/util/pki/renewaltime.go b/pkg/util/pki/renewaltime.go
@@ -68,7 +68,7 @@ func RenewBefore(actualDuration time.Duration, renewBefore *metav1.Duration, ren
 	if renewBefore != nil && renewBefore.Duration > 0 && renewBefore.Duration < actualDuration {
 		return renewBefore.Duration
 	} else if renewBeforePercentage != nil && *renewBeforePercentage > 0 && *renewBeforePercentage < 100 {
-		return actualDuration * time.Duration(100-*renewBeforePercentage) / 100
+		return actualDuration * time.Duration(*renewBeforePercentage) / 100
 	}
 
 	// Otherwise, default to renewing 2/3 through certificate's lifetime.
diff --git a/pkg/util/pki/renewaltime_test.go b/pkg/util/pki/renewaltime_test.go
@@ -66,11 +66,11 @@ func TestRenewalTime(t *testing.T) {
 			renewBefore:         &metav1.Duration{Duration: time.Hour * 25},
 			expectedRenewalTime: &metav1.Time{Time: now.Add(time.Hour * 16)},
 		},
-		"long lived cert, spec.renewBeforePercentage is set to renew 50% of the way": {
+		"long lived cert, spec.renewBeforePercentage is set to renew 30% before expiry": {
 			notBefore:           now,
 			notAfter:            now.Add(time.Hour * 730), // 1 month
-			renewBeforePct:      ptr.To(int32(50)),
-			expectedRenewalTime: &metav1.Time{Time: now.Add(time.Hour * 365)},
+			renewBeforePct:      ptr.To(int32(30)),
+			expectedRenewalTime: &metav1.Time{Time: now.Add(time.Hour * 511)}, // 70% of 1 month
 		},
 		// This test case is here to show the scenario where users set
 		// renewBefore to very slightly less than actual duration. This
@@ -115,7 +115,7 @@ func TestRenewBefore(t *testing.T) {
 		},
 		"spec.renewBeforePercentage is valid": {
 			renewBeforePct:      ptr.To(int32(25)),
-			expectedRenewBefore: 135 * time.Minute,
+			expectedRenewBefore: 45 * time.Minute,
 		},
 		"spec.renewBeforePercentage is too large so default is used": {
 			renewBeforePct:      ptr.To(int32(100)),

Original file line number	Diff line number	Diff line change
`@@ -68,7 +68,7 @@ func RenewBefore(actualDuration time.Duration, renewBefore *metav1.Duration, ren`
`68`	`68`	`if renewBefore != nil && renewBefore.Duration > 0 && renewBefore.Duration < actualDuration {`
`69`	`69`	`return renewBefore.Duration`
`70`	`70`	`} else if renewBeforePercentage != nil && renewBeforePercentage > 0 && renewBeforePercentage < 100 {`
`71`		`- return actualDuration * time.Duration(100-*renewBeforePercentage) / 100`
	`71`	`+ return actualDuration * time.Duration(*renewBeforePercentage) / 100`
`72`	`72`	`}`
`73`	`73`
`74`	`74`	`// Otherwise, default to renewing 2/3 through certificate's lifetime.`