chart: bugfix for bypassSecrets

[goshlanguage]

Fix bug introduced by #514

While #514 was tested using helm template, submitting the output to the API uncovered a bit of a logical issue.

This bug surfaces when bypassSecrets is set to true. When oidc is enabled, the secret for oidc is no longer created due to bypassSecrets, however its mount in the deployment still exists. When these conditions are present, the deployment fails because it attempts the mount the secret, which is missing the key oidcClientID.

How to use

Testing done

This time, I tested without bypassSecrets, and with bypassSecrets against a kind cluster locally

[yolossn]

The ne .Values.config.auth.bypassSecrets true check should be applied for the eq .Values.config.auth.mode "github" case also. It will be better to nest the auth related conditions inside ne .Values.config.auth.bypassSecrets true condition

[yolossn]

Aah, good catch. Left a comment.

[goshlanguage]

As bypassSecrets wasn't an expression, I switched to the existingSecrets pattern for oidc auth since its a well established pattern and better captures what I was trying to work into the PR that caused this. It's also nicer to the user in that they can supply the secret name and have it mounted automatically.

This simplifies the logic a bit, hope you like it.

Here are the permutations I've tested with this update, omitting the entire file outputs and including only if secrets is generated (for brevity):

default case

helm template  --output-dir test-output/ -f values.yaml . | grep secrets 
wrote test-output//nebraska/charts/postgresql/templates/secrets.yaml

As expected, no secrets are generated, because the deployment mounts what the subchart creates, and no auth is setup

postgresExistingSecret disabled and password set instead

helm template  --output-dir test-output/ . --set config.database.passwordExistingSecret.enabled=false --set config.database.password=foobar | grep secrets
wrote test-output//nebraska/charts/postgresql/templates/secrets.yaml
wrote test-output//nebraska/templates/secrets.yaml

secrets.yaml

---
apiVersion: v1
kind: Secret
metadata:
  name: RELEASE-NAME-nebraska
type: Opaque
data:
  dbPassword: Zm9vYmFy

secret populated with set password as expected

oidc enabled

helm template  --output-dir test-output/ . --set config.auth.mode=oidc --set config.auth.oidc.clientID=foo --set config.auth.oidc.clientSecret=bar | grep secrets
wrote test-output//nebraska/templates/secrets.yaml

secrets.yaml

---
apiVersion: v1
kind: Secret
metadata:
  name: RELEASE-NAME-nebraska
type: Opaque
data:
  oidcClientID: Zm9v
  oidcClientSecret: YmFy

oidc secrets passed through as expected, deployment mounts look good:

deployment.yaml

...
            - name: "NEBRASKA_OIDC_CLIENT_ID"
              valueFrom:
                secretKeyRef:
                  name: RELEASE-NAME-nebraska
                  key: oidcClientID
            - name: "NEBRASKA_OIDC_CLIENT_SECRET"
              valueFrom:
                secretKeyRef:
                  name: RELEASE-NAME-nebraska
                  key: oidcClientSecret
...

oidc secrets supplied

helm template --output-dir test-output/ . --set config.auth.mode=oidc --set config.auth.oidc.existingSecret=foo | grep secrets                                                    
wrote test-output//nebraska/charts/postgresql/templates/secrets.yaml

As expected, Only subchart secret created. The appropriate mount is supplied to the deployment to match the new key

deployment.yaml

            - name: "NEBRASKA_OIDC_CLIENT_ID"
              valueFrom:
                secretKeyRef:
                  name: foo
                  key: oidcClientID
            - name: "NEBRASKA_OIDC_CLIENT_SECRET"
              valueFrom:
                secretKeyRef:
                  name: foo
                  key: oidcClientSecret

[yolossn]

Hey,
From the first look, If I am not wrong the suggested config.auth.oidc.exisingSecret solution looks tailormade to the exact problem you are trying to solve ( using existing secret for OIDC Client ID and Secret). There can be another use case where someone wants to use existing secrets for github auth mode also, because of that I think bypassSecrets is a more generic solution. WDYT?

[goshlanguage]

