hcp: give a proper error when using conflicting build name

It was possible to put the same source 2 times in the build and when
using HCP, it would error eventually since they are considered the same
build from HCP side

internal/hcp/registry/hcl.go

@@ -22,6 +22,7 @@ type HCLRegistry struct {
 	bucket        *Bucket
 	ui            sdkpacker.Ui
 	metadata      *MetadataStore
+	buildNames    map[string]struct{}
 }
 
 const (
@@ -66,7 +67,7 @@ func (h *HCLRegistry) PopulateVersion(ctx context.Context) error {
 
 // StartBuild is invoked when one build for the configuration is starting to be processed
 func (h *HCLRegistry) StartBuild(ctx context.Context, build *packer.CoreBuild) error {
-	return h.bucket.startBuild(ctx, build.Type)
+	return h.bucket.startBuild(ctx, h.HCPBuildName(build))
 }
 
 // CompleteBuild is invoked when one build for the configuration has finished
@@ -76,7 +77,7 @@ func (h *HCLRegistry) CompleteBuild(
 	artifacts []sdkpacker.Artifact,
 	buildErr error,
 ) ([]sdkpacker.Artifact, error) {
-	buildName := build.Type
+	buildName := h.HCPBuildName(build)
 	buildMetadata, envMetadata := build.GetMetadata(), h.metadata
 	err := h.bucket.Version.AddMetadataToBuild(ctx, buildName, buildMetadata, envMetadata)
 	if err != nil {
@@ -144,20 +145,96 @@ func NewHCLRegistry(config *hcl2template.PackerConfig, ui sdkpacker.Ui) (*HCLReg
 		return nil, diags
 	}
 
-	for _, source := range build.Sources {
+	registry := &HCLRegistry{
-		bucket.RegisterBuildForComponent(source.String())
-	}
-
-	ui.Say(fmt.Sprintf("Tracking build on HCP Packer with fingerprint %q", bucket.Version.Fingerprint))
-
-	return &HCLRegistry{
 		configuration: config,
 		bucket:        bucket,
 		ui:            ui,
 		metadata:      &MetadataStore{},
-	}, nil
+		buildNames:    map[string]struct{}{},
+	}
+
+	ui.Say(fmt.Sprintf("Tracking build on HCP Packer with fingerprint %q", bucket.Version.Fingerprint))
+
+	return registry, diags.Extend(registry.registerAllComponents())
+}
+
+func (h *HCLRegistry) registerAllComponents() hcl.Diagnostics {
+	var diags hcl.Diagnostics
+
+	conflictSources := map[string]struct{}{}
+
+	// we currently support only one build block but it will change in the near future
+	for _, build := range h.configuration.Builds {
+		for _, source := range build.Sources {
+			// If we encounter the same source twice, we'll defer
+			// its addition to later, using both the build name
+			// and the source type as the name used for HCP Packer.
+			_, ok := h.buildNames[source.String()]
+			if !ok {
+				h.buildNames[source.String()] = struct{}{}
+				continue
+			}
+
+			conflictSources[source.String()] = struct{}{}
+			// We need to delete it to avoid having a false-positive
+			// when returning the name, since we'll be using
+			// the combination of build name + source.String()
+			delete(h.buildNames, source.String())
+		}
+	}
+
+	// Second pass is to take care of conflicting sources
+	//
+	// If the same source is used twice in the configuration, we need to
+	// have a way to differentiate the two on HCP, as each build should have
+	// a locally unique name.
+	//
+	// If that happens, we then use a combination of both the build name, and
+	// the source type.
+	for _, build := range h.configuration.Builds {
+		for _, source := range build.Sources {
+			if _, ok := conflictSources[source.String()]; !ok {
+				continue
+			}
+
+			buildName := source.String()
+			if build.Name != "" {
+				buildName = fmt.Sprintf("%s.%s", build.Name, buildName)
+			}
+
+			if _, ok := h.buildNames[buildName]; ok {
+				diags = append(diags, &hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Build name conflicts",
+					Subject:  &build.HCL2Ref.DefRange,
+					Detail: fmt.Sprintf("Two sources are used in the same build block, causing "+
+						"a conflict, there must only be one instance of %s", source.String()),
+				})
+			}
+			h.buildNames[buildName] = struct{}{}
+		}
+	}
+
+	if diags.HasErrors() {
+		return diags
+	}
+
+	for buildName := range h.buildNames {
+		h.bucket.RegisterBuildForComponent(buildName)
+	}
+	return diags
 }
 
 func (h *HCLRegistry) Metadata() Metadata {
 	return h.metadata
 }
+
+// HCPBuildName will return the properly formatted string taking name conflict into account
+func (h *HCLRegistry) HCPBuildName(build *packer.CoreBuild) string {
+	_, ok := h.buildNames[build.Type]
+	if ok {
+		return build.Type
+	}
+
+	return fmt.Sprintf("%s.%s", build.BuildName, build.Type)
+}

internal/hcp/registry/hcl_test.go

@@ -0,0 +1,319 @@
+package registry
+
+import (
+	"reflect"
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/packer/hcl2template"
+)
+
+func TestNewRegisterProperBuildName(t *testing.T) {
+	cases := map[string]struct {
+		expectedBuilds       []string
+		expectErr            bool
+		diagsSummaryContains string
+		builds               hcl2template.Builds
+	}{
+		"single build block with single source": {
+			expectErr:      false,
+			expectedBuilds: []string{"docker.ubuntu"},
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+					},
+				},
+			},
+		},
+		"single build block with name and with single source": {
+			expectErr:      false,
+			expectedBuilds: []string{"docker.ubuntu"},
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Name: "my-build-block",
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+					},
+				},
+			},
+		},
+		"single build block with 2 sources": {
+			expectErr:      false,
+			expectedBuilds: []string{"docker.alpine", "docker.ubuntu"},
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "alpine",
+							},
+						},
+					},
+				},
+			},
+		},
+		"single build block with 3 sources": {
+			expectErr:      false,
+			expectedBuilds: []string{"docker.alpine", "docker.ubuntu", "docker.arch"},
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "alpine",
+							},
+						},
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "arch",
+							},
+						},
+					},
+				},
+			},
+		},
+		"single build block with name and multiple sources": {
+			expectErr:      false,
+			expectedBuilds: []string{"docker.alpine", "docker.ubuntu"},
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Name: "my-build-block",
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "alpine",
+							},
+						},
+					},
+				},
+			},
+		},
+		"single build block with multiple identical sources create conflict": {
+			expectErr:            true,
+			diagsSummaryContains: "conflict",
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+					},
+				},
+			},
+		},
+		"multiple build block with different source": {
+			expectErr:      false,
+			expectedBuilds: []string{"docker.alpine", "docker.ubuntu"},
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+					},
+				},
+				&hcl2template.BuildBlock{
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "alpine",
+							},
+						},
+					},
+				},
+			},
+		},
+		"multiple build block with same source create conflict": {
+			expectErr:            true,
+			diagsSummaryContains: "conflict",
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "alpine",
+							},
+						},
+					},
+				},
+				&hcl2template.BuildBlock{
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "alpine",
+							},
+						},
+					},
+				},
+			},
+		},
+		"multiple build block with same source but with different build name": {
+			expectErr:      false,
+			expectedBuilds: []string{"build1.docker.ubuntu", "build2.docker.ubuntu"},
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Name: "build1",
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+					},
+				},
+				&hcl2template.BuildBlock{
+					Name: "build2",
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+					},
+				},
+			},
+		},
+		"multiple build block with same source but with only one declared build name": {
+			expectErr:      false,
+			expectedBuilds: []string{"docker.ubuntu", "build.docker.ubuntu"},
+			builds: hcl2template.Builds{
+				&hcl2template.BuildBlock{
+					Name: "build",
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+					},
+				},
+				&hcl2template.BuildBlock{
+					Sources: []hcl2template.SourceUseBlock{
+						{
+							SourceRef: hcl2template.SourceRef{
+								Type: "docker",
+								Name: "ubuntu",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for desc, tc := range cases {
+		t.Run(desc, func(t *testing.T) {
+
+			config := &hcl2template.PackerConfig{
+				Builds: tc.builds,
+			}
+
+			registry := HCLRegistry{
+				configuration: config,
+				bucket: &Bucket{
+					Name:    "test-bucket-" + desc,
+					Version: &Version{},
+				},
+				buildNames: map[string]struct{}{},
+			}
+
+			diags := registry.registerAllComponents()
+			if tc.diagsSummaryContains != "" {
+
+				containsMsg := false
+				for _, diag := range diags {
+					if strings.Contains(diag.Summary, tc.diagsSummaryContains) {
+						containsMsg = true
+					}
+				}
+				if !containsMsg {
+					t.Fatalf("diagnostics should contains '%s' in summary", tc.diagsSummaryContains)
+				}
+			}
+			if !tc.expectErr {
+				if diags.HasErrors() {
+					t.Fatalf("should not report error diagnostic: %v", diags)
+				}
+			}
+			if tc.expectErr {
+				if !diags.HasErrors() {
+					t.Fatal("should report error in this case")
+				}
+				return
+			}
+
+			actualExpectedBuilds := registry.bucket.Version.expectedBuilds
+
+			slices.Sort(tc.expectedBuilds)
+			slices.Sort(actualExpectedBuilds)
+
+			if !reflect.DeepEqual(tc.expectedBuilds, actualExpectedBuilds) {
+				t.Fatalf("expectedBuilds registered: %v, got: %v", tc.expectedBuilds, actualExpectedBuilds)
+			}
+		})
+	}
+}