From 2feaebdca598370ce5646cfc6c3a2ffa02ad1dc3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Feb 2015 21:51:14 -0800 Subject: [PATCH 1/2] config: substring containing computed value replaces element --- config/interpolate_walk.go | 4 +- config/interpolate_walk_test.go | 34 +++++- helper/schema/field_reader_config_test.go | 125 ++++++++++++++++++++-- helper/schema/schema_test.go | 36 +++++++ 4 files changed, 184 insertions(+), 15 deletions(-) diff --git a/config/interpolate_walk.go b/config/interpolate_walk.go index 17329e5a8..3cbb3e26f 100644 --- a/config/interpolate_walk.go +++ b/config/interpolate_walk.go @@ -152,12 +152,12 @@ func (w *interpolationWalker) Primitive(v reflect.Value) error { if w.loc == reflectwalk.SliceElem { parts := strings.Split(replaceVal, InterpSplitDelim) for _, p := range parts { - if p == UnknownVariableValue { + if strings.Contains(p, UnknownVariableValue) { remove = true break } } - } else if replaceVal == UnknownVariableValue { + } else if strings.Contains(replaceVal, UnknownVariableValue) { remove = true } if remove { diff --git a/config/interpolate_walk_test.go b/config/interpolate_walk_test.go index 9b2c34133..ce0671e0a 100644 --- a/config/interpolate_walk_test.go +++ b/config/interpolate_walk_test.go @@ -5,6 +5,7 @@ import ( "reflect" "testing" + "github.com/hashicorp/terraform/config/lang" "github.com/hashicorp/terraform/config/lang/ast" "github.com/mitchellh/reflectwalk" ) @@ -124,7 +125,7 @@ func TestInterpolationWalker_replace(t *testing.T) { "foo": "hello, ${var.foo}", }, Output: map[string]interface{}{ - "foo": "bar", + "foo": "hello, bar", }, Value: "bar", }, @@ -170,11 +171,38 @@ func TestInterpolationWalker_replace(t *testing.T) { Output: map[string]interface{}{}, Value: UnknownVariableValue + InterpSplitDelim + "baz", }, + + { + Input: map[string]interface{}{ + "foo": []interface{}{ + "${var.foo}/32", + "bing", + }, + }, + Output: map[string]interface{}{}, + Value: UnknownVariableValue, + }, } for i, tc := range cases { - fn := func(ast.Node) (string, error) { - return tc.Value, nil + config := &lang.EvalConfig{ + GlobalScope: &ast.BasicScope{ + VarMap: map[string]ast.Variable{ + "var.foo": ast.Variable{ + Value: tc.Value, + Type: ast.TypeString, + }, + }, + }, + } + + fn := func(root ast.Node) (string, error) { + value, _, err := lang.Eval(root, config) + if err != nil { + return "", err + } + + return value.(string), nil } w := &interpolationWalker{F: fn, Replace: true} diff --git a/helper/schema/field_reader_config_test.go b/helper/schema/field_reader_config_test.go index e0a88e737..33672eff2 100644 --- a/helper/schema/field_reader_config_test.go +++ b/helper/schema/field_reader_config_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/config/lang/ast" + "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/terraform" ) @@ -48,16 +50,6 @@ func TestConfigFieldReader(t *testing.T) { }) } -func testConfig( - t *testing.T, raw map[string]interface{}) *terraform.ResourceConfig { - rc, err := config.NewRawConfig(raw) - if err != nil { - t.Fatalf("err: %s", err) - } - - return terraform.NewResourceConfig(rc) -} - func TestConfigFieldReader_DefaultHandling(t *testing.T) { schema := map[string]*Schema{ "strWithDefault": &Schema{ @@ -142,3 +134,116 @@ func TestConfigFieldReader_DefaultHandling(t *testing.T) { } } } + +func TestConfigFieldReader_ComputedSet(t *testing.T) { + schema := map[string]*Schema{ + "strSet": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeString}, + Set: func(v interface{}) int { + return hashcode.String(v.(string)) + }, + }, + } + + cases := map[string]struct { + Addr []string + Result FieldReadResult + Config *terraform.ResourceConfig + Err bool + }{ + "set, normal": { + []string{"strSet"}, + FieldReadResult{ + Value: map[int]interface{}{ + 2356372769: "foo", + }, + Exists: true, + Computed: false, + }, + testConfig(t, map[string]interface{}{ + "strSet": []interface{}{"foo"}, + }), + false, + }, + + "set, computed element": { + []string{"strSet"}, + FieldReadResult{ + Value: nil, + Exists: true, + Computed: true, + }, + testConfigInterpolate(t, map[string]interface{}{ + "strSet": []interface{}{"${var.foo}"}, + }, map[string]ast.Variable{ + "var.foo": ast.Variable{ + Value: config.UnknownVariableValue, + Type: ast.TypeString, + }, + }), + false, + }, + + "set, computed element substring": { + []string{"strSet"}, + FieldReadResult{ + Value: nil, + Exists: true, + Computed: true, + }, + testConfigInterpolate(t, map[string]interface{}{ + "strSet": []interface{}{"${var.foo}/32"}, + }, map[string]ast.Variable{ + "var.foo": ast.Variable{ + Value: config.UnknownVariableValue, + Type: ast.TypeString, + }, + }), + false, + }, + } + + for name, tc := range cases { + r := &ConfigFieldReader{ + Schema: schema, + Config: tc.Config, + } + out, err := r.ReadField(tc.Addr) + if (err != nil) != tc.Err { + t.Fatalf("%s: err: %s", name, err) + } + if s, ok := out.Value.(*Set); ok { + // If it is a set, convert to the raw map + out.Value = s.m + if len(s.m) == 0 { + out.Value = nil + } + } + if !reflect.DeepEqual(tc.Result, out) { + t.Fatalf("%s: bad: %#v", name, out) + } + } +} + +func testConfig( + t *testing.T, raw map[string]interface{}) *terraform.ResourceConfig { + return testConfigInterpolate(t, raw, nil) +} + +func testConfigInterpolate( + t *testing.T, + raw map[string]interface{}, + vs map[string]ast.Variable) *terraform.ResourceConfig { + rc, err := config.NewRawConfig(raw) + if err != nil { + t.Fatalf("err: %s", err) + } + if len(vs) > 0 { + if err := rc.Interpolate(vs); err != nil { + t.Fatalf("err: %s", err) + } + } + + return terraform.NewResourceConfig(rc) +} diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 1e9616552..81f38aa76 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -2165,6 +2165,42 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + + // #56 - Set element computed substring + { + Schema: map[string]*Schema{ + "ports": &Schema{ + Type: TypeSet, + Required: true, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, + }, + + State: nil, + + Config: map[string]interface{}{ + "ports": []interface{}{1, "${var.foo}32"}, + }, + + ConfigVariables: map[string]string{ + "var.foo": config.UnknownVariableValue, + }, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "ports.#": &terraform.ResourceAttrDiff{ + Old: "", + New: "", + NewComputed: true, + }, + }, + }, + + Err: false, + }, } for i, tc := range cases { From 5e25dc54a79363d08c223a118b976159c998f3d5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Feb 2015 22:47:43 -0800 Subject: [PATCH 2/2] config: if any var is computed, the entire interpolation is computed --- config/interpolate_walk.go | 4 ++-- config/interpolate_walk_test.go | 34 +++------------------------------ config/raw_config.go | 23 ++++++++++++++++++++++ config/raw_config_test.go | 33 ++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 33 deletions(-) diff --git a/config/interpolate_walk.go b/config/interpolate_walk.go index 3cbb3e26f..17329e5a8 100644 --- a/config/interpolate_walk.go +++ b/config/interpolate_walk.go @@ -152,12 +152,12 @@ func (w *interpolationWalker) Primitive(v reflect.Value) error { if w.loc == reflectwalk.SliceElem { parts := strings.Split(replaceVal, InterpSplitDelim) for _, p := range parts { - if strings.Contains(p, UnknownVariableValue) { + if p == UnknownVariableValue { remove = true break } } - } else if strings.Contains(replaceVal, UnknownVariableValue) { + } else if replaceVal == UnknownVariableValue { remove = true } if remove { diff --git a/config/interpolate_walk_test.go b/config/interpolate_walk_test.go index ce0671e0a..9b2c34133 100644 --- a/config/interpolate_walk_test.go +++ b/config/interpolate_walk_test.go @@ -5,7 +5,6 @@ import ( "reflect" "testing" - "github.com/hashicorp/terraform/config/lang" "github.com/hashicorp/terraform/config/lang/ast" "github.com/mitchellh/reflectwalk" ) @@ -125,7 +124,7 @@ func TestInterpolationWalker_replace(t *testing.T) { "foo": "hello, ${var.foo}", }, Output: map[string]interface{}{ - "foo": "hello, bar", + "foo": "bar", }, Value: "bar", }, @@ -171,38 +170,11 @@ func TestInterpolationWalker_replace(t *testing.T) { Output: map[string]interface{}{}, Value: UnknownVariableValue + InterpSplitDelim + "baz", }, - - { - Input: map[string]interface{}{ - "foo": []interface{}{ - "${var.foo}/32", - "bing", - }, - }, - Output: map[string]interface{}{}, - Value: UnknownVariableValue, - }, } for i, tc := range cases { - config := &lang.EvalConfig{ - GlobalScope: &ast.BasicScope{ - VarMap: map[string]ast.Variable{ - "var.foo": ast.Variable{ - Value: tc.Value, - Type: ast.TypeString, - }, - }, - }, - } - - fn := func(root ast.Node) (string, error) { - value, _, err := lang.Eval(root, config) - if err != nil { - return "", err - } - - return value.(string), nil + fn := func(ast.Node) (string, error) { + return tc.Value, nil } w := &interpolationWalker{F: fn, Replace: true} diff --git a/config/raw_config.go b/config/raw_config.go index f8fcc2ca7..181a95f61 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -83,6 +83,29 @@ func (r *RawConfig) Config() map[string]interface{} { func (r *RawConfig) Interpolate(vs map[string]ast.Variable) error { config := langEvalConfig(vs) return r.interpolate(func(root ast.Node) (string, error) { + // We detect the variables again and check if the value of any + // of the variables is the computed value. If it is, then we + // treat this entire value as computed. + // + // We have to do this here before the `lang.Eval` because + // if any of the variables it depends on are computed, then + // the interpolation can fail at runtime for other reasons. Example: + // `${count.index+1}`: in a world where `count.index` is computed, + // this would fail a type check since the computed placeholder is + // a string, but realistically the whole value is just computed. + vars, err := DetectVariables(root) + if err != nil { + return "", err + } + for _, v := range vars { + varVal, ok := vs[v.FullKey()] + if ok && varVal.Value == UnknownVariableValue { + return UnknownVariableValue, nil + } + } + + // None of the variables we need are computed, meaning we should + // be able to properly evaluate. out, _, err := lang.Eval(root, config) if err != nil { return "", err diff --git a/config/raw_config_test.go b/config/raw_config_test.go index 324f98ee6..1b5de3e16 100644 --- a/config/raw_config_test.go +++ b/config/raw_config_test.go @@ -238,6 +238,39 @@ func TestRawConfig_unknown(t *testing.T) { } } +func TestRawConfig_unknownPartial(t *testing.T) { + raw := map[string]interface{}{ + "foo": "${var.bar}/32", + } + + rc, err := NewRawConfig(raw) + if err != nil { + t.Fatalf("err: %s", err) + } + + vars := map[string]ast.Variable{ + "var.bar": ast.Variable{ + Value: UnknownVariableValue, + Type: ast.TypeString, + }, + } + if err := rc.Interpolate(vars); err != nil { + t.Fatalf("err: %s", err) + } + + actual := rc.Config() + expected := map[string]interface{}{} + + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad: %#v", actual) + } + + expectedKeys := []string{"foo"} + if !reflect.DeepEqual(rc.UnknownKeys(), expectedKeys) { + t.Fatalf("bad: %#v", rc.UnknownKeys()) + } +} + func TestRawConfigValue(t *testing.T) { raw := map[string]interface{}{ "foo": "${var.bar}",