compose icon indicating copy to clipboard operation
compose copied to clipboard

Undefined default build arg is sent as empty string to docker build, instead of not sent

Open thomas-riccardi opened this issue 8 years ago • 28 comments

In issue #3281 compose < 1.8.0-rc1 incorrectly sent undefined default build args as 'None' string to docker build, it was fixed by PR #3449 (included in compose 1.8.0-rc1) by sending an empty string instead to docker build.

I think it would be better to just not send the undefined build args to docker build, so that the default value for the build arg defined in Dockerfile would apply.

Scenario:

Files

Dockerfile:

FROM ubuntu:14.04

ARG FOO=1
RUN echo "-${FOO}-"

CMD /bin/bash

docker-compose.yml:

version: '2'

services:
  test:
    build:
      context: .
      args:
        - FOO

Execution:

$ ./docker-compose-1.8.0-rc1 --verbose config
networks: {}
services:
  test:
    build:
      args:
        FOO: ''
      context: /home/riccardi/git/ses-docker/test-default-build-arg
version: '2.0'
volumes: {}

$ ./docker-compose-1.8.0-rc1 --verbose build
compose.config.config.find: Using configuration files: ./docker-compose.yml
docker.auth.auth.load_config: Found 'auths' section
docker.auth.auth.parse_auth: Found entry (registry=u'docker-registry-stag.systran.net:5000', username=u'systran')
compose.cli.command.get_client: docker-compose version 1.8.0-rc1, build 9bf6bc6
docker-py version: 1.8.1
CPython version: 2.7.9
OpenSSL version: OpenSSL 1.0.1e 11 Feb 2013
compose.cli.command.get_client: Docker base_url: http+docker://localunixsocket
compose.cli.command.get_client: Docker version: KernelVersion=4.4.0-21-generic, Os=linux, BuildTime=2016-04-26T23:30:23.291099901+00:00, ApiVersion=1.23, Version=1.11.1, GitCommit=5604cbe, Arch=amd64, GoVersion=go1.5.4
compose.service.build: Building test
compose.cli.verbose_proxy.proxy_callable: docker build <- (pull=False, stream=True, nocache=False, tag=u'testdefaultbuildarg_test', buildargs={u'FOO': ''}, rm=True, forcerm=False, path='/home/riccardi/git/ses-docker/test-default-build-arg', dockerfile=None)
docker.api.build._set_auth_headers: Looking for auth config
docker.api.build._set_auth_headers: Sending auth config (u'docker-registry-stag.systran.net:5000')
compose.cli.verbose_proxy.proxy_callable: docker build -> <generator object _stream_helper at 0x7f8a18739b40>
Step 1 : FROM ubuntu:14.04
 ---> 8f1bd21bd25c
Step 2 : ARG FOO=1
 ---> Using cache
 ---> a8c68a88ef6d
Step 3 : RUN echo "-${FOO}-"
 ---> Running in c03d7e477353
--
 ---> 17f6ac07ea06
Removing intermediate container c03d7e477353
Step 4 : CMD /bin/bash
 ---> Running in 47164716758d
 ---> 5ef78af6532b
Removing intermediate container 47164716758d
Successfully built 5ef78af6532b
compose.cli.verbose_proxy.proxy_callable: docker close <- ()
compose.cli.verbose_proxy.proxy_callable: docker close -> None

Issue

Expected result:

prints -1-.

Actual result:

prints --.

<1.8.0-rc1 result:

prints -None-.

Details

compose 1.8.0-rc1 behavior is better than previous compose versions: it could be accepted as a correct behavior. However, I believe the behavior could be improved further by expecting -1- as result, not --: if docker compose has no value for the build arg, then it should let docker build apply its own default value that comes from the Dockerfile.

Changing from -- to -1- is a breaking change, so I suggest to release it in 1.8.0 if this behavior is accepted.

Without this feature, I have to duplicate the default value both in Dockerfile and in .env read by docker-compose (or just in .env, but in this case the Dockerfile cannot be used alone, without docker-compose), and I think the best place for the default value is in the Dockerfile only.

thomas-riccardi avatar Jun 16 '16 08:06 thomas-riccardi

@aanand any thoughts ?

thomas-riccardi avatar Jun 21 '16 15:06 thomas-riccardi

Sorry about the delay - been catching up post-DockerCon.

So here's the tradeoff:

  1. If we leave the behaviour as it is in master, it's impossible to write a Compose file where the argument's value can be optionally overridden by the environment or .env file, unless you repeat the default value in the Compose file.
  2. If we change the behaviour as you suggest, it's impossible to override a build arg's default value with an empty value. i.e. if the Dockerfile contains FOO=1, I can't override that with FOO=.

