From 9d597fe6214f4154d8943b8464052f4fc9047fa4 Mon Sep 17 00:00:00 2001
From: a-palchikov <deemok@gmail.com>
Date: Wed, 30 Mar 2022 15:35:47 +0200
Subject: [PATCH] Use alternative exporter metadata representation to
 accommodate for mulitple exporters with the same ID

Signed-off-by: Dmitri Shelenin <deemok@gmail.com>
---
 client/client_test.go                     | 67 ++++++++++++++++-------
 client/exporters.go                       |  7 +--
 client/graph.go                           |  4 +-
 client/solve.go                           |  4 +-
 cmd/buildctl/build.go                     | 10 ++--
 cmd/buildctl/build_test.go                | 16 +++++-
 control/control.go                        |  8 +--
 exporter/containerimage/export.go         |  5 +-
 exporter/containerimage/exptypes/types.go |  1 +
 exporter/exporter.go                      |  1 -
 exporter/local/export.go                  |  4 --
 exporter/oci/export.go                    |  7 +--
 exporter/tar/export.go                    |  4 --
 solver/llbsolver/solver.go                |  4 +-
 14 files changed, 79 insertions(+), 63 deletions(-)

diff --git a/client/client_test.go b/client/client_test.go
index a51dc8730791d..dafa341137c5f 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -135,6 +135,7 @@ func TestIntegration(t *testing.T) {
 		testTarExporterSymlink,
 		testMultipleRegistryCacheImportExport,
 		testMultipleExporters,
+		testPreventsMultipleExportWithSameExporter,
 		testSourceMap,
 		testSourceMapFromRef,
 		testLazyImagePush,
@@ -259,7 +260,6 @@ func testBridgeNetworking(t *testing.T, sb integration.Sandbox) {
 	_, err = c.Solve(sb.Context(), def, SolveOpt{}, nil)
 	require.Error(t, err)
 }
-
 func testHostNetworking(t *testing.T, sb integration.Sandbox) {
 	if os.Getenv("BUILDKIT_RUN_NETWORK_INTEGRATION_TESTS") == "" {
 		t.SkipNow()
@@ -1894,7 +1894,7 @@ func testUser(t *testing.T, sb integration.Sandbox) {
 	checkAllReleasable(t, c, sb, true)
 }
 
-func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
+func testPreventsMultipleExportWithSameExporter(t *testing.T, sb integration.Sandbox) {
 	integration.SkipIfDockerd(t, sb, "multiple exporters")
 	requiresLinux(t)
 
@@ -1919,7 +1919,21 @@ func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
 			},
 		},
 	}, nil)
-	require.Error(t, err)
+	require.Errorf(t, err, "using multiple ExporterLocal is not supported")
+}
+
+func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
+	integration.SkipIfDockerd(t, sb, "multiple exporters")
+	requiresLinux(t)
+
+	c, err := New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	def, err := llb.Image("busybox").Marshal(context.TODO())
+	require.NoError(t, err)
+
+	destDir := t.TempDir()
 
 	registry, err := sb.NewRegistry()
 	if errors.Is(err, integration.ErrRequirements) {
@@ -1927,23 +1941,32 @@ func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
 	}
 	require.NoError(t, err)
 
-	target := registry + "/buildkit/build/exporter:image"
+	target1, target2 := registry+"/buildkit/build/exporter:image",
+		registry+"/buildkit/build/alternative:image"
 
