Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buildflags: handle unknown values from cty #2965

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jsternberg
Copy link
Collaborator

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.

Fixes #2960.

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]>
Comment on lines +722 to +747
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}"] },
]
}
Copy link
Member

@crazy-max crazy-max Feb 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was looking at how this example behaves in Buildx 0.19 and I got 13 diagnostics:

docker buildx bake --print
#1 [internal] load local bake definitions
#1 reading docker-bake.hcl 451B / 451B done
#1 DONE 0.0s
docker-bake.hcl:6
--------------------
   4 |                  ]
   5 |                  cache-from = [
   6 | >>>                      { type = "registry", ref = "user/app:${FOO1}" },
   7 |           "type=local,src=path/to/cache:${FOO2}",
   8 |                  ]
--------------------
ERROR: docker-bake.hcl:6,43-47: Unknown variable; There is no variable named "FOO1"., and 12 other diagnostic(s)

But here I got 8 diagnostics:

docker buildx bake --print
#1 [internal] load local bake definitions
#1 reading docker-bake.hcl 451B / 451B done
#1 DONE 0.0s
docker-bake.hcl:13
--------------------
  11 |                  ]
  12 |                  output = [
  13 | >>>                      { type = "oci", dest = "../${OUTPUT}.tar" },
  14 |                  ]
  15 |                  secret = [
--------------------
ERROR: docker-bake.hcl:13,33-39: Unknown variable; There is no variable named "OUTPUT"., and 7 other diagnostic(s)

Not for this PR but wonder if we could show all diags when --debug is set?

nit: Also seems to return OUTPUT as first diag with this PR but in older release returns what seems to be the first one FOO1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bake: panic value is unknown
2 participants