diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index bb18df6377..7c677b7be5 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -836,13 +836,39 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error { groups = append(groups, aws.String(v.(string))) } } - _, err := conn.ModifyInstanceAttribute(&ec2.ModifyInstanceAttributeInput{ - InstanceId: aws.String(d.Id()), - Groups: groups, + // If a user has multiple network interface attachments on the target EC2 instance, simply modifying the + // instance attributes via a `ModifyInstanceAttributes()` request would fail with the following error message: + // "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() { diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go index f4131b09aa..3c23079ee7 100644 --- a/builtin/providers/aws/resource_aws_instance_test.go +++ b/builtin/providers/aws/resource_aws_instance_test.go @@ -1018,6 +1018,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 { @@ -2013,3 +2041,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" + } +} +`