Buildkit: feature request: allow variables in the `RUN --mount=type=bind` values

[hakanurhan]

When I define an argument (ARG) or an environment variable (ENV) and then try to use it in the RUN --mount=type=bind,... command, I don't get the mount working.

How to reproduce:
Docker version:

$ docker version
Client:
 Version:           18.09.1
 API version:       1.39
 Go version:        go1.10.6
 Git commit:        4c52b90
 Built:             Wed Jan  9 19:35:01 2019
 OS/Arch:           linux/amd64
 Experimental:      false

Server: Docker Engine - Community
 Engine:
  Version:          18.09.1
  API version:      1.39 (minimum version 1.12)
  Go version:       go1.10.6
  Git commit:       4c52b90
  Built:            Wed Jan  9 19:06:30 2019
  OS/Arch:          linux/amd64
  Experimental:     false

I used the following Dockerfile with BUILDKIT=1 and it failed. If I only change ...target=${TEMPORARY_MOUNT_POINT}... to ...target=/tmp-mount..., it works

# syntax = docker/dockerfile:experimental
FROM ubuntu:14.04.5
ARG TEMPORARY_MOUNT_POINT=/tmp-mount
RUN --mount=type=bind,source=./,target=${TEMPORARY_MOUNT_POINT} \
        ls ${TEMPORARY_MOUNT_POINT}

I also tried using ENV instead of the ARG command, but I had the same results.

The failing and non-failing Dockerfile's are attached (I had to add .txt extension, they are ordinary Dockerfiles)
Dockerfile.fail.txt
Dockerfile.success.txt

[simon-weber]

I just ran into this. It's interesting that my --progress=plain ... output shows each command identically -- it made me think this was actually supported.

[matshch]

The same for me, wasted some time on exploring that variables just doesn't work in mount options, showing nonsense not found: not found error with all variables expanded in command name.

I think there could be two improvements:

1. Show commands that really executed. So, if variables isn't expanded by buildkit itself, they shouldn't be shown as expanded.
2. Document that variables can't be used in mount options or implement variable expansion.

[petr-motejlek]

+1

I too hate the fact that the full command line is printed out expanded, while it would appear that internally it's really not expanded, or perhaps expanded at a different stage of the build. It is really confusing to users.

I would like the expansions to be supported, but if they cannot, at least make the printed out commands show the variable names to make it easier for debug :).

[tonistiigi]

Marking the invalid expansion in output as a bug.

Technically we can do the expansion quite easily for the values of the csv, so --mount=type=$type but not so easily for the full strings, eg --mount=$mount because they are evaluated on parse time. Wondering if doing it only for the values might be confusing.

[petr-motejlek]

@tonistiigi , thank you. For my current use case, expanding only the leaves (--mount=type=$type,src=$src,dst=$dst would be enough. Not sure about others, though.

Cheers

[matshch]

I think expanding only values will be enough for almost everything, and it will be OK if this behavior will be documented.

[mowings]

Expanding values would cover almost all use cases, I think. In my case, I have multiple projects that need separate caches -- and because these builds are automated, I need to be able to pass in a couple of strings to determine which cache is used at build time.

I can work around my particular issue by creating subdirectories off of the cache target based on the passed-in build args, but this is far from ideal.

[alice-sawatzky]

while we're at it, lets make sure when we implement this that we implement it for all flag values. i just had to workaround --mount=type=ssh,uid=${UID} not working

[tonistiigi]

Can't make it work for the from option in the --mount as well. Same issue as with COPY --from. At least without a significantly complicated change and a slower builder algorithm. Global scope args could still be used in them like it is possible to use them now by defining a new single-line stage.

[Jim-Bar]

Can agree that expanding only the values would cover our use case.

What's the status of this?

[petr-motejlek]

@tonistiigi ,

Can you provide the "single-line" stage as an example? Not that I need to do that, but I wonder what you meant by it :).

[staticfloat]

I'd like to express my support for allowing variables in uid and gid, as otherwise it's very difficult to parameterize UID's in large multi-container setups that require coordinated user IDs.

[omermizr]

Hey @tonistiigi, can I take a stab at adding support for this?
I'm thinking:

• Support for expansion in the values.
• Support for expansion in "from" (for COPY as well?)
• Raise error when variable is expanded to an empty string.

WDYT?

[tonistiigi]

@omermizr Sure. For from it is quite tricky to add it, so probably a separate task. Moving Dockerfile's LLB generation over to #1426 should be a prerequisite for it.

I'm not sure if the empty value should be an automatic error. There a probably valid cases for that.

I'd add another task: Make sure progress output shows replaced variables correctly. Eg. var in csv key should not show up as replaced in progress. Instead of just replacing variables on the String() output of the command, it should be done per command logic so that only parts that support variables perform the replacement.