-	_, err = c.Solve(sb.Context(), def, SolveOpt{
+	resp, err := c.Solve(sb.Context(), def, SolveOpt{
 		Exports: []ExportEntry{
 			{
 				Type:      ExporterLocal,
-				OutputDir: destDir1,
+				OutputDir: destDir,
 			},
 			{
 				Type: ExporterImage,
 				Attrs: map[string]string{
-					"name": target,
+					"name": target1,
+				},
+			},
+			{
+				Type: ExporterImage,
+				Attrs: map[string]string{
+					"name": target2,
 				},
 			},
 		},
 	}, nil)
 	require.NoError(t, err)
+
+	validateMetadataContains(t, resp.ExportersResponse, "image.name", []string{target1, target2})
 }
 
 func testOCIExporter(t *testing.T, sb integration.Sandbox) {
@@ -2498,7 +2521,7 @@ func testBuildExportWithUncompressed(t *testing.T, sb integration.Sandbox) {
 	dt, err := content.ReadBlob(ctx, img.ContentStore(), img.Target())
 	require.NoError(t, err)
 
-	mfst := struct {
+	var mfst = struct {
 		MediaType string `json:"mediaType,omitempty"`
 		ocispecs.Manifest
 	}{}
@@ -2745,7 +2768,6 @@ func testPullZstdImage(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, err)
 	require.Equal(t, dt, []byte("zstd"))
 }
-
 func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) {
 	integration.SkipIfDockerd(t, sb, "direct push")
 	requiresLinux(t)
@@ -2886,7 +2908,7 @@ func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) {
 	dt, err = content.ReadBlob(ctx, img.ContentStore(), img.Target())
 	require.NoError(t, err)
 
-	mfst := struct {
+	var mfst = struct {
 		MediaType string `json:"mediaType,omitempty"`
 		ocispecs.Manifest
 	}{}
@@ -3725,8 +3747,7 @@ func testBasicCacheImportExport(t *testing.T, sb integration.Sandbox, cacheOptio
 			{
 				Type:      ExporterLocal,
 				OutputDir: destDir,
-			},
-		},
+			}},
 		CacheImports: cacheOptionsEntryImport,
 	}, nil)
 	require.NoError(t, err)
@@ -4588,7 +4609,7 @@ func testSourceMapFromRef(t *testing.T, sb integration.Sandbox) {
 
 	frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
 		st := llb.Scratch().File(
-			llb.Mkdir("foo/bar", 0600), // fails because /foo doesn't exist
+			llb.Mkdir("foo/bar", 0600), //fails because /foo doesn't exist
 			sm.Location([]*pb.Range{{Start: pb.Position{Line: 3, Character: 1}}}),
 		)
 
@@ -5821,6 +5842,14 @@ func makeSSHAgentSock(agent agent.Agent) (p string, cleanup func() error, err er
 	}, nil
 }
 
+func validateMetadataContains(t *testing.T, mds []map[string]string, key string, expectedValues []string) {
+	var values []string
+	for _, md := range mds {
+		values = append(values, md[key])
+	}
+	require.ElementsMatch(t, values, expectedValues)
+}
+
 type server struct {
 	l net.Listener
 }
@@ -5848,10 +5877,8 @@ func (*secModeInsecure) UpdateConfigFile(in string) string {
 	return in + "\n\ninsecure-entitlements = [\"security.insecure\"]\n"
 }
 
-var (
-	securitySandbox  integration.ConfigUpdater = &secModeSandbox{}
-	securityInsecure integration.ConfigUpdater = &secModeInsecure{}
-)
+var securitySandbox integration.ConfigUpdater = &secModeSandbox{}
+var securityInsecure integration.ConfigUpdater = &secModeInsecure{}
 
 type netModeHost struct{}
 
@@ -5865,10 +5892,8 @@ func (*netModeDefault) UpdateConfigFile(in string) string {
 	return in
 }
 
