Feature/add annotation for renewBefore path on ingress / gateway by titanlien · Pull Request #662 · gardener/cert-management

titanlien · 2026-02-20T13:50:15Z

How to categorize this PR?

/kind enhancement

What this PR does / why we need it:
Partially fix the renew window, we can increase renewBefore from 30days to 60 days.

Which issue(s) this PR fixes:
Related to #475

Special notes for your reviewer:
We can use annotation on ingress / gateway resource to define the renewBefore on certificates

Release note:

category: feature
target_group: operator

gardener-prow · 2026-02-20T13:50:21Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign martinweindel for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

CLAassistant · 2026-02-20T13:50:23Z

All committers have signed the CLA.

gardener-prow · 2026-02-20T13:50:26Z

Welcome @titanlien!

It looks like this is your first PR to gardener/cert-management 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if gardener/cert-management has its own contribution guidelines.

Thank you, and welcome to Gardener. 😃

gardener-prow · 2026-02-20T13:50:27Z

Hi @titanlien. Thanks for your PR.

I'm waiting for a gardener member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

MartinWeindel · 2026-02-23T08:48:09Z

/ok-to-test

titanlien · 2026-02-23T16:50:05Z

/test pull-cert-management-e2e-kind

MartinWeindel · 2026-02-24T09:59:55Z

@titanlien
Thanks for your contribution. Please give us some time to review your PR next week when @marc1404 is available again.

/hold

marc1404

Thank you for your contribution, @titanlien! 🙏

I left a few suggestions below. One general feedback: I appreciate that you split up changes into multiple commits. However, the second commit's message says it's about adding test code, while it also adjusts documentation and examples and adds logic to the certman2 code. It would've been nice to separate this more clearly (although it's totally manageable for the size of this changeset).

pkg/cert/source/defaults.go

pkg/cert/source/reconciler.go

examples/40-ingress-echoheaders.yaml

pkg/certman2/controller/source/common/certinput.go

test/certman2/integration/controller/source/source_test.go

titanlien · 2026-03-03T17:45:26Z

/ok-to-test

titanlien · 2026-03-04T14:38:00Z

/ok-to-test

marc1404

Thanks for restructuring the commit history! 🙏

pkg/controller/source/helper.go

examples/40-ingress-echoheaders.yaml

marc1404 · 2026-03-04T15:08:15Z

/unhold
/cla

gardener-prow · 2026-03-04T15:08:19Z

Successfully reached out to cla-assistant.io to initialize recheck of PR #662

titanlien · 2026-03-05T17:01:42Z

/ok-to-test

titanlien · 2026-03-05T17:34:21Z

/ok-to-test

marc1404 · 2026-03-06T07:40:51Z

@titanlien fyi: the ok-to-test label only has to be set once by the maintainers, it's not removed when you push changes afterwards :)

marc1404

Hopefully the last feedback round 🤞
Please see my comment here.

ℹ️ One small note: For reviewers, it's convenient if you keep comments unresolved. That way reviewers can easily come back and compare open comments to further changes being made :)

Update examples/40-ingress-echoheaders.yaml Co-authored-by: Marc Vornetran <marc1404@users.noreply.github.com>

pkg/certman2/controller/source/common/constants.go

marc1404 · 2026-03-12T14:33:43Z

pkg/cert/source/controller.go

+	// DefaultRenewBefore is the default renewBefore duration (720 hours / 30 days)
+	DefaultRenewBefore = 720 * time.Hour


I don't see this value being used anywhere else, it appears to be unused

marc1404 · 2026-03-12T14:43:58Z

pkg/certman2/controller/source/common/certinput.go

+	renewBefore := annotations[AnnotRenewBefore]
+
 	certInput.FollowCNAME = followCNAME
 	certInput.IssuerName = issuer
 	certInput.PreferredChain = preferredChain
 	certInput.PrivateKeyAlgorithm = algorithm
 	certInput.PrivateKeySize = keySize
 	certInput.PrivateKeyEncoding = encoding
