diff --git a/helper/schema/field_reader_config.go b/helper/schema/field_reader_config.go index e7ec0024c..3c01e8de2 100644 --- a/helper/schema/field_reader_config.go +++ b/helper/schema/field_reader_config.go @@ -10,8 +10,10 @@ import ( "github.com/mitchellh/mapstructure" ) -// ConfigFieldReader reads fields out of an untyped map[string]string to -// the best of its ability. +// ConfigFieldReader reads fields out of an untyped map[string]string to the +// best of its ability. It also applies defaults from the Schema. (The other +// field readers do not need default handling because they source fully +// populated data structures.) type ConfigFieldReader struct { Config *terraform.ResourceConfig Schema map[string]*Schema @@ -138,7 +140,16 @@ func (r *ConfigFieldReader) readPrimitive( k string, schema *Schema) (FieldReadResult, error) { raw, ok := r.Config.Get(k) if !ok { - return FieldReadResult{}, nil + // Nothing in config, but we might still have a default from the schema + var err error + raw, err = schema.DefaultValue() + if err != nil { + return FieldReadResult{}, fmt.Errorf("%s, error loading default: %s", k, err) + } + + if raw == nil { + return FieldReadResult{}, nil + } } var result string diff --git a/helper/schema/field_reader_config_test.go b/helper/schema/field_reader_config_test.go index 703b5ebfb..e0a88e737 100644 --- a/helper/schema/field_reader_config_test.go +++ b/helper/schema/field_reader_config_test.go @@ -1,6 +1,7 @@ package schema import ( + "reflect" "testing" "github.com/hashicorp/terraform/config" @@ -56,3 +57,88 @@ func testConfig( return terraform.NewResourceConfig(rc) } + +func TestConfigFieldReader_DefaultHandling(t *testing.T) { + schema := map[string]*Schema{ + "strWithDefault": &Schema{ + Type: TypeString, + Default: "ImADefault", + }, + "strWithDefaultFunc": &Schema{ + Type: TypeString, + DefaultFunc: func() (interface{}, error) { + return "FuncDefault", nil + }, + }, + } + + cases := map[string]struct { + Addr []string + Result FieldReadResult + Config *terraform.ResourceConfig + Err bool + }{ + "gets default value when no config set": { + []string{"strWithDefault"}, + FieldReadResult{ + Value: "ImADefault", + Exists: true, + Computed: false, + }, + testConfig(t, map[string]interface{}{}), + false, + }, + "config overrides default value": { + []string{"strWithDefault"}, + FieldReadResult{ + Value: "fromConfig", + Exists: true, + Computed: false, + }, + testConfig(t, map[string]interface{}{ + "strWithDefault": "fromConfig", + }), + false, + }, + "gets default from function when no config set": { + []string{"strWithDefaultFunc"}, + FieldReadResult{ + Value: "FuncDefault", + Exists: true, + Computed: false, + }, + testConfig(t, map[string]interface{}{}), + false, + }, + "config overrides default function": { + []string{"strWithDefaultFunc"}, + FieldReadResult{ + Value: "fromConfig", + Exists: true, + Computed: false, + }, + testConfig(t, map[string]interface{}{ + "strWithDefaultFunc": "fromConfig", + }), + 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 a list so its more easily checked. + out.Value = s.List() + } + if !reflect.DeepEqual(tc.Result, out) { + t.Fatalf("%s: bad: %#v", name, out) + } + } +} diff --git a/helper/schema/schema.go b/helper/schema/schema.go index b41e826a2..e9441b4e0 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -201,6 +201,24 @@ func (s *Schema) GoString() string { return fmt.Sprintf("*%#v", *s) } +// Returns a default value for this schema by either reading Default or +// evaluating DefaultFunc. If neither of these are defined, returns nil. +func (s *Schema) DefaultValue() (interface{}, error) { + if s.Default != nil { + return s.Default, nil + } + + if s.DefaultFunc != nil { + defaultValue, err := s.DefaultFunc() + if err != nil { + return nil, fmt.Errorf("error loading default: %s", err) + } + return defaultValue, nil + } + + return nil, nil +} + func (s *Schema) finalizeDiff( d *terraform.ResourceAttrDiff) *terraform.ResourceAttrDiff { if d == nil { @@ -373,23 +391,16 @@ func (m schemaMap) Input( continue } - // Skip if it has a default - if v.Default != nil { - continue + // Skip if it has a default value + defaultValue, err := v.DefaultValue() + if err != nil { + return nil, fmt.Errorf("%s: error loading default: %s", k, err) } - if f := v.DefaultFunc; f != nil { - value, err := f() - if err != nil { - return nil, fmt.Errorf( - "%s: error loading default: %s", k, err) - } - if value != nil { - continue - } + if defaultValue != nil { + continue } var value interface{} - var err error switch v.Type { case TypeBool: fallthrough @@ -835,16 +846,6 @@ func (m schemaMap) diffString( var originalN interface{} var os, ns string o, n, _, _ := d.diffChange(k) - if n == nil { - n = schema.Default - if schema.DefaultFunc != nil { - var err error - n, err = schema.DefaultFunc() - if err != nil { - return fmt.Errorf("%s, error loading default: %s", k, err) - } - } - } if schema.StateFunc != nil { originalN = n n = schema.StateFunc(n) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index c96c463b7..80cff8bd6 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -1,12 +1,15 @@ package schema import ( + "bytes" + "fmt" "os" "reflect" "testing" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/lang/ast" + "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/terraform" ) @@ -1886,6 +1889,60 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + + // #47 - https://github.com/hashicorp/terraform/issues/824 + { + Schema: map[string]*Schema{ + "block_device": &Schema{ + Type: TypeSet, + Optional: true, + Computed: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "device_name": &Schema{ + Type: TypeString, + Required: true, + }, + "delete_on_termination": &Schema{ + Type: TypeBool, + Optional: true, + Default: true, + }, + }, + }, + Set: func(v interface{}) int { + var buf bytes.Buffer + m := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%s-", m["device_name"].(string))) + buf.WriteString(fmt.Sprintf("%t-", m["delete_on_termination"].(bool))) + return hashcode.String(buf.String()) + }, + }, + }, + + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "block_device.#": "2", + "block_device.616397234.delete_on_termination": "true", + "block_device.616397234.device_name": "/dev/sda1", + "block_device.2801811477.delete_on_termination": "true", + "block_device.2801811477.device_name": "/dev/sdx", + }, + }, + + Config: map[string]interface{}{ + "block_device": []map[string]interface{}{ + map[string]interface{}{ + "device_name": "/dev/sda1", + }, + map[string]interface{}{ + "device_name": "/dev/sdx", + }, + }, + }, + Diff: nil, + Err: false, + }, } for i, tc := range cases { @@ -1918,7 +1975,7 @@ func TestSchemaMap_Diff(t *testing.T) { } func TestSchemaMap_Input(t *testing.T) { - cases := []struct { + cases := map[string]struct { Schema map[string]*Schema Config map[string]interface{} Input map[string]string @@ -1929,7 +1986,7 @@ func TestSchemaMap_Input(t *testing.T) { * String decode */ - { + "uses input on optional field with no config": { Schema: map[string]*Schema{ "availability_zone": &Schema{ Type: TypeString, @@ -1948,7 +2005,7 @@ func TestSchemaMap_Input(t *testing.T) { Err: false, }, - { + "input ignored when config has a value": { Schema: map[string]*Schema{ "availability_zone": &Schema{ Type: TypeString, @@ -1969,7 +2026,7 @@ func TestSchemaMap_Input(t *testing.T) { Err: false, }, - { + "input ignored when schema has a default": { Schema: map[string]*Schema{ "availability_zone": &Schema{ Type: TypeString, @@ -1987,7 +2044,7 @@ func TestSchemaMap_Input(t *testing.T) { Err: false, }, - { + "input ignored when default function returns a value": { Schema: map[string]*Schema{ "availability_zone": &Schema{ Type: TypeString, @@ -2007,7 +2064,7 @@ func TestSchemaMap_Input(t *testing.T) { Err: false, }, - { + "input used when default function returns nil": { Schema: map[string]*Schema{ "availability_zone": &Schema{ Type: TypeString, @@ -2048,11 +2105,11 @@ func TestSchemaMap_Input(t *testing.T) { actual, err := schemaMap(tc.Schema).Input(input, rc) if (err != nil) != tc.Err { - t.Fatalf("#%d err: %s", i, err) + t.Fatalf("#%v err: %s", i, err) } if !reflect.DeepEqual(tc.Result, actual.Config) { - t.Fatalf("#%d: bad:\n\n%#v", i, actual.Config) + t.Fatalf("#%v: bad:\n\ngot: %#v\nexpected: %#v", i, actual.Config, tc.Result) } } }