From 313017824429e0ce5a748f168a119e02986e4d4f Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 6 Nov 2024 10:23:59 -0800 Subject: [PATCH] dockerfile: fix running onbuild rules from inherited stages When inheriting from a stage in the same Dockerfile, if that stage defines ONBUILD rules then they should execute as they would if the parent stage would be first built into standalone image and then that image used as a base. This restores the behavior before BuildKit v0.17.0. Signed-off-by: Tonis Tiigi (cherry picked from commit 5d3589fe522feb77b816ab1030f9068f11e93cfd) --- frontend/dockerfile/dockerfile2llb/convert.go | 40 +++++--- frontend/dockerfile/dockerfile_test.go | 97 +++++++++++++++++++ 2 files changed, 123 insertions(+), 14 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index e763a72052c9..dbde755315cf 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -13,6 +13,7 @@ import ( "path/filepath" "regexp" "runtime" + "slices" "sort" "strconv" "strings" @@ -606,8 +607,18 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS for d := range allReachable { d.init() - if len(d.image.Config.OnBuild) > 0 { - if b, err := initOnBuildTriggers(d, d.image.Config.OnBuild, allDispatchStates); err != nil { + onbuilds := slices.Clone(d.image.Config.OnBuild) + if d.base != nil && !d.onBuildInit { + for _, cmd := range d.base.commands { + if obCmd, ok := cmd.Command.(*instructions.OnbuildCommand); ok { + onbuilds = append(onbuilds, obCmd.Expression) + } + } + d.onBuildInit = true + } + + if len(onbuilds) > 0 { + if b, err := initOnBuildTriggers(d, onbuilds, allDispatchStates); err != nil { return nil, parser.SetLocation(err, d.stage.Location) } else if b { newDeps = true @@ -1002,18 +1013,19 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { } type dispatchState struct { - opt dispatchOpt - state llb.State - image dockerspec.DockerOCIImage - platform *ocispecs.Platform - stage instructions.Stage - base *dispatchState - baseImg *dockerspec.DockerOCIImage // immutable, unlike image - dispatched bool - resolved bool // resolved is set to true if base image has been resolved - deps map[*dispatchState]instructions.Command - buildArgs []instructions.KeyValuePairOptional - commands []command + opt dispatchOpt + state llb.State + image dockerspec.DockerOCIImage + platform *ocispecs.Platform + stage instructions.Stage + base *dispatchState + baseImg *dockerspec.DockerOCIImage // immutable, unlike image + dispatched bool + resolved bool // resolved is set to true if base image has been resolved + onBuildInit bool + deps map[*dispatchState]instructions.Command + buildArgs []instructions.KeyValuePairOptional + commands []command // ctxPaths marks the paths this dispatchState uses from the build context. ctxPaths map[string]struct{} // paths marks the paths that are used by this dispatchState. diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index d8aed914b3a6..fd2298eb550b 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -126,6 +126,8 @@ var allTests = integration.TestFuncs( testEnvEmptyFormatting, testCacheMultiPlatformImportExport, testOnBuildCleared, + testOnBuildInheritedStageRun, + testOnBuildInheritedStageWithFrom, testOnBuildNewDeps, testOnBuildNamedContext, testOnBuildWithCacheMount, @@ -5028,6 +5030,101 @@ func testOnBuildNamedContext(t *testing.T, sb integration.Sandbox) { require.Equal(t, []byte("hello"), dt) } +func testOnBuildInheritedStageRun(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush) + f := getFrontend(t, sb) + + dockerfile := []byte(` +FROM busybox AS base +ONBUILD RUN mkdir -p /out && echo -n 11 >> /out/foo + +FROM base AS mid +RUN cp /out/foo /out/bar + +FROM scratch +COPY --from=mid /out/bar / +`) + + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + destDir := t.TempDir() + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + dt, err := os.ReadFile(filepath.Join(destDir, "bar")) + require.NoError(t, err) + require.Equal(t, "11", string(dt)) +} + +func testOnBuildInheritedStageWithFrom(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush) + f := getFrontend(t, sb) + + dockerfile := []byte(` +FROM alpine AS src +RUN mkdir -p /in && echo -n 12 > /in/file + +FROM busybox AS base +ONBUILD COPY --from=src /in/file /out/foo + +FROM base AS mid +RUN cp /out/foo /out/bar + +FROM scratch +COPY --from=mid /out/bar / +`) + + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + destDir := t.TempDir() + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + dt, err := os.ReadFile(filepath.Join(destDir, "bar")) + require.NoError(t, err) + require.Equal(t, "12", string(dt)) +} + func testOnBuildNewDeps(t *testing.T, sb integration.Sandbox) { integration.SkipOnPlatform(t, "windows") workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush)