Skip to content

Conversation

@kznrluk
Copy link

@kznrluk kznrluk commented Oct 25, 2025

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.

  1. It’s difficult to infer that controllers.main.containers.main.env maps to the env of immich-server just from its name. This is probably a limitation of bjw-s.common.loader.all, but as far as I know, Immich is the only chart currently using it.

  2. 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.yaml to see where the env variables 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.

@bo0tzz
Copy link
Member

bo0tzz commented Oct 25, 2025

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.

@kznrluk
Copy link
Author

kznrluk commented Oct 25, 2025

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.

@bo0tzz
Copy link
Member

bo0tzz commented Oct 25, 2025

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.

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.

@kznrluk
Copy link
Author

kznrluk commented Oct 30, 2025

You're right, my explanation was unclear.
To put it simply, what I meant is that if DB_PASSWORD supports valuesFrom, users would naturally assume that other values also support valuesFrom in the same way.

I also think it would be good to include a link to the bjw-s.common documentation.

@bo0tzz
Copy link
Member

bo0tzz commented Oct 30, 2025

I also think it would be good to include a link

There's already a whole bunch of links to it all over the place.

@kznrluk
Copy link
Author

kznrluk commented Nov 2, 2025

So, what changes would you like to see in this PR? Or are you not interested?

@GaidarOS
Copy link

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 sslmode=verify-full. So far most issues raised on the main repo or this have been for either sslmode=no-verify in the docs or sslmode=prefer in issues.
I'd argue that an example with mounting the ca and certs along with the sslcert/sslkey/sslrootcert params would be valuable. Especially since doing those mounts requires some digging into the working of the common chart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants