Skip to content

Commit

Permalink
Pass platform arg for stage state to ContextByName
Browse files Browse the repository at this point in the history 
Fixes image build contexts not respecting the platform specified in the
Dockerfile `FROM --platform` argument.
Before this change buildkit always resolves images using the target
platform, but `FROM` may specifiy something else (such as
$BUILDPLATFORM).

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
  • Loading branch information
@cpuguy83
cpuguy83 committed Aug 9, 2022
1 parent 8488654 commit ba4d7bc
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 24 deletions.
9 changes: 5 additions & 4 deletions frontend/dockerfile/builder/build.go
View file
Expand Up @@ -448,7 +448,7 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) {
Warn: func(msg, url string, detail [][]byte, location *parser.Range) {
c.Warn(ctx, defVtx, msg, warnOpts(sourceMap, location, detail, url))
},
ContextByName: contextByNameFunc(c, c.BuildOpts().SessionID, targetPlatforms[0]),
ContextByName: contextByNameFunc(c, c.BuildOpts().SessionID),
}

defer func() {
Expand Down Expand Up @@ -487,8 +487,9 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) {
if i != 0 {
opt.Warn = nil
}
opt.ContextByName = contextByNameFunc(c, c.BuildOpts().SessionID, tp)
opt.ContextByName = contextByNameFunc(c, c.BuildOpts().SessionID)
st, img, bi, err := dockerfile2llb.Dockerfile2LLB(ctx, dtDockerfile, opt)

if err != nil {
return err
}
Expand Down Expand Up @@ -803,8 +804,8 @@ func warnOpts(sm *llb.SourceMap, r *parser.Range, detail [][]byte, url string) c
return opts
}

