From 036d199dd63b4ca0f1ad1865c5c8f328a36f5d6b Mon Sep 17 00:00:00 2001
From: Clint Shryock <clint@ctshryock.com>
Date: Tue, 21 Apr 2015 17:07:30 -0500
Subject: [PATCH] provider/aws: Fix issue with updating VPC Security Group IDs
 for an Instance

Currently, we weren't correctly setting the ids, and are setting both
`security_groups` and `vpc_security_group_ids`. As a result, we really only use
the former.

We also don't actually update the latter in the `update` method.

This PR fixes both issues, correctly reading `security_groups` vs.
`vpc_security_group_ids` and allows users to update the latter without
destroying the Instance when in a VPC.
---
 .../providers/aws/resource_aws_instance.go    | 36 +++++++++++++++----
 .../aws/resource_aws_instance_test.go         |  4 +++
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go
index 837be6c87..c66e335b1 100644
--- a/builtin/providers/aws/resource_aws_instance.go
+++ b/builtin/providers/aws/resource_aws_instance.go
@@ -358,10 +358,11 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error {
 		// Security group names.
 		// For a nondefault VPC, you must use security group IDs instead.
 		// See http://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RunInstances.html
-		if hasSubnet {
+		sgs := v.(*schema.Set).List()
+		if len(sgs) > 0 && hasSubnet {
 			log.Printf("[WARN] Deprecated. Attempting to use 'security_groups' within a VPC instance. Use 'vpc_security_group_ids' instead.")
 		}
-		for _, v := range v.(*schema.Set).List() {
+		for _, v := range sgs {
 			str := v.(string)
 			groups = append(groups, aws.String(str))
 		}
@@ -620,11 +621,15 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error {
 	// IDs, we use IDs.
 	useID := instance.SubnetID != nil && *instance.SubnetID != ""
 	if v := d.Get("security_groups"); v != nil {
-		match := false
-		for _, v := range v.(*schema.Set).List() {
-			if strings.HasPrefix(v.(string), "sg-") {
-				match = true
-				break
+		match := useID
+		sgs := v.(*schema.Set).List()
+		if len(sgs) > 0 {
+			match = false
+			for _, v := range v.(*schema.Set).List() {
+				if strings.HasPrefix(v.(string), "sg-") {
+					match = true
+					break
+				}
 			}
 		}
 
@@ -677,6 +682,23 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
+	if d.HasChange("vpc_security_group_ids") {
+		var groups []*string
+		if v := d.Get("vpc_security_group_ids"); v != nil {
+			for _, v := range v.(*schema.Set).List() {
+				groups = append(groups, aws.String(v.(string)))
+			}
+		}
+		_, err := conn.ModifyInstanceAttribute(&ec2.ModifyInstanceAttributeInput{
+			InstanceID: aws.String(d.Id()),
+			Groups:     groups,
+		})
+		if err != nil {
+			return err
+		}
+
+	}
+
 	// TODO(mitchellh): wait for the attributes we modified to
 	// persist the change...
 
diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go
index 6cb7072b3..d5cc6c850 100644
--- a/builtin/providers/aws/resource_aws_instance_test.go
+++ b/builtin/providers/aws/resource_aws_instance_test.go
@@ -316,6 +316,10 @@ func TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckInstanceExists(
 						"aws_instance.foo_instance", &v),
+					resource.TestCheckResourceAttr(
+						"aws_instance.foo_instance", "security_groups.#", "0"),
+					resource.TestCheckResourceAttr(
+						"aws_instance.foo_instance", "vpc_security_group_ids.#", "1"),
 				),
 			},
 		},