Not to contradict, but its a pattern, not a tailor made solution, it can be applied to both. Would you like me to also add the pattern for github?

[yolossn]

Yes, please. let's also add it to github auth mode.

[goshlanguage]

For consistency, here are the same checks for the github implemention:

default

helm template  --output-dir test-output/ . | grep secrets

wrote test-output//nebraska/charts/postgresql/templates/secrets.yaml

github enabled

helm template  --output-dir test-output/ . --set config.auth.mode=github --set config.auth.github.clientID=foo --set config.auth.github.clientSecret=bar --set config.auth.github.sessionAuthKey=baz --set config.auth.github.sessionCryptKey=qux --set config.auth.github.webhookSecret=quux | grep secrets

wrote test-output//nebraska/charts/postgresql/templates/secrets.yaml
wrote test-output//nebraska/templates/secrets.yaml

secrets.yaml

apiVersion: v1
kind: Secret
metadata:
  name: RELEASE-NAME-nebraska
type: Opaque
data:
  ghClientSecret: YmFy
  ghSessionAuthKey: YmF6
  ghSessionCryptKey: cXV4
  ghWebhookSecret: cXV1eA==

deployment.yaml (some spec omitted for brevity)

            - name: "NEBRASKA_GITHUB_OAUTH_CLIENT_ID"
              value: "foo"
            - name: "NEBRASKA_GITHUB_OAUTH_CLIENT_SECRET"
              valueFrom:
                secretKeyRef:
                  name: RELEASE-NAME-nebraska
                  key: ghClientSecret
            - name: "NEBRASKA_GITHUB_SESSION_SECRET"
                  key: ghSessionAuthKey
            - name: "NEBRASKA_GITHUB_SESSION_CRYPT_KEY"
                  key: ghSessionCryptKey
            - name: "NEBRASKA_GITHUB_WEBHOOK_SECRET"
                  key: ghWebhookSecret

github secrets supplied

helm template  --output-dir test-output/ . --set config.auth.mode=github --set config.auth.github.clientID=foo --set config.auth.github.existingSecret=bar | grep secrets
wrote test-output//nebraska/charts/postgresql/templates/secrets.yaml

deployment.yaml

            - name: "NEBRASKA_GITHUB_OAUTH_CLIENT_SECRET"
              valueFrom:
                secretKeyRef:
                  name: bar
                  key: ghClientSecret
            - name: "NEBRASKA_GITHUB_SESSION_SECRET"
                  name: bar
                  key: ghSessionAuthKey
            - name: "NEBRASKA_GITHUB_SESSION_CRYPT_KEY"
                  name: bar
                  key: ghSessionCryptKey
            - name: "NEBRASKA_GITHUB_WEBHOOK_SECRET"
                  name: RELEASE-NAME-nebraska
                  key: ghWebhookSecret

[goshlanguage]

@yolossn what do you think?

[yolossn]

Tested locally by generating the template files for both oidc and github. Thanks for providing detailed cases in the PR comments. Just a small change in the README file. After that, we should be good to go.

[yolossn]

@goshlanguage Few nitpicks and then we should be good to go. Sorry I missed them earlier. Please combine the suggestion commits into one.

[goshlanguage]

Why not just enable squash commits on the repo? It's not the best developer experience imo.

[yolossn]

Sorry for the inconvenience, I'll discuss this with the team.

[goshlanguage]

No worries, just a suggestion, as the end result would be the same.

Downside being of course, you'd have to trust the collaborators to follow guidelines, so that's not always realistic unfortunately.

Thanks for working with me on this @yolossn

[joaquimrocha]

Hey @goshlanguage , thanks for this nice change!
Usually squashing commits means that a lot of the context is lost and so we try to rely on atomic commits and have guidelines for contribution.

In your case it'd not happen because it was a single commit but I do prefer a closer alignment with contributors because maybe they made an effort on the commit title/body and wouldn't be fair to to see it changed when squashing.

So we'll keep an eye on whether this option makes sense in the future. Cheers, and we hope to see more contributions from you. 🍻