inveniosoftware / helm-invenio

Helm charts for deploying an Invenio instance
https://helm-invenio.readthedocs.io
7 stars 18 forks source link

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

Open jaolwi opened 4 months ago

jaolwi commented 4 months ago

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 commented 4 months ago

Relates to https://github.com/inveniosoftware/helm-invenio/issues/112#issuecomment-2017524354

lindhe commented 4 months ago

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:

https://github.com/inveniosoftware/helm-invenio/blob/eb6544fdeb7e762972db1dae26f848c2f2e4c029/charts/invenio/templates/web-deployment.yaml#L28-L30

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…

https://github.com/inveniosoftware/helm-invenio/blob/eb6544fdeb7e762972db1dae26f848c2f2e4c029/charts/invenio/templates/web-deployment.yaml#L83-L89

jaolwi commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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.