feat: introduce namespaceOverride

[khaledCNTXT]

Motivation

Extended the Helm chart to allow for a namespace override via the new namespaceOverride value. This ensures that a custom namespace can be specified and used, otherwise defaults to the release namespace. This update enhances deployment flexibility in different Kubernetes environments.

Changes

add namespaceOverride to Values

[alexrashed]

Hi @khaledCNTXT!
Thanks a lot for your contribution! Could you please elaborate a bit on why you would like to use a different namespace than the release namespace here?
You could just define the release namespace when installing the chart:

helm install localstack localstack/localstack -n this-is-my-custom-namespace --create-namespace

[khaledCNTXT]

Hi @alexrashed
We are using Novu Helm, which relies on LocalStack as a dependency. Therefore, we need to install all the resources under the novu namespace. Additionally, we have other resources that need to be deployed to a different namespace.

[alexrashed]

Interesting. Helm would use the same namespace for dependencies as well, so why wouldn't LocalStack as a dependency of the chart you mentioned not be installed in the same? Maybe the chart you mean has some special namespace handling?
Could you maybe give us a minimum-reproducible-sample showcasing your issue?

[khaledCNTXT]

We have a Helm chart with multiple dependencies as follows:

apiVersion: v2
name: XXXXX
description: XXXXX
version: 0.1.0

dependencies:
  - name: cloudSqlProxy
    version: 1.0.0
    repository: "file://services/cloudSqlProxy"
    condition: cloudSqlProxy.enabled

  - name: cert-manager
    version: v1.15.1
    repository: https://charts.jetstack.io
    condition: cert-manager.enabled

  - name: novu
    repository: "git+https://github.com/novuhq/community-k8s@helm"
    version: "*"
    condition: novu.enabled

Key Points:

1. Namespace Deployment:
   Each dependency (cloudSqlProxy, cert-manager, novu) needs to be deployed in its own namespace.

2. Novu Sub-dependencies:
   Novu has additional dependencies such as localstack. These also need to be deployed in their own separate namespaces.

3. Values File:
   We're using a single values file to manage configurations for all dependencies.
   This values file contains a section for localstack, and we need to ensure the correct namespace is passed from this section for localstack.

[alexrashed]

Interesting, thanks a lot for the explanation. I can see that this can be useful, and it seems to be a somewhat-best-practice used in multiple charts. In fact, there's a utility function in bitnami/common (implemented in bitnami/charts#9396), which we already have as a dependency. It's also the same utility used in the novu chart here.

I would suggest to modify your PR such that:

• It does not define a new function, but uses the function in bitnami/commons.
• Set the namespace accordingly in all templates (not only in the deployment).

What do you think?

[khaledCNTXT]

sure i will update the PR

[khaledCNTXT]

I reviewed the documentation at
https://github.com/novuhq/community-k8s/tree/fb860df320ee6b47cc109355aa1e117b16cc5d1b/helm
and it appears that common.names.namespace is not a valid option for us to use. Is there something I might be missing

[alexrashed]

I reviewed the documentation at
https://github.com/novuhq/community-k8s/tree/fb860df320ee6b47cc109355aa1e117b16cc5d1b/helm
and it appears that common.names.namespace is not a valid option for us to use. Is there something I might be missing

I am not sure what you mean by that to be honest. We - same as the novu chart - both have the bitnami commons chart in our dependencies. Novu already uses the common.names.namespace in their chart (as linked above). We could do the exact same thing (see the suggested change in the comment).

Does that answer your question?

[alexrashed]

Suggested change

	namespace: {{ include "localstack.namespace" . }}
	namespace: {{ include "common.names.namespace" . | quote }}

[khaledCNTXT]

I was working on implementing this change when I noticed that the project uses localstack.name, localstack.fullname, and localstack.chart. We have two options: either we move everything under common.names.xxxx for consistency, or we maintain the current standard and continue using localstack.xxx. What do you think? For reference, other libraries like common have a separate file for names, such as _names.tpl.

[alexrashed]

I think trying to consolidate everything by using the common chart utils functions is out of scope for this PR.
I also don't see a reason to create an unnecessary layer of indirection by creating a new localstack.namespace if it is just going to use common.names.namespace.
So in my opinion, we should just use common.names.namespace everywhere (which is a backwards compatible change), no other changes necessary.

[khaledCNTXT]

I've just pushed a change. Please review it and let me know if it meets the expected requirements

[alexrashed]

There's a bit of a misunderstanding. I tried to explain that in the comments, but imho you should be able to just remove the two sections I commented on, and it should work just fine with the value you initially proposed (namespaceOverride).
Would you mind giving this a try? Do you have any questions on that? :)

[alexrashed]

If we can use common.names.namespaces from the subchart, this isn't necessary:

Suggested change

	common:
	names:
	namespaces: ""

[alexrashed]

I think there's a bit of a misunderstanding. My point in the previous comments is that we can just directly use common.names.namespace (exactly the same way as the Novu chart) because this is defined in the bitnami commons (which we also have as a dependency) here: https://github.com/bitnami/charts/blob/8ecfbadb2b93576772fd134aeedcb13ad20221d2/bitnami/common/templates/_names.tpl#L59-L64

So you can just remove this section, just use the (already by the subchart defined) common.names.namespaces everywhere (as you are already 🥳), and use the value namespaceOverride in the values.

Suggested change

	{{/*
	Create a default namespace for the app.
	We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
	If a common.names.namespace is provided, it will be used as the namespace.
	*/}}
	{{- define "common.names.namespace" -}}
	{{- if .Values.common.names.namespaces }}
	{{- .Values.common.names.namespaces | trunc 63 | trimSuffix "-" }}
	{{- else }}
	{{- .Release.Namespace | quote }}
	{{- end }}
	{{- end }}

[khaledCNTXT]

If we haven't defined our own 'common', Novu will not permit us to update it. Please try downloading the Novu repository and give it a try

[alexrashed]

What do you mean with "Novu will not permit us to update it"? Can't you just set arbitrary values for transitive charts? Wouldn't it just be novu.localstack.namespaceOverride?