core: Update validation options for undeclared variables (#12104)

* Update validation options for undeclared variables

In an effort to help users move from JSON to HCL2 templates the support for
variable definitions files are being updated to ignore undeclared
variable warnings on build execution. For legacy JSON templates builds no
warnings are displayed when var-files contain undeclared variables.

Since preferred mode HCL2 templates is to be explicit with variable
declarations - they must be declared to be used - validation for
undeclared variables still warns when running `packer validate`. A new
flag has been added to the validate command that can be used to disable
undeclared variable warnings.

* Update validation test for unused variables

Example Run
```
~>  go run . validate -no-warn-undeclared-var -var-file
command/test-fixtures/validate/var-file-tests/undeclared.pkrvars.hcl
command/test-fixtures/validate/var-file-tests/basic.pkr.hcl
The configuration is valid.

~>  go run . validate -var-file
command/test-fixtures/validate/var-file-tests/undeclared.pkrvars.hcl
command/test-fixtures/validate/var-file-tests/basic.pkr.hcl
Warning: Undefined variable

The variable "unused" was set but was not declared as an input variable.
To declare variable "unused" place this block in one of your .pkr.hcl
files,
such as variables.pkr.hcl

variable "unused" {
  type    = string
  default = null

}

The configuration is valid.

~>  go run . build -var-file
command/test-fixtures/validate/var-file-tests/undeclared.pkrvars.hcl
command/test-fixtures/validate/var-file-tests/basic.pkr.hcl
file.chocolate: output will be in this color.

Build 'file.chocolate' finished after 744 microseconds.

==> Wait completed after 798 microseconds

==> Builds finished. The artifacts of successful builds are:
--> file.chocolate: Stored file: chocolate.txt
```

* Rename Strict field to WarnOnUndeclaredVar

The field name Strict is a bit vague since it is only used for
checking against undeclared variables within a var-file definition.
To mitigate against potential overloading of this field it is
being renamed to be more explicit on its usage.

* command/build: Add warn-on-undeclared-var flag

Now that the default behaviour is to not display warnings for undeclared variables
an optional flag has been added to toggle the old behaviour.

```
~>  go run . build -warn-on-undeclared-var -var-file command/test-fixtures/validate/var-file-tests/undeclared.pkrvars.hcl command/test-fixtures/validate/var-file-tests/basic.pkr.hcl
Warning: Undefined variable

The variable "unused" was set but was not declared as an input variable.
To declare variable "unused" place this block in one of your .pkr.hcl files,
such as variables.pkr.hcl

variable "unused" {
  type    = string
  default = null

}

file.chocolate: output will be in this color.

Build 'file.chocolate' finished after 762 microseconds.

==> Wait completed after 799 microseconds

==> Builds finished. The artifacts of successful builds are:
--> file.chocolate: Stored file: chocolate.txt
```

command/build.go

@@ -400,7 +400,8 @@ Options:
   -parallel-builds=1            Number of builds to run in parallel. 1 disables parallelization. 0 means no limit (Default: 0)
   -timestamp-ui                 Enable prefixing of each ui output with an RFC3339 timestamp.
   -var 'key=value'              Variable for templates, can be used multiple times.
-  -var-file=path                JSON or HCL2 file containing user variables.
+  -var-file=path                JSON or HCL2 file containing user variables, can be used multiple times.
+  -warn-on-undeclared-var       Display warnings for user variable files containing undeclared variables.
 `
 
 	return strings.TrimSpace(helpText)

command/build_test.go

@@ -1149,21 +1149,13 @@ func TestBuildCmd(t *testing.T) {
 			},
 			expectedCode: 0,
 			outputCheck: func(out, err string) error {
-				if !strings.Contains(out, "Warning: Undefined variable") {
-					return fmt.Errorf("expected 'Warning: Undefined variable' in output, did not find it")
-				}
-
 				nbWarns := strings.Count(out, "Warning: ")
-				if nbWarns != 1 {
+				if nbWarns != 0 {
 					return fmt.Errorf(
-						"error: too many warnings in build output, expected 1, got %d",
+						"error: too many warnings in build output, expected 0, got %d",
 						nbWarns)
 				}
 
-				if !strings.Contains(out, "variable \"testvar\" {") {
-					return fmt.Errorf("missing definition example for undefined variable")
-				}
-
 				nbErrs := strings.Count(err, "Error: ")
 				if nbErrs != 0 {
 					return fmt.Errorf("error: expected build to succeed without errors, got %d",

command/cli.go

@@ -58,7 +58,7 @@ func (ma *MetaArgs) AddFlagSets(fs *flag.FlagSet) {
 	fs.Var(&ma.ConfigType, "config-type", "set to 'hcl2' to run in hcl2 mode when no file is passed.")
 }
 
-// MetaArgs defines commonalities between all comands
+// MetaArgs defines commonalities between all commands
 type MetaArgs struct {
 	// TODO(azr): in the future, I want to allow passing multiple path to
 	// merge HCL confs together; but this will probably need an RFC first.
@@ -68,6 +68,10 @@ type MetaArgs struct {
 	VarFiles     []string
 	// set to "hcl2" to force hcl2 mode
 	ConfigType configType
+
+	// WarnOnUndeclared does not have a common default, as the default varies per sub-command usage.
+	// Refer to individual command FlagSets for usage.
+	WarnOnUndeclaredVar bool
 }
 
 func (ba *BuildArgs) AddFlagSets(flags *flag.FlagSet) {
@@ -82,15 +86,17 @@ func (ba *BuildArgs) AddFlagSets(flags *flag.FlagSet) {
 	flagOnError := enumflag.New(&ba.OnError, "cleanup", "abort", "ask", "run-cleanup-provisioner")
 	flags.Var(flagOnError, "on-error", "")
 
+	flags.BoolVar(&ba.MetaArgs.WarnOnUndeclaredVar, "warn-on-undeclared-var", false, "Show warnings for variable files containing undeclared variables.")
 	ba.MetaArgs.AddFlagSets(flags)
 }
 
 // BuildArgs represents a parsed cli line for a `packer build`
 type BuildArgs struct {
 	MetaArgs
-	Color, Debug, Force, TimestampUi, MachineReadable bool
-	ParallelBuilds                                    int64
-	OnError                                           string
+	Debug, Force                        bool
+	Color, TimestampUi, MachineReadable bool
+	ParallelBuilds                      int64
+	OnError                             string
 }
 
 func (ia *InitArgs) AddFlagSets(flags *flag.FlagSet) {
@@ -129,14 +135,15 @@ type FixArgs struct {
 
 func (va *ValidateArgs) AddFlagSets(flags *flag.FlagSet) {
 	flags.BoolVar(&va.SyntaxOnly, "syntax-only", false, "check syntax only")
+	flags.BoolVar(&va.NoWarnUndeclaredVar, "no-warn-undeclared-var", false, "Ignore warnings for variable files containing undeclared variables.")
 
 	va.MetaArgs.AddFlagSets(flags)
 }
 
 // ValidateArgs represents a parsed cli line for a `packer validate`
 type ValidateArgs struct {
 	MetaArgs
-	SyntaxOnly bool
+	SyntaxOnly, NoWarnUndeclaredVar bool
 }
 
 func (va *InspectArgs) AddFlagSets(flags *flag.FlagSet) {

command/meta.go

@@ -136,6 +136,9 @@ func (m *Meta) GetConfigFromHCL(cla *MetaArgs) (*hcl2template.PackerConfig, int)
 		CorePackerVersionString: version.FormattedVersion(),
 		Parser:                  hclparse.NewParser(),
 		PluginConfig:            m.CoreConfig.Components.PluginConfig,
+		ValidationOptions: hcl2template.ValidationOptions{
+			WarnOnUndeclaredVar: cla.WarnOnUndeclaredVar,
+		},
 	}
 	cfg, diags := parser.Parse(cla.Path, cla.VarFiles, cla.Vars)
 	return cfg, writeDiags(m.Ui, parser.Files(), diags)

command/test-fixtures/validate/var-file-tests/basic.pkr.hcl

@@ -0,0 +1,17 @@
+packer {
+  required_version = ">= v1.0.0"
+}
+
+variable "test" {
+ type = string
+ default = null
+}
+
+source "file" "chocolate" {
+  target = "chocolate.txt"
+  content = "chocolate"
+}
+
+build {
+  sources = ["source.file.chocolate"]
+}

command/test-fixtures/validate/var-file-tests/basic.pkrvars.hcl

@@ -0,0 +1 @@
+test = "myvalue"

command/test-fixtures/validate/var-file-tests/undeclared.json

@@ -0,0 +1,3 @@
+{
+    "unused": ["one", "cookie", "two", "cookie"]
+}

command/test-fixtures/validate/var-file-tests/undeclared.pkrvars.hcl

@@ -0,0 +1 @@
+unused = "unused variables should warn"

command/validate.go

@@ -45,6 +45,12 @@ func (c *ValidateCommand) ParseArgs(args []string) (*ValidateArgs, int) {
 }
 
 func (c *ValidateCommand) RunContext(ctx context.Context, cla *ValidateArgs) int {
+	// By default we want to inform users of undeclared variables when validating but not during build time.
+	cla.MetaArgs.WarnOnUndeclaredVar = true
+	if cla.NoWarnUndeclaredVar {
+		cla.MetaArgs.WarnOnUndeclaredVar = false
+	}
+
 	packerStarter, ret := c.GetConfig(&cla.MetaArgs)
 	if ret != 0 {
 		return 1
@@ -95,12 +101,13 @@ Usage: packer validate [options] TEMPLATE
 
 Options:
 
-  -syntax-only           Only check syntax. Do not verify config of the template.
-  -except=foo,bar,baz    Validate all builds other than these.
-  -machine-readable      Produce machine-readable output.
-  -only=foo,bar,baz      Validate only these builds.
-  -var 'key=value'       Variable for templates, can be used multiple times.
-  -var-file=path         JSON or HCL2 file containing user variables.
+  -syntax-only                  Only check syntax. Do not verify config of the template.
+  -except=foo,bar,baz           Validate all builds other than these.
+  -only=foo,bar,baz             Validate only these builds.
+  -machine-readable             Produce machine-readable output.
+  -var 'key=value'              Variable for templates, can be used multiple times.
+  -var-file=path                JSON or HCL2 file containing user variables, can be used multiple times.
+  -no-warn-undeclared-var       Disable warnings for user variable files containing undeclared variables.
 `
 
 	return strings.TrimSpace(helpText)

command/validate_test.go

@@ -205,3 +205,132 @@ func TestValidateCommandExcept(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateCommand_VarFiles(t *testing.T) {
+	tt := []struct {
+		name     string
+		path     string
+		varfile  string
+		exitCode int
+	}{
+		{name: "with basic HCL var-file definition",
+			path:     filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "basic.pkr.hcl"),
+			varfile:  filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "basic.pkrvars.hcl"),
+			exitCode: 0,
+		},
+		{name: "with unused variable in var-file definition",
+			path:     filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "basic.pkr.hcl"),
+			varfile:  filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "undeclared.pkrvars.hcl"),
+			exitCode: 0,
+		},
+		{name: "with unused variable in JSON var-file definition",
+			path:     filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "basic.pkr.hcl"),
+			varfile:  filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "undeclared.json"),
+			exitCode: 0,
+		},
+	}
+	for _, tc := range tt {
+		t.Run(tc.path, func(t *testing.T) {
+			c := &ValidateCommand{
+				Meta: TestMetaFile(t),
+			}
+			tc := tc
+			args := []string{"-var-file", tc.varfile, tc.path}
+			if code := c.Run(args); code != tc.exitCode {
+				fatalCommand(t, c.Meta)
+			}
+		})
+	}
+}
+
+func TestValidateCommand_VarFilesWarnOnUndeclared(t *testing.T) {
+	tt := []struct {
+		name     string
+		path     string
+		varfile  string
+		exitCode int
+	}{
+		{name: "default warning with unused variable in HCL var-file definition",
+			path:     filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "basic.pkr.hcl"),
+			varfile:  filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "undeclared.pkrvars.hcl"),
+			exitCode: 0,
+		},
+		{name: "default warning with unused variable in JSON var-file definition",
+			path:     filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "basic.pkr.hcl"),
+			varfile:  filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "undeclared.json"),
+			exitCode: 0,
+		},
+	}
+	for _, tc := range tt {
+		t.Run(tc.path, func(t *testing.T) {
+			c := &ValidateCommand{
+				Meta: TestMetaFile(t),
+			}
+			tc := tc
+			args := []string{"-var-file", tc.varfile, tc.path}
+			if code := c.Run(args); code != tc.exitCode {
+				fatalCommand(t, c.Meta)
+			}
+
+			stdout, stderr := GetStdoutAndErrFromTestMeta(t, c.Meta)
+			expected := `Warning: Undefined variable
+
+The variable "unused" was set but was not declared as an input variable.
+To declare variable "unused" place this block in one of your .pkr.hcl files,
+such as variables.pkr.hcl
+
+variable "unused" {
+  type    = string
+  default = null
+}
+
+
+The configuration is valid.
+`
+			if diff := cmp.Diff(expected, stdout); diff != "" {
+				t.Errorf("Unexpected output: %s", diff)
+			}
+			t.Log(stderr)
+		})
+	}
+}
+
+func TestValidateCommand_VarFilesDisableWarnOnUndeclared(t *testing.T) {
+	tt := []struct {
+		name     string
+		path     string
+		varfile  string
+		exitCode int
+	}{
+		{name: "no-warn-undeclared-var with unused variable in HCL var-file definition",
+			path:     filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "basic.pkr.hcl"),
+			varfile:  filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "undeclared.pkrvars.hcl"),
+			exitCode: 0,
+		},
+		{name: "no-warn-undeclared-var with unused variable in JSON var-file definition",
+			path:     filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "basic.pkr.hcl"),
+			varfile:  filepath.Join(testFixture(filepath.Join("validate", "var-file-tests")), "undeclared.json"),
+			exitCode: 0,
+		},
+	}
+	for _, tc := range tt {
+		t.Run(tc.path, func(t *testing.T) {
+			c := &ValidateCommand{
+				Meta: TestMetaFile(t),
+			}
+			tc := tc
+			args := []string{"-no-warn-undeclared-var", "-var-file", tc.varfile, tc.path}
+			if code := c.Run(args); code != tc.exitCode {
+				fatalCommand(t, c.Meta)
+			}
+
+			stdout, stderr := GetStdoutAndErrFromTestMeta(t, c.Meta)
+			expected := `The configuration is valid.
+`
+			if diff := cmp.Diff(expected, stdout); diff != "" {
+				t.Errorf("Unexpected output: %s", diff)
+			}
+			t.Log(stderr)
+		})
+	}
+}

