From 8f6811da0c82ab45035b2e54817045b74b140949 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 16 Nov 2016 18:20:59 -0500 Subject: [PATCH 1/2] Add failing test for GH-10155 Overriding a map variable with the incorrect type panics --- terraform/variables_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/terraform/variables_test.go b/terraform/variables_test.go index 2a97f4edb..c29104e92 100644 --- a/terraform/variables_test.go +++ b/terraform/variables_test.go @@ -136,6 +136,18 @@ func TestVariables(t *testing.T) { "b": "1", }, }, + + "override map with string": { + "vars-basic", + map[string]string{ + "TF_VAR_c": `{"foo" = "a", "bar" = "baz"}`, + }, + map[string]interface{}{ + "c": "bar", + }, + true, + nil, + }, } for name, tc := range cases { From e331f05870143c74da1a52a898e94bdc60a0fd11 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 17 Nov 2016 09:52:01 -0500 Subject: [PATCH 2/2] Return an error for setting a non-map to a map Setting variables happens before context validation, so it's possible that the user could be trying to set an incorrect variable type to a map. Return a useful error rather than panicking. --- terraform/variables.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/terraform/variables.go b/terraform/variables.go index 7fab43370..300f2adb1 100644 --- a/terraform/variables.go +++ b/terraform/variables.go @@ -90,7 +90,9 @@ func Variables( switch varType { case config.VariableTypeMap: - varSetMap(result, k, varVal) + if err := varSetMap(result, k, varVal); err != nil { + return nil, err + } default: result[k] = varVal } @@ -108,7 +110,9 @@ func Variables( case config.VariableTypeList: result[k] = v case config.VariableTypeMap: - varSetMap(result, k, v) + if err := varSetMap(result, k, v); err != nil { + return nil, err + } case config.VariableTypeString: // Convert to a string and set. We don't catch any errors // here because the validation step later should catch @@ -134,16 +138,16 @@ func Variables( // varSetMap sets or merges the map in "v" with the key "k" in the // "current" set of variables. This is just a private function to remove // duplicate logic in Variables -func varSetMap(current map[string]interface{}, k string, v interface{}) { +func varSetMap(current map[string]interface{}, k string, v interface{}) error { existing, ok := current[k] if !ok { current[k] = v - return + return nil } existingMap, ok := existing.(map[string]interface{}) if !ok { - panic(fmt.Sprintf("%s is not a map, this is a bug in Terraform.", k)) + panic(fmt.Sprintf("%q is not a map, this is a bug in Terraform.", k)) } switch typedV := v.(type) { @@ -156,6 +160,7 @@ func varSetMap(current map[string]interface{}, k string, v interface{}) { existingMap[newKey] = newVal } default: - panic(fmt.Sprintf("%s is not a map, this is a bug in Terraform.", k)) + return fmt.Errorf("variable %q should be type map, got %s", k, hclTypeName(v)) } + return nil }