Skip to content

Commit

Permalink
buildflags: handle unknown values from cty
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
jsternberg committed Feb 3, 2025
1 parent b76fdca commit 9cc2231
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 42 deletions.
41 changes: 40 additions & 1 deletion bake/hcl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"reflect"
"testing"

hcl "github.com/hashicorp/hcl/v2"
"github.com/stretchr/testify/require"
)

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

func TestHCLAttrsCapsuleTypeVars(t *testing.T) {
func TestHCLAttrsCapsuleType_ObjectVars(t *testing.T) {
dt := []byte(`
variable "foo" {
default = "bar"
Expand Down Expand Up @@ -716,6 +717,44 @@ func TestHCLAttrsCapsuleTypeVars(t *testing.T) {
require.Equal(t, []string{"id=oci,src=/local/secret"}, stringify(web.Secrets))
}

func TestHCLAttrsCapsuleType_MissingVars(t *testing.T) {
dt := []byte(`
target "app" {
attest = [
"type=sbom,disabled=${SBOM}",
]
cache-from = [
{ type = "registry", ref = "user/app:${FOO1}" },
"type=local,src=path/to/cache:${FOO2}",
]
cache-to = [
{ type = "local", dest = "path/to/${BAR}" },
]
output = [
{ type = "oci", dest = "../${OUTPUT}.tar" },
]
secret = [
{ id = "mysecret", src = "/local/${SECRET}" },
]
ssh = [
{ id = "key", paths = ["path/to/${SSH_KEY}"] },
]
}
`)

var diags hcl.Diagnostics
_, err := ParseFile(dt, "docker-bake.hcl")
require.ErrorAs(t, err, &diags)

// One extra error because the value is unknown.
require.Len(t, diags, 8)
}

func TestHCLMultiFileAttrs(t *testing.T) {
dt := []byte(`
variable "FOO" {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/docker/buildx

go 1.22.0
go 1.23.0

require (
github.com/Masterminds/semver/v3 v3.2.1
Expand Down
8 changes: 5 additions & 3 deletions util/buildflags/attests_cty.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ func (e *Attests) FromCtyValue(in cty.Value, p cty.Path) error {

func (e *Attests) fromCtyValue(in cty.Value, p cty.Path) error {
*e = make([]*Attest, 0, in.LengthInt())
for elem := in.ElementIterator(); elem.Next(); {
_, value := elem.Element()

for value := range eachElement(in) {
entry := &Attest{}
if err := entry.FromCtyValue(value, p); err != nil {
return err
Expand Down Expand Up @@ -64,6 +62,10 @@ func (e *Attest) FromCtyValue(in cty.Value, p cty.Path) error {
e.Attrs = map[string]string{}
for it := conv.ElementIterator(); it.Next(); {
k, v := it.Element()
if !v.IsKnown() {
continue
}

switch key := k.AsString(); key {
case "type":
e.Type = v.AsString()
Expand Down
8 changes: 1 addition & 7 deletions util/buildflags/cache_cty.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,7 @@ func (o *CacheOptions) FromCtyValue(in cty.Value, p cty.Path) error {

func (o *CacheOptions) fromCtyValue(in cty.Value, p cty.Path) error {
*o = make([]*CacheOptionsEntry, 0, in.LengthInt())
for elem := in.ElementIterator(); elem.Next(); {
_, value := elem.Element()

if isEmpty(value) {
continue
}

for value := range eachElement(in) {
// Special handling for a string type to handle ref only format.
if value.Type() == cty.String {
entries, err := ParseCacheEntry([]string{value.AsString()})
Expand Down
8 changes: 1 addition & 7 deletions util/buildflags/export_cty.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,7 @@ func (e *Exports) FromCtyValue(in cty.Value, p cty.Path) error {

func (e *Exports) fromCtyValue(in cty.Value, p cty.Path) error {
*e = make([]*ExportEntry, 0, in.LengthInt())
for elem := in.ElementIterator(); elem.Next(); {
_, value := elem.Element()

if isEmpty(value) {
continue
}

for value := range eachElement(in) {
entry := &ExportEntry{}
if err := entry.FromCtyValue(value, p); err != nil {
return err
Expand Down
14 changes: 4 additions & 10 deletions util/buildflags/secrets_cty.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,7 @@ func (s *Secrets) FromCtyValue(in cty.Value, p cty.Path) error {

func (s *Secrets) fromCtyValue(in cty.Value, p cty.Path) error {
*s = make([]*Secret, 0, in.LengthInt())
for elem := in.ElementIterator(); elem.Next(); {
_, value := elem.Element()

if isEmpty(value) {
continue
}

for value := range eachElement(in) {
entry := &Secret{}
if err := entry.FromCtyValue(value, p); err != nil {
return err
Expand Down Expand Up @@ -71,13 +65,13 @@ func (e *Secret) FromCtyValue(in cty.Value, p cty.Path) error {
return err
}

if id := conv.GetAttr("id"); !id.IsNull() {
if id := conv.GetAttr("id"); !id.IsNull() && id.IsKnown() {
e.ID = id.AsString()
}
if src := conv.GetAttr("src"); !src.IsNull() {
if src := conv.GetAttr("src"); !src.IsNull() && src.IsKnown() {
e.FilePath = src.AsString()
}
if env := conv.GetAttr("env"); !env.IsNull() {
if env := conv.GetAttr("env"); !env.IsNull() && env.IsKnown() {
e.Env = env.AsString()
}
return nil
Expand Down
12 changes: 3 additions & 9 deletions util/buildflags/ssh_cty.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,7 @@ func (s *SSHKeys) FromCtyValue(in cty.Value, p cty.Path) error {

func (s *SSHKeys) fromCtyValue(in cty.Value, p cty.Path) error {
*s = make([]*SSH, 0, in.LengthInt())
for elem := in.ElementIterator(); elem.Next(); {
_, value := elem.Element()

if isEmpty(value) {
continue
}

for value := range eachElement(in) {
entry := &SSH{}
if err := entry.FromCtyValue(value, p); err != nil {
return err
Expand Down Expand Up @@ -71,10 +65,10 @@ func (e *SSH) FromCtyValue(in cty.Value, p cty.Path) error {
return err
}

if id := conv.GetAttr("id"); !id.IsNull() {
if id := conv.GetAttr("id"); !id.IsNull() && id.IsKnown() {
e.ID = id.AsString()
}
if paths := conv.GetAttr("paths"); !paths.IsNull() {
if paths := conv.GetAttr("paths"); !paths.IsNull() && paths.IsKnown() {
if err := gocty.FromCtyValue(paths, &e.Paths); err != nil {
return err
}
Expand Down
27 changes: 23 additions & 4 deletions util/buildflags/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package buildflags

import (
"iter"

"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/gocty"
)
Expand Down Expand Up @@ -34,7 +36,7 @@ func removeDupes[E comparable[E]](s []E) []E {
}

func getAndDelete(m map[string]cty.Value, attr string, gv interface{}) error {
if v, ok := m[attr]; ok {
if v, ok := m[attr]; ok && v.IsKnown() {
delete(m, attr)
return gocty.FromCtyValue(v, gv)
}
Expand All @@ -44,11 +46,28 @@ func getAndDelete(m map[string]cty.Value, attr string, gv interface{}) error {
func asMap(m map[string]cty.Value) map[string]string {
out := make(map[string]string, len(m))
for k, v := range m {
out[k] = v.AsString()
if v.IsKnown() {
out[k] = v.AsString()
}
}
return out
}

func isEmpty(v cty.Value) bool {
return v.Type() == cty.String && v.AsString() == ""
func isEmptyOrUnknown(v cty.Value) bool {
return !v.IsKnown() || (v.Type() == cty.String && v.AsString() == "")
}

func eachElement(in cty.Value) iter.Seq[cty.Value] {
return func(yield func(v cty.Value) bool) {
for elem := in.ElementIterator(); elem.Next(); {
_, value := elem.Element()
if isEmptyOrUnknown(value) {
continue
}

if !yield(value) {
return
}
}
}
}

0 comments on commit 9cc2231

Please sign in to comment.