hcl2template/parser.go

@@ -60,9 +60,11 @@ type Parser struct {
 
 	CorePackerVersionString string
 
-	*hclparse.Parser
-
 	PluginConfig *packer.PluginConfig
+
+	ValidationOptions
+
+	*hclparse.Parser
 }
 
 const (
@@ -138,6 +140,7 @@ func (p *Parser) Parse(filename string, varFiles []string, argVars map[string]st
 		Cwd:                     wd,
 		CorePackerVersionString: p.CorePackerVersionString,
 		HCPVars:                 map[string]cty.Value{},
+		ValidationOptions:       p.ValidationOptions,
 		parser:                  p,
 		files:                   files,
 	}

hcl2template/types.packer_config.go

@@ -68,7 +68,7 @@ type PackerConfig struct {
 }
 
 type ValidationOptions struct {
-	Strict bool
+	WarnOnUndeclaredVar bool
 }
 
 const (

hcl2template/types.variables.go

@@ -659,19 +659,19 @@ func (cfg *PackerConfig) collectInputVariableValues(env []string, files []*hcl.F
 		for name, attr := range attrs {
 			variable, found := variables[name]
 			if !found {
-				sev := hcl.DiagWarning
-				if cfg.ValidationOptions.Strict {
-					sev = hcl.DiagError
+				if !cfg.ValidationOptions.WarnOnUndeclaredVar {
+					continue
 				}
+
 				diags = append(diags, &hcl.Diagnostic{
-					Severity: sev,
+					Severity: hcl.DiagWarning,
 					Summary:  "Undefined variable",
-					Detail: fmt.Sprintf("A %[1]q variable was set but was "+
-						"not declared as an input variable. To declare "+
-						"variable %[1]q, place this block in one of your "+
-						".pkr.hcl files, such as variables.pkr.hcl\n\n"+
+					Detail: fmt.Sprintf("The variable %[1]q was set but was not declared as an input variable."+
+						"\nTo declare variable %[1]q place this block in one of your .pkr.hcl files, "+
+						"such as variables.pkr.hcl\n\n"+
 						"variable %[1]q {\n"+
-						"  type = string\n"+
+						"  type    = string\n"+
+						"  default = null\n"+
 						"}",
 						name),
 					Context: attr.Range.Ptr(),

hcl2template/types.variables_test.go

@@ -476,7 +476,7 @@ func TestParse_variables(t *testing.T) {
 				},
 				Basedir: filepath.Join("testdata", "variables"),
 			},
-			true, false,
+			false, false,
 			[]packersdk.Build{
 				&packer.CoreBuild{
 					Type:           "null.test",
@@ -850,7 +850,7 @@ func TestVariables_collectVariableValues(t *testing.T) {
 			},
 
 			// output
-			wantDiags:         true,
+			wantDiags:         false,
 			wantDiagsHasError: false,
 			wantVariables:     Variables{},
 			wantValues:        map[string]cty.Value{},
@@ -859,15 +859,15 @@ func TestVariables_collectVariableValues(t *testing.T) {
 		{name: "undefined but set value - pkrvar file - strict mode",
 			variables: Variables{},
 			validationOptions: ValidationOptions{
-				Strict: true,
+				WarnOnUndeclaredVar: true,
 			},
 			args: args{
 				hclFiles: []string{`undefined_string="value"`},
 			},
 
 			// output
 			wantDiags:         true,
-			wantDiagsHasError: true,
+			wantDiagsHasError: false,
 			wantVariables:     Variables{},
 			wantValues:        map[string]cty.Value{},
 		},