-
-
Notifications
You must be signed in to change notification settings - Fork 97
chore: add example database connection env vars #276
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
base: main
Are you sure you want to change the base?
Conversation
|
The reason I haven't documented specific env vars like this is because especially in an env like Kubernetes, people may well just want to set DB_URL from a secretRef instead of setting all the vars separately etc. The URL to the docs could be worth adding tho. |
|
Yes, I agree with your point. For that reason, writing it in a way that allows only DB_PASSWORD to be set via secretRef could indeed cause some confusion. However, if a user wants to retrieve DB_URL from a secretRef, given that the template doesn’t apply any special expansion only to DB_PASSWORD and that there’s another dependency called bjw-s.common.loader.all, they might expect the intermediate layer to interpret it correctly and try writing DB_URL in the same way as DB_PASSWORD. While I agree that documenting every possible pattern would be excessive, providing some simple examples of how each key can be defined would make things clearer and more user-friendly—especially for those who need to handle PostgreSQL migration. |
I don't quite understand what you're saying here? If there needs to be clarification on how the env vars can be defined/referenced, then we should refer to the library docs (https://github.com/bjw-s-labs/helm-charts/blob/common-4.3.0/charts/library/common/values.yaml#L274-L298) instead of repeating those here. |
|
You're right, my explanation was unclear. I also think it would be good to include a link to the |
There's already a whole bunch of links to it all over the place. |
|
So, what changes would you like to see in this PR? Or are you not interested? |
|
I'm sorry if I'm highjacking but, one of the things I'd definitely ask for, as it's quite lacking from what I could tell, is the inclusion of the full cert chain to use something like |
In this PR, I add commented-out examples of the database-related configuration values that are required for using Immich, along with links to the relevant documentation, to
values.yaml.It’s difficult to infer that
controllers.main.containers.main.envmaps to theenvofimmich-serverjust from its name. This is probably a limitation ofbjw-s.common.loader.all, but as far as I know, Immich is the only chart currently using it.For legacy chart users migrating their database settings, there is insufficient explanation of where to move those values. While the existing comments indicate the location, it still remains unclear which keys and values are valid and how they should be configured.
When migrating from chart version 0.9.x to 0.10.x, I used the following methods to investigate the database configuration values:
Checked
./charts/immich/templates/server.yamlto see where theenvvariables are defined.→ Failed: the expansion is hidden by
bjw-s.common.loader.all, and I couldn’t determine how it worked.Looked for information in the official documentation.
→ Failed: there was no explanation about these settings.
Ultimately, I found Issue #258 and relied on the actual configuration file shared by user kryoseu, which allowed me to configure it successfully.
#258 (comment)
The task of migrating the database connection settings took significantly more time than expected.
With this PR, the configuration process for the database should become much clearer.