From d5ad869033c296e77cb52cfb477e0bc5fe62f5cf Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Wed, 22 Jan 2025 13:18:38 -0600 Subject: [PATCH] buildflags: fix ref only format for command line and bake Signed-off-by: Jonathan A. Sternberg (cherry picked from commit 11c84973ef104e48eb88a41b5b23d6a559efe868) --- bake/bake.go | 39 ++--------------------------------- bake/bake_test.go | 22 ++++++++++++++++++++ bake/compose.go | 8 +++---- commands/build.go | 7 +++++-- util/buildflags/cache.go | 21 +++++++++++++++++-- util/buildflags/cache_cty.go | 17 ++++++++------- util/buildflags/cache_test.go | 11 +++++++++- 7 files changed, 72 insertions(+), 53 deletions(-) diff --git a/bake/bake.go b/bake/bake.go index c7979dcf..f365b65b 100644 --- a/bake/bake.go +++ b/bake/bake.go @@ -29,7 +29,6 @@ import ( "github.com/moby/buildkit/session/auth/authprovider" "github.com/moby/buildkit/util/entitlements" "github.com/pkg/errors" - "github.com/tonistiigi/go-csvvalue" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/convert" ) @@ -900,7 +899,7 @@ func (t *Target) AddOverrides(overrides map[string]Override, ent *EntitlementCon case "tags": t.Tags = o.ArrValue case "cache-from": - cacheFrom, err := parseCacheArrValues(o.ArrValue) + cacheFrom, err := buildflags.ParseCacheEntry(o.ArrValue) if err != nil { return err } @@ -913,7 +912,7 @@ func (t *Target) AddOverrides(overrides map[string]Override, ent *EntitlementCon } } case "cache-to": - cacheTo, err := parseCacheArrValues(o.ArrValue) + cacheTo, err := buildflags.ParseCacheEntry(o.ArrValue) if err != nil { return err } @@ -1585,37 +1584,3 @@ func parseArrValue[T any, PT arrValue[T]](s []string) ([]*T, error) { } return outputs, nil } - -func parseCacheArrValues(s []string) (buildflags.CacheOptions, error) { - var outs buildflags.CacheOptions - for _, in := range s { - if in == "" { - continue - } - - if !strings.Contains(in, "=") { - // This is ref only format. Each field in the CSV is its own entry. - fields, err := csvvalue.Fields(in, nil) - if err != nil { - return nil, err - } - - for _, field := range fields { - out := buildflags.CacheOptionsEntry{} - if err := out.UnmarshalText([]byte(field)); err != nil { - return nil, err - } - outs = append(outs, &out) - } - continue - } - - // Normal entry. - out := buildflags.CacheOptionsEntry{} - if err := out.UnmarshalText([]byte(in)); err != nil { - return nil, err - } - outs = append(outs, &out) - } - return outs, nil -} diff --git a/bake/bake_test.go b/bake/bake_test.go index 15a3333d..f1f0a9f2 100644 --- a/bake/bake_test.go +++ b/bake/bake_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/docker/buildx/util/buildflags" "github.com/moby/buildkit/util/entitlements" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -1759,6 +1760,27 @@ func TestAnnotations(t *testing.T) { require.Equal(t, "bar", bo["app"].Exports[0].Attrs["annotation-manifest[linux/amd64].foo"]) } +func TestRefOnlyCacheOptions(t *testing.T) { + fp := File{ + Name: "docker-bake.hcl", + Data: []byte( + `target "app" { + output = ["type=image,name=foo"] + cache-from = ["ref1,ref2"] + }`), + } + ctx := context.TODO() + m, _, err := ReadTargets(ctx, []File{fp}, []string{"app"}, nil, nil, &EntitlementConf{}) + require.NoError(t, err) + + require.Len(t, m, 1) + require.Contains(t, m, "app") + require.Equal(t, buildflags.CacheOptions{ + {Type: "registry", Attrs: map[string]string{"ref": "ref1"}}, + {Type: "registry", Attrs: map[string]string{"ref": "ref2"}}, + }, m["app"].CacheFrom) +} + func TestHCLEntitlements(t *testing.T) { fp := File{ Name: "docker-bake.hcl", diff --git a/bake/compose.go b/bake/compose.go index 58cfc80c..fba1f3de 100644 --- a/bake/compose.go +++ b/bake/compose.go @@ -145,12 +145,12 @@ func ParseCompose(cfgs []composetypes.ConfigFile, envs map[string]string) (*Conf labels[k] = &v } - cacheFrom, err := parseCacheArrValues(s.Build.CacheFrom) + cacheFrom, err := buildflags.ParseCacheEntry(s.Build.CacheFrom) if err != nil { return nil, err } - cacheTo, err := parseCacheArrValues(s.Build.CacheTo) + cacheTo, err := buildflags.ParseCacheEntry(s.Build.CacheTo) if err != nil { return nil, err } @@ -349,14 +349,14 @@ func (t *Target) composeExtTarget(exts map[string]interface{}) error { t.Tags = dedupSlice(append(t.Tags, xb.Tags...)) } if len(xb.CacheFrom) > 0 { - cacheFrom, err := parseCacheArrValues(xb.CacheFrom) + cacheFrom, err := buildflags.ParseCacheEntry(xb.CacheFrom) if err != nil { return err } t.CacheFrom = t.CacheFrom.Merge(cacheFrom) } if len(xb.CacheTo) > 0 { - cacheTo, err := parseCacheArrValues(xb.CacheTo) + cacheTo, err := buildflags.ParseCacheEntry(xb.CacheTo) if err != nil { return err } diff --git a/commands/build.go b/commands/build.go index 264c1340..91a5786a 100644 --- a/commands/build.go +++ b/commands/build.go @@ -183,14 +183,17 @@ func (o *buildOptions) toControllerOptions() (*controllerapi.BuildOptions, error } } - opts.CacheFrom, err = buildflags.ParseCacheEntry(o.cacheFrom) + cacheFrom, err := buildflags.ParseCacheEntry(o.cacheFrom) if err != nil { return nil, err } - opts.CacheTo, err = buildflags.ParseCacheEntry(o.cacheTo) + opts.CacheFrom = cacheFrom.ToPB() + + cacheTo, err := buildflags.ParseCacheEntry(o.cacheTo) if err != nil { return nil, err } + opts.CacheTo = cacheTo.ToPB() opts.Secrets, err = buildflags.ParseSecretSpecs(o.secrets) if err != nil { diff --git a/util/buildflags/cache.go b/util/buildflags/cache.go index db982e78..9cd5afb0 100644 --- a/util/buildflags/cache.go +++ b/util/buildflags/cache.go @@ -167,20 +167,37 @@ func (e *CacheOptionsEntry) validate(gv interface{}) error { return nil } -func ParseCacheEntry(in []string) ([]*controllerapi.CacheOptionsEntry, error) { +func ParseCacheEntry(in []string) (CacheOptions, error) { if len(in) == 0 { return nil, nil } opts := make(CacheOptions, 0, len(in)) for _, in := range in { + if !strings.Contains(in, "=") { + // This is ref only format. Each field in the CSV is its own entry. + fields, err := csvvalue.Fields(in, nil) + if err != nil { + return nil, err + } + + for _, field := range fields { + opt := CacheOptionsEntry{} + if err := opt.UnmarshalText([]byte(field)); err != nil { + return nil, err + } + opts = append(opts, &opt) + } + continue + } + var out CacheOptionsEntry if err := out.UnmarshalText([]byte(in)); err != nil { return nil, err } opts = append(opts, &out) } - return opts.ToPB(), nil + return opts, nil } func addGithubToken(ci *controllerapi.CacheOptionsEntry) { diff --git a/util/buildflags/cache_cty.go b/util/buildflags/cache_cty.go index abfcd008..05e2e6bc 100644 --- a/util/buildflags/cache_cty.go +++ b/util/buildflags/cache_cty.go @@ -30,6 +30,16 @@ func (o *CacheOptions) fromCtyValue(in cty.Value, p cty.Path) error { continue } + // Special handling for a string type to handle ref only format. + if value.Type() == cty.String { + entries, err := ParseCacheEntry([]string{value.AsString()}) + if err != nil { + return err + } + *o = append(*o, entries...) + continue + } + entry := &CacheOptionsEntry{} if err := entry.FromCtyValue(value, p); err != nil { return err @@ -52,13 +62,6 @@ func (o CacheOptions) ToCtyValue() cty.Value { } func (o *CacheOptionsEntry) FromCtyValue(in cty.Value, p cty.Path) error { - if in.Type() == cty.String { - if err := o.UnmarshalText([]byte(in.AsString())); err != nil { - return p.NewError(err) - } - return nil - } - conv, err := convert.Convert(in, cty.Map(cty.String)) if err != nil { return err diff --git a/util/buildflags/cache_test.go b/util/buildflags/cache_test.go index c4368c0d..02d2f8ba 100644 --- a/util/buildflags/cache_test.go +++ b/util/buildflags/cache_test.go @@ -37,7 +37,7 @@ func TestCacheOptions_DerivedVars(t *testing.T) { "session_token": "not_a_mitm_attack", }, }, - }, cacheFrom) + }, cacheFrom.ToPB()) } func TestCacheOptions(t *testing.T) { @@ -109,3 +109,12 @@ func TestCacheOptions(t *testing.T) { require.True(t, result.True()) }) } + +func TestCacheOptions_RefOnlyFormat(t *testing.T) { + opts, err := ParseCacheEntry([]string{"ref1", "ref2"}) + require.NoError(t, err) + require.Equal(t, CacheOptions{ + {Type: "registry", Attrs: map[string]string{"ref": "ref1"}}, + {Type: "registry", Attrs: map[string]string{"ref": "ref2"}}, + }, opts) +}