diff --git a/command/build_test.go b/command/build_test.go index d912e1319..e354fe02a 100644 --- a/command/build_test.go +++ b/command/build_test.go @@ -6,6 +6,7 @@ import ( "math" "os" "path/filepath" + "regexp" "runtime" "strings" "testing" @@ -1168,6 +1169,31 @@ func TestBuildCmd(t *testing.T) { return fmt.Errorf("error: expected build to succeed without errors, got %d", nbErrs) } + return nil + }, + }, + { + name: "hcl - build block without source", + args: []string{ + testFixture("hcl", "build_no_source.pkr.hcl"), + }, + expectedCode: 1, + outputCheck: func(_, err string) error { + if !strings.Contains(err, "Error: missing source reference") { + return fmt.Errorf("expected 'Error: missing source reference' in output, did not find it") + } + + nbErrs := strings.Count(err, "Error: ") + if nbErrs != 1 { + return fmt.Errorf( + "error: too many errors in stderr for build, expected 1, got %d", + nbErrs) + } + + logRegex := regexp.MustCompile("on.*build_no_source.pkr.hcl line 1") + if !logRegex.MatchString(err) { + return fmt.Errorf("error: missing context for error message") + } return nil }, diff --git a/command/test-fixtures/hcl/build_no_source.pkr.hcl b/command/test-fixtures/hcl/build_no_source.pkr.hcl new file mode 100644 index 000000000..af4fbda87 --- /dev/null +++ b/command/test-fixtures/hcl/build_no_source.pkr.hcl @@ -0,0 +1 @@ +build{} diff --git a/command/test-fixtures/version_req/base_success/version.pkr.hcl b/command/test-fixtures/version_req/base_success/version.pkr.hcl index d3ebe59b2..288698642 100644 --- a/command/test-fixtures/version_req/base_success/version.pkr.hcl +++ b/command/test-fixtures/version_req/base_success/version.pkr.hcl @@ -2,4 +2,10 @@ packer { required_version = ">= 1.0.0" } -build {} +source "null" "test" { + communicator = "none" +} + +build { + sources = ["null.test"] +} diff --git a/hcl2template/common_test.go b/hcl2template/common_test.go index 619a3e19a..567959e21 100644 --- a/hcl2template/common_test.go +++ b/hcl2template/common_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/packer/builder/null" dnull "github.com/hashicorp/packer/datasource/null" . "github.com/hashicorp/packer/hcl2template/internal" + hcl2template "github.com/hashicorp/packer/hcl2template/internal" packerregistry "github.com/hashicorp/packer/internal/registry" "github.com/hashicorp/packer/packer" "github.com/zclconf/go-cty/cty" @@ -372,6 +373,7 @@ var cmpOpts = []cmp.Option{ cmpopts.IgnoreFields(VariableAssignment{}, "Expr", // its an interface ), + cmpopts.IgnoreTypes(hcl2template.MockBuilder{}), cmpopts.IgnoreTypes(HCL2Ref{}), cmpopts.IgnoreTypes([]*LocalBlock{}), cmpopts.IgnoreTypes([]hcl.Range{}), diff --git a/hcl2template/testdata/build/post-processor_nonexistent.pkr.hcl b/hcl2template/testdata/build/post-processor_nonexistent.pkr.hcl index 3c1f20efe..4ea811f20 100644 --- a/hcl2template/testdata/build/post-processor_nonexistent.pkr.hcl +++ b/hcl2template/testdata/build/post-processor_nonexistent.pkr.hcl @@ -1,5 +1,8 @@ +source "null" "test" {} build { + sources = [ "null.test" ] + post-processor "nonexistent" { foo = "bar" } diff --git a/hcl2template/testdata/build/provisioner_nonexistent.pkr.hcl b/hcl2template/testdata/build/provisioner_nonexistent.pkr.hcl index d1f5b7c4e..a8d429057 100644 --- a/hcl2template/testdata/build/provisioner_nonexistent.pkr.hcl +++ b/hcl2template/testdata/build/provisioner_nonexistent.pkr.hcl @@ -1,5 +1,8 @@ +source "null" "test" {} build { + sources = [ "null.test" ] + provisioner "nonexistent" { foo = "bar" } diff --git a/hcl2template/testdata/datasources/basic.pkr.hcl b/hcl2template/testdata/datasources/basic.pkr.hcl index 4d7b9dcf9..70a51330a 100644 --- a/hcl2template/testdata/datasources/basic.pkr.hcl +++ b/hcl2template/testdata/datasources/basic.pkr.hcl @@ -86,4 +86,10 @@ data "amazon-ami" "test" { } } -build {} +source "null" "test" { + communicator = "none" +} + +build { + sources = ["null.test"] +} diff --git a/hcl2template/testdata/datasources/recursive.pkr.hcl b/hcl2template/testdata/datasources/recursive.pkr.hcl index 897395fbf..84acac92e 100644 --- a/hcl2template/testdata/datasources/recursive.pkr.hcl +++ b/hcl2template/testdata/datasources/recursive.pkr.hcl @@ -18,4 +18,10 @@ data "null" "bang" { input = "${data.null.baz.output}-with-marshmallows" } -build {} +source "null" "test" { + communicator = "none" +} + +build { + sources = ["null.test"] +} diff --git a/hcl2template/testdata/sources/basic.pkr.hcl b/hcl2template/testdata/sources/basic.pkr.hcl index c10d814dd..d7efb812e 100644 --- a/hcl2template/testdata/sources/basic.pkr.hcl +++ b/hcl2template/testdata/sources/basic.pkr.hcl @@ -87,4 +87,10 @@ source "virtualbox-iso" "ubuntu-1204" { } } -build {} +source "null" "test" { + communicator = "none" +} + +build { + sources = ["null.test"] +} diff --git a/hcl2template/testdata/variables/basic.pkr.hcl b/hcl2template/testdata/variables/basic.pkr.hcl index c0ef953f4..3c3c8132b 100644 --- a/hcl2template/testdata/variables/basic.pkr.hcl +++ b/hcl2template/testdata/variables/basic.pkr.hcl @@ -42,4 +42,10 @@ local "supersecret" { expression = "secretvar" } -build {} +source "null" "test" { + communicator = "none" +} + +build { + sources = ["null.test"] +} diff --git a/hcl2template/testdata/variables/complicated/d.pkr.hcl b/hcl2template/testdata/variables/complicated/d.pkr.hcl index d53f092e5..e005230b4 100644 --- a/hcl2template/testdata/variables/complicated/d.pkr.hcl +++ b/hcl2template/testdata/variables/complicated/d.pkr.hcl @@ -1 +1,7 @@ -build {} +source "null" "test" { + communicator = "none" +} + +build { + sources = ["null.test"] +} diff --git a/hcl2template/testdata/variables/empty.pkr.hcl b/hcl2template/testdata/variables/empty.pkr.hcl index d53f092e5..e005230b4 100644 --- a/hcl2template/testdata/variables/empty.pkr.hcl +++ b/hcl2template/testdata/variables/empty.pkr.hcl @@ -1 +1,7 @@ -build {} +source "null" "test" { + communicator = "none" +} + +build { + sources = ["null.test"] +} diff --git a/hcl2template/testdata/variables/foo-string.variable.pkr.hcl b/hcl2template/testdata/variables/foo-string.variable.pkr.hcl index adc6c3d04..f15ad1360 100644 --- a/hcl2template/testdata/variables/foo-string.variable.pkr.hcl +++ b/hcl2template/testdata/variables/foo-string.variable.pkr.hcl @@ -3,4 +3,10 @@ variable "foo" { default = "bar" } -build {} +source "null" "test" { + communicator = "none" +} + +build { + sources = ["null.test"] +} diff --git a/hcl2template/testdata/variables/validation/valid.pkr.hcl b/hcl2template/testdata/variables/validation/valid.pkr.hcl index 67c27dd26..dbab09d87 100644 --- a/hcl2template/testdata/variables/validation/valid.pkr.hcl +++ b/hcl2template/testdata/variables/validation/valid.pkr.hcl @@ -8,4 +8,10 @@ variable "image_id" { } } -build {} +source "null" "test" { + communicator = "none" +} + +build { + sources = ["null.test"] +} diff --git a/hcl2template/types.build.go b/hcl2template/types.build.go index dac9b5db4..3bb54b20c 100644 --- a/hcl2template/types.build.go +++ b/hcl2template/types.build.go @@ -110,7 +110,16 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB "name": cty.StringVal(b.Name), }) + // We rely on `hadSource` to determine which error to proc. + // + // If a source block is referenced in the build block, but isn't valid, we + // cannot rely on the `build.Sources' since it's only populated when a valid + // source is processed. + hadSource := false + for _, buildFrom := range b.FromSources { + hadSource = true + ref := sourceRefFromString(buildFrom) if ref == NoSource || @@ -156,6 +165,7 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB } build.HCPPackerRegistry = hcpPackerRegistry case sourceLabel: + hadSource = true ref, moreDiags := p.decodeBuildSource(block) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { @@ -216,5 +226,14 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB } } + if !hadSource { + diags = append(diags, &hcl.Diagnostic{ + Summary: "missing source reference", + Detail: "a build block must reference at least one source to be built", + Severity: hcl.DiagError, + Subject: block.DefRange.Ptr(), + }) + } + return build, diags } diff --git a/hcl2template/types.build_test.go b/hcl2template/types.build_test.go index b5123a039..17cd80861 100644 --- a/hcl2template/types.build_test.go +++ b/hcl2template/types.build_test.go @@ -73,6 +73,15 @@ func TestParse_build(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "build"), + Sources: map[SourceRef]SourceBlock{ + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, + }, Builds: Builds{ &BuildBlock{ ProvisionerBlocks: []*ProvisionerBlock{ @@ -80,6 +89,14 @@ func TestParse_build(t *testing.T) { PType: "nonexistent", }, }, + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, }, }, }, @@ -134,6 +151,15 @@ func TestParse_build(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "build"), + Sources: map[SourceRef]SourceBlock{ + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, + }, Builds: Builds{ &BuildBlock{ PostProcessorsLists: [][]*PostProcessorBlock{ @@ -143,6 +169,14 @@ func TestParse_build(t *testing.T) { }, }, }, + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, }, }, }, diff --git a/hcl2template/types.datasource_test.go b/hcl2template/types.datasource_test.go index 078a8a2ba..26c8c3c45 100644 --- a/hcl2template/types.datasource_test.go +++ b/hcl2template/types.datasource_test.go @@ -5,6 +5,8 @@ import ( "testing" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer/builder/null" + "github.com/hashicorp/packer/packer" ) func TestParse_datasource(t *testing.T) { @@ -17,7 +19,25 @@ func TestParse_datasource(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Builds: Builds{ - &BuildBlock{}, + &BuildBlock{ + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, + }, + }, + Sources: map[SourceRef]SourceBlock{ + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, }, Basedir: filepath.Join("testdata", "datasources"), Datasources: Datasources{ @@ -31,7 +51,15 @@ func TestParse_datasource(t *testing.T) { }, }, false, false, - []packersdk.Build{}, + []packersdk.Build{ + &packer.CoreBuild{ + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + }, + }, false, }, {"recursive datasources", @@ -40,7 +68,25 @@ func TestParse_datasource(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Builds: Builds{ - &BuildBlock{}, + &BuildBlock{ + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, + }, + }, + Sources: map[SourceRef]SourceBlock{ + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, }, Basedir: filepath.Join("testdata", "datasources"), Datasources: Datasources{ @@ -82,7 +128,15 @@ func TestParse_datasource(t *testing.T) { }, }, false, false, - []packersdk.Build{}, + []packersdk.Build{ + &packer.CoreBuild{ + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + }, + }, false, }, {"untyped datasource", diff --git a/hcl2template/types.source_test.go b/hcl2template/types.source_test.go index cc6d3fbb5..e19d921da 100644 --- a/hcl2template/types.source_test.go +++ b/hcl2template/types.source_test.go @@ -5,6 +5,8 @@ import ( "testing" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer/builder/null" + "github.com/hashicorp/packer/packer" ) func TestParse_source(t *testing.T) { @@ -17,7 +19,16 @@ func TestParse_source(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Builds: Builds{ - &BuildBlock{}, + &BuildBlock{ + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, + }, }, Basedir: filepath.Join("testdata", "sources"), Sources: map[SourceRef]SourceBlock{ @@ -28,10 +39,25 @@ func TestParse_source(t *testing.T) { Type: "virtualbox-iso", Name: "ubuntu-1204", }, + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, }, }, false, false, - []packersdk.Build{}, + []packersdk.Build{ + &packer.CoreBuild{ + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + }, + }, false, }, {"untyped source", @@ -61,15 +87,13 @@ func TestParse_source(t *testing.T) { parseTestArgs{"testdata/sources/nonexistent.pkr.hcl", nil, nil}, &PackerConfig{ CorePackerVersionString: lockedVersion, - Builds: Builds{ - &BuildBlock{}, - }, - Basedir: filepath.Join("testdata", "sources"), + Builds: nil, + Basedir: filepath.Join("testdata", "sources"), Sources: map[SourceRef]SourceBlock{ {Type: "nonexistent", Name: "ubuntu-1204"}: {Type: "nonexistent", Name: "ubuntu-1204"}, }, }, - false, false, + true, true, []packersdk.Build{}, false, }, diff --git a/hcl2template/types.variables_test.go b/hcl2template/types.variables_test.go index cbd4d63d4..1abe331a8 100644 --- a/hcl2template/types.variables_test.go +++ b/hcl2template/types.variables_test.go @@ -25,7 +25,25 @@ func TestParse_variables(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Builds: Builds{ - &BuildBlock{}, + &BuildBlock{ + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, + }, + }, + Sources: map[SourceRef]SourceBlock{ + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, }, Basedir: filepath.Join("testdata", "variables"), InputVariables: Variables{ @@ -105,7 +123,15 @@ func TestParse_variables(t *testing.T) { }, }, false, false, - []packersdk.Build{}, + []packersdk.Build{ + &packer.CoreBuild{ + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + }, + }, false, }, {"duplicate variable", @@ -279,7 +305,25 @@ func TestParse_variables(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Builds: Builds{ - &BuildBlock{}, + &BuildBlock{ + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, + }, + }, + Sources: map[SourceRef]SourceBlock{ + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, }, Basedir: "testdata/variables/complicated", InputVariables: Variables{ @@ -329,7 +373,15 @@ func TestParse_variables(t *testing.T) { }, }, false, false, - []packersdk.Build{}, + []packersdk.Build{ + &packer.CoreBuild{ + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + }, + }, false, }, {"recursive locals", @@ -352,7 +404,25 @@ func TestParse_variables(t *testing.T) { CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "variables"), Builds: Builds{ - &BuildBlock{}, + &BuildBlock{ + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, + }, + }, + Sources: map[SourceRef]SourceBlock{ + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, }, InputVariables: Variables{ "foo": &Variable{ @@ -366,7 +436,15 @@ func TestParse_variables(t *testing.T) { }, }, false, false, - []packersdk.Build{}, + []packersdk.Build{ + &packer.CoreBuild{ + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + }, + }, false, }, @@ -376,12 +454,38 @@ func TestParse_variables(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Builds: Builds{ - &BuildBlock{}, + &BuildBlock{ + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, + }, + }, + Sources: map[SourceRef]SourceBlock{ + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, }, Basedir: filepath.Join("testdata", "variables"), }, true, false, - []packersdk.Build{}, + []packersdk.Build{ + &packer.CoreBuild{ + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + }, + }, false, }, @@ -481,7 +585,25 @@ func TestParse_variables(t *testing.T) { CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "variables", "validation"), Builds: Builds{ - &BuildBlock{}, + &BuildBlock{ + Sources: []SourceUseBlock{ + { + SourceRef: SourceRef{ + Type: "null", + Name: "test", + }, + }, + }, + }, + }, + Sources: map[SourceRef]SourceBlock{ + { + Type: "null", + Name: "test", + }: { + Type: "null", + Name: "test", + }, }, InputVariables: Variables{ "image_id": &Variable{ @@ -499,7 +621,15 @@ func TestParse_variables(t *testing.T) { }, }, false, false, - []packersdk.Build{}, + []packersdk.Build{ + &packer.CoreBuild{ + Type: "null.test", + Builder: &null.Builder{}, + Provisioners: []packer.CoreBuildProvisioner{}, + PostProcessors: [][]packer.CoreBuildPostProcessor{}, + Prepared: true, + }, + }, false, },