Merge pull request #14299 from hashicorp/f-sg-add-issue-3205

provider/aws: Fix SG update on instance with multiple network interfaces

builtin/providers/aws/resource_aws_instance.go

@@ -843,13 +843,39 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error {
 				groups = append(groups, aws.String(v.(string)))
 			}
 		}
-		_, err := conn.ModifyInstanceAttribute(&ec2.ModifyInstanceAttributeInput{
+		// If a user has multiple network interface attachments on the target EC2 instance, simply modifying the
-			InstanceId: aws.String(d.Id()),
+		// instance attributes via a `ModifyInstanceAttributes()` request would fail with the following error message:
-			Groups:     groups,
+		// "There are multiple interfaces attached to instance 'i-XX'. Please specify an interface ID for the operation instead."
+		// Thus, we need to actually modify the primary network interface for the new security groups, as the primary
+		// network interface is where we modify/create security group assignments during Create.
+		log.Printf("[INFO] Modifying `vpc_security_group_ids` on Instance %q", d.Id())
+		instances, err := conn.DescribeInstances(&ec2.DescribeInstancesInput{
+			InstanceIds: []*string{aws.String(d.Id())},
 		})
 		if err != nil {
 			return err
 		}
+		instance := instances.Reservations[0].Instances[0]
+		var primaryInterface ec2.InstanceNetworkInterface
+		for _, ni := range instance.NetworkInterfaces {
+			if *ni.Attachment.DeviceIndex == 0 {
+				primaryInterface = *ni
+			}
+		}
+
+		if primaryInterface.NetworkInterfaceId == nil {
+			log.Print("[Error] Attempted to set vpc_security_group_ids on an instance without a primary network interface")
+			return fmt.Errorf(
+				"Failed to update vpc_security_group_ids on %q, which does not contain a primary network interface",
+				d.Id())
+		}
+
+		if _, err := conn.ModifyNetworkInterfaceAttribute(&ec2.ModifyNetworkInterfaceAttributeInput{
+			NetworkInterfaceId: primaryInterface.NetworkInterfaceId,
+			Groups:             groups,
+		}); err != nil {
+			return err
+		}
 	}
 
 	if d.HasChange("instance_type") && !d.IsNewResource() {

builtin/providers/aws/resource_aws_instance_test.go

@@ -1034,6 +1034,34 @@ func TestAccAWSInstance_addSecondaryInterface(t *testing.T) {
 	})
 }
 
+// https://github.com/hashicorp/terraform/issues/3205
+func TestAccAWSInstance_addSecurityGroupNetworkInterface(t *testing.T) {
+	var before ec2.Instance
+	var after ec2.Instance
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckInstanceDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccInstanceConfigAddSecurityGroupBefore,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInstanceExists("aws_instance.foo", &before),
+					resource.TestCheckResourceAttr("aws_instance.foo", "vpc_security_group_ids.#", "1"),
+				),
+			},
+			{
+				Config: testAccInstanceConfigAddSecurityGroupAfter,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInstanceExists("aws_instance.foo", &after),
+					resource.TestCheckResourceAttr("aws_instance.foo", "vpc_security_group_ids.#", "2"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckInstanceNotRecreated(t *testing.T,
 	before, after *ec2.Instance) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
@@ -2060,3 +2088,134 @@ resource "aws_instance" "foo" {
   }
 }
 `
+
+const testAccInstanceConfigAddSecurityGroupBefore = `
+resource "aws_vpc" "foo" {
+    cidr_block = "172.16.0.0/16"
+        tags {
+            Name = "tf-eni-test"
+        }
+}
+
+resource "aws_subnet" "foo" {
+    vpc_id = "${aws_vpc.foo.id}"
+    cidr_block = "172.16.10.0/24"
+    availability_zone = "us-west-2a"
+        tags {
+            Name = "tf-foo-instance-add-sg-test"
+        }
+}
+
+resource "aws_subnet" "bar" {
+    vpc_id = "${aws_vpc.foo.id}"
+    cidr_block = "172.16.11.0/24"
+    availability_zone = "us-west-2a"
+        tags {
+            Name = "tf-bar-instance-add-sg-test"
+        }
+}
+
+resource "aws_security_group" "foo" {
+  vpc_id = "${aws_vpc.foo.id}"
+  description = "foo"
+  name = "foo"
+}
+
+resource "aws_security_group" "bar" {
+  vpc_id = "${aws_vpc.foo.id}"
+  description = "bar"
+  name = "bar"
+}
+
+resource "aws_instance" "foo" {
+    ami = "ami-c5eabbf5"
+    instance_type = "t2.micro"
+    subnet_id = "${aws_subnet.bar.id}"
+    associate_public_ip_address = false
+    vpc_security_group_ids = [
+      "${aws_security_group.foo.id}"
+    ]
+    tags {
+        Name = "foo-instance-sg-add-test"
+    }
+}
+
+resource "aws_network_interface" "bar" {
+    subnet_id = "${aws_subnet.foo.id}"
+    private_ips = ["172.16.10.100"]
+    security_groups = ["${aws_security_group.foo.id}"]
+    attachment {
+        instance = "${aws_instance.foo.id}"
+        device_index = 1
+    }
+    tags {
+        Name = "bar_interface"
+    }
+}
+`
+
+const testAccInstanceConfigAddSecurityGroupAfter = `
+resource "aws_vpc" "foo" {
+    cidr_block = "172.16.0.0/16"
+        tags {
+            Name = "tf-eni-test"
+        }
+}
+
+resource "aws_subnet" "foo" {
+    vpc_id = "${aws_vpc.foo.id}"
+    cidr_block = "172.16.10.0/24"
+    availability_zone = "us-west-2a"
+        tags {
+            Name = "tf-foo-instance-add-sg-test"
+        }
+}
+
+resource "aws_subnet" "bar" {
+    vpc_id = "${aws_vpc.foo.id}"
+    cidr_block = "172.16.11.0/24"
+    availability_zone = "us-west-2a"
+        tags {
+            Name = "tf-bar-instance-add-sg-test"
+        }
+}
+
+resource "aws_security_group" "foo" {
+  vpc_id = "${aws_vpc.foo.id}"
+  description = "foo"
+  name = "foo"
+}
+
+resource "aws_security_group" "bar" {
+  vpc_id = "${aws_vpc.foo.id}"
+  description = "bar"
+  name = "bar"
+}
+
+resource "aws_instance" "foo" {
+    ami = "ami-c5eabbf5"
+    instance_type = "t2.micro"
+    subnet_id = "${aws_subnet.bar.id}"
+    associate_public_ip_address = false
+    vpc_security_group_ids = [
+      "${aws_security_group.foo.id}",
+      "${aws_security_group.bar.id}"
+    ]
+    tags {
+        Name = "foo-instance-sg-add-test"
+    }
+}
+
+resource "aws_network_interface" "bar" {
+    subnet_id = "${aws_subnet.foo.id}"
+    private_ips = ["172.16.10.100"]
+    security_groups = ["${aws_security_group.foo.id}"]
+    attachment {
+        instance = "${aws_instance.foo.id}"
+        device_index = 1
+    }
+    tags {
+        Name = "bar_interface"
+    }
+}
+`