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

Secret handling - where should passwords, etc. be added #117

Open
jaolwi opened this issue Mar 25, 2024 · 5 comments
Open

Secret handling - where should passwords, etc. be added #117

jaolwi opened this issue Mar 25, 2024 · 5 comments

Comments

@jaolwi
Copy link

jaolwi commented Mar 25, 2024

I think the best way to handle this is to leave it up to the user whether he enters the password as plaintext in the values.yaml or integrates it via an existing secret. Here bitnami also has good examples. This procedure could also be useful for all backend service connection strings like postgresql.

Example:

datacite:
    ## @param invenio.datacite.enabled Enable DataCite provider
    ##
    enabled: false
    ## @param invenio.datacite.username Datacite username
    ##
    username: ""
    ## @param invenio.datacite.password Datacite password 
    ##
    password: ""
    ## @param invenio.datacite.existingSecret Existing secret name for datacite username and password
    ##
    existingSecret: ""
    ## @param invenio.datacite.secretKeys.usernameKey Name of key in existing secret to use for username. Only used when `invenio.datacite.existingSecret` is set.
    ## @param invenio.datacite.secretKeys.passwordKey Name of key in existing secret to use for password. Only used when `invenio.datacite.existingSecret` is set.
    ##
    secretKeys:
      usernameKey: ""
      passwordKey: ""
@lindhe
Copy link
Contributor

lindhe commented Mar 25, 2024

Relates to #112 (comment)

@lindhe
Copy link
Contributor

lindhe commented Mar 25, 2024

One area where I would like to see some improved secrets management is for providing custom environment variables to Invenio. Today, custom environment variables can be added via a ConfigMap invenio-config:

envFrom:
- configMapRef:
name: invenio-config

But there's no way to add custom values via a Secret instead.

I see no reason to keep the ConfigMap, I think that it can be replaced by a secret altogether (replacing configMapRef with secretRef in the example above). But if we want to keep it, we should at least add a Secret too!

EDIT: Scratch that! There's already a mechanism for that!! Just implemented a bit differently…

{{- range .Values.invenio.extra_env_from_secret }}
- name: {{ .name }}
valueFrom:
secretKeyRef:
name: {{ .valueFrom.secretKeyRef.name }}
key: {{ .valueFrom.secretKeyRef.key }}
{{- end }}

@jaolwi
Copy link
Author

jaolwi commented Mar 25, 2024

I absolutely agree with you. To make the whole thing even more flexible for the user, you could also implement it in this way:

values.yaml

 ## @param invenio.envFrom Load environment variables from kubernetes secret or config map to each invenio pod (web. worker, worker-beat)
  ##
  envFrom: []
    # - secretRef:
    #     name: env-secret
    # - configMapRef:
    #     name: config-map

web-deployment.yaml

command:...
envFrom:
  {{- with .Values.invenio.envFrom }}
    {{- toYaml . | nindent 12 }}
  {{- end }}  
env:...

This means that the user is completely free to decide what he wants to add to the pod as env vars and from where.

@lindhe
Copy link
Contributor

lindhe commented Mar 25, 2024

I think that sounds great. Are we also aiming to distinguish between the envs for web and worker containers? Because today, it seems like both containers get the same variables added to them. Not sure if it's a big problem, just that it feels like an uncommon pattern and tight coupling.

@jaolwi
Copy link
Author

jaolwi commented Mar 25, 2024

Yes, I think it makes sense to implement this for the individual pods and to separate it. However, I am not yet familiar enough with the software to be able to say which env vars are necessary in the web and which in the worker or whether both always need the same ones.

egabancho added a commit to egabancho/helm-invenio that referenced this issue Dec 13, 2024
* Deprecates non camelcase sentry variables.

* Addresses secret handling as described in inveniosoftware#117.
egabancho added a commit to egabancho/helm-invenio that referenced this issue Dec 13, 2024
* Deprecates non-camelcase sentry variables.

* Addresses secret handling as described in inveniosoftware#117.
egabancho added a commit to egabancho/helm-invenio that referenced this issue Dec 13, 2024
* Deprecates non-camelcase sentry variables.

* Addresses secret handling as described in inveniosoftware#117.
egabancho added a commit to egabancho/helm-invenio that referenced this issue Dec 16, 2024
* Deprecates non-camelcase sentry variables.

* Addresses secret handling as described in inveniosoftware#117.
egabancho added a commit to egabancho/helm-invenio that referenced this issue Dec 16, 2024
* Deprecates non-camelcase sentry variables.

* Addresses secret handling as described in inveniosoftware#117.
egabancho added a commit to egabancho/helm-invenio that referenced this issue Dec 16, 2024
* Deprecates non-camelcase sentry variables.

* Addresses secret handling as described in inveniosoftware#117.
egabancho added a commit to egabancho/helm-invenio that referenced this issue Dec 16, 2024
* Deprecates non-camelcase sentry variables.

* Addresses secret handling as described in inveniosoftware#117.
egabancho added a commit to egabancho/helm-invenio that referenced this issue Dec 16, 2024
* Deprecates non-camelcase sentry variables.

* Addresses secret handling as described in inveniosoftware#117.
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