feat: add feature to prevent nested kubeconfig contexts#6555
Merged
Conversation
Contributor
|
Merging this PR will trigger the following deployment actions. Support deploymentsNo support upgrades will be triggered Staging deployments
Production deployments
|
yuvipanda
requested changes
Aug 13, 2025
yuvipanda
left a comment
yuvipanda
left a comment
Member
There was a problem hiding this comment.
This is a great response to solving the class of problems that caused that outage. Starship works for me but may not work for you! This also means you must exit out of a subshell before running use-cluster-credentials again. But the fact that this is 'opt-in' makes this safe because it doesn't change it for everyone.
I did leave a comment about an additional check you must make to make sure that you aren't in a nested context. I'd also like you to add at least a minimal one line piece of doc somewhere. After the check and the doc you can totally just merge.
Contributor
Author
|
I've added a test for non-empty |
Contributor
Author
|
I've addressed Yuvi's review — now merging. |
Contributor
Author
|
Thanks @yuvipanda — starship is already useful for me (I had not noticed that it has k8s turned off by default). However, I also like to work defensively — given that I use a tiling window manager, it's trivial to spin up a new terminal context, and that brings some helpful spatial identity to working on different clusters! I acknowledge that to some (not you!) this may feel all a little overboard, but it feels like a low barrier to feeling safer. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I recently targeted the wrong cluster because (I think) of a nested KUBECONFIG context — I invisibly lost the second subshell, and ended up working in the parent context.
This PR allows me to make that harder for myself.
Changes
I made some changes to avoid deploying everything to main here:
use-cluster-credentialsinto a separate submoduledeployer/commands/develop/**in CIDEPLOYER_NO_NESTED_KUBECONFIGflag to deny nested kubeconfig contextsWe should add
__init__.pyentries to our various submodules, but that's not needed here, so I'm leaving it!Details
This PR makes it possible for 2i2c engineers to set
export DEPLOYER_NO_NESTED_KUBECONFIG=1such that sequential
deployer use-cluster-credentialsinvocations fail.I think this might have avoided the problem I ran into when the kubeconfig context reverted due to a shell error. This would also be ameliorated by setting e.g. a PS1, which I have also done with starship.
I'm biasing to action here -- this is opt-in and I think we are all comfortable with these kind of small additions.