projectnessie / nessie

Nessie: Transactional Catalog for Data Lakes with Git-like semantics
https://projectnessie.org
Apache License 2.0
1.03k stars 129 forks source link

[Bug]: Helm chart does not specify metadata.namespace which breaks CD tool ArgoCD #9620

Closed ev1lm0nk3y closed 1 month ago

ev1lm0nk3y commented 1 month ago

What happened

The helm chart for nessie does not specify namespace in any resource, so

  1. you could accidentally install it in a namespace you didn't want to
  2. CD tools, like ArgoCD, don't know where to put the objects

How to reproduce it

  1. helm template testing nessie/nessie
  2. Check manifests for the lack of metadata.namespace entries

Nessie server type (docker/uber-jar/built from source) and version

0.98.0

Client type (Ex: UI/Spark/pynessie ...) and version

helm

Additional information

No response

adutra commented 1 month ago

Hi @ev1lm0nk3y thanks for reporting this!

The absence of namespace fields in our chart is actually by design. See this issue for context: https://github.com/helm/helm/issues/5465. Helm authors generally consider a best practice to not include any namespace field in any resource, so that Helm can honor the --namespace flag passed via the command line.

I'm not an expert in ArgoCD, but isn't ArgoCD capable of installing the chart in a given namespace? Looking at this page, it seems this should be possible.

ev1lm0nk3y commented 1 month ago

I was getting all kinds of errors while trying to deploy it so that is why I filed the bug. I understand honoring the namespace flag but the link you provided kinda said exactly what I'm proposing. What happens with helm, as far as i understand, the namespace flag doesn't add that value to the generated manifest, it can't because helm is effectively a templating engine. When that flag is used, helm will set that kube context to that namespace for the deployment, thus allowing all the resources to be installed without needing to explicitly define it.

Without namespace: {{ .Release.Namespace }} only the basic invocation of helm would work.

On Wed, Sep 25, 2024 at 7:22 AM Alexandre Dutra @.***> wrote:

Hi @ev1lm0nk3y https://github.com/ev1lm0nk3y thanks for reporting this!

The absence of namespace fields in our chart is actually by design. See this issue for context: helm/helm#5465 https://github.com/helm/helm/issues/5465. Helm authors generally consider a best practice to not include any namespace field in any resource, so that Helm can honor the --namespace flag passed via the command line.

I'm not an expert in ArgoCD, but isn't ArgoCD capable of installing the chart in a given namespace? Looking at this page https://argo-cd.readthedocs.io/en/stable/user-guide/helm/, it seems this should be possible.

— Reply to this email directly, view it on GitHub https://github.com/projectnessie/nessie/issues/9620#issuecomment-2374236003, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKXAADZBM5D4T5VAS63WUTZYLBJJAVCNFSM6AAAAABO2KETHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZUGIZTMMBQGM . You are receiving this because you were mentioned.Message ID: @.***>

ev1lm0nk3y commented 1 month ago

Also, while Argo can install helm charts in various namespaces, it is run from the argocd namespace usually and I haven't seen it do any trickery with the context before applying. ArgoCD will effectively run helm template --namespace foo myrelease repo/chart and then apply the output.

On Wed, Sep 25, 2024 at 7:40 AM Ryan Shatford @.***> wrote:

I was getting all kinds of errors while trying to deploy it so that is why I filed the bug. I understand honoring the namespace flag but the link you provided kinda said exactly what I'm proposing. What happens with helm, as far as i understand, the namespace flag doesn't add that value to the generated manifest, it can't because helm is effectively a templating engine. When that flag is used, helm will set that kube context to that namespace for the deployment, thus allowing all the resources to be installed without needing to explicitly define it.

Without namespace: {{ .Release.Namespace }} only the basic invocation of helm would work.

On Wed, Sep 25, 2024 at 7:22 AM Alexandre Dutra @.***> wrote:

Hi @ev1lm0nk3y https://github.com/ev1lm0nk3y thanks for reporting this!

The absence of namespace fields in our chart is actually by design. See this issue for context: helm/helm#5465 https://github.com/helm/helm/issues/5465. Helm authors generally consider a best practice to not include any namespace field in any resource, so that Helm can honor the --namespace flag passed via the command line.

I'm not an expert in ArgoCD, but isn't ArgoCD capable of installing the chart in a given namespace? Looking at this page https://argo-cd.readthedocs.io/en/stable/user-guide/helm/, it seems this should be possible.

— Reply to this email directly, view it on GitHub https://github.com/projectnessie/nessie/issues/9620#issuecomment-2374236003, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKXAADZBM5D4T5VAS63WUTZYLBJJAVCNFSM6AAAAABO2KETHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZUGIZTMMBQGM . You are receiving this because you were mentioned.Message ID: @.***>