-
Notifications
You must be signed in to change notification settings - Fork 94
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
Forward ENV to the sub-container #23
base: main
Are you sure you want to change the base?
Conversation
entrypoint.sh
Outdated
@@ -1,5 +1,7 @@ | |||
#!/usr/bin/env bash | |||
|
|||
env | grep -v '^#' | xargs > docker-run-action.env |
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.
This will not work as you may expect.
Your "fix multi line ENV" will here not work.
Please have a look to Dexus-Forks@dc7d5ec
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.
A other thing is what I forgot about to tell:
- DOCKER_VERSION -> both container the first one has this ENV and the second the one in the SMOKE test, has the same, but via this ENV forward you overwrite the second container ENV to be generated and shadow it with the ENV that is forwarded.
- PATH you copy the PATH form container 1 to container 2 >>> very bad and brings problems to find shell and other stuff.
- what do we need to forward?
- I think it would enough to forward all ENV with PREFIX and 2 more (that are exposed also to the second container)
- CI (this is the only exception form PREFIX -> this is "CI=TRUE" if you check for it.)
- HOME (this is exposed to docker 1 which is starting docker-container and also exposed there)
- INPUT_*
- GITHUB_*
- ACTIONS_*
- RUNNER_*
- I think it would enough to forward all ENV with PREFIX and 2 more (that are exposed also to the second container)
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.
We've been using env | egrep -v "^(#|;| |INPUT_*|PATH|HOME)" | awk '$1 ~ /^\w+=/' | xargs -0 > docker-run-action.env
in various workflows without any issues so far :)
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.
Why do you not forward the INPUT_*
ENVs?
This will result in a problem if you run via workflow_dispatcher
your input will not reach the container.
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 took a look at the contents of docker-run-action.env and it appears the current revision also forwarded HOSTNAME
, SHLVL
and DOCKER_VERSION
(among other DOCKER_*
variables 🤔
given the list above would it perhaps better to do env | egrep -v "^(#|;| |PATH|SHLVL|HOSTNAME|DOCKER_*)" | awk '$1 ~ /^\w+=/' | xargs -0 > docker-run-action.env
?
If we were to run any container in our Github workflow using the common approach of
uses: docker://registry.com/container:tag
- the said container would have all of the Github and user-defined environment variables.Since this action spins up a new container within a container, we lose the Github and any user-defined environment variables in the resulting container, which is unexpected behavior.
Changes: