kubernetes-sigs
:sparkles: Add CertificateValidityPeriod and CACertificateValidityPeriod to KubeadmConfig
What this PR does / why we need it:
Adds CertificateValidityPeriod and CACertificateValidityPeriod to KubeadmConfig
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #12289
/area provider/bootstrap-kubeadm
/hold
Added CertificateValidityPeriod to KubeadmConfig if the changes (especially conversions) are in right direction will do the similar changes for CACertificateValidityPeriod as well.
@sbueringer Please let me know your opinion. Thanks.
Added both the fields, Need to make changes to honor those fields while generating CA.
@Karthik-K-N Sorry can you rebase again? I think now we got all PRs merged for now. I'll look into the PR afterwards
@Karthik-K-N Sorry can you rebase again? I think now we got all PRs merged for now. I'll look into the PR afterwards
No issues, Thanks, I just rebased the PR, I have added both the fields with their conversions, I will just try to work on how to use these fields while generating certificate
Address all the review comments, The PR is ready for next review. Thanks.
LGTM label has been added.
Will try to take another look soon (I currently assume worst case next week)
@Karthik-K-N Can you rebase the PR please? :) I'll review directly afterwards
@Karthik-K-N Can you rebase the PR please? :) I'll review directly afterwards
Done. Thanks
Re: https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_cluster-api/12335/pull-cluster-api-e2e-main/1942878337315639296
e2e tests are fine, the one failure is currently expected
Addressed all the review comments, Please take a look. Please let me know if I need to add any more test cases. thanks
@Karthik-K-N Sorry can you please rebase again? I'll then review this ASAP
@Karthik-K-N Sorry can you please rebase again? I'll then review this ASAP
No issues, I will do that.
/assign @fabriziopandini
Apart from the last minor finding ready from my side.
PTAL :)
/lgtm
Thank you very much! Sorry again for the delayed reviews
/assign @fabriziopandini
LGTM label has been added.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: fabriziopandini
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [fabriziopandini]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@Karthik-K-N: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| pull-cluster-api-e2e-main | 9582a449ade4d320bf102e1cdc5953da6faaa273 | link | true | /test pull-cluster-api-e2e-main |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
Kudos to @Karthik-K-N for stepping up in implementing this feature Many thanks to @sbueringer for helping in getting this PR ready
/lgtm /approve
Thank you, Big thanks to @sbueringer for continued review and guidance. Looking forward to contribute more.