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

[imio/helm-plausible-analytics] bring your own secret #3

Closed
mrdotb opened this issue Oct 25, 2023 · 3 comments
Closed

[imio/helm-plausible-analytics] bring your own secret #3

mrdotb opened this issue Oct 25, 2023 · 3 comments

Comments

@mrdotb
Copy link
Contributor

mrdotb commented Oct 25, 2023

Bonjour 🇫🇷 ,

Is your feature request related to a problem? Please describe.
I want to define the secret myself. While it's possible to disable secret.yaml with .Values.secret.create the name of the secret cannot be specified. To bring your own secret you have to give it the same name as name: {{ include "plausible-analytics.fullname" . }}

Describe the solution you'd like
A clear and concise description of what you want to happen.
Make the secret name configurable like in bitnami/clickhouse with a new key in values existingSecret: ""

Describe alternatives you've considered

Additional context
I already implemented some changes locally but I'm pretty new to k8s / helm so I would like to have your input.

My idea is to add a new function in _helpers.tpl to get the specified secret name or default to fullname.

{{/*
Define the name of the secret to use
*/}}
{{- define "plausible-analytics.secretName" -}}
{{- if .Values.secret.existingSecret -}}
{{- .Values.secret.existingSecret -}}
{{- else -}}
{{- template "plausible-analytics.fullname" . -}}
{{- end -}}
{{- end -}}

Then use it like

            - name: CLICKHOUSE_USER
              valueFrom:
                secretKeyRef:
                  key: CLICKHOUSE_USER
                  name: {{ include "plausible-analytics.secretName" . }}

In deployment.yaml
There is some fix to make here there are many checks on values but if you bring your own secret it's does not make sense to refer values. Not sure what is the most idiomatic way to handle this case.

            {{- if .Values.clickhouse.url }}
            - name: CLICKHOUSE_DATABASE_URL
              valueFrom:
                secretKeyRef:
                  key: CLICKHOUSE_DATABASE_URL
                  name: {{ include "plausible-analytics.fullname" . }}
            {{- end }}
@alexnuttinck
Copy link
Member

Salut @mrdotb 🇧🇪,

Thanks for your proposal! In fact, we are also interested by this feature. We plan to use external-secret to manage our k8s secrets.

Can you open a PR? So, we could work together on this feature?
Your proposal to create a new function in _helpers.tpl seems great to me.

In deployment.yaml
There is some fix to make here there are many checks on values but if you bring your own secret it's does not make sense to refer values. Not sure what is the most idiomatic way to handle this case.

I wonder if we could not simply remove all these conditions and the same in the secret.yaml file, which would result to pass some emtpy variables if the variables are not set in the values.yaml. (We could also set some default values if it is not set in the values.yaml file). What do you think ?

Another solution would be to add the possibility to define extraEnv like this in the deployment.yaml file:

{{- if .Values.extraEnv }}
{{ toYaml .Values.extraEnv | indent 10 }}
{{- end }}
          ports:
            - name: http
              containerPort: 8000
              protocol: TCP

Then, we could set in the values.yaml something like:

extraEnv:
- name: CLICKHOUSE_DATABASE_URL
  valueFrom:
  secretKeyRef:
    key: CLICKHOUSE_DATABASE_URL
    name: my-extra-secret-file

It would allow to add some variables which are not defined by default in the helm chart.

@mrdotb
Copy link
Contributor Author

mrdotb commented Oct 31, 2023

On my forked version I already did the changes and it work with existing secret created with sealed-secrets or sops.

I wonder if we could not simply remove all these conditions and the same in the secret.yaml file, which would result to pass some emtpy variables if the variables are not set in the values.yaml. (We could also set some default values if it is not set in the values.yaml file). What do you think ?

About the check on secret value from values.yaml. I added more enabled boolean and check on that instead. The thing here to prevent problem would be the carefully check plausible ENV and make the difference between the mandatory / optional ENV.

The extraEnv is a good thing for futur env added in plausible.

I will rework my current fork and make a PR.

@mrdotb mrdotb mentioned this issue Nov 2, 2023
3 tasks
@alexnuttinck
Copy link
Member

Resoved by #4. Thanks!

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