From a67182543c63a08b9467eff6112c6c63c28c8fe8 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 15 Aug 2015 17:16:14 -0700 Subject: [PATCH] Nicer error when list/map assigned to string argument. Previous this would return the following sort of error: expected type 'string', got unconvertible type '[]interface {}' This is the raw error returned by the underlying mapstructure library. This is not a helpful error message for anyone who doesn't know Go's type system, and it exposes Terraform's internals to the UI. Instead we'll catch these cases before we try to use mapstructure and return a more straightforward message. By checking the type before the IsComputed exception this also avoids a crash caused when the assigned value is a computed list. Otherwise the list of interpolations is allowed through here and then crashes later during Diff when the value is not a primitive as expected. --- helper/schema/schema.go | 19 ++++++++++++++++++- helper/schema/schema_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index f4d860995..8ed813526 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -1178,8 +1178,25 @@ func (m schemaMap) validatePrimitive( raw interface{}, schema *Schema, c *terraform.ResourceConfig) ([]string, []error) { + + // Catch if the user gave a complex type where a primitive was + // expected, so we can return a friendly error message that + // doesn't contain Go type system terminology. + switch reflect.ValueOf(raw).Type().Kind() { + case reflect.Slice: + return nil, []error{ + fmt.Errorf("%s must be a single value, not a list", k), + } + case reflect.Map: + return nil, []error{ + fmt.Errorf("%s must be a single value, not a map", k), + } + default: // ok + } + if c.IsComputed(k) { - // If the key is being computed, then it is not an error + // If the key is being computed, then it is not an error as + // long as it's not a slice or map. return nil, nil } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 09eeef119..e43300c99 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -3409,6 +3409,36 @@ func TestSchemaMap_Validate(t *testing.T) { Err: true, }, + "Bad, should not allow lists to be assigned to string attributes": { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Required: true, + }, + }, + + Config: map[string]interface{}{ + "availability_zone": []interface{}{"foo", "bar", "baz"}, + }, + + Err: true, + }, + + "Bad, should not allow maps to be assigned to string attributes": { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Required: true, + }, + }, + + Config: map[string]interface{}{ + "availability_zone": map[string]interface{}{"foo": "bar", "baz": "thing"}, + }, + + Err: true, + }, + "Deprecated attribute usage generates warning, but not error": { Schema: map[string]*Schema{ "old_news": &Schema{