Merge pull request #4779 from hashicorp/aws-sg-drift-fix

provider/aws: Fix issue with detecting drift in AWS Security Groups rules

builtin/providers/aws/resource_aws_security_group.go

@@ -277,15 +277,21 @@ func resourceAwsSecurityGroupRead(d *schema.ResourceData, meta interface{}) erro
 
 	sg := sgRaw.(*ec2.SecurityGroup)
 
-	ingressRules := resourceAwsSecurityGroupIPPermGather(d, sg.IpPermissions)
-	egressRules := resourceAwsSecurityGroupIPPermGather(d, sg.IpPermissionsEgress)
+	ingressRules := resourceAwsSecurityGroupIPPermGather(d.Id(), sg.IpPermissions)
+	egressRules := resourceAwsSecurityGroupIPPermGather(d.Id(), sg.IpPermissionsEgress)
 
 	d.Set("description", sg.Description)
 	d.Set("name", sg.GroupName)
 	d.Set("vpc_id", sg.VpcId)
 	d.Set("owner_id", sg.OwnerId)
-	d.Set("ingress", ingressRules)
-	d.Set("egress", egressRules)
+	if err := d.Set("ingress", ingressRules); err != nil {
+		log.Printf("[WARN] Error setting Ingress rule set for (%s): %s", d.Id(), err)
+	}
+
+	if err := d.Set("egress", egressRules); err != nil {
+		log.Printf("[WARN] Error setting Egress rule set for (%s): %s", d.Id(), err)
+	}
+
 	d.Set("tags", tagsToMap(sg.Tags))
 	return nil
 }
@@ -394,7 +400,7 @@ func resourceAwsSecurityGroupRuleHash(v interface{}) int {
 	return hashcode.String(buf.String())
 }
 
-func resourceAwsSecurityGroupIPPermGather(d *schema.ResourceData, permissions []*ec2.IpPermission) []map[string]interface{} {
+func resourceAwsSecurityGroupIPPermGather(groupId string, permissions []*ec2.IpPermission) []map[string]interface{} {
 	ruleMap := make(map[string]map[string]interface{})
 	for _, perm := range permissions {
 		var fromPort, toPort int64
@@ -435,7 +441,7 @@ func resourceAwsSecurityGroupIPPermGather(d *schema.ResourceData, permissions []
 			groups = flattenSecurityGroups(perm.UserIdGroupPairs)
 		}
 		for i, id := range groups {
-			if id == d.Id() {
+			if id == groupId {
 				groups[i], groups = groups[len(groups)-1], groups[:len(groups)-1]
 				m["self"] = true
 			}
@@ -444,11 +450,14 @@ func resourceAwsSecurityGroupIPPermGather(d *schema.ResourceData, permissions []
 		if len(groups) > 0 {
 			raw, ok := m["security_groups"]
 			if !ok {
-				raw = make([]string, 0, len(groups))
+				raw = schema.NewSet(schema.HashString, nil)
+			}
+			list := raw.(*schema.Set)
+
+			for _, g := range groups {
+				list.Add(g)
 			}
-			list := raw.([]string)
 
-			list = append(list, groups...)
 			m["security_groups"] = list
 		}
 	}
@@ -456,6 +465,7 @@ func resourceAwsSecurityGroupIPPermGather(d *schema.ResourceData, permissions []
 	for _, m := range ruleMap {
 		rules = append(rules, m)
 	}
+
 	return rules
 }
 

builtin/providers/aws/resource_aws_security_group_test.go

@@ -10,9 +10,82 @@ import (
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/hashicorp/terraform/helper/resource"
+	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 )
 
+func TestResourceAwsSecurityGroupIPPermGather(t *testing.T) {
+	raw := []*ec2.IpPermission{
+		&ec2.IpPermission{
+			IpProtocol: aws.String("tcp"),
+			FromPort:   aws.Int64(int64(1)),
+			ToPort:     aws.Int64(int64(-1)),
+			IpRanges:   []*ec2.IpRange{&ec2.IpRange{CidrIp: aws.String("0.0.0.0/0")}},
+			UserIdGroupPairs: []*ec2.UserIdGroupPair{
+				&ec2.UserIdGroupPair{
+					GroupId: aws.String("sg-22222"),
+				},
+			},
+		},
+		&ec2.IpPermission{
+			IpProtocol: aws.String("tcp"),
+			FromPort:   aws.Int64(int64(80)),
+			ToPort:     aws.Int64(int64(80)),
+			UserIdGroupPairs: []*ec2.UserIdGroupPair{
+				&ec2.UserIdGroupPair{
+					GroupId: aws.String("foo"),
+				},
+			},
+		},
+	}
+
+	local := []map[string]interface{}{
+		map[string]interface{}{
+			"protocol":    "tcp",
+			"from_port":   int64(1),
+			"to_port":     int64(-1),
+			"cidr_blocks": []string{"0.0.0.0/0"},
+			"self":        true,
+		},
+		map[string]interface{}{
+			"protocol":  "tcp",
+			"from_port": int64(80),
+			"to_port":   int64(80),
+			"security_groups": schema.NewSet(schema.HashString, []interface{}{
+				"foo",
+			}),
+		},
+	}
+
+	out := resourceAwsSecurityGroupIPPermGather("sg-22222", raw)
+	for _, i := range out {
+		// loop and match rules, because the ordering is not guarneteed
+		for _, l := range local {
+			if i["from_port"] == l["from_port"] {
+
+				if i["to_port"] != l["to_port"] {
+					t.Fatalf("to_port does not match")
+				}
+
+				if _, ok := i["cidr_blocks"]; ok {
+					if !reflect.DeepEqual(i["cidr_blocks"], l["cidr_blocks"]) {
+						t.Fatalf("error matching cidr_blocks")
+					}
+				}
+
+				if _, ok := i["security_groups"]; ok {
+					outSet := i["security_groups"].(*schema.Set)
+					localSet := l["security_groups"].(*schema.Set)
+
+					if !outSet.Equal(localSet) {
+						t.Fatalf("Security Group sets are not equal")
+					}
+				}
+			}
+		}
+	}
+}
+
 func TestAccAWSSecurityGroup_basic(t *testing.T) {
 	var group ec2.SecurityGroup