puppetlabs / puppetserver-helm-chart

The Helm Chart for Puppet Server
Apache License 2.0
51 stars 56 forks source link

extraSecrets are not mounted to pods #192

Open ldaneliukas opened 10 months ago

ldaneliukas commented 10 months ago

Describe the Bug

https://github.com/puppetlabs/puppetserver-helm-chart/pull/125 introduced extraSecrets and these are added as volumes to all pods. The issue is, there they are not actually mounted (i.e. no volumeMounts are created). So whilst the secret is added, it can't really be used for anything within the existing containers.

Expected Behavior

The value for extraSecrets should require not only a name but a mountPath too, which would create volume mounts on the specified path in all containers. Changing it to a hash with key being the secret name and value the path could work, for example:

puppetserver:
  extraSecrets:
    myBigSecret: "/etc/puppetlabs/secretPath"

Then:

volumes:
  {{- range $key, $value := .Values.puppetserver.extraSecrets }}
  - name: {{ $key }}-volume
    secret:
      secretName: {{ $key }}
  {{- end }}

volumeMounts:
  {{- range $key, $value := .Values.puppetserver.extraSecrets }}
  - name: {{ $key }}-volume
    readOnly: true
    mountPath: {{ $value }}
  {{- end }}

Steps to Reproduce

  1. Add extraSecrets in your values.
  2. Deploy.
  3. You can't access the secrets as they aren't mounted.

Additional Context

I can create a PR for this.

ldaneliukas commented 10 months ago

I just noticed that this is a duplicate of https://github.com/puppetlabs/puppetserver-helm-chart/issues/166 , somehow I managed to miss it when searching existing issues.

cpiment commented 6 months ago

Hi @ldaneliukas, I see this has not been yet fixed. Do you mind if I open a PR using your solution? I thought about changing extrasecrets to an array of objects like this:

puppetserver:
  extraSecrets:
    - name: myBigSecret
      path: "/etc/puppetlabs/secretPath"
    - name: myOtherSecret
      path: "/etc/puppetlabs/otherSecretPath"

But I like your solution more.

ldaneliukas commented 6 months ago

Hi @cpiment, I have completely blanked on this proposal. If you could open a PR for this, it'd be great! 🙇