Add test

aws/resource_aws_security_group.go

@@ -688,14 +688,14 @@ func resourceAwsSecurityGroupUpdateRules(
 			n = new(schema.Set)
 		}
 
-		os := expandRules(o.(*schema.Set))
-		ns := expandRules(n.(*schema.Set))
+		os := resourceAwsSecurityGroupExpandRules(o.(*schema.Set))
+		ns := resourceAwsSecurityGroupExpandRules(n.(*schema.Set))
 
-		remove, err := expandIPPerms(group, collapseRules(ruleset, os.Difference(ns).List()))
+		remove, err := expandIPPerms(group, resourceAwsSecurityGroupCollapseRules(ruleset, os.Difference(ns).List()))
 		if err != nil {
 			return err
 		}
-		add, err := expandIPPerms(group, collapseRules(ruleset, ns.Difference(os).List()))
+		add, err := expandIPPerms(group, resourceAwsSecurityGroupCollapseRules(ruleset, ns.Difference(os).List()))
 		if err != nil {
 			return err
 		}
@@ -1143,7 +1143,34 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface
 	return saves
 }
 
-func collapseRules(ruleset string, rules []interface{}) []interface{} {
+// Duplicate ingress/egress block structure and fill out all
+// the required fields
+func resourceAwsSecurityGroupCopyRule(src map[string]interface{}, self bool, k string, v interface{}) map[string]interface{} {
+	var keys_to_copy = []string{"description", "from_port", "to_port", "protocol"}
+
+	dst := make(map[string]interface{})
+	for _, key := range keys_to_copy {
+		if val, ok := src[key]; ok {
+			dst[key] = val
+		}
+	}
+	if k != "" {
+		dst[k] = v
+	}
+	if _, ok := src["self"]; ok {
+		dst["self"] = self
+	}
+	return dst
+}
+
+// Given a set of SG rules (ingress/egress blocks), this function
+// will group the rules by from_port/to_port/protocol/description
+// tuples. This is inverse operation of
+// resourceAwsSecurityGroupExpandRules()
+//
+// For more detail, see comments for
+// resourceAwsSecurityGroupExpandRules()
+func resourceAwsSecurityGroupCollapseRules(ruleset string, rules []interface{}) []interface{} {
 
 	var keys_to_collapse = []string{"cidr_blocks", "ipv6_cidr_blocks", "prefix_list_ids", "security_groups"}
 
@@ -1185,25 +1212,53 @@ func collapseRules(ruleset string, rules []interface{}) []interface{} {
 	return values
 }
 
-func copyRule(src map[string]interface{}, self bool, k string, v interface{}) map[string]interface{} {
-	var keys_to_copy = []string{"description", "from_port", "to_port", "protocol"}
-
-	dst := make(map[string]interface{})
-	for _, key := range keys_to_copy {
-		if val, ok := src[key]; ok {
-			dst[key] = val
-		}
-	}
-	if k != "" {
-		dst[k] = v
-	}
-	if _, ok := src["self"]; ok {
-		dst["self"] = self
-	}
-	return dst
-}
-
-func expandRules(rules *schema.Set) *schema.Set {
+// resourceAwsSecurityGroupExpandRules works in pair with
+// resourceAwsSecurityGroupCollapseRules and is used as a
+// workaround for the problem explained in
+// https://github.com/terraform-providers/terraform-provider-aws/pull/4726
+//
+// This function converts every ingress/egress block that
+// contains multiple rules to multiple blocks with only one
+// rule. Doing a Difference operation on such a normalized
+// set helps to avoid unnecessary removal of unchanged
+// rules during the Apply step.
+//
+// For example, in terraform syntax, the following block:
+//
+// ingress {
+//   from_port = 80
+//   to_port = 80
+//   protocol = "tcp"
+//   cidr_blocks = [
+//     "192.168.0.1/32",
+//     "192.168.0.2/32",
+//   ]
+// }
+//
+// will be converted to the two blocks below:
+//
+// ingress {
+//   from_port = 80
+//   to_port = 80
+//   protocol = "tcp"
+//   cidr_blocks = [ "192.168.0.1/32" ]
+// }
+//
+// ingress {
+//   from_port = 80
+//   to_port = 80
+//   protocol = "tcp"
+//   cidr_blocks = [ "192.168.0.2/32" ]
+// }
+//
+// Then the Difference operation is executed on the new set
+// to find which rules got modified, and the resulting set
+// is then passed to resourceAwsSecurityGroupCollapseRules
+// to convert the "diff" back to a more compact form for
+// execution. Such compact form helps reduce the number of
+// API calls.
+//
+func resourceAwsSecurityGroupExpandRules(rules *schema.Set) *schema.Set {
 	var keys_to_expand = []string{"cidr_blocks", "ipv6_cidr_blocks", "prefix_list_ids", "security_groups"}
 
 	normalized := schema.NewSet(resourceAwsSecurityGroupRuleHash, nil)
@@ -1212,7 +1267,7 @@ func expandRules(rules *schema.Set) *schema.Set {
 		rule := rawRule.(map[string]interface{})
 
 		if v, ok := rule["self"]; ok && v.(bool) {
-			new_rule := copyRule(rule, true, "", nil)
+			new_rule := resourceAwsSecurityGroupCopyRule(rule, true, "", nil)
 			normalized.Add(new_rule)
 		}
 		for _, key := range keys_to_expand {
@@ -1229,11 +1284,11 @@ func expandRules(rules *schema.Set) *schema.Set {
 					if key == "security_groups" {
 						new_v := schema.NewSet(schema.HashString, nil)
 						new_v.Add(v)
-						new_rule = copyRule(rule, false, key, new_v)
+						new_rule = resourceAwsSecurityGroupCopyRule(rule, false, key, new_v)
 					} else {
 						new_v := make([]interface{}, 0)
 						new_v = append(new_v, v)
-						new_rule = copyRule(rule, false, key, new_v)
+						new_rule = resourceAwsSecurityGroupCopyRule(rule, false, key, new_v)
 					}
 					normalized.Add(new_rule)
 				}
@@ -1244,6 +1299,8 @@ func expandRules(rules *schema.Set) *schema.Set {
 	return normalized
 }
 
+// Convert type-to_port-from_port-protocol-description tuple
+// to a hash to use as a key in Set.
 func idCollapseHash(rType, protocol string, toPort, fromPort int64, description string) string {
 	var buf bytes.Buffer
 	buf.WriteString(fmt.Sprintf("%s-", rType))