honeycombio / helm-charts

Helm repository and charts for Honeycomb
Apache License 2.0
30 stars 39 forks source link

Add helm namespace to templates #155

Closed alex-bezek closed 2 years ago

alex-bezek commented 2 years ago

Which problem is this PR solving?

While helm install can respect the kubectl context namespace or accept a namespace flag, when using helm template, this information is lost if not persisted to the generated files. This means tooling that requires a helm template -> kubectl install has to work around the missing namespace.

Short description of the changes

Adds in the default .Release.Namespace value to the namespace for each namespace resource. https://helm.sh/docs/chart_template_guide/builtin_objects/ . Similar changes have been made such as https://github.com/helm/charts/pull/17937/files#diff-4dcbfca5f8fd1c82a179cf60e0448926548323d101f0321679e2fd1e3a2dc665R12 found on the original issue https://github.com/helm/helm/issues/5465

How to verify that this has the expected result

❯ kns
Context "ngrok-dev-us" modified.
Active namespace is "default".
❯ helm template ./charts/opentelemetry-collector | grep -i namespace
  namespace: default
  namespace: default
  namespace: default
  namespace: default
  namespace: default
❯ kns
Context "ngrok-dev-us" modified.
Active namespace is "kube-system".
❯ helm template ./charts/opentelemetry-collector | grep -i namespace
  namespace: kube-system
  namespace: kube-system
  namespace: kube-system
  namespace: kube-system
  namespace: kube-system
kentquirk commented 2 years ago

@alex-bezek Apologies for the delay. This looks good -- the main question before I approve it is whether there are any updates needed to the README file? I don't know enough to know if something there should change as a result of this.

alex-bezek commented 2 years ago

No problem @kentquirk. This change is just leveraging an existing helm built in value the Release object. https://helm.sh/docs/chart_template_guide/builtin_objects/ . This particular one, the namespace, is set by helm. If you do a helm install it uses the namespace of your current kubecontext. If using helm template there is a --namespace flag.

I wouldn't document the built-in objects in your readme, but if you'd like I could add just a section explaining that namespaces are added to all namespace-able objects and it uses the helm internals to do so.

kentquirk commented 2 years ago

Sounds like we don't need any documentation changes. I just wanted to be sure that there wasn't something non-obvious.