buildkit, build: add support for `additionalBuildContext` in builds via `--build-context` #3978

flouthoc · 2022-05-10T10:26:13Z

As of now for complicated builds the ability to only access files from one location became quite limiting OTOH we already support multi-stage builds where you can copy files from other parts of the Containerfile by adding the --from flag and pointing it to the name of another Containerfile stage or a remote image but this is still limited.

The new named build context feature is an extension of this pattern. You can now define additional build contexts when running the build command, give them a name, and then access them inside a Containerfile the same way you previously did with build stages.

Additional build contexts can be defined with a new --build-context [name]=[value] flag. The key component defines the name for your build context and the value can be:

 * Local directory – e.g. --build-context project2=../path/to/project2/src
 * HTTP URL to a tarball – e.g. --build-context src=https://example.org/releases/src.tar
 * Container image – Define with a `docker-image://` prefix, e.g. --build-context alpine=docker-image://alpine:3.15, ( also supports `docker://`, `container-image://`)

On the Containerfile side, you can reference the build context on all commands that accept the from parameter. Here’s how that might look:

FROM [name]
COPY --from=[name] ...
RUN --mount=from=[name] …

The value of [name] is matched with the following priority order:

Named build context defined with --build-context [name]=..
Stage defined with AS [name] inside Dockerfile
Remote image [name] in a container registry

Added Features

Pinning images for FROM,COPY and ADD.
Specifying multiple buildcontexts from different projects
and using them with --from in ADD and COPY directive
Override a Remote Dependency with a Local One.
Using additional context from external Tar

More Context: https://www.docker.com/blog/dockerfiles-now-support-multiple-build-contexts/

