From ecb4c7debb18ba9996d5d2bb023c283b4bd462d7 Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Wed, 13 Apr 2022 13:19:25 -0500 Subject: [PATCH 1/2] Alter behavior of ResolveGlobalVariables * do not error when encountering string values that are not literal global variables * this ensures all values _except_ literal global variables are left unmodified * modify unit test to test a maps of size greater than 1 and mixed types (plain strings and global variables); issue went uncaught because iteration over maps does not have a guaranteed order and the code continued to succeed in limited testing --- pkg/config/config.go | 13 ++++---- pkg/config/config_test.go | 56 ++++++++++++++++++----------------- pkg/reswriter/packerwriter.go | 5 +++- 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 010292f8cb..bff4b2d2f1 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -454,12 +454,10 @@ func ConvertMapToCty(iMap map[string]interface{}) (map[string]cty.Value, error) // ResolveGlobalVariables given a map of strings to cty.Value types, will examine // all cty.Values that are of type cty.String. If they are literal global variables, -// then they are replaces by the cty.Value of the corresponding entry in yc.Vars -// the cty.Value types that are string literal global variables to their value +// then they are replaced by the cty.Value of the corresponding entry in +// yc.Vars. All other cty.Values are unmodified. // ERROR: if conversion from yc.Vars to map[string]cty.Value fails -// ERROR: if (somehow) the cty.String cannot be covnerted to a Go string -// ERROR: if there are literal variables which are not globals -// (this will be a use case we should consider) +// ERROR: if (somehow) the cty.String cannot be converted to a Go string // ERROR: rely on HCL TraverseAbs to bubble up "diagnostics" when the global variable // being resolved does not exist in yc.Vars func (yc *YamlConfig) ResolveGlobalVariables(ctyMap map[string]cty.Value) error { @@ -477,7 +475,8 @@ func (yc *YamlConfig) ResolveGlobalVariables(ctyMap map[string]cty.Value) error return err } ctx, varName, found := IdentifyLiteralVariable(valString) - // confirm literal and that it is global + // only attempt resolution on global literal variables + // leave all other strings alone (including non-global) if found && ctx == "var" { varTraversal := hcl.Traversal{ hcl.TraverseRoot{Name: ctx}, @@ -488,8 +487,6 @@ func (yc *YamlConfig) ResolveGlobalVariables(ctyMap map[string]cty.Value) error return diags } ctyMap[key] = newVal - } else { - return fmt.Errorf("%s was not a literal global variable ((var.name))", valString) } } } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index da52ed2dda..330b1a221e 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -505,49 +505,51 @@ func (s *MySuite) TestConvertMapToCty(c *C) { func (s *MySuite) TestResolveGlobalVariables(c *C) { var err error - var testkey = "testkey" + var testkey1 = "testkey1" + var testkey2 = "testkey2" + var testkey3 = "testkey3" bc := getBlueprintConfigForTest() ctyMap := make(map[string]cty.Value) err = bc.Config.ResolveGlobalVariables(ctyMap) c.Assert(err, IsNil) - // confirm that a plain string (non-variable) is unchanged and errors + // confirm plain string is unchanged and does not error testCtyString := cty.StringVal("testval") - ctyMap[testkey] = testCtyString + ctyMap[testkey1] = testCtyString err = bc.Config.ResolveGlobalVariables(ctyMap) - c.Assert(err, NotNil) - c.Assert(ctyMap[testkey], Equals, testCtyString) + c.Assert(err, IsNil) + c.Assert(ctyMap[testkey1], Equals, testCtyString) - // confirm that a literal, but not global, variable is unchanged and errors + // confirm literal, non-global, variable is unchanged and does not error testCtyString = cty.StringVal("((module.testval))") - ctyMap[testkey] = testCtyString + ctyMap[testkey1] = testCtyString err = bc.Config.ResolveGlobalVariables(ctyMap) - c.Assert(err, NotNil) - c.Assert(ctyMap[testkey], Equals, testCtyString) + c.Assert(err, IsNil) + c.Assert(ctyMap[testkey1], Equals, testCtyString) // confirm failed resolution of a literal global testCtyString = cty.StringVal("((var.test_global_var))") - ctyMap[testkey] = testCtyString + ctyMap[testkey1] = testCtyString err = bc.Config.ResolveGlobalVariables(ctyMap) + c.Assert(err, NotNil) c.Assert(err.Error(), Matches, ".*Unsupported attribute;.*") - // confirm successful resolution a literal global string - testGlobalVar := "test_global_var" - testGlobalVarString := "testval" - bc.Config.Vars[testGlobalVar] = testGlobalVarString - testCtyString = cty.StringVal(fmt.Sprintf("((var.%s))", testGlobalVar)) - ctyMap[testkey] = testCtyString - err = bc.Config.ResolveGlobalVariables(ctyMap) - c.Assert(err, IsNil) - c.Assert(ctyMap[testkey], Equals, cty.StringVal(testGlobalVarString)) - - // confirm successful resolution a literal global boolean - testGlobalVar = "test_global_var" - testGlobalVarBool := true - bc.Config.Vars[testGlobalVar] = testGlobalVarBool - testCtyString = cty.StringVal(fmt.Sprintf("((var.%s))", testGlobalVar)) - ctyMap[testkey] = testCtyString + // confirm successful resolution of literal globals in presence of other strings + testGlobalVarString := "test_global_string" + testGlobalValString := "testval" + testGlobalVarBool := "test_global_bool" + testGlobalValBool := "testval" + testPlainString := "plain-string" + bc.Config.Vars[testGlobalVarString] = testGlobalValString + bc.Config.Vars[testGlobalVarBool] = testGlobalValBool + testCtyString = cty.StringVal(fmt.Sprintf("((var.%s))", testGlobalVarString)) + testCtyBool := cty.StringVal(fmt.Sprintf("((var.%s))", testGlobalVarBool)) + ctyMap[testkey1] = testCtyString + ctyMap[testkey2] = testCtyBool + ctyMap[testkey3] = cty.StringVal(testPlainString) err = bc.Config.ResolveGlobalVariables(ctyMap) c.Assert(err, IsNil) - c.Assert(ctyMap[testkey], Equals, cty.BoolVal(testGlobalVarBool)) + c.Assert(ctyMap[testkey1], Equals, cty.StringVal(testGlobalValString)) + c.Assert(ctyMap[testkey2], Equals, cty.StringVal(testGlobalValBool)) + c.Assert(ctyMap[testkey3], Equals, cty.StringVal(testPlainString)) } diff --git a/pkg/reswriter/packerwriter.go b/pkg/reswriter/packerwriter.go index 05b93b7321..c8714cf4c8 100644 --- a/pkg/reswriter/packerwriter.go +++ b/pkg/reswriter/packerwriter.go @@ -61,7 +61,10 @@ func (w PackerWriter) writeResourceLevel(yamlConfig *config.YamlConfig, bpDirect return fmt.Errorf( "error converting global vars to cty for writing: %v", err) } - yamlConfig.ResolveGlobalVariables(ctySettings) + err = yamlConfig.ResolveGlobalVariables(ctySettings) + if err != nil { + return err + } resPath := filepath.Join(groupPath, res.ID) err = writePackerAutovars(ctySettings, resPath) if err != nil { From fe0cc93f84b1171afc1bcd9160c0b5e19c5c7013 Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Mon, 11 Apr 2022 10:29:00 -0500 Subject: [PATCH 2/2] Update test script to support validation of Packer template syntax --- tools/validate_configs/validate_configs.sh | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tools/validate_configs/validate_configs.sh b/tools/validate_configs/validate_configs.sh index 86879b07e3..d07293bb0d 100755 --- a/tools/validate_configs/validate_configs.sh +++ b/tools/validate_configs/validate_configs.sh @@ -50,6 +50,10 @@ run_test() { } for folder in ./*; do cd "$folder" + pkrdirs=() + while IFS= read -r -d $'\n'; do + pkrdirs+=("$REPLY") + done < <(find . -name "*.pkr.hcl" -printf '%h\n' | sort -u) if [ -f 'main.tf' ]; then tfpw=$(pwd) terraform init -no-color -backend=false >"${exampleFile}.init" || @@ -62,8 +66,16 @@ run_test() { echo "*** ERROR: terraform validate failed for ${example}, logs in ${tfpw}" exit 1 } + elif [ ${#pkrdirs[@]} -gt 0 ]; then + for pkrdir in "${pkrdirs[@]}"; do + packer validate -syntax-only "${pkrdir}" >/dev/null || + { + echo "*** ERROR: packer validate failed for ${example}" + exit 1 + } + done else - echo "terraform not found in folder ${BLUEPRINT}/${folder}. Skipping." + echo "neither packer nor terraform found in folder ${BLUEPRINT}/${folder}. Skipping." fi cd .. # back to blueprint folder done