diff --git a/CHANGELOG.md b/CHANGELOG.md index f69148195..b5bc6a55b 100644 --- a/CHANGELOG.md +++ b/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 + ".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: diff --git a/command/build.go b/command/build.go index 1cf9cdd61..66a2ec4c9 100644 --- a/command/build.go +++ b/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 { diff --git a/command/build_test.go b/command/build_test.go index a306fbde2..8aa30c720 100644 --- a/command/build_test.go +++ b/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 { diff --git a/command/test-fixtures/provisioners/provisioner-only-except.json b/command/test-fixtures/provisioners/provisioner-only-except.json new file mode 100644 index 000000000..6eee42387 --- /dev/null +++ b/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"] + } + ] +} diff --git a/command/test-fixtures/provisioners/provisioner-only-except.pkr.hcl b/command/test-fixtures/provisioners/provisioner-only-except.pkr.hcl new file mode 100644 index 000000000..811896538 --- /dev/null +++ b/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"] + } +} diff --git a/command/validate.go b/command/validate.go index 657d71ebc..08eced959 100644 --- a/command/validate.go +++ b/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, }) diff --git a/hcl2template/common_test.go b/hcl2template/common_test.go index d94cf945a..5b1c68fbd 100644 --- a/hcl2template/common_test.go +++ b/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) } diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 8325d0e90..ef67f33e7 100644 --- a/hcl2template/types.packer_config.go +++ b/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(` diff --git a/internal/hcp/registry/hcl.go b/internal/hcp/registry/hcl.go index ccbc1f577..baa2b5cb2 100644 --- a/internal/hcp/registry/hcl.go +++ b/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) { diff --git a/internal/hcp/registry/json.go b/internal/hcp/registry/json.go index 20e1f808e..f15c79aed 100644 --- a/internal/hcp/registry/json.go +++ b/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) } diff --git a/internal/hcp/registry/null_registry.go b/internal/hcp/registry/null_registry.go index fba9db4be..dbab4197d 100644 --- a/internal/hcp/registry/null_registry.go +++ b/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) { diff --git a/internal/hcp/registry/registry.go b/internal/hcp/registry/registry.go index e9aa3f9f1..16e82eb7f 100644 --- a/internal/hcp/registry/registry.go +++ b/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. diff --git a/packer/core.go b/packer/core.go index 40b6560ed..e8da2b2ee 100644 --- a/packer/core.go +++ b/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. diff --git a/packer/run_interfaces.go b/packer/run_interfaces.go index 0eff3fa4c..418da184c 100644 --- a/packer/run_interfaces.go +++ b/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 {