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 c1a73e91..e0fe1e2f 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 00000000..c18545f2 --- /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 a2554401..c4368c0d 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 75a7f161..70d1c615 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 00000000..720d21e6 --- /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 200b7010..482bc04a 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 00000000..d33eb064 --- /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()) + }) +}