From 1a9adc29b3c74d90c2ee70d2626180ab5837d196 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 7 Jun 2019 15:56:20 -0700 Subject: [PATCH 1/3] fixing interpolation fix sensitive_vars test which never worked but somehow was passing before this change. --- packer/core.go | 59 ++++++++------ packer/core_test.go | 81 +++++++++++++++++-- .../build-basic-interpolated-required.json | 9 +++ .../build-var-file-interpolate.json | 3 + .../build-var-file-interpolate2.json | 3 + ....json => build-variables-interpolate.json} | 0 ...json => build-variables-interpolate2.json} | 0 7 files changed, 124 insertions(+), 31 deletions(-) create mode 100644 packer/test-fixtures/build-basic-interpolated-required.json create mode 100644 packer/test-fixtures/build-var-file-interpolate.json create mode 100644 packer/test-fixtures/build-var-file-interpolate2.json rename packer/test-fixtures/{variables.json => build-variables-interpolate.json} (100%) rename packer/test-fixtures/{variables2.json => build-variables-interpolate2.json} (100%) diff --git a/packer/core.go b/packer/core.go index 0b3670371..d9b12fcb5 100644 --- a/packer/core.go +++ b/packer/core.go @@ -2,6 +2,7 @@ package packer import ( "fmt" + "log" "sort" ttmp "text/template" @@ -306,7 +307,6 @@ func (c *Core) init() error { if c.variables == nil { c.variables = make(map[string]string) } - // Go through the variables and interpolate the environment and // user variables @@ -314,7 +314,6 @@ func (c *Core) init() error { ctx.EnableEnv = true ctx.UserVariables = make(map[string]string) shouldRetry := true - tryCount := 0 changed := false failedInterpolation := "" @@ -337,32 +336,43 @@ func (c *Core) init() error { // interpolating them. Please don't actually nest your variables in 100 // layers of other variables. Please. - for shouldRetry == true { - shouldRetry = false - for k, v := range c.Template.Variables { - // Ignore variables that are required - if v.Required { - continue - } + // c.Template.Variables is populated by variables defined within the Template + // itself + // c.variables is populated by variables read in from the command line and + // var-files. + // We need to read the keys from both, then loop over all of them to figure + // out the appropriate interpolations. + + allVariables := make(map[string]string) + // load in template variables + log.Printf("\n\n\n\nMegan template.Variables is %#v", c.Template.Variables) + for k, v := range c.Template.Variables { + allVariables[k] = v.Default + } - // Ignore variables that have a value already - if _, ok := c.variables[k]; ok { - continue - } + // overwrite template variables with command-line-read variables + log.Printf("Megan c.variables is %#v", c.variables) + for k, v := range c.variables { + allVariables[k] = v + } + for i := 0; i < 100; i++ { + shouldRetry = false + // First, loop over the variables in the template + for k, v := range allVariables { // Interpolate the default - def, err := interpolate.Render(v.Default, ctx) + renderedV, err := interpolate.Render(v, ctx) + log.Printf("Megan k is %s, renderedV is %s and err is %s", k, renderedV, err) switch err.(type) { case nil: // We only get here if interpolation has succeeded, so something is // different in this loop than in the last one. changed = true - c.variables[k] = def + c.variables[k] = renderedV ctx.UserVariables = c.variables case ttmp.ExecError: shouldRetry = true - tryCount++ - failedInterpolation = fmt.Sprintf(`"%s": "%s"`, k, v.Default) + failedInterpolation = fmt.Sprintf(`"%s": "%s"`, k, v) continue default: return fmt.Errorf( @@ -371,10 +381,9 @@ func (c *Core) init() error { k, err) } } - if tryCount >= 100 { + if !shouldRetry { break } - } if (changed == false) && (shouldRetry == true) { @@ -384,20 +393,18 @@ func (c *Core) init() error { "required.", failedInterpolation) } + log.Printf("Megan rendering sensitive variables now...") for _, v := range c.Template.SensitiveVariables { - def, err := interpolate.Render(v.Default, ctx) - if err != nil { - return fmt.Errorf( - "error interpolating default value for '%#v': %s", - v, err) - } - c.secrets = append(c.secrets, def) + // log.Printf("k is %#v, v is %#v", k, v) + secret := ctx.UserVariables[v.Key] + c.secrets = append(c.secrets, secret) } // Interpolate the push configuration if _, err := interpolate.RenderInterface(&c.Template.Push, c.Context()); err != nil { return fmt.Errorf("Error interpolating 'push': %s", err) } + log.Printf("Megan ctx.UserVariables is %#v", ctx.UserVariables) return nil } diff --git a/packer/core_test.go b/packer/core_test.go index dfe6b29b7..e1ffa5b59 100644 --- a/packer/core_test.go +++ b/packer/core_test.go @@ -524,6 +524,8 @@ func TestCoreValidate(t *testing.T) { } } +// Tests that we can properly interpolate user variables defined within the +// packer template func TestCore_InterpolateUserVars(t *testing.T) { cases := []struct { File string @@ -531,7 +533,7 @@ func TestCore_InterpolateUserVars(t *testing.T) { Err bool }{ { - "variables.json", + "build-variables-interpolate.json", map[string]string{ "foo": "bar", "bar": "bar", @@ -541,7 +543,7 @@ func TestCore_InterpolateUserVars(t *testing.T) { false, }, { - "variables2.json", + "build-variables-interpolate2.json", map[string]string{}, true, }, @@ -576,6 +578,74 @@ func TestCore_InterpolateUserVars(t *testing.T) { } } +// Tests that we can properly interpolate user variables defined within a +// var-file provided alongside the Packer template +func TestCore_InterpolateUserVars_VarFile(t *testing.T) { + cases := []struct { + File string + Variables map[string]string + Expected map[string]string + Err bool + }{ + { + // tests that we can interpolate from var files when var isn't set in + // originating template + "build-basic-interpolated.json", + map[string]string{ + "name": "gotta-{{user `my_var`}}", + "my_var": "interpolate-em-all", + }, + map[string]string{ + "name": "gotta-interpolate-em-all", + "my_var": "interpolate-em-all"}, + false, + }, + { + // tests that we can interpolate from var files when var is set in + // originating template as required + "build-basic-interpolated-required.json", + map[string]string{ + "name": "gotta-{{user `my_var`}}", + "my_var": "interpolate-em-all", + }, + map[string]string{ + "name": "gotta-interpolate-em-all", + "my_var": "interpolate-em-all"}, + false, + }, + } + for _, tc := range cases { + f, err := os.Open(fixtureDir(tc.File)) + if err != nil { + t.Fatalf("err: %s", err) + } + + tpl, err := template.Parse(f) + f.Close() + if err != nil { + t.Fatalf("err: %s\n\n%s", tc.File, err) + } + + ccf, err := NewCore(&CoreConfig{ + Template: tpl, + Version: "1.0.0", + Variables: tc.Variables, + }) + + if (err != nil) != tc.Err { + t.Fatalf("err: %s\n\n%s", tc.File, err) + } + if !tc.Err { + for k, v := range ccf.variables { + if tc.Expected[k] != v { + t.Fatalf("Expected value %s for key %s but got %s", + tc.Expected[k], k, v) + } + } + } + } +} + func TestSensitiveVars(t *testing.T) { cases := []struct { File string @@ -595,9 +665,10 @@ func TestSensitiveVars(t *testing.T) { // interpolated { "sensitive-variables.json", - map[string]string{"foo": "{{build_name}}"}, - []string{"foo"}, - "test", + map[string]string{"foo": "bar", + "bang": "{{ user `foo`}}"}, + []string{"bang"}, + "bar", false, }, } diff --git a/packer/test-fixtures/build-basic-interpolated-required.json b/packer/test-fixtures/build-basic-interpolated-required.json new file mode 100644 index 000000000..2cdd3df63 --- /dev/null +++ b/packer/test-fixtures/build-basic-interpolated-required.json @@ -0,0 +1,9 @@ +{ + "variables": { + "name": null + }, + "builders": [{ + "name": "{{upper `name`}}", + "type": "test" + }] +} diff --git a/packer/test-fixtures/build-var-file-interpolate.json b/packer/test-fixtures/build-var-file-interpolate.json new file mode 100644 index 000000000..d2f8f5225 --- /dev/null +++ b/packer/test-fixtures/build-var-file-interpolate.json @@ -0,0 +1,3 @@ +{ + "name": "{{ user `my_var` }}" +} \ No newline at end of file diff --git a/packer/test-fixtures/build-var-file-interpolate2.json b/packer/test-fixtures/build-var-file-interpolate2.json new file mode 100644 index 000000000..c7a4c9635 --- /dev/null +++ b/packer/test-fixtures/build-var-file-interpolate2.json @@ -0,0 +1,3 @@ +{ + "my_var": "interpolate this, yall" +} \ No newline at end of file diff --git a/packer/test-fixtures/variables.json b/packer/test-fixtures/build-variables-interpolate.json similarity index 100% rename from packer/test-fixtures/variables.json rename to packer/test-fixtures/build-variables-interpolate.json diff --git a/packer/test-fixtures/variables2.json b/packer/test-fixtures/build-variables-interpolate2.json similarity index 100% rename from packer/test-fixtures/variables2.json rename to packer/test-fixtures/build-variables-interpolate2.json From ca99cbd2d224d57e2926cf4e1bd70598db9af896 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 10 Jun 2019 11:34:57 -0700 Subject: [PATCH 2/3] remove loglines --- packer/core.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packer/core.go b/packer/core.go index d9b12fcb5..d4d004649 100644 --- a/packer/core.go +++ b/packer/core.go @@ -2,7 +2,6 @@ package packer import ( "fmt" - "log" "sort" ttmp "text/template" @@ -345,13 +344,11 @@ func (c *Core) init() error { allVariables := make(map[string]string) // load in template variables - log.Printf("\n\n\n\nMegan template.Variables is %#v", c.Template.Variables) for k, v := range c.Template.Variables { allVariables[k] = v.Default } // overwrite template variables with command-line-read variables - log.Printf("Megan c.variables is %#v", c.variables) for k, v := range c.variables { allVariables[k] = v } @@ -362,7 +359,6 @@ func (c *Core) init() error { for k, v := range allVariables { // Interpolate the default renderedV, err := interpolate.Render(v, ctx) - log.Printf("Megan k is %s, renderedV is %s and err is %s", k, renderedV, err) switch err.(type) { case nil: // We only get here if interpolation has succeeded, so something is @@ -393,7 +389,6 @@ func (c *Core) init() error { "required.", failedInterpolation) } - log.Printf("Megan rendering sensitive variables now...") for _, v := range c.Template.SensitiveVariables { // log.Printf("k is %#v, v is %#v", k, v) secret := ctx.UserVariables[v.Key] @@ -404,7 +399,6 @@ func (c *Core) init() error { if _, err := interpolate.RenderInterface(&c.Template.Push, c.Context()); err != nil { return fmt.Errorf("Error interpolating 'push': %s", err) } - log.Printf("Megan ctx.UserVariables is %#v", ctx.UserVariables) return nil } From ddb4d77dc86b1305d8e1f0f01c9c4653b18b3c03 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Tue, 11 Jun 2019 11:09:22 +0200 Subject: [PATCH 3/3] Update packer/core.go remove commented log line --- packer/core.go | 1 - 1 file changed, 1 deletion(-) diff --git a/packer/core.go b/packer/core.go index d4d004649..4d01a846c 100644 --- a/packer/core.go +++ b/packer/core.go @@ -390,7 +390,6 @@ func (c *Core) init() error { } for _, v := range c.Template.SensitiveVariables { - // log.Printf("k is %#v, v is %#v", k, v) secret := ctx.UserVariables[v.Key] c.secrets = append(c.secrets, secret) }