Dockerfile: required ARG not validated in global space #4847

thaJeztah · 2024-04-16T08:14:34Z

After discussing with @tonistiigi on Slack, I'm opening this as a separate ticket; this was originally posted as a solution for / part of the ticket below;

Dockerfile: Mark ARG as required #3208

only thing that looks incorrect seems handling global arg errors that seems to be caused by skipped error handling 4963ed7#diff-905097ba6a4e36b3afbb1204e52%5B%E2%80%A6%5D0915f2a845c768bc84c6af5db1e1fR88 . looks like intentional as global args could be unused, but obviously I don’t remember the details anymore. without knowing what case this is protecting against, atm I would say that erroring out there is more correct.

Starting with this Dockerfile;

# syntax=docker/dockerfile:1

# REQUIRED_ARG1 is required global arg. It is not used in any stage below,
# but I expect it to produce an error if not set.
ARG REQUIRED_ARG1=${REQUIRED_ARG1:?}

# REQUIRED_ARG2 is required global arg. It is overridden in stage two, and
# I expect it to produce an error if not set.
ARG REQUIRED_ARG2=${REQUIRED_ARG2:?}

# REQUIRED_ARG2 is required global arg. It is used in stage three, and I expect
# it to produce an error if not set.
ARG REQUIRED_ARG3=${REQUIRED_ARG3:?}

# REQUIRED_ARG4 is required global arg. It is used in stage four, and I expect
# it to produce an error if not set.
ARG REQUIRED_ARG4=${REQUIRED_ARG4:?}


FROM ubuntu:16.04 AS one
RUN printenv | grep REQUIRED || true

FROM ubuntu:18.04 AS two
RUN printenv | grep REQUIRED || true
ARG REQUIRED_ARG2=${REQUIRED_ARG2:?}
RUN printenv | grep REQUIRED || true

FROM ubuntu:20.04 AS three
RUN printenv | grep REQUIRED || true
ARG REQUIRED_ARG3
RUN printenv | grep REQUIRED || true

FROM ubuntu$REQUIRED_ARG4 AS four
RUN printenv | grep REQUIRED || true

FROM ubuntu${REQUIRED_ARG5:?} AS five
RUN printenv | grep REQUIRED || true

Stage one (attempt one)

