Skip to content

Commit

Permalink
Update validation options for undeclared variables
Browse files Browse the repository at this point in the history
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 -warn-on-undeclared=false -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
```
  • Loading branch information
nywilken committed Nov 10, 2022
1 parent 113bc5e commit 9f1dfc4
Show file tree
Hide file tree
Showing 12 changed files with 193 additions and 37 deletions.
12 changes: 2 additions & 10 deletions command/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 9 additions & 6 deletions command/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -129,14 +131,15 @@ 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)
}

// 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) {
Expand Down
3 changes: 3 additions & 0 deletions command/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 17 additions & 0 deletions command/test-fixtures/validate/var-file-tests/basic.pkr.hcl
Original file line number Diff line number Diff line change
@@ -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"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test = "myvalue"
3 changes: 3 additions & 0 deletions command/test-fixtures/validate/var-file-tests/undeclared.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"unused": ["one", "cookie", "two", "cookie"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
unused = "unused variables should warn"
16 changes: 10 additions & 6 deletions command/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
131 changes: 130 additions & 1 deletion command/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
})
}
}
7 changes: 5 additions & 2 deletions hcl2template/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ type Parser struct {

CorePackerVersionString string

*hclparse.Parser

PluginConfig *packer.PluginConfig

ValidationOptions

*hclparse.Parser
}

const (
Expand Down Expand Up @@ -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,
}
Expand Down
18 changes: 9 additions & 9 deletions hcl2template/types.variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
6 changes: 3 additions & 3 deletions hcl2template/types.variables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -850,7 +850,7 @@ func TestVariables_collectVariableValues(t *testing.T) {
},

// output
wantDiags: true,
wantDiags: false,
wantDiagsHasError: false,
wantVariables: Variables{},
wantValues: map[string]cty.Value{},
Expand All @@ -867,7 +867,7 @@ func TestVariables_collectVariableValues(t *testing.T) {

// output
wantDiags: true,
wantDiagsHasError: true,
wantDiagsHasError: false,
wantVariables: Variables{},
wantValues: map[string]cty.Value{},
},
Expand Down

0 comments on commit 9f1dfc4

Please sign in to comment.