From 3aed658dc49e268de7a0c3374085cc8438da3ba5 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Tue, 21 Jan 2025 15:05:23 -0600 Subject: [PATCH] buildflags: marshal attestations into json with extra attributes correctly `MarshalJSON` would not include the extra attributes because it iterated over the target map rather than the source map. Also fixes JSON unmarshaling for SSH and secrets. The intention was to unmarshal into the struct, but `UnmarshalText` takes priority over the default struct unmarshaling so it didn't work as intended. Tests have been added for all marshaling and unmarshaling methods. Signed-off-by: Jonathan A. Sternberg --- util/buildflags/attests.go | 2 +- util/buildflags/attests_test.go | 79 ++++++++++++++++++++++++++++++ util/buildflags/cache_test.go | 72 ++++++++++++++++++++++++++++ util/buildflags/secrets.go | 17 +++++++ util/buildflags/secrets_test.go | 84 ++++++++++++++++++++++++++++++++ util/buildflags/ssh.go | 15 ++++++ util/buildflags/ssh_test.go | 85 +++++++++++++++++++++++++++++++++ 7 files changed, 353 insertions(+), 1 deletion(-) create mode 100644 util/buildflags/attests_test.go create mode 100644 util/buildflags/secrets_test.go create mode 100644 util/buildflags/ssh_test.go diff --git a/util/buildflags/attests.go b/util/buildflags/attests.go index c1a73e91fe88..e0fe1e2feff5 100644 --- a/util/buildflags/attests.go +++ b/util/buildflags/attests.go @@ -91,7 +91,7 @@ func (a *Attest) ToPB() *controllerapi.Attest { func (a *Attest) MarshalJSON() ([]byte, error) { m := make(map[string]interface{}, len(a.Attrs)+2) - for k, v := range m { + for k, v := range a.Attrs { m[k] = v } m["type"] = a.Type diff --git a/util/buildflags/attests_test.go b/util/buildflags/attests_test.go new file mode 100644 index 000000000000..c18545f2f737 --- /dev/null +++ b/util/buildflags/attests_test.go @@ -0,0 +1,79 @@ +package buildflags + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + "github.com/zclconf/go-cty/cty" +) + +func TestAttests(t *testing.T) { + t.Run("MarshalJSON", func(t *testing.T) { + attests := Attests{ + {Type: "provenance", Attrs: map[string]string{"mode": "max"}}, + {Type: "sbom", Disabled: true}, + } + + expected := `[{"type":"provenance","mode":"max"},{"type":"sbom","disabled":true}]` + actual, err := json.Marshal(attests) + require.NoError(t, err) + require.JSONEq(t, expected, string(actual)) + }) + + t.Run("UnmarshalJSON", func(t *testing.T) { + in := `[{"type":"provenance","mode":"max"},{"type":"sbom","disabled":true}]` + + var actual Attests + err := json.Unmarshal([]byte(in), &actual) + require.NoError(t, err) + + expected := Attests{ + {Type: "provenance", Attrs: map[string]string{"mode": "max"}}, + {Type: "sbom", Disabled: true, Attrs: map[string]string{}}, + } + require.Equal(t, expected, actual) + }) + + t.Run("FromCtyValue", func(t *testing.T) { + in := cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "type": cty.StringVal("provenance"), + "mode": cty.StringVal("max"), + }), + cty.StringVal("type=sbom,disabled=true"), + }) + + var actual Attests + err := actual.FromCtyValue(in, nil) + require.NoError(t, err) + + expected := Attests{ + {Type: "provenance", Attrs: map[string]string{"mode": "max"}}, + {Type: "sbom", Disabled: true, Attrs: map[string]string{}}, + } + require.Equal(t, expected, actual) + }) + + t.Run("ToCtyValue", func(t *testing.T) { + attests := Attests{ + {Type: "provenance", Attrs: map[string]string{"mode": "max"}}, + {Type: "sbom", Disabled: true}, + } + + actual := attests.ToCtyValue() + expected := cty.ListVal([]cty.Value{ + cty.MapVal(map[string]cty.Value{ + "type": cty.StringVal("provenance"), + "mode": cty.StringVal("max"), + }), + cty.MapVal(map[string]cty.Value{ + "type": cty.StringVal("sbom"), + "disabled": cty.StringVal("true"), + }), + }) + + result := actual.Equals(expected) + require.True(t, result.True()) + }) +} diff --git a/util/buildflags/cache_test.go b/util/buildflags/cache_test.go index a25544011d79..c4368c0d470c 100644 --- a/util/buildflags/cache_test.go +++ b/util/buildflags/cache_test.go @@ -1,10 +1,12 @@ package buildflags import ( + "encoding/json" "testing" "github.com/docker/buildx/controller/pb" "github.com/stretchr/testify/require" + "github.com/zclconf/go-cty/cty" ) func TestCacheOptions_DerivedVars(t *testing.T) { @@ -37,3 +39,73 @@ func TestCacheOptions_DerivedVars(t *testing.T) { }, }, cacheFrom) } + +func TestCacheOptions(t *testing.T) { + t.Run("MarshalJSON", func(t *testing.T) { + cache := CacheOptions{ + {Type: "registry", Attrs: map[string]string{"ref": "user/app:cache"}}, + {Type: "local", Attrs: map[string]string{"src": "path/to/cache"}}, + } + + expected := `[{"type":"registry","ref":"user/app:cache"},{"type":"local","src":"path/to/cache"}]` + actual, err := json.Marshal(cache) + require.NoError(t, err) + require.JSONEq(t, expected, string(actual)) + }) + + t.Run("UnmarshalJSON", func(t *testing.T) { + in := `[{"type":"registry","ref":"user/app:cache"},{"type":"local","src":"path/to/cache"}]` + + var actual CacheOptions + err := json.Unmarshal([]byte(in), &actual) + require.NoError(t, err) + + expected := CacheOptions{ + {Type: "registry", Attrs: map[string]string{"ref": "user/app:cache"}}, + {Type: "local", Attrs: map[string]string{"src": "path/to/cache"}}, + } + require.Equal(t, expected, actual) + }) + + t.Run("FromCtyValue", func(t *testing.T) { + in := cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "type": cty.StringVal("registry"), + "ref": cty.StringVal("user/app:cache"), + }), + cty.StringVal("type=local,src=path/to/cache"), + }) + + var actual CacheOptions + err := actual.FromCtyValue(in, nil) + require.NoError(t, err) + + expected := CacheOptions{ + {Type: "registry", Attrs: map[string]string{"ref": "user/app:cache"}}, + {Type: "local", Attrs: map[string]string{"src": "path/to/cache"}}, + } + require.Equal(t, expected, actual) + }) + + t.Run("ToCtyValue", func(t *testing.T) { + attests := CacheOptions{ + {Type: "registry", Attrs: map[string]string{"ref": "user/app:cache"}}, + {Type: "local", Attrs: map[string]string{"src": "path/to/cache"}}, + } + + actual := attests.ToCtyValue() + expected := cty.ListVal([]cty.Value{ + cty.MapVal(map[string]cty.Value{ + "type": cty.StringVal("registry"), + "ref": cty.StringVal("user/app:cache"), + }), + cty.MapVal(map[string]cty.Value{ + "type": cty.StringVal("local"), + "src": cty.StringVal("path/to/cache"), + }), + }) + + result := actual.Equals(expected) + require.True(t, result.True()) + }) +} diff --git a/util/buildflags/secrets.go b/util/buildflags/secrets.go index 75a7f1613c2a..70d1c615cb43 100644 --- a/util/buildflags/secrets.go +++ b/util/buildflags/secrets.go @@ -1,6 +1,7 @@ package buildflags import ( + "encoding/json" "strings" controllerapi "github.com/docker/buildx/controller/pb" @@ -73,6 +74,22 @@ func (s *Secret) ToPB() *controllerapi.Secret { } } +func (s *Secret) UnmarshalJSON(data []byte) error { + var v struct { + ID string `json:"id,omitempty"` + FilePath string `json:"src,omitempty"` + Env string `json:"env,omitempty"` + } + if err := json.Unmarshal(data, &v); err != nil { + return err + } + + s.ID = v.ID + s.FilePath = v.FilePath + s.Env = v.Env + return nil +} + func (s *Secret) UnmarshalText(text []byte) error { value := string(text) fields, err := csvvalue.Fields(value, nil) diff --git a/util/buildflags/secrets_test.go b/util/buildflags/secrets_test.go new file mode 100644 index 000000000000..720d21e632d2 --- /dev/null +++ b/util/buildflags/secrets_test.go @@ -0,0 +1,84 @@ +package buildflags + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + "github.com/zclconf/go-cty/cty" +) + +func TestSecrets(t *testing.T) { + t.Run("MarshalJSON", func(t *testing.T) { + secrets := Secrets{ + {ID: "mysecret", FilePath: "/local/secret"}, + {ID: "mysecret2", Env: "TOKEN"}, + } + + expected := `[{"id":"mysecret","src":"/local/secret"},{"id":"mysecret2","env":"TOKEN"}]` + actual, err := json.Marshal(secrets) + require.NoError(t, err) + require.JSONEq(t, expected, string(actual)) + }) + + t.Run("UnmarshalJSON", func(t *testing.T) { + in := `[{"id":"mysecret","src":"/local/secret"},{"id":"mysecret2","env":"TOKEN"}]` + + var actual Secrets + err := json.Unmarshal([]byte(in), &actual) + require.NoError(t, err) + + expected := Secrets{ + {ID: "mysecret", FilePath: "/local/secret"}, + {ID: "mysecret2", Env: "TOKEN"}, + } + require.Equal(t, expected, actual) + }) + + t.Run("FromCtyValue", func(t *testing.T) { + in := cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("mysecret"), + "src": cty.StringVal("/local/secret"), + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("mysecret2"), + "env": cty.StringVal("TOKEN"), + }), + }) + + var actual Secrets + err := actual.FromCtyValue(in, nil) + require.NoError(t, err) + + expected := Secrets{ + {ID: "mysecret", FilePath: "/local/secret"}, + {ID: "mysecret2", Env: "TOKEN"}, + } + require.Equal(t, expected, actual) + }) + + t.Run("ToCtyValue", func(t *testing.T) { + secrets := Secrets{ + {ID: "mysecret", FilePath: "/local/secret"}, + {ID: "mysecret2", Env: "TOKEN"}, + } + + actual := secrets.ToCtyValue() + expected := cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("mysecret"), + "src": cty.StringVal("/local/secret"), + "env": cty.StringVal(""), + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("mysecret2"), + "src": cty.StringVal(""), + "env": cty.StringVal("TOKEN"), + }), + }) + + result := actual.Equals(expected) + require.True(t, result.True()) + }) +} diff --git a/util/buildflags/ssh.go b/util/buildflags/ssh.go index 200b701024d8..482bc04abd7f 100644 --- a/util/buildflags/ssh.go +++ b/util/buildflags/ssh.go @@ -2,6 +2,7 @@ package buildflags import ( "cmp" + "encoding/json" "slices" "strings" @@ -76,6 +77,20 @@ func (s *SSH) ToPB() *controllerapi.SSH { } } +func (s *SSH) UnmarshalJSON(data []byte) error { + var v struct { + ID string `json:"id,omitempty"` + Paths []string `json:"paths,omitempty"` + } + if err := json.Unmarshal(data, &v); err != nil { + return err + } + + s.ID = v.ID + s.Paths = v.Paths + return nil +} + func (s *SSH) UnmarshalText(text []byte) error { parts := strings.SplitN(string(text), "=", 2) diff --git a/util/buildflags/ssh_test.go b/util/buildflags/ssh_test.go new file mode 100644 index 000000000000..d33eb064fd74 --- /dev/null +++ b/util/buildflags/ssh_test.go @@ -0,0 +1,85 @@ +package buildflags + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + "github.com/zclconf/go-cty/cty" +) + +func TestSSHKeys(t *testing.T) { + t.Run("MarshalJSON", func(t *testing.T) { + sshkeys := SSHKeys{ + {ID: "default", Paths: []string{}}, + {ID: "key", Paths: []string{"path/to/key"}}, + } + + expected := `[{"id":"default"},{"id":"key","paths":["path/to/key"]}]` + actual, err := json.Marshal(sshkeys) + require.NoError(t, err) + require.JSONEq(t, expected, string(actual)) + }) + + t.Run("UnmarshalJSON", func(t *testing.T) { + in := `[{"id":"default"},{"id":"key","paths":["path/to/key"]}]` + + var actual SSHKeys + err := json.Unmarshal([]byte(in), &actual) + require.NoError(t, err) + + expected := SSHKeys{ + {ID: "default"}, + {ID: "key", Paths: []string{"path/to/key"}}, + } + require.Equal(t, expected, actual) + }) + + t.Run("FromCtyValue", func(t *testing.T) { + in := cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("default"), + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("key"), + "paths": cty.TupleVal([]cty.Value{ + cty.StringVal("path/to/key"), + }), + }), + }) + + var actual SSHKeys + err := actual.FromCtyValue(in, nil) + require.NoError(t, err) + + expected := SSHKeys{ + {ID: "default"}, + {ID: "key", Paths: []string{"path/to/key"}}, + } + require.Equal(t, expected, actual) + }) + + t.Run("ToCtyValue", func(t *testing.T) { + sshkeys := SSHKeys{ + {ID: "default", Paths: []string{}}, + {ID: "key", Paths: []string{"path/to/key"}}, + } + + actual := sshkeys.ToCtyValue() + expected := cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("default"), + "paths": cty.ListValEmpty(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("key"), + "paths": cty.ListVal([]cty.Value{ + cty.StringVal("path/to/key"), + }), + }), + }) + + result := actual.Equals(expected) + require.True(t, result.True()) + }) +}