Merge pull request #9346 from hashicorp/hcl-validation-command

command/validate: Add support for HCL2 config files

command/test-fixtures/validate-invalid/bad_provisioner.json

@@ -0,0 +1,15 @@
+{
+  "builders":[
+    {
+      "type":"file",
+      "target":"chocolate.txt",
+      "content":"chocolate"
+    }
+  ],
+  "provisioners": [
+    {
+      "type": "file",
+      "comment": "unknown field"
+    }
+  ]
+}

command/test-fixtures/validate-invalid/broken.json

@@ -0,0 +1,10 @@
+{
+  "builders":[
+    {
+      "type":"file",
+      "target":"chocolate.txt",
+      "content":"chocolate"
+    }
+  ],
+  "provisioners": "not an array"
+}

command/test-fixtures/validate-invalid/missing_build_block.pkr.hcl

@@ -0,0 +1,9 @@
+source "file" "chocolate" {
+  target = "chocolate.txt"
+  content = "chocolate"
+}
+
+build {
+  sources = ["source.file.cho"]
+}
+

command/test-fixtures/validate/build.json

@@ -0,0 +1,9 @@
+{
+  "builders":[
+    {
+      "type":"file",
+      "target":"chocolate.txt",
+      "content":"chocolate"
+    }
+  ]
+}

command/test-fixtures/validate/build.pkr.hcl

@@ -0,0 +1,8 @@
+source "file" "chocolate" {
+  target = "chocolate.txt"
+  content = "chocolate"
+}
+
+build {
+  sources = ["source.file.chocolate"]
+}

command/test-fixtures/validate/build_with_vars.pkr.hcl

@@ -0,0 +1,13 @@
+variable "target" {
+  type = string
+  default = "chocolate.txt"
+}
+
+source "file" "chocolate" {
+  target = var.target
+  content = "chocolate"
+}
+
+build {
+  sources = ["source.file.chocolate"]
+}

command/validate.go

@@ -2,16 +2,10 @@ package command
 
 import (
 	"context"
-	"encoding/json"
-	"fmt"
-	"log"
 	"strings"
 
-	"github.com/hashicorp/packer/fix"
 	"github.com/hashicorp/packer/packer"
-	"github.com/hashicorp/packer/template"
 
-	"github.com/google/go-cmp/cmp"
 	"github.com/posener/complete"
 )
 
