-
Notifications
You must be signed in to change notification settings - Fork 663
Bail early on initdb logic if we detect that it isn't needed #183
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
Bail early on initdb logic if we detect that it isn't needed #183
Conversation
|
This probably conflicts with #182, but I'll happily rebase either PR if so. |
3.0/docker-entrypoint.sh
Outdated
| shouldPerformInitdb= | ||
| if [ -z "$shouldPerformInitdb" ]; then | ||
| # if we've got any MONGO_INITDB_xxx environment variables set, we should initdb | ||
| for var in "${!MONGO_INITDB_@}"; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be more specific about needing user and pass together and warn or even fail if they only have one? Technically MONGO_INITDB_DATABASE doesn't matter unless they have scripts since a database is not created unless something is inserted into a collection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
mongo/3.5/docker-entrypoint.sh
Line 118 in 8b9ff68
| if [ "$MONGO_INITDB_ROOT_USERNAME" ] && [ "$MONGO_INITDB_ROOT_PASSWORD" ]; then |
mongo/3.5/docker-entrypoint.sh
Line 196 in 8b9ff68
| if [ "$MONGO_INITDB_ROOT_USERNAME" ] && [ "$MONGO_INITDB_ROOT_PASSWORD" ]; then |
initdb output).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So perhaps the loop could be simplified to just be part of the if above it:
file_env 'MONGO_INITDB_ROOT_USERNAME'
file_env 'MONGO_INITDB_ROOT_PASSWORD'
+ # pre-check a few factors to see if it's even worth bothering with initdb
+ shouldPerformInitdb=
if [ "$MONGO_INITDB_ROOT_USERNAME" ] && [ "$MONGO_INITDB_ROOT_PASSWORD" ]; then
# if we have a username/password, let's set "--auth"
_mongod_hack_ensure_arg '--auth' "$@"
set -- "${mongodHackedArgs[@]}"
+ shouldPerformInitdb="true" # 1/yes/true/something that works for -n and not -z
+ elif [ "$MONGO_INITDB_ROOT_USERNAME" ] || [ "$MONGO_INITDB_ROOT_PASSWORD" ]; then
+ have="${MONGO_INITDB_ROOT_USERNAME:+MONGO_INITDB_ROOT_USERNAME}${MONGO_INITDB_ROOT_PASSWORD:+MONGO_INITDB_ROOT_PASSWORD}"
+ haveNot="${MONGO_INITDB_ROOT_USERNAME:+MONGO_INITDB_ROOT_PASSWORD}${MONGO_INITDB_ROOT_PASSWORD:+MONGO_INITDB_ROOT_USERNAME}"
+ echo >&2 "'$have' specified but '$haveNot' is missing"
+ echo >&2 "both must be specified for a user to be created"
+ exit 1
fiThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, feel free to commit your changes to the branch. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Although IMO that have and haveNot is pretty complicated -- even as a seasoned shell-er, this is a bit hard to parse, so I'd rather either have another if or just make the error message generic and make the user figure out which one they're missing)
be strict about supplying one and not the other
|
New commit added! $ docker run -it --rm -e MONGO_INITDB_ROOT_USERNAME=admin 3925e6d4ebf3
error: missing 'MONGO_INITDB_ROOT_USERNAME' or 'MONGO_INITDB_ROOT_PASSWORD'
both must be specified for a user to be created.
$ echo $?
1 |
|
LGTM |
No description provided.