From c0d4fac78971d25d461260f03a2f843d19a5d74c Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Fri, 1 Mar 2024 11:21:02 -0500 Subject: [PATCH] packer: make GetBuilds return *[]CoreBuild The GetBuilds function, available on both HCL2 and legacy JSON configuration objects, used to return the Build interface. This typing by interface is not useful in this instance, since all the uses of `GetBuilds' are self-contained within Packer, and we're never using any other implementation for it than `*CoreBuild`. We've been relying on the dynamic type for all the builds being *CoreBuild in some places of the code, so to avoid potential surprises in the future, we'll change the signature now so it returns only concrete types. --- command/build.go | 2 +- hcl2template/common_test.go | 2 +- .../types.build.hcp_packer_registry_test.go | 9 +- hcl2template/types.build_test.go | 29 +++---- hcl2template/types.datasource_test.go | 5 +- hcl2template/types.packer_config.go | 5 +- hcl2template/types.packer_config_test.go | 13 ++- hcl2template/types.source_test.go | 5 +- hcl2template/types.variables_test.go | 86 +++++++++---------- packer/core.go | 6 +- packer/run_interfaces.go | 2 +- 11 files changed, 79 insertions(+), 85 deletions(-) diff --git a/command/build.go b/command/build.go index 4727e195d..9ccf9755b 100644 --- a/command/build.go +++ b/command/build.go @@ -162,7 +162,7 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int packer.UiColorYellow, packer.UiColorBlue, } - buildUis := make(map[packersdk.Build]packersdk.Ui) + buildUis := make(map[*packer.CoreBuild]packersdk.Ui) for i := range builds { ui := c.Ui if cla.Color { diff --git a/hcl2template/common_test.go b/hcl2template/common_test.go index 08cdc3f67..fe5663cf1 100644 --- a/hcl2template/common_test.go +++ b/hcl2template/common_test.go @@ -72,7 +72,7 @@ type parseTest struct { parseWantDiags bool parseWantDiagHasErrors bool - getBuildsWantBuilds []packersdk.Build + getBuildsWantBuilds []*packer.CoreBuild getBuildsWantDiags bool // getBuildsWantDiagHasErrors bool } diff --git a/hcl2template/types.build.hcp_packer_registry_test.go b/hcl2template/types.build.hcp_packer_registry_test.go index 54103f7cb..31f9707f2 100644 --- a/hcl2template/types.build.hcp_packer_registry_test.go +++ b/hcl2template/types.build.hcp_packer_registry_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/hashicorp/hcl/v2" - packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/hashicorp/packer/builder/null" "github.com/hashicorp/packer/packer" "github.com/zclconf/go-cty/cty" @@ -48,7 +47,7 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ BuildName: "bucket-slug", Type: "null.test", @@ -91,7 +90,7 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "virtualbox-iso.ubuntu-1204", Prepared: true, @@ -145,7 +144,7 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "virtualbox-iso.ubuntu-1204", Prepared: true, @@ -276,7 +275,7 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ BuildName: "bucket-slug", Type: "null.test", diff --git a/hcl2template/types.build_test.go b/hcl2template/types.build_test.go index d94dee4f8..08120e243 100644 --- a/hcl2template/types.build_test.go +++ b/hcl2template/types.build_test.go @@ -7,7 +7,6 @@ import ( "path/filepath" "testing" - packersdk "github.com/hashicorp/packer-plugin-sdk/packer" . "github.com/hashicorp/packer/hcl2template/internal" "github.com/hashicorp/packer/packer" "github.com/zclconf/go-cty/cty" @@ -55,7 +54,7 @@ func TestParse_build(t *testing.T) { }, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, true, }, {"untyped provisioner", @@ -104,7 +103,7 @@ func TestParse_build(t *testing.T) { }, }, true, true, - []packersdk.Build{&packer.CoreBuild{ + []*packer.CoreBuild{&packer.CoreBuild{ Provisioners: []packer.CoreBuildProvisioner{}, }}, false, @@ -120,7 +119,7 @@ func TestParse_build(t *testing.T) { }, }, true, true, - []packersdk.Build{&packer.CoreBuild{ + []*packer.CoreBuild{&packer.CoreBuild{ Builder: emptyMockBuilder, CleanupProvisioner: packer.CoreBuildProvisioner{ PType: "shell-local", @@ -145,7 +144,7 @@ func TestParse_build(t *testing.T) { Builds: nil, }, true, true, - []packersdk.Build{&packer.CoreBuild{}}, + []*packer.CoreBuild{&packer.CoreBuild{}}, false, }, {"nonexistent post-processor", @@ -184,7 +183,7 @@ func TestParse_build(t *testing.T) { }, }, true, true, - []packersdk.Build{&packer.CoreBuild{ + []*packer.CoreBuild{&packer.CoreBuild{ PostProcessors: [][]packer.CoreBuildPostProcessor{}, }}, true, @@ -198,7 +197,7 @@ func TestParse_build(t *testing.T) { Builds: nil, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, {"named build", @@ -225,7 +224,7 @@ func TestParse_build(t *testing.T) { }, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, true, }, {"post-processor with only and except", @@ -280,7 +279,7 @@ func TestParse_build(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "virtualbox-iso.ubuntu-1204", BuilderType: "virtualbox-iso", @@ -397,7 +396,7 @@ func TestParse_build(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "virtualbox-iso.ubuntu-1204", BuilderType: "virtualbox-iso", @@ -488,7 +487,7 @@ func TestParse_build(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "virtualbox-iso.ubuntu-1204", BuilderType: "virtualbox-iso", @@ -551,7 +550,7 @@ func TestParse_build(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ BuildName: "build-name", Type: "virtualbox-iso.ubuntu-1204", @@ -574,7 +573,7 @@ func TestParse_build(t *testing.T) { Builds: nil, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, {"use build.name in post-processor block", @@ -606,7 +605,7 @@ func TestParse_build(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ BuildName: "test-build", Type: "virtualbox-iso.ubuntu-1204", @@ -664,7 +663,7 @@ func TestParse_build(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ BuildName: "build-name-test", Type: "virtualbox-iso.ubuntu-1204", diff --git a/hcl2template/types.datasource_test.go b/hcl2template/types.datasource_test.go index f539af994..14f399599 100644 --- a/hcl2template/types.datasource_test.go +++ b/hcl2template/types.datasource_test.go @@ -7,7 +7,6 @@ import ( "path/filepath" "testing" - packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/hashicorp/packer/builder/null" "github.com/hashicorp/packer/packer" ) @@ -54,7 +53,7 @@ func TestParse_datasource(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "null.test", BuilderType: "null", @@ -142,7 +141,7 @@ func TestParse_datasource(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "null.test", BuilderType: "null", diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index bf0e9636c..6bcc047d1 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/hcl/v2/hclsyntax" - packersdk "github.com/hashicorp/packer-plugin-sdk/packer" pkrfunction "github.com/hashicorp/packer/hcl2template/function" "github.com/hashicorp/packer/packer" "github.com/zclconf/go-cty/cty" @@ -643,8 +642,8 @@ 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, hcl.Diagnostics) { - res := []packersdk.Build{} +func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]*packer.CoreBuild, hcl.Diagnostics) { + res := []*packer.CoreBuild{} var diags hcl.Diagnostics possibleBuildNames := []string{} diff --git a/hcl2template/types.packer_config_test.go b/hcl2template/types.packer_config_test.go index 99915e4a0..688f74039 100644 --- a/hcl2template/types.packer_config_test.go +++ b/hcl2template/types.packer_config_test.go @@ -9,7 +9,6 @@ import ( "time" "github.com/hashicorp/go-version" - packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/hashicorp/packer-plugin-sdk/template/config" "github.com/hashicorp/packer/hcl2template/addrs" . "github.com/hashicorp/packer/hcl2template/internal" @@ -205,7 +204,7 @@ func TestParser_complete(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "virtualbox-iso.ubuntu-1204", BuilderType: "virtualbox-iso", @@ -569,7 +568,7 @@ func TestParser_no_init(t *testing.T) { Builds: nil, }, false, false, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, @@ -578,7 +577,7 @@ func TestParser_no_init(t *testing.T) { parseTestArgs{"testdata/init/duplicate_required_plugins", nil, nil}, nil, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, {"invalid_inexplicit_source.pkr.hcl", @@ -598,7 +597,7 @@ func TestParser_no_init(t *testing.T) { Basedir: filepath.Clean("testdata/init"), }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, {"invalid_short_source.pkr.hcl", @@ -618,7 +617,7 @@ func TestParser_no_init(t *testing.T) { Basedir: filepath.Clean("testdata/init"), }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, {"invalid_inexplicit_source_2.pkr.hcl", @@ -638,7 +637,7 @@ func TestParser_no_init(t *testing.T) { Basedir: filepath.Clean("testdata/init"), }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, } diff --git a/hcl2template/types.source_test.go b/hcl2template/types.source_test.go index e31916368..e362032b0 100644 --- a/hcl2template/types.source_test.go +++ b/hcl2template/types.source_test.go @@ -7,7 +7,6 @@ import ( "path/filepath" "testing" - packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/hashicorp/packer/builder/null" "github.com/hashicorp/packer/packer" ) @@ -52,7 +51,7 @@ func TestParse_source(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "null.test", BuilderType: "null", @@ -98,7 +97,7 @@ func TestParse_source(t *testing.T) { }, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, {"used source with unknown type fails", diff --git a/hcl2template/types.variables_test.go b/hcl2template/types.variables_test.go index 218469fb6..50e3fdcf3 100644 --- a/hcl2template/types.variables_test.go +++ b/hcl2template/types.variables_test.go @@ -10,7 +10,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl/v2" - packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/hashicorp/packer/builder/null" . "github.com/hashicorp/packer/hcl2template/internal" "github.com/hashicorp/packer/packer" @@ -126,7 +125,7 @@ func TestParse_variables(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "null.test", BuilderType: "null", @@ -156,7 +155,7 @@ func TestParse_variables(t *testing.T) { }, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, {"duplicate variable in variables", @@ -177,7 +176,7 @@ func TestParse_variables(t *testing.T) { }, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, {"duplicate local block", @@ -200,7 +199,7 @@ func TestParse_variables(t *testing.T) { }, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, {"invalid default type", @@ -221,7 +220,7 @@ func TestParse_variables(t *testing.T) { }, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, @@ -240,7 +239,7 @@ func TestParse_variables(t *testing.T) { }, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, @@ -258,7 +257,7 @@ func TestParse_variables(t *testing.T) { }, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, true, }, @@ -291,7 +290,7 @@ func TestParse_variables(t *testing.T) { }, }, true, true, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "null", BuilderType: "null", @@ -378,7 +377,7 @@ func TestParse_variables(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "null.test", BuilderType: "null", @@ -399,7 +398,7 @@ func TestParse_variables(t *testing.T) { LocalVariables: nil, }, true, true, - []packersdk.Build{}, + []*packer.CoreBuild{}, false, }, @@ -442,7 +441,7 @@ func TestParse_variables(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "null.test", BuilderType: "null", @@ -484,7 +483,7 @@ func TestParse_variables(t *testing.T) { Basedir: filepath.Join("testdata", "variables"), }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "null.test", BuilderType: "null", @@ -542,47 +541,48 @@ func TestParse_variables(t *testing.T) { }, }, false, false, - []packersdk.Build{&packer.CoreBuild{ - Type: "null.null-builder", - BuilderType: "null", - Prepared: true, - Builder: &null.Builder{}, - Provisioners: []packer.CoreBuildProvisioner{ - { - PType: "shell", - Provisioner: &packer.RetriedProvisioner{ - MaxRetries: 1, - Provisioner: &HCL2Provisioner{ - Provisioner: &MockProvisioner{ - Config: MockConfig{ - NestedMockConfig: NestedMockConfig{ - Tags: []MockTag{}, + []*packer.CoreBuild{ + &packer.CoreBuild{ + Type: "null.null-builder", + BuilderType: "null", + Prepared: true, + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{ + { + PType: "shell", + Provisioner: &packer.RetriedProvisioner{ + MaxRetries: 1, + Provisioner: &HCL2Provisioner{ + Provisioner: &MockProvisioner{ + Config: MockConfig{ + NestedMockConfig: NestedMockConfig{ + Tags: []MockTag{}, + }, + NestedSlice: []NestedMockConfig{}, }, - NestedSlice: []NestedMockConfig{}, }, }, }, }, - }, - { - PType: "shell", - Provisioner: &packer.RetriedProvisioner{ - MaxRetries: 1, - Provisioner: &HCL2Provisioner{ - Provisioner: &MockProvisioner{ - Config: MockConfig{ - NestedMockConfig: NestedMockConfig{ - Tags: []MockTag{}, + { + PType: "shell", + Provisioner: &packer.RetriedProvisioner{ + MaxRetries: 1, + Provisioner: &HCL2Provisioner{ + Provisioner: &MockProvisioner{ + Config: MockConfig{ + NestedMockConfig: NestedMockConfig{ + Tags: []MockTag{}, + }, + NestedSlice: []NestedMockConfig{}, }, - NestedSlice: []NestedMockConfig{}, }, }, }, }, }, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, }, - PostProcessors: [][]packer.CoreBuildPostProcessor{}, - }, }, false, }, @@ -630,7 +630,7 @@ func TestParse_variables(t *testing.T) { }, }, false, false, - []packersdk.Build{ + []*packer.CoreBuild{ &packer.CoreBuild{ Type: "null.test", BuilderType: "null", diff --git a/packer/core.go b/packer/core.go index f6724cda9..70a8af761 100644 --- a/packer/core.go +++ b/packer/core.go @@ -314,9 +314,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, hcl.Diagnostics) { +func (c *Core) GetBuilds(opts GetBuildsOptions) ([]*CoreBuild, hcl.Diagnostics) { buildNames := c.BuildNames(opts.Only, opts.Except) - builds := []packersdk.Build{} + builds := []*CoreBuild{} diags := hcl.Diagnostics{} for _, n := range buildNames { b, err := c.Build(n) @@ -362,7 +362,7 @@ func (c *Core) GetBuilds(opts GetBuildsOptions) ([]packersdk.Build, hcl.Diagnost } // Build returns the Build object for the given name. -func (c *Core) Build(n string) (packersdk.Build, error) { +func (c *Core) Build(n string) (*CoreBuild, error) { // Setup the builder configBuilder, ok := c.builds[n] if !ok { diff --git a/packer/run_interfaces.go b/packer/run_interfaces.go index 3b7170899..c956f4918 100644 --- a/packer/run_interfaces.go +++ b/packer/run_interfaces.go @@ -24,7 +24,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, hcl.Diagnostics) + GetBuilds(GetBuildsOptions) ([]*CoreBuild, hcl.Diagnostics) } type Evaluator interface {