Merge pull request #8569 from hashicorp/fix_8539

[Fix 8539] fix crash when build.sources is set to an invalid name

hcl2template/parser.go

@@ -2,9 +2,6 @@ package hcl2template
 
 import (
 	"fmt"
-	"io/ioutil"
-	"path/filepath"
-	"strings"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclparse"
@@ -37,39 +34,14 @@ type Parser struct {
 	PostProcessorsSchemas packer.PostProcessorStore
 }
 
-const hcl2FileExt = ".pkr.hcl"
+const (
+	hcl2FileExt     = ".pkr.hcl"
+	hcl2JsonFileExt = ".pkr.json"
+)
 
 func (p *Parser) parse(filename string) (*PackerConfig, hcl.Diagnostics) {
-	var diags hcl.Diagnostics
 
-	hclFiles := []string{}
-	jsonFiles := []string{}
-	if strings.HasSuffix(filename, hcl2FileExt) {
-		hclFiles = append(hclFiles, filename)
-	} else if strings.HasSuffix(filename, ".json") {
-		jsonFiles = append(jsonFiles, filename)
-	} else {
-		fileInfos, err := ioutil.ReadDir(filename)
-		if err != nil {
-			diag := &hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Cannot read hcl directory",
-				Detail:   err.Error(),
-			}
-			diags = append(diags, diag)
-		}
-		for _, fileInfo := range fileInfos {
-			if fileInfo.IsDir() {
-				continue
-			}
-			filename := filepath.Join(filename, fileInfo.Name())
-			if strings.HasSuffix(filename, hcl2FileExt) {
-				hclFiles = append(hclFiles, filename)
-			} else if strings.HasSuffix(filename, ".json") {
-				jsonFiles = append(jsonFiles, filename)
-			}
-		}
-	}
+	hclFiles, jsonFiles, diags := GetHCL2Files(filename)
 
 	var files []*hcl.File
 	for _, filename := range hclFiles {
@@ -155,8 +127,6 @@ func (p *Parser) parseFile(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics {
 			}
 			cfg.Builds = append(cfg.Builds, build)
 
-		default:
-			panic(fmt.Sprintf("unexpected block type %q", block.Type)) // TODO(azr): err
 		}
 	}
 

hcl2template/testdata/build.pkr.hcl/basic.pkr.hcl

@@ -0,0 +1,125 @@
+
+// starts resources to provision them.
+build {
+    sources = [
+        "source.amazon-ebs.ubuntu-1604",
+        "source.virtualbox-iso.ubuntu-1204",
+    ]
+
+    provisioner "shell" {
+        string   = "string"
+        int      = 42
+        int64    = 43
+        bool     = true
+        trilean  = true
+        duration = "10s"
+        map_string_string {
+            a = "b"
+            c = "d"
+        }
+        slice_string = [
+            "a",
+            "b",
+            "c",
+        ]
+
+        nested {
+            string   = "string"
+            int      = 42
+            int64    = 43
+            bool     = true
+            trilean  = true
+            duration = "10s"
+            map_string_string {
+                a = "b"
+                c = "d"
+            }
+            slice_string = [
+                "a",
+                "b",
+                "c",
+            ]
+        }
+
+        nested_slice {
+        }
+    }
+
+    provisioner "file" {
+        string   = "string"
+        int      = 42
+        int64    = 43
+        bool     = true
+        trilean  = true
+        duration = "10s"
+        map_string_string {
+            a = "b"
+            c = "d"
+        }
+        slice_string = [
+            "a",
+            "b",
+            "c",
+        ]
+
+        nested {
+            string   = "string"
+            int      = 42
+            int64    = 43
+            bool     = true
+            trilean  = true
+            duration = "10s"
+            map_string_string {
+                a = "b"
+                c = "d"
+            }
+            slice_string = [
+                "a",
+                "b",
+                "c",
+            ]
+        }
+
+        nested_slice {
+        }
+    }
+
+    post-processor "amazon-import" { 
+        string   = "string"
+        int      = 42
+        int64    = 43
+        bool     = true
+        trilean  = true
+        duration = "10s"
+        map_string_string {
+            a = "b"
+            c = "d"
+        }
+        slice_string = [
+            "a",
+            "b",
+            "c",
+        ]
+
+        nested {
+            string   = "string"
+            int      = 42
+            int64    = 43
+            bool     = true
+            trilean  = true
+            duration = "10s"
+            map_string_string {
+                a = "b"
+                c = "d"
+            }
+            slice_string = [
+                "a",
+                "b",
+                "c",
+            ]
+        }
+
+        nested_slice {
+        }
+    }
+}

hcl2template/testdata/build/invalid_source_reference.pkr.hcl

@@ -0,0 +1,4 @@
+
+build {
+  sources = ["ami"]
+}

hcl2template/testdata/unknown/block_type.pkr.hcl

@@ -0,0 +1,3 @@
+
+potato {
+}

hcl2template/types.build.go

@@ -48,26 +48,17 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block) (*BuildBlock, hcl.Diagnosti
 	for _, buildFrom := range b.FromSources {
 		ref := sourceRefFromString(buildFrom)
 
-		if ref == NoSource {
-			diags = append(diags, &hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Invalid " + sourceLabel + " reference",
-				Detail: "A " + sourceLabel + " type must start with a letter and " +
-					"may contain only letters, digits, underscores, and dashes." +
-					"A valid source reference looks like: `source.type.name`",
-				Subject: &block.LabelRanges[0],
-			})
-			continue
-		}
-		if !hclsyntax.ValidIdentifier(ref.Type) ||
+		if ref == NoSource ||
+			!hclsyntax.ValidIdentifier(ref.Type) ||
 			!hclsyntax.ValidIdentifier(ref.Name) {
 			diags = append(diags, &hcl.Diagnostic{
 				Severity: hcl.DiagError,
 				Summary:  "Invalid " + sourceLabel + " reference",
-				Detail: "A " + sourceLabel + " type must start with a letter and " +
+				Detail: "A " + sourceLabel + " type is made of three parts that are" +
+					"split by a dot `.`; each part must start with a letter and " +
 					"may contain only letters, digits, underscores, and dashes." +
 					"A valid source reference looks like: `source.type.name`",
-				Subject: &block.LabelRanges[0],
+				Subject: block.DefRange.Ptr(),
 			})
 			continue
 		}
@@ -75,7 +66,8 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block) (*BuildBlock, hcl.Diagnosti
 		build.Froms = append(build.Froms, ref)
 	}
 
