feat: Allow users to inject credentials file separately

brunoluiz commented 1 year ago

Helm charts certainly make things easier and more standardised, so I am glad this exists. My suggestion/feature request would be to allow users to inject the credentials file from a secret value, as at the moment the only way to pass the tunnel secret is through setting .Values.cloudflare.secret.

This is not ideal because:

a. It requires the whole chart to be a secret, which in many cases it might not be (example: if it just lives in Github) b. Doesn't give flexibility for users to use external secrets managers

A couple of solutions are possible:

1. Encoding the whole local `values.yaml` as a secret

Correct if I am wrong, but this seems to be the only solution with the current approach (raw secret in the values.yaml). ExternalSecrets users can leverage the templating engine to create a values.yaml that then is used by helm (well, at least with Flux it is possible to reference secrets as values).

If this is the way to go, perhaps might be good to leave some notes in the README on the safest way of doing this.

2. Change `cloudflared` to accept all credentials values via env variables and allow env injection in the chart

This is quite a common approach in other projects but probably would require refactoring not only this chart, but as well cloudflared. Possibly unlikely?

3. Chart accepts a `credentialsSecretRef`

In this case, users can map the credential file secret somewhere else and, the secrets.yaml shouldn't be used if this is set. Instead, the deployment.yaml would load the secret defined by the user.

This would be very handy, not attaching this project to any specific secret management solution, while still providing users standardisation through Helm.

froblesmartin commented 1 year ago

Hi @brunoluiz ! I will work on the 3rd solution as is the simple and standard way. Probably will raise a PR later today.

In the end, there are many different ways of managing secrets in K8s, but the most standard way for a Helm chart to face this is to just allow referencing a secret, and then whoever uses the chart, can generate that secret however they want.

uhthomas commented 1 year ago

I was going to suggest this could be closed, but the cloudflare-tunnel-remote chart needs this change too.

brunoluiz commented 1 year ago

@uhthomas I guess it can be tracked separately

uhthomas commented 1 year ago

@uhthomas I guess it can be tracked separately

It would probably be easier to keep this issue open.

c0mponents commented 7 months ago

@uhthomas could we get a review on @hedgieinsocks PR #68 ?

cloudflare / helm-charts