It's a difficult call, but I think what sways it is the analogous behaviour of docker build --build-arg. From manual testing, I think the behaviour in master mirrors that of docker build --build-arg, and should therefore not be changed. Here's an example:

$ cat Dockerfile
FROM alpine
ARG FOO=1
RUN ["sh", "-c", "echo '>>>' FOO=$FOO '<<<'"]

$ docker build --no-cache .
Sending build context to Docker daemon 3.072 kB
Step 1 : FROM alpine
 ---> 4e38e38c8ce0
Step 2 : ARG FOO=1
 ---> Running in 71f0b88d767c
 ---> 0a5b70556553
Removing intermediate container 71f0b88d767c
Step 3 : RUN sh -c echo '>>>' FOO=$FOO '<<<'
 ---> Running in fa5ec958c69b
>>> FOO=1 <<<
 ---> c9e8ee8d0593
Removing intermediate container fa5ec958c69b
Successfully built c9e8ee8d0593

$ docker build --build-arg FOO --no-cache .
Sending build context to Docker daemon 3.072 kB
Step 1 : FROM alpine
 ---> 4e38e38c8ce0
Step 2 : ARG FOO=1
 ---> Running in 8a4c80867daa
 ---> 68a4c7db60b9
Removing intermediate container 8a4c80867daa
Step 3 : RUN sh -c echo '>>>' FOO=$FOO '<<<'
 ---> Running in fff6482af7a4
>>> FOO= <<<
 ---> d2e4fdee15fb
Removing intermediate container fff6482af7a4
Successfully built d2e4fdee15fb

As you can see, specifying an empty value for FOO does indeed override the default set in the Dockerfile. As much as possible, in cases like these we try to stick to mirroring the behaviour of the docker client. I can hear a case for the alternate behaviour being better, but if it's going to change in Compose, it should change in Engine first.

aanand avatar Jun 27 '16 23:06 aanand

If we change the behaviour as you suggest, it's impossible to override a build arg's default value with an empty value. i.e. if the Dockerfile contains FOO=1, I can't override that with FOO=.

FOO= and FOO are not the same thing, if you want to override ARG FOO=1 from the Dockerfile with empty string, you can either put args: [ FOO= ] in the docker-compose.yml, or in the .env, so I don't see any issue here.

However, your 3rd point is valid: indeed, docker engine treats --build-arg FOO as --build-arg FOO= if FOO is not defined in the docker build process environment.

I understand your point about keeping the same behavior on both engine and compose. I'll open an issue in docker engine for that.

I'd like to point out that this docker engine feature is not documented anywhere (neither in https://docs.docker.com/engine/reference/commandline/build/, nor in https://docs.docker.com/engine/reference/builder/). I suggest to not document it for compose either so it'll be easier to change the behavior without it being considered as a breaking change.

Thanks

PS: maybe this issue can be closed for now; I'm not sure what procedure to follow in this case.

thomas-riccardi avatar Jun 28 '16 16:06 thomas-riccardi

We can leave this open for the purposes of discussion and to track the Engine issue - please link to it from here once you've created it. Thanks!

aanand avatar Jun 28 '16 19:06 aanand

Similarly ran into this problem and agree with the proposal by @triccardi-systran. I'd also like to mention that without using a .env file, the optional override is impossible. I often use Compose from a subdirectory, and as .env is read from the current working directory (https://docs.docker.com/compose/env-file/), it effectively prevents me from using the subdirectory feature as such (unless I have a .env file defined in every subdirectory...). The only solution in my case ends up having to use MYBUILDARG=thedefault docker-compose build ... for all build commands :/

I also think Compose should support passing the build args as arguments to docker-compose build, i.e. just like docker-compose run now supports the -e option (to mirror docker), docker-compose build could support the --build-arg option as well. For my use case at least, this would eliminate the need to even use an unset option in args, as instead of

args:
  - MYBUILDARG

I could then use docker-compose build myservice --build-arg MYBUILDARG=argggg

agilgur5 avatar Jul 29 '16 09:07 agilgur5

@agilgur5 I like your docker-compose build myservice --build-arg MYBUILDARG=argggg idea! You probably should open a new issue for that.

thomas-riccardi avatar Jul 29 '16 10:07 thomas-riccardi

I've encountered this impacting use of proxy related variables, and part of the it is that the behaviour between args and environment is not consistent.

Reading https://docs.docker.com/compose/compose-file/#/args and https://docs.docker.com/compose/compose-file/#/environment, one would expect the same behaviour from both.

