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..7ed051c3489 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. @@ -67,7 +67,8 @@ type MetaArgs struct { Vars map[string]string VarFiles []string // set to "hcl2" to force hcl2 mode - ConfigType configType + ConfigType configType + StrictValidation bool } func (ba *BuildArgs) AddFlagSets(flags *flag.FlagSet) { @@ -88,9 +89,10 @@ func (ba *BuildArgs) AddFlagSets(flags *flag.FlagSet) { // 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 +131,7 @@ type FixArgs struct { func (va *ValidateArgs) AddFlagSets(flags *flag.FlagSet) { flags.BoolVar(&va.SyntaxOnly, "syntax-only", false, "check syntax only") + flags.BoolVar(&va.WarnOnUndeclared, "warn-on-undeclared", true, "show warnings for pkrvars definition files containing undeclared variables") va.MetaArgs.AddFlagSets(flags) } @@ -136,7 +139,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, WarnOnUndeclared bool } func (va *InspectArgs) AddFlagSets(flags *flag.FlagSet) { diff --git a/command/meta.go b/command/meta.go index bf5a6d662d4..afc04228ff8 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{ + Strict: cla.StrictValidation, + }, } 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 8bef1ffc6db..b892ed787dc 100644 --- a/command/validate.go +++ b/command/validate.go @@ -45,6 +45,8 @@ func (c *ValidateCommand) ParseArgs(args []string) (*ValidateArgs, int) { } func (c *ValidateCommand) RunContext(ctx context.Context, cla *ValidateArgs) int { + cla.StrictValidation = cla.WarnOnUndeclared + packerStarter, ret := c.GetConfig(&cla.MetaArgs) if ret != 0 { return 1 @@ -59,6 +61,7 @@ func (c *ValidateCommand) RunContext(ctx context.Context, cla *ValidateArgs) int diags := packerStarter.Initialize(packer.InitializeOptions{ SkipDatasourcesExecution: true, }) + ret = writeDiags(c.Ui, nil, diags) if ret != 0 { return ret @@ -95,12 +98,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. + -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. + -warn-on-undeclared=false Disable warnings for JSON or HCL2 files containing undeclared variables. (Default true) ` return strings.TrimSpace(helpText) diff --git a/command/validate_test.go b/command/validate_test.go index eb63d73061d..0ca249fb66c 100644 --- a/command/validate_test.go +++ b/command/validate_test.go @@ -138,7 +138,7 @@ func TestValidateCommandBadVersion(t *testing.T) { } stdout, stderr := GetStdoutAndErrFromTestMeta(t, c.Meta) - expected := `Error: + expected := `Error: This template requires Packer version 101.0.0 or higher; using 100.0.0 @@ -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: "warn-on-undeclared=true 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: "warn-on-undeclared=true 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.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: "warn-on-undeclared=false 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: "warn-on-undeclared=false 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{"-warn-on-undeclared=false", "-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 76831731eba..b51622bd499 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 ( @@ -137,6 +139,7 @@ func (p *Parser) Parse(filename string, varFiles []string, argVars map[string]st Basedir: basedir, Cwd: wd, CorePackerVersionString: p.CorePackerVersionString, + ValidationOptions: p.ValidationOptions, parser: p, files: files, } diff --git a/hcl2template/types.variables.go b/hcl2template/types.variables.go index 9131d5f3382..8624991168b 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.Strict == false { + 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..190def39809 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{}, @@ -867,7 +867,7 @@ func TestVariables_collectVariableValues(t *testing.T) { // output wantDiags: true, - wantDiagsHasError: true, + wantDiagsHasError: false, wantVariables: Variables{}, wantValues: map[string]cty.Value{}, },