From 7462eb174238cefc0f33cbd0a304caa845895f4e Mon Sep 17 00:00:00 2001 From: clint shryock Date: Wed, 20 Jan 2016 10:52:46 -0600 Subject: [PATCH] provider/aws: Fix issue with detecting drift in AWS Security Groups in-line rules --- .../aws/resource_aws_security_group.go | 28 ++++--- .../aws/resource_aws_security_group_test.go | 73 +++++++++++++++++++ 2 files changed, 92 insertions(+), 9 deletions(-) diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go index 67d4c356c0..7b2208571e 100644 --- a/builtin/providers/aws/resource_aws_security_group.go +++ b/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 } diff --git a/builtin/providers/aws/resource_aws_security_group_test.go b/builtin/providers/aws/resource_aws_security_group_test.go index 2ae9d283b2..dadd13ac97 100644 --- a/builtin/providers/aws/resource_aws_security_group_test.go +++ b/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