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

fix(helm): reference secret-env by fullname instead release-name #2406

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

swarnat
Copy link

@swarnat swarnat commented Dec 29, 2024

Related to helm Charts:
Currently the reference to secret is done by

envFrom:
- secretRef:
  name: {{ .Release.Name }}-env

Link

but secret object is created like this:

metadata:
  name: {{ include "umap.fullname" . }}-env

Link

Works as long as no Sub-Chart feature is used, when individual configuration is stored within a git repo.
Then the subchart name is added to fullname. (And I think if custom fullName is set)

To be safe, {{ include "umap.fullname" . }} is correct.

Should be no breaking change, when it is working at the moment. (Because then both return same value)
It is only a breaking change, if it is not working at the moment.

@davidbgk
Copy link
Contributor

@swarnat thank you for your contribution!

There are two other places where {{ .Release.Name }} is referenced, do you think it's better to be consistent with a {{ include "umap.fullname" . }} call too?

@swarnat
Copy link
Author

swarnat commented Dec 29, 2024

I just validate that. For configMaps yes.

For the postgis/cnpg cluster I'm not sure, but I think yes, how second level of subchart is handled. Never worked with cnpg
I do not get it running, because the CRD postgresql.cnpg.io/Cluster is missing, but expected.
I currently prepare an environment to test it within an empty k8s system and will update the PR.

Update: Check done. cnpg-app is named without the fullname, because all depencies are handled all the same level. I just updated the ConfigMap reference

But the setup of cnpg is not working, at the moment. It was necessary to use this operator setup before:
https://cloudnative-pg.io/documentation/1.22/installation_upgrade/ Maybe there is better solution for integrated postgis deployment, which works out of the box.

@yohanboniface
Copy link
Member

Thanks!

cc @NaPs :)

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

Successfully merging this pull request may close these issues.

3 participants