+	certInput.RenewBefore = renewBefore


nit: I don't see the local variable renewBefore being used anywhere else. What about inlining it?

Suggested change

renewBefore := annotations[AnnotRenewBefore]

certInput.FollowCNAME = followCNAME

certInput.IssuerName = issuer

certInput.PreferredChain = preferredChain

certInput.PrivateKeyAlgorithm = algorithm

certInput.PrivateKeySize = keySize

certInput.PrivateKeyEncoding = encoding

certInput.RenewBefore = renewBefore

certInput.FollowCNAME = followCNAME

certInput.IssuerName = issuer

certInput.PreferredChain = preferredChain

certInput.PrivateKeyAlgorithm = algorithm

certInput.PrivateKeySize = keySize

certInput.PrivateKeyEncoding = encoding

certInput.RenewBefore = annotations[AnnotRenewBefore]

marc1404 · 2026-03-12T14:54:12Z

pkg/controller/source/helper.go


 	api "github.com/gardener/cert-management/pkg/apis/cert/v1alpha1"
 	"github.com/gardener/cert-management/pkg/cert/source"
+	"github.com/gardener/cert-management/pkg/certman2/controller/source/common"


This wasn't visible before I approved the workflow runs: There are currently two code branches in this repository. Everything in pkg/certman2 is a rewrite of the project using controller-runtime. Everything outside is the legacy codebase. We've added .import-restrictions files to catch unintentional cross-imports between these branches.

This change violates the import restriction:

ERROR: "github.com/gardener/cert-management/pkg/controller/source" -> "github.com/gardener/cert-management/pkg/certman2/controller/source/common" is forbidden by cert-management/pkg/controller/.import-restrictions make: *** [check-imports] Error 1

Shared utilities should be moved to pkg/shared

marc1404 · 2026-03-12T15:03:49Z

pkg/certman2/controller/source/common/certinput.go

