-
Notifications
You must be signed in to change notification settings - Fork 620
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
Add environment variable flag to CLI in order to respect kubeconfig namespace #5028
Conversation
…amespace Signed-off-by: Adrian Berger <[email protected]>
Anyone willing to take a look at it? |
Thanks for submitting the change. I'm afraid this will have to go through wider discussion with other developers and maintainers to determine how we would like to solve the issue. The discussion in #3453 contains two separate solutions for this without a finalized solution. Going through some of the docs that refer to kubeconfig context namespace
it seems to me that it's a very explicit configuration that a user has to manually set. By default, it's empty. So the existence of the field seems like an explicit declaration. There could be an argument that the CLI respects the kubeconfig context and if the namespace is part of the context, it should also be respected. Absence of the context namespace, which is mostly the case, would default to the flux default. At present, respecting the kubeconfig seems more elegant to me than introducing |
Thank you for taking the time to take a look at this issue. If you have discussed/decided how & if you want to address this, please let me know if I can help in any way. |
There are definitely two different use cases we need to cater to, at least. The first user is the sys-admin, who has cluster-admin, and the second user is an app-admin who likely has admin but only within a tenant namespace, or namespaces. The tenant with multiple namespaces would really like to follow the kubectx namespace, but the sys-admin with zero tenants (a single-tenant system) would prefer to have the default Since Flux is not multi-tenant enabled by default, you have to enable some patches or flags at bootstrap time, and you have to generate tenants (which bootstrap does not do) it's clear enough that the single-tenant use case is the default one. It is a goal of the documentation to coach the early users of Flux to keep their Flux resources together, and to not put anything else inside of the "home" So, I agree that there's a quality-of-life change necessary here, but I need to take a closer look at what this change does to see if it can handle both use cases. (Is there another use case I didn't consider? Maybe the tenant with only one namespace, but that is a special case I think the current |
I am a "sys-admin" in my organization and I would also prefer flux following the kubeconfig context namespace. Since most users who work via the command line also use There are many scripts, plugins etc. already supporting the kubeconfig context namespace, so I see no point in going in another direction here. Since the change is very small to have our desired functionality, I'm also open to close this PR and we will just patch the Flux CLI for our needs. |
We are still considering this, there's some discussion happening at the dev meeting right now. I'm posting my thoughts in #3453 - thanks for your patience. |
@adberger we discussed this again in the dev meeting and everyone agreed that this is a bug. We should just respect the namespace in kubeconfig context, if present. No need to introducing another mechanism to enforce it. You can close this PR, and if you're up for it, please submit another fix in line with the agreement. Thanks for bringing this issue to our attention again. |
This PR is a follow-up from the following issue: #3453
Since this is my first contribution, I probably need some help in order to get this merged.
For example: Where should I add documentation?
For the already existing flag
FLUX_SYSTEM_NAMESPACE
I didn't found any reference in the repository.