Given a service defined as follows:

services:
  jenkins:
    image: jenkins:latest
    build:
      context: ./jenkins
      args:
        - HTTP_PROXY
        - HTTPS_PROXY
        - NO_PROXY
        - http_proxy
        - https_proxy
        - no_proxy
    environment:
      - HTTP_PROXY
      - HTTPS_PROXY
      - NO_PROXY
      - http_proxy
      - https_proxy
      - no_proxy
      - JAVA_OPTS=-Djenkins.install.runSetupWizard=false
      - JENKINS_OPTS=--prefix=/jenkins

If you happen to only have http_proxy & https_proxy defined but not HTTP_PROXY & HTTPS_PROXY defined what you will see during the build is that HTTP_PROXY and HTTPS_PROXY are set to '', which causes anything checking the environment for these variables to assume that their existance means no proxy is defined, rather than moving onto checking the lowercase versions as well.

This can cause all sorts of opaque issues building with docker-compose behind proxies because the behaviour for the environment versions of these is different. I've discovered that apk will not check for http_proxy if it finds HTTP_PROXY set to an empty string.

You could argue the software should check for both variations, however given the difference in behavour when dealing with environment settings, such as if you have an image built, and using the same unset/set variables listed above, and run docker-compose run -u root jenkins env you will not see HTTP_PROXY or HTTPS_PROXY appearing in the output.

This is because for environment:

    environment:
        - SOMEVAR

SOMEVAR is considered optional, and is only picked up from the environment of the user if defined.

It seems like it would be much better to have the behaviour of args and environment consistent, and since environment has been around for longer, I would suggest that following it's behaviour rather than changing environment to follow the newer behaviour of args will have less of an impact.

Perhaps it would be better if instead of having:

    build:
        args:
            - SOMEVAR

pass through a variable as being empty, require that someone provide '=', with nothing after it to indicate that it's intended to override with an empty value.

This would match the existing behaviour for environment, and avoid surprising behaviour.

Override SOMEVAR in Dockerfile only if defined in user environment:

    build:
        args:
            - SOMEVAR

OR

    build:
        args:
            SOMEVAR:

Override SOMEVAR in Dockerfile with blank value

    build:
        args:
            - SOMEVAR=

or

    build:
        args:
            SOMEVAR: ""

Override SOMEVAR in Dockerfile with different value

    build:
        args:
            - SOMEVAR=my_value

or

    build:
        args:
            SOMEVAR: my_value

This seems to give the flexibility needed to be able to override with blank values, while also retaining the ability to pass through certain env variables only if set.

electrofelix avatar Nov 23 '16 17:11 electrofelix