@@ -51,137 +45,27 @@ func (c *ValidateCommand) ParseArgs(args []string) (*ValidateArgs, int) {
 }
 
 func (c *ValidateCommand) RunContext(ctx context.Context, cla *ValidateArgs) int {
-	// Parse the template
-	tpl, err := template.ParseFile(cla.Path)
-	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to parse template: %s", err))
+	packerStarter, ret := c.GetConfig(&cla.MetaArgs)
+	if ret != 0 {
 		return 1
 	}
 
 	// If we're only checking syntax, then we're done already
 	if cla.SyntaxOnly {
-		c.Ui.Say("Syntax-only check passed. Everything looks okay.")
 		return 0
 	}
 
-	// Get the core
-	core, err := c.Meta.Core(tpl, &cla.MetaArgs)
-	if err != nil {
-		c.Ui.Error(err.Error())
-		return 1
-	}
+	_, diags := packerStarter.GetBuilds(packer.GetBuildsOptions{
+		Only:   cla.Only,
+		Except: cla.Except,
+	})
 
-	errs := make([]error, 0)
-	warnings := make(map[string][]string)
+	fixerDiags := packerStarter.FixConfig(packer.FixConfigOptions{
+		Mode: packer.Diff,
+	})
+	diags = append(diags, fixerDiags...)
 
-	// Get the builds we care about
-	buildNames := core.BuildNames(c.CoreConfig.Only, c.CoreConfig.Except)
-	builds := make([]packer.Build, 0, len(buildNames))
-	for _, n := range buildNames {
-		b, err := core.Build(n)
-		if err != nil {
-			c.Ui.Error(fmt.Sprintf(
-				"Failed to initialize build '%s': %s",
-				n, err))
-			return 1
-		}
-
-		builds = append(builds, b)
-	}
-
-	// Check the configuration of all builds
-	for _, b := range builds {
-		log.Printf("Preparing build: %s", b.Name())
-		warns, err := b.Prepare()
-		if len(warns) > 0 {
-			warnings[b.Name()] = warns
-		}
-		if err != nil {
-			errs = append(errs, fmt.Errorf("Errors validating build '%s'. %s", b.Name(), err))
-		}
-	}
-
-	// Check if any of the configuration is fixable
-	var rawTemplateData map[string]interface{}
-	input := make(map[string]interface{})
-	templateData := make(map[string]interface{})
-	json.Unmarshal(tpl.RawContents, &rawTemplateData)
-	for k, v := range rawTemplateData {
-		if vals, ok := v.([]interface{}); ok {
-			if len(vals) == 0 {
-				continue
-			}
-		}
-		templateData[strings.ToLower(k)] = v
-		input[strings.ToLower(k)] = v
-	}
-
-	// fix rawTemplateData into input
-	for _, name := range fix.FixerOrder {
-		var err error
-		fixer, ok := fix.Fixers[name]
-		if !ok {
-			panic("fixer not found: " + name)
-		}
-		input, err = fixer.Fix(input)
-		if err != nil {
-			c.Ui.Error(fmt.Sprintf("Error checking against fixers: %s", err))
-			return 1
-		}
-	}
-	// delete empty top-level keys since the fixers seem to add them
-	// willy-nilly
-	for k := range input {
-		ml, ok := input[k].([]map[string]interface{})
-		if !ok {
-			continue
-		}
-		if len(ml) == 0 {
-			delete(input, k)
-		}
-	}
-	// marshal/unmarshal to make comparable to templateData
-	var fixedData map[string]interface{}
-	// Guaranteed to be valid json, so we can ignore errors
-	j, _ := json.Marshal(input)
-	json.Unmarshal(j, &fixedData)
-
-	if diff := cmp.Diff(templateData, fixedData); diff != "" {
-		c.Ui.Say("[warning] Fixable configuration found.")
-		c.Ui.Say("You may need to run `packer fix` to get your build to run")
-		c.Ui.Say("correctly. See debug log for more information.\n")
-		log.Printf("Fixable config differences:\n%s", diff)
-	}
-
-	if len(errs) > 0 {
-		c.Ui.Error("Template validation failed. Errors are shown below.\n")
-		for i, err := range errs {
-			c.Ui.Error(err.Error())
-
-			if (i + 1) < len(errs) {
-				c.Ui.Error("")
-			}
-		}
-		return 1
-	}
-
-	if len(warnings) > 0 {
-		c.Ui.Say("Template validation succeeded, but there were some warnings.")
-		c.Ui.Say("These are ONLY WARNINGS, and Packer will attempt to build the")
-		c.Ui.Say("template despite them, but they should be paid attention to.\n")
-
-		for build, warns := range warnings {
-			c.Ui.Say(fmt.Sprintf("Warnings for build '%s':\n", build))
-			for _, warning := range warns {
-				c.Ui.Say(fmt.Sprintf("* %s", warning))
-			}
-		}
-
-		return 0
-	}
-
-	c.Ui.Say("Template validated successfully.")
-	return 0
+	return writeDiags(c.Ui, nil, diags)
 }
 
 func (*ValidateCommand) Help() string {

command/validate_test.go

@@ -5,6 +5,62 @@ import (
 	"testing"
 )
 
+func TestValidateCommand(t *testing.T) {
+	tt := []struct {
+		path     string
+		exitCode int
+	}{
+		{path: filepath.Join(testFixture("validate"), "build.json")},
+		{path: filepath.Join(testFixture("validate"), "build.pkr.hcl")},
+		{path: filepath.Join(testFixture("validate"), "build_with_vars.pkr.hcl")},
+		{path: filepath.Join(testFixture("validate-invalid"), "bad_provisioner.json"), exitCode: 1},
+		{path: filepath.Join(testFixture("validate-invalid"), "missing_build_block.pkr.hcl"), exitCode: 1},
+	}
+
+	c := &ValidateCommand{
+		Meta: testMetaFile(t),
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.path, func(t *testing.T) {
+			tc := tc
+			args := []string{tc.path}
+			if code := c.Run(args); code != tc.exitCode {
+				fatalCommand(t, c.Meta)
+			}
+		})
+	}
+}
+
+func TestValidateCommand_SyntaxOnly(t *testing.T) {
+	tt := []struct {
+		path     string
+		exitCode int
+	}{
+		{path: filepath.Join(testFixture("validate"), "build.json")},
+		{path: filepath.Join(testFixture("validate"), "build.pkr.hcl")},
+		{path: filepath.Join(testFixture("validate"), "build_with_vars.pkr.hcl")},
+		{path: filepath.Join(testFixture("validate-invalid"), "bad_provisioner.json")},
+		{path: filepath.Join(testFixture("validate-invalid"), "missing_build_block.pkr.hcl")},
+		{path: filepath.Join(testFixture("validate-invalid"), "broken.json"), exitCode: 1},
+	}
+
+	c := &ValidateCommand{
+		Meta: testMetaFile(t),
+	}
+	c.CoreConfig.Version = "102.0.0"
+
+	for _, tc := range tt {
+		t.Run(tc.path, func(t *testing.T) {
+			tc := tc
+			args := []string{"-syntax-only", tc.path}
+			if code := c.Run(args); code != tc.exitCode {
+				fatalCommand(t, c.Meta)
+			}
+		})
+	}
+}
+
 func TestValidateCommandOKVersion(t *testing.T) {
 	c := &ValidateCommand{
 		Meta: testMetaFile(t),

hcl2template/types.packer_config.go

@@ -450,3 +450,8 @@ func (p *PackerConfig) handleEval(line string) (out string, exit bool, diags hcl
 
 	return PrintableCtyValue(val), false, diags
 }
+
+func (p *PackerConfig) FixConfig(_ packer.FixConfigOptions) (diags hcl.Diagnostics) {
+	// No Fixers exist for HCL2 configs so there is nothing to do here for now.
+	return
+}

helper/config/decode.go

@@ -159,7 +159,7 @@ func Decode(target interface{}, config *DecodeOpts, raws ...interface{}) error {
 				if fixable {
 					unusedErr = fmt.Errorf("Deprecated configuration key: '%s'."+
 						" Please call `packer fix` against your template to "+
-						"update your template to be compatable with the current "+
+						"update your template to be compatible with the current "+
 						"version of Packer. Visit "+
 						"https://www.packer.io/docs/commands/fix/ for more detail.",
 						unused)

packer/core.go

@@ -1,6 +1,7 @@
 package packer
 
 import (
+	"encoding/json"
 	"fmt"
 	"log"
 	"regexp"
@@ -9,6 +10,7 @@ import (
 
 	ttmp "text/template"
 
+	"github.com/google/go-cmp/cmp"
 	multierror "github.com/hashicorp/go-multierror"
 	version "github.com/hashicorp/go-version"
 	"github.com/hashicorp/hcl/v2"
@@ -420,6 +422,66 @@ func (c *Core) EvaluateExpression(line string) (string, bool, hcl.Diagnostics) {
 	}
 }
 
+func (c *Core) FixConfig(opts FixConfigOptions) hcl.Diagnostics {
+	var diags hcl.Diagnostics
+
+	// Remove once we have support for the Inplace FixConfigMode
+	if opts.Mode != Diff {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  fmt.Sprintf("FixConfig only supports template diff; FixConfigMode %d not supported", opts.Mode),
+		})
+
+		return diags
+	}
+
+	var rawTemplateData map[string]interface{}
+	input := make(map[string]interface{})
+	templateData := make(map[string]interface{})
+	if err := json.Unmarshal(c.Template.RawContents, &rawTemplateData); err != nil {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  fmt.Sprintf("unable to read the contents of the JSON configuration file: %s", err),
+			Detail:   err.Error(),
+		})
+		return diags
+	}
+	// Hold off on Diff for now - need to think about displaying to user.
+	// delete empty top-level keys since the fixers seem to add them
+	// willy-nilly
+	for k := range input {
+		ml, ok := input[k].([]map[string]interface{})
+		if !ok {
+			continue
+		}
+		if len(ml) == 0 {
+			delete(input, k)
+		}
+	}
+	// marshal/unmarshal to make comparable to templateData
+	var fixedData map[string]interface{}
+	// Guaranteed to be valid json, so we can ignore errors
+	j, _ := json.Marshal(input)
+	if err := json.Unmarshal(j, &fixedData); err != nil {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  fmt.Sprintf("unable to read the contents of the JSON configuration file: %s", err),
+			Detail:   err.Error(),
+		})
+
+		return diags
+	}
+
+	if diff := cmp.Diff(templateData, fixedData); diff != "" {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Fixable configuration found.\nPlease run `packer fix` to get your build to run correctly.\nSee debug log for more information.",
+			Detail:   diff,
+		})
+	}
+	return diags
+}
+
 // validate does a full validation of the template.
 //
 // This will automatically call template.validate() in addition to doing

