From 76dca009e0f048ea35e86f3737e7f63503378789 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 29 Mar 2017 08:34:45 -0700 Subject: [PATCH] Allow escaped interpolation-like sequences in variable defaults The variable validator assumes that any AST node it gets from an interpolation walk is an indicator of an interpolation. Unfortunately, back in f223be15 we changed the interpolation walker to emit a LiteralNode as a way to signal that the result is a literal but not identical to the input due to escapes. The existence of this issue suggests a bit of a design smell in that the interpolation walker interface at first glance appears to skip over all literals, but it actually emits them in this one situation. In the long run we should perhaps think about whether the abstraction is right here, but this is a shallow, tactical change that fixes #13001. --- command/test-fixtures/validate-valid/main.tf | 5 +++++ config/config.go | 11 +++++++++-- config/config_test.go | 7 +++++++ .../validate-var-default-interpolate-escaped/main.tf | 5 +++++ 4 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 config/test-fixtures/validate-var-default-interpolate-escaped/main.tf diff --git a/command/test-fixtures/validate-valid/main.tf b/command/test-fixtures/validate-valid/main.tf index fd9da13e0..2dcb1eccd 100644 --- a/command/test-fixtures/validate-valid/main.tf +++ b/command/test-fixtures/validate-valid/main.tf @@ -1,3 +1,8 @@ +variable "var_with_escaped_interp" { + # This is here because in the past it failed. See Github #13001 + default = "foo-$${bar.baz}" +} + resource "test_instance" "foo" { ami = "bar" diff --git a/config/config.go b/config/config.go index bf064e57a..f944cadd3 100644 --- a/config/config.go +++ b/config/config.go @@ -285,8 +285,15 @@ func (c *Config) Validate() error { } interp := false - fn := func(ast.Node) (interface{}, error) { - interp = true + fn := func(n ast.Node) (interface{}, error) { + // LiteralNode is a literal string (outside of a ${ ... } sequence). + // interpolationWalker skips most of these. but in particular it + // visits those that have escaped sequences (like $${foo}) as a + // signal that *some* processing is required on this string. For + // our purposes here though, this is fine and not an interpolation. + if _, ok := n.(*ast.LiteralNode); !ok { + interp = true + } return "", nil } diff --git a/config/config_test.go b/config/config_test.go index b391295c8..68a93d9b6 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -572,6 +572,13 @@ func TestConfigValidate_varDefaultInterpolate(t *testing.T) { } } +func TestConfigValidate_varDefaultInterpolateEscaped(t *testing.T) { + c := testConfig(t, "validate-var-default-interpolate-escaped") + if err := c.Validate(); err != nil { + t.Fatalf("should be valid, but got err: %s", err) + } +} + func TestConfigValidate_varDup(t *testing.T) { c := testConfig(t, "validate-var-dup") if err := c.Validate(); err == nil { diff --git a/config/test-fixtures/validate-var-default-interpolate-escaped/main.tf b/config/test-fixtures/validate-var-default-interpolate-escaped/main.tf new file mode 100644 index 000000000..da2758f6a --- /dev/null +++ b/config/test-fixtures/validate-var-default-interpolate-escaped/main.tf @@ -0,0 +1,5 @@ +variable "foo" { + # This should be considered valid since the sequence is escaped and is + # thus not actually an interpolation. + default = "foo bar $${aws_instance.foo.bar}" +}