From b66988c824a888a2c78290adea3b054e8ef325ab Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 1 Feb 2023 10:12:36 +0000 Subject: [PATCH 1/8] bake: fix loop references Signed-off-by: Justin Chadwell (cherry picked from commit 48357ee0c6a2fb4e2e68e49ac06386f2fafa51c6) --- bake/compose.go | 1 + bake/remote.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/bake/compose.go b/bake/compose.go index 990fcee10d7..8c4e5fe45b1 100644 --- a/bake/compose.go +++ b/bake/compose.go @@ -78,6 +78,7 @@ func ParseCompose(cfgs []compose.ConfigFile, envs map[string]string) (*Config, e // compose does not support nil values for labels labels := map[string]*string{} for k, v := range s.Build.Labels { + v := v labels[k] = &v } diff --git a/bake/remote.go b/bake/remote.go index 802e9209451..939957d0353 100644 --- a/bake/remote.go +++ b/bake/remote.go @@ -34,9 +34,9 @@ func ReadRemoteFiles(ctx context.Context, nodes []builder.Node, url string, name var files []File var node *builder.Node - for _, n := range nodes { + for i, n := range nodes { if n.Err == nil { - node = &n + node = &nodes[i] continue } } From ae278ce450a76138768b9ce94d09580ed32d2484 Mon Sep 17 00:00:00 2001 From: CrazyMax Date: Fri, 3 Feb 2023 21:23:29 +0100 Subject: [PATCH 2/8] builder: fix docker context not validated Signed-off-by: CrazyMax (cherry picked from commit 0e544fe83509add4a992ee33079924301b8adfa8) --- store/storeutil/storeutil.go | 1 + 1 file changed, 1 insertion(+) diff --git a/store/storeutil/storeutil.go b/store/storeutil/storeutil.go index e6c06828d0e..4e6b0898a13 100644 --- a/store/storeutil/storeutil.go +++ b/store/storeutil/storeutil.go @@ -93,6 +93,7 @@ func GetNodeGroup(txn *store.Txn, dockerCli command.Cli, name string) (*store.No Endpoint: name, }, }, + DockerContext: true, } if ng.LastActivity, err = txn.GetLastActivity(ng); err != nil { return nil, err From 582cc04be6162f10adcd13918ee37a6245449191 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 6 Feb 2023 10:53:49 +0000 Subject: [PATCH 3/8] build: add docs for boolean attestation flags Signed-off-by: Justin Chadwell (cherry picked from commit 07548bc898d803272a8be4f1bd281b0415ee4c73) --- docs/reference/buildx_build.md | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/docs/reference/buildx_build.md b/docs/reference/buildx_build.md index f56ab1f6828..60ea921a8bb 100644 --- a/docs/reference/buildx_build.md +++ b/docs/reference/buildx_build.md @@ -88,6 +88,9 @@ BuildKit currently supports: Use `--attest=type=provenance` to generate provenance for an image at build-time. Alternatively, you can use the [`--provenance` shorthand](#provenance). + By default, a minimal provenance attestation will be created for the build + result, which will only be attached for images pushed to registries. + For more information, see [here](https://docs.docker.com/build/attestations/slsa-provenance/). ### Allow extra privileged entitlement (--allow) @@ -477,8 +480,20 @@ $ docker buildx build --load --progress=plain . ### Create provenance attestations (--provenance) -Shorthand for [`--attest=type=provenance`](#attest). Enables provenance -attestations for the build result. +Shorthand for [`--attest=type=provenance`](#attest), used to configure +provenance attestations for the build result. For example, +`--provenance=mode=max` can be used as an abbreviation for +`--attest=type=provenance,mode=max`. + +Additionally, `--provenance` can be used with boolean values to broadly enable +or disable provenance attestations. For example, `--provenance=false` can be +used to disable all provenance attestations, while `--provenance=true` can be +used to enable all provenance attestations. + +By default, a minimal provenance attestation will be created for the build +result, which will only be attached for images pushed to registries. + +For more information, see [here](https://docs.docker.com/build/attestations/slsa-provenance/). ### Push the build result to a registry (--push) @@ -487,8 +502,16 @@ build result to registry. ### Create SBOM attestations (--sbom) -Shorthand for [`--attest=type=sbom`](#attest). Enables SBOM attestations for -the build result. +Shorthand for [`--attest=type=sbom`](#attest), used to configure SBOM +attestations for the build result. For example, +`--sbom=generator=/` can be used as an abbreviation for +`--attest=type=sbom,generator=/`. + +Additionally, `--sbom` can be used with boolean values to broadly enable or +disable SBOM attestations. For example, `--sbom=false` can be used to disable +all SBOM attestations. + +For more information, see [here](https://docs.docker.com/build/attestations/sbom/). ### Secret to expose to the build (--secret) From cc87bd104ea8af19d9f5a62d5c5ef2b71d764ef8 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 6 Feb 2023 14:29:02 +0000 Subject: [PATCH 4/8] bake: avoid early-exit for resolution failures With changes made to allow lazy evaluation, we were early exiting if an undefined name was detected, either for a variable or a function. This had two key implications: 1. The error messages changed, and became significantly less informative. For example, we went from: > Unknown variable; There is no variable named "FO". Did you mean "FOO"?, and 1 other diagnostic(s) To > Invalid expression; undefined variable "FO" 2. Any issues in our function detection from funcCalls which cause JSON functions to be erroneously detected cause invalid functions to be resolved, which causes new name resolution errors. To avoid the above problems, we can defer the error from an undefined name until HCL evaluation - which produces the more informative errors, and does not suffer from incorrectly detecting JSON functions. Signed-off-by: Justin Chadwell (cherry picked from commit dc8a2b03987f1266ace9a7cdcba92f5e4ccdb8ed) --- bake/hcl_test.go | 18 ++++++++++++++++++ bake/hclparser/hclparser.go | 23 +++++++++++++++++------ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/bake/hcl_test.go b/bake/hcl_test.go index 6d6fda0e276..56d705add04 100644 --- a/bake/hcl_test.go +++ b/bake/hcl_test.go @@ -670,6 +670,24 @@ func TestJSONFunctions(t *testing.T) { require.Equal(t, ptrstr("pre-"), c.Targets[0].Args["v1"]) } +func TestJSONInvalidFunctions(t *testing.T) { + dt := []byte(`{ + "target": { + "app": { + "args": { + "v1": "myfunc(\"foo\")" + } + } + }}`) + + c, err := ParseFile(dt, "docker-bake.json") + require.NoError(t, err) + + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, c.Targets[0].Name, "app") + require.Equal(t, ptrstr(`myfunc("foo")`), c.Targets[0].Args["v1"]) +} + func TestHCLFunctionInAttr(t *testing.T) { dt := []byte(` function "brace" { diff --git a/bake/hclparser/hclparser.go b/bake/hclparser/hclparser.go index 6a8a7926064..23477c43e05 100644 --- a/bake/hclparser/hclparser.go +++ b/bake/hclparser/hclparser.go @@ -62,7 +62,9 @@ type parser struct { doneB map[*hcl.Block]map[string]struct{} } -func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.Diagnostics { +var errUndefined = errors.New("undefined") + +func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}, allowMissing bool) hcl.Diagnostics { fns, hcldiags := funcCalls(exp) if hcldiags.HasErrors() { return hcldiags @@ -70,6 +72,9 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.D for _, fn := range fns { if err := p.resolveFunction(fn); err != nil { + if allowMissing && errors.Is(err, errUndefined) { + continue + } return hcl.Diagnostics{ &hcl.Diagnostic{ Severity: hcl.DiagError, @@ -128,6 +133,9 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.D } } if err := p.resolveBlock(blocks[0], target); err != nil { + if allowMissing && errors.Is(err, errUndefined) { + continue + } return hcl.Diagnostics{ &hcl.Diagnostic{ Severity: hcl.DiagError, @@ -140,6 +148,9 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.D } } else { if err := p.resolveValue(v.RootName()); err != nil { + if allowMissing && errors.Is(err, errUndefined) { + continue + } return hcl.Diagnostics{ &hcl.Diagnostic{ Severity: hcl.DiagError, @@ -167,7 +178,7 @@ func (p *parser) resolveFunction(name string) error { if _, ok := p.ectx.Functions[name]; ok { return nil } - return errors.Errorf("undefined function %s", name) + return errors.Wrapf(errUndefined, "function %q does not exit", name) } if _, ok := p.progressF[name]; ok { return errors.Errorf("function cycle not allowed for %s", name) @@ -217,7 +228,7 @@ func (p *parser) resolveFunction(name string) error { return diags } - if diags := p.loadDeps(f.Result.Expr, params); diags.HasErrors() { + if diags := p.loadDeps(f.Result.Expr, params, false); diags.HasErrors() { return diags } @@ -255,7 +266,7 @@ func (p *parser) resolveValue(name string) (err error) { if _, builtin := p.opt.Vars[name]; !ok && !builtin { vr, ok := p.vars[name] if !ok { - return errors.Errorf("undefined variable %q", name) + return errors.Wrapf(errUndefined, "variable %q does not exit", name) } def = vr.Default } @@ -270,7 +281,7 @@ func (p *parser) resolveValue(name string) (err error) { return } - if diags := p.loadDeps(def.Expr, nil); diags.HasErrors() { + if diags := p.loadDeps(def.Expr, nil, true); diags.HasErrors() { return diags } vv, diags := def.Expr.Value(p.ectx) @@ -395,7 +406,7 @@ func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err err return diag } for _, a := range content.Attributes { - diag := p.loadDeps(a.Expr, nil) + diag := p.loadDeps(a.Expr, nil, true) if diag.HasErrors() { return diag } From 02cf539a085db336ee5582d05640150e74a2cd18 Mon Sep 17 00:00:00 2001 From: CrazyMax Date: Tue, 7 Feb 2023 13:26:25 +0100 Subject: [PATCH 5/8] gitutil: override the locale to ensure consistent output Signed-off-by: CrazyMax (cherry picked from commit a8eb2a7fbe3fe316ec428bf1c723815afac1b128) --- util/gitutil/gitutil.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/util/gitutil/gitutil.go b/util/gitutil/gitutil.go index 3aa33013bad..74ec513e145 100644 --- a/util/gitutil/gitutil.go +++ b/util/gitutil/gitutil.go @@ -3,6 +3,7 @@ package gitutil import ( "bytes" "context" + "os" "os/exec" "strings" @@ -116,6 +117,9 @@ func (c *Git) run(args ...string) (string, error) { cmd.Dir = c.wd } + // Override the locale to ensure consistent output + cmd.Env = append(os.Environ(), "LC_ALL=C") + stdout := bytes.Buffer{} stderr := bytes.Buffer{} cmd.Stdout = &stdout From 661af29d46517c69871f3980d9d9be2c389050f2 Mon Sep 17 00:00:00 2001 From: CrazyMax Date: Fri, 3 Feb 2023 14:29:43 +0100 Subject: [PATCH 6/8] build: check reachable git commits Signed-off-by: CrazyMax (cherry picked from commit fd5884189cc0d24a92c98fd4747847361c49c9c0) --- build/git.go | 2 +- util/gitutil/gitutil.go | 9 +++++++++ util/gitutil/gitutil_test.go | 12 ++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/build/git.go b/build/git.go index 47f3fc1cf6a..e7b5d8eb3b1 100644 --- a/build/git.go +++ b/build/git.go @@ -64,7 +64,7 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st return res, nil } - if sha, err := gitc.FullCommit(); err != nil { + if sha, err := gitc.FullCommit(); err != nil && !gitutil.IsUnknownRevision(err) { return res, errors.Wrapf(err, "buildx: failed to get git commit") } else if sha != "" { if gitc.IsDirty() { diff --git a/util/gitutil/gitutil.go b/util/gitutil/gitutil.go index 74ec513e145..69e6aa3194f 100644 --- a/util/gitutil/gitutil.go +++ b/util/gitutil/gitutil.go @@ -138,3 +138,12 @@ func (c *Git) clean(out string, err error) (string, error) { } return out, err } + +func IsUnknownRevision(err error) bool { + if err == nil { + return false + } + // https://github.com/git/git/blob/a6a323b31e2bcbac2518bddec71ea7ad558870eb/setup.c#L204 + errMsg := strings.ToLower(err.Error()) + return strings.Contains(errMsg, "unknown revision or path not in the working tree") || strings.Contains(errMsg, "bad revision") +} diff --git a/util/gitutil/gitutil_test.go b/util/gitutil/gitutil_test.go index 28992646b31..050d4515917 100644 --- a/util/gitutil/gitutil_test.go +++ b/util/gitutil/gitutil_test.go @@ -46,6 +46,18 @@ func TestGitShortCommit(t *testing.T) { require.Equal(t, 7, len(out)) } +func TestGitFullCommitErr(t *testing.T) { + Mktmp(t) + c, err := New() + require.NoError(t, err) + + GitInit(c, t) + + _, err = c.FullCommit() + require.Error(t, err) + require.True(t, IsUnknownRevision(err)) +} + func TestGitTagsPointsAt(t *testing.T) { Mktmp(t) c, err := New() From 0fad89c3b90e162ac6e746da70e4aacbbe6f46cc Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 7 Feb 2023 11:34:17 +0000 Subject: [PATCH 7/8] bake: avoid nesting error diagnostics With changes to the lazy evaluation, the evaluation order is no longer fixed - this means that we can follow long and confusing paths to get to an error. Because of the co-recursive nature of the lazy evaluation, we need to take special care that the original HCL diagnostics are not discarded and are preserved so that the original source of the error can be detected. Preserving the full trace is not necessary, and probably not useful to the user - all of the file that is not lazily loaded will be eagerly loaded after all struct blocks are loaded - so the error would be found regardless. Signed-off-by: Justin Chadwell (cherry picked from commit fbb4f4dec86541dd243e99816ef84b3a4d4741c2) --- bake/hclparser/expr.go | 10 +--- bake/hclparser/hclparser.go | 100 +++++++++++------------------------- 2 files changed, 30 insertions(+), 80 deletions(-) diff --git a/bake/hclparser/expr.go b/bake/hclparser/expr.go index 23a99f54097..80ecb0205c8 100644 --- a/bake/hclparser/expr.go +++ b/bake/hclparser/expr.go @@ -14,15 +14,7 @@ func funcCalls(exp hcl.Expression) ([]string, hcl.Diagnostics) { if !ok { fns, err := jsonFuncCallsRecursive(exp) if err != nil { - return nil, hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid expression", - Detail: err.Error(), - Subject: exp.Range().Ptr(), - Context: exp.Range().Ptr(), - }, - } + return nil, wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr()) } return fns, nil } diff --git a/bake/hclparser/hclparser.go b/bake/hclparser/hclparser.go index 23477c43e05..ec378cf98be 100644 --- a/bake/hclparser/hclparser.go +++ b/bake/hclparser/hclparser.go @@ -75,15 +75,7 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}, allow if allowMissing && errors.Is(err, errUndefined) { continue } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid expression", - Detail: err.Error(), - Subject: exp.Range().Ptr(), - Context: exp.Range().Ptr(), - }, - } + return wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr()) } } @@ -136,30 +128,14 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}, allow if allowMissing && errors.Is(err, errUndefined) { continue } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid expression", - Detail: err.Error(), - Subject: v.SourceRange().Ptr(), - Context: v.SourceRange().Ptr(), - }, - } + return wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr()) } } else { if err := p.resolveValue(v.RootName()); err != nil { if allowMissing && errors.Is(err, errUndefined) { continue } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid expression", - Detail: err.Error(), - Subject: v.SourceRange().Ptr(), - Context: v.SourceRange().Ptr(), - }, - } + return wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr()) } } } @@ -325,14 +301,7 @@ func (p *parser) resolveValue(name string) (err error) { func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err error) { name := block.Labels[0] if err := p.opt.ValidateLabel(name); err != nil { - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid name", - Detail: err.Error(), - Subject: &block.LabelRanges[0], - }, - } + return wrapErrorDiagnostic("Invalid name", err, &block.LabelRanges[0], &block.LabelRanges[0]) } if _, ok := p.doneB[block]; !ok { @@ -584,15 +553,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics { return diags } r := p.vars[k].Body.MissingItemRange() - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid value", - Detail: err.Error(), - Subject: &r, - Context: &r, - }, - } + return wrapErrorDiagnostic("Invalid value", err, &r, &r) } } @@ -615,15 +576,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics { } } } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid function", - Detail: err.Error(), - Subject: subject, - Context: context, - }, - } + return wrapErrorDiagnostic("Invalid function", err, subject, context) } } @@ -684,15 +637,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics { continue } } else { - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid attribute", - Detail: err.Error(), - Subject: &b.LabelRanges[0], - Context: &b.DefRange, - }, - } + return wrapErrorDiagnostic("Invalid block", err, &b.LabelRanges[0], &b.DefRange) } } @@ -737,21 +682,34 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics { if diags, ok := err.(hcl.Diagnostics); ok { return diags } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid attribute", - Detail: err.Error(), - Subject: &p.attrs[k].Range, - Context: &p.attrs[k].Range, - }, - } + return wrapErrorDiagnostic("Invalid attribute", err, &p.attrs[k].Range, &p.attrs[k].Range) } } return nil } +// wrapErrorDiagnostic wraps an error into a hcl.Diagnostics object. +// If the error is already an hcl.Diagnostics object, it is returned as is. +func wrapErrorDiagnostic(message string, err error, subject *hcl.Range, context *hcl.Range) hcl.Diagnostics { + switch err := err.(type) { + case *hcl.Diagnostic: + return hcl.Diagnostics{err} + case hcl.Diagnostics: + return err + default: + return hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: message, + Detail: err.Error(), + Subject: subject, + Context: context, + }, + } + } +} + func setLabel(v reflect.Value, lbl string) int { // cache field index? numFields := v.Elem().Type().NumField() From a73d07ff7a5835c7b357c18e4512ff710a2cd6c7 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Fri, 10 Feb 2023 11:09:02 +0000 Subject: [PATCH 8/8] imagetools: process com.docker.reference.* annotations To give us the option later down the road of producing recommended OCI names in BuildKit (using com instead of vnd, woops), we need to update Buildx to be able to process both. Ideally, if a Buildx/BuildKit release hadn't been made we could just switch over, but since we have, we'd need to support both (at least for a while, eventually we could consider deprecating+removing the vnd variant). Signed-off-by: Justin Chadwell (cherry picked from commit 642f28f439e9da9bbd528f8a46506ca3bdc95028) --- util/imagetools/loader.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/util/imagetools/loader.go b/util/imagetools/loader.go index 9bba4e21f46..a165156678a 100644 --- a/util/imagetools/loader.go +++ b/util/imagetools/loader.go @@ -21,8 +21,11 @@ import ( "golang.org/x/sync/errgroup" ) -const ( - annotationReference = "vnd.docker.reference.digest" +var ( + annotationReferences = []string{ + "com.docker.reference.digest", + "vnd.docker.reference.digest", // TODO: deprecate/remove after migration to new annotation + } ) type contentCache interface { @@ -167,8 +170,13 @@ func (l *loader) fetch(ctx context.Context, fetcher remotes.Fetcher, desc ocispe } r.mu.Unlock() - ref, ok := desc.Annotations[annotationReference] - if ok { + found := false + for _, annotationReference := range annotationReferences { + ref, ok := desc.Annotations[annotationReference] + if !ok { + continue + } + refdgst, err := digest.Parse(ref) if err != nil { return err @@ -176,7 +184,10 @@ func (l *loader) fetch(ctx context.Context, fetcher remotes.Fetcher, desc ocispe r.mu.Lock() r.refs[refdgst] = append(r.refs[refdgst], desc.Digest) r.mu.Unlock() - } else { + found = true + break + } + if !found { p := desc.Platform if p == nil { p, err = l.readPlatformFromConfig(ctx, fetcher, mfst.Config)