diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go index 846b7eec0..e45b7dc71 100644 --- a/builtin/providers/aws/resource_aws_security_group.go +++ b/builtin/providers/aws/resource_aws_security_group.go @@ -227,7 +227,7 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er } // AWS defaults all Security Groups to have an ALLOW ALL egress rule. Here we - // revoke that rule, so users don't unknowningly have/use it. + // revoke that rule, so users don't unknowingly have/use it. group := resp.(*ec2.SecurityGroup) if group.VpcId != nil && *group.VpcId != "" { log.Printf("[DEBUG] Revoking default egress rule for Security Group for %s", d.Id()) @@ -273,13 +273,26 @@ func resourceAwsSecurityGroupRead(d *schema.ResourceData, meta interface{}) erro sg := sgRaw.(*ec2.SecurityGroup) - ingressRules := resourceAwsSecurityGroupIPPermGather(d.Id(), sg.IpPermissions) - egressRules := resourceAwsSecurityGroupIPPermGather(d.Id(), sg.IpPermissionsEgress) + remoteIngressRules := resourceAwsSecurityGroupIPPermGather(d.Id(), sg.IpPermissions) + remoteEgressRules := resourceAwsSecurityGroupIPPermGather(d.Id(), sg.IpPermissionsEgress) + + // + // TODO enforce the seperation of ips and security_groups in a rule block + // + + localIngressRules := d.Get("ingress").(*schema.Set).List() + localEgressRules := d.Get("egress").(*schema.Set).List() + + // Loop through the local state of rules, doing a match against the remote + // ruleSet we built above. + ingressRules := matchRules("ingress", localIngressRules, remoteIngressRules) + egressRules := matchRules("egress", localEgressRules, remoteEgressRules) d.Set("description", sg.Description) d.Set("name", sg.GroupName) d.Set("vpc_id", sg.VpcId) d.Set("owner_id", sg.OwnerId) + if err := d.Set("ingress", ingressRules); err != nil { log.Printf("[WARN] Error setting Ingress rule set for (%s): %s", d.Id(), err) } @@ -593,3 +606,219 @@ func SGStateRefreshFunc(conn *ec2.EC2, id string) resource.StateRefreshFunc { return group, "exists", nil } } + +// matchRules receives the group id, type of rules, and the local / remote maps +// of rules. We iterate through the local set of rules trying to find a matching +// remote rule, which may be structured differently because of how AWS +// aggregates the rules under the to, from, and type. +// +// +// Matching rules are written to state, with their elements removed from the +// remote set +// +// If no match is found, we'll write the remote rule to state and let the graph +// sort things out +func matchRules(rType string, local []interface{}, remote []map[string]interface{}) []map[string]interface{} { + // For each local ip or security_group, we need to match against the remote + // ruleSet until all ips or security_groups are found + + // saves represents the rules that have been identified to be saved to state, + // in the appropriate d.Set("{ingress,egress}") call. + var saves []map[string]interface{} + for _, raw := range local { + l := raw.(map[string]interface{}) + + var selfVal bool + if v, ok := l["self"]; ok { + selfVal = v.(bool) + } + + // matching against self is required to detect rules that only include self + // as the rule. resourceAwsSecurityGroupIPPermGather parses the group out + // and replaces it with self if it's ID is found + localHash := idHash(rType, l["protocol"].(string), int64(l["to_port"].(int)), int64(l["from_port"].(int)), selfVal) + + // loop remote rules, looking for a matching hash + for _, r := range remote { + var remoteSelfVal bool + if v, ok := r["self"]; ok { + remoteSelfVal = v.(bool) + } + + // hash this remote rule and compare it for a match consideration with the + // local rule we're examining + rHash := idHash(rType, r["protocol"].(string), r["to_port"].(int64), r["from_port"].(int64), remoteSelfVal) + if rHash == localHash { + var numExpectedCidrs, numExpectedSGs, numRemoteCidrs, numRemoteSGs int + var matchingCidrs []string + var matchingSGs []string + + // grab the local/remote cidr and sg groups, capturing the expected and + // actual counts + lcRaw, ok := l["cidr_blocks"] + if ok { + numExpectedCidrs = len(l["cidr_blocks"].([]interface{})) + } + lsRaw, ok := l["security_groups"] + if ok { + numExpectedSGs = len(l["security_groups"].(*schema.Set).List()) + } + + rcRaw, ok := r["cidr_blocks"] + if ok { + numRemoteCidrs = len(r["cidr_blocks"].([]string)) + } + + rsRaw, ok := r["security_groups"] + if ok { + numRemoteSGs = len(r["security_groups"].(*schema.Set).List()) + } + + // check some early failures + if numExpectedCidrs > numRemoteCidrs { + log.Printf("[DEBUG] Local rule has more CIDR blocks, continuing (%d/%d)", numExpectedCidrs, numRemoteCidrs) + continue + } + if numExpectedSGs > numRemoteSGs { + log.Printf("[DEBUG] Local rule has more Security Groups, continuing (%d/%d)", numExpectedSGs, numRemoteSGs) + continue + } + + // match CIDRs by converting both to sets, and using Set methods + var localCidrs []interface{} + if lcRaw != nil { + localCidrs = lcRaw.([]interface{}) + } + localCidrSet := schema.NewSet(schema.HashString, localCidrs) + + // remote cidrs are presented as a slice of strings, so we need to + // reformat them into a slice of interfaces to be used in creating the + // remote cidr set + var remoteCidrs []string + if rcRaw != nil { + remoteCidrs = rcRaw.([]string) + } + // convert remote cidrs to a set, for easy comparisions + var list []interface{} + for _, s := range remoteCidrs { + list = append(list, s) + } + remoteCidrSet := schema.NewSet(schema.HashString, list) + + // Build up a list of local cidrs that are found in the remote set + for _, s := range localCidrSet.List() { + if remoteCidrSet.Contains(s) { + matchingCidrs = append(matchingCidrs, s.(string)) + } + } + + // match SGs. Both local and remote are already sets + var localSGSet *schema.Set + if lsRaw == nil { + localSGSet = schema.NewSet(schema.HashString, nil) + } else { + localSGSet = lsRaw.(*schema.Set) + } + + var remoteSGSet *schema.Set + if rsRaw == nil { + remoteSGSet = schema.NewSet(schema.HashString, nil) + } else { + remoteSGSet = rsRaw.(*schema.Set) + } + + // Build up a list of local security groups that are found in the remote set + for _, s := range localSGSet.List() { + if remoteSGSet.Contains(s) { + matchingSGs = append(matchingSGs, s.(string)) + } + } + + // compare equalities for matches. + // If we found the number of cidrs and number of sgs, we declare a + // match, and then remove those elements from the remote rule, so that + // this remote rule can still be considered by other local rules + if numExpectedCidrs == len(matchingCidrs) { + if numExpectedSGs == len(matchingSGs) { + // confirm that self references match + var lSelf bool + var rSelf bool + if _, ok := l["self"]; ok { + lSelf = l["self"].(bool) + } + if _, ok := r["self"]; ok { + rSelf = r["self"].(bool) + } + if rSelf == lSelf { + delete(r, "self") + // pop local cidrs from remote + diffCidr := remoteCidrSet.Difference(localCidrSet) + var newCidr []string + for _, cRaw := range diffCidr.List() { + newCidr = append(newCidr, cRaw.(string)) + } + + // reassigning + if len(newCidr) > 0 { + r["cidr_blocks"] = newCidr + } else { + delete(r, "cidr_blocks") + } + + // pop local sgs from remote + diffSGs := remoteSGSet.Difference(localSGSet) + if len(diffSGs.List()) > 0 { + r["security_groups"] = diffSGs + } else { + delete(r, "security_groups") + } + + saves = append(saves, l) + } + } + } + } + } + } + + // Here we catch any remote rules that have not been stripped of all self, + // cidrs, and security groups. We'll add remote rules here that have not been + // matched locally, and let the graph sort things out. This will happen when + // rules are added externally to Terraform + for _, r := range remote { + var lenCidr, lenSGs int + if rCidrs, ok := r["cidr_blocks"]; ok { + lenCidr = len(rCidrs.([]string)) + } + + if rawSGs, ok := r["security_groups"]; ok { + lenSGs = len(rawSGs.(*schema.Set).List()) + } + + if _, ok := r["self"]; ok { + if r["self"].(bool) == true { + lenSGs++ + } + } + + if lenSGs+lenCidr > 0 { + log.Printf("[DEBUG] Found a remote Rule that wasn't empty: (%#v)", r) + saves = append(saves, r) + } + } + + return saves +} + +// Creates a unique hash for the type, ports, and protocol, used as a key in +// maps +func idHash(rType, protocol string, toPort, fromPort int64, self bool) string { + var buf bytes.Buffer + buf.WriteString(fmt.Sprintf("%s-", rType)) + buf.WriteString(fmt.Sprintf("%d-", toPort)) + buf.WriteString(fmt.Sprintf("%d-", fromPort)) + buf.WriteString(fmt.Sprintf("%s-", protocol)) + buf.WriteString(fmt.Sprintf("%t-", self)) + + return fmt.Sprintf("rule-%d", hashcode.String(buf.String())) +} diff --git a/builtin/providers/aws/resource_aws_security_group_rules_matching_test.go b/builtin/providers/aws/resource_aws_security_group_rules_matching_test.go new file mode 100644 index 000000000..d3cd721b5 --- /dev/null +++ b/builtin/providers/aws/resource_aws_security_group_rules_matching_test.go @@ -0,0 +1,693 @@ +package aws + +import ( + "log" + "testing" + + "github.com/hashicorp/terraform/helper/schema" +) + +// testing rulesForGroupPermissions +func TestRulesMixedMatching(t *testing.T) { + cases := []struct { + groupId string + local []interface{} + remote []map[string]interface{} + saves []map[string]interface{} + }{ + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"172.8.0.0/16", "10.0.0.0/16"}, + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "10.0.0.0/16"}, + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "10.0.0.0/16"}, + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + }, + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "10.0.0.0/16"}, + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "10.0.0.0/16"}, + }, + }, + }, + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"172.8.0.0/16", "10.0.0.0/16"}, + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "10.0.0.0/16"}, + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "10.0.0.0/16"}, + }, + }, + }, + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + }, + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"172.8.0.0/16"}, + }, + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"192.168.0.0/16"}, + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "192.168.0.0/16"}, + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16"}, + }, + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []string{"192.168.0.0/16"}, + }, + }, + }, + { + local: []interface{}{}, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "10.0.0.0/16"}, + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "10.0.0.0/16"}, + }, + }, + }, + // local and remote differ + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"172.8.0.0/16"}, + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"10.0.0.0/16"}, + }, + }, + // Because this is the remote rule being saved, we need to check for int64 + // encoding. We could convert this code, but ultimately Terraform doesn't + // care it's for the reflect.DeepEqual in this test + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"10.0.0.0/16"}, + }, + }, + }, + // local with more rules and the remote (the remote should then be saved) + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"172.8.0.0/16", "10.8.0.0/16", "192.168.0.0/16"}, + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "192.168.0.0/16"}, + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "192.168.0.0/16"}, + }, + }, + }, + // 3 local rules + // this should trigger a diff (not shown) + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"172.8.0.0/16"}, + }, + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"10.8.0.0/16"}, + }, + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"192.168.0.0/16"}, + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "192.168.0.0/16"}, + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16"}, + }, + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []string{"192.168.0.0/16"}, + }, + }, + }, + // a local rule with 2 cidrs, remote has 4 cidrs, shoudl be saved to match + // the local but also an extra rule found + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"172.8.0.0/16", "10.8.0.0/16"}, + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "192.168.0.0/16", "10.8.0.0/16", "206.8.0.0/16"}, + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "10.8.0.0/16"}, + }, + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"192.168.0.0/16", "206.8.0.0/16"}, + }, + }, + }, + // testing some SGS + { + local: []interface{}{}, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(22), + "to_port": int64(22), + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876"}), + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + // we're saving the remote, so it will be int64 encoded + "from_port": int64(22), + "to_port": int64(22), + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876"}), + }, + }, + }, + // two local blocks that match a single remote group, but are saved as two + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 22, + "to_port": 22, + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876"}), + }, + map[string]interface{}{ + "from_port": 22, + "to_port": 22, + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-4444"}), + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(22), + "to_port": int64(22), + "protocol": "tcp", + "security_groups": schema.NewSet( + schema.HashString, + []interface{}{ + "sg-9876", + "sg-4444", + }, + ), + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": 22, + "to_port": 22, + "protocol": "tcp", + "security_groups": schema.NewSet( + schema.HashString, + []interface{}{ + "sg-9876", + }, + ), + }, + map[string]interface{}{ + "from_port": 22, + "to_port": 22, + "protocol": "tcp", + "security_groups": schema.NewSet( + schema.HashString, + []interface{}{ + "sg-4444", + }, + ), + }, + }, + }, + // test self with other rules + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 22, + "to_port": 22, + "protocol": "tcp", + "self": true, + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(22), + "to_port": int64(22), + "protocol": "tcp", + "self": true, + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(22), + "to_port": int64(22), + "protocol": "tcp", + "self": true, + }, + }, + }, + // test self + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 22, + "to_port": 22, + "protocol": "tcp", + "self": true, + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(22), + "to_port": int64(22), + "protocol": "tcp", + "self": true, + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(22), + "to_port": int64(22), + "protocol": "tcp", + "self": true, + }, + }, + }, + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 22, + "to_port": 22, + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(22), + "to_port": int64(22), + "protocol": "tcp", + "self": true, + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(22), + "to_port": int64(22), + "protocol": "tcp", + "self": true, + }, + }, + }, + // mix of sgs and cidrs + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"172.8.0.0/16", "10.8.0.0/16", "192.168.0.0/16"}, + }, + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "192.168.0.0/16"}, + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "192.168.0.0/16"}, + }, + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + }, + { + local: []interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "cidr_blocks": []interface{}{"172.8.0.0/16", "10.8.0.0/16", "192.168.0.0/16"}, + }, + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "self": true, + }, + }, + remote: []map[string]interface{}{ + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "192.168.0.0/16"}, + "self": true, + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + saves: []map[string]interface{}{ + map[string]interface{}{ + "from_port": 80, + "to_port": 8000, + "protocol": "tcp", + "self": true, + }, + map[string]interface{}{ + "from_port": int64(80), + "to_port": int64(8000), + "protocol": "tcp", + "cidr_blocks": []string{"172.8.0.0/16", "192.168.0.0/16"}, + "security_groups": schema.NewSet(schema.HashString, []interface{}{"sg-9876", "sg-4444"}), + }, + }, + }, + } + for i, c := range cases { + saves := matchRules("ingress", c.local, c.remote) + log.Printf("\n======\n\nSaves:\n%#v\n\nCS Saves:\n%#v\n\n======\n", saves, c.saves) + if err != nil { + t.Fatal(err) + } + log.Printf("\n\tTest %d:\n", i) + + if len(saves) != len(c.saves) { + t.Fatalf("Expected %d saves, got %d", len(c.saves), len(saves)) + } + + shouldFind := len(c.saves) + var found int + for _, s := range saves { + for _, cs := range c.saves { + // deep equal cannot compare schema.Set's directly + // make sure we're not failing the reflect b/c of ports/type + for _, attr := range []string{"to_port", "from_port", "type"} { + if s[attr] != cs[attr] { + continue + } + } + + var numExpectedCidrs, numExpectedSGs, numRemoteCidrs, numRemoteSGs int + // var matchingCidrs []string + // var matchingSGs []string + + var cidrsMatch, sGsMatch bool + + if _, ok := s["cidr_blocks"]; ok { + switch s["cidr_blocks"].(type) { + case []string: + numExpectedCidrs = len(s["cidr_blocks"].([]string)) + default: + numExpectedCidrs = len(s["cidr_blocks"].([]interface{})) + } + + } + if _, ok := s["security_groups"]; ok { + numExpectedSGs = len(s["security_groups"].(*schema.Set).List()) + } + + if _, ok := cs["cidr_blocks"]; ok { + numRemoteCidrs = len(cs["cidr_blocks"].([]string)) + } + + if _, ok := cs["security_groups"]; ok { + numRemoteSGs = len(cs["security_groups"].(*schema.Set).List()) + } + + // skip early + if numExpectedSGs != numRemoteSGs { + log.Printf("\n\ncontinuning on numRemoteSGs \n\n") + continue + } + if numExpectedCidrs != numRemoteCidrs { + log.Printf("\n\ncontinuning numRemoteCidrs\n\n") + continue + } + + if numExpectedCidrs == 0 { + cidrsMatch = true + } + if numExpectedSGs == 0 { + sGsMatch = true + } + + // convert save cidrs to set + var lcs []interface{} + if _, ok := s["cidr_blocks"]; ok { + switch s["cidr_blocks"].(type) { + case []string: + for _, c := range s["cidr_blocks"].([]string) { + lcs = append(lcs, c) + } + default: + for _, c := range s["cidr_blocks"].([]interface{}) { + lcs = append(lcs, c) + } + } + } + savesCidrs := schema.NewSet(schema.HashString, lcs) + + // convert cs cidrs to set + var cslcs []interface{} + if _, ok := cs["cidr_blocks"]; ok { + for _, c := range cs["cidr_blocks"].([]string) { + cslcs = append(cslcs, c) + } + } + csCidrs := schema.NewSet(schema.HashString, cslcs) + + if csCidrs.Equal(savesCidrs) { + log.Printf("\nmatched cidrs") + cidrsMatch = true + } + + if rawS, ok := s["security_groups"]; ok { + outSet := rawS.(*schema.Set) + if rawL, ok := cs["security_groups"]; ok { + localSet := rawL.(*schema.Set) + if outSet.Equal(localSet) { + log.Printf("\nmatched sgs") + sGsMatch = true + } + } + } + + var lSelf bool + var rSelf bool + if _, ok := s["self"]; ok { + lSelf = s["self"].(bool) + } + if _, ok := cs["self"]; ok { + rSelf = cs["self"].(bool) + } + + if (sGsMatch && cidrsMatch) && (lSelf == rSelf) { + found++ + } + } + } + + if found != shouldFind { + t.Fatalf("Bad sg rule matches (%d / %d)", found, shouldFind) + } + } +} diff --git a/builtin/providers/aws/resource_aws_security_group_test.go b/builtin/providers/aws/resource_aws_security_group_test.go index dadd13ac9..71d3c39ce 100644 --- a/builtin/providers/aws/resource_aws_security_group_test.go +++ b/builtin/providers/aws/resource_aws_security_group_test.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/service/ec2" + "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" @@ -383,6 +384,66 @@ func TestAccAWSSecurityGroup_DefaultEgress(t *testing.T) { }) } +// Testing drift detection with groups containing the same port and types +func TestAccAWSSecurityGroup_drift(t *testing.T) { + var group ec2.SecurityGroup + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSecurityGroupDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSSecurityGroupConfig_drift(), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSecurityGroupExists("aws_security_group.web", &group), + resource.TestCheckResourceAttr( + "aws_security_group.web", "description", "Used in the terraform acceptance tests"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.protocol", "tcp"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.from_port", "80"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.to_port", "8000"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.cidr_blocks.#", "1"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.cidr_blocks.0", "10.0.0.0/8"), + ), + }, + }, + }) +} + +func TestAccAWSSecurityGroup_drift_complex(t *testing.T) { + var group ec2.SecurityGroup + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSecurityGroupDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSSecurityGroupConfig_drift_complex(), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSecurityGroupExists("aws_security_group.web", &group), + resource.TestCheckResourceAttr( + "aws_security_group.web", "description", "Used in the terraform acceptance tests"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.protocol", "tcp"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.from_port", "80"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.to_port", "8000"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.cidr_blocks.#", "1"), + resource.TestCheckResourceAttr( + "aws_security_group.web", "ingress.3629188364.cidr_blocks.0", "10.0.0.0/8"), + ), + }, + }, + }) +} + func testAccCheckAWSSecurityGroupDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).ec2conn @@ -556,6 +617,25 @@ func TestAccAWSSecurityGroup_tags(t *testing.T) { }) } +func TestAccAWSSecurityGroup_CIDRandGroups(t *testing.T) { + var group ec2.SecurityGroup + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSecurityGroupDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSSecurityGroupCombindCIDRandGroups, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSecurityGroupExists("aws_security_group.mixed", &group), + // testAccCheckAWSSecurityGroupAttributes(&group), + ), + }, + }, + }) +} + func testAccCheckAWSSecurityGroupAttributesChanged(group *ec2.SecurityGroup) resource.TestCheckFunc { return func(s *terraform.State) error { p := []*ec2.IpPermission{ @@ -931,3 +1011,133 @@ resource "aws_security_group" "baz" { description = "Used in the terraform acceptance tests" } ` + +func testAccAWSSecurityGroupConfig_drift() string { + return fmt.Sprintf(` +resource "aws_security_group" "web" { + name = "tf_acc_%d" + description = "Used in the terraform acceptance tests" + + ingress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } + + ingress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["206.0.0.0/8"] + } + + tags { + Name = "tf-acc-test" + } +} +`, acctest.RandInt()) +} + +func testAccAWSSecurityGroupConfig_drift_complex() string { + return fmt.Sprintf(` +resource "aws_security_group" "otherweb" { + name = "tf_acc_%d" + description = "Used in the terraform acceptance tests" +} + +resource "aws_security_group" "web" { + name = "tf_acc_%d" + description = "Used in the terraform acceptance tests" + + ingress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } + + ingress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["206.0.0.0/8"] + } + + ingress { + protocol = "tcp" + from_port = 22 + to_port = 22 + security_groups = ["${aws_security_group.otherweb.id}"] + } + + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["206.0.0.0/8"] + } + + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } + + egress { + protocol = "tcp" + from_port = 22 + to_port = 22 + security_groups = ["${aws_security_group.otherweb.id}"] + } + + tags { + Name = "tf-acc-test" + } +}`, acctest.RandInt(), acctest.RandInt()) +} + +const testAccAWSSecurityGroupCombindCIDRandGroups = ` +resource "aws_security_group" "two" { + name = "tf-test-1" + tags { + Name = "tf-test-1" + } +} + +resource "aws_security_group" "one" { + name = "tf-test-2" + tags { + Name = "tf-test-w" + } +} + +resource "aws_security_group" "three" { + name = "tf-test-3" + tags { + Name = "tf-test-3" + } +} + +resource "aws_security_group" "mixed" { + name = "tf-mix-test" + + ingress { + from_port = 80 + to_port = 80 + protocol = "tcp" + cidr_blocks = ["10.0.0.0/16", "10.1.0.0/16", "10.7.0.0/16"] + + security_groups = [ + "${aws_security_group.one.id}", + "${aws_security_group.two.id}", + "${aws_security_group.three.id}", + ] + } + + tags { + Name = "tf-mix-test" + } +} +`