hcl2: report error on build without sources

When a template describes a build block without a source reference, the
build should be considered invalid as we won't have a CoreBuild produced
as a result of the need to have both.

In current versions of Packer, this will produce an error message
hinting that nothing will happen because of the lack of either build or
source block.

This commit takes the defined block, and points out to it as missing a
source block as being the reason why nothing is happening, making it
clearer what is required for an HCL2 build to be processed.
This commit is contained in:
Lucas Bajolet 2022-09-23 16:59:17 -04:00 committed by Lucas Bajolet
parent 61c56e161c
commit c0e7e7bd3c
19 changed files with 380 additions and 30 deletions

View file

@ -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
},

View file

@ -0,0 +1 @@
build{}

View file

@ -2,4 +2,10 @@ packer {
required_version = ">= 1.0.0"
}
build {}
source "null" "test" {
communicator = "none"
}
build {
sources = ["null.test"]
}

View file

@ -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{}),

View file

@ -1,5 +1,8 @@
source "null" "test" {}
build {
sources = [ "null.test" ]
post-processor "nonexistent" {
foo = "bar"
}

View file

@ -1,5 +1,8 @@
source "null" "test" {}
build {
sources = [ "null.test" ]
provisioner "nonexistent" {
foo = "bar"
}

View file

@ -86,4 +86,10 @@ data "amazon-ami" "test" {
}
}
build {}
source "null" "test" {
communicator = "none"
}
build {
sources = ["null.test"]
}

View file

@ -18,4 +18,10 @@ data "null" "bang" {
input = "${data.null.baz.output}-with-marshmallows"
}
build {}
source "null" "test" {
communicator = "none"
}
build {
sources = ["null.test"]
}

View file

@ -87,4 +87,10 @@ source "virtualbox-iso" "ubuntu-1204" {
}
}
build {}
source "null" "test" {
communicator = "none"
}
build {
sources = ["null.test"]
}

View file

@ -42,4 +42,10 @@ local "supersecret" {
expression = "secretvar"
}
build {}
source "null" "test" {
communicator = "none"
}
build {
sources = ["null.test"]
}

View file

@ -1 +1,7 @@
build {}
source "null" "test" {
communicator = "none"
}
build {
sources = ["null.test"]
}

View file

@ -1 +1,7 @@
build {}
source "null" "test" {
communicator = "none"
}
build {
sources = ["null.test"]
}

View file

@ -3,4 +3,10 @@ variable "foo" {
default = "bar"
}
build {}
source "null" "test" {
communicator = "none"
}
build {
sources = ["null.test"]
}

View file

@ -8,4 +8,10 @@ variable "image_id" {
}
}
build {}
source "null" "test" {
communicator = "none"
}
build {
sources = ["null.test"]
}

View file

@ -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
}

View file

@ -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",
},
},
},
},
},
},

View file

@ -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",

View file

@ -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,
},

View file

@ -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,
},