Merge pull request #2305 from hashicorp/b-aws-sg-rule-self

provider/aws: Fix Security Group Rule self reference bug

builtin/providers/aws/resource_aws_security_group_rule.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
+	"github.com/aws/aws-sdk-go/aws/awsutil"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/hashicorp/terraform/helper/hashcode"
 	"github.com/hashicorp/terraform/helper/schema"
@@ -91,8 +92,8 @@ func resourceAwsSecurityGroupRuleCreate(d *schema.ResourceData, meta interface{}
 
 	switch ruleType {
 	case "ingress":
-		log.Printf("[DEBUG] Authorizing security group %s %s rule: %#v",
-			sg_id, "Ingress", perm)
+		log.Printf("[DEBUG] Authorizing security group %s %s rule: %s",
+			sg_id, "Ingress", awsutil.StringValue(perm))
 
 		req := &ec2.AuthorizeSecurityGroupIngressInput{
 			GroupID:       sg.GroupID,
@@ -202,8 +203,8 @@ func resourceAwsSecurityGroupRuleDelete(d *schema.ResourceData, meta interface{}
 	ruleType := d.Get("type").(string)
 	switch ruleType {
 	case "ingress":
-		log.Printf("[DEBUG] Revoking security group %#v %s rule: %#v",
-			sg_id, "ingress", perm)
+		log.Printf("[DEBUG] Revoking rule (%s) from security group %s:\n%s",
+			"ingress", sg_id, awsutil.StringValue(perm))
 		req := &ec2.RevokeSecurityGroupIngressInput{
 			GroupID:       sg.GroupID,
 			IPPermissions: []*ec2.IPPermission{perm},
@@ -297,22 +298,29 @@ func expandIPPerm(d *schema.ResourceData, sg *ec2.SecurityGroup) *ec2.IPPermissi
 	perm.ToPort = aws.Long(int64(d.Get("to_port").(int)))
 	perm.IPProtocol = aws.String(d.Get("protocol").(string))
 
-	var groups []string
+	// build a group map that behaves like a set
+	groups := make(map[string]bool)
 	if raw, ok := d.GetOk("source_security_group_id"); ok {
-		groups = append(groups, raw.(string))
+		groups[raw.(string)] = true
 	}
 
 	if v, ok := d.GetOk("self"); ok && v.(bool) {
 		if sg.VPCID != nil && *sg.VPCID != "" {
-			groups = append(groups, *sg.GroupID)
+			groups[*sg.GroupID] = true
 		} else {
-			groups = append(groups, *sg.GroupName)
+			groups[*sg.GroupName] = true
 		}
 	}
 
 	if len(groups) > 0 {
 		perm.UserIDGroupPairs = make([]*ec2.UserIDGroupPair, len(groups))
-		for i, name := range groups {
+		// build string list of group name/ids
+		var gl []string
+		for k, _ := range groups {
+			gl = append(gl, k)
+		}
+
+		for i, name := range gl {
 			ownerId, id := "", name
 			if items := strings.Split(id, "/"); len(items) > 1 {
 				ownerId, id = items[0], items[1]

builtin/providers/aws/resource_aws_security_group_rule_test.go

@@ -196,6 +196,24 @@ func TestAccAWSSecurityGroupRule_Egress(t *testing.T) {
 	})
 }
 
+func TestAccAWSSecurityGroupRule_SelfReference(t *testing.T) {
+	var group ec2.SecurityGroup
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSSecurityGroupRuleConfigSelfReference,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckAWSSecurityGroupRuleDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).ec2conn
 
@@ -390,3 +408,34 @@ resource "aws_security_group_rule" "ingress_2" {
   security_group_id = "${aws_security_group.web.id}"
 }
 `
+
+// check for GH-1985 regression
+const testAccAWSSecurityGroupRuleConfigSelfReference = `
+provider "aws" {
+  region = "us-west-2"
+}
+
+resource "aws_vpc" "main" {
+  cidr_block = "10.0.0.0/16"
+  tags {
+    Name = "sg-self-test"
+  }
+}
+
+resource "aws_security_group" "web" {
+  name = "main"
+  vpc_id = "${aws_vpc.main.id}"
+  tags {
+    Name = "sg-self-test"
+  }
+}
+
+resource "aws_security_group_rule" "self" {
+  type = "ingress"
+  protocol = "-1"
+  from_port = 0
+  to_port = 0
+  self = true
+  security_group_id = "${aws_security_group.web.id}"
+}
+`