-
Notifications
You must be signed in to change notification settings - Fork 39.1k
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
Explicit namespace from kubeconfig should override in-cluster config #44570
Conversation
cc @kubernetes/sig-cli-pr-reviews |
563a5ad
to
7841a4b
Compare
test/e2e/kubectl.go
Outdated
tokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token | ||
`), os.FileMode(0755))) | ||
framework.Logf("copying %s to the %s pod", iccOverride.Name(), simplePodName) | ||
framework.RunKubectlOrDie("cp", iccOverride.Name(), ns+"/"+simplePodName+":/tmp/icc-override.kubeconfig") |
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.
I am not sure if this is a regression, but it appears you can't change the destination filename; i.e. kubectl cp sourcefile pod:/tmp/destfile
will yield /tmp/sourcefile
inside the container :-/ I discovered this a few days ago when testing something else.
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.
Here's an example of what it's running:
Running '/workspace/kubernetes/platforms/linux/amd64/kubectl --server=https://104.198.236.9 --kubeconfig=/workspace/.kube/config cp /tmp/icc-override.kubeconfig363380968 e2e-tests-kubectl-bp2b4/nginx:/tmp/icc-override.kubeconfig'
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.
huh... that's super-annoying
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.
@fraenkel, is this a known issue? ^
kubectl cp is really tar which makes anything difficult. To really do cp
we would need a helper on the server side which we don't have.
|
updated test to match kubeconfig filenames |
tests green, PTAL |
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.
Changes lgtm but want second eyes because this code always breaks
} | ||
|
||
// if we got a default namespace, determine whether it was explicit or implicit | ||
if raw, err := mergedKubeConfig.RawConfig(); err == nil { |
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.
When the Kubeconfig revolution comes, this method will be first up against the wall. But the tests are convincing
/approve |
…#44862-upstream-release-1.6 Automatic merge from submit-queue Automated cherry pick of #44570 #44862 Cherry pick of #44570 #44862 on release-1.6. #44570: Explicit namespace from kubeconfig should override in-cluster #44862: Stop treating in-cluster-config namespace as an override ```release-note * kubectl commands run inside a pod using a kubeconfig file now use the namespace specified in the kubeconfig file, instead of using the pod namespace. If no kubeconfig file is used, or the kubeconfig does not specify a namespace, the pod namespace is still used as a fallback. * Restored the ability of kubectl running inside a pod to consume resource files specifying a different namespace than the one the pod is running in. ```
Commit found in the "release-1.6" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked. |
Fixes #43662
If an explicitly specified namespace is read from a kubeconfig file, we should not fall back to in-cluster config