🌱 expose cert refresh duration for testing via env.

[morvencao]

Summary

Related issue(s)

Fixes #

Summary by CodeRabbit

• Chores
  • Made certificate reload interval configurable via environment variable CERT_CALLBACK_REFRESH_DURATION (default: 5 minutes).

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Introduces a test-configurable certificate reload interval by adding an exported variable CertCallbackRefreshDuration (default 5 minutes) and an init() function that reads the CERT_CALLBACK_REFRESH_DURATION environment variable to override the default at runtime.

Changes

Cohort / File(s)	Change Summary
Certificate Reload Interval Configuration pkg/cloudevents/generic/options/cert/rotation.go	Added exported variable CertCallbackRefreshDuration with default of 5 minutes; added init() function to read and parse CERT_CALLBACK_REFRESH_DURATION environment variable for test-time override, with warning logged on invalid input

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Simple environment variable parsing pattern with straightforward default override logic
• Single file modification with localized scope
• Standard init-time configuration approach with no complex dependencies or side effects

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is empty except for the template boilerplate; it lacks substantive content in the Summary section and missing Related issue reference.	Provide a clear summary of the changes and any related issue numbers. The Summary should explain why the cert refresh duration needs to be configurable and how to use it.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: exposing certificate refresh duration for testing via an environment variable, matching the AI-generated summary.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[qiujian16]

/approve
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: morvencao, qiujian16

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [qiujian16]

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

pkg/cloudevents/generic/options/cert/rotation.go (2)

30-37: Test-only duration override is documented clearly; consider clarifying exported usage

The comments clearly state the default (5m) and the test-only env override, which is good. Since CertCallbackRefreshDuration is exported, callers can also override it programmatically in tests or specialized setups, independent of the env var. If that is intentional, you might mention it in the comment; if not, consider making the variable unexported and relying solely on the env-based hook.

---

39-54: Strengthen CERT_CALLBACK_REFRESH_DURATION validation and align comment with behavior

The init() logic is generally solid: it only overrides on a set env var, uses time.ParseDuration, and logs/keeps the default on parse errors. Because this value is used as the interval for wait.PollUntilContextCancel, a zero or negative duration (which ParseDuration accepts) could lead to extremely tight polling and unnecessary CPU usage if someone misconfigures the env var. (pkg.go.dev)

Two concrete suggestions:

1. Validate for non-positive durations as well as parse errors

   Treat d <= 0 as invalid and keep the default, logging a warning:

   func init() {
   	// TEST-ONLY OVERRIDE:
   	// Allow integration tests to reduce reload intervals by setting
   	// CERT_CALLBACK_REFRESH_DURATION to a valid Go duration string (e.g., "10s").
   	//
   	// If the variable is not set or is invalid, the default (5m) is preserved.
   	if v := os.Getenv("CERT_CALLBACK_REFRESH_DURATION"); v != "" {
   		d, err := time.ParseDuration(v)
   		if err != nil {
   -			// Optional: log or print a warning
   -			klog.Warningf("invalid CERT_CALLBACK_REFRESH_DURATION (%q): %v, using default\n", v, err)
   -			return
   -		}
   -		CertCallbackRefreshDuration = d
   +			klog.Warningf("invalid CERT_CALLBACK_REFRESH_DURATION (%q): %v, using default", v, err)
   +			return
   +		}
   +		if d <= 0 {
   +			klog.Warningf("CERT_CALLBACK_REFRESH_DURATION must be > 0, got %q; using default", v)
   +			return
   +		}
   +		CertCallbackRefreshDuration = d
   	}
   }

2. Remove the stale “Optional: log or print a warning” comment

   Since you are already logging via klog.Warningf, that comment is now misleading; the diff above drops it.

These tweaks make the override safer under misconfiguration while keeping the test-only behavior you’re aiming for.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e7a48e and bb47173.

📒 Files selected for processing (1)

• pkg/cloudevents/generic/options/cert/rotation.go (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: unit
• GitHub Check: integration
• GitHub Check: verify

🔇 Additional comments (1)

pkg/cloudevents/generic/options/cert/rotation.go (1)

3-12: New os import for env override is appropriate

os is only used to read CERT_CALLBACK_REFRESH_DURATION in init(), which matches the new behavior; no issues here.