From 18c3042c47d8e668cba7b8902023b61d769db483 Mon Sep 17 00:00:00 2001 From: Colin Hebert Date: Sat, 7 Mar 2015 17:04:53 +1100 Subject: [PATCH 1/6] Differenciate security groups in VPC and in non VPC env --- .../providers/aws/resource_aws_instance.go | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index ee62d305e703..c7dee37f58fd 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -101,6 +101,16 @@ func resourceAwsInstance() *schema.Resource { }, }, + "vpc_security_groups_ids": &schema.Schema{ + Type: schema.TypeSet, + Optional: true, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: func(v interface{}) int { + return hashcode.String(v.(string)) + }, + }, + "public_dns": &schema.Schema{ Type: schema.TypeString, Computed: true, @@ -282,10 +292,17 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { } if v := d.Get("security_groups"); v != nil { + if runOpts.SubnetId != "" { + 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() { str := v.(string) var g ec2.SecurityGroup + // Deprecated, stop using the subnet ID here if runOpts.SubnetId != "" { g.Id = str } else { @@ -296,6 +313,17 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { } } + if v := d.Get("vpc_security_group_ids"); v != nil { + for _, v := range v.(*schema.Set).List() { + str := v.(string) + + var g ec2.SecurityGroup + g.Id = str + + runOpts.SecurityGroups = append(runOpts.SecurityGroups, g) + } + } + blockDevices := make([]interface{}, 0) if v := d.Get("block_device"); v != nil { @@ -431,7 +459,9 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error { // we use IDs if we're in a VPC. However, if we previously had an // all-name list of security groups, we use names. Or, if we had any // IDs, we use IDs. + // TODO: check the VPC ID instead? useID := instance.SubnetId != "" + // Deprecated: vpc security groups should be defined in vpc_security_group_ids if v := d.Get("security_groups"); v != nil { match := false for _, v := range v.(*schema.Set).List() { @@ -446,14 +476,18 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error { // Build up the security groups sgs := make([]string, len(instance.SecurityGroups)) - for i, sg := range instance.SecurityGroups { - if useID { + + if useID { + for i, sg := range instance.SecurityGroups { sgs[i] = sg.Id - } else { + } + d.Set("vpc_security_group_ids", sgs) + } else { + for i, sg := range instance.SecurityGroups { sgs[i] = sg.Name } + d.Set("security_groups", sgs) } - d.Set("security_groups", sgs) blockDevices := make(map[string]ec2.BlockDevice) for _, bd := range instance.BlockDevices { From 268920126e9206e4388dc59c0552f31bd8a8b277 Mon Sep 17 00:00:00 2001 From: Colin Hebert Date: Sat, 7 Mar 2015 17:09:13 +1100 Subject: [PATCH 2/6] Update documentation to reflect the new vpc_security_group_ids parameter --- website/source/docs/providers/aws/r/instance.html.markdown | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/website/source/docs/providers/aws/r/instance.html.markdown b/website/source/docs/providers/aws/r/instance.html.markdown index 94f042af3fb7..15ea59121ada 100644 --- a/website/source/docs/providers/aws/r/instance.html.markdown +++ b/website/source/docs/providers/aws/r/instance.html.markdown @@ -34,9 +34,9 @@ The following arguments are supported: EBS-optimized. * `instance_type` - (Required) The type of instance to start * `key_name` - (Optional) The key name to use for the instance. -* `security_groups` - (Optional) A list of security group IDs or names to associate with. - If you are within a non-default VPC, you'll need to use the security group ID. Otherwise, - for EC2 and the default VPC, use the security group name. +* `security_groups` - (Optional) A list of security group names to associate with. + If you are within a non-default VPC, you'll need to use `vpc_security_group_ids` instead. +* `vpc_security_group_ids` - (Optional) A list of security group IDs to associate with. * `subnet_id` - (Optional) The VPC Subnet ID to launch in. * `associate_public_ip_address` - (Optional) Associate a public ip address with an instance in a VPC. * `private_ip` - (Optional) Private IP address to associate with the @@ -86,4 +86,5 @@ The following attributes are exported: * `public_dns` - The public DNS name of the instance * `public_ip` - The public IP address. * `security_groups` - The associated security groups. +* `vpc_security_group_ids` - The associated security groups in non-default VPC. * `subnet_id` - The VPC subnet ID. From 9e233a5cd78d9b78053cdee2ed186f65f7aca4c9 Mon Sep 17 00:00:00 2001 From: Colin Hebert Date: Sat, 7 Mar 2015 17:14:04 +1100 Subject: [PATCH 3/6] Fix typo --- builtin/providers/aws/resource_aws_instance.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index c7dee37f58fd..2683181be7bb 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -101,7 +101,7 @@ func resourceAwsInstance() *schema.Resource { }, }, - "vpc_security_groups_ids": &schema.Schema{ + "vpc_security_group_ids": &schema.Schema{ Type: schema.TypeSet, Optional: true, Computed: true, From 2260045dc8b2b2be68b4ae1d156df25f188658e8 Mon Sep 17 00:00:00 2001 From: Colin Hebert Date: Sat, 7 Mar 2015 17:16:59 +1100 Subject: [PATCH 4/6] Improve backward compatibility for now --- builtin/providers/aws/resource_aws_instance.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index 2683181be7bb..37241c68303d 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -459,7 +459,6 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error { // we use IDs if we're in a VPC. However, if we previously had an // all-name list of security groups, we use names. Or, if we had any // IDs, we use IDs. - // TODO: check the VPC ID instead? useID := instance.SubnetId != "" // Deprecated: vpc security groups should be defined in vpc_security_group_ids if v := d.Get("security_groups"); v != nil { @@ -481,7 +480,12 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error { for i, sg := range instance.SecurityGroups { sgs[i] = sg.Id } - d.Set("vpc_security_group_ids", sgs) + // Keep some backward compatibility. The user is warned on creation. + if d.Get("security_groups") != nil { + d.Set("security_groups", sgs) + } else { + d.Set("vpc_security_group_ids", sgs) + } } else { for i, sg := range instance.SecurityGroups { sgs[i] = sg.Name From 675f8ea7b9ac6a04f21be63b3a1942884776e731 Mon Sep 17 00:00:00 2001 From: Colin Hebert Date: Sat, 7 Mar 2015 17:20:51 +1100 Subject: [PATCH 5/6] Print log in one line --- builtin/providers/aws/resource_aws_instance.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index 37241c68303d..f78e0bec126c 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -293,9 +293,7 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { if v := d.Get("security_groups"); v != nil { if runOpts.SubnetId != "" { - log.Printf( - "[WARN] Deprecated. Attempting to use 'security_groups' within a VPC instance. Use 'vpc_security_group_ids' instead." - ) + 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() { From 3337503edaf0562d355f306657270b292af7e55d Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Wed, 15 Apr 2015 12:17:21 -0500 Subject: [PATCH 6/6] update test and documentation for vpc ids in instances --- .../aws/resource_aws_instance_test.go | 64 +++++++++++++++++++ .../providers/aws/r/instance.html.markdown | 2 +- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go index 2dd863c10df4..d03bb7159dff 100644 --- a/builtin/providers/aws/resource_aws_instance_test.go +++ b/builtin/providers/aws/resource_aws_instance_test.go @@ -255,6 +255,25 @@ func TestAccAWSInstance_NetworkInstanceSecurityGroups(t *testing.T) { }) } +func TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs(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: testAccInstanceNetworkInstanceVPCSecurityGroupIDs, + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists( + "aws_instance.foo_instance", &v), + ), + }, + }, + }) +} + func TestAccAWSInstance_tags(t *testing.T) { var v ec2.Instance @@ -645,3 +664,48 @@ resource "aws_eip" "foo_eip" { depends_on = ["aws_internet_gateway.gw"] } ` + +const testAccInstanceNetworkInstanceVPCSecurityGroupIDs = ` +resource "aws_internet_gateway" "gw" { + vpc_id = "${aws_vpc.foo.id}" +} + +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" + tags { + Name = "tf-network-test" + } +} + +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" + vpc_security_group_ids = ["${aws_security_group.tf_test_foo.id}"] + subnet_id = "${aws_subnet.foo.id}" + 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"] +} +` diff --git a/website/source/docs/providers/aws/r/instance.html.markdown b/website/source/docs/providers/aws/r/instance.html.markdown index d3cb333382c0..7a0f441b02ba 100644 --- a/website/source/docs/providers/aws/r/instance.html.markdown +++ b/website/source/docs/providers/aws/r/instance.html.markdown @@ -126,5 +126,5 @@ The following attributes are exported: * `public_dns` - The public DNS name of the instance * `public_ip` - The public IP address. * `security_groups` - The associated security groups. -* `vpc_security_group_ids` - The associated security groups in non-default VPC. +* `vpc_security_group_ids` - The associated security groups in non-default VPC * `subnet_id` - The VPC subnet ID.