openshift-ci · 2022-05-10T10:26:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [flouthoc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

flouthoc · 2022-05-10T10:30:18Z

More about this feature here: https://www.docker.com/blog/dockerfiles-now-support-multiple-build-contexts/

nalind

Does a stage named using "FROM base AS name" but referred to in a "COPY --from=" instruction that refers to it by its numeric position require additional handling?

Needs to test cases with a stage named using "FROM base AS name" syntax and a build context named "name" to check that "COPY --from" looks in the right place, and that "FROM name" does the expected thing.

Needs to test "RUN --mount from=...." pointing to the various kinds of additional build contexts.

The patch touches platformsForBaseImages(), so we should test how it affects the --all-platforms build flag.

internal/util/util.go

cmd/buildah/build.go

docs/buildah-build.1.md

define/build.go

imagebuildah/executor.go

imagebuildah/stage_executor.go

internal/util/util.go

docs/buildah-build.1.md

flouthoc · 2022-05-11T09:38:26Z

Does a stage named using "FROM base AS name" but referred to in a "COPY --from=" instruction that refers to it by its numeric position require additional handling?

Needs to test cases with a stage named using "FROM base AS name" syntax and a build context named "name" to check that "COPY --from" looks in the right place, and that "FROM name" does the expected thing.

Needs to test "RUN --mount from=...." pointing to the various kinds of additional build contexts.

The patch touches platformsForBaseImages(), so we should test how it affects the --all-platforms build flag.

@nalind Added new tests as listed below which should cover all the requested scenarios, also added comments where we match with buildx parity.

ok 1 build-with-additional-build-context and COPY, test pinning image
ok 2 build-with-additional-build-context and COPY, stagename and additional-context conflict
ok 3 build-with-additional-build-context and COPY, additionalContext and numeric value of stage
ok 4 build-with-additional-build-context and FROM, stagename and additional-context conflict
ok 5 build-with-additional-build-context and COPY, additional context from host
ok 6 build-with-additional-build-context and COPY, additional context from external URL
ok 7 build-with-additional-build-context and FROM, pin busybox to alpine
ok 8 build-with-additional-build-context and RUN --mount=from=, additional-context and also test conflict with stagename
ok 9 build-with-additional-build-context and RUN --mount=from=, additional-context not image and also test conflict with stagename
ok 10 bud-multiple-platform for --all-platform with additional-build-context

docs/buildah-build.1.md

imagebuildah/build.go

nalind · 2022-05-12T13:33:48Z

imagebuildah/stage_executor.go

-			if isStage, err := s.executor.waitForStage(s.ctx, from, s.stages[:s.index]); isStage && err != nil {
-				return err
+			if additionalBuildContext, ok := s.executor.additionalBuildContexts[from]; ok {
+				if additionalBuildContext != nil {


If we never put nil values in the map, then we don't need to check for them.

Just added a check so in future if someone edits top level code we don't endup SEGSEV. But I can drop it if needed.

Go ahead and drop it. I can't conceive of a use case for setting nil values in this map.

dropped this nil and other similar nil check for additionalBuildContext.

imagebuildah/stage_executor.go

pkg/parse/parse.go

nalind · 2022-05-12T21:38:33Z

Should have a test with "RUN --mount=type=bind,from=..." where the additional build context specified as the "from" value was given as a URL, and at least one of the "RUN --mount" tests should be using the "src" option to look in a subdirectory of the additional build context, to make sure we're not forgetting that in the logic.

flouthoc · 2022-05-13T11:03:59Z

@nalind Added a new test build-with-additional-build-context and RUN --mount=from=, additional-context is URL and mounted from subdir that should cover both.

flouthoc · 2022-05-16T08:19:20Z

@nalind Could you PTAL again.

nalind

Nits in error messages, a couple of redundant variables, and would like an answer about https://github.com/containers/buildah/pull/3978/files#r871374285.

cmd/buildah/build.go

imagebuildah/build.go

imagebuildah/executor.go

imagebuildah/stage_executor.go

tests/bud.bats

nalind · 2022-05-17T17:37:19Z

There's still a redundant --jobs=1 in the bud-multiple-platform for --all-platform with additional-build-context test, otherwise LGTM.

As builds got more complicated, the ability to only access files from one location became quite limiting. With `multi-stage` builds where you can `copy` files from other parts of the Containerfile by adding the `--from` flag and pointing it to the name of another Containerfile stage or a remote image. The new named build context feature is an extension of this pattern. You can now define additional build contexts when running the build command, give them a name, and then access them inside a Dockerfile the same way you previously did with build stages. Additional build contexts can be defined with a new `--build-context [name]=[value]` flag. The key component defines the name for your build context and the value can be: ```console Local directory – e.g. --build-context project2=../path/to/project2/src HTTP URL to a tarball – e.g. --build-context src=https://example.org/releases/src.tar Container image – Define with a docker-image:// prefix, e.g. --build-context alpine=docker-image://alpine:3.15, ( also supports docker://, container-image:// ) ``` On the Containerfile side, you can reference the build context on all commands that accept the “from” parameter. Here’s how that might look: ```Dockerfile FROM [name] COPY --from=[name] ... RUN --mount=from=[name] … ``` The value of [name] is matched with the following priority order: * Named build context defined with `--build-context [name]=..` * Stage defined with `AS [name]` inside Dockerfile * Remote image `[name]` in a container registry Added Features * Pinning images for `FROM` and `COPY` * Specifying multiple buildcontexts from different projects and using them with `--from` in `ADD` and `COPY` directive * Override a Remote Dependency with a Local One. * Using additional context from external `Tar` Signed-off-by: Aditya R <[email protected]>

flouthoc · 2022-05-17T18:14:55Z

There's still a redundant --jobs=1 in the bud-multiple-platform for --all-platform with additional-build-context test, otherwise LGTM.

Fixed.

flouthoc · 2022-05-18T05:18:44Z

@rhatdan @TomSweeneyRedHat PTAL

rhatdan · 2022-05-18T12:44:57Z

Nice work @flouthoc
/lgtm

…remote Feature of additional build context added here containers/buildah#3978 already exists on `podman` following PR just enables this feature of `podman-remote` and `podman on macOS` setups. Signed-off-by: Aditya R <[email protected]>

openshift-ci bot added the approved label May 10, 2022

flouthoc added the buildkit label May 10, 2022

flouthoc changed the title ~~buildkit, build: add support for additionalBuildContext in builds via --build-context~~ buildkit, build: add support for additionalBuildContext in builds via --build-context May 10, 2022

flouthoc force-pushed the buildkit-add-support-for-additional-context branch from 2ba51cd to bf81f15 Compare May 10, 2022 10:27

flouthoc force-pushed the buildkit-add-support-for-additional-context branch 4 times, most recently from 08dfee6 to b1bb97f Compare May 10, 2022 12:33

flouthoc requested review from nalind and rhatdan May 10, 2022 14:47

nalind requested changes May 10, 2022

View reviewed changes

TomSweeneyRedHat reviewed May 10, 2022

View reviewed changes

docs/buildah-build.1.md Outdated Show resolved Hide resolved

flouthoc force-pushed the buildkit-add-support-for-additional-context branch 2 times, most recently from 91fe328 to d6819c8 Compare May 11, 2022 09:36

flouthoc requested a review from nalind May 11, 2022 09:38

flouthoc force-pushed the buildkit-add-support-for-additional-context branch 2 times, most recently from 585e4dd to f7d0cc3 Compare May 11, 2022 09:52