USHIFT-6590: Align certificate expiry dates#6219
USHIFT-6590: Align certificate expiry dates#6219pacevedom wants to merge 2 commits intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@pacevedom: This pull request references USHIFT-6590 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target either version "4.22." or "openshift-4.22.", but it targets "openshift-4.21" instead. DetailsIn response to this: 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pacevedom The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@pacevedom: This pull request references USHIFT-6590 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughAligns certificate expirations to the next midnight by adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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. Comment |
openshift-ci
bot
added
the
approved
|
@pacevedom: This pull request references USHIFT-6590 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pkg/cmd/init.go`:
- Around line 66-77: The code builds nextMidnight and alignValidity using the
local time zone via startTime := time.Now(); replace this with UTC to avoid DST
drift: set startTime to time.Now().UTC() (and ensure nextMidnight is constructed
in that same UTC location) so that nextMidnight, targetExpiration and the
alignValidity(baseValidity) calculation are anchored to a fixed UTC midnight
rather than a potentially-shifting local midnight; update references to
startTime, nextMidnight and alignValidity accordingly.
| startTime := time.Now() | ||
| nextMidnight := time.Date( | ||
| startTime.Year(), | ||
| startTime.Month(), | ||
| startTime.Day()+1, | ||
| 0, 0, 0, 0, | ||
| startTime.Location(), | ||
| ) | ||
| alignValidity := func(baseValidity time.Duration) time.Duration { | ||
| targetExpiration := nextMidnight.Add(baseValidity) | ||
| return time.Until(targetExpiration) | ||
| } |
There was a problem hiding this comment.
Consider using UTC to avoid DST drift.
startTime.Location() uses the system's local timezone. If DST transitions occur during the certificate's validity window, nextMidnight.Add(baseValidity) won't land on midnight—durations are absolute, but wall-clock midnight shifts by an hour at DST boundaries.
Using time.Now().UTC() would guarantee the expiry anchor is always exactly midnight.
Proposed fix
- startTime := time.Now()
+ startTime := time.Now().UTC()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| startTime := time.Now() | |
| nextMidnight := time.Date( | |
| startTime.Year(), | |
| startTime.Month(), | |
| startTime.Day()+1, | |
| 0, 0, 0, 0, | |
| startTime.Location(), | |
| ) | |
| alignValidity := func(baseValidity time.Duration) time.Duration { | |
| targetExpiration := nextMidnight.Add(baseValidity) | |
| return time.Until(targetExpiration) | |
| } | |
| startTime := time.Now().UTC() | |
| nextMidnight := time.Date( | |
| startTime.Year(), | |
| startTime.Month(), | |
| startTime.Day()+1, | |
| 0, 0, 0, 0, | |
| startTime.Location(), | |
| ) | |
| alignValidity := func(baseValidity time.Duration) time.Duration { | |
| targetExpiration := nextMidnight.Add(baseValidity) | |
| return time.Until(targetExpiration) | |
| } |
🤖 Prompt for AI Agents
In `@pkg/cmd/init.go` around lines 66 - 77, The code builds nextMidnight and
alignValidity using the local time zone via startTime := time.Now(); replace
this with UTC to avoid DST drift: set startTime to time.Now().UTC() (and ensure
nextMidnight is constructed in that same UTC location) so that nextMidnight,
targetExpiration and the alignValidity(baseValidity) calculation are anchored to
a fixed UTC midnight rather than a potentially-shifting local midnight; update
references to startTime, nextMidnight and alignValidity accordingly.
|
@pacevedom: This pull request references USHIFT-6590 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@pacevedom: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
2 participants
Summary by CodeRabbit