func contextByNameFunc(c client.Client, sessionID string, p *ocispecs.Platform) func(context.Context, string, string) (*llb.State, *dockerfile2llb.Image, *binfotypes.BuildInfo, error) {
return func(ctx context.Context, name, resolveMode string) (*llb.State, *dockerfile2llb.Image, *binfotypes.BuildInfo, error) {
func contextByNameFunc(c client.Client, sessionID string) func(context.Context, string, string, *ocispecs.Platform) (*llb.State, *dockerfile2llb.Image, *binfotypes.BuildInfo, error) {
return func(ctx context.Context, name, resolveMode string, p *ocispecs.Platform) (*llb.State, *dockerfile2llb.Image, *binfotypes.BuildInfo, error) {
named, err := reference.ParseNormalizedNamed(name)
if err != nil {
return nil, nil, nil, errors.Wrapf(err, "invalid context name %s", name)
Expand Down
45 changes: 25 additions & 20 deletions frontend/dockerfile/dockerfile2llb/convert.go
View file
Expand Up @@ -71,7 +71,7 @@ type ConvertOpt struct {
SourceMap *llb.SourceMap
Hostname string
Warn func(short, url string, detail [][]byte, location *parser.Range)
ContextByName func(ctx context.Context, name, resolveMode string) (*llb.State, *Image, *binfotypes.BuildInfo, error)
ContextByName func(ctx context.Context, name, resolveMode string, p *ocispecs.Platform) (*llb.State, *Image, *binfotypes.BuildInfo, error)
}

func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State, *Image, *binfotypes.BuildInfo, error) {
Expand Down Expand Up @@ -123,10 +123,10 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
buildInfo := &binfotypes.BuildInfo{}
buildInfoDepsMu := sync.Mutex{}
contextByName := opt.ContextByName
opt.ContextByName = func(ctx context.Context, name, resolveMode string) (*llb.State, *Image, *binfotypes.BuildInfo, error) {
opt.ContextByName = func(ctx context.Context, name, resolveMode string, p *ocispecs.Platform) (*llb.State, *Image, *binfotypes.BuildInfo, error) {
if !strings.EqualFold(name, "scratch") && !strings.EqualFold(name, "context") {
if contextByName != nil {
st, img, bi, err := contextByName(ctx, name, resolveMode)
st, img, bi, err := contextByName(ctx, name, resolveMode, p)
if err != nil {
return nil, nil, nil, err
}
Expand Down Expand Up @@ -225,8 +225,28 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
outline: outline.clone(),
}

if v := st.Platform; v != "" {
v, u, err := shlex.ProcessWordWithMatches(v, metaArgsToMap(optMetaArgs))
if err != nil {
return nil, nil, parser.WithLocation(errors.Wrapf(err, "failed to process arguments for platform %s", v), st.Location)
}

p, err := platforms.Parse(v)
if err != nil {
return nil, nil, parser.WithLocation(errors.Wrapf(err, "failed to parse platform %s", v), st.Location)
}
for k := range u {
used[k] = struct{}{}
}
ds.platform = &p
}

if st.Name != "" {
s, img, bi, err := opt.ContextByName(ctx, st.Name, opt.ImageResolveMode.String())
p := ds.platform
if p == nil {
p = opt.TargetPlatform
}
s, img, bi, err := opt.ContextByName(ctx, st.Name, opt.ImageResolveMode.String(), p)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -255,21 +275,6 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
ds.stageName = fmt.Sprintf("stage-%d", i)
}

if v := st.Platform; v != "" {
v, u, err := shlex.ProcessWordWithMatches(v, metaArgsToMap(optMetaArgs))
if err != nil {
return nil, nil, parser.WithLocation(errors.Wrapf(err, "failed to process arguments for platform %s", v), st.Location)
}

p, err := platforms.Parse(v)
if err != nil {
return nil, nil, parser.WithLocation(errors.Wrapf(err, "failed to parse platform %s", v), st.Location)
}
for k := range u {
used[k] = struct{}{}
}
ds.platform = &p
}
allDispatchStates.addState(ds)

for k := range used {
Expand Down Expand Up @@ -378,7 +383,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
d.stage.BaseName = reference.TagNameOnly(ref).String()

var isScratch bool
st, img, bi, err := opt.ContextByName(ctx, d.stage.BaseName, opt.ImageResolveMode.String())
st, img, bi, err := opt.ContextByName(ctx, d.stage.BaseName, opt.ImageResolveMode.String(), platform)
if err != nil {
return err
}
Expand Down
74 changes: 74 additions & 0 deletions frontend/dockerfile/dockerfile_test.go
View file
Expand Up @@ -115,6 +115,7 @@ var allTests = integration.TestFuncs(
testUlimit,
testCgroupParent,
testNamedImageContext,
testNamedImageContextPlatform,
testNamedImageContextTimestamps,
testNamedLocalContext,
testNamedOCILayoutContext,
Expand Down Expand Up @@ -5188,6 +5189,79 @@ COPY --from=base /env_foobar /
require.Contains(t, string(dt), "/foobar:")
}

func testNamedImageContextPlatform(t *testing.T, sb integration.Sandbox) {
ctx := sb.Context()

c, err := client.New(ctx, sb.Address())
require.NoError(t, err)
defer c.Close()

registry, err := sb.NewRegistry()
if errors.Is(err, integration.ErrRequirements) {
t.Skip(err.Error())
}
require.NoError(t, err)

// Build a base image and force buildkit to generate a manifest list.
dockerfile := []byte(`FROM --platform=$BUILDPLATFORM alpine:latest`)
target := registry + "/buildkit/testnamedimagecontextplatform:latest"

dir, err := integration.Tmpdir(
t,
fstest.CreateFile("Dockerfile", dockerfile, 0600),
)
require.NoError(t, err)

f := getFrontend(t, sb)

_, err = f.Solve(sb.Context(), c, client.SolveOpt{
FrontendAttrs: map[string]string{
"build-arg:BUILDKIT_MULTI_PLATFORM": "true",
},
LocalDirs: map[string]string{
builder.DefaultLocalNameDockerfile: dir,
builder.DefaultLocalNameContext: dir,
},
Exports: []client.ExportEntry{
{
Type: client.ExporterImage,
Attrs: map[string]string{
"name": target,
"push": "true",
},
},
},
}, nil)
require.NoError(t, err)

dockerfile = []byte(`
FROM --platform=$BUILDPLATFORM busybox AS target
RUN echo hello
`)

dir, err = integration.Tmpdir(
t,
fstest.CreateFile("Dockerfile", dockerfile, 0600),
)
require.NoError(t, err)

f = getFrontend(t, sb)

_, err = f.Solve(sb.Context(), c, client.SolveOpt{
FrontendAttrs: map[string]string{
"context:busybox": "docker-image://" + target,
// random platform that would never exist so it doesn't conflict with the build machine
// here we specifically want to make sure that the platform chosen for the image source is the one in the dockerfile not the target platform.
"platform": "darwin/ppc64le",
},
LocalDirs: map[string]string{
builder.DefaultLocalNameDockerfile: dir,
builder.DefaultLocalNameContext: dir,
},
}, nil)
require.NoError(t, err)
}

func testNamedImageContextTimestamps(t *testing.T, sb integration.Sandbox) {
ctx := sb.Context()

Expand Down

0 comments on commit ba4d7bc

Please sign in to comment.