Skip to content

entrypoint tests if POSTGRES_PASSWORD is empty instead of testing if it is unset #683

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

Closed
SpoonMeiser opened this issue Feb 17, 2020 · 4 comments · Fixed by #685
Closed

Comments

@SpoonMeiser
Copy link

The behaviour was changed in commit 42ce743

The comments introduced here say:

# error if both POSTGRES_PASSWORD is unset and POSTGRES_HOST_AUTH_METHOD is not 'trust'

But the actual test against POSTGRES_PASSWORD is [ -z "$POSTGRES_PASSWORD" ] which is different from what the comment suggests.

The correct test to match the comment would be: [ ! -v POSTGRES_PASSWORD ].

The current implementation prevents the password explicitly being set to the empty string, and issues a misleading error when it is.

@SpoonMeiser SpoonMeiser changed the title Script tests if POSTGRES_PASSWORD is empty instead of testing if it is unset entrypoint tests if POSTGRES_PASSWORD is empty instead of testing if it is unset Feb 17, 2020
@Denkneb
Copy link

Denkneb commented Feb 17, 2020

I have a similar issue

@allan-simon
Copy link

same, we needed to force 12.1 , 12.2 having the issue

@tianon
Copy link
Member

tianon commented Feb 17, 2020

The intent of the word "unset" here is "has no value / empty value" -- looking through https://stackoverflow.com/a/5424012/433558 and https://dba.stackexchange.com/a/83233/201322 it doesn't seem like setting the password explicitly to an empty value is actually meaningful?

I imagine you're likely running into #681, and instead intended to set POSTGRES_HOST_AUTH_METHOD=trust, which will disable the requirement for passwords (as in the previous image), although with the caveat that you should really give https://www.postgresql.org/docs/current/auth-trust.html a read and make sure you understand the implications of doing so.

@SpoonMeiser
Copy link
Author

For us, we're using this as part of a CI job for tests, so there isn't really a security concern.

This change in behaviour broke our tests.

If the behaviour is correct, the error message needs to be fixed, because we were explicitly setting POSTGRES_PASSWORD but we're getting an error message that it wasn't set.

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 a pull request may close this issue.

4 participants