Feature: config in a ConfigMap

[mheuser]

Overview

This PR adds the option to use a ConfigMap to store the content of config.

What this PR does / why we need it

Closes #83

Special notes for your reviewer

Checklist

• [x] Change log updated in Chart.yaml (see the contributing guide for details)
• [x] Chart version bumped in Chart.yaml (see the contributing guide for details)
• [x] Documentation regenerated by running make docs

[sagikazarmark]

Thanks for the PR @mheuser !

Can you please clarify why using a secret does not fit your use case? Dex config usually does contain information that's worth protecting from the prying eyes.

[mheuser]

Since I use gitops for managing my clusters which are staged (dev, stage, prod), I try to reuse as much configuration as possible between them. So I try to use only secrets where needed to have as much text files that I can check into git. The dex config makes it possible to get secrets from the env which then can be injected via env/secretKeyRef which I manage then with external secrets.

As it is implemented now the default case will still be config as secret and optionally as config map.

Hape that makes some sense to you @sagikazarmark

[pluhmen]

@sagikazarmark This PR is important to get the deployment of dex' via ArgoCD working.

Without ArgoCD does not recognize that there is a change as the secret never gets generated. So to have a GitOps style config managment (secrets are out sourced) I patched my helm chart and now it works like a charm.

[sagikazarmark]

@mheuser sorry, I still don't understand why it matters that the config is stored in secret or not. It will get mounted the same way and you can still use external secrets or env vars to get secret values into the pod.

So what's the benefit of being able to choose between configmap and secret? From my vantage point it's an extra complexity with no actual benefit.

@pluhmen what secret does never get generated? What does ArgoCD has to do with it? Can you please elaborate?

[jkroepke]

I'm using https://github.com/stakater/Reloader here to solving this issue.

[sagikazarmark]

Reloader is great for automatically triggering a rollout when an externally managed secret changes.

When the chart managed secret changes, however, the deployment is also changed: https://github.com/dexidp/helm-charts/blob/master/charts/dex/templates/deployment.yaml#L25

So I'm still not sure how using a configmap instead is related.

[pluhmen]

Sorry, but I don't understand this discussion, we as users want to configure the application by using a CONFIGmap and want to have separated secrets.

In addition we showed that we are using GitOps style to deploy services like dex. At the moment this does not work without this PR and we are forced to patch manually. :(

@sagikazarmark So the question is, will this PR ever being merged?

[jkroepke]

@pluhmen You can deploy your service using GitOps without this PR. Currently, no one understand it. You said you need this because of GitOps, but GitOps works withs secrets. too.

[pluhmen]

@jkroepke & @sagikazarmark I've tested the upstream helm chart w/o changing the values file but that patch. Now it works, even with ArgoCD. :roll_eyes:

Sorry for the stress I made, I can't remember what was wrong back in the days and in the meantime several things changed... :peace_symbol:

[jcogilvie]

Here's a concrete use case: I use terraform. I don't want my helm values files containing secrets, because the template outputs can be public in a terraform plan or on a PR (we have a tool that posts diffs).

Instead, I want everything configurable in helm with links to externally-created secrets (which dex already supports). This way, non-secrets can be cleanly diffable in the helm chart while secrets are not revealed.

As it is today, this chart expects config to be populated in the values, which in turn puts secrets in the values. (As an aside, I believe this breaks the environment variable interpolation feature of dex config if I try to use external secrets for values that are actually secret.)

So either this PR, or a fix to variable interpolation, are necessary for my case.

[jcogilvie]

@sagikazarmark can you take another look at this in light of my use case, please?