Support disabling pulling credentials from awsAccessSecret.name in the Helm chart

[joelthompson]

/feature

Is your feature request related to a problem? Please describe.

The Helm chart for the DaemonSet's s3-plugin container will always configure the AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and AWS_SESSION_TOKEN environment variables to be sourced from the awsAccessSecret.name secret:

mountpoint-s3-csi-driver/charts/aws-mountpoint-s3-csi-driver/templates/node.yaml

Lines 102 to 121 in 5cf881c

	{{- with .Values.awsAccessSecret }}
	- name: AWS_ACCESS_KEY_ID
	valueFrom:
	secretKeyRef:
	name: {{ .name }}
	key: {{ .keyId }}
	optional: true
	- name: AWS_SECRET_ACCESS_KEY
	valueFrom:
	secretKeyRef:
	name: {{ .name }}
	key: {{ .accessKey }}
	optional: true
	- name: AWS_SESSION_TOKEN
	valueFrom:
	secretKeyRef:
	name: {{ .name }}
	key: {{ .sessionToken }}
	optional: true
	{{- end }}

While the variables are set as optional: true, I think this is still bad practice from a security hygiene perspective -- the best practice would be to use EKS Pod Identity or IRSA, rather than static credentials, so that should be the default behavior, and I don't want these environment variables configured at all. It's also potentially confusing when looking at the rendered DaemonSet configuration to see the different AWS_* environment variables configured when they're not being used.

Further, the default value for the secret name is very generic, aws-secret, and so if something else creates a Secret in the same namespace named aws-secret then all of a sudden, my driver pods will start to use those credentials instead of the IRSA credentials, and that could cause it to start to fail.

Lastly, if somebody wants to use credentials from aws-secret but forgets to create it, then it should fail loudly saying that the secret doesn't exist, rather than failing silently with the environment variables just not getting set.

Describe the solution you'd like in detail

Perhaps something like:

awsAccessSecret:
  enabled: true
  optional: true

And then the environment variables in the chart could be:

            {{- with .Values.awsAccessSecret }}
            {{- if .enabled }}
            - name: AWS_ACCESS_KEY_ID
              valueFrom:
                secretKeyRef:
                  name: {{ .name }}
                  key: {{ .keyId }}
                  optional: {{ .optional }}
# ...
            {{- end }}
            {{- end }}

Then, I can set awsAccessSecret.enabled to false, and folks who want to can set awsAccessSecret.optional to false.

Describe alternatives you've considered

Set awsAccessSecret.name to something like s3-csi-secret-creds-do-not-exist to ensure my pods will never pick up the wrong credentials when a secret with a very generic name gets created. It's still ugly, but it's more clear what the intent is.

Additional context

[unexge]

Hey @joelthompson, I think I agree with most of your points. Disabling long-term AWS credentials should be already possible though - as we're using with in our Helm template here https://github.com/awslabs/mountpoint-s3-csi-driver/blob/v1.10.0/charts/aws-mountpoint-s3-csi-driver/templates/node.yaml#L102-L121, if you pass null for awsAccessSecret during your Helm installation it should omit that section completely:

helm upgrade --install aws-mountpoint-s3-csi-driver \
    --namespace kube-system \
    --set awsAccessSecret=null \
    aws-mountpoint-s3-csi-driver/aws-mountpoint-s3-csi-driver

[sstarcher]

I verified that setting it to null works. I would think that should be the default instead of the dummy values.