packer/run_interfaces.go

@@ -28,6 +28,7 @@ type Evaluator interface {
 type Handler interface {
 	Evaluator
 	BuildGetter
+	ConfigFixer
 }
 
 //go:generate enumer -type FixConfigMode
@@ -44,7 +45,7 @@ const (
 )
 
 type FixConfigOptions struct {
-	DiffOnly bool
+	Mode FixConfigMode
 }
 
 type ConfigFixer interface {

website/pages/docs/commands/validate.mdx

@@ -33,13 +33,11 @@ Errors validating build 'vmware'. 1 error(s) occurred:
 - `-syntax-only` - Only the syntax of the template is checked. The
   configuration is not validated.
 
-- `-except=foo,bar,baz` - Builds all the builds and post-processors except
-  those with the given comma-separated names. Build and post-processor names
-  by default are the names of their builders, unless a specific `name`
-  attribute is specified within the configuration. A post-processor with an
-  empty name will be ignored.
+- `-except=foo,bar,baz` - Validates all the builds except those with the
+  comma-separated names. Build names by default are the names of their
+  builders, unless a specific `name` attribute is specified within the configuration.
 
-- `-only=foo,bar,baz` - Only build the builds with the given comma-separated
+- `-only=foo,bar,baz` - Only validate the builds with the given comma-separated
   names. Build names by default are the names of their builders, unless a
   specific `name` attribute is specified within the configuration.