From 036d199dd63b4ca0f1ad1865c5c8f328a36f5d6b Mon Sep 17 00:00:00 2001 From: Clint Shryock 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 837be6c877de..c66e335b1d67 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 6cb7072b38b6..d5cc6c850295 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"), ), }, },