Merge pull request #575 from ijc/frontend-set-export-name

exporters: allow frontend control of the exported image name

client/client_test.go

@@ -53,6 +53,7 @@ func TestClientIntegration(t *testing.T) {
 		testUser,
 		testOCIExporter,
 		testWhiteoutParentDir,
+		testFrontendImageNaming,
 		testDuplicateWhiteouts,
 		testSchema1Image,
 		testMountWithNoSource,
@@ -125,6 +126,156 @@ func testNetworkMode(t *testing.T, sb integration.Sandbox) {
 	require.Contains(t, err.Error(), "network.host is not allowed")
 }
 
+func testFrontendImageNaming(t *testing.T, sb integration.Sandbox) {
+	requiresLinux(t)
+	t.Parallel()
+	c, err := New(context.TODO(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	registry, err := sb.NewRegistry()
+	if errors.Cause(err) == integration.ErrorRequirements {
+		t.Skip(err.Error())
+	}
+	require.NoError(t, err)
+
+	checkImageName := map[string]func(out, imageName string, exporterResponse map[string]string){
+		ExporterOCI: func(out, imageName string, exporterResponse map[string]string) {
+			// Nothing to check
+			return
+		},
+		ExporterDocker: func(out, imageName string, exporterResponse map[string]string) {
+			require.Contains(t, exporterResponse, "image.name")
+			require.Equal(t, exporterResponse["image.name"], "docker.io/library/"+imageName)
+
+			dt, err := ioutil.ReadFile(out)
+			require.NoError(t, err)
+
+			m, err := testutil.ReadTarToMap(dt, false)
+			require.NoError(t, err)
+
+			_, ok := m["oci-layout"]
+			require.True(t, ok)
+
+			var index ocispec.Index
+			err = json.Unmarshal(m["index.json"].Data, &index)
+			require.NoError(t, err)
+			require.Equal(t, 2, index.SchemaVersion)
+			require.Equal(t, 1, len(index.Manifests))
+
+			var dockerMfst []struct {
+				RepoTags []string
+			}
+			err = json.Unmarshal(m["manifest.json"].Data, &dockerMfst)
+			require.NoError(t, err)
+			require.Equal(t, 1, len(dockerMfst))
+			require.Equal(t, 1, len(dockerMfst[0].RepoTags))
+			require.Equal(t, "docker.io/library/"+imageName, dockerMfst[0].RepoTags[0])
+		},
+		ExporterImage: func(_, imageName string, exporterResponse map[string]string) {
+			require.Contains(t, exporterResponse, "image.name")
+			require.Equal(t, exporterResponse["image.name"], imageName)
+
+			// check if we can pull (requires containerd)
+			var cdAddress string
+			if cd, ok := sb.(interface {
+				ContainerdAddress() string
+			}); !ok {
+				return
+			} else {
+				cdAddress = cd.ContainerdAddress()
+			}
+
+			// TODO: make public pull helper function so this can be checked for standalone as well
+
+			client, err := containerd.New(cdAddress)
+			require.NoError(t, err)
+			defer client.Close()
+
+			ctx := namespaces.WithNamespace(context.Background(), "buildkit")
+
+			// check image in containerd
+			_, err = client.ImageService().Get(ctx, imageName)
+			require.NoError(t, err)
+
+			// deleting image should release all content
+			err = client.ImageService().Delete(ctx, imageName, images.SynchronousDelete())
+			require.NoError(t, err)
+
+			checkAllReleasable(t, c, sb, true)
+
+			_, err = client.Pull(ctx, imageName)
+			require.NoError(t, err)
+
+			err = client.ImageService().Delete(ctx, imageName, images.SynchronousDelete())
+			require.NoError(t, err)
+		},
+	}
+
+	// A caller provided name takes precedence over one returned by the frontend. Iterate over both options.
+	for _, winner := range []string{"frontend", "caller"} {
+		winner := winner // capture loop variable.
+
+		// The double layer of `t.Run` here is required so
+		// that the inner-most tests (with the actual
+		// functionality) have definitely completed before the
+		// sandbox and registry cleanups (defered above) are run.
+		t.Run(winner, func(t *testing.T) {
+			for _, exp := range []string{ExporterOCI, ExporterDocker, ExporterImage} {
+				exp := exp // capture loop variable.
+				t.Run(exp, func(t *testing.T) {
+					destDir, err := ioutil.TempDir("", "buildkit")
+					require.NoError(t, err)
+					defer os.RemoveAll(destDir)
+
+					so := SolveOpt{
+						Exporter:      exp,
+						ExporterAttrs: map[string]string{},
+					}
+
+					out := filepath.Join(destDir, "out.tar")
+
+					imageName := "image-" + exp + "-fe:latest"
+
+					switch exp {
+					case ExporterOCI:
+						t.Skip("oci exporter does not support named images")
+					case ExporterDocker:
+						outW, err := os.Create(out)
+						require.NoError(t, err)
+						so.ExporterOutput = outW
+					case ExporterImage:
+						imageName = registry + "/" + imageName
+						so.ExporterAttrs["push"] = "true"
+					}
+
+					feName := imageName
+					switch winner {
+					case "caller":
+						feName = "loser:latest"
+						so.ExporterAttrs["name"] = imageName
+					case "frontend":
+						so.ExporterAttrs["name"] = "*"
+					}
+
+					frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
+						res := gateway.NewResult()
+						res.AddMeta("image.name", []byte(feName))
+						return res, nil
+					}
+
+					resp, err := c.Build(context.TODO(), so, "", frontend, nil)
+					require.NoError(t, err)
+
+					checkImageName[exp](out, imageName, resp.ExporterResponse)
+				})
+			}
+		})
+	}
+
+	checkAllReleasable(t, c, sb, true)
+}
+
 func testSecretMounts(t *testing.T, sb integration.Sandbox) {
 	t.Parallel()
 
@@ -534,12 +685,13 @@ func testOCIExporter(t *testing.T, sb integration.Sandbox) {
 		outW, err := os.Create(out)
 		require.NoError(t, err)
 		target := "example.com/buildkit/testoci:latest"
-
+		attrs := map[string]string{}
+		if exp == ExporterDocker {
+			attrs["name"] = target
+		}
 		_, err = c.Solve(context.TODO(), def, SolveOpt{
-			Exporter: exp,
-			ExporterAttrs: map[string]string{
-				"name": target,
-			},
+			Exporter:       exp,
+			ExporterAttrs:  attrs,
 			ExporterOutput: outW,
 		}, nil)
 		require.NoError(t, err)

exporter/containerimage/export.go

@@ -115,6 +115,12 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source)
 		e.opt.ImageWriter.ContentStore().Delete(context.TODO(), desc.Digest)
 	}()
 
+	resp := make(map[string]string)
+
+	if n, ok := src.Metadata["image.name"]; e.targetName == "*" && ok {
+		e.targetName = string(n)
+	}
+
 	if e.targetName != "" {
 		targetNames := strings.Split(e.targetName, ",")
 		for _, targetName := range targetNames {
@@ -143,9 +149,9 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source)
 				}
 			}
 		}
