diff --git a/command/build.go b/command/build.go index d2977d0ae36..1cf9cdd61cc 100644 --- a/command/build.go +++ b/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) diff --git a/command/build_test.go b/command/build_test.go index e354fe02a97..a306fbde252 100644 --- a/command/build_test.go +++ b/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", diff --git a/command/cli.go b/command/cli.go index a9486cd5282..826835ef2b3 100644 --- a/command/cli.go +++ b/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,6 +135,7 @@ 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) } @@ -136,7 +143,7 @@ func (va *ValidateArgs) AddFlagSets(flags *flag.FlagSet) { // 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) { diff --git a/command/meta.go b/command/meta.go index d6ad45ae96b..3f166595604 100644 --- a/command/meta.go +++ b/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) diff --git a/command/test-fixtures/validate/var-file-tests/basic.pkr.hcl b/command/test-fixtures/validate/var-file-tests/basic.pkr.hcl new file mode 100644 index 00000000000..1fd332583be --- /dev/null +++ b/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"] +} diff --git a/command/test-fixtures/validate/var-file-tests/basic.pkrvars.hcl b/command/test-fixtures/validate/var-file-tests/basic.pkrvars.hcl new file mode 100644 index 00000000000..efa60c7226d --- /dev/null +++ b/command/test-fixtures/validate/var-file-tests/basic.pkrvars.hcl @@ -0,0 +1 @@ +test = "myvalue" diff --git a/command/test-fixtures/validate/var-file-tests/undeclared.json b/command/test-fixtures/validate/var-file-tests/undeclared.json new file mode 100644 index 00000000000..5004198d96b --- /dev/null +++ b/command/test-fixtures/validate/var-file-tests/undeclared.json @@ -0,0 +1,3 @@ +{ + "unused": ["one", "cookie", "two", "cookie"] +} diff --git a/command/test-fixtures/validate/var-file-tests/undeclared.pkrvars.hcl b/command/test-fixtures/validate/var-file-tests/undeclared.pkrvars.hcl new file mode 100644 index 00000000000..dace598cee6 --- /dev/null +++ b/command/test-fixtures/validate/var-file-tests/undeclared.pkrvars.hcl @@ -0,0 +1 @@ +unused = "unused variables should warn" diff --git a/command/validate.go b/command/validate.go index c8ac937107a..304db1a380e 100644 --- a/command/validate.go +++ b/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) diff --git a/command/validate_test.go b/command/validate_test.go index eb63d73061d..5b9b346aaf6 100644 --- a/command/validate_test.go +++ b/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) + }) + } +} diff --git a/hcl2template/parser.go b/hcl2template/parser.go index 95b18ad1b61..d2c5a85ecd2 100644 --- a/hcl2template/parser.go +++ b/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, } diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 55e0f80ea50..8325d0e908b 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -68,7 +68,7 @@ type PackerConfig struct { } type ValidationOptions struct { - Strict bool + WarnOnUndeclaredVar bool } const ( diff --git a/hcl2template/types.variables.go b/hcl2template/types.variables.go index 9131d5f3382..77928747c8d 100644 --- a/hcl2template/types.variables.go +++ b/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(), diff --git a/hcl2template/types.variables_test.go b/hcl2template/types.variables_test.go index 1abe331a85b..607862c9080 100644 --- a/hcl2template/types.variables_test.go +++ b/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,7 +859,7 @@ 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"`}, @@ -867,7 +867,7 @@ func TestVariables_collectVariableValues(t *testing.T) { // output wantDiags: true, - wantDiagsHasError: true, + wantDiagsHasError: false, wantVariables: Variables{}, wantValues: map[string]cty.Value{}, },