From dbc890e4019b5bc8dbd457fe603edb829d439aa0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 30 Sep 2014 14:19:16 -0700 Subject: [PATCH] providers/aws: add `self` to ingress [GH-219] --- CHANGELOG.md | 4 +- .../aws/resource_aws_security_group.go | 26 +++----- .../aws/resource_aws_security_group_test.go | 63 +++++++++++++++++++ builtin/providers/aws/structure.go | 17 +++-- builtin/providers/aws/structure_test.go | 44 +++++++++---- .../aws/r/security_group.html.markdown | 2 + 6 files changed, 120 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 048297476..01af98894 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,9 @@ IMPROVEMENTS: * providers/aws: New resource `db_subnet_group`. [GH-295] * providers/aws: Add `map_public_ip_on_launch` for subnets. [GH-285] * providers/aws: Add `iam_instance_profile` for instances. [GH-319] + * providers/aws: add `internal` option for ELBs. [GH-303] + * providers/aws: add `self` option for security groups for ingress + rules with self as source. [GH-303] * providers/google: Support `target_tags` for firewalls. [GH-324] BUG FIXES: @@ -34,7 +37,6 @@ BUG FIXES: * core: Plugin loading from CWD works properly. * providers/aws: autoscaling_group can be launched into a vpc [GH-259] * providers/aws: not an error when RDS instance is deleted manually. [GH-307] - * providers/aws: add `internal` option for ELBs. [GH-303] ## 0.2.2 (September 9, 2014) diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go index 9e787d80f..1f627500c 100644 --- a/builtin/providers/aws/resource_aws_security_group.go +++ b/builtin/providers/aws/resource_aws_security_group.go @@ -69,6 +69,11 @@ func resourceAwsSecurityGroup() *schema.Resource { Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, }, + + "self": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, }, }, Set: resourceAwsSecurityGroupIngressHash, @@ -143,7 +148,6 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er } d.SetId(createResp.Id) - group := createResp.SecurityGroup log.Printf("[INFO] Security Group ID: %s", d.Id()) @@ -163,21 +167,7 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er d.Id(), err) } - // Expand the "ingress" array to goamz compat []ec2.IPPerm - ingressRaw := d.Get("ingress") - if ingressRaw == nil { - ingressRaw = new(schema.Set) - } - ingressList := ingressRaw.(*schema.Set).List() - if len(ingressList) > 0 { - ingressRules := expandIPPerms(ingressList) - _, err = ec2conn.AuthorizeSecurityGroup(group, ingressRules) - if err != nil { - return fmt.Errorf("Error authorizing security group ingress rules: %s", err) - } - } - - return resourceAwsSecurityGroupRead(d, meta) + return resourceAwsSecurityGroupUpdate(d, meta) } func resourceAwsSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) error { @@ -206,8 +196,8 @@ func resourceAwsSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) er os := o.(*schema.Set) ns := n.(*schema.Set) - remove := expandIPPerms(os.Difference(ns).List()) - add := expandIPPerms(ns.Difference(os).List()) + remove := expandIPPerms(d.Id(), os.Difference(ns).List()) + add := expandIPPerms(d.Id(), ns.Difference(os).List()) // TODO: We need to handle partial state better in the in-between // in this update. diff --git a/builtin/providers/aws/resource_aws_security_group_test.go b/builtin/providers/aws/resource_aws_security_group_test.go index abd66593a..0774b71dd 100644 --- a/builtin/providers/aws/resource_aws_security_group_test.go +++ b/builtin/providers/aws/resource_aws_security_group_test.go @@ -43,6 +43,51 @@ func TestAccAWSSecurityGroup_normal(t *testing.T) { }) } +func TestAccAWSSecurityGroup_self(t *testing.T) { + var group ec2.SecurityGroupInfo + + checkSelf := func(s *terraform.State) (err error) { + defer func() { + if e := recover(); e != nil { + err = fmt.Errorf("bad: %#v", group) + } + }() + + if group.IPPerms[0].SourceGroups[0].Id != group.Id { + return fmt.Errorf("bad: %#v", group) + } + + return nil + } + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSecurityGroupDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSSecurityGroupConfigSelf, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSecurityGroupExists("aws_security_group.web", &group), + resource.TestCheckResourceAttr( + "aws_security_group.web", "name", "terraform_acceptance_test_example"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "description", "Used in the terraform acceptance tests"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.0.protocol", "tcp"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.0.from_port", "80"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.0.to_port", "8000"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.0.self", "true"), + checkSelf, + ), + }, + }, + }) +} + func TestAccAWSSecurityGroup_vpc(t *testing.T) { var group ec2.SecurityGroupInfo @@ -215,6 +260,10 @@ func testAccCheckAWSSecurityGroupAttributes(group *ec2.SecurityGroupInfo) resour return fmt.Errorf("Bad description: %s", group.Description) } + if len(group.IPPerms) == 0 { + return fmt.Errorf("No IPPerms") + } + // Compare our ingress if !reflect.DeepEqual(group.IPPerms[0], p) { return fmt.Errorf( @@ -311,6 +360,20 @@ resource "aws_security_group" "web" { } ` +const testAccAWSSecurityGroupConfigSelf = ` +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 = 8000 + self = true + } +} +` + const testAccAWSSecurityGroupConfigVpc = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" diff --git a/builtin/providers/aws/structure.go b/builtin/providers/aws/structure.go index 0cc196a94..fee36600a 100644 --- a/builtin/providers/aws/structure.go +++ b/builtin/providers/aws/structure.go @@ -41,7 +41,7 @@ func expandListeners(configured []interface{}) ([]elb.Listener, error) { // Takes the result of flatmap.Expand for an array of ingress/egress // security group rules and returns EC2 API compatible objects -func expandIPPerms(configured []interface{}) []ec2.IPPerm { +func expandIPPerms(id string, configured []interface{}) []ec2.IPPerm { perms := make([]ec2.IPPerm, len(configured)) for i, mRaw := range configured { var perm ec2.IPPerm @@ -51,11 +51,20 @@ func expandIPPerms(configured []interface{}) []ec2.IPPerm { perm.ToPort = m["to_port"].(int) perm.Protocol = m["protocol"].(string) + var groups []string if raw, ok := m["security_groups"]; ok { list := raw.([]interface{}) - perm.SourceGroups = make([]ec2.UserSecurityGroup, len(list)) - for i, v := range list { - name := v.(string) + for _, v := range list { + groups = append(groups, v.(string)) + } + } + if v, ok := m["self"]; ok && v.(bool) { + groups = append(groups, id) + } + + if len(groups) > 0 { + perm.SourceGroups = make([]ec2.UserSecurityGroup, len(groups)) + for i, name := range groups { ownerId, id := "", name if items := strings.Split(id, "/"); len(items) > 1 { ownerId, id = items[0], items[1] diff --git a/builtin/providers/aws/structure_test.go b/builtin/providers/aws/structure_test.go index 2d3cbb47f..81e67a9b6 100644 --- a/builtin/providers/aws/structure_test.go +++ b/builtin/providers/aws/structure_test.go @@ -44,26 +44,44 @@ func Test_expandIPPerms(t *testing.T) { "foo/sg-22222", }, }, + map[string]interface{}{ + "protocol": "icmp", + "from_port": 1, + "to_port": -1, + "self": true, + }, } - perms := expandIPPerms(expanded) + perms := expandIPPerms("foo", expanded) - expected := ec2.IPPerm{ - Protocol: "icmp", - FromPort: 1, - ToPort: -1, - SourceIPs: []string{"0.0.0.0/0"}, - SourceGroups: []ec2.UserSecurityGroup{ - ec2.UserSecurityGroup{ - Id: "sg-11111", + expected := []ec2.IPPerm{ + ec2.IPPerm{ + Protocol: "icmp", + FromPort: 1, + ToPort: -1, + SourceIPs: []string{"0.0.0.0/0"}, + SourceGroups: []ec2.UserSecurityGroup{ + ec2.UserSecurityGroup{ + Id: "sg-11111", + }, + ec2.UserSecurityGroup{ + OwnerId: "foo", + Id: "sg-22222", + }, }, - ec2.UserSecurityGroup{ - OwnerId: "foo", - Id: "sg-22222", + }, + ec2.IPPerm{ + Protocol: "icmp", + FromPort: 1, + ToPort: -1, + SourceGroups: []ec2.UserSecurityGroup{ + ec2.UserSecurityGroup{ + Id: "foo", + }, }, }, } - if !reflect.DeepEqual(perms[0], expected) { + if !reflect.DeepEqual(perms, expected) { t.Fatalf( "Got:\n\n%#v\n\nExpected:\n\n%#v\n", perms[0], diff --git a/website/source/docs/providers/aws/r/security_group.html.markdown b/website/source/docs/providers/aws/r/security_group.html.markdown index 7ada3f35b..89323a996 100644 --- a/website/source/docs/providers/aws/r/security_group.html.markdown +++ b/website/source/docs/providers/aws/r/security_group.html.markdown @@ -41,6 +41,8 @@ The `ingress` block supports: * `from_port` - (Required) The start port. * `protocol` - (Required) The protocol. * `security_groups` - (Optional) List of security group IDs. Cannot be used with `cidr_blocks`. +* `self` - (Optional) If true, the security group itself will be added as + a source to this ingress rule. * `to_port` - (Required) The end range port. ## Attributes Reference