Skip to content
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

Improve handling of secret variables and files in autoconfig #2141

Closed

Conversation

pathob
Copy link
Contributor

@pathob pathob commented Jan 8, 2024

In the recent state, the autoconfig feature required either all database secrets to be passed as environment variables, or all to be passed as secret files, but didn't allow to use a mix of these variants, e.g. passing the database user and name as environment variables, and only the password as a file.

This is now fixed and allows working with database values the same way, as the Postgres and MySQL/MariaDB images do; with mixed variants also.

In the recent state, the autoconfig feature required either all database
secrets to be passed as environment variables, or all to be passed as
secret files, but didn't allow to use a mix of these variants, e.g.
passing the database user and name as environment variables, and only
the password as a file.

This is now fixed and allows working with database values the same way,
as the Postgres and MySQL/MariaDB images do; with mixed variants also.

Signed-off-by: Patrick Hobusch <patrick@hobusch.net>
@pathob
Copy link
Contributor Author

pathob commented Jan 16, 2024

@J0WI 👀

@pathob
Copy link
Contributor Author

pathob commented Jan 24, 2024

Hello @J0WI @joshtrichards how can I help to move this forward?

@J0WI
Copy link
Contributor

J0WI commented Feb 8, 2024

Duplicate of #1996 and #1942. (Didn't had the chance to look at it yet.)

@pathob
Copy link
Contributor Author

pathob commented Feb 9, 2024

@J0WI I think the logic of my PR is a bit better, because it also checks the existence of the files. But tbh there seems something wrong with Nextcloud. In my deployment I manually needed to set the NEXTCLOUD_DB_ENV_POSTGRES_HOST variable, because it was missing, even though POSTGRES_HOST is correctly set. Unfortunately I couldn't find the location yet where these NEXTCLOUD_DB_ENV variables get "generated".

It would be nice to move the PR forward, please let me know how I can help...

@pathob
Copy link
Contributor Author

pathob commented Feb 19, 2024

Unfortunately, the way this PR was (not) handled led me to the conclusion that this image is not suitable for production. Since the AIO image violates basic Docker principles, I will replace my Docker-based pilot setup with the service from NixOS.

@pathob pathob closed this Feb 19, 2024
@pathob pathob deleted the autoconfig-secret-files-improvements branch February 19, 2024 19:08
@joshtrichards
Copy link
Member

joshtrichards commented Feb 19, 2024

But tbh there seems something wrong with Nextcloud. In my deployment I manually needed to set the NEXTCLOUD_DB_ENV_POSTGRES_HOST variable, because it was missing, even though POSTGRES_HOST is correctly set. Unfortunately I couldn't find the location yet where these NEXTCLOUD_DB_ENV variables get "generated".

Where precisely are you seeing references to (or a dependency on) this variable (NEXTCLOUD_DB_ENV_POSTGRES_HOST)?

For what it's worth, I just brought up a from scratch test stack with the Apache v28 image with PostgreSQL and had no problems.

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

Successfully merging this pull request may close these issues.

3 participants