mastodon / chart

Helm chart for Mastodon deployment in Kubernetes
GNU Affero General Public License v3.0
154 stars 89 forks source link

Set the OIDC client secret in an existingSecret in the helm chart #19

Open rasca opened 1 year ago

rasca commented 1 year ago

Steps to reproduce the problem

Install from helm chart.

Expected behaviour

The oidc client secret should be set in an existing secret for extra security (on par with the rest of the secure tokens)

Actual behaviour

You need to set it in the values

Detailed description

I've created a pull request to fix this: https://github.com/mastodon/mastodon/pull/20803

Specifications

v3.5.5 v4.0.2

SISheogorath commented 1 year ago

In what perspective would this prove security? Helm uses secrets to store its state and it creates secrets to hold that information.

From a Kubernetes point of view it doesn't vhange the domain of the secrets.

You can of course argue that someone might use a controller to generate secrets by either having an OIDC operator or just by using a controller to manage secrets (e.g. sealed secrets, vault, …) however, this doesn't make secrets more secure, just changes the way they are created. What makes them more secure in this case, would be your opinionated way of deploying a helm chart, since (as already mentioned) you might store these values outside the chart.

In short, yes, I think it makes sense to allow external/existing secrets to be used, but I don't think it's for security, rather than convenience reasons.

deepy commented 1 year ago

Currently the OIDC client secret is set in a configmap, so the additional security comes from access control (like allowing reading configmaps but not secrets)

I agree that it should be moved from the ConfigMap to a Secret as its a sensitive value

renchap commented 1 year ago

This will definitely be a good idea to provide a way to use an existing Secret for this.

It could be done similar to https://github.com/mastodon/chart/pull/38 and I will review any PR implementing this!

deepy commented 1 year ago

You should probably move the value from the ConfigMap to the Secret regardless of supporting external secrets