From dbeeab448a8bedd405f5cdce7e9a7243c9c22256 Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Wed, 30 Nov 2022 15:25:25 -0500 Subject: [PATCH] cmd/hcl2_upgrade: Generate variable block for all referenced user input variables (#12136) Currently the HCL2 upgrade command generates a valid variable block for all variables within the JSON variables property. However JSON templates in Packer support variable interpolation for input variables defined within a variable definition file without it being declared within the variables property. When upgrading a JSON template to HCL2 the user variable reference gets converted to var.. The upgraded template ultimately fails on validation or build execution with undefined variable errors, with this change the upgrade command will now create a variable block for all input variables referenced within the build template to ensure all required variables been defined. --- command/hcl2_upgrade.go | 48 ++++++++++++++++++- command/hcl2_upgrade_test.go | 7 +-- .../undeclared-variables/expected.pkr.hcl | 26 ++++++++++ .../undeclared-variables/input.json | 18 +++++++ 4 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 command/test-fixtures/hcl2_upgrade/undeclared-variables/expected.pkr.hcl create mode 100644 command/test-fixtures/hcl2_upgrade/undeclared-variables/input.json diff --git a/command/hcl2_upgrade.go b/command/hcl2_upgrade.go index a9dec8251..e149e1af0 100644 --- a/command/hcl2_upgrade.go +++ b/command/hcl2_upgrade.go @@ -623,6 +623,40 @@ func variableTransposeTemplatingCalls(s []byte) (isLocal bool, body []byte) { return isLocal, str.Bytes() } +// referencedUserVariables executes parts of blocks as go template files finding user variables referenced +// within the template. This function should be called once to extract those variables referenced via the {{user `...`}} +// template function. The resulting map will contain variables defined in the JSON variables property, and some that +// are declared via var-files; to avoid duplicates the results of this function should be reconciled against tpl.Variables. +func referencedUserVariables(s []byte) map[string]*template.Variable { + userVars := make([]string, 0) + funcMap := texttemplate.FuncMap{ + "user": func(in string) string { + userVars = append(userVars, in) + return "" + }, + } + + tpl, err := texttemplate.New("hcl2_upgrade"). + Funcs(funcMap). + Parse(string(s)) + if err != nil { + return nil + } + + if err := tpl.Execute(&bytes.Buffer{}, nil); err != nil { + return nil + } + + vars := make(map[string]*template.Variable) + for _, v := range userVars { + vars[v] = &template.Variable{ + Key: v, + Required: true, + } + } + return vars +} + func jsonBodyToHCL2Body(out *hclwrite.Body, kvs map[string]interface{}) { ks := []string{} for k := range kvs { @@ -836,6 +870,17 @@ func (p *VariableParser) Parse(tpl *template.Template) error { p.localsOut = []byte{} } + // JSON supports variable declaration via var-files. + // User variables that might be defined in a var-file + // but not in the actual JSON template should be accounted for. + userVars := referencedUserVariables(tpl.RawContents) + for name, variable := range userVars { + if _, ok := tpl.Variables[name]; ok { + continue + } + tpl.Variables[name] = variable + } + variables := []*template.Variable{} { // sort variables to avoid map's randomness @@ -852,7 +897,8 @@ func (p *VariableParser) Parse(tpl *template.Template) error { // field with the "Default" value from the JSON variable. // Interpolate Jsonval first as an hcl variable to determine if it is - // a local. + // a local. Variables referencing some form of variable expression must be defined as a local in HCL2, + // as variables in HCL2 must have a known value at parsing time. isLocal, _ := variableTransposeTemplatingCalls([]byte(variable.Default)) sensitive := false if isSensitiveVariable(variable.Key, tpl.SensitiveVariables) { diff --git a/command/hcl2_upgrade_test.go b/command/hcl2_upgrade_test.go index 598ae4a3f..45da6a1cd 100644 --- a/command/hcl2_upgrade_test.go +++ b/command/hcl2_upgrade_test.go @@ -24,15 +24,16 @@ func Test_hcl2_upgrade(t *testing.T) { {folder: "source-name", flags: []string{"-with-annotations"}}, {folder: "error-cleanup-provisioner", flags: []string{"-with-annotations"}}, {folder: "aws-access-config", flags: []string{}}, - {folder: "variables-only", flags: []string{}}, - {folder: "variables-with-variables", flags: []string{}}, - {folder: "complete-variables-with-template-engine", flags: []string{}}, {folder: "escaping", flags: []string{}}, {folder: "vsphere_linux_options_and_network_interface", exitCode: 1, flags: []string{}}, {folder: "nonexistent", flags: []string{}, exitCode: 1, exitEarly: true}, {folder: "placeholders", flags: []string{}, exitCode: 0}, {folder: "ami_test", flags: []string{}, exitCode: 0}, {folder: "azure_shg", flags: []string{}, exitCode: 0}, + {folder: "variables-only", flags: []string{}}, + {folder: "variables-with-variables", flags: []string{}}, + {folder: "complete-variables-with-template-engine", flags: []string{}}, + {folder: "undeclared-variables", flags: []string{}, exitCode: 0}, } for _, tc := range tc { diff --git a/command/test-fixtures/hcl2_upgrade/undeclared-variables/expected.pkr.hcl b/command/test-fixtures/hcl2_upgrade/undeclared-variables/expected.pkr.hcl new file mode 100644 index 000000000..cee87f967 --- /dev/null +++ b/command/test-fixtures/hcl2_upgrade/undeclared-variables/expected.pkr.hcl @@ -0,0 +1,26 @@ + +variable "communicator_type" { + type = string +} + +variable "foo" { + type = string + default = "bar" +} + +variable "scriptpath" { + type = string +} + +source "null" "autogenerated_1" { + communicator = "${var.communicator_type}" +} + +build { + sources = ["source.null.autogenerated_1"] + + provisioner "shell" { + script = "${var.scriptpath}" + } + +} diff --git a/command/test-fixtures/hcl2_upgrade/undeclared-variables/input.json b/command/test-fixtures/hcl2_upgrade/undeclared-variables/input.json new file mode 100644 index 000000000..ba6e19858 --- /dev/null +++ b/command/test-fixtures/hcl2_upgrade/undeclared-variables/input.json @@ -0,0 +1,18 @@ +{ + "variables": { + "foo": "bar" + }, + "builders": [ + { + "type": "null", + "communicator": "{{ user `communicator_type` }}" + } + ], + "provisioners": [ + { + "type": "shell", + "script": "{{user `scriptpath`}}" + } + ] +} +