-	content, diags := b.Config.Content(buildSchema)
+	content, moreDiags := b.Config.Content(buildSchema)
+	diags = append(diags, moreDiags...)
 	for _, block := range content.Blocks {
 		switch block.Type {
 		case buildProvisionerLabel:

hcl2template/types.build_test.go

@@ -21,7 +21,7 @@ func TestParse_build(t *testing.T) {
 								Type: "amazon-ebs",
 								Name: "ubuntu-1604",
 							},
-							ref,
+							refVBIsoUbuntu1204,
 						},
 						ProvisionerBlocks: []*ProvisionerBlock{
 							{
@@ -83,6 +83,16 @@ func TestParse_build(t *testing.T) {
 			[]packer.Build{},
 			false,
 		},
+		{"invalid source",
+			defaultParser,
+			parseTestArgs{"testdata/build/invalid_source_reference.pkr.hcl"},
+			&PackerConfig{
+				Builds: nil,
+			},
+			true, true,
+			[]packer.Build{},
+			false,
+		},
 	}
 	testParse(t, tests)
 }

hcl2template/types.packer_config.go

@@ -103,5 +103,6 @@ func (p *Parser) Parse(path string) ([]packer.Build, hcl.Diagnostics) {
 		return nil, diags
 	}
 
-	return p.getBuilds(cfg)
+	builds, moreDiags := p.getBuilds(cfg)
+	return builds, append(diags, moreDiags...)
 }

hcl2template/types.packer_config_test.go

@@ -6,7 +6,10 @@ import (
 	"github.com/hashicorp/packer/packer"
 )
 
