-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
normalize server url before writing to config #12591
normalize server url before writing to config #12591
Conversation
[test] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overwrite.
clusters := make(map[string]*clientcmdapi.Cluster) | ||
clusters[clusterNick] = &clientcmdapi.Cluster{ | ||
Server: o.APIServerURL, | ||
Server: serverNormalized, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better approach would be to overwrite o.APIServerURL
so that the code below sees the new value.
e6c1458
to
88f2338
Compare
@enj thanks, review comment addressed |
integration check flaked on #12558 re[test] |
LGTM, @juanvallejo squash into one? |
88f2338
to
83fbdc8
Compare
@fabianofranz thanks, done! |
[merge] |
Evaluated for origin merge up to 83fbdc8 |
conformance check flaked on #10773 re[test] |
Evaluated for origin test up to 83fbdc8 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13250/) (Base Commit: c5868ac) |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13281/) (Base Commit: c82fb7d) (Image: devenv-rhel7_5770) |
Related bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1393943
This patch updates
$ openshift admin create-api-client-config
command to normalize the specified server url before creating the client kubeconfig.cc @openshift/cli-review