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

@@ -11,6 +11,14 @@
      display the proper filename and line number where the unknown reference
      resides. [GH-12167](https://github.com/hashicorp/packer/pull/12167)
 
+### NOTES:
+* core: Users will see some changes in how names are displayed during a Packer
+     build for JSON templates. Previously only the builder type or the builder
+     name, if it was set, would be displayed. Now for named builders
+     (`"name":"mybuilder"`) the builder'ss type and name will be displayed (i.e
+     "<type>.mybuilder". This does not impact the behavior of options such as
+     only or except, they will continue to work as they did before.)
+
 ## 1.8.5 (December 12, 2022)
 
 ### NOTES:

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) {

internal/hcp/registry/json.go

@@ -2,6 +2,7 @@ package registry
 
 import (
 	"context"
+	"fmt"
 	"path/filepath"
 
 	"github.com/hashicorp/hcl/v2"
@@ -27,8 +28,17 @@ func NewJSONMetadataRegistry(config *packer.Core) (*JSONMetadataRegistry, hcl.Di
 	}
 
 	for _, b := range config.Template.Builders {
+		buildName := b.Name
+
+		// By default, if the name is unspecified, it will be assigned the type
+		//
+		// If the two are different, we can compose the HCP build name from both
+		if b.Name != b.Type {
+			buildName = fmt.Sprintf("%s.%s", b.Type, b.Name)
+		}
+
 		// Get all builds slated within config ignoring any only or exclude flags.
-		bucket.RegisterBuildForComponent(packer.HCPName(b))
+		bucket.RegisterBuildForComponent(buildName)
 	}
 
 	return &JSONMetadataRegistry{
@@ -57,16 +67,16 @@ func (h *JSONMetadataRegistry) PopulateIteration(ctx context.Context) error {
 }
 
 // StartBuild is invoked when one build for the configuration is starting to be processed
-func (h *JSONMetadataRegistry) StartBuild(ctx context.Context, buildName string) error {
-	return h.bucket.startBuild(ctx, buildName)
+func (h *JSONMetadataRegistry) StartBuild(ctx context.Context, build sdkpacker.Build) error {
+	return h.bucket.startBuild(ctx, build.Name())
 }
 
 // CompleteBuild is invoked when one build for the configuration has finished
 func (h *JSONMetadataRegistry) 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)
+	return h.bucket.completeBuild(ctx, build.Name(), artifacts, buildErr)
 }

internal/hcp/registry/null_registry.go

@@ -13,13 +13,13 @@ func (r nullRegistry) PopulateIteration(context.Context) error {
 	return nil
 }
 
-func (r nullRegistry) StartBuild(context.Context, string) error {
+func (r nullRegistry) StartBuild(context.Context, sdkpacker.Build) error {
 	return nil
 }
 
 func (r nullRegistry) CompleteBuild(
 	ctx context.Context,
-	buildName string,
+	build sdkpacker.Build,
 	artifacts []sdkpacker.Artifact,
 	buildErr error,
 ) ([]sdkpacker.Artifact, error) {

internal/hcp/registry/registry.go

@@ -14,8 +14,8 @@ import (
 type Registry interface {
 	//Configure(packer.Handler)
 	PopulateIteration(context.Context) error
-	StartBuild(context.Context, string) error
-	CompleteBuild(ctx context.Context, buildName string, artifacts []sdkpacker.Artifact, buildErr error) ([]sdkpacker.Artifact, error)
+	StartBuild(context.Context, sdkpacker.Build) error
+	CompleteBuild(ctx context.Context, build sdkpacker.Build, artifacts []sdkpacker.Artifact, buildErr error) ([]sdkpacker.Artifact, error)
 }
 
 // New instanciates the appropriate registry for the Packer configuration template type.

packer/core.go

@@ -268,10 +268,9 @@ func (c *Core) generateCoreBuildProvisioner(rawP *template.Provisioner, rawName
 
 // This is used for json templates to launch the build plugins.
 // They will be prepared via b.Prepare() later.
-func (c *Core) GetBuilds(opts GetBuildsOptions) ([]packersdk.Build, map[string]string, hcl.Diagnostics) {
+func (c *Core) GetBuilds(opts GetBuildsOptions) ([]packersdk.Build, hcl.Diagnostics) {
 	buildNames := c.BuildNames(opts.Only, opts.Except)
 	builds := []packersdk.Build{}
-	hcpTranslationMap := map[string]string{}
 	diags := hcl.Diagnostics{}
 	for _, n := range buildNames {
 		b, err := c.Build(n)
@@ -284,8 +283,6 @@ func (c *Core) GetBuilds(opts GetBuildsOptions) ([]packersdk.Build, map[string]s
 			continue
 		}
 
-		hcpTranslationMap[n] = HCPName(c.builds[n])
-
 		// Now that build plugin has been launched, call Prepare()
 		log.Printf("Preparing build: %s", b.Name())
 		b.SetDebug(opts.Debug)
@@ -315,25 +312,7 @@ func (c *Core) GetBuilds(opts GetBuildsOptions) ([]packersdk.Build, map[string]s
 			}
 		}
 	}
-	return builds, hcpTranslationMap, diags
-}
-
-// HCPName is a helper to get a curated HCP name for a legacy JSON builder.
-//
-// In order to make the naming scheme between HCL2 and JSON more consistent,
-// we implement a similar kind of logic on both template types.
-//
-// This means that when for HCL2 templates we have a build name formed of
-// the source type and the source name, we will do the name here for JSON.
-func HCPName(builder *template.Builder) string {
-	// By default, if the name is unspecified, it will be assigned the type
-	//
-	// No need to repeat ourselves here, so we can keep the current behaviour
-	if builder.Name == builder.Type {
-		return builder.Name
-	}
-
-	return fmt.Sprintf("%s.%s", builder.Type, builder.Name)
+	return builds, diags
 }
 
 // Build returns the Build object for the given name.
@@ -441,8 +420,8 @@ func (c *Core) Build(n string) (packersdk.Build, error) {
 
 	// Return a structure that contains the plugins, their types, variables, and
 	// the raw builder config loaded from the json template
-	return &CoreBuild{
-		Type:               n,
+	cb := &CoreBuild{
+		Type:               configBuilder.Name,
 		Builder:            builder,
 		BuilderConfig:      configBuilder.Config,
 		BuilderType:        configBuilder.Type,
@@ -451,7 +430,13 @@ func (c *Core) Build(n string) (packersdk.Build, error) {
 		CleanupProvisioner: cleanupProvisioner,
 		TemplatePath:       c.Template.Path,
 		Variables:          c.variables,
-	}, nil
+	}
+
+	if configBuilder.Type != configBuilder.Name {
+		cb.BuildName = configBuilder.Type
+	}
+
+	return cb, nil
 }
 
 // Context returns an interpolation context.

packer/run_interfaces.go

@@ -21,7 +21,7 @@ type BuildGetter interface {
 	// GetBuilds return all possible builds for a config. It also starts all
 	// builders.
 	// TODO(azr): rename to builder starter ?
-	GetBuilds(GetBuildsOptions) ([]packersdk.Build, map[string]string, hcl.Diagnostics)
+	GetBuilds(GetBuildsOptions) ([]packersdk.Build, hcl.Diagnostics)
 }
 
 type Evaluator interface {