+// Returns (nil, warning) if the string is invalid or the duration is less than 5 minutes.
+// Returns (nil, "") if the string is empty.
+// The default value of DefaultRenewBefore is applied by the certificate controller if nil is returned.
+func ParseRenewBefore(renewBeforeStr string) (*metav1.Duration, string) {


Please add unit tests for this function

marc1404 · 2026-03-12T15:09:08Z

pkg/certman2/controller/source/common/certinput.go

+// Returns (nil, warning) if the string is invalid or the duration is less than 5 minutes.
+// Returns (nil, "") if the string is empty.
+// The default value of DefaultRenewBefore is applied by the certificate controller if nil is returned.
+func ParseRenewBefore(renewBeforeStr string) (*metav1.Duration, string) {


I'm not too happy with the returned string for the warning message; let's return a proper Go error instead. The callers can then decide what to do with the error value (discard and fallback to default value or log when a logger is available)

Suggested change

func ParseRenewBefore(renewBeforeStr string) (*metav1.Duration, string) {

func ParseRenewBefore(renewBeforeStr string) (*metav1.Duration, error) {

marc1404 · 2026-03-12T15:11:40Z

pkg/certman2/controller/source/common/certinput.go

 	PrivateKeyAlgorithm string
 	PrivateKeySize      int
 	PrivateKeyEncoding  string
+	RenewBefore         string


I suggest not storing the raw annotation value in the CertInput struct; instead, parse it directly and handle it as a Duration. This would be aligned with how it was done in the legacy code branch

Suggested change

RenewBefore string

RenewBefore *metav1.Duration

Co-authored-by: Marc Vornetran <marc1404@users.noreply.github.com>

gardener-ci-robot · 2026-04-12T06:46:25Z

The Gardener project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:

After 30d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 14d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as active with /lifecycle active
Mark this PR as fresh with /remove-lifecycle stale
Mark this PR as rotten with /lifecycle rotten
Close this PR with /close

/lifecycle stale

gardener-prow bot added the kind/enhancement Enhancement, improvement, extension label Feb 20, 2026

gardener-prow bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 20, 2026

gardener-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. labels Feb 20, 2026

titanlien changed the title ~~Feature/add annotatio renew before~~ Feature/add annotation for renewBefore path on ingress / gateway Feb 20, 2026

gardener-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 23, 2026

gardener-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 24, 2026

titanlien force-pushed the feature/add-annotatio-renew-before branch 2 times, most recently from 9980929 to 87a9e25 Compare February 27, 2026 17:35

marc1404 reviewed Mar 3, 2026

View reviewed changes

titanlien marked this pull request as draft March 3, 2026 16:24

gardener-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 3, 2026

titanlien force-pushed the feature/add-annotatio-renew-before branch 4 times, most recently from 30696ad to 3a81b99 Compare March 3, 2026 17:42

titanlien marked this pull request as ready for review March 3, 2026 17:44

gardener-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 3, 2026

titanlien requested a review from marc1404 March 3, 2026 17:44

titanlien force-pushed the feature/add-annotatio-renew-before branch from 3a81b99 to ae535c6 Compare March 4, 2026 14:37

marc1404 reviewed Mar 4, 2026

View reviewed changes

pkg/controller/source/helper.go Show resolved Hide resolved

examples/40-ingress-echoheaders.yaml Outdated Show resolved Hide resolved

gardener-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 4, 2026

titanlien force-pushed the feature/add-annotatio-renew-before branch from 852a8c4 to 5530faa Compare March 5, 2026 17:00

titanlien requested a review from marc1404 March 5, 2026 17:00

titanlien force-pushed the feature/add-annotatio-renew-before branch from 5530faa to 37799e2 Compare March 5, 2026 17:34

marc1404 reviewed Mar 6, 2026

View reviewed changes

titanlien and others added 3 commits March 6, 2026 16:08

Add a renewBefore path on annotation on ingress

28b659f

Add test code for renewBefore

dc44649

Add renewBefore in docs / example code

47a328c

Update examples/40-ingress-echoheaders.yaml Co-authored-by: Marc Vornetran <marc1404@users.noreply.github.com>

titanlien force-pushed the feature/add-annotatio-renew-before branch from 37799e2 to 47a328c Compare March 6, 2026 15:34

titanlien requested a review from marc1404 March 6, 2026 15:34

marc1404 reviewed Mar 12, 2026

View reviewed changes

Update pkg/certman2/controller/source/common/constants.go

8d3dd88

Co-authored-by: Marc Vornetran <marc1404@users.noreply.github.com>

gardener-prow bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 12, 2026

		// DefaultRenewBefore is the default renewBefore duration (720 hours / 30 days)
		DefaultRenewBefore = 720 * time.Hour

	func ParseRenewBefore(renewBeforeStr string) (*metav1.Duration, string) {
	func ParseRenewBefore(renewBeforeStr string) (*metav1.Duration, error) {

Conversation

titanlien commented Feb 20, 2026

Uh oh!

gardener-prow bot commented Feb 20, 2026

Uh oh!

CLAassistant commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gardener-prow bot commented Feb 20, 2026

Uh oh!

gardener-prow bot commented Feb 20, 2026

Uh oh!

MartinWeindel commented Feb 23, 2026

Uh oh!

titanlien commented Feb 23, 2026

Uh oh!

MartinWeindel commented Feb 24, 2026

Uh oh!

marc1404 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

titanlien commented Mar 3, 2026

Uh oh!

titanlien commented Mar 4, 2026

Uh oh!

marc1404 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

marc1404 commented Mar 4, 2026

Uh oh!

gardener-prow bot commented Mar 4, 2026

Uh oh!

titanlien commented Mar 5, 2026

Uh oh!

titanlien commented Mar 5, 2026

Uh oh!

marc1404 commented Mar 6, 2026

Uh oh!

marc1404 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marc1404 Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

marc1404 Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

marc1404 Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

marc1404 Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

marc1404 Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

marc1404 Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

gardener-ci-robot commented Apr 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CLAassistant commented Feb 20, 2026 •

edited

Loading

marc1404 left a comment •

edited

Loading