From 385b17d6797009ff5258417ebd35eab16ce728a3 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 17 Jun 2015 13:58:01 -0500 Subject: [PATCH] provider/template: don't error when rendering fails in Exists The Exists function can run in a context where the contents of the template have changed, but it uses the old set of variables from the state. This means that when the set of variables changes, rendering will fail in Exists. This was returning an error, but really it just needs to be treated as a scenario where the template needs re-rendering. fixes #2344 and possibly a few other template issues floating around --- builtin/providers/template/resource.go | 15 ++++- builtin/providers/template/resource_test.go | 62 ++++++++++++++++++--- helper/resource/testing.go | 8 +++ 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/builtin/providers/template/resource.go b/builtin/providers/template/resource.go index 8eb3ce9eb..9019dcfc9 100644 --- a/builtin/providers/template/resource.go +++ b/builtin/providers/template/resource.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "fmt" "io/ioutil" + "log" "os" "path/filepath" @@ -75,7 +76,13 @@ func Delete(d *schema.ResourceData, meta interface{}) error { func Exists(d *schema.ResourceData, meta interface{}) (bool, error) { rendered, err := render(d) if err != nil { - return false, err + if _, ok := err.(templateRenderError); ok { + log.Printf("[DEBUG] Got error while rendering in Exists: %s", err) + log.Printf("[DEBUG] Returning false so the template re-renders using latest variables from config.") + return false, nil + } else { + return false, err + } } return hash(rendered) == d.Id(), nil } @@ -87,6 +94,8 @@ func Read(d *schema.ResourceData, meta interface{}) error { return nil } +type templateRenderError error + var readfile func(string) ([]byte, error) = ioutil.ReadFile // testing hook func render(d *schema.ResourceData) (string, error) { @@ -105,7 +114,9 @@ func render(d *schema.ResourceData) (string, error) { rendered, err := execute(string(buf), vars) if err != nil { - return "", fmt.Errorf("failed to render %v: %v", filename, err) + return "", templateRenderError( + fmt.Errorf("failed to render %v: %v", filename, err), + ) } return rendered, nil diff --git a/builtin/providers/template/resource_test.go b/builtin/providers/template/resource_test.go index 13b6cf052..7f461325a 100644 --- a/builtin/providers/template/resource_test.go +++ b/builtin/providers/template/resource_test.go @@ -34,15 +34,7 @@ func TestTemplateRendering(t *testing.T) { Providers: testProviders, Steps: []r.TestStep{ r.TestStep{ - Config: ` -resource "template_file" "t0" { - filename = "mock" - vars = ` + tt.vars + ` -} -output "rendered" { - value = "${template_file.t0.rendered}" -} -`, + Config: testTemplateConfig(tt.vars), Check: func(s *terraform.State) error { got := s.RootModule().Outputs["rendered"] if tt.want != got { @@ -55,3 +47,55 @@ output "rendered" { }) } } + +// https://github.com/hashicorp/terraform/issues/2344 +func TestTemplateVariableChange(t *testing.T) { + steps := []struct { + vars string + template string + want string + }{ + {`{a="foo"}`, `${a}`, `foo`}, + {`{b="bar"}`, `${b}`, `bar`}, + } + + var testSteps []r.TestStep + for i, step := range steps { + testSteps = append(testSteps, r.TestStep{ + PreConfig: func(template string) func() { + return func() { + readfile = func(string) ([]byte, error) { + return []byte(template), nil + } + } + }(step.template), + Config: testTemplateConfig(step.vars), + Check: func(i int, want string) r.TestCheckFunc { + return func(s *terraform.State) error { + got := s.RootModule().Outputs["rendered"] + if want != got { + return fmt.Errorf("[%d] got:\n%q\nwant:\n%q\n", i, got, want) + } + return nil + } + }(i, step.want), + }) + } + + r.Test(t, r.TestCase{ + Providers: testProviders, + Steps: testSteps, + }) +} + +func testTemplateConfig(vars string) string { + return ` +resource "template_file" "t0" { + filename = "mock" + vars = ` + vars + ` +} +output "rendered" { + value = "${template_file.t0.rendered}" +} + ` +} diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 6832146b2..3835e8639 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -60,6 +60,10 @@ type TestCase struct { // potentially complex update logic. In general, simply create/destroy // tests will only need one step. type TestStep struct { + // PreConfig is called before the Config is applied to perform any per-step + // setup that needs to happen + PreConfig func() + // Config a string of the configuration to give to Terraform. Config string @@ -160,6 +164,10 @@ func testStep( opts terraform.ContextOpts, state *terraform.State, step TestStep) (*terraform.State, error) { + if step.PreConfig != nil { + step.PreConfig() + } + cfgPath, err := ioutil.TempDir("", "tf-test") if err != nil { return state, fmt.Errorf(