From bb5542e26e9fa26dec84bf8351ec9e0ecc1b5a40 Mon Sep 17 00:00:00 2001 From: Andrea Luzzardi Date: Mon, 12 Apr 2021 17:45:38 -0700 Subject: [PATCH] Proper support for Docker Image metadata - Both FetchContainer and DockerBuild read the image metadata and convert to LLB (e.g. `ENV foo bar` in Dockerfile shows up in `op.#Exec`) - Image metadata is "sticky" between Pipelines (e.g. `op.#Load` will re-use the same metadata) - Image metadata is injected back to #PushContainer, so that DockerBuild+PushContainer and FetchContainer+PushContainer do not lose any metadata. - Tests for all the above Fixes #142 Signed-off-by: Andrea Luzzardi --- dagger/pipeline.go | 108 ++++++++++++++++++------------ dagger/solver.go | 32 +++++++-- tests/llb/dockerbuild/main.cue | 29 ++++++++ tests/llb/push-container/main.cue | 76 +++++++++++++++++++++ 4 files changed, 195 insertions(+), 50 deletions(-) diff --git a/dagger/pipeline.go b/dagger/pipeline.go index 212ffc63..7cddcfd0 100644 --- a/dagger/pipeline.go +++ b/dagger/pipeline.go @@ -14,7 +14,9 @@ import ( "github.com/docker/distribution/reference" bk "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/exporter/containerimage/exptypes" dockerfilebuilder "github.com/moby/buildkit/frontend/dockerfile/builder" + "github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb" "github.com/moby/buildkit/frontend/dockerfile/dockerignore" bkgw "github.com/moby/buildkit/frontend/gateway/client" bkpb "github.com/moby/buildkit/solver/pb" @@ -34,6 +36,7 @@ type Pipeline struct { s Solver state llb.State result bkgw.Reference + image dockerfile2llb.Image computed *compiler.Value } @@ -61,6 +64,10 @@ func (p *Pipeline) FS() fs.FS { return NewBuildkitFS(p.result) } +func (p *Pipeline) ImageConfig() dockerfile2llb.Image { + return p.image +} + func (p *Pipeline) Computed() *compiler.Value { return p.computed } @@ -255,13 +262,9 @@ func (p *Pipeline) Copy(ctx context.Context, op *compiler.Value, st llb.State) ( if err := from.Do(ctx, op.Lookup("from")); err != nil { return st, err } - fromResult, err := from.Result() - if err != nil { - return st, err - } return st.File( llb.Copy( - fromResult, + from.State(), src, dest, // FIXME: allow more configurable llb options @@ -455,10 +458,6 @@ func (p *Pipeline) mount(ctx context.Context, dest string, mnt *compiler.Value) if err := from.Do(ctx, mnt.Lookup("from")); err != nil { return nil, err } - fromResult, err := from.Result() - if err != nil { - return nil, err - } // possibly construct mount options for LLB from var mo []llb.MountOption // handle "path" option @@ -469,7 +468,7 @@ func (p *Pipeline) mount(ctx context.Context, dest string, mnt *compiler.Value) } mo = append(mo, llb.SourcePath(mps)) } - return llb.AddMount(dest, fromResult, mo...), nil + return llb.AddMount(dest, from.State(), mo...), nil } func (p *Pipeline) Export(ctx context.Context, op *compiler.Value, st llb.State) (llb.State, error) { @@ -559,7 +558,8 @@ func (p *Pipeline) Load(ctx context.Context, op *compiler.Value, st llb.State) ( if err := from.Do(ctx, op.Lookup("from")); err != nil { return st, err } - return from.Result() + p.image = from.ImageConfig() + return from.State(), nil } func (p *Pipeline) FetchContainer(ctx context.Context, op *compiler.Value, st llb.State) (llb.State, error) { @@ -581,15 +581,19 @@ func (p *Pipeline) FetchContainer(ctx context.Context, op *compiler.Value, st ll ) // Load image metadata and convert to to LLB. - // FIXME: metadata MUST be injected back into the gateway result - // FIXME: there are unhandled sections of the image config - image, err := p.s.ResolveImageConfig(ctx, ref.String(), llb.ResolveImageConfigOpt{ + p.image, err = p.s.ResolveImageConfig(ctx, ref.String(), llb.ResolveImageConfigOpt{ LogName: p.vertexNamef("load metadata for %s", ref.String()), }) if err != nil { return st, err } + return applyImageToState(p.image, st), nil +} + +// applyImageToState converts an image config into LLB instructions +func applyImageToState(image dockerfile2llb.Image, st llb.State) llb.State { + // FIXME: there are unhandled sections of the image config for _, env := range image.Config.Env { k, v := parseKeyValue(env) st = st.AddEnv(k, v) @@ -600,7 +604,7 @@ func (p *Pipeline) FetchContainer(ctx context.Context, op *compiler.Value, st ll if image.Config.User != "" { st = st.User(image.Config.User) } - return st, nil + return st } func parseKeyValue(env string) (string, string) { @@ -626,12 +630,7 @@ func (p *Pipeline) PushContainer(ctx context.Context, op *compiler.Value, st llb // Add the default tag "latest" to a reference if it only has a repo name. ref = reference.TagNameOnly(ref) - pushSt, err := p.Result() - if err != nil { - return st, err - } - - _, err = p.s.Export(ctx, pushSt, bk.ExportEntry{ + _, err = p.s.Export(ctx, p.State(), &p.image, bk.ExportEntry{ Type: bk.ExporterImage, Attrs: map[string]string{ "name": ref.String(), @@ -692,11 +691,7 @@ func (p *Pipeline) DockerBuild(ctx context.Context, op *compiler.Value, st llb.S if err := from.Do(ctx, dockerContext); err != nil { return st, err } - fromResult, err := from.Result() - if err != nil { - return st, err - } - contextDef, err = p.s.Marshal(ctx, fromResult) + contextDef, err = p.s.Marshal(ctx, from.State()) if err != nil { return st, err } @@ -722,48 +717,77 @@ func (p *Pipeline) DockerBuild(ctx context.Context, op *compiler.Value, st llb.S } } + opts, err := dockerBuildOpts(op) + if err != nil { + return st, err + } + req := bkgw.SolveRequest{ Frontend: "dockerfile.v0", - FrontendOpt: make(map[string]string), + FrontendOpt: opts, FrontendInputs: map[string]*bkpb.Definition{ dockerfilebuilder.DefaultLocalNameContext: contextDef, dockerfilebuilder.DefaultLocalNameDockerfile: dockerfileDef, }, } + res, err := p.s.SolveRequest(ctx, req) + if err != nil { + return st, err + } + if meta, ok := res.Metadata[exptypes.ExporterImageConfigKey]; ok { + if err := json.Unmarshal(meta, &p.image); err != nil { + return st, fmt.Errorf("failed to unmarshal image config: %w", err) + } + } + + ref, err := res.SingleRef() + if err != nil { + return st, err + } + st, err = ref.ToState() + if err != nil { + return st, err + } + return applyImageToState(p.image, st), nil +} + +func dockerBuildOpts(op *compiler.Value) (map[string]string, error) { + opts := map[string]string{} + if dockerfilePath := op.Lookup("dockerfilePath"); dockerfilePath.Exists() { filename, err := dockerfilePath.String() if err != nil { - return st, err + return nil, err } - req.FrontendOpt["filename"] = filename + opts["filename"] = filename } if buildArgs := op.Lookup("buildArg"); buildArgs.Exists() { fields, err := buildArgs.Fields() if err != nil { - return st, err + return nil, err } for _, buildArg := range fields { v, err := buildArg.Value.String() if err != nil { - return st, err + return nil, err } - req.FrontendOpt["build-arg:"+buildArg.Label] = v + opts["build-arg:"+buildArg.Label] = v } } if labels := op.Lookup("label"); labels.Exists() { fields, err := labels.Fields() if err != nil { - return st, err + return nil, err } for _, label := range fields { s, err := label.Value.String() if err != nil { - return st, err + return nil, err } - req.FrontendOpt["label:"+label.Label] = s + opts["label:"+label.Label] = s } } @@ -771,30 +795,26 @@ func (p *Pipeline) DockerBuild(ctx context.Context, op *compiler.Value, st llb.S p := []string{} list, err := platforms.List() if err != nil { - return st, err + return nil, err } for _, platform := range list { s, err := platform.String() if err != nil { - return st, err + return nil, err } p = append(p, s) } if len(p) > 0 { - req.FrontendOpt["platform"] = strings.Join(p, ",") + opts["platform"] = strings.Join(p, ",") } if len(p) > 1 { - req.FrontendOpt["multi-platform"] = "true" + opts["multi-platform"] = "true" } } - res, err := p.s.SolveRequest(ctx, req) - if err != nil { - return st, err - } - return res.ToState() + return opts, nil } func (p *Pipeline) WriteFile(ctx context.Context, op *compiler.Value, st llb.State) (llb.State, error) { diff --git a/dagger/solver.go b/dagger/solver.go index 8449a94e..c20fef27 100644 --- a/dagger/solver.go +++ b/dagger/solver.go @@ -9,6 +9,7 @@ import ( bk "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb" bkgw "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/session" @@ -63,14 +64,13 @@ func (s Solver) ResolveImageConfig(ctx context.Context, ref string, opts llb.Res } // Solve will block until the state is solved and returns a Reference. -func (s Solver) SolveRequest(ctx context.Context, req bkgw.SolveRequest) (bkgw.Reference, error) { +func (s Solver) SolveRequest(ctx context.Context, req bkgw.SolveRequest) (*bkgw.Result, error) { // call solve res, err := s.gw.Solve(ctx, req) if err != nil { return nil, bkCleanError(err) } - // always use single reference (ignore multiple outputs & metadata) - return res.SingleRef() + return res, nil } // Solve will block until the state is solved and returns a Reference. @@ -93,7 +93,7 @@ func (s Solver) Solve(ctx context.Context, st llb.State) (bkgw.Reference, error) Msg("solving") // call solve - return s.SolveRequest(ctx, bkgw.SolveRequest{ + res, err := s.SolveRequest(ctx, bkgw.SolveRequest{ Definition: def, // makes Solve() to block until LLB graph is solved. otherwise it will @@ -101,13 +101,18 @@ func (s Solver) Solve(ctx context.Context, st llb.State) (bkgw.Reference, error) // will be evaluated on export or if you access files on it. Evaluate: true, }) + if err != nil { + return nil, err + } + + return res.SingleRef() } // Export will export `st` to `output` // FIXME: this is currently impleneted as a hack, starting a new Build session // within buildkit from the Control API. Ideally the Gateway API should allow to // Export directly. -func (s Solver) Export(ctx context.Context, st llb.State, output bk.ExportEntry) (*bk.SolveResponse, error) { +func (s Solver) Export(ctx context.Context, st llb.State, img *dockerfile2llb.Image, output bk.ExportEntry) (*bk.SolveResponse, error) { def, err := s.Marshal(ctx, st) if err != nil { return nil, err @@ -131,9 +136,24 @@ func (s Solver) Export(ctx context.Context, st llb.State, output bk.ExportEntry) }() return s.control.Build(ctx, opts, "", func(ctx context.Context, c bkgw.Client) (*bkgw.Result, error) { - return c.Solve(ctx, bkgw.SolveRequest{ + res, err := c.Solve(ctx, bkgw.SolveRequest{ Definition: def, }) + if err != nil { + return nil, err + } + + // Attach the image config if provided + if img != nil { + config, err := json.Marshal(img) + if err != nil { + return nil, fmt.Errorf("failed to marshal image config: %w", err) + } + + res.AddMeta(exptypes.ExporterImageConfigKey, config) + } + + return res, nil }, ch) } diff --git a/tests/llb/dockerbuild/main.cue b/tests/llb/dockerbuild/main.cue index 4cce6d8a..27843933 100644 --- a/tests/llb/dockerbuild/main.cue +++ b/tests/llb/dockerbuild/main.cue @@ -91,3 +91,32 @@ TestBuildPlatform: #up: [ platforms: ["linux/amd64"] }, ] + +TestImageMetadata: #up: [ + op.#DockerBuild & { + dockerfile: """ + FROM alpine:latest@sha256:ab00606a42621fb68f2ed6ad3c88be54397f981a7b70a79db3d1172b11c4367d + ENV CHECK foobar + ENV DOUBLECHECK test + """ + }, + op.#Exec & { + args: ["sh", "-c", #""" + env + test "$CHECK" = "foobar" + """#] + }, +] + +// Make sure the metadata is carried over with a `Load` +TestImageMetadataIndirect: #up: [ + op.#Load & { + from: TestImageMetadata + }, + op.#Exec & { + args: ["sh", "-c", #""" + env + test "$DOUBLECHECK" = "test" + """#] + }, +] diff --git a/tests/llb/push-container/main.cue b/tests/llb/push-container/main.cue index b094d694..d73da547 100644 --- a/tests/llb/push-container/main.cue +++ b/tests/llb/push-container/main.cue @@ -54,3 +54,79 @@ TestPushContainer: { }, ] } + +// Ensures image metadata is preserved in a push +TestPushContainerMetadata: { + // Generate a random number + random: { + string + #up: [ + op.#Load & {from: alpine.#Image}, + op.#Exec & { + args: ["sh", "-c", "echo -n $RANDOM > /rand"] + }, + op.#Export & { + source: "/rand" + }, + ] + } + + // `docker build` using an `ENV` and push the image + push: { + ref: "daggerio/ci-test:\(random)-dockerbuild" + #up: [ + op.#DockerBuild & { + dockerfile: #""" + FROM alpine:latest@sha256:ab00606a42621fb68f2ed6ad3c88be54397f981a7b70a79db3d1172b11c4367d + ENV CHECK \#(random) + """# + }, + op.#PushContainer & { + "ref": ref + }, + ] + } + + // Pull the image down and make sure the ENV is preserved + check: #up: [ + op.#FetchContainer & { + ref: push.ref + }, + op.#Exec & { + args: [ + "sh", "-c", #""" + env + test "$CHECK" = "\#(random)" + """#, + ] + }, + ] + + // Do a FetchContainer followed by a PushContainer, make sure + // the ENV is preserved + pullPush: { + ref: "daggerio/ci-test:\(random)-pullpush" + + #up: [ + op.#FetchContainer & { + ref: push.ref + }, + op.#PushContainer & { + "ref": ref + }, + ] + } + + pullPushCheck: #up: [ + op.#FetchContainer & { + ref: pullPush.ref + }, + op.#Exec & { + args: [ + "sh", "-c", #""" + test "$CHECK" = "\#(random)" + """#, + ] + }, + ] +}