From b9a4a6eed0325deece3f11ebed2586890b375dcf Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sat, 21 Sep 2024 21:24:02 -0700 Subject: [PATCH 1/2] dockerfile: update args definitions to llb.EnvList This avoids many temporary conversion between maps/slices and shell.EnvGetter. Signed-off-by: Tonis Tiigi --- frontend/dockerfile/dockerfile2llb/convert.go | 154 ++++++------------ .../dockerfile/dockerfile2llb/platform.go | 35 ++-- 2 files changed, 71 insertions(+), 118 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index bf2ec86525c2..b87b2d4fa974 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -228,10 +228,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS platformOpt := buildPlatformOpt(&opt) - optMetaArgs := getPlatformArgs(platformOpt) - for i, arg := range optMetaArgs { - optMetaArgs[i] = setKVValue(arg, opt.BuildArgs) - } + globalArgs := platformArgs(platformOpt, opt.BuildArgs) dockerfile, err := parser.Parse(bytes.NewReader(dt)) if err != nil { @@ -253,7 +250,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS proxyEnv := proxyEnvFromBuildArgs(opt.BuildArgs) - stages, metaArgs, err := instructions.Parse(dockerfile.AST, lint) + stages, argCmds, err := instructions.Parse(dockerfile.AST, lint) if err != nil { return nil, err } @@ -268,11 +265,11 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS // Validate that base images continue to be valid even // when no build arguments are used. - validateBaseImagesWithDefaultArgs(stages, shlex, metaArgs, optMetaArgs, lint) + validateBaseImagesWithDefaultArgs(stages, shlex, argCmds, globalArgs, lint) // Rebuild the arguments using the provided build arguments // for the remainder of the build. - optMetaArgs, outline.allArgs, err = buildMetaArgs(optMetaArgs, shlex, metaArgs, opt.BuildArgs) + globalArgs, outline.allArgs, err = buildMetaArgs(globalArgs, shlex, argCmds, opt.BuildArgs) if err != nil { return nil, err } @@ -286,9 +283,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS // set base state for every image for i, st := range stages { - env := metaArgsToEnvs(optMetaArgs) - nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, env) - reportUnusedFromArgs(metaArgsKeys(optMetaArgs), nameMatch.Unmatched, st.Location, lint) + nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, globalArgs) + reportUnusedFromArgs(globalArgs, nameMatch.Unmatched, st.Location, lint) used := nameMatch.Matched if used == nil { used = map[string]struct{}{} @@ -314,10 +310,9 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS } if v := st.Platform; v != "" { - platEnv := metaArgsToEnvs(optMetaArgs) - platMatch, err := shlex.ProcessWordWithMatches(v, platEnv) - reportUnusedFromArgs(metaArgsKeys(optMetaArgs), platMatch.Unmatched, st.Location, lint) - reportRedundantTargetPlatform(st.Platform, platMatch, st.Location, platEnv, lint) + platMatch, err := shlex.ProcessWordWithMatches(v, globalArgs) + reportUnusedFromArgs(globalArgs, platMatch.Unmatched, st.Location, lint) + reportRedundantTargetPlatform(st.Platform, platMatch, st.Location, globalArgs, lint) reportConstPlatformDisallowed(st.Name, platMatch, st.Location, lint) if err != nil { @@ -327,14 +322,14 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS if platMatch.Result == "" { err := errors.Errorf("empty platform value from expression %s", v) err = parser.WithLocation(err, st.Location) - err = wrapSuggestAny(err, platMatch.Unmatched, metaArgsKeys(optMetaArgs)) + err = wrapSuggestAny(err, platMatch.Unmatched, globalArgs.Keys()) return nil, err } p, err := platforms.Parse(platMatch.Result) if err != nil { err = parser.WithLocation(err, st.Location) - err = wrapSuggestAny(err, platMatch.Unmatched, metaArgsKeys(optMetaArgs)) + err = wrapSuggestAny(err, platMatch.Unmatched, globalArgs.Keys()) return nil, parser.WithLocation(errors.Wrapf(err, "failed to parse platform %s", v), st.Location) } @@ -653,7 +648,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS opt := dispatchOpt{ allDispatchStates: allDispatchStates, - metaArgs: optMetaArgs, + globalArgs: globalArgs, buildArgValues: opt.BuildArgs, shlex: shlex, buildContext: llb.NewState(buildContext), @@ -687,15 +682,22 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS ctxPaths[p] = struct{}{} } - locals := []instructions.KeyValuePairOptional{} - locals = append(locals, d.opt.metaArgs...) - locals = append(locals, d.buildArgs...) - for _, a := range locals { - switch a.Key { - case sbomScanStage: - d.scanStage = isEnabledForStage(d.stageName, a.ValueString()) - case sbomScanContext: - d.scanContext = isEnabledForStage(d.stageName, a.ValueString()) + for _, name := range []string{sbomScanContext, sbomScanStage} { + var b bool + if v, ok := d.opt.globalArgs.Get(name); ok { + b = isEnabledForStage(d.stageName, v) + } + for _, kv := range d.buildArgs { + if kv.Key == name && kv.Value != nil { + b = isEnabledForStage(d.stageName, *kv.Value) + } + } + if b { + if name == sbomScanContext { + d.scanContext = true + } else { + d.scanStage = true + } } } } @@ -751,48 +753,6 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS return target, nil } -func metaArgsToEnvs(metaArgs []instructions.KeyValuePairOptional) shell.EnvGetter { - return &envsFromKeyValuePairs{in: metaArgs} -} - -type envsFromKeyValuePairs struct { - in []instructions.KeyValuePairOptional - once sync.Once - m map[string]string -} - -func (e *envsFromKeyValuePairs) init() { - if len(e.in) == 0 { - return - } - e.m = make(map[string]string, len(e.in)) - for _, kv := range e.in { - e.m[kv.Key] = kv.ValueString() - } -} - -func (e *envsFromKeyValuePairs) Get(key string) (string, bool) { - e.once.Do(e.init) - v, ok := e.m[key] // windows: case-insensitive - return v, ok -} - -func (e *envsFromKeyValuePairs) Keys() []string { - keys := make([]string, len(e.in)) - for i, kp := range e.in { - keys[i] = kp.Key - } - return keys -} - -func metaArgsKeys(metaArgs []instructions.KeyValuePairOptional) []string { - s := make([]string, 0, len(metaArgs)) - for _, arg := range metaArgs { - s = append(s, arg.Key) - } - return s -} - func toCommand(ic instructions.Command, allDispatchStates *dispatchStates) (command, error) { cmd := command{Command: ic} if c, ok := ic.(*instructions.CopyCommand); ok { @@ -828,7 +788,7 @@ func toCommand(ic instructions.Command, allDispatchStates *dispatchStates) (comm type dispatchOpt struct { allDispatchStates *dispatchStates - metaArgs []instructions.KeyValuePairOptional + globalArgs shell.EnvGetter buildArgValues map[string]string shlex *shell.Lex buildContext llb.State @@ -1731,12 +1691,10 @@ func dispatchArg(d *dispatchState, c *instructions.ArgCommand, opt *dispatchOpt) skipArgInfo := false // skip the arg info if the arg is inherited from global scope if !hasDefault && !hasValue { - for _, ma := range opt.metaArgs { - if ma.Key == arg.Key { - arg.Value = ma.Value - skipArgInfo = true - hasDefault = false - } + if v, ok := opt.globalArgs.Get(arg.Key); ok { + arg.Value = &v + skipArgInfo = true + hasDefault = false } } @@ -1827,13 +1785,6 @@ func parseKeyValue(env string) (string, string) { return parts[0], v } -func setKVValue(kvpo instructions.KeyValuePairOptional, values map[string]string) instructions.KeyValuePairOptional { - if v, ok := values[kvpo.Key]; ok { - kvpo.Value = &v - } - return kvpo -} - func dfCmd(cmd interface{}) llb.ConstraintsOpt { // TODO: add fmt.Stringer to instructions.Command to remove interface{} var cmdStr string @@ -2278,8 +2229,7 @@ func reportUnmatchedVariables(cmd instructions.Command, buildArgs []instructions if len(unmatched) == 0 { return } - options := metaArgsKeys(opt.metaArgs) - options = append(options, env.Keys()...) + options := env.Keys() for cmdVar := range unmatched { if _, nonEnvOk := nonEnvArgs[cmdVar]; nonEnvOk { continue @@ -2340,9 +2290,9 @@ func toPBLocation(sourceIndex int, location []parser.Range) pb.Location { } } -func reportUnusedFromArgs(values []string, unmatched map[string]struct{}, location []parser.Range, lint *linter.Linter) { +func reportUnusedFromArgs(args shell.EnvGetter, unmatched map[string]struct{}, location []parser.Range, lint *linter.Linter) { for arg := range unmatched { - suggest, _ := suggest.Search(arg, values, true) + suggest, _ := suggest.Search(arg, args.Keys(), true) msg := linter.RuleUndefinedArgInFrom.Format(arg, suggest) lint.Run(&linter.RuleUndefinedArgInFrom, location, msg) } @@ -2464,10 +2414,10 @@ func validateNoSecretKey(instruction, key string, location []parser.Range, lint } } -func validateBaseImagesWithDefaultArgs(stages []instructions.Stage, shlex *shell.Lex, metaArgs []instructions.ArgCommand, optMetaArgs []instructions.KeyValuePairOptional, lint *linter.Linter) { +func validateBaseImagesWithDefaultArgs(stages []instructions.Stage, shlex *shell.Lex, argsCmds []instructions.ArgCommand, args *llb.EnvList, lint *linter.Linter) { // Build the arguments as if no build options were given // and using only defaults. - optMetaArgs, _, err := buildMetaArgs(optMetaArgs, shlex, metaArgs, nil) + args, _, err := buildMetaArgs(args, shlex, argsCmds, nil) if err != nil { // Abandon running the linter. We'll likely fail after this point // with the same error but we shouldn't error here inside @@ -2476,7 +2426,7 @@ func validateBaseImagesWithDefaultArgs(stages []instructions.Stage, shlex *shell } for _, st := range stages { - nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, metaArgsToEnvs(optMetaArgs)) + nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, args) if err != nil { return } @@ -2489,32 +2439,32 @@ func validateBaseImagesWithDefaultArgs(stages []instructions.Stage, shlex *shell } } -func buildMetaArgs(metaArgs []instructions.KeyValuePairOptional, shlex *shell.Lex, argCommands []instructions.ArgCommand, buildArgs map[string]string) ([]instructions.KeyValuePairOptional, map[string]argInfo, error) { +func buildMetaArgs(args *llb.EnvList, shlex *shell.Lex, argCommands []instructions.ArgCommand, buildArgs map[string]string) (*llb.EnvList, map[string]argInfo, error) { allArgs := make(map[string]argInfo) for _, cmd := range argCommands { - for _, metaArg := range cmd.Args { - info := argInfo{definition: metaArg, location: cmd.Location()} - if v, ok := buildArgs[metaArg.Key]; !ok { - if metaArg.Value != nil { - result, err := shlex.ProcessWordWithMatches(*metaArg.Value, metaArgsToEnvs(metaArgs)) + for _, kp := range cmd.Args { + info := argInfo{definition: kp, location: cmd.Location()} + if v, ok := buildArgs[kp.Key]; !ok { + if kp.Value != nil { + result, err := shlex.ProcessWordWithMatches(*kp.Value, args) if err != nil { return nil, nil, parser.WithLocation(err, cmd.Location()) } - metaArg.Value = &result.Result + kp.Value = &result.Result info.deps = result.Matched } } else { - metaArg.Value = &v + kp.Value = &v } - metaArgs = append(metaArgs, metaArg) - if metaArg.Value != nil { - info.value = *metaArg.Value + if kp.Value != nil { + args = args.AddOrReplace(kp.Key, *kp.Value) + info.value = *kp.Value } - allArgs[metaArg.Key] = info + allArgs[kp.Key] = info } } - return metaArgs, allArgs, nil + return args, allArgs, nil } type emptyEnvs struct{} diff --git a/frontend/dockerfile/dockerfile2llb/platform.go b/frontend/dockerfile/dockerfile2llb/platform.go index 16e6e3da8fa4..b4e666ee8410 100644 --- a/frontend/dockerfile/dockerfile2llb/platform.go +++ b/frontend/dockerfile/dockerfile2llb/platform.go @@ -2,7 +2,7 @@ package dockerfile2llb import ( "github.com/containerd/platforms" - "github.com/moby/buildkit/frontend/dockerfile/instructions" + "github.com/moby/buildkit/client/llb" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -36,23 +36,26 @@ func buildPlatformOpt(opt *ConvertOpt) *platformOpt { } } -func getPlatformArgs(po *platformOpt) []instructions.KeyValuePairOptional { +func platformArgs(po *platformOpt, overrides map[string]string) *llb.EnvList { bp := po.buildPlatforms[0] tp := po.targetPlatform - m := map[string]string{ - "BUILDPLATFORM": platforms.Format(bp), - "BUILDOS": bp.OS, - "BUILDARCH": bp.Architecture, - "BUILDVARIANT": bp.Variant, - "TARGETPLATFORM": platforms.Format(tp), - "TARGETOS": tp.OS, - "TARGETARCH": tp.Architecture, - "TARGETVARIANT": tp.Variant, + s := [...][2]string{ + {"BUILDPLATFORM", platforms.Format(bp)}, + {"BUILDOS", bp.OS}, + {"BUILDARCH", bp.Architecture}, + {"BUILDVARIANT", bp.Variant}, + {"TARGETPLATFORM", platforms.Format(tp)}, + {"TARGETOS", tp.OS}, + {"TARGETARCH", tp.Architecture}, + {"TARGETVARIANT", tp.Variant}, } - opts := make([]instructions.KeyValuePairOptional, 0, len(m)) - for k, v := range m { - s := v - opts = append(opts, instructions.KeyValuePairOptional{Key: k, Value: &s}) + env := &llb.EnvList{} + for _, kv := range s { + v := kv[1] + if ov, ok := overrides[kv[0]]; ok { + v = ov + } + env = env.AddOrReplace(kv[0], v) } - return opts + return env } From 4cf7a1a0d287baf99b5723d8284db36a9d4e5032 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 23 Sep 2024 11:01:21 -0700 Subject: [PATCH 2/2] dockerfile: add support for ONBUILD in combination to from This adds support for the ONBUILD commands to refer to other stages in commands like `COPY --from=` or `RUN --mount=from=` . The source may be a stage in the calling Dockerfile, implicit image or named build context. Signed-off-by: Tonis Tiigi --- frontend/dockerfile/dockerfile2llb/convert.go | 338 ++++++++++-------- .../dockerfile2llb/convert_runmount.go | 2 +- frontend/dockerfile/dockerfile_test.go | 322 +++++++++++++++++ 3 files changed, 513 insertions(+), 149 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index b87b2d4fa974..bf6a9745b011 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -350,7 +350,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS return nil, err } if s != nil { - ds.noinit = true + ds.dispatched = true ds.state = *s if img != nil { // timestamps are inherited as-is, regardless to SOURCE_DATE_EPOCH @@ -446,146 +446,175 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS allStageNames = append(allStageNames, s.stageName) } } - allReachable := allReachableStages(target) - baseCtx := ctx - eg, ctx := errgroup.WithContext(ctx) - for i, d := range allDispatchStates.states { - _, reachable := allReachable[d] - if opt.AllStages { - reachable = true - } - // resolve image config for every stage - if d.base == nil && !d.noinit { - if d.stage.BaseName == emptyImageName { - d.state = llb.Scratch() - d.image = emptyImage(platformOpt.targetPlatform) - d.platform = &platformOpt.targetPlatform - if d.unregistered { - d.noinit = true - } - continue + resolveReachableStages := func(ctx context.Context, all []*dispatchState, target *dispatchState) (map[*dispatchState]struct{}, error) { + allReachable := allReachableStages(target) + eg, ctx := errgroup.WithContext(ctx) + for i, d := range all { + _, reachable := allReachable[d] + if opt.AllStages { + reachable = true } - func(i int, d *dispatchState) { - eg.Go(func() (err error) { - defer func() { + // resolve image config for every stage + if d.base == nil && !d.dispatched && !d.resolved { + d.resolved = reachable // avoid re-resolving if called again after onbuild + if d.stage.BaseName == emptyImageName { + d.state = llb.Scratch() + d.image = emptyImage(platformOpt.targetPlatform) + d.platform = &platformOpt.targetPlatform + if d.unregistered { + d.dispatched = true + } + continue + } + func(i int, d *dispatchState) { + eg.Go(func() (err error) { + defer func() { + if err != nil { + err = parser.WithLocation(err, d.stage.Location) + } + if d.unregistered { + // implicit stages don't need further dispatch + d.dispatched = true + } + }() + origName := d.stage.BaseName + ref, err := reference.ParseNormalizedNamed(d.stage.BaseName) if err != nil { - err = parser.WithLocation(err, d.stage.Location) + return errors.Wrapf(err, "failed to parse stage name %q", d.stage.BaseName) } - if d.unregistered { - // implicit stages don't need further dispatch - d.noinit = true + platform := d.platform + if platform == nil { + platform = &platformOpt.targetPlatform } - }() - origName := d.stage.BaseName - ref, err := reference.ParseNormalizedNamed(d.stage.BaseName) - if err != nil { - return errors.Wrapf(err, "failed to parse stage name %q", d.stage.BaseName) - } - platform := d.platform - if platform == nil { - platform = &platformOpt.targetPlatform - } - d.stage.BaseName = reference.TagNameOnly(ref).String() + d.stage.BaseName = reference.TagNameOnly(ref).String() - var isScratch bool - st, img, err := namedContext(ctx, d.stage.BaseName, dockerui.ContextOpt{ - ResolveMode: opt.ImageResolveMode.String(), - Platform: platform, - AsyncLocalOpts: d.asyncLocalOpts, - }) - if err != nil { - return err - } - if st != nil { - if img != nil { - d.image = *img - } else { - d.image = emptyImage(platformOpt.targetPlatform) - } - d.state = st.Platform(*platform) - d.platform = platform - return nil - } - if reachable { - prefix := "[" - if opt.MultiPlatformRequested && platform != nil { - prefix += platforms.Format(*platform) + " " - } - prefix += "internal]" - mutRef, dgst, dt, err := metaResolver.ResolveImageConfig(ctx, d.stage.BaseName, sourceresolver.Opt{ - LogName: fmt.Sprintf("%s load metadata for %s", prefix, d.stage.BaseName), - Platform: platform, - ImageOpt: &sourceresolver.ResolveImageOpt{ - ResolveMode: opt.ImageResolveMode.String(), - }, + var isScratch bool + st, img, err := namedContext(ctx, d.stage.BaseName, dockerui.ContextOpt{ + ResolveMode: opt.ImageResolveMode.String(), + Platform: platform, + AsyncLocalOpts: d.asyncLocalOpts, }) if err != nil { - return suggest.WrapError(errors.Wrap(err, origName), origName, append(allStageNames, commonImageNames()...), true) + return err } - - if ref.String() != mutRef { - ref, err = reference.ParseNormalizedNamed(mutRef) - if err != nil { - return errors.Wrapf(err, "failed to parse ref %q", mutRef) + if st != nil { + if img != nil { + d.image = *img + } else { + d.image = emptyImage(platformOpt.targetPlatform) } + d.state = st.Platform(*platform) + d.platform = platform + return nil } - var img dockerspec.DockerOCIImage - if err := json.Unmarshal(dt, &img); err != nil { - return errors.Wrap(err, "failed to parse image config") - } - d.baseImg = cloneX(&img) // immutable - img.Created = nil - // if there is no explicit target platform, try to match based on image config - if d.platform == nil && platformOpt.implicitTarget { - p := autoDetectPlatform(img, *platform, platformOpt.buildPlatforms) - platform = &p - } - if dgst != "" { - ref, err = reference.WithDigest(ref, dgst) + if reachable { + prefix := "[" + if opt.MultiPlatformRequested && platform != nil { + prefix += platforms.Format(*platform) + " " + } + prefix += "internal]" + mutRef, dgst, dt, err := metaResolver.ResolveImageConfig(ctx, d.stage.BaseName, sourceresolver.Opt{ + LogName: fmt.Sprintf("%s load metadata for %s", prefix, d.stage.BaseName), + Platform: platform, + ImageOpt: &sourceresolver.ResolveImageOpt{ + ResolveMode: opt.ImageResolveMode.String(), + }, + }) if err != nil { - return err + return suggest.WrapError(errors.Wrap(err, origName), origName, append(allStageNames, commonImageNames()...), true) } - } - d.stage.BaseName = ref.String() - if len(img.RootFS.DiffIDs) == 0 { - isScratch = true - // schema1 images can't return diffIDs so double check :( - for _, h := range img.History { - if !h.EmptyLayer { - isScratch = false - break + + if ref.String() != mutRef { + ref, err = reference.ParseNormalizedNamed(mutRef) + if err != nil { + return errors.Wrapf(err, "failed to parse ref %q", mutRef) + } + } + var img dockerspec.DockerOCIImage + if err := json.Unmarshal(dt, &img); err != nil { + return errors.Wrap(err, "failed to parse image config") + } + d.baseImg = cloneX(&img) // immutable + img.Created = nil + // if there is no explicit target platform, try to match based on image config + if d.platform == nil && platformOpt.implicitTarget { + p := autoDetectPlatform(img, *platform, platformOpt.buildPlatforms) + platform = &p + } + if dgst != "" { + ref, err = reference.WithDigest(ref, dgst) + if err != nil { + return err } } + d.stage.BaseName = ref.String() + if len(img.RootFS.DiffIDs) == 0 { + isScratch = true + // schema1 images can't return diffIDs so double check :( + for _, h := range img.History { + if !h.EmptyLayer { + isScratch = false + break + } + } + } + d.image = img } - d.image = img - } - if isScratch { - d.state = llb.Scratch() - } else { - d.state = llb.Image(d.stage.BaseName, - dfCmd(d.stage.SourceCode), - llb.Platform(*platform), - opt.ImageResolveMode, - llb.WithCustomName(prefixCommand(d, "FROM "+d.stage.BaseName, opt.MultiPlatformRequested, platform, emptyEnvs{})), - location(opt.SourceMap, d.stage.Location), - ) - if reachable { - validateBaseImagePlatform(origName, *platform, d.image.Platform, d.stage.Location, lint) + if isScratch { + d.state = llb.Scratch() + } else { + d.state = llb.Image(d.stage.BaseName, + dfCmd(d.stage.SourceCode), + llb.Platform(*platform), + opt.ImageResolveMode, + llb.WithCustomName(prefixCommand(d, "FROM "+d.stage.BaseName, opt.MultiPlatformRequested, platform, emptyEnvs{})), + location(opt.SourceMap, d.stage.Location), + ) + if reachable { + validateBaseImagePlatform(origName, *platform, d.image.Platform, d.stage.Location, lint) + } } - } - d.platform = platform - return nil - }) - }(i, d) + d.platform = platform + return nil + }) + }(i, d) + } + } + + if err := eg.Wait(); err != nil { + return nil, err } + return allReachable, nil } - if err := eg.Wait(); err != nil { - return nil, err + var allReachable map[*dispatchState]struct{} + for { + allReachable, err = resolveReachableStages(ctx, allDispatchStates.states, target) + if err != nil { + return nil, err + } + + // initialize onbuild triggers in case they create new dependencies + newDeps := false + 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 { + return nil, err + } else if b { + newDeps = true + } + d.image.Config.OnBuild = nil + } + } + // in case new dependencies were added, we need to re-resolve reachable stages + if !newDeps { + break + } } - ctx = baseCtx buildContext := &mutableOutput{} ctxPaths := map[string]struct{}{} @@ -605,11 +634,12 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS for _, d := range allDispatchStates.states { if !opt.AllStages { - if _, ok := allReachable[d]; !ok || d.noinit { + if _, ok := allReachable[d]; !ok || d.dispatched { continue } } d.init() + d.dispatched = true // Ensure platform is set. if d.platform == nil { @@ -666,11 +696,6 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS dockerIgnoreMatcher: dockerIgnoreMatcher, } - if err = dispatchOnBuildTriggers(d, d.image.Config.OnBuild, opt); err != nil { - return nil, parser.WithLocation(err, d.stage.Location) - } - d.image.Config.OnBuild = nil - for _, cmd := range d.commands { if err := dispatch(d, cmd, opt); err != nil { return nil, parser.WithLocation(err, cmd.Location()) @@ -928,7 +953,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { var ignoreMatcher *patternmatcher.PatternMatcher if len(cmd.sources) != 0 { src := cmd.sources[0] - if !src.noinit { + if !src.dispatched { return errors.Errorf("cannot copy from stage %q, it needs to be defined before current stage %q", c.From, d.stageName) } l = src.state @@ -970,17 +995,18 @@ 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 - noinit bool - 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 + 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. @@ -1012,8 +1038,6 @@ func (ds *dispatchState) asyncLocalOpts() []llb.LocalOption { // from the base image. func (ds *dispatchState) init() { // mark as initialized, used to determine states that have not been dispatched yet - ds.noinit = true - if ds.base == nil { return } @@ -1072,28 +1096,46 @@ type command struct { sources []*dispatchState } -func dispatchOnBuildTriggers(d *dispatchState, triggers []string, opt dispatchOpt) error { +// initOnBuildTriggers initializes the onbuild triggers and creates the commands and dependecies for them. +// It returns true if there were any new dependencies added that need to be resolved. +func initOnBuildTriggers(d *dispatchState, triggers []string, allDispatchStates *dispatchStates) (bool, error) { + hasNewDeps := false + commands := make([]command, 0, len(triggers)) + for _, trigger := range triggers { ast, err := parser.Parse(strings.NewReader(trigger)) if err != nil { - return err + return false, err } if len(ast.AST.Children) != 1 { - return errors.New("onbuild trigger should be a single expression") + return false, errors.New("onbuild trigger should be a single expression") } ic, err := instructions.ParseCommand(ast.AST.Children[0]) if err != nil { - return err + return false, err } - cmd, err := toCommand(ic, opt.allDispatchStates) + cmd, err := toCommand(ic, allDispatchStates) if err != nil { - return err + return false, err } - if err := dispatch(d, cmd, opt); err != nil { - return err + if len(cmd.sources) > 0 { + hasNewDeps = true + } + + commands = append(commands, cmd) + + for _, src := range cmd.sources { + if src != nil { + d.deps[src] = cmd + if src.unregistered { + allDispatchStates.addState(src) + } + } } } - return nil + d.commands = append(commands, d.commands...) + + return hasNewDeps, nil } func dispatchEnv(d *dispatchState, c *instructions.EnvCommand, lint *linter.Linter) error { diff --git a/frontend/dockerfile/dockerfile2llb/convert_runmount.go b/frontend/dockerfile/dockerfile2llb/convert_runmount.go index cc9d858641d2..1044ec360014 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_runmount.go +++ b/frontend/dockerfile/dockerfile2llb/convert_runmount.go @@ -72,7 +72,7 @@ func dispatchRunMounts(d *dispatchState, c *instructions.RunCommand, sources []* if mount.From != "" { src := sources[i] st = src.state - if !src.noinit { + if !src.dispatched { return nil, errors.Errorf("cannot mount from stage %q to %q, stage needs to be defined before current command", mount.From, mount.Target) } } diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index 8d4f25401752..d05bfaadab34 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -126,6 +126,9 @@ var allTests = integration.TestFuncs( testEnvEmptyFormatting, testCacheMultiPlatformImportExport, testOnBuildCleared, + testOnBuildNewDeps, + testOnBuildNamedContext, + testOnBuildWithCacheMount, testFrontendUseForwardedSolveResults, testFrontendEvaluate, testFrontendInputs, @@ -4674,6 +4677,325 @@ ONBUILD RUN mkdir -p /out && echo -n 11 >> /out/foo require.Equal(t, "11", string(dt)) } +func testOnBuildNamedContext(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter, workers.FeatureOCILayout) + // create an image with onbuild that relies on "otherstage" when imported + ctx := sb.Context() + + c, err := client.New(ctx, sb.Address()) + require.NoError(t, err) + defer c.Close() + + // create a tempdir where we will store the OCI layout + ocidir := t.TempDir() + + ociDockerfile := []byte(` + FROM busybox:latest + ONBUILD COPY --from=otherstage /testfile /out/foo + `) + inDir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", ociDockerfile, 0600), + ) + + f := getFrontend(t, sb) + + outW := bytes.NewBuffer(nil) + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: inDir, + dockerui.DefaultLocalNameContext: inDir, + }, + Exports: []client.ExportEntry{ + { + Type: client.ExporterOCI, + Output: fixedWriteCloser(nopWriteCloser{outW}), + }, + }, + }, nil) + require.NoError(t, err) + + // extract the tar stream to the directory as OCI layout + m, err := testutil.ReadTarToMap(outW.Bytes(), false) + require.NoError(t, err) + + for filename, content := range m { + fullFilename := path.Join(ocidir, filename) + err = os.MkdirAll(path.Dir(fullFilename), 0755) + require.NoError(t, err) + if content.Header.FileInfo().IsDir() { + err = os.MkdirAll(fullFilename, 0755) + require.NoError(t, err) + } else { + err = os.WriteFile(fullFilename, content.Data, 0644) + require.NoError(t, err) + } + } + + var index ocispecs.Index + err = json.Unmarshal(m[ocispecs.ImageIndexFile].Data, &index) + require.NoError(t, err) + require.Equal(t, 1, len(index.Manifests)) + digest := index.Manifests[0].Digest.Hex() + + store, err := local.NewStore(ocidir) + ociID := "ocione" + require.NoError(t, err) + + dockerfile := []byte(` + FROM alpine AS otherstage + RUN echo -n "hello" > /testfile + + FROM base AS inputstage + + FROM scratch + COPY --from=inputstage /out/foo /bar +`) + + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + destDir := t.TempDir() + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + FrontendAttrs: map[string]string{ + "context:base": fmt.Sprintf("oci-layout:%s@sha256:%s", ociID, digest), + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + OCIStores: map[string]content.Store{ + ociID: store, + }, + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir, + }, + }, + }, nil) + require.NoError(t, err) + + dt, err := os.ReadFile(filepath.Join(destDir, "bar")) + require.NoError(t, err) + require.Equal(t, []byte("hello"), dt) +} + +func testOnBuildNewDeps(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush) + f := getFrontend(t, sb) + + registry, err := sb.NewRegistry() + if errors.Is(err, integration.ErrRequirements) { + t.Skip(err.Error()) + } + require.NoError(t, err) + + dockerfile := []byte(` +FROM busybox +ONBUILD COPY --from=alpine /etc/alpine-release /out/alpine-release2 +`) + + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + target := registry + "/buildkit/testonbuilddeps:base" + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterImage, + Attrs: map[string]string{ + "push": "true", + "name": target, + }, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + dockerfile = []byte(fmt.Sprintf(` + FROM %s AS base + RUN cat /out/alpine-release2 > /out/alpine-release3 + FROM scratch + COPY --from=base /out / + `, target)) + + dir = integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + 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, "alpine-release3")) + require.NoError(t, err) + require.Greater(t, len(dt), 5) + + // build another onbuild image to test nested case + dockerfile = []byte(` +FROM alpine +ONBUILD RUN --mount=type=bind,target=/in,from=inputstage mkdir /out && cat /in/foo > /out/bar && cat /in/out/alpine-release2 > /out/bar2 +`) + + dir = integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + target2 := registry + "/buildkit/testonbuilddeps:base2" + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterImage, + Attrs: map[string]string{ + "push": "true", + "name": target2, + }, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + dockerfile = []byte(fmt.Sprintf(` + FROM %s AS inputstage + RUN cat /out/alpine-release2 > /out/alpine-release4 + RUN echo -n foo > /foo + FROM %s AS base + RUN echo -n bar3 > /out/bar3 + FROM scratch + COPY --from=base /out / + `, target, target2)) + + dir = integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + 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, "foo", string(dt)) + + dt, err = os.ReadFile(filepath.Join(destDir, "bar2")) + require.NoError(t, err) + require.Greater(t, len(dt), 5) + + dt, err = os.ReadFile(filepath.Join(destDir, "bar3")) + require.NoError(t, err) + require.Equal(t, "bar3", string(dt)) +} + +func testOnBuildWithCacheMount(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush) + f := getFrontend(t, sb) + + registry, err := sb.NewRegistry() + if errors.Is(err, integration.ErrRequirements) { + t.Skip(err.Error()) + } + require.NoError(t, err) + + dockerfile := []byte(` +FROM busybox +ONBUILD RUN --mount=type=cache,target=/cache echo -n 42 >> /cache/foo && echo -n 11 >> /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() + + target := registry + "/buildkit/testonbuild:base" + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterImage, + Attrs: map[string]string{ + "push": "true", + "name": target, + }, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + dockerfile = []byte(fmt.Sprintf(`FROM %s +RUN --mount=type=cache,target=/cache [ "$(cat /cache/foo)" = "42" ] && [ "$(cat /bar)" = "11" ] + `, target)) + + dir = integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) +} + func testCacheMultiPlatformImportExport(t *testing.T, sb integration.Sandbox) { integration.SkipOnPlatform(t, "windows") workers.CheckFeatureCompat(t, sb,