-
-
Notifications
You must be signed in to change notification settings - Fork 259
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: allow dsn to be configured optionally #678
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! :)
Can we add one test case for that? In the hacks/values
directory we have the values.yaml that are used in our CI. I'd like this optional DSN option to be enabled for one one of the charts (so loaded from an external secret, the secret can be stored and loaded from here https://github.com/ory/k8s/tree/master/hacks/manifests)
I think we fall into helm/helm#12438 here 😞 We can try with the |
@Demonsthere Thanks for all of the help with the tests. It looks like it is all working now, and it matches how Hydra handles this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there!
Great job with the pr :) I would have a minot nitpick before we merge this: since now we see in the values for kratos what exactly needs to be modified to allow sideloading dsn, could you update the docs to show that? We could also link in the docs to the values files for kratos for a real life example!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updating docs :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Related Issue or Design Document
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact security@ory.sh) from the maintainers to push the changes.
Further comments