Skip to content

Commit abc85c3

Browse files
jsternbergjsternberg
committed
buildflags: handle unknown values from cty
Update the buildflags cty code to handle unknown values. When hcl decodes a value with an invalid variable name, it appends a diagnostic for the error and then returns an unknown value so it can continue processing the file and finding more errors. The iteration code has now been changed to use a rangefunc from go 1.23 and it skips empty or unknown values. Empty values are valid when they are skipped and unknown values will have a diagnostic for itself. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
1 parent b76fdca commit abc85c3

File tree

9 files changed

+88
-43
lines changed

9 files changed

+88
-43
lines changed

‎bake/hcl_test.go‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package bake
22

33
import (
44
"reflect"
5+
"regexp"
56
"testing"
67

8+
hcl "github.com/hashicorp/hcl/v2"
79
"github.com/stretchr/testify/require"
810
)
911

@@ -647,7 +649,7 @@ func TestHCLAttrsCapsuleType(t *testing.T) {
647649
require.Equal(t, []string{"default", "key=path/to/key"}, stringify(c.Targets[0].SSH))
648650
}
649651

650-
func TestHCLAttrsCapsuleTypeVars(t *testing.T) {
652+
func TestHCLAttrsCapsuleType_ObjectVars(t *testing.T) {
651653
dt := []byte(`
652654
variable "foo" {
653655
default = "bar"
@@ -716,6 +718,52 @@ func TestHCLAttrsCapsuleTypeVars(t *testing.T) {
716718
require.Equal(t, []string{"id=oci,src=/local/secret"}, stringify(web.Secrets))
717719
}
718720

721+
func TestHCLAttrsCapsuleType_MissingVars(t *testing.T) {
722+
dt := []byte(`
723+
target "app" {
724+
attest = [
725+
"type=sbom,disabled=${SBOM}",
726+
]
727+
728+
cache-from = [
729+
{ type = "registry", ref = "user/app:${FOO1}" },
730+
"type=local,src=path/to/cache:${FOO2}",
731+
]
732+
733+
cache-to = [
734+
{ type = "local", dest = "path/to/${BAR}" },
735+
]
736+
737+
output = [
738+
{ type = "oci", dest = "../${OUTPUT}.tar" },
739+
]
740+
741+
secret = [
742+
{ id = "mysecret", src = "/local/${SECRET}" },
743+
]
744+
745+
ssh = [
746+
{ id = "key", paths = ["path/to/${SSH_KEY}"] },
747+
]
748+
}
749+
`)
750+
751+
var diags hcl.Diagnostics
752+
_, err := ParseFile(dt, "docker-bake.hcl")
753+
require.ErrorAs(t, err, &diags)
754+
755+
re := regexp.MustCompile(`There is no variable named "([\w\d_]+)"`)
756+
var actual []string
757+
for _, diag := range diags {
758+
if m := re.FindStringSubmatch(diag.Error()); m != nil {
759+
actual = append(actual, m[1])
760+
}
761+
}
762+
require.ElementsMatch(t,
763+
[]string{"SBOM", "FOO1", "FOO2", "BAR", "OUTPUT", "SECRET", "SSH_KEY"},
764+
actual)
765+
}
766+
719767
func TestHCLMultiFileAttrs(t *testing.T) {
720768
dt := []byte(`
721769
variable "FOO" {

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/docker/buildx
22

3-
go 1.22.0
3+
go 1.23.0
44

55
require (
66
github.com/Masterminds/semver/v3 v3.2.1

‎hack/dockerfiles/lint.Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ ARG XX_VERSION=1.6.1
77
ARG GOLANGCI_LINT_VERSION=1.62.0
88
ARG GOPLS_VERSION=v0.26.0
99
# disabled: deprecated unusedvariable simplifyrange
10-
ARG GOPLS_ANALYZERS="embeddirective fillreturns infertypeargs nonewvars norangeoverfunc noresultvalues simplifycompositelit simplifyslice undeclaredname unusedparams useany"
10+
ARG GOPLS_ANALYZERS="embeddirective fillreturns infertypeargs nonewvars noresultvalues simplifycompositelit simplifyslice undeclaredname unusedparams useany"
1111

1212
FROM --platform=$BUILDPLATFORM tonistiigi/xx:${XX_VERSION} AS xx
1313

‎util/buildflags/attests_cty.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@ func (e *Attests) FromCtyValue(in cty.Value, p cty.Path) error {
2424

2525
func (e *Attests) fromCtyValue(in cty.Value, p cty.Path) error {
2626
*e = make([]*Attest, 0, in.LengthInt())
27-
for elem := in.ElementIterator(); elem.Next(); {
28-
_, value := elem.Element()
29-
27+
for value := range eachElement(in) {
3028
entry := &Attest{}
3129
if err := entry.FromCtyValue(value, p); err != nil {
3230
return err
@@ -64,6 +62,10 @@ func (e *Attest) FromCtyValue(in cty.Value, p cty.Path) error {
6462
e.Attrs = map[string]string{}
6563
for it := conv.ElementIterator(); it.Next(); {
6664
k, v := it.Element()
65+
if !v.IsKnown() {
66+
continue
67+
}
68+
6769
switch key := k.AsString(); key {
6870
case "type":
6971
e.Type = v.AsString()

‎util/buildflags/cache_cty.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,7 @@ func (o *CacheOptions) FromCtyValue(in cty.Value, p cty.Path) error {
2323

2424
func (o *CacheOptions) fromCtyValue(in cty.Value, p cty.Path) error {
2525
*o = make([]*CacheOptionsEntry, 0, in.LengthInt())
26-
for elem := in.ElementIterator(); elem.Next(); {
27-
_, value := elem.Element()
28-
29-
if isEmpty(value) {
30-
continue
31-
}
32-
26+
for value := range eachElement(in) {
3327
// Special handling for a string type to handle ref only format.
3428
if value.Type() == cty.String {
3529
entries, err := ParseCacheEntry([]string{value.AsString()})

‎util/buildflags/export_cty.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,7 @@ func (e *Exports) FromCtyValue(in cty.Value, p cty.Path) error {
2323

2424
func (e *Exports) fromCtyValue(in cty.Value, p cty.Path) error {
2525
*e = make([]*ExportEntry, 0, in.LengthInt())
26-
for elem := in.ElementIterator(); elem.Next(); {
27-
_, value := elem.Element()
28-
29-
if isEmpty(value) {
30-
continue
31-
}
32-
26+
for value := range eachElement(in) {
3327
entry := &ExportEntry{}
3428
if err := entry.FromCtyValue(value, p); err != nil {
3529
return err

‎util/buildflags/secrets_cty.go‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,7 @@ func (s *Secrets) FromCtyValue(in cty.Value, p cty.Path) error {
3030

3131
func (s *Secrets) fromCtyValue(in cty.Value, p cty.Path) error {
3232
*s = make([]*Secret, 0, in.LengthInt())
33-
for elem := in.ElementIterator(); elem.Next(); {
34-
_, value := elem.Element()
35-
36-
if isEmpty(value) {
37-
continue
38-
}
39-
33+
for value := range eachElement(in) {
4034
entry := &Secret{}
4135
if err := entry.FromCtyValue(value, p); err != nil {
4236
return err
@@ -71,13 +65,13 @@ func (e *Secret) FromCtyValue(in cty.Value, p cty.Path) error {
7165
return err
7266
}
7367

74-
if id := conv.GetAttr("id"); !id.IsNull() {
68+
if id := conv.GetAttr("id"); !id.IsNull() && id.IsKnown() {
7569
e.ID = id.AsString()
7670
}
77-
if src := conv.GetAttr("src"); !src.IsNull() {
71+
if src := conv.GetAttr("src"); !src.IsNull() && src.IsKnown() {
7872
e.FilePath = src.AsString()
7973
}
80-
if env := conv.GetAttr("env"); !env.IsNull() {
74+
if env := conv.GetAttr("env"); !env.IsNull() && env.IsKnown() {
8175
e.Env = env.AsString()
8276
}
8377
return nil

‎util/buildflags/ssh_cty.go‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,7 @@ func (s *SSHKeys) FromCtyValue(in cty.Value, p cty.Path) error {
3030

3131
func (s *SSHKeys) fromCtyValue(in cty.Value, p cty.Path) error {
3232
*s = make([]*SSH, 0, in.LengthInt())
33-
for elem := in.ElementIterator(); elem.Next(); {
34-
_, value := elem.Element()
35-
36-
if isEmpty(value) {
37-
continue
38-
}
39-
33+
for value := range eachElement(in) {
4034
entry := &SSH{}
4135
if err := entry.FromCtyValue(value, p); err != nil {
4236
return err
@@ -71,10 +65,10 @@ func (e *SSH) FromCtyValue(in cty.Value, p cty.Path) error {
7165
return err
7266
}
7367

74-
if id := conv.GetAttr("id"); !id.IsNull() {
68+
if id := conv.GetAttr("id"); !id.IsNull() && id.IsKnown() {
7569
e.ID = id.AsString()
7670
}
77-
if paths := conv.GetAttr("paths"); !paths.IsNull() {
71+
if paths := conv.GetAttr("paths"); !paths.IsNull() && paths.IsKnown() {
7872
if err := gocty.FromCtyValue(paths, &e.Paths); err != nil {
7973
return err
8074
}

‎util/buildflags/utils.go‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package buildflags
22

33
import (
4+
"iter"
5+
46
"github.com/zclconf/go-cty/cty"
57
"github.com/zclconf/go-cty/cty/gocty"
68
)
@@ -34,7 +36,7 @@ func removeDupes[E comparable[E]](s []E) []E {
3436
}
3537

3638
func getAndDelete(m map[string]cty.Value, attr string, gv interface{}) error {
37-
if v, ok := m[attr]; ok {
39+
if v, ok := m[attr]; ok && v.IsKnown() {
3840
delete(m, attr)
3941
return gocty.FromCtyValue(v, gv)
4042
}
@@ -44,11 +46,28 @@ func getAndDelete(m map[string]cty.Value, attr string, gv interface{}) error {
4446
func asMap(m map[string]cty.Value) map[string]string {
4547
out := make(map[string]string, len(m))
4648
for k, v := range m {
47-
out[k] = v.AsString()
49+
if v.IsKnown() {
50+
out[k] = v.AsString()
51+
}
4852
}
4953
return out
5054
}
5155

52-
func isEmpty(v cty.Value) bool {
53-
return v.Type() == cty.String && v.AsString() == ""
56+
func isEmptyOrUnknown(v cty.Value) bool {
57+
return !v.IsKnown() || (v.Type() == cty.String && v.AsString() == "")
58+
}
59+
60+
func eachElement(in cty.Value) iter.Seq[cty.Value] {
61+
return func(yield func(v cty.Value) bool) {
62+
for elem := in.ElementIterator(); elem.Next(); {
63+
_, value := elem.Element()
64+
if isEmptyOrUnknown(value) {
65+
continue
66+
}
67+
68+
if !yield(value) {
69+
return
70+
}
71+
}
72+
}
5473
}

0 commit comments

Comments
 (0)