Skip to content

Commit 3130178

Browse files
committed
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 <tonistiigi@gmail.com> (cherry picked from commit 5d3589f)
1 parent fd61877 commit 3130178

File tree

2 files changed

+123
-14
lines changed

2 files changed

+123
-14
lines changed

frontend/dockerfile/dockerfile2llb/convert.go

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"path/filepath"
1414
"regexp"
1515
"runtime"
16+
"slices"
1617
"sort"
1718
"strconv"
1819
"strings"
@@ -606,8 +607,18 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
606607
for d := range allReachable {
607608
d.init()
608609

609-
if len(d.image.Config.OnBuild) > 0 {
610-
if b, err := initOnBuildTriggers(d, d.image.Config.OnBuild, allDispatchStates); err != nil {
610+
onbuilds := slices.Clone(d.image.Config.OnBuild)
611+
if d.base != nil && !d.onBuildInit {
612+
for _, cmd := range d.base.commands {
613+
if obCmd, ok := cmd.Command.(*instructions.OnbuildCommand); ok {
614+
onbuilds = append(onbuilds, obCmd.Expression)
615+
}
616+
}
617+
d.onBuildInit = true
618+
}
619+
620+
if len(onbuilds) > 0 {
621+
if b, err := initOnBuildTriggers(d, onbuilds, allDispatchStates); err != nil {
611622
return nil, parser.SetLocation(err, d.stage.Location)
612623
} else if b {
613624
newDeps = true
@@ -1002,18 +1013,19 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
10021013
}
10031014

10041015
type dispatchState struct {
1005-
opt dispatchOpt
1006-
state llb.State
1007-
image dockerspec.DockerOCIImage
1008-
platform *ocispecs.Platform
1009-
stage instructions.Stage
1010-
base *dispatchState
1011-
baseImg *dockerspec.DockerOCIImage // immutable, unlike image
1012-
dispatched bool
1013-
resolved bool // resolved is set to true if base image has been resolved
1014-
deps map[*dispatchState]instructions.Command
1015-
buildArgs []instructions.KeyValuePairOptional
1016-
commands []command
1016+
opt dispatchOpt
1017+
state llb.State
1018+
image dockerspec.DockerOCIImage
1019+
platform *ocispecs.Platform
1020+
stage instructions.Stage
1021+
base *dispatchState
1022+
baseImg *dockerspec.DockerOCIImage // immutable, unlike image
1023+
dispatched bool
1024+
resolved bool // resolved is set to true if base image has been resolved
1025+
onBuildInit bool
1026+
deps map[*dispatchState]instructions.Command
1027+
buildArgs []instructions.KeyValuePairOptional
1028+
commands []command
10171029
// ctxPaths marks the paths this dispatchState uses from the build context.
10181030
ctxPaths map[string]struct{}
10191031
// paths marks the paths that are used by this dispatchState.

frontend/dockerfile/dockerfile_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ var allTests = integration.TestFuncs(
126126
testEnvEmptyFormatting,
127127
testCacheMultiPlatformImportExport,
128128
testOnBuildCleared,
129+
testOnBuildInheritedStageRun,
130+
testOnBuildInheritedStageWithFrom,
129131
testOnBuildNewDeps,
130132
testOnBuildNamedContext,
131133
testOnBuildWithCacheMount,
@@ -5028,6 +5030,101 @@ func testOnBuildNamedContext(t *testing.T, sb integration.Sandbox) {
50285030
require.Equal(t, []byte("hello"), dt)
50295031
}
50305032

5033+
func testOnBuildInheritedStageRun(t *testing.T, sb integration.Sandbox) {
5034+
integration.SkipOnPlatform(t, "windows")
5035+
workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush)
5036+
f := getFrontend(t, sb)
5037+
5038+
dockerfile := []byte(`
5039+
FROM busybox AS base
5040+
ONBUILD RUN mkdir -p /out && echo -n 11 >> /out/foo
5041+
5042+
FROM base AS mid
5043+
RUN cp /out/foo /out/bar
5044+
5045+
FROM scratch
5046+
COPY --from=mid /out/bar /
5047+
`)
5048+
5049+
dir := integration.Tmpdir(
5050+
t,
5051+
fstest.CreateFile("Dockerfile", dockerfile, 0600),
5052+
)
5053+
5054+
c, err := client.New(sb.Context(), sb.Address())
5055+
require.NoError(t, err)
5056+
defer c.Close()
5057+
5058+
destDir := t.TempDir()
5059+
5060+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
5061+
Exports: []client.ExportEntry{
5062+
{
5063+
Type: client.ExporterLocal,
5064+
OutputDir: destDir,
5065+
},
5066+
},
5067+
LocalMounts: map[string]fsutil.FS{
5068+
dockerui.DefaultLocalNameDockerfile: dir,
5069+
dockerui.DefaultLocalNameContext: dir,
5070+
},
5071+
}, nil)
5072+
require.NoError(t, err)
5073+
5074+
dt, err := os.ReadFile(filepath.Join(destDir, "bar"))
5075+
require.NoError(t, err)
5076+
require.Equal(t, "11", string(dt))
5077+
}
5078+
5079+
func testOnBuildInheritedStageWithFrom(t *testing.T, sb integration.Sandbox) {
5080+
integration.SkipOnPlatform(t, "windows")
5081+
workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush)
5082+
f := getFrontend(t, sb)
5083+
5084+
dockerfile := []byte(`
5085+
FROM alpine AS src
5086+
RUN mkdir -p /in && echo -n 12 > /in/file
5087+
5088+
FROM busybox AS base
5089+
ONBUILD COPY --from=src /in/file /out/foo
5090+
5091+
FROM base AS mid
5092+
RUN cp /out/foo /out/bar
5093+
5094+
FROM scratch
5095+
COPY --from=mid /out/bar /
5096+
`)
5097+
5098+
dir := integration.Tmpdir(
5099+
t,
5100+
fstest.CreateFile("Dockerfile", dockerfile, 0600),
5101+
)
5102+
5103+
c, err := client.New(sb.Context(), sb.Address())
5104+
require.NoError(t, err)
5105+
defer c.Close()
5106+
5107+
destDir := t.TempDir()
5108+
5109+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
5110+
Exports: []client.ExportEntry{
5111+
{
5112+
Type: client.ExporterLocal,
5113+
OutputDir: destDir,
5114+
},
5115+
},
5116+
LocalMounts: map[string]fsutil.FS{
5117+
dockerui.DefaultLocalNameDockerfile: dir,
5118+
dockerui.DefaultLocalNameContext: dir,
5119+
},
5120+
}, nil)
5121+
require.NoError(t, err)
5122+
5123+
dt, err := os.ReadFile(filepath.Join(destDir, "bar"))
5124+
require.NoError(t, err)
5125+
require.Equal(t, "12", string(dt))
5126+
}
5127+
50315128
func testOnBuildNewDeps(t *testing.T, sb integration.Sandbox) {
50325129
integration.SkipOnPlatform(t, "windows")
50335130
workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush)

0 commit comments

Comments
 (0)