-var (
-	hostNetwork    integration.ConfigUpdater = &netModeHost{}
-	defaultNetwork integration.ConfigUpdater = &netModeDefault{}
-)
+var hostNetwork integration.ConfigUpdater = &netModeHost{}
+var defaultNetwork integration.ConfigUpdater = &netModeDefault{}
 
 func fixedWriteCloser(wc io.WriteCloser) func(map[string]string) (io.WriteCloser, error) {
 	return func(map[string]string) (io.WriteCloser, error) {
diff --git a/client/exporters.go b/client/exporters.go
index 10ee7507538de..4f0e13a77b6a7 100644
--- a/client/exporters.go
+++ b/client/exporters.go
@@ -1,15 +1,10 @@
 package client
 
 const (
-	// TODO: keep these in sync with the corresponding exporter Name constants
+	// Keep these in sync with the corresponding exporter Name constants
 	ExporterImage  = "image"
 	ExporterLocal  = "local"
 	ExporterTar    = "tar"
 	ExporterOCI    = "oci"
 	ExporterDocker = "docker"
-
-	// ExporterCommon defines the name of the exporter
-	// with metadata aggregated from alternative sources
-	// like the cache exporter or the frontend
-	ExporterCommon = "<auto>"
 )
diff --git a/client/graph.go b/client/graph.go
index e13ab9d799130..d9f2d2cc96aef 100644
--- a/client/graph.go
+++ b/client/graph.go
@@ -56,6 +56,6 @@ type SolveStatus struct {
 type SolveResponse struct {
 	// ExporterResponse is also used for CacheExporter
 	ExporterResponse map[string]string
-	// ExportersResponse maps exporter name -> metadata
-	ExportersResponse map[string]map[string]string
+	// ExportersResponse lists metadata from output exporters
+	ExportersResponse []map[string]string
 }
diff --git a/client/solve.go b/client/solve.go
index db481fc8cb703..e170f3ad9ecfd 100644
--- a/client/solve.go
+++ b/client/solve.go
@@ -230,9 +230,9 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
 			return errors.Wrap(err, "failed to solve")
 		}
 
-		exportersResponse := make(map[string]map[string]string)
+		exportersResponse := make([]map[string]string, 0, len(resp.ExportersResponse))
 		for _, resp := range resp.ExportersResponse {
-			exportersResponse[resp.Name] = resp.Response
+			exportersResponse = append(exportersResponse, resp.Response)
 		}
 
 		result = &SolveResponse{
diff --git a/cmd/buildctl/build.go b/cmd/buildctl/build.go
index bb6eda47f79b4..6f01fe03c3dc8 100644
--- a/cmd/buildctl/build.go
+++ b/cmd/buildctl/build.go
@@ -300,7 +300,7 @@ func buildAction(clicontext *cli.Context) error {
 		}
 
 		metadataFile := clicontext.String("metadata-file")
-		if metadataFile != "" && len(resp.ExportersResponse) != 0 {
+		if metadataFile != "" && (resp.ExporterResponse != nil || len(resp.ExportersResponse) != 0) {
 			if err := writeMetadataFile(metadataFile, resp); err != nil {
 				return err
 			}
@@ -326,11 +326,11 @@ func writeMetadataFile(filename string, resp *client.SolveResponse) (err error)
 		}
 		return continuity.AtomicWriteFile(filename, b, 0666)
 	}
-	result := map[string]map[string]interface{}{
-		client.ExporterCommon: marshalExporterMetadata(resp.ExporterResponse),
+	result := []map[string]interface{}{
+		marshalExporterMetadata(resp.ExporterResponse),
 	}
-	for name, md := range resp.ExportersResponse {
-		result[name] = marshalExporterMetadata(md)
+	for _, md := range resp.ExportersResponse {
+		result = append(result, marshalExporterMetadata(md))
 	}
 	b, err := json.MarshalIndent(result, "", "  ")
 	if err != nil {
diff --git a/cmd/buildctl/build_test.go b/cmd/buildctl/build_test.go
index c4f1cb8012ae4..73dbe7f6c94fc 100644
--- a/cmd/buildctl/build_test.go
+++ b/cmd/buildctl/build_test.go
@@ -143,12 +143,11 @@ func testBuildMetadataFile(t *testing.T, sb integration.Sandbox) {
 	metadataBytes, err := os.ReadFile(metadataFile)
 	require.NoError(t, err)
 
-	var metadata map[string]map[string]interface{}
+	var metadata []map[string]interface{}
 	err = json.Unmarshal(metadataBytes, &metadata)
 	require.NoError(t, err)
 
-	require.Contains(t, metadata, containerimage.Name)
-	exporterMetadata := metadata[containerimage.Name]
+	exporterMetadata := validateExporterMetadata(t, metadata, containerimage.Name)
 
 	require.Contains(t, exporterMetadata, "image.name")
 	require.Equal(t, imageName, exporterMetadata["image.name"])
@@ -183,6 +182,17 @@ func testBuildMetadataFile(t *testing.T, sb integration.Sandbox) {
 	}
 }
 
+func validateExporterMetadata(t *testing.T, metadata []map[string]interface{}, exporter string) (result map[string]interface{}) {
+	require.NotEmpty(t, metadata)
+	for _, md := range metadata {
+		if typ, ok := md[exptypes.ExporterTypeKey]; ok && typ == exporter {
+			return md
+		}
+	}
+	require.Failf(t, "required exporter metadata not found in the list", "exporter %s", exporter)
+	return nil
+}
+
 func marshal(ctx context.Context, st llb.State) (io.Reader, error) {
 	def, err := st.Marshal(ctx)
 	if err != nil {
diff --git a/control/control.go b/control/control.go
index 75b9fa678ce74..2e4cb51f0c69a 100644
--- a/control/control.go
+++ b/control/control.go
@@ -6,20 +6,20 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/moby/buildkit/util/bklog"
-
 	controlapi "github.com/moby/buildkit/api/services/control"
 	apitypes "github.com/moby/buildkit/api/types"
 	"github.com/moby/buildkit/cache/remotecache"
 	"github.com/moby/buildkit/client"
 	controlgateway "github.com/moby/buildkit/control/gateway"
 	"github.com/moby/buildkit/exporter"
+	"github.com/moby/buildkit/exporter/containerimage/exptypes"
 	"github.com/moby/buildkit/frontend"
 	"github.com/moby/buildkit/session"
 	"github.com/moby/buildkit/session/grpchijack"
 	"github.com/moby/buildkit/solver"
 	"github.com/moby/buildkit/solver/llbsolver"
 	"github.com/moby/buildkit/solver/pb"
+	"github.com/moby/buildkit/util/bklog"
 	"github.com/moby/buildkit/util/imageutil"
 	"github.com/moby/buildkit/util/throttle"
 	"github.com/moby/buildkit/util/tracing/transform"
@@ -326,9 +326,9 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*
 	}
 
 	resps := make([]*controlapi.ExporterResponse, 0, len(resp.ExportersResponse))
-	for name, md := range resp.ExportersResponse {
+	for _, md := range resp.ExportersResponse {
 		resps = append(resps, &controlapi.ExporterResponse{
-			Name:     name,
+			Name:     md[exptypes.ExporterTypeKey],
 			Response: md,
 		})
 	}
diff --git a/exporter/containerimage/export.go b/exporter/containerimage/export.go
index cc58f6f39b2ae..9c5902f13b800 100644
--- a/exporter/containerimage/export.go
+++ b/exporter/containerimage/export.go
@@ -244,10 +244,6 @@ func (e *imageExporterInstance) Name() string {
 	return "exporting to image"
 }
 
-func (e *imageExporterInstance) ID() string {
-	return Name
-}
-
 func (e *imageExporterInstance) Config() exporter.Config {
 	return exporter.Config{
 		Compression: e.compression(),
@@ -378,6 +374,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source,
 		return nil, err
 	}
 	resp[exptypes.ExporterImageDescriptorKey] = base64.StdEncoding.EncodeToString(dtdesc)
+	resp[exptypes.ExporterTypeKey] = Name
 
 	return resp, nil
 }
diff --git a/exporter/containerimage/exptypes/types.go b/exporter/containerimage/exptypes/types.go
index a18d660a5c4ab..09464ce3793e4 100644
--- a/exporter/containerimage/exptypes/types.go
+++ b/exporter/containerimage/exptypes/types.go
@@ -13,6 +13,7 @@ const (
 	ExporterInlineCache          = "containerimage.inlinecache"
 	ExporterBuildInfo            = "containerimage.buildinfo"
 	ExporterPlatformsKey         = "refs.platforms"
+	ExporterTypeKey              = "exporter.type"
 )
 
 type Platforms struct {
diff --git a/exporter/exporter.go b/exporter/exporter.go
index ad5e01fa13449..610481b710f64 100644
--- a/exporter/exporter.go
+++ b/exporter/exporter.go
@@ -13,7 +13,6 @@ type Exporter interface {
 
 type ExporterInstance interface {
 	Name() string
-	ID() string
 	Config() Config
 	Export(ctx context.Context, src Source, sessionID string) (map[string]string, error)
 }
diff --git a/exporter/local/export.go b/exporter/local/export.go
index 8d9dbd862a577..045b9d28a5686 100644
--- a/exporter/local/export.go
+++ b/exporter/local/export.go
@@ -47,10 +47,6 @@ func (e *localExporterInstance) Name() string {
 	return "exporting to client"
 }
 
-func (e *localExporterInstance) ID() string {
-	return Name
-}
-
 func (e *localExporter) Config() exporter.Config {
 	return exporter.Config{}
 }
diff --git a/exporter/oci/export.go b/exporter/oci/export.go
index 9525f1ffe7ef4..009920b4c6bfb 100644
--- a/exporter/oci/export.go
+++ b/exporter/oci/export.go
@@ -179,10 +179,6 @@ func (e *imageExporterInstance) Name() string {
 	return "exporting to oci image format"
 }
 
-func (e *imageExporterInstance) ID() string {
-	return string(e.opt.Variant)
-}
-
 func (e *imageExporterInstance) Config() exporter.Config {
 	return exporter.Config{
 		Compression: e.compression(),
@@ -237,6 +233,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source,
 
 	resp := make(map[string]string)
 
+	resp[exptypes.ExporterTypeKey] = string(e.opt.Variant)
 	resp[exptypes.ExporterImageDigestKey] = desc.Digest.String()
 	if v, ok := desc.Annotations[exptypes.ExporterConfigDigestKey]; ok {
 		resp[exptypes.ExporterImageConfigDigestKey] = v
@@ -357,7 +354,7 @@ func normalizedNames(name string) ([]string, error) {
 		return nil, nil
 	}
 	names := strings.Split(name, ",")
-	tagNames := make([]string, len(names))
+	var tagNames = make([]string, len(names))
 	for i, name := range names {
 		parsed, err := reference.ParseNormalizedNamed(name)
 		if err != nil {
diff --git a/exporter/tar/export.go b/exporter/tar/export.go
index 464b69bd9d9fa..af0367db319a8 100644
--- a/exporter/tar/export.go
+++ b/exporter/tar/export.go
@@ -66,10 +66,6 @@ func (e *localExporterInstance) Name() string {
 	return "exporting to client"
 }
 
-func (e *localExporterInstance) ID() string {
-	return Name
-}
-
 func (e *localExporterInstance) Config() exporter.Config {
 	return exporter.Config{}
 }
diff --git a/solver/llbsolver/solver.go b/solver/llbsolver/solver.go
index 7fecd6f517863..1a4196041e8a8 100644
--- a/solver/llbsolver/solver.go
+++ b/solver/llbsolver/solver.go
@@ -188,7 +188,7 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
 		}
 	}
 
-	exportersResponse := make(map[string]map[string]string)
+	exportersResponse := []map[string]string{}
 	for _, exp := range out.Exporters {
 		inp := exporter.Source{
 			Metadata: res.Metadata,
@@ -262,7 +262,7 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
 				return err
 			}
 			if resp != nil {
-				exportersResponse[exp.ID()] = resp
+				exportersResponse = append(exportersResponse, resp)
 			}
 			return nil
 		}); err != nil {