OCPBUGS-62792: Fix self-signed serving cert using 30ns lifetime instead of 30 days#2155
Conversation
Signed-off-by: jubittajohn <jujohn@redhat.com>
|
@jubittajohn: This pull request references Jira Issue OCPBUGS-62792, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughUpdated certificate validity duration in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/assign @sanchezl |
|
/jira refresh |
|
@sanchezl: This pull request references Jira Issue OCPBUGS-62792, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
|
@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: wangke19. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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 kubernetes-sigs/prow repository. |
|
/retitle OCPBUGS-62792: Fix self-signed serving cert using 30ns lifetime instead of 30 days |
| config.ServingInfo.KeyFile = filepath.Join(temporaryCertDir, "tls.key") | ||
| // nothing can trust this, so we don't really care about hostnames | ||
| servingCert, err := ca.MakeServerCert(sets.New("localhost"), 30) | ||
| servingCert, err := ca.MakeServerCert(sets.New("localhost"), time.Hour*24*30) |
There was a problem hiding this comment.
nice find!
the bug is real, yet no one has complained. I think this might be because the self-signed path almost never occurs in real clusters. When it does happen, I think the self-signed certificate only needs to last until the real certificate arrives.
worth adding a unit test ?
should we backport ?
please don't forget to bump the operators.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jubittajohn, p0lyn0mial, sanchezl 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 |
|
@jubittajohn: all tests passed! 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. |
|
@jubittajohn: Jira Issue OCPBUGS-62792: Some pull requests linked via external trackers have merged: The following pull request, linked via external tracker, has not merged: All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-62792 has not been moved to the MODIFIED state. 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. |
library-go crypto MakeSelfSignedCA and MakeServerCert were updated to use time.Duration instead of days, Update MakeServerCert() call in AddDefaultRotationToConfig() to reflect this change (#1943 that introduced this change)