bake: merge targets and vars from multiple JSON files

[crazy-max]

fixes #756
fixes docker/bake-action#61

while checking the issue reported in docker/bake-action#61, I found out that the HCL diagnostic being reported was wrong:

  /usr/bin/docker buildx bake --file ./docker-bake.hcl --file /tmp/docker-metadata-action-GGKyIw/docker-metadata-action-bake.json --file /tmp/docker-metadata-action-fZ5d0s/docker-metadata-action-bake.json --file /tmp/docker-metadata-action-toPOSl/docker-metadata-action-bake.json --file /tmp/docker-metadata-action-Cqo0nf/docker-metadata-action-bake.json --metadata-file /tmp/docker-build-push-Rpx0C2/metadata-file binary dind proxy tools --print
  ./docker-bake.hcl:2
  --------------------
     1 |     // filled by GitHub Actions
     2 | >>> target "docker-metadata-k3d" {}
     3 |     target "docker-metadata-k3d-dind" {}
     4 |     target "docker-metadata-k3d-proxy" {}
  --------------------
  error: ./docker-bake.hcl:2,1-7: Unexpected "target" block; Blocks are not allowed here., and 3 other diagnostic(s)
  Error: The process '/usr/bin/docker' failed with exit code 1

Blocks are not allowed here. should be filtered out as it's ignored while checking HCL attributes:

buildx/bake/hclparser/hclparser.go

Line 306 in 1ca30a5

if d.Detail != "Blocks are not allowed here." {

Now that the diag is filtered out we have the right diagnostic being reported: https://github.com/crazy-max/buildx-buildkit-tests/tree/main/bake-action-61

$ docker buildx bake -f docker-bake.hcl -f json-target/binary.json -f json-target/dind.json -f json-target/proxy.json binary --print
json-target/dind.json:2
--------------------
   1 |     {
   2 | >>>   "target": {
   3 |         "docker-metadata-k3d-dind": {
   4 |           "tags": [
--------------------
error: json-target/dind.json:2,3-11: Duplicate argument; Argument "target" was already set at json-target/binary.json:2,3-11, and 1 other diagnostic(s)

This PR also filters out the diagnostic Argument "x" was already set at only for reserved names so we can merge targets from multiple JSON files: https://github.com/crazy-max/buildx-buildkit-tests/tree/main/buildx-756

$ docker buildx bake -f docker-bake.json -f docker-bake.override.json --print
{
  "target": {
    "default": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "tags": [
        "test123"
      ]
    }
  }
}

Same with https://github.com/crazy-max/buildx-buildkit-tests/tree/main/bake-action-61

$ docker buildx bake -f docker-bake.hcl -f json-target/binary.json -f json-target/dind.json -f json-target/proxy.json binary --print
{
  "group": {
    "default": {
      "targets": [
        "binary"
      ]
    }
  },
  "target": {
    "binary": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "args": {
        "DOCKER_META_IMAGES": "ghcr.io/k3d-io/k3d",
        "DOCKER_META_VERSION": "5.4.0-dev.0"
      },
      "labels": {
        "org.opencontainers.image.created": "2022-03-14T13:36:05.512Z",
        "org.opencontainers.image.description": "Little helper to run CNCF's k3s in Docker",
        "org.opencontainers.image.licenses": "MIT",
        "org.opencontainers.image.revision": "984839e5b4c30f46510f314340f77911099cb3dc",
        "org.opencontainers.image.source": "https://github.com/k3d-io/k3d",
        "org.opencontainers.image.title": "k3d",
        "org.opencontainers.image.url": "https://github.com/k3d-io/k3d",
        "org.opencontainers.image.version": "5.4.0-dev.0"
      },
      "tags": [
        "ghcr.io/k3d-io/k3d:5.4.0-dev.0",
        "ghcr.io/k3d-io/k3d:sha-984839e"
      ],
      "target": "binary"
    }
  }
}

@tonistiigi I'm not sure if we should skip Argument "x" was already set at diags for all reserved names or only skip for the target one.

cc @felipecrs @iwilltry42

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

[tonistiigi]

Please also add tests that actually check the attributes feature with multiple files, not only the reserved blocks.

[crazy-max]

Last commit adds support for vars too.

[tonistiigi]

Does it still work when hcl also defines ABC = "old" and foo.json "ABC": "old2"

[crazy-max]

Indeed I tried with attributes in global scope an doesn't seem to work atm (see last commit)

=== RUN   TestCombineHCLAndJSONAttrs
    hcl_test.go:790: 
        	Error Trace:	hcl_test.go:790
        	Error:      	Not equal: 
        	            	expected: map[string]string{"a":"pre-ghi-jkl"}
        	            	actual  : map[string]string{"a":"pre-foo-jkl"}

I guess it's how merging blocks are handled for attributes in global scope where only the first one is retained atm:

ABC = "foo"
variable "DEF" {
  default = ""
}
group "default" {
  targets = ["one"]
}
target "one" {
  args = {
    a = "pre-${ABC}"
  }
}
target "two" {
  args = {
    b = "pre-${DEF}"
  }
}

{"ABC": "oof", "variable": {"DEF": {"default": "bar"}}, "target": { "one": { "args": {"a": "pre-${ABC}-${DEF}"}} } }

{"ABC": "ghi", "DEF": "jkl"}

[tonistiigi]

Does it at least work with HCL-only?

[crazy-max]

Same with HCL only (see test in last commit) 😞:

=== RUN   TestCombineHCLAttrs
    hcl_test.go:801: 
        	Error Trace:	hcl_test.go:801
        	Error:      	Not equal: 
        	            	expected: map[string]string{"a":"pre-ghi-jkl"}
        	            	actual  : map[string]string{"a":"pre-foo-jkl"}

[crazy-max]

Looks like we missed a case in #541. Changing this test with a global attr FOO = "abc" in first file makes it failed:

func TestHCLMultiFileAttrs(t *testing.T) {
	os.Unsetenv("FOO")
	dt := []byte(`
		FOO = "abc"
		target "app" {
			args = {
				v1 = "pre-${FOO}"
			}
		}
		`)
	dt2 := []byte(`
		FOO="def"
		`)

	c, err := ParseFiles([]File{
		{Data: dt, Name: "c1.hcl"},
		{Data: dt2, Name: "c2.hcl"},
	}, nil)
	require.NoError(t, err)
	require.Equal(t, 1, len(c.Targets))
	require.Equal(t, c.Targets[0].Name, "app")
	require.Equal(t, "pre-def", c.Targets[0].Args["v1"])

	os.Setenv("FOO", "ghi")

	c, err = ParseFiles([]File{
		{Data: dt, Name: "c1.hcl"},
		{Data: dt2, Name: "c2.hcl"},
	}, nil)
	require.NoError(t, err)

	require.Equal(t, 1, len(c.Targets))
	require.Equal(t, c.Targets[0].Name, "app")
	require.Equal(t, "pre-ghi", c.Targets[0].Args["v1"])
}

=== RUN   TestHCLMultiFileAttrs
    hcl_test.go:488: 
        	Error Trace:	hcl_test.go:488
        	Error:      	Not equal: 
        	            	expected: "pre-def"
        	            	actual  : "pre-abc"

[crazy-max]

Ah JustAttributes skips repeated attributes: https://github.com/hashicorp/hcl/blob/c7ee8b78101c33b4dfed2641d78cf5e9651eabb8/merged.go#L109-L120

We might need to bring in this func in the hclparser to fix that for us wdyt?

CI failing

[crazy-max]

@tonistiigi As discussed, attributes merge support has been removed. Only targets and vars are supported atm. Will be done in a follow-up.