Skip to content

Commit

Permalink
dockerfile: fix running onbuild rules from inherited stages
Browse files Browse the repository at this point in the history
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 <[email protected]>
(cherry picked from commit 5d3589f)
  • Loading branch information
tonistiigi committed Nov 7, 2024
1 parent fd61877 commit 3130178
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 14 deletions.
40 changes: 26 additions & 14 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"path/filepath"
"regexp"
"runtime"
"slices"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
97 changes: 97 additions & 0 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ var allTests = integration.TestFuncs(
testEnvEmptyFormatting,
testCacheMultiPlatformImportExport,
testOnBuildCleared,
testOnBuildInheritedStageRun,
testOnBuildInheritedStageWithFrom,
testOnBuildNewDeps,
testOnBuildNamedContext,
testOnBuildWithCacheMount,
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 3130178

Please sign in to comment.