From 749db242f4716047fe271f770d1a119ec8989ef1 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 12 Mar 2015 11:04:31 -0500 Subject: [PATCH 1/6] Fix issue with Network interfaces and an instance-level security groups (#1188) --- .../providers/aws/resource_aws_instance.go | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index 5475e74540..28c7b79e7f 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -292,6 +292,17 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { subnet, hasSubnet := d.GetOk("subnet_id") subnetID := subnet.(string) + var groups []string + if v := d.Get("security_groups"); v != nil { + // 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 + for _, v := range v.(*schema.Set).List() { + str := v.(string) + groups = append(groups, str) + } + } + if hasSubnet && associatePublicIPAddress { // If we have a non-default VPC / Subnet specified, we can flag // AssociatePublicIpAddress to get a Public IP assigned. By default these are not provided. @@ -310,6 +321,10 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { ni.PrivateIPAddress = aws.String(v.(string)) } + if len(groups) > 0 { + ni.Groups = groups + } + runOpts.NetworkInterfaces = []ec2.InstanceNetworkInterfaceSpecification{ni} } else { if subnetID != "" { @@ -319,21 +334,6 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { if v, ok := d.GetOk("private_ip"); ok { runOpts.PrivateIPAddress = aws.String(v.(string)) } - } - - if v, ok := d.GetOk("key_name"); ok { - runOpts.KeyName = aws.String(v.(string)) - } - - if v := d.Get("security_groups"); v != nil { - // 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 - var groups []string - for _, v := range v.(*schema.Set).List() { - str := v.(string) - groups = append(groups, str) - } if runOpts.SubnetID != nil && *runOpts.SubnetID != "" { runOpts.SecurityGroupIDs = groups @@ -342,6 +342,10 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { } } + if v, ok := d.GetOk("key_name"); ok { + runOpts.KeyName = aws.String(v.(string)) + } + blockDevices := make([]interface{}, 0) if v := d.Get("block_device"); v != nil { From 3a5918d013739bc798d57ae0bc644047c2d00c09 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 12 Mar 2015 14:26:10 -0500 Subject: [PATCH 2/6] Add acceptance test for Network / Instance security group fix --- .../aws/resource_aws_instance_test.go | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go index 3a9c165880..1076c49e02 100644 --- a/builtin/providers/aws/resource_aws_instance_test.go +++ b/builtin/providers/aws/resource_aws_instance_test.go @@ -207,6 +207,25 @@ func TestAccAWSInstance_vpc(t *testing.T) { }) } +func TestAccInstance_NetworkInstanceSecurityGroups(t *testing.T) { + var v ec2.Instance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccInstanceNetworkInstanceSecurityGroups, + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists( + "aws_instance.foo_instance", &v), + ), + }, + }, + }) +} + func TestAccAWSInstance_tags(t *testing.T) { var v ec2.Instance @@ -533,3 +552,44 @@ resource "aws_instance" "foo" { private_ip = "10.1.1.42" } ` + +const testAccInstanceNetworkInstanceSecurityGroups = ` +resource "aws_internet_gateway" "gw" { + vpc_id = "${aws_vpc.foo.id}" +} + +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" +} + +resource "aws_security_group" "tf_test_foo" { + name = "tf_test_foo" + description = "foo" + vpc_id="${aws_vpc.foo.id}" + + ingress { + protocol = "icmp" + from_port = -1 + to_port = -1 + cidr_blocks = ["0.0.0.0/0"] + } +} + +resource "aws_subnet" "foo" { + cidr_block = "10.1.1.0/24" + vpc_id = "${aws_vpc.foo.id}" +} + +resource "aws_instance" "foo_instance" { + ami = "ami-21f78e11" + instance_type = "t1.micro" + security_groups = ["${aws_security_group.tf_test_foo.id}"] + subnet_id = "${aws_subnet.foo.id}" + associate_public_ip_address = true +} + +resource "aws_eip" "foo_eip" { + instance = "${aws_instance.foo_instance.id}" + vpc = true +} +` From 33fdc0c63f4202a145edf84cb8d0e96f1a3cd50f Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 12 Mar 2015 15:01:24 -0500 Subject: [PATCH 3/6] update the new test config --- builtin/providers/aws/resource_aws_instance_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go index 1076c49e02..f85928cc54 100644 --- a/builtin/providers/aws/resource_aws_instance_test.go +++ b/builtin/providers/aws/resource_aws_instance_test.go @@ -556,10 +556,14 @@ resource "aws_instance" "foo" { const testAccInstanceNetworkInstanceSecurityGroups = ` resource "aws_internet_gateway" "gw" { vpc_id = "${aws_vpc.foo.id}" + depends_on = ["aws_eip.foo_eip"] } resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" + tags { + Name = "tf-network-test" + } } resource "aws_security_group" "tf_test_foo" { From 670d22e18f1f7991475da33546b42bfdceffe0f1 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Fri, 13 Mar 2015 14:58:05 -0500 Subject: [PATCH 4/6] update acceptance test with the correct depends_on declarations --- builtin/providers/aws/resource_aws_instance_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go index e5f460954f..461a5e9261 100644 --- a/builtin/providers/aws/resource_aws_instance_test.go +++ b/builtin/providers/aws/resource_aws_instance_test.go @@ -556,7 +556,6 @@ resource "aws_instance" "foo" { const testAccInstanceNetworkInstanceSecurityGroups = ` resource "aws_internet_gateway" "gw" { vpc_id = "${aws_vpc.foo.id}" - depends_on = ["aws_eip.foo_eip"] } resource "aws_vpc" "foo" { @@ -590,10 +589,12 @@ resource "aws_instance" "foo_instance" { security_groups = ["${aws_security_group.tf_test_foo.id}"] subnet_id = "${aws_subnet.foo.id}" associate_public_ip_address = true + depends_on = ["aws_internet_gateway.gw"] } resource "aws_eip" "foo_eip" { instance = "${aws_instance.foo_instance.id}" vpc = true + depends_on = ["aws_internet_gateway.gw"] } ` From 2d62e55b0edce36d07653cf09257a1835f9ea974 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Fri, 13 Mar 2015 15:39:17 -0500 Subject: [PATCH 5/6] Add note to Intergent Gateway resource on using depends_on --- .../docs/providers/aws/r/internet_gateway.html.markdown | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/website/source/docs/providers/aws/r/internet_gateway.html.markdown b/website/source/docs/providers/aws/r/internet_gateway.html.markdown index ec79f922a4..40fedbb2ae 100644 --- a/website/source/docs/providers/aws/r/internet_gateway.html.markdown +++ b/website/source/docs/providers/aws/r/internet_gateway.html.markdown @@ -29,6 +29,11 @@ The following arguments are supported: * `vpc_id` - (Required) The VPC ID to create in. * `tags` - (Optional) A mapping of tags to assign to the resource. +-> **Note:** When using Internet Gateways with AWS Instances or Elastic IPs, +it is recommended to denote that they depend on the Internet Gateway created, +via the `depends_on` attribute: +`depends_on = ["aws_internet_gateway.gw"]`. + ## Attributes Reference The following attributes are exported: From cd15c9aaf9bbca910a19b22d68a00c04ce2cbc8e Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Fri, 13 Mar 2015 16:04:27 -0500 Subject: [PATCH 6/6] clean up docs --- .../aws/r/internet_gateway.html.markdown | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/website/source/docs/providers/aws/r/internet_gateway.html.markdown b/website/source/docs/providers/aws/r/internet_gateway.html.markdown index 40fedbb2ae..cefedc6ad7 100644 --- a/website/source/docs/providers/aws/r/internet_gateway.html.markdown +++ b/website/source/docs/providers/aws/r/internet_gateway.html.markdown @@ -29,10 +29,17 @@ The following arguments are supported: * `vpc_id` - (Required) The VPC ID to create in. * `tags` - (Optional) A mapping of tags to assign to the resource. --> **Note:** When using Internet Gateways with AWS Instances or Elastic IPs, -it is recommended to denote that they depend on the Internet Gateway created, -via the `depends_on` attribute: -`depends_on = ["aws_internet_gateway.gw"]`. +-> **Note:** It's recommended to denote that the AWS Instance or Elastic IP depends on the Internet Gateway. For example: + + + resource "aws_internet_gateway" "gw" { + vpc_id = "${aws_vpc.main.id}" + } + + resource "aws_instance" "foo" { + depends_on = ["aws_internet_gateway.gw"] + } + ## Attributes Reference