From 473b03ccae3857344f33ae1f1a6adc013d62f40d Mon Sep 17 00:00:00 2001
From: Paul Hinze <paul@hashicorp.com>
Date: Sat, 21 Feb 2015 14:26:46 -0600
Subject: [PATCH] providers/aws: fix source_dest_check on instance creation

The `SourceDestCheck` attribute can only be changed via
`ModifyInstance`, so the AWS instance resource's `Create` function calls
out to `Update` before it returns to take care of applying
`source_dest_check` properly.

The `Update` function originally guarded against unnecessary API calls
with `GetOk`, which worked fine until #993 when we changed the `GetOk`
semantics to no longer distinguish between "configured and zero-value"
and "not configured".

I attempted in #1003 to fix this by switching to `HasChange` for the
guard, but this does not work in the `Create` case.

I played around with a few different ideas, none of which worked:

(a) Setting `Default: true` on `source_dest_check' has no effect

(b) Setting `Computed: true` on `source_dest_check' and adding a `d.Set`
    call in the `Read` function (which will initially set the value to `true`
    after instance creation). I really thought I could get this to work,
    but it results in the following:

```go
d.Get('source_dest_check')       // true
d.HasChange('source_dest_check') // false
d.GetChange('source_dest_check') // old: false, new: false
```

I couldn't figure out a way of coherently dealing with that result, so I
ended up throwing up my hands and giving up on the guard altogether.
We'll call `ModifyInstance` more than we have to, but this at least
yields expected behavior for both Creates and Updates.

Fixes #1020
---
 .../providers/aws/resource_aws_instance.go    | 23 +++++++------------
 .../aws/resource_aws_instance_test.go         | 18 ++++++++++-----
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go
index 789297815..4e32dac2c 100644
--- a/builtin/providers/aws/resource_aws_instance.go
+++ b/builtin/providers/aws/resource_aws_instance.go
@@ -487,25 +487,18 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error {
 
 func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error {
 	ec2conn := meta.(*AWSClient).ec2conn
-
-	modify := false
 	opts := new(ec2.ModifyInstance)
 
-	if d.HasChange("source_dest_check") {
-		opts.SetSourceDestCheck = true
-		opts.SourceDestCheck = d.Get("source_dest_check").(bool)
-		modify = true
+	opts.SetSourceDestCheck = true
+	opts.SourceDestCheck = d.Get("source_dest_check").(bool)
+
+	log.Printf("[INFO] Modifying instance %s: %#v", d.Id(), opts)
+	if _, err := ec2conn.ModifyInstance(d.Id(), opts); err != nil {
+		return err
 	}
 
-	if modify {
-		log.Printf("[INFO] Modifying 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...
-	}
+	// TODO(mitchellh): wait for the attributes we modified to
+	// persist the change...
 
 	if err := setTags(ec2conn, d); err != nil {
 		return err
diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go
index bf59b2a4d..c54751f0e 100644
--- a/builtin/providers/aws/resource_aws_instance_test.go
+++ b/builtin/providers/aws/resource_aws_instance_test.go
@@ -145,10 +145,17 @@ func TestAccAWSInstance_sourceDestCheck(t *testing.T) {
 		CheckDestroy: testAccCheckInstanceDestroy,
 		Steps: []resource.TestStep{
 			resource.TestStep{
-				Config: testAccInstanceConfigSourceDest,
+				Config: testAccInstanceConfigSourceDestDisable,
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckInstanceExists(
-						"aws_instance.foo", &v),
+					testAccCheckInstanceExists("aws_instance.foo", &v),
+					testCheck(false),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccInstanceConfigSourceDestEnable,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInstanceExists("aws_instance.foo", &v),
 					testCheck(true),
 				),
 			},
@@ -156,8 +163,7 @@ func TestAccAWSInstance_sourceDestCheck(t *testing.T) {
 			resource.TestStep{
 				Config: testAccInstanceConfigSourceDestDisable,
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckInstanceExists(
-						"aws_instance.foo", &v),
+					testAccCheckInstanceExists("aws_instance.foo", &v),
 					testCheck(false),
 				),
 			},
@@ -388,7 +394,7 @@ resource "aws_instance" "foo" {
 }
 `
 
-const testAccInstanceConfigSourceDest = `
+const testAccInstanceConfigSourceDestEnable = `
 resource "aws_vpc" "foo" {
 	cidr_block = "10.1.0.0/16"
 }