Still agree with all the above comments, but just wanted to note that as of v1.9 and the 2.1 file format, Compose supports bash-style inline defaults, which means optional build args without a .env file are possible now. As @triccardi-systran noted though, you have to duplicate the default value in Dockerfile to be in docker-compose.yml as well, due to this bug and no support for --build-arg (#3790), which is quite annoying and prone to mistakes.

With inline defaults it looks likes this:

build:
  args:
    - SOMEBUILDARG=$SOMEBUILDARG:-thedefault

agilgur5 avatar Nov 24 '16 23:11 agilgur5

bash-style Inline defaults also has the drawback that the default value must be duplicated on each usage of the variable.

thomas-riccardi avatar Nov 25 '16 17:11 thomas-riccardi

Indeed, that's why I wrote:

As @triccardi-systran noted though, you have to duplicate the default value in Dockerfile to be in docker-compose.yml as well, due to this bug and no support for --build-arg (#3790), which is quite annoying and prone to mistakes.

Just that with inline defaults you don't need a .env file anymore

agilgur5 avatar Nov 26 '16 20:11 agilgur5

Something like this would be amazing:

# docker-compose.yml
services:
  app:
    build:
      context: .
      args:
        - NODE_VERSION=${cat .node-version}
# Dockerfile
FROM node:${NODE_VERSION}

gitviola avatar Jan 03 '18 16:01 gitviola

My workaround example:

# docker-compose.yml
services:
  app:
    build:
      args:
        CONTAINERPILOT_VERSION: "${CONTAINERPILOT_VERSION:-}"
# Dockerfile
ARG CONTAINERPILOT_VERSION
ENV CONTAINERPILOT_VERSION=${CONTAINERPILOT_VERSION:-"3.8.0"}

with this implementation I need to define default version only in Dockerfile, and my docker-compose file is proxying ARG from optional env var

ghost avatar Jul 27 '19 21:07 ghost

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jan 24 '20 01:01 stale[bot]

Let's create activity here to keep the issue open, because I assume the issue is still there if no compose developer talked about it here, and we have recent documented workarounds...

thomas-riccardi avatar Jan 24 '20 09:01 thomas-riccardi

This issue has been automatically marked as not stale anymore due to the recent activity.

stale[bot] avatar Jan 24 '20 09:01 stale[bot]

This issue does not have a true workaround. The workarounds presented do not solve the core issue as requested by @thomas-riccardi :

  • having a default for ARGs being defined in Dockerfile only (which would be the apt place as it allows one to use Dockerfiles without compose and keep defaults behavior consistent)
  • not having to specify these defaults in multiple places in order to achieve the above

The workaround given by @idetoile does not work for ARGs. They still behave as when this issue was first open, regardless of having been able to use bash inline defaults.

Furthermore, use of bash inline defaults are documented nowhere, apart from being nonchalantly glossed over in the section on Scope for ARGs in Dockerfile reference, so this particular issue is the documentation for the matter, which also adds to the frustration.

Anyway,

Given compose file:

version: '3'

services: 
    test:
        build:
            context: .
            args:
                FOO: ${FOO:-}

and Dockerfile:

FROM alpine
ARG FOO=${FOO:-2}
RUN echo "-- $FOO --"

Issuing docker build --build-arg FOO --no-cache .

Results in the relevant lines being:

Step 3/3 : RUN echo "-- $FOO --"
 ---> Running in 48cd1eaaaf2c
-- 2 --

Whereas running docker-compose build results in:

Step 3/3 : RUN echo "-- $FOO --"
 ---> Running in 935f9a469215
--  --

Which means that the 2017 merge of PR to @thomas-riccardi issue in docker upstream solved this issue in docker itself, but compose still does something wrong when passing the argument.

bmarkovic avatar Mar 23 '20 12:03 bmarkovic

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 19 '20 13:09 stale[bot]

AFAIK the problem still exists and is why I've had to avoid docker compose for building of images due to the inconsistency

electrofelix avatar Sep 19 '20 14:09 electrofelix

This issue has been automatically marked as not stale anymore due to the recent activity.

stale[bot] avatar Sep 19 '20 14:09 stale[bot]

This might explain what I'm seeing. My docker compose file includes:

    build:
      context: .
      dockerfile: Dockerfile
      args:
        - NAME=${NAME:-Fred}

My docker file includes:

ARG NAME="${NAME}"
RUN echo "Name: $NAME"

Running:

docker-compose -f new-docker-compose.yml build myservice

Results in:

Name:

Docker compose: docker-compose-1.25.4-1.fc31.src.rpm Docker: docker-ce-19.03.13-3.fc31.src.rpm

Thanks

stodge avatar Sep 30 '20 15:09 stodge

@bmarkovic is correct. This issue still exists and there seems to be no way to use the default specified in the Dockerfile if building from docker-compose.

agconti avatar Apr 05 '21 15:04 agconti

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 09 '21 20:11 stale[bot]

One thing that might help is if a maintainer could confirm if any of the proposed behaviour changes would be acceptable if a PR was put together?

electrofelix avatar Nov 09 '21 22:11 electrofelix

This issue has been automatically marked as not stale anymore due to the recent activity.

stale[bot] avatar Nov 09 '21 22:11 stale[bot]

FIve years 😢 With this, and no support for build time secrets, docker-compose is more of a headache than it is useful!

agates4 avatar Feb 14 '22 16:02 agates4

My suggestion is to just pass build arguments defined in docker-compose.yml to specified service context, or to pass them to each specific context if general build command was used. Please also ensure that using --build flag in other commands like docker-compose create should also attach build arguments defined in specified configuration file. When you have multiple arguments it's a real headache to put them all in explicitly. I would like to define them once. It should be mentioned that using --build-arg in such case should not result in undefined behavior. My suggestion is to add arguments defined in configuration file, then apply --build-arg arguments overriding values defined in configuration file and also to add a flag like --clean-build-args to instruct compose not to load build arguments from configuration file, but use only those specified in --build-arg. Another suggestion is to move those flags a level up, so that create and other commands would be managed in the same way. Thank you.

pprudev avatar Mar 13 '22 15:03 pprudev

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 21 '22 10:09 stale[bot]

This is a real issue that needs to be resolved

agates4 avatar Sep 21 '22 10:09 agates4

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar May 21 '23 23:05 stale[bot]

This issue is still relevant

daniel-shuy avatar May 22 '23 02:05 daniel-shuy