DSN is not optional for automigration

[Dennor]

DSN is not optional for automigration

[Demonsthere]

So, if I read this correctly, the pod fails because it doesn't recognise dsn as being set. In your values.yaml you dont specify the dsn in the config, not supply it as an env. The issue i think comes from here https://github.com/ory/k8s/blob/master/helm/charts/kratos/templates/deployment-kratos.yaml#L151 between the interaction of a user supplied dsn vs external secret 🤔

[Dennor]

Actually, it fails even before that, it fails here

k8s/helm/charts/kratos/templates/job-migration.yaml

Line 63 in 598c91b

{{- if not (empty ( include "kratos.dsn" . )) }}

. I'm not sure if the deployment would work, as it did not get past automigration but I assume the result would be same.

Just to note, this is an existing setup, what is failing is not new install, it's the upgrade.

But overall, yes, you are correct. I'm supplying secret externally and referring to it through secret.nameOverride, until now, that was working, because there was no check for emptiness on dsn in config. Now, with that check there, I need to set dsn to whatever.

Of course, it's not a big issue, I've just set dsn to whatever in my values.yaml and it works again, but I'm not sure if that's the intent here 😄

[Dennor]

Just one more note, in default values.yaml there's this:

k8s/helm/charts/kratos/values.yaml

Line 82 in 598c91b

# -- Provide custom name of existing secret, or custom name of secret to be created

Which I think implies that it's meant to be used the way I've used it? 🤔 Unless I've misread that comment 🤔

[Demonsthere]

No, you understand correctly :) Those two options are there to support similar use cases:

• The secretnameOverride allows you to use an existing secret that is sideloaded with the chart
• The Env overrides allows for more customization like construction of the DSN from envs injected into the system at runtime, or even supporting different dsn for the main application and for example courier (you can set different connection parameters for them, as by default they would reuse the same dsn string)

I would say this is a small (as there is a simple workaround as you noticed) regression that we need to address in the upgrade pipeline 🤔

[Demonsthere]

Hi there! I pushed a fix to the generation logic, could you please verify from the master version if your case is supported properly? I updated the test cases to cover more options, and it looks ok, but would like real user feedback before making the next release 😉