From 53cdbcc9fa933a024c685e8c7d7b984adcd83f5b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2014 11:18:00 -0700 Subject: [PATCH] providers/aws: security group ingress rules can be updated --- CHANGELOG.md | 2 + .../aws/resource_aws_security_group.go | 72 +++++++++++++++ .../aws/resource_aws_security_group_test.go | 91 ++++++++++++++++++- 3 files changed, 164 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e67f9523d..b00e84378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ IMPROVEMENTS: can be used to configure custom providers and provisioners. [GH-192] * providers/aws: EIPs now expose `allocation_id` and `public_ip` attributes. + * providers/aws: Security group rules can be updated without a + destroy/create. BUG FIXES: diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go index 19d221da8..e8ca36107 100644 --- a/builtin/providers/aws/resource_aws_security_group.go +++ b/builtin/providers/aws/resource_aws_security_group.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "reflect" "time" "github.com/hashicorp/terraform/helper/resource" @@ -141,6 +142,77 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er } func resourceAwsSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) error { + p := meta.(*ResourceProvider) + ec2conn := p.ec2conn + + sgRaw, _, err := SGStateRefreshFunc(ec2conn, d.Id())() + if err != nil { + return err + } + if sgRaw == nil { + d.SetId("") + return nil + } + group := sgRaw.(*ec2.SecurityGroupInfo).SecurityGroup + + if d.HasChange("ingress") { + o, n := d.GetChange("ingress") + if o == nil { + o = []interface{}{} + } + if n == nil { + n = []interface{}{} + } + + oldRules := expandIPPerms(o.([]interface{})) + newRules := expandIPPerms(n.([]interface{})) + + var add, remove []ec2.IPPerm + for _, p := range newRules { + // Check if we have had this rule before + exists := false + for _, old := range oldRules { + if reflect.DeepEqual(old, p) { + exists = true + break + } + } + if exists { + continue + } + add = append(add, p) + } + for _, p := range oldRules { + // Check if we have this rule to add + exists := false + for _, n := range newRules { + if reflect.DeepEqual(n, p) { + exists = true + break + } + } + if exists { + continue + } + remove = append(remove, p) + } + + // TODO: We need to handle partial state better in the in-between + // in this update. + + // Authorize the new rules + _, err := ec2conn.AuthorizeSecurityGroup(group, add) + if err != nil { + return fmt.Errorf("Error authorizing security group ingress rules: %s", err) + } + + // Revoke the old rules + _, err = ec2conn.RevokeSecurityGroup(group, remove) + if err != nil { + return fmt.Errorf("Error authorizing security group ingress rules: %s", err) + } + } + return nil } diff --git a/builtin/providers/aws/resource_aws_security_group_test.go b/builtin/providers/aws/resource_aws_security_group_test.go index 8941b7d66..8e86547cf 100644 --- a/builtin/providers/aws/resource_aws_security_group_test.go +++ b/builtin/providers/aws/resource_aws_security_group_test.go @@ -99,12 +99,31 @@ func TestAccAWSSecurityGroup_MultiIngress(t *testing.T) { testAccCheckAWSSecurityGroupExists("aws_security_group.web", &group), ), }, + }, + }) +} + +func TestAccAWSSecurityGroup_Change(t *testing.T) { + var group ec2.SecurityGroupInfo + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSecurityGroupDestroy, + Steps: []resource.TestStep{ resource.TestStep{ - Config: testAccAWSSecurityGroupConfigMultiIngress, + Config: testAccAWSSecurityGroupConfig, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupExists("aws_security_group.web", &group), ), }, + resource.TestStep{ + Config: testAccAWSSecurityGroupConfigChange, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSecurityGroupExists("aws_security_group.web", &group), + testAccCheckAWSSecurityGroupAttributesChanged(&group), + ), + }, }, }) } @@ -210,6 +229,55 @@ func testAccCheckAWSSecurityGroupAttributes(group *ec2.SecurityGroupInfo) resour } } +func testAccCheckAWSSecurityGroupAttributesChanged(group *ec2.SecurityGroupInfo) resource.TestCheckFunc { + return func(s *terraform.State) error { + p := []ec2.IPPerm{ + ec2.IPPerm{ + FromPort: 80, + ToPort: 9000, + Protocol: "tcp", + SourceIPs: []string{"10.0.0.0/8"}, + }, + ec2.IPPerm{ + FromPort: 80, + ToPort: 1234, + Protocol: "tcp", + SourceIPs: []string{"10.0.0.0/8"}, + }, + } + + if group.Name != "terraform_acceptance_test_example" { + return fmt.Errorf("Bad name: %s", group.Name) + } + + if group.Description != "Used in the terraform acceptance tests" { + return fmt.Errorf("Bad description: %s", group.Description) + } + + // Compare our ingress + if len(group.IPPerms) != 2 { + return fmt.Errorf( + "Got:\n\n%#v\n\nExpected:\n\n%#v\n", + group.IPPerms, + p) + } + + if group.IPPerms[0].ToPort == 1234 { + group.IPPerms[1], group.IPPerms[0] = + group.IPPerms[0], group.IPPerms[1] + } + + if !reflect.DeepEqual(group.IPPerms, p) { + return fmt.Errorf( + "Got:\n\n%#v\n\nExpected:\n\n%#v\n", + group.IPPerms, + p) + } + + return nil + } +} + const testAccAWSSecurityGroupConfig = ` resource "aws_security_group" "web" { name = "terraform_acceptance_test_example" @@ -224,6 +292,27 @@ resource "aws_security_group" "web" { } ` +const testAccAWSSecurityGroupConfigChange = ` +resource "aws_security_group" "web" { + name = "terraform_acceptance_test_example" + description = "Used in the terraform acceptance tests" + + ingress { + protocol = "tcp" + from_port = 80 + to_port = 9000 + cidr_blocks = ["10.0.0.0/8"] + } + + ingress { + protocol = "tcp" + from_port = 80 + to_port = 1234 + cidr_blocks = ["10.0.0.0/8"] + } +} +` + const testAccAWSSecurityGroupConfigVpc = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16"