hcp: remove superfluous return value on GetBuilds

This commit irons out one of the pain points of the HCP rework by
introducing a HCPPublisher interface, implemented both by the JSON Core,
and the HCL2 PackerConfig, which keeps a map of the build names used by
Packer to the build names pushed on HCP.

This in turn lets us go back to the old implementation of the GetBuilds
function, which returns a list of (filtered) builds, and eventually an
error if something went wrong while processing.

CHANGELOG.md

@@ -1,5 +1,11 @@
 ## 1.8.6 (Upcoming)
 
+### NOTES:
+* hcp: users of HCP Packer will see some changes in how names are displayed during a
+       packer build for JSON templates. Previously only the name or the type were
+       displayed if both were available, and now both will be displayed. This does
+       not impact the behaviour of options such as only or except, they will continue
+       to work as they did before.
 
 ## 1.8.5 (December 12, 2022)
 

command/build.go

@@ -107,7 +107,7 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int
 		})
 	}
 
-	builds, hcpMap, diags := packerStarter.GetBuilds(packer.GetBuildsOptions{
+	builds, diags := packerStarter.GetBuilds(packer.GetBuildsOptions{
 		Only:    cla.Only,
 		Except:  cla.Except,
 		Debug:   cla.Debug,
@@ -219,7 +219,7 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int
 
 			defer limitParallel.Release(1)
 
-			err := hcpRegistry.StartBuild(buildCtx, hcpMap[name])
+			err := hcpRegistry.StartBuild(buildCtx, b)
 			// Seems odd to require this error check here. Now that it is an error we can just exit with diag
 			if err != nil {
 				// If the build is already done, we skip without a warning
@@ -249,7 +249,7 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int
 
 			runArtifacts, hcperr := hcpRegistry.CompleteBuild(
 				buildCtx,
-				hcpMap[name],
+				b,
 				runArtifacts,
 				err)
 			if hcperr != nil {

command/build_test.go

@@ -1113,6 +1113,137 @@ func TestBuildCommand_ParseArgs(t *testing.T) {
 	}
 }
 
+// TestProvisionerOnlyExcept checks that only/except blocks in provisioners/post-processors behave as expected
+func TestProvisionerAndPostProcessorOnlyExcept(t *testing.T) {
+	tests := []struct {
+		name         string
+		args         []string
+		expectedCode int
+		outputCheck  func(string, string) error
+	}{
+		{
+			"json - only named build",
+			[]string{
+				"-only", "packer",
+				testFixture("provisioners", "provisioner-only-except.json"),
+			},
+			0,
+			func(out, _ string) error {
+				if !strings.Contains(out, "packer provisioner packer and null") {
+					return fmt.Errorf("missing expected provisioner output")
+				}
+
+				if !strings.Contains(out, "packer post-processor packer and null") {
+					return fmt.Errorf("missing expected post-processor output")
+				}
+
+				if strings.Contains(out, "null post-processor") || strings.Contains(out, "null provisioner") {
+					return fmt.Errorf("found traces of unnamed provisioner/post-processor, should not")
+				}
+
+				return nil
+			},
+		},
+		{
+			"json - only unnamed build",
+			[]string{
+				"-only", "null",
+				testFixture("provisioners", "provisioner-only-except.json"),
+			},
+			0,
+			func(out, _ string) error {
+				if !strings.Contains(out, "null provisioner null and null") {
+					return fmt.Errorf("missing expected provisioner output")
+				}
+
+				if !strings.Contains(out, "null post-processor null and null") {
+					return fmt.Errorf("missing expected post-processor output")
+				}
+
+				if strings.Contains(out, "packer post-processor") || strings.Contains(out, "packer provisioner") {
+					return fmt.Errorf("found traces of named provisioner/post-processor, should not")
+				}
+
+				return nil
+			},
+		},
+		{
+			"hcl - only one source build",
+			[]string{
+				"-only", "null.packer",
+				testFixture("provisioners", "provisioner-only-except.pkr.hcl"),
+			},
+			0,
+			func(out, _ string) error {
+				if !strings.Contains(out, "packer provisioner packer and null") {
+					return fmt.Errorf("missing expected provisioner output")
+				}
+
+				if !strings.Contains(out, "packer post-processor packer and null") {
+					return fmt.Errorf("missing expected post-processor output")
+				}
+
+				if strings.Contains(out, "other post-processor") || strings.Contains(out, "other provisioner") {
+					return fmt.Errorf("found traces of other provisioner/post-processor, should not")
+				}
+
+				return nil
+			},
+		},
+		{
+			"hcl - only other build",
+			[]string{
+				"-only", "null.other",
+				testFixture("provisioners", "provisioner-only-except.pkr.hcl"),
+			},
+			0,
+			func(out, _ string) error {
+				if !strings.Contains(out, "other provisioner other and null") {
+					return fmt.Errorf("missing expected provisioner output")
+				}
+
+				if !strings.Contains(out, "other post-processor other and null") {
+					return fmt.Errorf("missing expected post-processor output")
+				}
+
+				if strings.Contains(out, "packer post-processor") || strings.Contains(out, "packer provisioner") {
+					return fmt.Errorf("found traces of \"packer\" source provisioner/post-processor, should not")
+				}
+
+				return nil
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &BuildCommand{
+				Meta: TestMetaFile(t),
+			}
+
+			exitCode := c.Run(tt.args)
+			if exitCode != tt.expectedCode {
+				t.Errorf("process exit code mismatch: expected %d, got %d",
+					tt.expectedCode,
+					exitCode)
+			}
+
+			out, stderr := GetStdoutAndErrFromTestMeta(t, c.Meta)
+			err := tt.outputCheck(out, stderr)
+			if err != nil {
+				if len(out) != 0 {
+					t.Logf("command stdout: %q", out)
+				}
+
+				if len(stderr) != 0 {
+					t.Logf("command stderr: %q", stderr)
+				}
+				t.Error(err.Error())
+			}
+		})
+	}
+}
+
 // TestBuildCmd aims to test the build command, with output validation
 func TestBuildCmd(t *testing.T) {
 	tests := []struct {

command/test-fixtures/provisioners/provisioner-only-except.json

@@ -0,0 +1,37 @@
+{
+  "builders": [
+    {
+      "type": "null",
+      "communicator": "none"
+    },
+    {
+      "type": "null",
+      "name": "packer",
+      "communicator": "none"
+    }
+  ],
+  "provisioners": [
+    {
+      "type": "shell-local",
+      "inline": ["echo packer provisioner {{build_name}} and {{build_type}}"],
+      "only": ["packer"]
+    },
+    {
+      "type": "shell-local",
+      "inline": ["echo null provisioner {{build_name}} and {{build_type}}"],
+      "except": ["packer"]
+    }
+  ],
+  "post-processors": [
+    {
+      "type": "shell-local",
+      "inline": ["echo packer post-processor {{build_name}} and {{build_type}}"],
+      "only": ["packer"]
+    },
+    {
+      "type": "shell-local",
+      "inline": ["echo null post-processor {{build_name}} and {{build_type}}"],
+      "except": ["packer"]
+    }
+  ]
+}

command/test-fixtures/provisioners/provisioner-only-except.pkr.hcl

@@ -0,0 +1,31 @@
+source "null" "packer" {
+	communicator = "none"
+}
+
+source "null" "other" {
+	communicator = "none"
+}
+
+build {
+	sources = ["sources.null.packer", "null.other"]
+
+	provisioner "shell-local" {
+		inline = ["echo packer provisioner {{build_name}} and {{build_type}}"]
+		only   = ["null.packer"]
+	}
+
+	provisioner "shell-local" {
+		inline = ["echo other provisioner {{build_name}} and {{build_type}}"]
+		except = ["null.packer"]
+	}
+
+	post-processor "shell-local" {
+		inline = ["echo packer post-processor {{build_name}} and {{build_type}}"]
+		only   = ["null.packer"]
+	}
+
+	post-processor "shell-local" {
+		inline = ["echo other post-processor {{build_name}} and {{build_type}}"]
+		except = ["null.packer"]
+	}
+}

command/validate.go

@@ -70,7 +70,7 @@ func (c *ValidateCommand) RunContext(ctx context.Context, cla *ValidateArgs) int
 		return ret
 	}
 
-	_, _, diags = packerStarter.GetBuilds(packer.GetBuildsOptions{
+	_, diags = packerStarter.GetBuilds(packer.GetBuildsOptions{
 		Only:   cla.Only,
 		Except: cla.Except,
 	})

hcl2template/common_test.go

@@ -106,7 +106,7 @@ func testParse(t *testing.T, tests []parseTest) {
 				return
 			}
 
-			gotBuilds, _, gotDiags := gotCfg.GetBuilds(packer.GetBuildsOptions{})
+			gotBuilds, gotDiags := gotCfg.GetBuilds(packer.GetBuildsOptions{})
 			if tt.getBuildsWantDiags == (gotDiags == nil) {
 				t.Fatalf("Parser.getBuilds() unexpected diagnostics. %s", gotDiags)
 			}

hcl2template/types.packer_config.go

@@ -564,20 +564,17 @@ func (cfg *PackerConfig) getCoreBuildPostProcessors(source SourceUseBlock, block
 // GetBuilds returns a list of packer Build based on the HCL2 parsed build
 // blocks. All Builders, Provisioners and Post Processors will be started and
 // configured.
-func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Build, map[string]string, hcl.Diagnostics) {
+func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Build, hcl.Diagnostics) {
 	res := []packersdk.Build{}
 	var diags hcl.Diagnostics
 	possibleBuildNames := []string{}
 
-	// hcpTranslationMap maps the local name of a Corebuild to its HCP name
-	hcpTranslationMap := map[string]string{}
-
 	cfg.debug = opts.Debug
 	cfg.force = opts.Force
 	cfg.onError = opts.OnError
 
 	if len(cfg.Builds) == 0 {
-		return res, hcpTranslationMap, append(diags, &hcl.Diagnostic{
+		return res, append(diags, &hcl.Diagnostic{
 			Summary:  "Missing build block",
 			Detail:   "A build block with one or more sources is required for executing a build.",
 			Severity: hcl.DiagError,
@@ -602,8 +599,6 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu
 				Type:      srcUsage.String(),
 			}
 
-			hcpTranslationMap[pcb.Name()] = srcUsage.String()
-
 			pcb.SetDebug(cfg.debug)
 			pcb.SetForce(cfg.force)
 			pcb.SetOnError(cfg.onError)
@@ -615,7 +610,7 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu
 			if len(opts.Only) > 0 {
 				onlyGlobs, diags := convertFilterOption(opts.Only, "only")
 				if diags.HasErrors() {
-					return nil, nil, diags
+					return nil, diags
 				}
 				cfg.only = onlyGlobs
 				include := false
@@ -635,7 +630,7 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu
 			if len(opts.Except) > 0 {
 				exceptGlobs, diags := convertFilterOption(opts.Except, "except")
 				if diags.HasErrors() {
-					return nil, nil, diags
+					return nil, diags
 				}
 				cfg.except = exceptGlobs
 				exclude := false
@@ -732,7 +727,7 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu
 				"These could also be matched with a glob pattern like: 'happycloud.*'", possibleBuildNames),
 		})
 	}
-	return res, hcpTranslationMap, diags
+	return res, diags
 }
 
 var PackerConsoleHelp = strings.TrimSpace(`

internal/hcp/registry/hcl.go

@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/hcp-sdk-go/clients/cloud-packer-service/stable/2021-04-30/models"
 	sdkpacker "github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/hashicorp/packer/hcl2template"
+	"github.com/hashicorp/packer/packer"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/gocty"
 )
@@ -45,18 +46,28 @@ func (h *HCLMetadataRegistry) PopulateIteration(ctx context.Context) error {
 }
 
 // StartBuild is invoked when one build for the configuration is starting to be processed
-func (h *HCLMetadataRegistry) StartBuild(ctx context.Context, buildName string) error {
-	return h.bucket.startBuild(ctx, buildName)
+func (h *HCLMetadataRegistry) StartBuild(ctx context.Context, build sdkpacker.Build) error {
+	name := build.Name()
+	cb, ok := build.(*packer.CoreBuild)
+	if ok {
+		name = cb.Type
+	}
+	return h.bucket.startBuild(ctx, name)
 }
 
 // CompleteBuild is invoked when one build for the configuration has finished
 func (h *HCLMetadataRegistry) CompleteBuild(
 	ctx context.Context,
-	buildName string,
+	build sdkpacker.Build,
 	artifacts []sdkpacker.Artifact,
 	buildErr error,
 ) ([]sdkpacker.Artifact, error) {
-	return h.bucket.completeBuild(ctx, buildName, artifacts, buildErr)
+	name := build.Name()
+	cb, ok := build.(*packer.CoreBuild)
+	if ok {
+		name = cb.Type
+	}
+	return h.bucket.completeBuild(ctx, name, artifacts, buildErr)
 }
 
 func NewHCLMetadataRegistry(config *hcl2template.PackerConfig) (*HCLMetadataRegistry, hcl.Diagnostics) {