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

@@ -18,10 +18,11 @@ import (
 
 // HCLRegistry is a HCP handler made for handling HCL configurations
 type HCLRegistry struct {
-	configuration *hcl2template.PackerConfig
-	bucket        *Bucket
-	ui            sdkpacker.Ui
-	metadata      *MetadataStore
+	configuration  *hcl2template.PackerConfig
+	bucket         *Bucket
+	ui             sdkpacker.Ui
+	metadata       *MetadataStore
+	fullBuildNames fullBuildNames
 }
 
 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.fullBuildNames.get(build.BuildName, build.Type))
 }
 
 // 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.fullBuildNames.get(build.BuildName, build.Type)
 	buildMetadata, envMetadata := build.GetMetadata(), h.metadata
 	err := h.bucket.Version.AddMetadataToBuild(ctx, buildName, buildMetadata, envMetadata)
 	if err != nil {
@@ -144,20 +145,72 @@ func NewHCLRegistry(config *hcl2template.PackerConfig, ui sdkpacker.Ui) (*HCLReg
 		return nil, diags
 	}
 
-	for _, source := range build.Sources {
-		bucket.RegisterBuildForComponent(source.String())
+	registry := &HCLRegistry{
+		configuration:  config,
+		bucket:         bucket,
+		ui:             ui,
+		metadata:       &MetadataStore{},
+		fullBuildNames: make(fullBuildNames),
 	}
 
 	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
+	return registry, registry.registerAllComponents()
+}
+
+func (h *HCLRegistry) registerAllComponents() hcl.Diagnostics {
+	var diags hcl.Diagnostics
+
+	// 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 !h.fullBuildNames.set(build.Name, source.String()) {
+				diags = append(diags, &hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Build name conflicts",
+					Subject:  &build.HCL2Ref.DefRange,
+					Detail: "When using HCP registry, build names are used as identifier. " +
+						"In this case, there is 2 or more build called " + h.fullBuildNames.get(build.Name, source.String()),
+				})
+			}
+		}
+	}
+
+	if diags.HasErrors() {
+		return diags
+	}
+
+	for sourceName, buildNames := range h.fullBuildNames {
+		for buildName := range buildNames {
+			h.bucket.RegisterBuildForComponent(h.fullBuildNames.get(buildName, sourceName))
+		}
+	}
+	return diags
 }
 
 func (h *HCLRegistry) Metadata() Metadata {
 	return h.metadata
 }
+
+type fullBuildNames map[string]map[string]struct{}
+
+// set will return true if properly inserted and false if the values are conflicting
+func (b fullBuildNames) set(buildName string, sourceName string) bool {
+	if _, ok := b[sourceName]; !ok {
+		b[sourceName] = map[string]struct{}{buildName: {}}
+		return true
+	}
+	if _, ok := b[sourceName][buildName]; !ok {
+		b[sourceName][buildName] = struct{}{}
+		return true
+	}
+	return false
+}
+
+// get will return the properly formatted string taking name conflict into account
+func (b fullBuildNames) get(buildName string, sourceName string) string {
+	if len(b[sourceName]) > 1 {
+		return fmt.Sprintf("%s.%s", buildName, sourceName)
+	}
+	return sourceName
+}

internal/hcp/registry/hcl_test.go

@@ -0,0 +1,264 @@
+package registry
+
+import (
+	"reflect"
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/packer/hcl2template"
+)
+
+func TestNewRegisterProperBuildName(t *testing.T) {
+	cases := map[string]struct {
+		expectedBuilds       []string
+		err                  bool
+		diagsSummaryContains string
+		builds               hcl2template.Builds
+	}{
+		"single build block with single source": {
+			err:            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": {
+			err:            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 multiple sources": {
+			err:            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 name and multiple sources": {
+			err:            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": {
+			err:                  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": {
+			err:            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": {
+			err:                  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": {
+			err:            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",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	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{},
+				},
+				fullBuildNames: map[string]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.err {
+				if diags.HasErrors() {
+					t.Fatalf("should not report error diagnostic: %v", diags)
+				}
+			}
+			if tc.err {
+				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)
+			}
+		})
+	}
+}