From 2fd2470c92cf4eb5807ffbc199b87cf686f05ea9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2014 18:38:43 -0700 Subject: [PATCH 01/11] providers/aws: resource_aws_instance partially to schema.Resource --- builtin/providers/aws/provider.go | 1 + .../providers/aws/resource_aws_instance.go | 345 ++++++++++-------- builtin/providers/aws/resources.go | 8 - 3 files changed, 189 insertions(+), 165 deletions(-) diff --git a/builtin/providers/aws/provider.go b/builtin/providers/aws/provider.go index 5efea8640..0a231fdc9 100644 --- a/builtin/providers/aws/provider.go +++ b/builtin/providers/aws/provider.go @@ -17,6 +17,7 @@ func Provider() *schema.Provider { return &schema.Provider{ ResourcesMap: map[string]*schema.Resource{ "aws_eip": resourceAwsEip(), + "aws_instance": resourceAwsInstance(), "aws_security_group": resourceAwsSecurityGroup(), }, } diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index 2df7dea45..ca0a5aef3 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -1,72 +1,159 @@ package aws import ( - "crypto/sha1" - "encoding/hex" "fmt" "log" - "strconv" "strings" "time" - "github.com/hashicorp/terraform/flatmap" - "github.com/hashicorp/terraform/helper/diff" + "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/goamz/ec2" ) -func resource_aws_instance_create( - s *terraform.ResourceState, - d *terraform.ResourceDiff, - meta interface{}) (*terraform.ResourceState, error) { +/* + PreProcess: map[string]diff.PreProcessFunc{ + "user_data": func(v string) string { + hash := sha1.Sum([]byte(v)) + return hex.EncodeToString(hash[:]) + }, + }, + } +*/ + +func resourceAwsInstance() *schema.Resource { + return &schema.Resource{ + Create: resourceAwsInstanceCreate, + Read: resourceAwsInstanceRead, + Update: resourceAwsInstanceUpdate, + Delete: resourceAwsInstanceDelete, + + Schema: map[string]*schema.Schema{ + "ami": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + + "associate_public_ip_address": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + }, + + "availability_zone": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + + "instance_type": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + + "key_name": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + + "subnet_id": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + + "private_ip": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + Computed: true, + }, + + "source_dest_check": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + // TODO: Hidden + }, + + "user_data": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + ForceNew: true, + // TODO: Process + }, + + "security_groups": &schema.Schema{ + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: func(v interface{}) int { + return hashcode.String(v.(string)) + }, + }, + + "public_dns": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + }, + + "public_ip": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + }, + + "private_dns": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + }, + }, + } +} + +func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { p := meta.(*ResourceProvider) ec2conn := p.ec2conn - // Merge the diff into the state so that we have all the attributes - // properly. - rs := s.MergeDiff(d) - delete(rs.Attributes, "source_dest_check") - // Figure out user data userData := "" - if attr, ok := d.Attributes["user_data"]; ok { - userData = attr.NewExtra.(string) + if v := d.Get("user_data"); v != nil { + userData = v.(string) } associatePublicIPAddress := false - if rs.Attributes["associate_public_ip_address"] == "true" { - associatePublicIPAddress = true + if v := d.Get("associate_public_ip_addresss"); v != nil { + associatePublicIPAddress = v.(bool) } // Build the creation struct runOpts := &ec2.RunInstances{ - ImageId: rs.Attributes["ami"], - AvailZone: rs.Attributes["availability_zone"], - InstanceType: rs.Attributes["instance_type"], - KeyName: rs.Attributes["key_name"], - SubnetId: rs.Attributes["subnet_id"], - PrivateIPAddress: rs.Attributes["private_ip"], + ImageId: d.Get("ami").(string), + AvailZone: d.Get("availability_zone").(string), + InstanceType: d.Get("instance_type").(string), + KeyName: d.Get("key_name").(string), + SubnetId: d.Get("subnet_id").(string), + PrivateIPAddress: d.Get("private_ip").(string), AssociatePublicIpAddress: associatePublicIPAddress, UserData: []byte(userData), } - if raw := flatmap.Expand(rs.Attributes, "security_groups"); raw != nil { - if sgs, ok := raw.([]interface{}); ok { - for _, sg := range sgs { - str, ok := sg.(string) - if !ok { - continue - } - var g ec2.SecurityGroup - if runOpts.SubnetId != "" { - g.Id = str - } else { - g.Name = str - } + if v := d.Get("security_groups"); v != nil { + for _, v := range v.([]interface{}) { + str := v.(string) - runOpts.SecurityGroups = append(runOpts.SecurityGroups, g) + var g ec2.SecurityGroup + if runOpts.SubnetId != "" { + g.Id = str + } else { + g.Name = str } + + runOpts.SecurityGroups = append(runOpts.SecurityGroups, g) } } @@ -74,14 +161,14 @@ func resource_aws_instance_create( log.Printf("[DEBUG] Run configuration: %#v", runOpts) runResp, err := ec2conn.RunInstances(runOpts) if err != nil { - return nil, fmt.Errorf("Error launching source instance: %s", err) + return fmt.Errorf("Error launching source instance: %s", err) } instance := &runResp.Instances[0] log.Printf("[INFO] Instance ID: %s", instance.InstanceId) // Store the resulting ID so we can look this up later - rs.ID = instance.InstanceId + d.SetId(instance.InstanceId) // Wait for the instance to become running so we can get some attributes // that aren't available until later. @@ -99,9 +186,8 @@ func resource_aws_instance_create( } instanceRaw, err := stateConf.WaitForState() - if err != nil { - return rs, fmt.Errorf( + return fmt.Errorf( "Error waiting for instance (%s) to become ready: %s", instance.InstanceId, err) } @@ -109,213 +195,158 @@ func resource_aws_instance_create( instance = instanceRaw.(*ec2.Instance) // Initialize the connection info - rs.ConnInfo["type"] = "ssh" - rs.ConnInfo["host"] = instance.PublicIpAddress + /* + TODO: conninfo + rs.ConnInfo["type"] = "ssh" + rs.ConnInfo["host"] = instance.PublicIpAddress + */ // Set our attributes - rs, err = resource_aws_instance_update_state(rs, instance) - if err != nil { - return rs, err + if err := resourceAwsInstanceRead(d, meta); err != nil { + return err } // Update if we need to - return resource_aws_instance_update(rs, d, meta) + return resourceAwsInstanceUpdate(d, meta) } -func resource_aws_instance_update( - s *terraform.ResourceState, - d *terraform.ResourceDiff, - meta interface{}) (*terraform.ResourceState, error) { +func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error { p := meta.(*ResourceProvider) ec2conn := p.ec2conn - rs := s.MergeDiff(d) modify := false opts := new(ec2.ModifyInstance) - if attr, ok := d.Attributes["source_dest_check"]; ok { - modify = true - opts.SourceDestCheck = attr.New != "" && attr.New != "false" + if d.HasChange("source_dest_check") { + opts.SourceDestCheck = d.Get("source_dest_check").(bool) opts.SetSourceDestCheck = true - rs.Attributes["source_dest_check"] = strconv.FormatBool( - opts.SourceDestCheck) } if modify { - log.Printf("[INFO] Modifing instance %s: %#v", s.ID, opts) - if _, err := ec2conn.ModifyInstance(s.ID, opts); err != nil { - return s, err + log.Printf("[INFO] Modifing instance %s: %#v", d.Id(), opts) + if _, err := ec2conn.ModifyInstance(d.Id(), opts); err != nil { + return err } // TODO(mitchellh): wait for the attributes we modified to // persist the change... } - return rs, nil + return nil } -func resource_aws_instance_destroy( - s *terraform.ResourceState, - meta interface{}) error { +func resourceAwsInstanceDelete(d *schema.ResourceData, meta interface{}) error { p := meta.(*ResourceProvider) ec2conn := p.ec2conn - log.Printf("[INFO] Terminating instance: %s", s.ID) - if _, err := ec2conn.TerminateInstances([]string{s.ID}); err != nil { + log.Printf("[INFO] Terminating instance: %s", d.Id()) + if _, err := ec2conn.TerminateInstances([]string{d.Id()}); err != nil { return fmt.Errorf("Error terminating instance: %s", err) } log.Printf( "[DEBUG] Waiting for instance (%s) to become terminated", - s.ID) + d.Id()) stateConf := &resource.StateChangeConf{ Pending: []string{"pending", "running", "shutting-down", "stopped", "stopping"}, Target: "terminated", - Refresh: InstanceStateRefreshFunc(ec2conn, s.ID), + Refresh: InstanceStateRefreshFunc(ec2conn, d.Id()), Timeout: 10 * time.Minute, Delay: 10 * time.Second, MinTimeout: 3 * time.Second, } _, err := stateConf.WaitForState() - if err != nil { return fmt.Errorf( "Error waiting for instance (%s) to terminate: %s", - s.ID, err) + d.Id(), err) } + d.SetId("") return nil } -func resource_aws_instance_diff( - s *terraform.ResourceState, - c *terraform.ResourceConfig, - meta interface{}) (*terraform.ResourceDiff, error) { - b := &diff.ResourceBuilder{ - Attrs: map[string]diff.AttrType{ - "ami": diff.AttrTypeCreate, - "availability_zone": diff.AttrTypeCreate, - "instance_type": diff.AttrTypeCreate, - "key_name": diff.AttrTypeCreate, - "private_ip": diff.AttrTypeCreate, - "security_groups": diff.AttrTypeCreate, - "subnet_id": diff.AttrTypeCreate, - "source_dest_check": diff.AttrTypeUpdate, - "user_data": diff.AttrTypeCreate, - "associate_public_ip_address": diff.AttrTypeCreate, - }, - - ComputedAttrs: []string{ - "availability_zone", - "key_name", - "public_dns", - "public_ip", - "private_dns", - "private_ip", - "security_groups", - "subnet_id", - }, - - PreProcess: map[string]diff.PreProcessFunc{ - "user_data": func(v string) string { - hash := sha1.Sum([]byte(v)) - return hex.EncodeToString(hash[:]) - }, - }, - } - - return b.Diff(s, c) -} - -func resource_aws_instance_refresh( - s *terraform.ResourceState, - meta interface{}) (*terraform.ResourceState, error) { +func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error { p := meta.(*ResourceProvider) ec2conn := p.ec2conn - resp, err := ec2conn.Instances([]string{s.ID}, ec2.NewFilter()) + resp, err := ec2conn.Instances([]string{d.Id()}, ec2.NewFilter()) if err != nil { // If the instance was not found, return nil so that we can show // that the instance is gone. if ec2err, ok := err.(*ec2.Error); ok && ec2err.Code == "InvalidInstanceID.NotFound" { - return nil, nil + d.SetId("") + return nil } // Some other error, report it - return s, err + return err } // If nothing was found, then return no state if len(resp.Reservations) == 0 { - return nil, nil + d.SetId("") + return nil } instance := &resp.Reservations[0].Instances[0] // If the instance is terminated, then it is gone if instance.State.Name == "terminated" { - return nil, nil + d.SetId("") + return nil } - return resource_aws_instance_update_state(s, instance) -} + d.Set("availability_zone", instance.AvailZone) + d.Set("key_name", instance.KeyName) + d.Set("public_dns", instance.DNSName) + d.Set("public_ip", instance.PublicIpAddress) + d.Set("private_dns", instance.PrivateDNSName) + d.Set("private_ip", instance.PrivateIpAddress) + d.Set("subnet_id", instance.SubnetId) -func resource_aws_instance_update_state( - s *terraform.ResourceState, - instance *ec2.Instance) (*terraform.ResourceState, error) { - s.Attributes["availability_zone"] = instance.AvailZone - s.Attributes["key_name"] = instance.KeyName - s.Attributes["public_dns"] = instance.DNSName - s.Attributes["public_ip"] = instance.PublicIpAddress - s.Attributes["private_dns"] = instance.PrivateDNSName - s.Attributes["private_ip"] = instance.PrivateIpAddress - s.Attributes["subnet_id"] = instance.SubnetId - s.Dependencies = nil + var deps []terraform.ResourceDependency - // Extract the existing security groups - useID := false - if raw := flatmap.Expand(s.Attributes, "security_groups"); raw != nil { - if sgs, ok := raw.([]interface{}); ok { - for _, sg := range sgs { - str, ok := sg.(string) - if !ok { - continue - } - - if strings.HasPrefix(str, "sg-") { - useID = true - break - } + // Determine whether we're referring to security groups with + // IDs or names. We use a heuristic to figure this out. By default, + // we use IDs if we're in a VPC. However, if we previously had an + // all-name list of security groups, we use names. Or, if we had any + // IDs, we use IDs. + useID := instance.SubnetId != "" + if v := d.Get("security_groups"); v != nil { + match := false + for _, v := range v.([]interface{}) { + if strings.HasPrefix(v.(string), "sg-") { + match = true + break } } + + useID = match } // Build up the security groups sgs := make([]string, len(instance.SecurityGroups)) for i, sg := range instance.SecurityGroups { - if instance.SubnetId != "" && useID { + if useID { sgs[i] = sg.Id } else { sgs[i] = sg.Name } - s.Dependencies = append(s.Dependencies, - terraform.ResourceDependency{ID: sg.Id}, - ) + deps = append(deps, terraform.ResourceDependency{ID: sg.Id}) } - flatmap.Map(s.Attributes).Merge(flatmap.Flatten(map[string]interface{}{ - "security_groups": sgs, - })) + d.Set("security_groups", sgs) + // If we're in a VPC, we depend on the subnet if instance.SubnetId != "" { - s.Dependencies = append(s.Dependencies, - terraform.ResourceDependency{ID: instance.SubnetId}, - ) + deps = append(deps, terraform.ResourceDependency{ID: instance.SubnetId}) } - return s, nil + d.SetDependencies(deps) + return nil } // InstanceStateRefreshFunc returns a resource.StateRefreshFunc that is used to watch diff --git a/builtin/providers/aws/resources.go b/builtin/providers/aws/resources.go index c43a4ce94..41ddd1d41 100644 --- a/builtin/providers/aws/resources.go +++ b/builtin/providers/aws/resources.go @@ -47,14 +47,6 @@ func init() { Refresh: resource_aws_elb_refresh, }, - "aws_instance": resource.Resource{ - Create: resource_aws_instance_create, - Destroy: resource_aws_instance_destroy, - Diff: resource_aws_instance_diff, - Refresh: resource_aws_instance_refresh, - Update: resource_aws_instance_update, - }, - "aws_internet_gateway": resource.Resource{ Create: resource_aws_internet_gateway_create, Destroy: resource_aws_internet_gateway_destroy, From 3d3789920def6f2b46fcbd1741cb46a2a7ec3e6f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2014 22:15:47 -0700 Subject: [PATCH 02/11] helper/schema: can set conninfo --- helper/schema/resource_data.go | 20 ++++++++++++++++++++ helper/schema/resource_data_test.go | 16 ++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index ccb1183f9..eb8eaaba3 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -91,6 +91,19 @@ func (d *ResourceData) Id() string { return result } +// ConnInfo returns the connection info for this resource. +func (d *ResourceData) ConnInfo() map[string]string { + if d.newState != nil { + return d.newState.ConnInfo + } + + if d.state != nil { + return d.state.ConnInfo + } + + return nil +} + // Dependencies returns the dependencies in this state. func (d *ResourceData) Dependencies() []terraform.ResourceDependency { if d.newState != nil { @@ -111,6 +124,12 @@ func (d *ResourceData) SetId(v string) { d.newState.ID = v } +// SetConnInfo sets the connection info for a resource. +func (d *ResourceData) SetConnInfo(v map[string]string) { + d.once.Do(d.init) + d.newState.ConnInfo = v +} + // SetDependencies sets the dependencies of a resource. func (d *ResourceData) SetDependencies(ds []terraform.ResourceDependency) { d.once.Do(d.init) @@ -123,6 +142,7 @@ func (d *ResourceData) State() *terraform.ResourceState { var result terraform.ResourceState result.ID = d.Id() result.Attributes = d.stateObject("", d.schema) + result.ConnInfo = d.ConnInfo() result.Dependencies = d.Dependencies() if v := d.Id(); v != "" { diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index 072e8d7d8..3fee496c3 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -1531,6 +1531,22 @@ func TestResourceDataState(t *testing.T) { } } +func TestResourceDataSetConnInfo(t *testing.T) { + d := &ResourceData{} + d.SetConnInfo(map[string]string{ + "foo": "bar", + }) + + expected := map[string]string{ + "foo": "bar", + } + + actual := d.State() + if !reflect.DeepEqual(actual.ConnInfo, expected) { + t.Fatalf("bad: %#v", actual) + } +} + func TestResourceDataSetDependencies(t *testing.T) { d := &ResourceData{} d.SetDependencies([]terraform.ResourceDependency{ From 37cf52fa27e0a48bf23925113e2f73e8482abbc7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2014 22:19:33 -0700 Subject: [PATCH 03/11] helper/schema: if no ID is set then return nil --- helper/schema/resource.go | 2 +- helper/schema/resource_data.go | 7 +++++++ helper/schema/resource_data_test.go | 18 +++++++++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/helper/schema/resource.go b/helper/schema/resource.go index ff0cea07f..fcf795186 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -108,7 +108,7 @@ func (r *Resource) Refresh( err = r.Read(data, meta) state := data.State() - if state.ID == "" { + if state != nil && state.ID == "" { state = nil } diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index eb8eaaba3..b81719d18 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -141,6 +141,13 @@ func (d *ResourceData) SetDependencies(ds []terraform.ResourceDependency) { func (d *ResourceData) State() *terraform.ResourceState { var result terraform.ResourceState result.ID = d.Id() + + // If we have no ID, then this resource doesn't exist and we just + // return nil. + if result.ID == "" { + return nil + } + result.Attributes = d.stateObject("", d.schema) result.ConnInfo = d.ConnInfo() result.Dependencies = d.Dependencies() diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index 3fee496c3..82a94da31 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -1524,7 +1524,21 @@ func TestResourceDataState(t *testing.T) { } } + // Set an ID so that the state returned is not nil + idSet := false + if d.Id() == "" { + idSet = true + d.SetId("foo") + } + actual := d.State() + + // If we set an ID, then undo what we did so the comparison works + if actual != nil && idSet { + actual.ID = "" + delete(actual.Attributes, "id") + } + if !reflect.DeepEqual(actual, tc.Result) { t.Fatalf("Bad: %d\n\n%#v", i, actual) } @@ -1533,6 +1547,7 @@ func TestResourceDataState(t *testing.T) { func TestResourceDataSetConnInfo(t *testing.T) { d := &ResourceData{} + d.SetId("foo") d.SetConnInfo(map[string]string{ "foo": "bar", }) @@ -1549,6 +1564,7 @@ func TestResourceDataSetConnInfo(t *testing.T) { func TestResourceDataSetDependencies(t *testing.T) { d := &ResourceData{} + d.SetId("foo") d.SetDependencies([]terraform.ResourceDependency{ terraform.ResourceDependency{ID: "foo"}, }) @@ -1580,7 +1596,7 @@ func TestResourceDataSetId_clear(t *testing.T) { d.SetId("") actual := d.State() - if actual.ID != "" { + if actual != nil { t.Fatalf("bad: %#v", actual) } } From 7be2f1b091d05f6379d9f4bc9fe6871c990a8d49 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2014 23:03:04 -0700 Subject: [PATCH 04/11] helper/schema: add GetOk --- .../providers/aws/resource_aws_instance.go | 10 +- helper/schema/resource_data.go | 162 +++++++++++------ helper/schema/resource_data_test.go | 172 +++++++++++++++++- 3 files changed, 276 insertions(+), 68 deletions(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index ca0a5aef3..0db83a2d3 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -78,7 +78,6 @@ func resourceAwsInstance() *schema.Resource { "source_dest_check": &schema.Schema{ Type: schema.TypeBool, Optional: true, - // TODO: Hidden }, "user_data": &schema.Schema{ @@ -195,11 +194,10 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { instance = instanceRaw.(*ec2.Instance) // Initialize the connection info - /* - TODO: conninfo - rs.ConnInfo["type"] = "ssh" - rs.ConnInfo["host"] = instance.PublicIpAddress - */ + d.SetConnInfo(map[string]string{ + "type": "ssh", + "host": instance.PublicIpAddress, + }) // Set our attributes if err := resourceAwsInstanceRead(d, meta); err != nil { diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index b81719d18..bc8e1c3c1 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -23,6 +23,15 @@ const ( getSourceSet ) +// getResult is the internal structure that is generated when a Get +// is called that contains some extra data that might be used. +type getResult struct { + Value interface{} + Exists bool +} + +var getResultEmpty getResult + // ResourceData is used to query and set the attributes of a resource. type ResourceData struct { schema map[string]*Schema @@ -36,25 +45,38 @@ type ResourceData struct { once sync.Once } -// Get returns the data for the given key, or nil if the key doesn't exist. +// Get returns the data for the given key, or nil if the key doesn't exist +// in the schema. // -// The type of the data returned will be according to the schema specified. -// Primitives will be their respective types in Go, lists will always be -// []interface{}, and sub-resources will be map[string]interface{}. +// If the key does exist in the schema but doesn't exist in the configuration, +// then the default value for that type will be returned. For strings, this is +// "", for numbers it is 0, etc. +// +// If you also want to test if something is set at all, use GetOk. func (d *ResourceData) Get(key string) interface{} { - var parts []string - if key != "" { - parts = strings.Split(key, ".") - } - - return d.getObject("", parts, d.schema, getSourceSet) + v, _ := d.GetOk(key) + return v } // GetChange returns the old and new value for a given key. // // If there is no change, then old and new will simply be the same. func (d *ResourceData) GetChange(key string) (interface{}, interface{}) { - return d.getChange(key, getSourceConfig, getSourceDiff) + o, n := d.getChange(key, getSourceConfig, getSourceDiff) + return o.Value, n.Value +} + +// GetOk returns the data for the given key and whether or not the key +// existed or not in the configuration. The second boolean result will also +// be false if a key is given that isn't in the schema at all. +func (d *ResourceData) GetOk(key string) (interface{}, bool) { + var parts []string + if key != "" { + parts = strings.Split(key, ".") + } + + r := d.getObject("", parts, d.schema, getSourceSet) + return r.Value, r.Exists } // HasChange returns whether or not the given key has been changed. @@ -171,15 +193,21 @@ func (d *ResourceData) init() { func (d *ResourceData) diffChange(k string) (interface{}, interface{}, bool) { // Get the change between the state and the config. o, n := d.getChange(k, getSourceState, getSourceConfig) + if !o.Exists { + o.Value = nil + } + if !n.Exists { + n.Value = nil + } // Return the old, new, and whether there is a change - return o, n, !reflect.DeepEqual(o, n) + return o.Value, n.Value, !reflect.DeepEqual(o.Value, n.Value) } func (d *ResourceData) getChange( key string, oldLevel getSource, - newLevel getSource) (interface{}, interface{}) { + newLevel getSource) (getResult, getResult) { var parts, parts2 []string if key != "" { parts = strings.Split(key, ".") @@ -195,7 +223,7 @@ func (d *ResourceData) get( k string, parts []string, schema *Schema, - source getSource) interface{} { + source getSource) getResult { switch schema.Type { case TypeList: return d.getList(k, parts, schema, source) @@ -218,24 +246,25 @@ func (d *ResourceData) getSet( k string, parts []string, schema *Schema, - source getSource) interface{} { + source getSource) getResult { s := &Set{F: schema.Set} + result := getResult{Value: s} raw := d.getList(k, nil, schema, source) - if raw == nil { + if !raw.Exists { if len(parts) > 0 { return d.getList(k, parts, schema, source) } - return s + return result } - list := raw.([]interface{}) + list := raw.Value.([]interface{}) if len(list) == 0 { if len(parts) > 0 { return d.getList(k, parts, schema, source) } - return s + return result } // This is a reverse map of hash code => index in config used to @@ -269,12 +298,12 @@ func (d *ResourceData) getSet( index := parts[0] indexInt, err := strconv.ParseInt(index, 0, 0) if err != nil { - return nil + return getResultEmpty } codes := s.listCode() if int(indexInt) >= len(codes) { - return nil + return getResultEmpty } code := codes[indexInt] realIndex := indexMap[code] @@ -283,17 +312,19 @@ func (d *ResourceData) getSet( return d.getList(k, parts, schema, source) } - return s + result.Exists = true + return result } func (d *ResourceData) getMap( k string, parts []string, schema *Schema, - source getSource) interface{} { + source getSource) getResult { elemSchema := &Schema{Type: TypeString} result := make(map[string]interface{}) + resultSet := false prefix := k + "." if d.state != nil && source >= getSourceState { @@ -303,7 +334,8 @@ func (d *ResourceData) getMap( } single := k[len(prefix):] - result[single] = d.getPrimitive(k, nil, elemSchema, source) + result[single] = d.getPrimitive(k, nil, elemSchema, source).Value + resultSet = true } } @@ -311,6 +343,7 @@ func (d *ResourceData) getMap( // For config, we always set the result to exactly what was requested if m, ok := d.config.Get(k); ok { result = m.(map[string]interface{}) + resultSet = true } else { result = nil } @@ -321,13 +354,14 @@ func (d *ResourceData) getMap( if !strings.HasPrefix(k, prefix) { continue } + resultSet = true single := k[len(prefix):] if v.NewRemoved { delete(result, single) } else { - result[single] = d.getPrimitive(k, nil, elemSchema, source) + result[single] = d.getPrimitive(k, nil, elemSchema, source).Value } } } @@ -338,6 +372,8 @@ func (d *ResourceData) getMap( if !strings.HasPrefix(k, prefix) { continue } + resultSet = true + if !cleared { // We clear the results if they are in the set map result = make(map[string]interface{}) @@ -345,30 +381,34 @@ func (d *ResourceData) getMap( } single := k[len(prefix):] - result[single] = d.getPrimitive(k, nil, elemSchema, source) + result[single] = d.getPrimitive(k, nil, elemSchema, source).Value } } // If we're requesting a specific element, return that + var resultValue interface{} = result if len(parts) > 0 { - return result[parts[0]] + resultValue = result[parts[0]] } - return result + return getResult{ + Value: resultValue, + Exists: resultSet, + } } func (d *ResourceData) getObject( k string, parts []string, schema map[string]*Schema, - source getSource) interface{} { + source getSource) getResult { if len(parts) > 0 { // We're requesting a specific key in an object key := parts[0] parts = parts[1:] s, ok := schema[key] if !ok { - return nil + return getResultEmpty } if k != "" { @@ -383,17 +423,20 @@ func (d *ResourceData) getObject( // Get the entire object result := make(map[string]interface{}) for field, _ := range schema { - result[field] = d.getObject(k, []string{field}, schema, source) + result[field] = d.getObject(k, []string{field}, schema, source).Value } - return result + return getResult{ + Value: result, + Exists: true, + } } func (d *ResourceData) getList( k string, parts []string, schema *Schema, - source getSource) interface{} { + source getSource) getResult { if len(parts) > 0 { // We still have parts left over meaning we're accessing an // element of this list. @@ -403,12 +446,7 @@ func (d *ResourceData) getList( // Special case if we're accessing the count of the list if idx == "#" { schema := &Schema{Type: TypeInt} - result := d.get(k+".#", parts, schema, source) - if result == nil { - result = 0 - } - - return result + return d.get(k+".#", parts, schema, source) } key := fmt.Sprintf("%s.%s", k, idx) @@ -421,22 +459,24 @@ func (d *ResourceData) getList( } // Get the entire list. - result := make( - []interface{}, - d.getList(k, []string{"#"}, schema, source).(int)) + count := d.getList(k, []string{"#"}, schema, source) + result := make([]interface{}, count.Value.(int)) for i, _ := range result { is := strconv.FormatInt(int64(i), 10) - result[i] = d.getList(k, []string{is}, schema, source) + result[i] = d.getList(k, []string{is}, schema, source).Value } - return result + return getResult{ + Value: result, + Exists: count.Exists, + } } func (d *ResourceData) getPrimitive( k string, parts []string, schema *Schema, - source getSource) interface{} { + source getSource) getResult { var result string var resultSet bool if d.state != nil && source >= getSourceState { @@ -474,13 +514,15 @@ func (d *ResourceData) getPrimitive( } if !resultSet { - return nil + result = "" } + var resultValue interface{} switch schema.Type { case TypeBool: if result == "" { - return false + resultValue = false + break } v, err := strconv.ParseBool(result) @@ -488,13 +530,14 @@ func (d *ResourceData) getPrimitive( panic(err) } - return v + resultValue = v case TypeString: // Use the value as-is. We just put this case here to be explicit. - return result + resultValue = result case TypeInt: if result == "" { - return 0 + resultValue = 0 + break } v, err := strconv.ParseInt(result, 0, 0) @@ -502,10 +545,15 @@ func (d *ResourceData) getPrimitive( panic(err) } - return int(v) + resultValue = int(v) default: panic(fmt.Sprintf("Unknown type: %s", schema.Type)) } + + return getResult{ + Value: resultValue, + Exists: resultSet, + } } func (d *ResourceData) set( @@ -727,10 +775,10 @@ func (d *ResourceData) stateList( prefix string, schema *Schema) map[string]string { countRaw := d.get(prefix, []string{"#"}, schema, getSourceSet) - if countRaw == nil { + if !countRaw.Exists { return nil } - count := countRaw.(int) + count := countRaw.Value.(int) result := make(map[string]string) if count > 0 { @@ -759,13 +807,13 @@ func (d *ResourceData) stateMap( prefix string, schema *Schema) map[string]string { v := d.getMap(prefix, nil, schema, getSourceSet) - if v == nil { + if !v.Exists { return nil } elemSchema := &Schema{Type: TypeString} result := make(map[string]string) - for mk, _ := range v.(map[string]interface{}) { + for mk, _ := range v.Value.(map[string]interface{}) { mp := fmt.Sprintf("%s.%s", prefix, mk) for k, v := range d.stateSingle(mp, elemSchema) { result[k] = v @@ -822,11 +870,11 @@ func (d *ResourceData) stateSet( prefix string, schema *Schema) map[string]string { raw := d.get(prefix, nil, schema, getSourceSet) - if raw == nil { + if !raw.Exists { return nil } - set := raw.(*Set) + set := raw.Value.(*Set) list := set.List() result := make(map[string]string) result[prefix+".#"] = strconv.FormatInt(int64(len(list)), 10) diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index 82a94da31..f701bbc85 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -37,9 +37,8 @@ func TestResourceDataGet(t *testing.T) { }, }, - Key: "availability_zone", - - Value: nil, + Key: "availability_zone", + Value: "", }, { @@ -524,7 +523,7 @@ func TestResourceDataGetChange(t *testing.T) { Key: "availability_zone", - OldValue: nil, + OldValue: "", NewValue: "foo", }, @@ -577,6 +576,169 @@ func TestResourceDataGetChange(t *testing.T) { } } +func TestResourceDataGetOk(t *testing.T) { + cases := []struct { + Schema map[string]*Schema + State *terraform.ResourceState + Diff *terraform.ResourceDiff + Key string + Value interface{} + Ok bool + }{ + /* + * Primitives + */ + { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + }, + + State: nil, + + Diff: &terraform.ResourceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "availability_zone": &terraform.ResourceAttrDiff{ + Old: "", + New: "", + }, + }, + }, + + Key: "availability_zone", + Value: "", + Ok: true, + }, + + { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + }, + + State: nil, + + Diff: nil, + + Key: "availability_zone", + Value: "", + Ok: false, + }, + + /* + * Lists + */ + + { + Schema: map[string]*Schema{ + "ports": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeInt}, + }, + }, + + State: nil, + + Diff: nil, + + Key: "ports", + Value: []interface{}{}, + Ok: false, + }, + + /* + * Map + */ + + { + Schema: map[string]*Schema{ + "ports": &Schema{ + Type: TypeMap, + Optional: true, + }, + }, + + State: nil, + + Diff: nil, + + Key: "ports", + Value: map[string]interface{}{}, + Ok: false, + }, + + /* + * Set + */ + + { + Schema: map[string]*Schema{ + "ports": &Schema{ + Type: TypeSet, + Optional: true, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { return a.(int) }, + }, + }, + + State: nil, + + Diff: nil, + + Key: "ports", + Value: []interface{}{}, + Ok: false, + }, + + { + Schema: map[string]*Schema{ + "ports": &Schema{ + Type: TypeSet, + Optional: true, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { return a.(int) }, + }, + }, + + State: nil, + + Diff: nil, + + Key: "ports.0", + Value: 0, + Ok: false, + }, + } + + for i, tc := range cases { + d, err := schemaMap(tc.Schema).Data(tc.State, tc.Diff) + if err != nil { + t.Fatalf("err: %s", err) + } + + v, ok := d.GetOk(tc.Key) + if s, ok := v.(*Set); ok { + v = s.List() + } + + if !reflect.DeepEqual(v, tc.Value) { + t.Fatalf("Bad: %d\n\n%#v", i, v) + } + if ok != tc.Ok { + t.Fatalf("Bad: %d\n\n%#v", i, ok) + } + } +} + func TestResourceDataHasChange(t *testing.T) { cases := []struct { Schema map[string]*Schema @@ -771,7 +933,7 @@ func TestResourceDataSet(t *testing.T) { Err: true, GetKey: "availability_zone", - GetValue: nil, + GetValue: "", }, // List of primitives, set element From d009ea800afce9ba948e8b625f47ab3e9705ce25 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 22 Aug 2014 08:45:54 -0700 Subject: [PATCH 05/11] helper/schema: add support for StateFunc --- helper/schema/resource_data.go | 13 +++++++++++- helper/schema/resource_data_test.go | 29 +++++++++++++++++++++++++++ helper/schema/schema.go | 16 ++++++++++++--- helper/schema/schema_test.go | 31 +++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 4 deletions(-) diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index bc8e1c3c1..e716e288f 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -28,6 +28,7 @@ const ( type getResult struct { Value interface{} Exists bool + Schema *Schema } var getResultEmpty getResult @@ -200,6 +201,10 @@ func (d *ResourceData) diffChange(k string) (interface{}, interface{}, bool) { n.Value = nil } + if n.Exists && n.Schema.StateFunc != nil { + n.Value = n.Schema.StateFunc(n.Value) + } + // Return the old, new, and whether there is a change return o.Value, n.Value, !reflect.DeepEqual(o.Value, n.Value) } @@ -248,7 +253,7 @@ func (d *ResourceData) getSet( schema *Schema, source getSource) getResult { s := &Set{F: schema.Set} - result := getResult{Value: s} + result := getResult{Schema: schema, Value: s} raw := d.getList(k, nil, schema, source) if !raw.Exists { if len(parts) > 0 { @@ -394,6 +399,7 @@ func (d *ResourceData) getMap( return getResult{ Value: resultValue, Exists: resultSet, + Schema: schema, } } @@ -429,6 +435,9 @@ func (d *ResourceData) getObject( return getResult{ Value: result, Exists: true, + Schema: &Schema{ + Elem: schema, + }, } } @@ -469,6 +478,7 @@ func (d *ResourceData) getList( return getResult{ Value: result, Exists: count.Exists, + Schema: schema, } } @@ -553,6 +563,7 @@ func (d *ResourceData) getPrimitive( return getResult{ Value: resultValue, Exists: resultSet, + Schema: schema, } } diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index f701bbc85..ba07eca50 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -1430,6 +1430,35 @@ func TestResourceDataState(t *testing.T) { }, }, + // Basic primitive with StateFunc set + { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + StateFunc: func(interface{}) string { return "" }, + }, + }, + + State: nil, + + Diff: &terraform.ResourceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "availability_zone": &terraform.ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, + + Result: &terraform.ResourceState{ + Attributes: map[string]string{ + "availability_zone": "foo", + }, + }, + }, + // List { Schema: map[string]*Schema{ diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 0b3383263..23f5c34fa 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -41,8 +41,14 @@ type Schema struct { // // If ForceNew is true, then a change in this resource necessitates // the creation of a new resource. - Computed bool - ForceNew bool + // + // StateFunc is a function called to change the value of this before + // storing it in the state (and likewise before comparing for diffs). + // The use for this is for example with large strings, you may want + // to simply store the hash of it. + Computed bool + ForceNew bool + StateFunc SchemaStateFunc // The following fields are only set for a TypeList or TypeSet Type. // @@ -66,7 +72,11 @@ type Schema struct { // SchemaSetFunc is a function that must return a unique ID for the given // element. This unique ID is used to store the element in a hash. -type SchemaSetFunc func(a interface{}) int +type SchemaSetFunc func(interface{}) int + +// SchemaStateFunc is a function used to convert some type to a string +// to be stored in the state. +type SchemaStateFunc func(interface{}) string func (s *Schema) finalizeDiff( d *terraform.ResourceAttrDiff) *terraform.ResourceAttrDiff { diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index ee06514fe..b457d4f89 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -76,6 +76,37 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + // String with StateFunc + { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + StateFunc: func(a interface{}) string { + return a.(string) + "!" + }, + }, + }, + + State: nil, + + Config: map[string]interface{}{ + "availability_zone": "foo", + }, + + Diff: &terraform.ResourceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "availability_zone": &terraform.ResourceAttrDiff{ + Old: "", + New: "foo!", + }, + }, + }, + + Err: false, + }, + /* * Int decode */ From f26a2700a132ed1cd8c2d0bae604ed148c7672d9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 22 Aug 2014 08:46:03 -0700 Subject: [PATCH 06/11] fmt --- builtin/providers/aws/resource_aws_security_group.go | 2 +- config/raw_config.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go index bb3a8cfff..b160dfddd 100644 --- a/builtin/providers/aws/resource_aws_security_group.go +++ b/builtin/providers/aws/resource_aws_security_group.go @@ -3,8 +3,8 @@ package aws import ( "bytes" "fmt" - "sort" "log" + "sort" "time" "github.com/hashicorp/terraform/helper/hashcode" diff --git a/config/raw_config.go b/config/raw_config.go index d10a16010..2674476a2 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -24,9 +24,9 @@ const UnknownVariableValue = "74D93920-ED26-11E3-AC10-0800200C9A66" // RawConfig supports a query-like interface to request // information from deep within the structure. type RawConfig struct { - Raw map[string]interface{} + Raw map[string]interface{} Interpolations []Interpolation - Variables map[string]InterpolatedVariable + Variables map[string]InterpolatedVariable config map[string]interface{} unknownKeys []string From ba68be5672814d0f0ad502164183b28bc638c2ba Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 22 Aug 2014 08:46:48 -0700 Subject: [PATCH 07/11] providers/aws: use StateFunc to process the user_data --- builtin/providers/aws/resource_aws_instance.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index 0db83a2d3..8cf300bdc 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -1,6 +1,8 @@ package aws import ( + "crypto/sha1" + "encoding/hex" "fmt" "log" "strings" @@ -84,7 +86,10 @@ func resourceAwsInstance() *schema.Resource { Type: schema.TypeString, Optional: true, ForceNew: true, - // TODO: Process + StateFunc: func(v interface{}) string { + hash := sha1.Sum([]byte(v.(string))) + return hex.EncodeToString(hash[:]) + }, }, "security_groups": &schema.Schema{ From 50026a6d5cc62a73ddd26da6a1619db6af7a1e64 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 22 Aug 2014 08:57:44 -0700 Subject: [PATCH 08/11] helper/schema: When having a StateFunc, make sure NewExtra contains original --- helper/schema/resource_data.go | 11 +++++++---- helper/schema/resource_data_test.go | 26 ++++++++++++++++++++++++++ helper/schema/schema.go | 6 ++++++ helper/schema/schema_test.go | 5 +++-- 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index e716e288f..f79d1b4a3 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -201,10 +201,6 @@ func (d *ResourceData) diffChange(k string) (interface{}, interface{}, bool) { n.Value = nil } - if n.Exists && n.Schema.StateFunc != nil { - n.Value = n.Schema.StateFunc(n.Value) - } - // Return the old, new, and whether there is a change return o.Value, n.Value, !reflect.DeepEqual(o.Value, n.Value) } @@ -512,6 +508,13 @@ func (d *ResourceData) getPrimitive( attrD, ok := d.diff.Attributes[k] if ok && !attrD.NewComputed { result = attrD.New + if attrD.NewExtra != nil { + err := mapstructure.WeakDecode(attrD.NewExtra, &result) + if err != nil { + panic(err) + } + } + resultSet = true } } diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index ba07eca50..9fbb4c9d2 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -68,6 +68,32 @@ func TestResourceDataGet(t *testing.T) { Value: "foo", }, + { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + }, + + State: nil, + + Diff: &terraform.ResourceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "availability_zone": &terraform.ResourceAttrDiff{ + Old: "", + New: "foo!", + NewExtra: "foo", + }, + }, + }, + + Key: "availability_zone", + Value: "foo", + }, + { Schema: map[string]*Schema{ "availability_zone": &Schema{ diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 23f5c34fa..515b080d2 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -386,8 +386,13 @@ func (m schemaMap) diffString( schema *Schema, diff *terraform.ResourceDiff, d *ResourceData) error { + var originalN interface{} var os, ns string o, n, _ := d.diffChange(k) + if schema.StateFunc != nil { + originalN = n + n = schema.StateFunc(n) + } if err := mapstructure.WeakDecode(o, &os); err != nil { return fmt.Errorf("%s: %s", k, err) } @@ -411,6 +416,7 @@ func (m schemaMap) diffString( diff.Attributes[k] = schema.finalizeDiff(&terraform.ResourceAttrDiff{ Old: os, New: ns, + NewExtra: originalN, NewRemoved: removed, }) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index b457d4f89..ac1f827e5 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -98,8 +98,9 @@ func TestSchemaMap_Diff(t *testing.T) { Diff: &terraform.ResourceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ "availability_zone": &terraform.ResourceAttrDiff{ - Old: "", - New: "foo!", + Old: "", + New: "foo!", + NewExtra: "foo", }, }, }, From 9ed601d5419ad67e3e1936f746700396c9a27820 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 22 Aug 2014 12:09:06 -0700 Subject: [PATCH 09/11] helper/schema: store state with processed properly --- helper/schema/resource_data.go | 36 +++++++++++++++++++++-------- helper/schema/resource_data_test.go | 11 +++++---- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index f79d1b4a3..a841e3ba4 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -26,9 +26,10 @@ const ( // getResult is the internal structure that is generated when a Get // is called that contains some extra data that might be used. type getResult struct { - Value interface{} - Exists bool - Schema *Schema + Value interface{} + ValueProcessed interface{} + Exists bool + Schema *Schema } var getResultEmpty getResult @@ -71,13 +72,17 @@ func (d *ResourceData) GetChange(key string) (interface{}, interface{}) { // existed or not in the configuration. The second boolean result will also // be false if a key is given that isn't in the schema at all. func (d *ResourceData) GetOk(key string) (interface{}, bool) { + r := d.getRaw(key) + return r.Value, r.Exists +} + +func (d *ResourceData) getRaw(key string) getResult { var parts []string if key != "" { parts = strings.Split(key, ".") } - r := d.getObject("", parts, d.schema, getSourceSet) - return r.Value, r.Exists + return d.getObject("", parts, d.schema, getSourceSet) } // HasChange returns whether or not the given key has been changed. @@ -484,6 +489,7 @@ func (d *ResourceData) getPrimitive( schema *Schema, source getSource) getResult { var result string + var resultProcessed interface{} var resultSet bool if d.state != nil && source >= getSourceState { result, resultSet = d.state.Attributes[k] @@ -509,6 +515,10 @@ func (d *ResourceData) getPrimitive( if ok && !attrD.NewComputed { result = attrD.New if attrD.NewExtra != nil { + // If NewExtra != nil, then we have processed data as the New, + // so we store that but decode the unprocessed data into result + resultProcessed = result + err := mapstructure.WeakDecode(attrD.NewExtra, &result) if err != nil { panic(err) @@ -564,9 +574,10 @@ func (d *ResourceData) getPrimitive( } return getResult{ - Value: resultValue, - Exists: resultSet, - Schema: schema, + Value: resultValue, + ValueProcessed: resultProcessed, + Exists: resultSet, + Schema: schema, } } @@ -858,11 +869,16 @@ func (d *ResourceData) stateObject( func (d *ResourceData) statePrimitive( prefix string, schema *Schema) map[string]string { - v := d.Get(prefix) - if v == nil { + raw := d.getRaw(prefix) + if !raw.Exists { return nil } + v := raw.Value + if raw.ValueProcessed != nil { + v = raw.ValueProcessed + } + var vs string switch schema.Type { case TypeBool: diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index 9fbb4c9d2..f1773ed06 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -83,9 +83,9 @@ func TestResourceDataGet(t *testing.T) { Diff: &terraform.ResourceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ "availability_zone": &terraform.ResourceAttrDiff{ - Old: "", - New: "foo!", - NewExtra: "foo", + Old: "", + New: "foo!", + NewExtra: "foo", }, }, }, @@ -1472,8 +1472,9 @@ func TestResourceDataState(t *testing.T) { Diff: &terraform.ResourceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ "availability_zone": &terraform.ResourceAttrDiff{ - Old: "", - New: "foo", + Old: "", + New: "foo", + NewExtra: "foo!", }, }, }, From eff8306a6ce1fecfc6b0cad43cd4e1039f2094e5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 22 Aug 2014 12:18:08 -0700 Subject: [PATCH 10/11] helper/schema: don't mark things computed if an ID is set --- helper/schema/schema.go | 11 ++++++++--- helper/schema/schema_test.go | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 515b080d2..3e2d6f429 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -401,9 +401,14 @@ func (m schemaMap) diffString( } if os == ns { - // They're the same value, return no diff as long as we're not - // computing a new value. - if os != "" || !schema.Computed { + // They're the same value. If there old value is not blank or we + // have an ID, then return right away since we're already setup. + if os != "" || d.Id() != "" { + return nil + } + + // Otherwise, only continue if we're computed + if !schema.Computed { return nil } } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index ac1f827e5..0b6b9ef0d 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -76,6 +76,27 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + { + Schema: map[string]*Schema{ + "availability_zone": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + }, + + State: &terraform.ResourceState{ + ID: "foo", + }, + + Config: map[string]interface{}{}, + + Diff: nil, + + Err: false, + }, + // String with StateFunc { Schema: map[string]*Schema{ From 8f70920c1729fd5028550281280680403e435827 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 22 Aug 2014 12:20:06 -0700 Subject: [PATCH 11/11] providers/aws: fix some other issues with aws_instance --- builtin/providers/aws/resource_aws_instance.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index 8cf300bdc..d911ad975 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -60,8 +60,9 @@ func resourceAwsInstance() *schema.Resource { "key_name": &schema.Schema{ Type: schema.TypeString, - Required: true, + Optional: true, ForceNew: true, + Computed: true, }, "subnet_id": &schema.Schema{ @@ -147,7 +148,7 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { } if v := d.Get("security_groups"); v != nil { - for _, v := range v.([]interface{}) { + for _, v := range v.(*schema.Set).List() { str := v.(string) var g ec2.SecurityGroup @@ -320,7 +321,7 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error { useID := instance.SubnetId != "" if v := d.Get("security_groups"); v != nil { match := false - for _, v := range v.([]interface{}) { + for _, v := range v.(*schema.Set).List() { if strings.HasPrefix(v.(string), "sg-") { match = true break