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

feat(chart): Support for adding EnvVars to the dex pod #1460

Merged
merged 7 commits into from
Feb 22, 2024

Conversation

sennerholm
Copy link
Contributor

For example to add a CliettSecret from a secret already created in the cluster

Sample:

api:
    host: <fqdn>
    oidc:
      enabled: true
      dex:
        enabled: true
        envVars:
          - name: CLIENT_SECRET
            valueFrom:
              secretKeyRef:
                name: github-dex
                key: dex.github.clientSecret

        connectors:
          - type: github
            id: github
            name: GitHub
            config:
              clientID: <theClientIdFromGithub>
              clientSecret: "$CLIENT_SECRET"
              redirectURI: https://<fqdn>/dex/callback
              orgs:
              - name: <YourGithubOrg>

Inspired by the config to the upstream dexidp chart at: https://artifacthub.io/packages/helm/dex/dex

…tSecret from an secret

Signed-off-by: Mikael Sennerholm <mikael@sennerholm.net>
@sennerholm sennerholm requested a review from a team as a code owner February 5, 2024 21:25
Copy link

netlify bot commented Feb 5, 2024

Deploy Preview for docs-kargo-akuity-io ready!

Name Link
🔨 Latest commit 7ce582e
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/65d7bc6e7ea46a0008ef8fc1
😎 Deploy Preview https://deploy-preview-1460.kargo.akuity.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sennerholm
Copy link
Contributor Author

If you have any testcases for the helm chart, please point me in the right direction and I will try to update those.

Signed-off-by: Mikael Sennerholm <mikael@sennerholm.net>
@sennerholm sennerholm changed the title Support for adding EnvVars to the dex pod, feat: Support for adding EnvVars to the dex pod Feb 5, 2024
@sennerholm sennerholm changed the title feat: Support for adding EnvVars to the dex pod feat(chart): Support for adding EnvVars to the dex pod Feb 5, 2024
Signed-off-by: Mikael Sennerholm <mikael@sennerholm.net>
@krancour
Copy link
Member

krancour commented Feb 16, 2024

Thanks @sennerholm!

This solves something I knew was a problem and hadn't really gotten around to dealing with yet.

For the exact reason that connector configs can contain sensitive information, the chart puts all connector config in a secret... but that doesn't really achieve much. Your connector config still appears in plain text in your Kargo installation's values.yaml file -- sensitive info and all -- which makes it unsuitable for storing in a git repo.

Your approach opens the possibility of secrets being managed independently of the chart through some other solution, like Sealed Secrets, for instance, and then merely referenced from the connector config.

Love it!

If you don't mind, I may amend this PR with some changes to the wording in the docs, but I'm leaving your approach exactly as is.

@sennerholm
Copy link
Contributor Author

sennerholm commented Feb 16, 2024 via email

Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
@krancour krancour enabled auto-merge February 22, 2024 21:28
@krancour
Copy link
Member

Thanks again @sennerholm! This really is a great improvement.

@krancour krancour added this pull request to the merge queue Feb 22, 2024
Merged via the queue into akuity:main with commit c159aa8 Feb 22, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants