Merge branch 'master' into aws-go-route53

* master:
  update cHANGELOG
  helper/schema: zero value of a set should be empty
  helper/schema: GetOk now only returns true if set to non-zero value
  update CHANGELOG
  providers/aws: fix bad arg giving wrong type [GH-992]
  update CHANGELOG
  update CHANGELOG
  Update CHANGELOG
  update CHANGELOG
  providers/aws: test for allowing in-place lC update
  providers/aws: support updating ASG launch config [GH-904]
  helper/schema: GetChange shouldn't return true when no change
  helper/schema: empty map values should show up in diff [GH-968]
  helper/schema: clarify test
  helper/schema: show in diff when no config is going to empty set
  config: bare splat variables should not be allowed in provisioners

CHANGELOG.md

@@ -41,6 +41,7 @@ BUG FIXES:
       lack of a value. [GH-952]
   * core: If a set type becomes empty, the state will be properly updated
       to remove it. [GH-952]
+  * core: Bare "splat" variables are not allowed in provisioners. [GH-636]
   * command/apply: Won't try to initialize modules in some cases when
       no arguments are given. [GH-780]
   * command/apply: Fix regression where user variables weren't asked [GH-736]
@@ -52,7 +53,12 @@ BUG FIXES:
   * provider/aws: Instance should ignore root EBS devices. [GH-877]
   * provider/aws: Fix `aws_db_instance` to not recreate each time. [GH-874]
   * provider/aws: ASG termination policies are synced with remote state. [GH-923]
+  * provider/aws: ASG launch configuration setting can now be updated in-place. [GH-904]
   * provider/aws: No read error when subnet is manually deleted. [GH-889]
+  * provider/aws: Tags with empty values (empty string) are properly
+      managed. [GH-968]
+  * provider/aws: Fix case where route table would delete its routes
+      on an unrelated change. [GH-990]
   * provider/google: Fix bug preventing instances with metadata from being
       created [GH-884].
 
@@ -62,6 +68,9 @@ PLUGIN CHANGES:
   * New `helper/schema` field for resources: `Exists` must point to a function
       to check for the existence of a resource. This is used to properly
       handle the case where the resource was manually deleted. [GH-766]
+  * There is a semantic change in `GetOk` where it will return `true` if
+      there is any value in the diff that is _non-zero_. Before, it would
+      return true only if there was a value in the diff.
 
 ## 0.3.6 (January 6, 2015)
 

builtin/providers/aws/resource_aws_autoscaling_group.go

@@ -29,7 +29,6 @@ func resourceAwsAutoscalingGroup() *schema.Resource {
 			"launch_configuration": &schema.Schema{
 				Type:     schema.TypeString,
 				Required: true,
-				ForceNew: true,
 			},
 
 			"desired_capacity": &schema.Schema{
@@ -150,17 +149,17 @@ func resourceAwsAutoscalingGroupCreate(d *schema.ResourceData, meta interface{})
 		autoScalingGroupOpts.SetHealthCheckGracePeriod = true
 	}
 
-	if v, ok := d.GetOk("load_balancers"); ok {
+	if v, ok := d.GetOk("load_balancers"); ok && v.(*schema.Set).Len() > 0 {
 		autoScalingGroupOpts.LoadBalancerNames = expandStringList(
 			v.(*schema.Set).List())
 	}
 
-	if v, ok := d.GetOk("vpc_zone_identifier"); ok {
+	if v, ok := d.GetOk("vpc_zone_identifier"); ok && v.(*schema.Set).Len() > 0 {
 		autoScalingGroupOpts.VPCZoneIdentifier = expandStringList(
 			v.(*schema.Set).List())
 	}
 
-	if v, ok := d.GetOk("termination_policies"); ok {
+	if v, ok := d.GetOk("termination_policies"); ok && v.(*schema.Set).Len() > 0 {
 		autoScalingGroupOpts.TerminationPolicies = expandStringList(
 			v.(*schema.Set).List())
 	}
@@ -214,6 +213,10 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{})
 		opts.SetDesiredCapacity = true
 	}
 
+	if d.HasChange("launch_configuration") {
+		opts.LaunchConfigurationName = d.Get("launch_configuration").(string)
+	}
+
 	if d.HasChange("min_size") {
 		opts.MinSize = d.Get("min_size").(int)
 		opts.SetMinSize = true

builtin/providers/aws/resource_aws_autoscaling_group_test.go

@@ -11,6 +11,7 @@ import (
 
 func TestAccAWSAutoScalingGroup_basic(t *testing.T) {
 	var group autoscaling.AutoScalingGroup
+	var lc autoscaling.LaunchConfiguration
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -47,8 +48,11 @@ func TestAccAWSAutoScalingGroup_basic(t *testing.T) {
 				Config: testAccAWSAutoScalingGroupConfigUpdate,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSAutoScalingGroupExists("aws_autoscaling_group.bar", &group),
+					testAccCheckAWSLaunchConfigurationExists("aws_launch_configuration.new", &lc),
 					resource.TestCheckResourceAttr(
 						"aws_autoscaling_group.bar", "desired_capacity", "5"),
+					resource.TestCheckResourceAttrPtr(
+						"aws_autoscaling_group.bar", "launch_configuration", &lc.Name),
 				),
 			},
 		},
@@ -217,6 +221,12 @@ resource "aws_launch_configuration" "foobar" {
   instance_type = "t1.micro"
 }
 
+resource "aws_launch_configuration" "new" {
+  name = "foobarautoscaling-terraform-test-new"
+  image_id = "ami-21f78e11"
+  instance_type = "t1.micro"
+}
+
 resource "aws_autoscaling_group" "bar" {
   availability_zones = ["us-west-2a"]
   name = "foobar3-terraform-test"
@@ -227,7 +237,7 @@ resource "aws_autoscaling_group" "bar" {
   desired_capacity = 5
   force_delete = true
 
-  launch_configuration = "${aws_launch_configuration.foobar.name}"
+  launch_configuration = "${aws_launch_configuration.new.name}"
 }
 `
 

builtin/providers/aws/resource_aws_route_table.go

@@ -150,6 +150,9 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error
 			m := route.(map[string]interface{})
 
 			// Delete the route as it no longer exists in the config
+			log.Printf(
+				"[INFO] Deleting route from %s: %s",
+				d.Id(), m["cidr_block"].(string))
 			_, err := ec2conn.DeleteRoute(
 				d.Id(), m["cidr_block"].(string))
 			if err != nil {
@@ -172,6 +175,7 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error
 				InstanceId:           m["instance_id"].(string),
 			}
 
+			log.Printf("[INFO] Creating route for %s: %#v", d.Id(), opts)
 			_, err := ec2conn.CreateRoute(&opts)
 			if err != nil {
 				return err

builtin/providers/aws/resource_aws_security_group.go

@@ -338,13 +338,13 @@ func resourceAwsSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) er
 	}
 	group := sgRaw.(*ec2.SecurityGroupInfo).SecurityGroup
 
-	err = resourceAwsSecurityGroupUpdateRules(d, "ingress", ec2conn, group)
+	err = resourceAwsSecurityGroupUpdateRules(d, "ingress", meta, group)
 	if err != nil {
 		return err
 	}
 
 	if d.Get("vpc_id") != nil {
-		err = resourceAwsSecurityGroupUpdateRules(d, "egress", ec2conn, group)
+		err = resourceAwsSecurityGroupUpdateRules(d, "egress", meta, group)
 		if err != nil {
 			return err
 		}

config/config.go

@@ -465,6 +465,13 @@ func (c *Config) rawConfigs() map[string]*RawConfig {
 		source := fmt.Sprintf("resource '%s'", rc.Id())
 		result[source+" count"] = rc.RawCount
 		result[source+" config"] = rc.RawConfig
+
+		for i, p := range rc.Provisioners {
+			subsource := fmt.Sprintf(
+				"%s provisioner %s (#%d)",
+				source, p.Type, i+1)
+			result[subsource] = p.RawConfig
+		}
 	}
 
 	for _, o := range c.Outputs {

config/config_test.go

@@ -249,6 +249,13 @@ func TestConfigValidate_varMultiNonSlice(t *testing.T) {
 	}
 }
 
+func TestConfigValidate_varMultiNonSliceProvisioner(t *testing.T) {
+	c := testConfig(t, "validate-var-multi-non-slice-provisioner")
+	if err := c.Validate(); err == nil {
+		t.Fatal("should not be valid")
+	}
+}
+
 func TestConfigValidate_varMultiFunctionCall(t *testing.T) {
 	c := testConfig(t, "validate-var-multi-func")
 	if err := c.Validate(); err != nil {

config/test-fixtures/validate-var-multi-non-slice-provisioner/main.tf

@@ -0,0 +1,9 @@
+resource "aws_instance" "foo" {
+    count = 3
+}
+
+resource "aws_instance" "bar" {
+    provisioner "local-exec" {
+        foo = "${aws_instance.foo.*.id}"
+    }
+}

helper/resource/testing.go

@@ -275,6 +275,15 @@ func TestCheckResourceAttr(name, key, value string) TestCheckFunc {
 	}
 }
 
+// TestCheckResourceAttrPtr is like TestCheckResourceAttr except the
+// value is a pointer so that it can be updated while the test is running.
+// It will only be dereferenced at the point this step is run.
+func TestCheckResourceAttrPtr(name string, key string, value *string) TestCheckFunc {
+	return func(s *terraform.State) error {
+		return TestCheckResourceAttr(name, key, *value)(s)
+	}
+}
+
 // TestT is the interface used to handle the test lifecycle of a test.
 //
 // Users should just use a *testing.T object, which implements this.

helper/schema/resource_data.go

@@ -65,18 +65,31 @@ func (d *ResourceData) Get(key string) interface{} {
 // set and the new value is. This is common, for example, for boolean
 // fields which have a zero value of false.
 func (d *ResourceData) GetChange(key string) (interface{}, interface{}) {
-	o, n := d.getChange(key, getSourceState, getSourceDiff|getSourceExact)
+	o, n := d.getChange(key, getSourceState, getSourceDiff)
 	return o.Value, n.Value
 }
 
 // GetOk returns the data for the given key and whether or not the key
-// has been set.
+// has been set to a non-zero value at some point.
 //
 // The first result will not necessarilly be nil if the value doesn't exist.
 // The second result should be checked to determine this information.
 func (d *ResourceData) GetOk(key string) (interface{}, bool) {
 	r := d.getRaw(key, getSourceSet)
-	return r.Value, r.Exists && !r.Computed
+	exists := r.Exists && !r.Computed
+	if exists {
+		// If it exists, we also want to verify it is not the zero-value.
+		value := r.Value
+		zero := r.Schema.Type.Zero()
+
+		if eq, ok := value.(Equal); ok {
+			exists = !eq.Equal(zero)
+		} else {
+			exists = !reflect.DeepEqual(value, zero)
+		}
+	}
+
+	return r.Value, exists
 }
 
 func (d *ResourceData) getRaw(key string, level getSource) getResult {
@@ -361,11 +374,13 @@ func (d *ResourceData) get(addr []string, source getSource) getResult {
 	}
 
 	// If the result doesn't exist, then we set the value to the zero value
-	if result.Value == nil {
-		if schemaL := addrToSchema(addr, d.schema); len(schemaL) > 0 {
-			schema := schemaL[len(schemaL)-1]
-			result.Value = result.ValueOrZero(schema)
-		}
+	var schema *Schema
+	if schemaL := addrToSchema(addr, d.schema); len(schemaL) > 0 {
+		schema = schemaL[len(schemaL)-1]
+	}
+
+	if result.Value == nil && schema != nil {
+		result.Value = result.ValueOrZero(schema)
 	}
 
 	// Transform the FieldReadResult into a getResult. It might be worth
@@ -375,5 +390,6 @@ func (d *ResourceData) get(addr []string, source getSource) getResult {
 		ValueProcessed: result.ValueProcessed,
 		Computed:       result.Computed,
 		Exists:         result.Exists,
+		Schema:         schema,
 	}
 }

helper/schema/resource_data_test.go

@@ -828,7 +828,7 @@ func TestResourceDataGetOk(t *testing.T) {
 
 			Key:   "availability_zone",
 			Value: "",
-			Ok:    true,
+			Ok:    false,
 		},
 
 		{
@@ -961,6 +961,32 @@ func TestResourceDataGetOk(t *testing.T) {
 			Value: 0,
 			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: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"ports.#": &terraform.ResourceAttrDiff{
+						Old: "0",
+						New: "0",
+					},
+				},
+			},
+
+			Key:   "ports",
+			Value: []interface{}{},
+			Ok:    false,
+		},
 	}
 
 	for i, tc := range cases {
@@ -1104,6 +1130,38 @@ func TestResourceDataHasChange(t *testing.T) {
 
 			Change: true,
 		},
+
+		// https://github.com/hashicorp/terraform/issues/927
+		{
+			Schema: map[string]*Schema{
+				"ports": &Schema{
+					Type:     TypeSet,
+					Optional: true,
+					Elem:     &Schema{Type: TypeInt},
+					Set:      func(a interface{}) int { return a.(int) },
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"ports.#":  "1",
+					"ports.80": "80",
+				},
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"tags.foo": &terraform.ResourceAttrDiff{
+						Old: "",
+						New: "bar",
+					},
+				},
+			},
+
+			Key: "ports",
+
+			Change: false,
+		},
 	}
 
 	for i, tc := range cases {