-var ref = SourceRef{Type: "virtualbox-iso", Name: "ubuntu-1204"}
+var (
+	refVBIsoUbuntu1204  = SourceRef{Type: "virtualbox-iso", Name: "ubuntu-1204"}
+	refAWSEBSUbuntu1204 = SourceRef{Type: "amazon-ebs", Name: "ubuntu-1604"}
+)
 
 func TestParser_complete(t *testing.T) {
 	defaultParser := getBasicParser()
@@ -17,11 +20,11 @@ func TestParser_complete(t *testing.T) {
 			parseTestArgs{"testdata/complete"},
 			&PackerConfig{
 				Sources: map[SourceRef]*Source{
-					ref: &Source{Type: "virtualbox-iso", Name: "ubuntu-1204"},
+					refVBIsoUbuntu1204: &Source{Type: "virtualbox-iso", Name: "ubuntu-1204"},
 				},
 				Builds: Builds{
 					&BuildBlock{
-						Froms: []SourceRef{ref},
+						Froms: []SourceRef{refVBIsoUbuntu1204},
 						ProvisionerBlocks: []*ProvisionerBlock{
 							{PType: "shell"},
 							{PType: "file"},
@@ -50,6 +53,50 @@ func TestParser_complete(t *testing.T) {
 			},
 			false,
 		},
+		{"dir with no config files",
+			defaultParser,
+			parseTestArgs{"testdata/empty"},
+			nil,
+			true, true,
+			nil,
+			false,
+		},
+		{name: "inexistent dir",
+			parser:                 defaultParser,
+			args:                   parseTestArgs{"testdata/inexistent"},
+			parseWantCfg:           nil,
+			parseWantDiags:         true,
+			parseWantDiagHasErrors: true,
+		},
+		{name: "folder named build.pkr.hcl with an unknown src",
+			parser: defaultParser,
+			args:   parseTestArgs{"testdata/build.pkr.hcl"},
+			parseWantCfg: &PackerConfig{
+				Builds: Builds{
+					&BuildBlock{
+						Froms: []SourceRef{refAWSEBSUbuntu1204, refVBIsoUbuntu1204},
+						ProvisionerBlocks: []*ProvisionerBlock{
+							{PType: "shell"},
+							{PType: "file"},
+						},
+						PostProcessors: []*PostProcessorBlock{
+							{PType: "amazon-import"},
+						},
+					},
+				},
+			},
+			parseWantDiags:         false,
+			parseWantDiagHasErrors: false,
+			getBuildsWantBuilds:    []packer.Build{},
+			getBuildsWantDiags:     true,
+		},
+		{name: "unknown block type",
+			parser:                 defaultParser,
+			args:                   parseTestArgs{"testdata/unknown"},
+			parseWantCfg:           &PackerConfig{},
+			parseWantDiags:         true,
+			parseWantDiagHasErrors: true,
+		},
 	}
 	testParse(t, tests)
 }

hcl2template/utils.go

@@ -1,6 +1,13 @@
 package hcl2template
 
-import "github.com/hashicorp/hcl/v2"
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/hashicorp/hcl/v2"
+)
 
 func warningErrorsToDiags(block *hcl.Block, warnings []string, err error) hcl.Diagnostics {
 	var diags hcl.Diagnostics
@@ -21,3 +28,63 @@ func warningErrorsToDiags(block *hcl.Block, warnings []string, err error) hcl.Di
 	}
 	return diags
 }
+
+func isDir(name string) (bool, error) {
+	s, err := os.Stat(name)
+	if err != nil {
+		return false, err
+	}
+	return s.IsDir(), nil
+}
+
+func GetHCL2Files(filename string) (hclFiles, jsonFiles []string, diags hcl.Diagnostics) {
+	isDir, err := isDir(filename)
+	if err != nil {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Cannot tell wether " + filename + " is a directory",
+			Detail:   err.Error(),
+		})
+		return nil, nil, diags
+	}
+	if !isDir {
+		if strings.HasSuffix(filename, hcl2JsonFileExt) {
+			return nil, []string{filename}, diags
+		}
+		if strings.HasSuffix(filename, hcl2FileExt) {
+			return []string{filename}, nil, diags
+		}
+	}
+
+	fileInfos, err := ioutil.ReadDir(filename)
+	if err != nil {
+		diag := &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Cannot read hcl directory",
+			Detail:   err.Error(),
+		}
+		diags = append(diags, diag)
+		return nil, nil, diags
+	}
+	for _, fileInfo := range fileInfos {
+		if fileInfo.IsDir() {
+			continue
+		}
+		filename := filepath.Join(filename, fileInfo.Name())
+		if strings.HasSuffix(filename, hcl2FileExt) {
+			hclFiles = append(hclFiles, filename)
+		} else if strings.HasSuffix(filename, hcl2JsonFileExt) {
+			jsonFiles = append(jsonFiles, filename)
+		}
+	}
+	if len(hclFiles)+len(jsonFiles) == 0 {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Could not find any config file in " + filename,
+			Detail: "A config file must be suffixed with `.pkr.hcl` or " +
+				"`.pkr.json`. A folder can be referenced.",
+		})
+	}
+
+	return hclFiles, jsonFiles, diags
+}