kubeadm lowercases all domain names passed as additional SANs#64718
kubeadm lowercases all domain names passed as additional SANs#64718k8s-github-robot merged 1 commit intokubernetes:masterfrom
Conversation
|
/cc @kubernetes/sig-cluster-lifecycle-pr-reviews |
|
I could honestly go either way on this. Is this additional complexity worth helping users out in this corner case? |
There was a problem hiding this comment.
i think we should notify the user about SANs that were lowercased. (edit: possibly for higher verbosity levels).
luxas
left a comment
There was a problem hiding this comment.
As an UX improvement, this is probably a good thing to do. Not too much code, most of it is unit testing 👍
Please fix mine and @neolit123's review comments though.
There was a problem hiding this comment.
This must be called by configutil.SetInitDynamicDefaults instead of here to be run every time we load a config instead of just for kubeadm init
|
/kind bug |
timothysc
left a comment
There was a problem hiding this comment.
/approve
defer @neolit123 for lgtm
|
looks great! we can delete some code in the quickstart with this change :) |
84d7686 to
b76a20c
Compare
|
|
Some domains, like ELBs, output a domain name with uppercase letters. To accept these, we lowercase all arguments passed to ----apiserver-cert-extra-sans
b76a20c to
db52cd4
Compare
|
thanks for adding that verbose output! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liztio, neolit123, timothysc 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 |
|
[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process @liztio @luxas @neolit123 @timothysc Pull Request Labels
|
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue (batch tested with PRs 63322, 64718, 64708, 64775, 64777). If you want to cherry-pick this change to another branch, please follow the instructions here. |
|
Hello there! @liztio I'm Zach Arnold working on Docs for the 1.11 release. This PR was identified as one needing some documentation in the https://github.com/kubernetes/website repo around your contributions (thanks by the way!) When you have some time, could you please modify/add/remove the relevant content that needs changing in our documentation repo? Thanks! Please let me or my colleague Misty know (@zparnold/@misty on K8s Slack) if you need any assistance with the documentation. |
What this PR does / why we need it:
Some domains, like ELBs, output a domain name with uppercase letters. To
accept these, we lowercase all arguments passed to ----apiserver-cert-extra-sans
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 #kubeadm/827