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

Include unset variables when parsing env_file #2259

Closed
wants to merge 1 commit into from

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Jan 16, 2020

- What I did
Made --env_file consistent with --env regarding unset variables

Close #2254

Also see #1019

- How I did it
unset variables (VAR without a =) which can't be resolved to a value by emptyFn are not excluded from result, so that they can override container environment the same way --env do.

This is consistent with https://docs.docker.com/engine/reference/commandline/run/#set-environment-variables--e---env---env-file

if no = is provided and that variable is not exported in your local environment, the variable won’t be set in the container.

- How to verify it
Dockerfile:

FROM alpine
ENV FOO=BAR

env_file:

FOO
  • run docker run and confirm FOO is set by image in container environment
  • run docker run --env FOO and confirm FOO is not set in container environment
  • run docker run --env_file foo.env and confirm FOO is not set in container environment

- Description for the changelog
Fix environment file parsing for empty variables.

- A picture of a cute animal (not mandatory but encouraged)
image

Those can be used to override (remove) variable from container
just like --env do

Signed-off-by: Nicolas De Loof <[email protected]>
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

thaJeztah commented Jan 16, 2020

Looks like the behavior changed between docker cli v1.4.x and v1.5.x

docker build -t foo -<<EOF
FROM busybox
ENV ENV_VAR_IN_IMAGE=image_value
EOF

Docker 1.4 sets the variable to "" when using -e / --env and no value is set and the variable is not found in the current environment.

docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock --entrypoint=/bin/sh docker:1.4
/ # docker --version
Docker version 1.4.1, build 5bc2ff8

/ # docker run --rm foo sh -c 'env | grep ENV_VAR_IN_IMAGE || echo variable not set'
ENV_VAR_IN_IMAGE=image_value

/ # docker run --rm -e ENV_VAR_IN_IMAGE foo sh -c 'env | grep ENV_VAR_IN_IMAGE || echo variable not set'
ENV_VAR_IN_IMAGE=

/ # docker run --rm -e ENV_VAR_IN_IMAGE= foo sh -c 'env | grep ENV_VAR_IN_IMAGE || echo variable not set'
ENV_VAR_IN_IMAGE=

/ # ENV_VAR_IN_IMAGE=override docker run --rm -e ENV_VAR_IN_IMAGE foo sh -c 'env | grep ENV_VAR_IN_IMAGE || echo variable not set'
ENV_VAR_IN_IMAGE=override

/ # docker run --rm -e NEW_ENV_VAR foo sh -c 'env | grep NEW_ENV_VAR || echo variable not set'
NEW_ENV_VAR=

/ # docker run --rm -e NEW_ENV_VAR= foo sh -c 'env | grep NEW_ENV_VAR || echo variable not set'
NEW_ENV_VAR=

/ # NEW_ENV_VAR=from_environment docker run --rm -e NEW_ENV_VAR foo sh -c 'env | grep NEW_ENV_VAR || echo variable not set'
NEW_ENV_VAR=from_environment

Whereas docker 1.5 and up unset the env-var when using -e / --env and no value is set and the variable is not found in the current environment.

docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock docker:1.5 sh
/ # docker --version
Docker version 1.5.0, build a8a31ef

/ # docker run --rm foo sh -c 'env | grep ENV_VAR_IN_IMAGE || echo variable not set'
ENV_VAR_IN_IMAGE=image_value

/ # docker run --rm -e ENV_VAR_IN_IMAGE foo sh -c 'env | grep ENV_VAR_IN_IMAGE || echo variable not set'
variable not set

/ # docker run --rm -e ENV_VAR_IN_IMAGE= foo sh -c 'env | grep ENV_VAR_IN_IMAGE || echo variable not set'
ENV_VAR_IN_IMAGE=

/ # ENV_VAR_IN_IMAGE=override docker run --rm -e ENV_VAR_IN_IMAGE foo sh -c 'env | grep ENV_VAR_IN_IMAGE || echo variable not set'
ENV_VAR_IN_IMAGE=override

/ # docker run --rm -e NEW_ENV_VAR foo sh -c 'env | grep NEW_ENV_VAR || echo variable not set'
variable not set

/ # docker run --rm -e NEW_ENV_VAR= foo sh -c 'env | grep NEW_ENV_VAR || echo variable not set'
NEW_ENV_VAR=

/ # NEW_ENV_VAR=from_environment docker run --rm -e NEW_ENV_VAR foo sh -c 'env | grep NEW_ENV_VAR || echo variable not set'
NEW_ENV_VAR=from_environment

@thaJeztah
Copy link
Member

While the v1.4.x doesn't look correct (it should not set it to an empty string), I'm not sure if the v1.5.x and up behavior is correct, or if it should ignore the -e, so:

image env -e result
not set not set not set not set
not set not set -e SOME_ENV not set
not set not set -e SOME_ENV= SOME_ENV=
not set not set -e SOME_ENV=flag SOME_ENV=flag
not set SOME_ENV=env not set not set
not set SOME_ENV=env -e SOME_ENV SOME_ENV=env
not set SOME_ENV=env -e SOME_ENV= SOME_ENV=
not set SOME_ENV=env -e SOME_ENV=flag SOME_ENV=flag
SOME_ENV=image not set not set SOME_ENV=image
SOME_ENV=image not set -e SOME_ENV SOME_ENV=image
SOME_ENV=image not set -e SOME_ENV= SOME_ENV=
SOME_ENV=image not set -e SOME_ENV=flag SOME_ENV=flag
SOME_ENV=image SOME_ENV=env not set SOME_ENV=image
SOME_ENV=image SOME_ENV=env -e SOME_ENV SOME_ENV=env
SOME_ENV=image SOME_ENV=env -e SOME_ENV= SOME_ENV=
SOME_ENV=image SOME_ENV=env -e SOME_ENV=flag SOME_ENV=flag

The behavior above is (I think) consistent with --build-arg, and allows scenarios to optionally override env-vars in the container depending on what env-var is present in the environment, while unconditionally setting the -e option;

# by default, the image runs with `LOG_LEVEL=error`
docker run -d -e LOG_LEVEL some_image

# but this can be overridden from the current environent
export LOG_LEVEL=debug
docker run -d -e LOG_LEVEL some_image

@thaJeztah thaJeztah closed this Apr 14, 2022
@thaJeztah thaJeztah reopened this Apr 14, 2022
@ndeloof ndeloof closed this May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent handling of empty env vars
5 participants