-
Notifications
You must be signed in to change notification settings - Fork 82
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
Feature: config in a ConfigMap #84
base: master
Are you sure you want to change the base?
Conversation
feat: Optional configMap for config feat: Optional configMap for config feat: Optional configMap for config chore: Updated version and docs chore: working on release action Revert "chore: Updated version and docs" This reverts commit 78e551c. chore: Updated version and docs Revert "chore: working on release action" This reverts commit 7d68e18.
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. |
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. 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 |
@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. |
@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? |
I'm using https://github.com/stakater/Reloader here to solving this issue. |
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. |
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? |
@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. |
@jkroepke & @sagikazarmark I've tested the upstream helm chart w/o changing the values file but that patch. Now it works, even with ArgoCD. 🙄 Sorry for the stress I made, I can't remember what was wrong back in the days and in the meantime several things changed... ☮️ |
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. |
@sagikazarmark can you take another look at this in light of my use case, please? |
@sagikazarmark I still have the need for this PR, my use cae is gitops where I render the manifests into a gitops manifest repo to be able to diff the changes of all manifests in a PR before I will let argo-cd do its magic. Another Option would be to have the secret use If u would give it a go I would update the PR. |
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
Chart.yaml
(see the contributing guide for details)Chart.yaml
(see the contributing guide for details)make docs