+		resp["image.name"] = e.targetName
 	}
 
-	return map[string]string{
-		"containerimage.digest": desc.Digest.String(),
-	}, nil
+	resp["containerimage.digest"] = desc.Digest.String()
+	return resp, nil
 }

exporter/oci/export.go

@@ -42,6 +42,17 @@ func New(opt Opt) (exporter.Exporter, error) {
 	return im, nil
 }
 
+func normalize(name string) (string, error) {
+	if name == "" {
+		return "", nil
+	}
+	parsed, err := reference.ParseNormalizedNamed(name)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to parse %s", name)
+	}
+	return reference.TagNameOnly(parsed).String(), nil
+}
+
 func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exporter.ExporterInstance, error) {
 	id := session.FromContext(ctx)
 	if id == "" {
@@ -61,11 +72,13 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp
 	for k, v := range opt {
 		switch k {
 		case keyImageName:
-			parsed, err := reference.ParseNormalizedNamed(v)
-			if err != nil {
-				return nil, errors.Wrapf(err, "failed to parse %s", v)
+			i.name = v
+			if i.name != "*" {
+				i.name, err = normalize(i.name)
+				if err != nil {
+					return nil, err
+				}
 			}
-			i.name = reference.TagNameOnly(parsed).String()
 		case ociTypes:
 			ot = new(bool)
 			if v == "" {
@@ -128,6 +141,18 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source)
 	}
 	desc.Annotations[ocispec.AnnotationCreated] = time.Now().UTC().Format(time.RFC3339)
 
+	resp := make(map[string]string)
+
+	if n, ok := src.Metadata["image.name"]; e.name == "*" && ok {
+		if e.name, err = normalize(string(n)); err != nil {
+			return nil, err
+		}
+	}
+
+	if e.name != "" {
+		resp["image.name"] = e.name
+	}
+
 	exp, err := getExporter(e.opt.Variant, e.name)
 	if err != nil {
 		return nil, err
@@ -142,7 +167,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source)
 		w.Close()
 		return nil, report(err)
 	}
-	return nil, report(w.Close())
+	return resp, report(w.Close())
 }
 
 func oneOffProgress(ctx context.Context, id string) func(err error) error {
@@ -165,6 +190,9 @@ func oneOffProgress(ctx context.Context, id string) func(err error) error {
 func getExporter(variant ExporterVariant, name string) (images.Exporter, error) {
 	switch variant {
 	case VariantOCI:
+		if name != "" {
+			return nil, errors.New("oci exporter cannot export named image")
+		}
 		return &oci.V1Exporter{}, nil
 	case VariantDocker:
 		return &dockerexporter.DockerExporter{Name: name}, nil