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

Environment variable substitution and other entrypoint scripts #431

Closed
rgov opened this issue Jun 24, 2020 · 9 comments
Closed

Environment variable substitution and other entrypoint scripts #431

rgov opened this issue Jun 24, 2020 · 9 comments

Comments

@rgov
Copy link

rgov commented Jun 24, 2020

I have an entry point script that derives a path and then uses sed to write it into my nginx config.

I recently moved this over to a script in /docker-entrypoint.d. Then I noticed that there is the envsubst script, which maybe I could re-use instead of sed.

However, there is no way for my earlier entry point script to set an environment variable that gets picked up by the envsubst script.

Any thoughts on how this could be accomplished? For instance, I could append to a .env file that gets source'd. I'm also willing to concede it might be overly complicated to try to make such a thing work.

@tianon
Copy link
Contributor

tianon commented Jun 24, 2020

Why not just export in your script before you invoke the image entrypoint?

#!/bin/sh
export FOO_BAR='baz buzz'
exec /docker-entrypoint.sh "$@"

@rgov
Copy link
Author

rgov commented Jun 24, 2020

Yes I could do this. Thanks for the suggestion. It does push my script outside of the .d directory but it's not a solution I would completely reject.

@MShekow
Copy link

MShekow commented Apr 6, 2022

I would like to suggest a different alternative where https://github.com/nginxinc/docker-nginx/blob/master/entrypoint/docker-entrypoint.sh#L22 sources the scripts, instead of just executing them. See nginxinc/docker-nginx-unprivileged#86 for details. Would this be something you would consider?

@thresheek
Copy link
Collaborator

See #687 for another PR willing to do similar things.

I'm reluctant to changing the docker-entrypoint.sh script to source the scripts since it might break someone's workflow.

Adding an option to source a variables.env or a similarly named file from a .d does look like a good enough option to provide that functionality and make everyone a bit happier.

What do you think?

@JuniorJPDJ
Copy link
Contributor

JuniorJPDJ commented Jul 20, 2022

My proposal is adding additional case for eg. *.envsh files in this switch:

case "$f" in
*.sh)
if [ -x "$f" ]; then
echo >&3 "$0: Launching $f";
"$f"
else
# warn on shell scripts without exec bit
echo >&3 "$0: Ignoring $f, not executable";
fi
;;
*) echo >&3 "$0: Ignoring $f";;
esac

Those files would be sourced instead of being called directly. This would preserve backwards compatibility and add this feature we want and make some things possible without override of docker container entrypoint.

What do you think guys? This will solve my problem and it looks like it will solve @rgov's problems too.

@thresheek
Copy link
Collaborator

thresheek commented Jul 20, 2022

FWIW, my variant of patch is something like:

diff --git a/entrypoint/docker-entrypoint.sh b/entrypoint/docker-entrypoint.sh
index 72d5cd9..07f7c93 100755
--- a/entrypoint/docker-entrypoint.sh
+++ b/entrypoint/docker-entrypoint.sh
@@ -13,6 +13,11 @@ if [ "$1" = "nginx" -o "$1" = "nginx-debug" ]; then
     if /usr/bin/find "/docker-entrypoint.d/" -mindepth 1 -maxdepth 1 -type f -print -quit 2>/dev/null | read v; then
         echo >&3 "$0: /docker-entrypoint.d/ is not empty, will attempt to perform configuration"

+        echo >&3 "$0: Looking for an .env var file in /docker-entrypoint.d/"
+        if [ -r "/docker-entrypoint.d/.env" ]; then
+            echo "$0: Sourcing /docker-entrypoint.d/.env"
+            source /docker-entrypoint.d/.env;
+        fi
         echo >&3 "$0: Looking for shell scripts in /docker-entrypoint.d/"
         find "/docker-entrypoint.d/" -follow -type f -print | sort -V | while read -r f; do
             case "$f" in

@JuniorJPDJ
Copy link
Contributor

JuniorJPDJ commented Jul 20, 2022

This one is IMO worse as it doesn't allow defining more than one hook script, naming it in user-readable form and doesn't allow changing order of executing.

Allowing more than one script is crucial in situations like when one script is provided by image FROMing from nginx and you want to add another layer above it modifying the behaviour.

@JuniorJPDJ
Copy link
Contributor

I updated #687 to show my solution ;D

@thresheek
Copy link
Collaborator

Fixed in 6675128

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

No branches or pull requests

5 participants