tweak validation & add tests

hcl2template/parser.go

@@ -123,7 +123,7 @@ func (p *Parser) parse(filename string, vars map[string]string) (*PackerConfig,
 			varFiles = append(varFiles, f)
 		}
 
-		diags = append(diags, cfg.InputVariables.collectVariableValues(os.Environ(), varFiles, vars)...)
+		diags = append(diags, cfg.collectInputVariableValues(os.Environ(), varFiles, vars)...)
 	}
 
 	// decode the actual content

hcl2template/types.packer_config.go

@@ -25,10 +25,16 @@ type PackerConfig struct {
 	InputVariables Variables
 	LocalVariables Variables
 
+	ValidationOptions
+
 	// Builds is the list of Build blocks defined in the config files.
 	Builds Builds
 }
 
+type ValidationOptions struct {
+	Strict bool
+}
+
 // EvalContext returns the *hcl.EvalContext that will be passed to an hcl
 // decoder in order to tell what is the actual value of a var or a local and
 // the list of defined functions.

hcl2template/types.variables.go

@@ -223,8 +223,9 @@ func (variables *Variables) decodeVariableBlock(block *hcl.Block, ectx *hcl.Eval
 // them.
 const VarEnvPrefix = "PKR_VAR_"
 
-func (variables Variables) collectVariableValues(env []string, files []*hcl.File, argv map[string]string) hcl.Diagnostics {
+func (cfg *PackerConfig) collectInputVariableValues(env []string, files []*hcl.File, argv map[string]string) hcl.Diagnostics {
 	var diags hcl.Diagnostics
+	variables := cfg.InputVariables
 
 	for _, raw := range env {
 		if !strings.HasPrefix(raw, VarEnvPrefix) {
@@ -321,7 +322,20 @@ func (variables Variables) collectVariableValues(env []string, files []*hcl.File
 		for name, attr := range attrs {
 			variable, found := variables[name]
 			if !found {
-				// No file defines this variable; let's skip it
+				sev := hcl.DiagWarning
+				if cfg.ValidationOptions.Strict {
+					sev = hcl.DiagError
+				}
+				diags = append(diags, &hcl.Diagnostic{
+					Severity: sev,
+					Summary:  "Undefined variable",
+					Detail: fmt.Sprintf("A %q variable was set but was "+
+						"not found in known variables. To declare "+
+						"variable %q, place this block in one of your"+
+						".pkr files, such as variables.pkr.hcl",
+						name, name),
+					Context: attr.Range.Ptr(),
+				})
 				continue
 			}
 
@@ -351,8 +365,8 @@ func (variables Variables) collectVariableValues(env []string, files []*hcl.File
 		variable, found := variables[name]
 		if !found {
 			diags = append(diags, &hcl.Diagnostic{
-				Severity: hcl.DiagWarning,
+				Severity: hcl.DiagError,
-				Summary:  "Unknown -var variable",
+				Summary:  "Undefined -var variable",
 				Detail: fmt.Sprintf("A %q variable was passed in the command "+
 					"line but was not found in known variables."+
 					"To declare variable %q, place this block in one of your"+

hcl2template/types.variables_test.go

@@ -247,12 +247,14 @@ func TestVariables_collectVariableValues(t *testing.T) {
 		argv     map[string]string
 	}
 	tests := []struct {
-		name          string
+		name              string
-		variables     Variables
+		variables         Variables
-		args          args
+		validationOptions ValidationOptions
-		wantDiags     bool
+		args              args
-		wantVariables Variables
+		wantDiags         bool
-		wantValues    map[string]cty.Value
+		wantDiagsHasError bool
+		wantVariables     Variables
+		wantValues        map[string]cty.Value
 	}{
 
 		{name: "string",
@@ -333,11 +335,39 @@ func TestVariables_collectVariableValues(t *testing.T) {
 			},
 		},
 
-		{name: "undefined but set value",
+		{name: "undefined but set value - pkrvar file - normal mode",
 			variables: Variables{},
 			args: args{
-				env:      []string{`PKR_VAR_unused_string=value`},
+				hclFiles: []string{`undefined_string="value"`},
-				hclFiles: []string{`unused_string="value"`},
+			},
+
+			// output
+			wantDiags:         true,
+			wantDiagsHasError: false,
+			wantVariables:     Variables{},
+			wantValues:        map[string]cty.Value{},
+		},
+
+		{name: "undefined but set value - pkrvar file - strict mode",
+			variables: Variables{},
+			validationOptions: ValidationOptions{
+				Strict: true,
+			},
+			args: args{
+				hclFiles: []string{`undefined_string="value"`},
+			},
+
+			// output
+			wantDiags:         true,
+			wantDiagsHasError: true,
+			wantVariables:     Variables{},
+			wantValues:        map[string]cty.Value{},
+		},
+
+		{name: "undefined but set value - env",
+			variables: Variables{},
+			args: args{
+				env: []string{`PKR_VAR_undefined_string=value`},
 			},
 
 			// output
@@ -346,18 +376,19 @@ func TestVariables_collectVariableValues(t *testing.T) {
 			wantValues:    map[string]cty.Value{},
 		},
 
-		{name: "undefined but set value - args",
+		{name: "undefined but set value - argv",
 			variables: Variables{},
 			args: args{
 				argv: map[string]string{
-					"unused_string": "value",
+					"undefined_string": "value",
 				},
 			},
 
 			// output
-			wantDiags:     true,
+			wantDiags:         true,
-			wantVariables: Variables{},
+			wantDiagsHasError: true,
-			wantValues:    map[string]cty.Value{},
+			wantVariables:     Variables{},
+			wantValues:        map[string]cty.Value{},
 		},
 
 		{name: "value not corresponding to type - env",
@@ -371,7 +402,8 @@ func TestVariables_collectVariableValues(t *testing.T) {
 			},
 
 			// output
-			wantDiags: true,
+			wantDiags:         true,
+			wantDiagsHasError: true,
 			wantVariables: Variables{
 				"used_string": &Variable{
 					Type:     cty.String,
@@ -394,7 +426,8 @@ func TestVariables_collectVariableValues(t *testing.T) {
 			},
 
 			// output
-			wantDiags: true,
+			wantDiags:         true,
+			wantDiagsHasError: true,
 			wantVariables: Variables{
 				"used_string": &Variable{
 					Type:         cty.String,
@@ -419,7 +452,8 @@ func TestVariables_collectVariableValues(t *testing.T) {
 			},
 
 			// output
-			wantDiags: true,
+			wantDiags:         true,
+			wantDiagsHasError: true,
 			wantVariables: Variables{
 				"used_string": &Variable{
 					Type:     cty.String,
@@ -438,9 +472,10 @@ func TestVariables_collectVariableValues(t *testing.T) {
 			},
 
 			// output
-			wantDiags:     true,
+			wantDiags:         true,
-			wantVariables: Variables{},
+			wantDiagsHasError: true,
-			wantValues:    map[string]cty.Value{},
+			wantVariables:     Variables{},
+			wantValues:        map[string]cty.Value{},
 		},
 	}
 	for _, tt := range tests {
@@ -454,9 +489,17 @@ func TestVariables_collectVariableValues(t *testing.T) {
 				}
 				files = append(files, file)
 			}
-			if gotDiags := tt.variables.collectVariableValues(tt.args.env, files, tt.args.argv); (gotDiags == nil) == tt.wantDiags {
+			cfg := &PackerConfig{
+				InputVariables:    tt.variables,
+				ValidationOptions: tt.validationOptions,
+			}
+			gotDiags := cfg.collectInputVariableValues(tt.args.env, files, tt.args.argv)
+			if (gotDiags == nil) == tt.wantDiags {
 				t.Fatalf("Variables.collectVariableValues() = %v, want %v", gotDiags, tt.wantDiags)
 			}
+			if tt.wantDiagsHasError != gotDiags.HasErrors() {
+				t.Fatalf("Variables.collectVariableValues() unexpected diagnostics HasErrors. %s", gotDiags)
+			}
 			if diff := cmp.Diff(fmt.Sprintf("%#v", tt.wantVariables), fmt.Sprintf("%#v", tt.variables)); diff != "" {
 				t.Fatalf("didn't get expected variables: %s", diff)
 			}