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
- [GH-172]
+ * providers/aws: You can enable and disable dns settings for VPCs. [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,58 +181,34 @@ 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{}))
- newRules := expandIPPerms(n.([]interface{}))
-
- var add, remove []ec2.IPPerm
- for _, p := range newRules {
- // 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)
- }
+ os := o.(*schema.Set)
+ ns := n.(*schema.Set)
+
+ remove := expandIPPerms(os.Difference(ns).List())
+ add := expandIPPerms(ns.Difference(os).List())
 
  // TODO: We need to handle partial state better in the in-between
  // in this update.
-
- // 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(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)
+ }
  }
 
- // Revoke the old rules
- _, err = ec2conn.RevokeSecurityGroup(group, remove)
- 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)
+ }
  }
  }
 

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()
  }