providers/aws: security group ingress rules treated as set [GH-87]

/cc @pearkes - !!!

CHANGELOG.md

@@ -25,8 +25,7 @@ IMPROVEMENTS:
       attributes.
   * providers/aws: Security group rules can be updated without a
       destroy/create.
-  * providers/aws: You can enable and disable dns settings for VPCs
+  * providers/aws: You can enable and disable dns settings for VPCs. [GH-172]
-    [GH-172]
 
 BUG FIXES:
 
@@ -36,6 +35,8 @@ BUG FIXES:
   * providers/aws: Fix issues around failing to read EIPs. [GH-122]
   * providers/aws: Autoscaling groups now register and export load
     balancers. [GH-207]
+  * providers/aws: Ingress results are treated as a set, so order doesn't
+      matter anymore. [GH-87]
   * providers/heroku: If you delete the `config_vars` block, config vars
       are properly nuked.
   * providers/heroku: Domains and drains are deleted before the app.

builtin/providers/aws/resource_aws_security_group.go

@@ -1,11 +1,12 @@
 package aws
 
 import (
+	"bytes"
 	"fmt"
 	"log"
-	"reflect"
 	"time"
 
+	"github.com/hashicorp/terraform/helper/hashcode"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
@@ -38,7 +39,7 @@ func resourceAwsSecurityGroup() *schema.Resource {
 			},
 
 			"ingress": &schema.Schema{
-				Type:     schema.TypeList,
+				Type:     schema.TypeSet,
 				Required: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
@@ -70,6 +71,7 @@ func resourceAwsSecurityGroup() *schema.Resource {
 						},
 					},
 				},
+				Set: resourceAwsSecurityGroupIngressHash,
 			},
 
 			"owner_id": &schema.Schema{
@@ -80,6 +82,27 @@ func resourceAwsSecurityGroup() *schema.Resource {
 	}
 }
 
+func resourceAwsSecurityGroupIngressHash(v interface{}) int {
+	var buf bytes.Buffer
+	m := v.(map[string]interface{})
+	buf.WriteString(fmt.Sprintf("%d-", m["from_port"].(int)))
+	buf.WriteString(fmt.Sprintf("%d-", m["to_port"].(int)))
+	buf.WriteString(fmt.Sprintf("%d-", m["protocol"].(string)))
+
+	if v, ok := m["cidr_blocks"]; ok {
+		for _, raw := range v.([]interface{}) {
+			buf.WriteString(fmt.Sprintf("%s-", raw.(string)))
+		}
+	}
+	if v, ok := m["security_groups"]; ok {
+		for _, raw := range v.([]interface{}) {
+			buf.WriteString(fmt.Sprintf("%s-", raw.(string)))
+		}
+	}
+
+	return hashcode.String(buf.String())
+}
+
 func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) error {
 	p := meta.(*ResourceProvider)
 	ec2conn := p.ec2conn
@@ -127,9 +150,9 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er
 	// Expand the "ingress" array to goamz compat []ec2.IPPerm
 	ingressRaw := d.Get("ingress")
 	if ingressRaw == nil {
-		ingressRaw = []interface{}{}
+		ingressRaw = new(schema.Set)
 	}
-	ingressList := ingressRaw.([]interface{})
+	ingressList := ingressRaw.(*schema.Set).List()
 	if len(ingressList) > 0 {
 		ingressRules := expandIPPerms(ingressList)
 		_, err = ec2conn.AuthorizeSecurityGroup(group, ingressRules)
@@ -158,60 +181,36 @@ func resourceAwsSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) er
 	if d.HasChange("ingress") {
 		o, n := d.GetChange("ingress")
 		if o == nil {
-			o = []interface{}{}
+			o = new(schema.Set)
 		}
 		if n == nil {
-			n = []interface{}{}
+			n = new(schema.Set)
 		}
 
-		oldRules := expandIPPerms(o.([]interface{}))
+		os := o.(*schema.Set)
-		newRules := expandIPPerms(n.([]interface{}))
+		ns := n.(*schema.Set)
 
-		var add, remove []ec2.IPPerm
+		remove := expandIPPerms(os.Difference(ns).List())
-		for _, p := range newRules {
+		add := expandIPPerms(ns.Difference(os).List())
-			// Check if we have had this rule before
-			exists := false
-			for _, old := range oldRules {
-				if reflect.DeepEqual(old, p) {
-					exists = true
-					break
-				}
-			}
-			if exists {
-				continue
-			}
-			add = append(add, p)
-		}
-		for _, p := range oldRules {
-			// Check if we have this rule to add
-			exists := false
-			for _, n := range newRules {
-				if reflect.DeepEqual(n, p) {
-					exists = true
-					break
-				}
-			}
-			if exists {
-				continue
-			}
-			remove = append(remove, p)
-		}
 
 		// TODO: We need to handle partial state better in the in-between
 		// in this update.
-
+		if len(add) > 0 {
 			// Authorize the new rules
 			_, err := ec2conn.AuthorizeSecurityGroup(group, add)
 			if err != nil {
 				return fmt.Errorf("Error authorizing security group ingress rules: %s", err)
 			}
+		}
 
+		if len(remove) > 0 {
 			// Revoke the old rules
 			_, err = ec2conn.RevokeSecurityGroup(group, remove)
 			if err != nil {
 				return fmt.Errorf("Error authorizing security group ingress rules: %s", err)
 			}
 		}
+	}
 
 	return nil
 }

helper/schema/schema.go

@@ -239,6 +239,12 @@ func (m schemaMap) diffList(
 	diff *terraform.ResourceDiff,
 	d *ResourceData) error {
 	o, n, _ := d.diffChange(k)
+	if o == nil {
+		o = []interface{}{}
+	}
+	if n == nil {
+		n = []interface{}{}
+	}
 	if s, ok := o.(*Set); ok {
 		o = s.List()
 	}