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

Autoconfig environment _FILE variables #2062

Open
fopina opened this issue Sep 8, 2023 · 6 comments
Open

Autoconfig environment _FILE variables #2062

fopina opened this issue Sep 8, 2023 · 6 comments

Comments

@fopina
Copy link

fopina commented Sep 8, 2023

I just spent 1 hour googling and experimenting with my docker setup because nextcloud container would always use sqlite database no matter what I did.

Finally decided to look into the code and found

} elseif (getenv('POSTGRES_DB_FILE') && getenv('POSTGRES_USER_FILE') && getenv('POSTGRES_PASSWORD_FILE') && getenv('POSTGRES_HOST')) {
$AUTOCONFIG['dbtype'] = 'pgsql';
$AUTOCONFIG['dbname'] = trim(file_get_contents(getenv('POSTGRES_DB_FILE')));
$AUTOCONFIG['dbuser'] = trim(file_get_contents(getenv('POSTGRES_USER_FILE')));
$AUTOCONFIG['dbpass'] = trim(file_get_contents(getenv('POSTGRES_PASSWORD_FILE')));
$AUTOCONFIG['dbhost'] = getenv('POSTGRES_HOST');
$autoconfig_enabled = true;
} elseif (getenv('POSTGRES_DB') && getenv('POSTGRES_USER') && getenv('POSTGRES_PASSWORD') && getenv('POSTGRES_HOST')) {
$AUTOCONFIG['dbtype'] = 'pgsql';
$AUTOCONFIG['dbname'] = getenv('POSTGRES_DB');
$AUTOCONFIG['dbuser'] = getenv('POSTGRES_USER');
$AUTOCONFIG['dbpass'] = getenv('POSTGRES_PASSWORD');
$AUTOCONFIG['dbhost'] = getenv('POSTGRES_HOST');
$autoconfig_enabled = true;
}

I think it's confusing to say that _FILE suffix is supported but not mentioning that if you want to use ONE variable from file (ie POSTGRES_PASSWORD), you have to use ALL variables from file...

Rather than documenting this behavior, I think it'd be more interesting to fix it and allow each variable independently such as

# pseudo-code

check_env($variable) {
 return getenv($variable) || getenv($variable + "_FILE")
}

...
} elseif (check_env('POSTGRES_DB_FILE') && check_env('POSTGRES_USER_FILE') && check_env('POSTGRES_PASSWORD_FILE') && check_env('POSTGRES_HOST')) {
...
@joshtrichards
Copy link
Member

It is documented, in the very last paragraph in the Docker Secrets section after the example compose file. It's easy to miss though.

As for mixing, I see a PR (#1996) someone proposed for that. It needs testing, review, and feedback. Feel free to jump in on it and share your thoughts if you're interested in it getting farther along.

@nsrosenqvist
Copy link

I struggled with this for about 1h as well. Functionally the linked PR looks alright, but code wise it is much harder to read. Would be great if a maintainer could give their feedback on the PR.

@coaxial
Copy link

coaxial commented Nov 23, 2023

It is documented, in the very last paragraph in the Docker Secrets section after the example compose file.

It could be made clearer. I've also spent quite a bit of time wondering why it's using sqlite. From reading the docs over and over, and the bit after the example docker-compose file, it's not clear to me that all the POSTGRES_ variables must use the _FILE suffix rather than mix and match. I expected the user and password to use secrets but the dbname not to.

@joshtrichards
Copy link
Member

@nsrosenqvist You're welcome to provide feedback on that PR as well. Why don't you do a review of it and/or at least post your comment there? After all... this is a community project. :-)

@coaxial 👍 Feel free to suggest some edits that you think might make it clearer - ideally in a PR.

@coaxial
Copy link

coaxial commented Nov 23, 2023

I can't find any reference to a PR on that topic in this issue. @joshtrichards could you share a link to the PR you're referring to?

@Adios
Copy link

Adios commented Jan 15, 2024

Additionally, since the autoconfig process is run by www-data, so I had to make sure /run/secrets/xxxxxx are readable by www-data. Otherwise the initial screen would simply indicate there is an error.

I came into the mixed _FILE variables first, and then this permission thing. It is tool dependant though. In my case, the secret files are owned by root, and podman-compose implements file-based secret by bind mount directly.

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

No branches or pull requests

5 participants