Merge pull request #318 from dagger/docker-image-metadata

Proper support for Docker Image metadata

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
+	p.image, err = p.s.ResolveImageConfig(ctx, ref.String(), llb.ResolveImageConfigOpt{
-	// FIXME: there are unhandled sections of the image config
-	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()
+	_, err = p.s.Export(ctx, p.State(), &p.image, bk.ExportEntry{
-	if err != nil {
-		return st, err
-	}
-
-	_, err = p.s.Export(ctx, pushSt, 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()
+		contextDef, err = p.s.Marshal(ctx, from.State())
-		if err != nil {
-			return st, err
-		}
-		contextDef, err = p.s.Marshal(ctx, fromResult)
 		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)
+	return opts, nil
-	if err != nil {
-		return st, err
-	}
-	return res.ToState()
 }
 
 func (p *Pipeline) WriteFile(ctx context.Context, op *compiler.Value, st llb.State) (llb.State, error) {

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, nil
-	return res.SingleRef()
 }
 
 // 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)
 }
 

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"
+			"""#]
+	},
+]

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)"
+                """#,
+			]
+		},
+	]
+}