Skip to content
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 'name' label to created namespaces #4231

Merged
merged 1 commit into from
Jul 5, 2018
Merged

Conversation

munnerz
Copy link
Contributor

@munnerz munnerz commented Jun 16, 2018

This pull request changes Helm to add a 'name' label to namespaces it creates.

This is useful because a number of Kubernetes extension points allow configuring whether webhook/policy applies to a namespace based only on labels, and not namespace name (e.g. NetworkPolicy & Validating/MutatingWebhookConfiguration).

This PR will allow me to selectively disable resource validation on the namespace that my controller/application is deployed into.

This has been requested in #3503 (although that issue requests more metadata). Custom labels have been requested in #4178.

Given the security considerations with custom labels, and the complication in complexity with adding more metadata, for now, I have opted to add a simple 'name' label.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 16, 2018
@munnerz
Copy link
Contributor Author

munnerz commented Jun 16, 2018

I have also not implemented ensuring the label exists on the namespace, for the cases where a user may have pre-provisioned a namespace. This also means the label presumably won't be added on chart upgrades too.

@munnerz
Copy link
Contributor Author

munnerz commented Jun 29, 2018

Any chance this PR can get some eyes on it? Not sure what the release cycle/process is for Helm, but I'm really keen to get this in some form into Helm! Right now it is not possible for me to set up validating webhooks that exclude a particular namespace, as I am unable to set a label on that namespace 😬

/cc @bacongobbler

@prydonius
Copy link
Member

+1 seems like a reasonable add to me. I'll defer to @bacongobbler or someone else who works more on core to LGTM

Copy link
Member

@bacongobbler bacongobbler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bacongobbler bacongobbler merged commit cc826d3 into helm:master Jul 5, 2018
@Stono
Copy link

Stono commented Jul 11, 2018

#pleasereleasethis :D

@bacongobbler
Copy link
Member

@Stono feel free to either test this PR using either v2.10.0-rc.2 or the canaries for the time being. We're working on an official release; just fixing up a few snags we've found during testing. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants