buildinfo: check nil attrs

Signed-off-by: CrazyMax <[email protected]>

client/build_test.go

@@ -2006,6 +2006,9 @@ func testClientGatewayFrontendAttrs(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, err)
 	defer c.Close()
 
+	fooattrval := "bar"
+	bazattrval := "fuu"
+
 	b := func(ctx context.Context, c client.Client) (*client.Result, error) {
 		st := llb.Image("busybox:latest").Run(
 			llb.ReadonlyRootFS(),
@@ -2018,13 +2021,17 @@ func testClientGatewayFrontendAttrs(t *testing.T, sb integration.Sandbox) {
 		res, err := c.Solve(ctx, client.SolveRequest{
 			Definition: def.ToPB(),
 			FrontendOpt: map[string]string{
-				"build-arg:foo": "bar",
+				"build-arg:foo": fooattrval,
 			},
 		})
 		require.Contains(t, res.Metadata, exptypes.ExporterBuildInfo)
 		var bi binfotypes.BuildInfo
 		require.NoError(t, json.Unmarshal(res.Metadata[exptypes.ExporterBuildInfo], &bi))
-		require.Equal(t, map[string]string{"build-arg:foo": "bar"}, bi.Attrs)
+		require.Contains(t, bi.Attrs, "build-arg:foo")
+		bi.Attrs["build-arg:baz"] = &bazattrval
+		bmbi, err := json.Marshal(bi)
+		require.NoError(t, err)
+		res.AddMeta(exptypes.ExporterBuildInfo, bmbi)
 		return res, err
 	}
 
@@ -2037,8 +2044,11 @@ func testClientGatewayFrontendAttrs(t *testing.T, sb integration.Sandbox) {
 
 	var bi binfotypes.BuildInfo
 	require.NoError(t, json.Unmarshal(decbi, &bi))
+
 	require.Contains(t, bi.Attrs, "build-arg:foo")
-	require.Equal(t, "bar", bi.Attrs["build-arg:foo"])
+	require.Equal(t, &fooattrval, bi.Attrs["build-arg:foo"])
+	require.Contains(t, bi.Attrs, "build-arg:baz")
+	require.Equal(t, &bazattrval, bi.Attrs["build-arg:baz"])
 
 	checkAllReleasable(t, c, sb, true)
 }

client/client_test.go

@@ -5090,7 +5090,8 @@ func testBuildInfoExporter(t *testing.T, sb integration.Sandbox) {
 	err = json.Unmarshal(decbi, &exbi)
 	require.NoError(t, err)
 
-	require.Equal(t, exbi.Attrs, map[string]string{"build-arg:foo": "bar"})
+	attrval := "bar"
+	require.Equal(t, exbi.Attrs, map[string]*string{"build-arg:foo": &attrval})
 	require.Equal(t, len(exbi.Sources), 1)
 	require.Equal(t, exbi.Sources[0].Type, binfotypes.SourceTypeDockerImage)
 	require.Equal(t, exbi.Sources[0].Ref, "docker.io/library/busybox:latest")
@@ -5174,8 +5175,9 @@ func testBuildInfoInline(t *testing.T, sb integration.Sandbox) {
 			require.NoError(t, err)
 
 			if tt.buildAttrs {
+				attrval := "bar"
 				require.Contains(t, bi.Attrs, "build-arg:foo")
-				require.Equal(t, bi.Attrs["build-arg:foo"], "bar")
+				require.Equal(t, bi.Attrs["build-arg:foo"], &attrval)
 			} else {
 				require.NotContains(t, bi.Attrs, "build-arg:foo")
 			}

solver/llbsolver/bridge.go

@@ -23,7 +23,6 @@ import (
 	"github.com/moby/buildkit/solver/pb"
 	"github.com/moby/buildkit/util/bklog"
 	"github.com/moby/buildkit/util/buildinfo"
-	binfotypes "github.com/moby/buildkit/util/buildinfo/types"
 	"github.com/moby/buildkit/util/flightcontrol"
 	"github.com/moby/buildkit/util/progress"
 	"github.com/moby/buildkit/worker"
@@ -166,20 +165,14 @@ func (b *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest, sid st
 
 	if len(res.Refs) > 0 {
 		for p := range res.Refs {
-			dtbi, errm := buildinfo.GetMetadata(res.Metadata, fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, p), binfotypes.BuildInfo{
-				Frontend: req.Frontend,
-				Attrs:    req.FrontendOpt,
-			})
+			dtbi, errm := buildinfo.GetMetadata(res.Metadata, fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, p), req.Frontend, req.FrontendOpt)
 			if errm != nil {
 				return nil, err
 			}
 			res.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, p)] = dtbi
 		}
 	} else {
-		dtbi, errm := buildinfo.GetMetadata(res.Metadata, exptypes.ExporterBuildInfo, binfotypes.BuildInfo{
-			Frontend: req.Frontend,
-			Attrs:    req.FrontendOpt,
-		})
+		dtbi, errm := buildinfo.GetMetadata(res.Metadata, exptypes.ExporterBuildInfo, req.Frontend, req.FrontendOpt)
 		if errm != nil {
 			return nil, err
 		}

util/buildinfo/buildinfo.go

@@ -178,9 +178,12 @@ var knownAttrs = []string{
 
 // filterAttrs filters frontent opt by picking only those that
 // could effectively change the build result.
-func filterAttrs(attrs map[string]string) map[string]string {
-	filtered := make(map[string]string)
+func filterAttrs(attrs map[string]*string) map[string]*string {
+	filtered := make(map[string]*string)
 	for k, v := range attrs {
+		if v == nil {
+			continue
+		}
 		// Control args are filtered out
 		if isControlArg(k) {
 			continue
@@ -223,7 +226,7 @@ func isControlArg(attrKey string) bool {
 
 // GetMetadata returns buildinfo metadata for the specified key. If the key
 // is already there, result will be merged.
-func GetMetadata(metadata map[string][]byte, key string, bi binfotypes.BuildInfo) ([]byte, error) {
+func GetMetadata(metadata map[string][]byte, key string, reqFrontend string, reqAttrs map[string]string) ([]byte, error) {
 	var (
 		dtbi []byte
 		err  error
@@ -233,33 +236,45 @@ func GetMetadata(metadata map[string][]byte, key string, bi binfotypes.BuildInfo
 		if errm := json.Unmarshal(v, &mbi); errm != nil {
 			return nil, errors.Wrapf(errm, "failed to unmarshal build info for %q", key)
 		}
-		if bi.Frontend != "" {
-			mbi.Frontend = bi.Frontend
+		if reqFrontend != "" {
+			mbi.Frontend = reqFrontend
 		}
-		mbi.Attrs = reduceMap(mbi.Attrs, bi.Attrs)
+		mbi.Attrs = convertMap(reduceMap(reqAttrs, mbi.Attrs))
 		dtbi, err = json.Marshal(mbi)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to marshal build info for %q", key)
 		}
 	} else {
-		dtbi, err = json.Marshal(bi)
+		dtbi, err = json.Marshal(binfotypes.BuildInfo{
+			Frontend: reqFrontend,
+			Attrs:    convertMap(reqAttrs),
+		})
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to marshal build info for %q", key)
 		}
 	}
 	return dtbi, nil
 }
 
-// reduceMap joins map string (union)
-func reduceMap(original map[string]string, merger map[string]string) map[string]string {
+func reduceMap(original map[string]string, merger map[string]*string) map[string]string {
 	if original == nil && merger == nil {
 		return nil
 	}
 	if original == nil {
 		original = map[string]string{}
 	}
 	for k, v := range merger {
-		original[k] = v
+		if v != nil {
+			original[k] = *v
+		}
 	}
 	return original
 }
+
+func convertMap(m map[string]string) map[string]*string {
+	res := make(map[string]*string)
+	for k, v := range m {
+		res[k] = &v
+	}
+	return res
+}

util/buildinfo/buildinfo_test.go

@@ -94,11 +94,11 @@ func TestMergeSources(t *testing.T) {
 func TestFormat(t *testing.T) {
 	bi := binfotypes.BuildInfo{
 		Frontend: "dockerfile.v0",
-		Attrs: map[string]string{
-			"build-arg:foo": "bar",
-			"context":       "https://github.com/crazy-max/buildkit-buildsources-test.git#master",
-			"filename":      "Dockerfile",
-			"source":        "crazymax/dockerfile:master",
+		Attrs: map[string]*string{
+			"build-arg:foo": stringPtr("bar"),
+			"context":       stringPtr("https://github.com/crazy-max/buildkit-buildsources-test.git#master"),
+			"filename":      stringPtr("Dockerfile"),
+			"source":        stringPtr("crazymax/dockerfile:master"),
 		},
 		Sources: []binfotypes.Source{
 			{
@@ -155,22 +155,28 @@ func TestFormat(t *testing.T) {
 func TestReduceMap(t *testing.T) {
 	cases := []struct {
 		name     string
-		m1       map[string]string
+		m1       map[string]*string
 		m2       map[string]string
 		expected map[string]string
 	}{
 		{
 			name:     "first",
-			m1:       map[string]string{"foo": "bar", "abc": "def"},
+			m1:       map[string]*string{"foo": stringPtr("bar"), "abc": stringPtr("def")},
 			m2:       map[string]string{"bar": "foo", "abc": "ghi"},
 			expected: map[string]string{"foo": "bar", "abc": "def", "bar": "foo"},
 		},
 		{
 			name:     "last",
-			m1:       map[string]string{"bar": "foo", "abc": "ghi"},
+			m1:       map[string]*string{"bar": stringPtr("foo"), "abc": stringPtr("ghi")},
 			m2:       map[string]string{"foo": "bar", "abc": "def"},
 			expected: map[string]string{"bar": "foo", "abc": "ghi", "foo": "bar"},
 		},
+		{
+			name:     "nilattr",
+			m1:       map[string]*string{"foo": stringPtr("bar"), "abc": stringPtr("fgh"), "baz": nil},
+			m2:       map[string]string{"foo": "bar", "baz": "fuu"},
+			expected: map[string]string{"foo": "bar", "abc": "fgh", "baz": "fuu"},
+		},
 		{
 			name:     "null1",
 			m1:       nil,
@@ -179,7 +185,7 @@ func TestReduceMap(t *testing.T) {
 		},
 		{
 			name:     "null2",
-			m1:       map[string]string{"foo": "bar", "abc": "def"},
+			m1:       map[string]*string{"foo": stringPtr("bar"), "abc": stringPtr("def")},
 			m2:       nil,
 			expected: map[string]string{"foo": "bar", "abc": "def"},
 		},
@@ -191,3 +197,8 @@ func TestReduceMap(t *testing.T) {
 		})
 	}
 }
+
+// stringPtr returns a pointer to the string value passed in.
+func stringPtr(v string) *string {
+	return &v
+}

util/buildinfo/types/types.go

@@ -20,7 +20,7 @@ type BuildInfo struct {
 	// Frontend defines the frontend used to build.
 	Frontend string `json:"frontend,omitempty"`
 	// Attrs defines build request attributes.
-	Attrs map[string]string `json:"attrs,omitempty"`
+	Attrs map[string]*string `json:"attrs,omitempty"`
 	// Sources defines build dependencies.
 	Sources []Source `json:"sources,omitempty"`
 }