diff --git a/internal/hcp/registry/hcl.go b/internal/hcp/registry/hcl.go index 019b4d860..adbe8b4df 100644 --- a/internal/hcp/registry/hcl.go +++ b/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 { - bucket.RegisterBuildForComponent(source.String()) - } - - ui.Say(fmt.Sprintf("Tracking build on HCP Packer with fingerprint %q", bucket.Version.Fingerprint)) - - return &HCLRegistry{ + registry := &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) +} diff --git a/internal/hcp/registry/hcl_test.go b/internal/hcp/registry/hcl_test.go new file mode 100644 index 000000000..d909baf02 --- /dev/null +++ b/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) + } + }) + } +}