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

Permit use of Secrets instead of inline text for iSCSI authentication #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ndrwstn
Copy link

@ndrwstn ndrwstn commented Jan 10, 2025

This patch creates a new key under storageClasses for existingSecret where for each of the secrets can be provided directly (name and namepace) instead of managed by the chart. E.g.:

storageClasses:
	- name: iscsi-delete
		defaultClass: true
		reclaimPolicy: Delete
		volumeBindingMode: WaitForFirstConsumer
    allowVolumeExpansion: true
    parameters:
      fstype: ext4
    existingSecrets:
    	node-stage-secret:
    		name: node-stage-secret-dcsi-iscsi-delete
    		namespace: storage

The secret must be contain validly formatted keys. E.g.:

apiVersion: v1
kind: Secret
metadata:
 name: node-stage-secret-dcsi-iscsi-delete
 # namespace: storage
type: Opaque
stringData:
 node-db.node.session.auth.authmethod: "CHAP"
 node-db.node.session.auth.username: "<ISCSI_USERNAME>"
 node-db.node.session.auth.password: "<ISCSI_PASSWORD>"
 node-db.node.session.auth.username_in: "<ISCSI_USERNAME_IN>"
 node-db.node.session.auth.password_in: "<ISCSI_PASSWORD_IN>"

I chose to implement this as "all or nothing" regarding secrets management, with the belief that if someone is already in the weeds for one of the secrets, they would likely manually define all. So if the existingSecrets value exists (for a specific StorageClass), you must define all of the secrets that are necessary for that class. (Though some StorageClasses might use existingSecrets and others could use secrets--though I don't know why anyone would do that.)

I made this patch as I wasn't able to get any sort of secret injection to properly work on the existing implementation, and found several people referencing that they just encrypt the plain-text credentials in place. That seemed inelegant to me, given there is an entire secrets ecosystem in k8s (and I prefer to use ExternalSecrets to manage as much as possible.)

I'd be happy to write an update to the values file to reflect these changes if you find this PR suitable for inclusion.

…de Helm creation of any Secrets

- if existingSecrets found, user must provide for properly named and formatted `Secret` for that storage class.
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

Successfully merging this pull request may close these issues.

1 participant