From a34f75fc6a025c53b0858995e36c3260da774df1 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 4 Apr 2017 12:01:13 -0700 Subject: [PATCH 1/2] core: fix crash when computed nested map given in module block This crash resulted because the type switch checked for either of two types but the type assertion within it assumed only one of them. A straightforward (if inelegant) fix is to simply duplicate the relevant case block and change the type assertion, thus allowing the types to match up in all cases. This fixes #13297. --- terraform/eval_variable.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/terraform/eval_variable.go b/terraform/eval_variable.go index 47bd2ea2b67f..09ec7dbf7793 100644 --- a/terraform/eval_variable.go +++ b/terraform/eval_variable.go @@ -174,9 +174,15 @@ func (n *EvalVariableBlock) setUnknownVariableValueForPath(path string) error { // Otherwise find the correct point in the tree and then set to unknown var current interface{} = n.VariableValues[pathComponents[0]] for i := 1; i < len(pathComponents); i++ { - switch current.(type) { - case []interface{}, []map[string]interface{}: - tCurrent := current.([]interface{}) + switch tCurrent := current.(type) { + case []interface{}: + index, err := strconv.Atoi(pathComponents[i]) + if err != nil { + return fmt.Errorf("Cannot convert %s to slice index in path %s", + pathComponents[i], path) + } + current = tCurrent[index] + case []map[string]interface{}: index, err := strconv.Atoi(pathComponents[i]) if err != nil { return fmt.Errorf("Cannot convert %s to slice index in path %s", @@ -184,7 +190,6 @@ func (n *EvalVariableBlock) setUnknownVariableValueForPath(path string) error { } current = tCurrent[index] case map[string]interface{}: - tCurrent := current.(map[string]interface{}) if val, hasVal := tCurrent[pathComponents[i]]; hasVal { current = val continue From df4c0a00c33de9e973d11784fe29ff6a4cd92596 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 4 Apr 2017 11:31:58 -0700 Subject: [PATCH 2/2] core: basic test of EvalVariableBlock This previously lacked tests altogether. This new test verifies the "happy path", ensuring that both literal and computed values pass through correctly into the VariableValues map. --- terraform/eval_variable.go | 1 - terraform/eval_variable_test.go | 77 +++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/terraform/eval_variable.go b/terraform/eval_variable.go index 09ec7dbf7793..b39a4d198fc9 100644 --- a/terraform/eval_variable.go +++ b/terraform/eval_variable.go @@ -114,7 +114,6 @@ type EvalVariableBlock struct { VariableValues map[string]interface{} } -// TODO: test func (n *EvalVariableBlock) Eval(ctx EvalContext) (interface{}, error) { // Clear out the existing mapping for k, _ := range n.VariableValues { diff --git a/terraform/eval_variable_test.go b/terraform/eval_variable_test.go index 05fc2b850cd9..eb7ff92a3116 100644 --- a/terraform/eval_variable_test.go +++ b/terraform/eval_variable_test.go @@ -3,6 +3,8 @@ package terraform import ( "reflect" "testing" + + "github.com/hashicorp/terraform/config" ) func TestCoerceMapVariable(t *testing.T) { @@ -140,3 +142,78 @@ func TestCoerceMapVariable(t *testing.T) { } } } + +func TestEvalVariableBlock(t *testing.T) { + rc, err := config.NewRawConfig(map[string]interface{}{ + "known": "foo", + "known_list": []interface{}{"foo"}, + "known_map": map[string]interface{}{ + "foo": "foo", + }, + "known_list_of_maps": []map[string]interface{}{ + map[string]interface{}{ + "foo": "foo", + }, + }, + "computed_map": map[string]interface{}{}, + "computed_list_of_maps": []map[string]interface{}{ + map[string]interface{}{}, + }, + // No computed_list right now, because that isn't currently supported: + // EvalVariableBlock assumes the final step of the path will always + // be a map. + }) + if err != nil { + t.Fatalf("config.NewRawConfig failed: %s", err) + } + + cfg := NewResourceConfig(rc) + cfg.ComputedKeys = []string{ + "computed", + "computed_map.foo", + "computed_list_of_maps.0.foo", + } + + n := &EvalVariableBlock{ + VariableValues: map[string]interface{}{ + // Should be cleared out on Eval + "should_be_deleted": true, + }, + Config: &cfg, + } + + ctx := &MockEvalContext{} + val, err := n.Eval(ctx) + if err != nil { + t.Fatalf("n.Eval failed: %s", err) + } + if val != nil { + t.Fatalf("n.Eval returned non-nil result: %#v", val) + } + + got := n.VariableValues + want := map[string]interface{}{ + "known": "foo", + "known_list": []interface{}{"foo"}, + "known_map": map[string]interface{}{ + "foo": "foo", + }, + "known_list_of_maps": []interface{}{ + map[string]interface{}{ + "foo": "foo", + }, + }, + "computed": config.UnknownVariableValue, + "computed_map": map[string]interface{}{ + "foo": config.UnknownVariableValue, + }, + "computed_list_of_maps": []interface{}{ + map[string]interface{}{ + "foo": config.UnknownVariableValue, + }, + }, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("Incorrect variables\ngot: %#v\nwant: %#v", got, want) + } +}