diff --git a/hcl2template/testdata/hcp_par/multibuild-hcp-registry-in-build-block.pkr.hcl b/hcl2template/testdata/hcp_par/multibuild-hcp-registry-in-build-block.pkr.hcl new file mode 100644 index 00000000000..82cd0845151 --- /dev/null +++ b/hcl2template/testdata/hcp_par/multibuild-hcp-registry-in-build-block.pkr.hcl @@ -0,0 +1,15 @@ +source "null" "test" { + communicator = "none" +} + +build { + name = "bucket-slug-1" + hcp_packer_registry { + bucket_name = "ok-Bucket-name-1" + } + sources = ["null.test"] +} +build { + name = "bucket-slug-2" + sources = ["null.test"] +} diff --git a/hcl2template/testdata/hcp_par/multibuild-ok.pkr.hcl b/hcl2template/testdata/hcp_par/multibuild-ok.pkr.hcl new file mode 100644 index 00000000000..c4f69c92f0f --- /dev/null +++ b/hcl2template/testdata/hcp_par/multibuild-ok.pkr.hcl @@ -0,0 +1,16 @@ +source "null" "test" { + communicator = "none" +} + +hcp_packer_registry { + bucket_name = "ok-Bucket-name-1" +} + +build { + name = "bucket-slug-1" + sources = ["null.test"] +} +build { + name = "bucket-slug-2" + sources = ["null.test"] +} diff --git a/hcl2template/types.build.hcp_packer_registry_test.go b/hcl2template/types.build.hcp_packer_registry_test.go index 5238c08c224..a5cafbc6af2 100644 --- a/hcl2template/types.build.hcp_packer_registry_test.go +++ b/hcl2template/types.build.hcp_packer_registry_test.go @@ -123,9 +123,8 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { }, false, &getHCPPackerRegistry{ - wantBlock: &HCPPackerRegistryBlock{Slug: "ok-Bucket-name-1"}, wantDiag: true, - wantDiagHasError: false, + wantDiagHasError: true, }, }, {"bucket name OK multiple block second build block", @@ -186,9 +185,8 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { }, false, &getHCPPackerRegistry{ - wantBlock: &HCPPackerRegistryBlock{Slug: "ok-Bucket-name-1"}, wantDiag: true, - wantDiagHasError: false, + wantDiagHasError: true, }, }, {"bucket name OK multiple block multiple declaration", @@ -250,7 +248,6 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { }, false, &getHCPPackerRegistry{ - wantBlock: &HCPPackerRegistryBlock{Slug: "ok-Bucket-name-1"}, wantDiag: true, wantDiagHasError: true, }, @@ -661,6 +658,132 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { wantDiagHasError: true, }, }, - } + + {"multiple build block with Hcp Packer Registry in a build block", + defaultParser, + parseTestArgs{"testdata/hcp_par/multibuild-hcp-registry-in-build-block.pkr.hcl", nil, nil}, + &PackerConfig{ + CorePackerVersionString: lockedVersion, + Basedir: filepath.Join("testdata", "hcp_par"), + Sources: map[SourceRef]SourceBlock{ + refNull: { + Type: "null", + Name: "test", + block: &hcl.Block{ + Type: "source", + }, + }, + }, + Builds: Builds{ + { + Name: "bucket-slug-1", + HCPPackerRegistry: &HCPPackerRegistryBlock{Slug: "ok-Bucket-name-1"}, + Sources: []SourceUseBlock{ + { + SourceRef: refNull, + }, + }, + }, + { + Name: "bucket-slug-2", + Sources: []SourceUseBlock{ + { + SourceRef: refNull, + }, + }, + }, + }, + }, + false, false, + []*packer.CoreBuild{ + &packer.CoreBuild{ + BuildName: "bucket-slug-1", + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + BuilderType: "null", + }, + &packer.CoreBuild{ + BuildName: "bucket-slug-2", + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + BuilderType: "null", + }, + }, + false, + &getHCPPackerRegistry{ + // wantBlock: &HCPPackerRegistryBlock{Slug: "ok-Bucket-name-1"}, + wantDiag: true, + wantDiagHasError: true, + }, + }, + {"multiple build block", + defaultParser, + parseTestArgs{"testdata/hcp_par/multibuild-ok.pkr.hcl", nil, nil}, + &PackerConfig{ + CorePackerVersionString: lockedVersion, + Basedir: filepath.Join("testdata", "hcp_par"), + HCPPackerRegistry: &HCPPackerRegistryBlock{Slug: "ok-Bucket-name-1"}, + Sources: map[SourceRef]SourceBlock{ + refNull: { + Type: "null", + Name: "test", + block: &hcl.Block{ + Type: "source", + }, + }, + }, + Builds: Builds{ + { + Name: "bucket-slug-1", + Sources: []SourceUseBlock{ + { + SourceRef: refNull, + }, + }, + }, + { + Name: "bucket-slug-2", + Sources: []SourceUseBlock{ + { + SourceRef: refNull, + }, + }, + }, + }, + }, + false, false, + []*packer.CoreBuild{ + &packer.CoreBuild{ + BuildName: "bucket-slug-1", + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + BuilderType: "null", + }, + &packer.CoreBuild{ + BuildName: "bucket-slug-2", + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + BuilderType: "null", + }, + }, + false, + &getHCPPackerRegistry{ + wantBlock: &HCPPackerRegistryBlock{Slug: "ok-Bucket-name-1"}, + wantDiag: false, + wantDiagHasError: false, + }, + }} testParse(t, tests) } diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 17039e82aa4..dddb902ecd7 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -650,25 +650,10 @@ func (cfg *PackerConfig) GetHCPPackerRegistryBlock() (*HCPPackerRegistryBlock, h var block *HCPPackerRegistryBlock var diags hcl.Diagnostics - multipleRegistryDiag := func(block *HCPPackerRegistryBlock) *hcl.Diagnostic { - return &hcl.Diagnostic{ - Summary: "Multiple HCP Packer registry block declaration", - Subject: block.HCL2Ref.DefRange.Ptr(), - Severity: hcl.DiagError, - Detail: "Multiple " + buildHCPPackerRegistryLabel + " blocks have been found, only one can be defined " + - "in HCL2 templates. Starting with Packer 1.12.1, it is recommended to move it to the " + - "top-level configuration instead of within a build block.", - } - } - // We start by looking in the build blocks - for _, build := range cfg.Builds { - if build.HCPPackerRegistry != nil { - if block != nil { - // error multiple build block - diags = diags.Append(multipleRegistryDiag(build.HCPPackerRegistry)) - continue - } - block = build.HCPPackerRegistry + // build block leveled hcp registry is only tolerated when there is only one build block + if len(cfg.Builds) == 1 { + if cfg.Builds[0].HCPPackerRegistry != nil { + block = cfg.Builds[0].HCPPackerRegistry diags = diags.Append(&hcl.Diagnostic{ Summary: "Build block level " + buildHCPPackerRegistryLabel + " are deprecated", Subject: &block.DefRange, @@ -677,10 +662,30 @@ func (cfg *PackerConfig) GetHCPPackerRegistryBlock() (*HCPPackerRegistryBlock, h "top-level configuration instead of within a build block.", }) } + } else { + // else we make sure no HCPPackerRegistry has been declared + for _, build := range cfg.Builds { + if build.HCPPackerRegistry != nil { + diags = diags.Append(&hcl.Diagnostic{ + Summary: "Build block level " + buildHCPPackerRegistryLabel + " are illegal in multiple builds", + Subject: &build.HCPPackerRegistry.DefRange, + Severity: hcl.DiagError, + Detail: "When using multiple builds block with HCP Packer, it is mandatory to move " + + "hcp_packer_registry to the top-level configuration instead of within a build block", + }) + } + } } - if block != nil && cfg.HCPPackerRegistry != nil { - diags = diags.Append(multipleRegistryDiag(block)) + diags = diags.Append( + &hcl.Diagnostic{ + Summary: "Multiple HCP Packer registry block declaration", + Subject: block.HCL2Ref.DefRange.Ptr(), + Severity: hcl.DiagError, + Detail: "Multiple " + buildHCPPackerRegistryLabel + " blocks have been found, only one can be defined " + + "in HCL2 templates. Starting with Packer 1.12.1, it is recommended to move it to the " + + "top-level configuration instead of within a build block.", + }) } if cfg.HCPPackerRegistry != nil { diff --git a/internal/hcp/registry/hcl.go b/internal/hcp/registry/hcl.go index 5c6e9d15f60..67281d9132b 100644 --- a/internal/hcp/registry/hcl.go +++ b/internal/hcp/registry/hcl.go @@ -93,18 +93,6 @@ func (h *HCLRegistry) VersionStatusSummary() { func NewHCLRegistry(config *hcl2template.PackerConfig, ui sdkpacker.Ui) (*HCLRegistry, hcl.Diagnostics) { var diags hcl.Diagnostics - if len(config.Builds) > 1 { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Multiple " + buildLabel + " blocks", - Detail: fmt.Sprintf("For HCP Packer Registry enabled builds, only one " + buildLabel + - " block can be defined. Please remove any additional " + buildLabel + - " block(s). If this " + buildLabel + " is not meant for the HCP Packer registry please " + - "clear any HCP_PACKER_* environment variables."), - }) - - return nil, diags - } registryConfig, rcDiags := config.GetHCPPackerRegistryBlock() diags = diags.Extend(rcDiags) @@ -176,9 +164,9 @@ func (h *HCLRegistry) registerAllComponents() 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. @@ -205,23 +193,46 @@ func (h *HCLRegistry) registerAllComponents() hcl.Diagnostics { // If that happens, we then use a combination of both the build name, and // the source type. for _, build := range h.configuration.Builds { + // this set is used to catch duplicate sources in the same block + sources := map[string]struct{}{} for _, source := range build.Sources { + if _, ok := sources[source.String()]; !ok { + sources[source.String()] = struct{}{} + } else { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Source conflicts", + Subject: &build.HCL2Ref.DefRange, + Detail: fmt.Sprintf("Two or more sources are identical inside the same "+ + "build block, there should be only one instance of %s", source.String()), + }) + continue + } if _, ok := conflictSources[source.String()]; !ok { continue } - buildName := source.String() - if build.Name != "" { - buildName = fmt.Sprintf("%s.%s", build.Name, buildName) + if build.Name == "" { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Missing mandatory build name", + Subject: &build.HCL2Ref.DefRange, + Detail: "A build name is required when using the same source in two or" + + " more different builds", + }) + continue } + buildName := fmt.Sprintf("%s.%s", build.Name, source.String()) + 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()), + Detail: fmt.Sprintf("Two sources are used in the same build block or "+ + "two builds have the same name, causing a conflict, there must only "+ + "be one instance of %s", source.String()), }) } h.buildNames[buildName] = struct{}{} diff --git a/internal/hcp/registry/hcl_test.go b/internal/hcp/registry/hcl_test.go index d909baf020d..5ee7f3cdbb3 100644 --- a/internal/hcp/registry/hcl_test.go +++ b/internal/hcp/registry/hcl_test.go @@ -172,7 +172,7 @@ func TestNewRegisterProperBuildName(t *testing.T) { }, "multiple build block with same source create conflict": { expectErr: true, - diagsSummaryContains: "conflict", + diagsSummaryContains: "mandatory build name", builds: hcl2template.Builds{ &hcl2template.BuildBlock{ Sources: []hcl2template.SourceUseBlock{ @@ -237,8 +237,7 @@ func TestNewRegisterProperBuildName(t *testing.T) { }, }, "multiple build block with same source but with only one declared build name": { - expectErr: false, - expectedBuilds: []string{"docker.ubuntu", "build.docker.ubuntu"}, + expectErr: true, builds: hcl2template.Builds{ &hcl2template.BuildBlock{ Name: "build",