Trying to build stage one, produces an error because stage five is also evaluated (although it's not immediately clear why; possibly it constructs a list of all sources (AS five could be used as part of another stage), but for that it may not have to evaluate the FROM part);

docker build --no-cache --progress=plain --target=one .
#0 building with "desktop-linux" instance using docker driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 1.17kB done
#1 DONE 0.0s

#2 resolve image config for docker-image://docker.io/docker/dockerfile:1
#2 DONE 0.5s

#3 docker-image://docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56
#3 resolve docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56 done
#3 CACHED
Dockerfile:36
--------------------
  34 |     RUN printenv | grep REQUIRED || true
  35 |
  36 | >>> FROM ubuntu${REQUIRED_ARG5:?} AS five
  37 |     RUN printenv | grep REQUIRED || true
  38 |
--------------------
ERROR: failed to solve: failed to process "ubuntu${REQUIRED_ARG5:?}": REQUIRED_ARG5: is not allowed to be unset

⚠️ 👉 Commenting out stage five for the rest of the attempts;

Stage one

Building stage one does not produce an error; all of the global args are ignored.

docker build --no-cache --progress=plain --target=one .
#0 building with "desktop-linux" instance using docker driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 1.17kB done
#1 DONE 0.0s

#2 resolve image config for docker-image://docker.io/docker/dockerfile:1
#2 DONE 0.5s

#3 docker-image://docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56
#3 resolve docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56 done
#3 CACHED

#4 [internal] load metadata for docker.io/library/ubuntu:16.04
#4 DONE 1.0s

#5 [internal] load .dockerignore
#5 transferring context: 2B done
#5 DONE 0.0s

#6 [one 1/2] FROM docker.io/library/ubuntu:16.04@sha256:1f1a2d56de1d604801a9671f301190704c25d604a416f59e03c04f5c6ffee0d6
#6 resolve docker.io/library/ubuntu:16.04@sha256:1f1a2d56de1d604801a9671f301190704c25d604a416f59e03c04f5c6ffee0d6 done
#6 sha256:6225a0253717abdc2ee23ea211c1c439c93b84231ec0a4f1c74762a205ba7234 173B / 173B 0.1s done
#6 sha256:000560be91651dcbf476ebacb8bf1f1339694a3327f8e6da2519e0b29b33eb5d 479B / 479B 0.3s done
#6 sha256:66927c6d1d3d2e9321c4893f7f2105b7cd23dfb082853d97ec08f188e271e612 0B / 854B 0.2s
#6 sha256:828b35a09f0b2f3d1dead43aa2468ff5eba6c463423b3fff7ee6d150f6fd1b6b 0B / 41.24MB 0.2s
#6 sha256:66927c6d1d3d2e9321c4893f7f2105b7cd23dfb082853d97ec08f188e271e612 854B / 854B 0.5s done
#6 sha256:828b35a09f0b2f3d1dead43aa2468ff5eba6c463423b3fff7ee6d150f6fd1b6b 9.44MB / 41.24MB 0.6s
#6 sha256:828b35a09f0b2f3d1dead43aa2468ff5eba6c463423b3fff7ee6d150f6fd1b6b 27.26MB / 41.24MB 0.8s
#6 sha256:828b35a09f0b2f3d1dead43aa2468ff5eba6c463423b3fff7ee6d150f6fd1b6b 37.75MB / 41.24MB 0.9s
#6 sha256:828b35a09f0b2f3d1dead43aa2468ff5eba6c463423b3fff7ee6d150f6fd1b6b 41.24MB / 41.24MB 1.1s done
#6 extracting sha256:828b35a09f0b2f3d1dead43aa2468ff5eba6c463423b3fff7ee6d150f6fd1b6b
#6 extracting sha256:828b35a09f0b2f3d1dead43aa2468ff5eba6c463423b3fff7ee6d150f6fd1b6b 0.5s done
#6 DONE 1.6s

#6 [one 1/2] FROM docker.io/library/ubuntu:16.04@sha256:1f1a2d56de1d604801a9671f301190704c25d604a416f59e03c04f5c6ffee0d6
#6 extracting sha256:66927c6d1d3d2e9321c4893f7f2105b7cd23dfb082853d97ec08f188e271e612 done
#6 extracting sha256:000560be91651dcbf476ebacb8bf1f1339694a3327f8e6da2519e0b29b33eb5d done
#6 extracting sha256:6225a0253717abdc2ee23ea211c1c439c93b84231ec0a4f1c74762a205ba7234 done
#6 DONE 1.6s

#7 [one 2/2] RUN printenv | grep REQUIRED || true
#7 DONE 0.3s

#8 exporting to image
#8 exporting layers 0.0s done
#8 exporting manifest sha256:0fa10fb29df39140ae41472849121f1d2f8dc7669a5f2503ba9d4ed9d3f7fce4 done
#8 exporting config sha256:4249bc72d5754db3f4eb14c0027ff2a7cca07930f4039d8b5924a4a9913a5d0e done
#8 exporting attestation manifest sha256:947f99022d490fa6941e697be0bcb0427bc9df0b28ef7d4c40048af4697ed721 done
#8 exporting manifest list sha256:16748c0de02bfed86467923b562c4b90ae488b5a9a56251ccacc0418e3c781b2 done
#8 naming to moby-dangling@sha256:16748c0de02bfed86467923b562c4b90ae488b5a9a56251ccacc0418e3c781b2 done
#8 unpacking to moby-dangling@sha256:16748c0de02bfed86467923b562c4b90ae488b5a9a56251ccacc0418e3c781b2 done
#8 DONE 0.1s

Building stage two

Building stage two produces an error at the location of the override (which also marks the arg as required); this could be considered the expected behavior; it looks to optimize, and not pull the ubuntu:18.04 image (which is good);

docker build --no-cache --progress=plain --target=two .
#0 building with "desktop-linux" instance using docker driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 1.17kB done
#1 DONE 0.0s

#2 resolve image config for docker-image://docker.io/docker/dockerfile:1
#2 ...

#3 [auth] docker/dockerfile:pull token for registry-1.docker.io
#3 DONE 0.0s

#2 resolve image config for docker-image://docker.io/docker/dockerfile:1
#2 DONE 1.0s

#4 docker-image://docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56
#4 resolve docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56 done
#4 CACHED

#5 [auth] library/ubuntu:pull token for registry-1.docker.io
#5 DONE 0.0s

#6 [internal] load metadata for docker.io/library/ubuntu:18.04
#6 DONE 1.3s
Dockerfile:25
--------------------
  23 |     FROM ubuntu:18.04 AS two
  24 |     RUN printenv | grep REQUIRED || true
  25 | >>> ARG REQUIRED_ARG2=${REQUIRED_ARG2:?}
  26 |     RUN printenv | grep REQUIRED || true
  27 |
--------------------
ERROR: failed to solve: failed to process "${REQUIRED_ARG2:?}": REQUIRED_ARG2: is not allowed to be unset

Building stage three

Building stage three completes successfully; this looks unexpected, as the stage explicitly defines REQUIRED_ARG3. Also note that the argument is set as an environment variable, but as an empty value (REQUIRED_ARG3=). Given that the arg is defined without = in the stage, it should probably not be set (but maybe it's set because of the global arg, and validation is skipped there).

If all validation worked, I also would expect it to optimize execution, and to not pull the image.

docker build --no-cache --progress=plain --target=three .
#0 building with "desktop-linux" instance using docker driver

#1 [internal] load build definition from Dockerfile
#1 DONE 0.0s

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 1.17kB done
#1 DONE 0.0s

#2 resolve image config for docker-image://docker.io/docker/dockerfile:1
#2 DONE 0.5s

#3 docker-image://docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56
#3 resolve docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56 0.0s done
#3 CACHED

#4 [internal] load metadata for docker.io/library/ubuntu:20.04
#4 DONE 0.9s

#5 [internal] load .dockerignore
#5 transferring context: 2B done
#5 DONE 0.0s

#6 [three 1/3] FROM docker.io/library/ubuntu:20.04@sha256:71b82b8e734f5cd0b3533a16f40ca1271f28d87343972bb4cd6bd6c38f1bd38e
#6 resolve docker.io/library/ubuntu:20.04@sha256:71b82b8e734f5cd0b3533a16f40ca1271f28d87343972bb4cd6bd6c38f1bd38e done
#6 sha256:fb67143098b346a05ff35775af9ba34ccf9a89e4079f4ceb9a51b05ae257e78c 0B / 25.97MB 0.2s
#6 sha256:fb67143098b346a05ff35775af9ba34ccf9a89e4079f4ceb9a51b05ae257e78c 16.78MB / 25.97MB 0.3s
#6 sha256:fb67143098b346a05ff35775af9ba34ccf9a89e4079f4ceb9a51b05ae257e78c 25.97MB / 25.97MB 0.4s done
#6 extracting sha256:fb67143098b346a05ff35775af9ba34ccf9a89e4079f4ceb9a51b05ae257e78c
#6 extracting sha256:fb67143098b346a05ff35775af9ba34ccf9a89e4079f4ceb9a51b05ae257e78c 0.3s done
#6 DONE 0.7s

#7 [three 2/3] RUN printenv | grep REQUIRED || true
#7 DONE 0.2s

#8 [three 3/3] RUN printenv | grep REQUIRED || true
#8 0.124 REQUIRED_ARG3=
#8 DONE 0.1s

#9 exporting to image
#9 exporting layers 0.0s done
#9 exporting manifest sha256:daa90315573e6562e62d082c860e50dd8c11aa3fedf1a6ba48fd16f066daeb27 done
#9 exporting config sha256:67160cdc23b990a2ecd580ab0ba068b52aeb6806bbd1f837194448745ec255e5 done
#9 exporting attestation manifest sha256:dc97c994283f1490ecbbc4b73d5cd3f1b8e7d0ce6f0c238ce3f74ea710787abe done
#9 exporting manifest list sha256:854b59ff0fedfe668ed70e2baae2998d931b8d092f4a0d6a466f72fb2910f4aa done
#9 naming to moby-dangling@sha256:854b59ff0fedfe668ed70e2baae2998d931b8d092f4a0d6a466f72fb2910f4aa done
#9 unpacking to moby-dangling@sha256:854b59ff0fedfe668ed70e2baae2998d931b8d092f4a0d6a466f72fb2910f4aa done
#9 DONE 0.1s

Building stage four

Stage four also builds successfully, but because it evaluates REQUIRED_ARG4 to an empty string, it does not produce an error for the build-arg not being set;

docker build --no-cache --progress=plain --target=four .
#0 building with "desktop-linux" instance using docker driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 1.17kB done
#1 DONE 0.0s

#2 resolve image config for docker-image://docker.io/docker/dockerfile:1
#2 DONE 0.9s

#3 docker-image://docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56
#3 resolve docker.io/docker/dockerfile:1@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56 done
#3 CACHED

#4 [internal] load metadata for docker.io/library/ubuntu:latest
#4 DONE 0.9s

#5 [internal] load .dockerignore
#5 transferring context: 2B done
#5 DONE 0.0s

#6 [four 1/2] FROM docker.io/library/ubuntu:latest@sha256:1b8d8ff4777f36f19bfe73ee4df61e3a0b789caeff29caa019539ec7c9a57f95
#6 resolve docker.io/library/ubuntu:latest@sha256:1b8d8ff4777f36f19bfe73ee4df61e3a0b789caeff29caa019539ec7c9a57f95 done
#6 sha256:70104cd59e2a443b9d9a13a6bce3bbf1ae78261c4198a40bf69d6e0515abe06a 0B / 27.36MB 0.2s
#6 sha256:70104cd59e2a443b9d9a13a6bce3bbf1ae78261c4198a40bf69d6e0515abe06a 15.73MB / 27.36MB 0.3s
#6 sha256:70104cd59e2a443b9d9a13a6bce3bbf1ae78261c4198a40bf69d6e0515abe06a 27.36MB / 27.36MB 0.4s done
#6 DONE 0.4s

#6 [four 1/2] FROM docker.io/library/ubuntu:latest@sha256:1b8d8ff4777f36f19bfe73ee4df61e3a0b789caeff29caa019539ec7c9a57f95
#6 extracting sha256:70104cd59e2a443b9d9a13a6bce3bbf1ae78261c4198a40bf69d6e0515abe06a
#6 extracting sha256:70104cd59e2a443b9d9a13a6bce3bbf1ae78261c4198a40bf69d6e0515abe06a 0.4s done
#6 DONE 0.8s

#7 [four 2/2] RUN printenv | grep REQUIRED || true
#7 DONE 0.2s

#8 exporting to image
#8 exporting layers 0.0s done
#8 exporting manifest sha256:01aaf2f9cda017a07939a3699b4a0889d9442af478ff4c285238ed0d488cb5d4 done
#8 exporting config sha256:4fba09546a6e91a665d091506a8b09f5e2b64af479d55cfb9f03631c3b834048 done
#8 exporting attestation manifest sha256:b29ab894a7a4f78d9c97ae10ec7a728543008e9c90579255d54a2033fb0e457c done
#8 exporting manifest list sha256:fec8edf55e9a9eb1344573dbe9c1b031fcc9efa7c79c1ffbe5a748cebf95a5be done
#8 naming to moby-dangling@sha256:fec8edf55e9a9eb1344573dbe9c1b031fcc9efa7c79c1ffbe5a748cebf95a5be done
#8 unpacking to moby-dangling@sha256:fec8edf55e9a9eb1344573dbe9c1b031fcc9efa7c79c1ffbe5a748cebf95a5be done
#8 DONE 0.1s

Reproduced on docker desktop with docker engine 26.0.0 with the containerd image-store enabled;

Client:
 Cloud integration: v1.0.35+desktop.13
 Version:           26.0.0
 API version:       1.45
 Go version:        go1.21.8
 Git commit:        2ae903e
 Built:             Wed Mar 20 15:14:46 2024
 OS/Arch:           darwin/arm64
 Context:           desktop-linux

Server: Docker Desktop 4.30.0 (145546)
 Engine:
  Version:          26.0.0
  API version:      1.45 (minimum version 1.24)
  Go version:       go1.21.8
  Git commit:       8b79278
  Built:            Wed Mar 20 15:18:02 2024
  OS/Arch:          linux/arm64
  Experimental:     false
 containerd:
  Version:          1.6.28
  GitCommit:        ae07eda36dd25f8a1b98dfbf587313b99c0190bb
 runc:
  Version:          1.1.12
  GitCommit:        v1.1.12-0-g51d5e94
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

The text was updated successfully, but these errors were encountered:

tonistiigi · 2024-04-16T16:37:49Z

only thing that looks incorrect seems handling global arg errors that seems to be caused by skipped error handling

I think this is still the only thing that can be considered issue. I'm not 100% what case we wanted to avoid by not handling error there, other than avoiding ARG used by other target failing the build. I'm in favor of adding the error handling back, but note that this make all the examples fail.

Stage one (attempt one)
Trying to build stage one, produces an error because stage five is also evaluated

Indeed, there is no such optimization to not evaluate base image names when determining dependencies between stages.

Stage one
Building stage three
Building stage four

These all look the same. There is no ? evaluation in this build other than the global ones described earlier.

The current behavior is:

Errors from ? in global args are unhandled. This should probably change.
ARG REQUIRED_ARG2=${REQUIRED_ARG2:?} effectively means same as _default=${REQUIRED_ARG2:?}; REQUIRED_ARG2=${REQUIRED_ARG2:-$_default} in bash.
Like in bash the error happens only on line with ?. There is no optimization if the variable set with ? is used later in the script or not.

thaJeztah · 2024-04-18T08:00:51Z

I'm in favor of adding the error handling back, but note that this make all the examples fail.

Yes, I think it's ok to make the examples fail; at least, I think the intent of using ? here is for making it fail if not specified. If that's not the case, the user should move that part to where it's used.

Some of these I added to see if evaluation was "early-binding" or "late-binding" (for better words), basically, like = vs := in Make. Maybe that would even be the "full" solution for this ambiguity, although I'm not sure if we still can change that. Currently (at least as far as global ARG goes), it's late-binding, which would match = in Make; if we change that, it means we would for sure paint shut the possibility of introducing := if we would make that behavior controllable by the user.

Indeed, there is no such optimization to not evaluate base image names when determining dependencies between stages.

Yes, this was me being curious, but I could see some use-cases for this. For example, it could be that you may want to have an argument that should be required for a specific stage (and want it to fail immediately when trying to build that stage), but don't want to make it required for other stages, e.g.;

ARG REQUIRED_ARG5

FROM alpine AS lint
RUN ...

FROM ubuntu${REQUIRED_ARG5:?} AS build
RUN ...

I can create a separate ticket for that for consideration

These all look the same. There is no ? evaluation in this build other than the global ones described earlier.

I think for stage four there would be; the FROM depends on REQUIRED_ARG4 and would have to evaluate the global REQUIRED_ARG4 arg (which has ? evaluation); again somewhat related to "late-binding" (or not)

thaJeztah added kind/bug area/dockerfile labels Apr 16, 2024

tonistiigi linked a pull request Apr 17, 2024 that will close this issue

dockerfile: avoid ignoring errors global arg expansion #4856

Open

thompson-shaun added the accepted label May 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dockerfile: required ARG not validated in global space #4847

Dockerfile: required ARG not validated in global space #4847

thaJeztah commented Apr 16, 2024

tonistiigi commented Apr 16, 2024

thaJeztah commented Apr 18, 2024

Dockerfile: required ARG not validated in global space #4847

Dockerfile: required ARG not validated in global space #4847

Comments

thaJeztah commented Apr 16, 2024

Stage one (attempt one)

Stage one

Building stage two

Building stage three

Building stage four

tonistiigi commented Apr 16, 2024

thaJeztah commented Apr 18, 2024