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

[Helm] Remove if-else handling of the namespace key #19

Closed
ermik opened this issue Jun 29, 2019 · 6 comments
Closed

[Helm] Remove if-else handling of the namespace key #19

ermik opened this issue Jun 29, 2019 · 6 comments

Comments

@ermik
Copy link
Contributor

ermik commented Jun 29, 2019

Since 09b61e5 we added a namespace key to the kubernetes manifests generated in Hydra helm chart. I hoped to address it via #15 & #18, but perhaps it's better to discuss here.

First of all, Helm always writes a namespace, and more than that kubernetes doesn't have a notion of "no namespace" — rather they have a "default". The principles of working with namespaces are decided by the owner of the cluster, and it is most common to separate it into staging / prod, for multiple versions of the same grouping of applications, and/or define layers and semantic groupings in the cluster such as core, monitoring, edge, etc.

Then, Helm never actually deploys without a namespace. If you template the current chart you will see that helm will ignore the if-else clause by the virtue of specifying the namespace default.

Lastly, it is a common administrative practice to forbid deployment to default for reasons of security and clear team/org boundaries. But this is not be the case for someone trying out a chart in minikube or on their personal cloud.

With these in mind, I usually specify namespace key explicitly, as we already did, to bring the template and the result of helm's execution closer together (no magic keys appearing, everything is easily readable in either form, and easy to compare). It does not need a conditional. We should advise deployment in a dedicated namespace (also protects secrets).

@aeneasr
Copy link
Member

aeneasr commented Jun 29, 2019

So would it be enough to simply remove the if namespace? Personally, I think it's not our task to educate developers on Kubernetes and/or helm best practices. We should provide the capabilities of following those best practices but make default one-off installations helm install [-set demo=true] ory/hydra very easy

@ermik
Copy link
Contributor Author

ermik commented Jun 30, 2019

Yes, I think we should remove the if namespace conditionals. Then, we have a couple of options:

  1. noop: do nothing more; helm writes the namespace key by default, and kubelet would add default if manifest was missing the namespace key

  2. neutral: we can specify that developers should deploy sensitive resources separately) in the chart documentation and values file — but it's is up to them to figure out the way to do it

  3. proactive: drop silent dependency on .Release.Namespace (keeping it as a fallback) and guide/enable users to separate resources into multiple namespaces via values.yaml:

    namespaces: 
        internal: "security"
        external: "edge"

    (e.g. Ingress would go into external, Deployment, ConfigMap, Secret — internal).

    If not specified — all resources will be deployed to .Release.Namespace which comes from either --namespace argument or is default.

  4. extreme: enforce namespacing of resources by removing Release.Namespace fallback for sensitive objects — either:

    • require internal to be specified in values.yaml
      OR
    • prohibit use of default regardless of where it was specified.

@aeneasr
Copy link
Member

aeneasr commented Jul 1, 2019

The noop only works with helm install, not helm template, which is why it was added in the first place!

@ermik
Copy link
Contributor Author

ermik commented Jul 1, 2019

I implemented the noop changes in #21 and it seems to work correctly when running helm template (see the rendering I posted). Let me know your thoughts.

@aeneasr
Copy link
Member

aeneasr commented Jul 31, 2019

I think we can close this, right? :)

@ermik
Copy link
Contributor Author

ermik commented Aug 1, 2019

Yup. Further improvements via follow up (you could open it as RFC / Feedback)

@ermik ermik closed this as completed Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants