Change all firewall related resources to take a cidr_list

Also some additional tweaks to improve performance and add in a little
concurrency to speed things up a little.

builtin/providers/cloudstack/provider_test.go

@@ -84,8 +84,11 @@ func testAccPreCheck(t *testing.T) {
 	if v := os.Getenv("CLOUDSTACK_NETWORK_1"); v == "" {
 		t.Fatal("CLOUDSTACK_NETWORK_1 must be set for acceptance tests")
 	}
-	if v := os.Getenv("CLOUDSTACK_NETWORK_1_IPADDRESS"); v == "" {
+	if v := os.Getenv("CLOUDSTACK_NETWORK_1_IPADDRESS1"); v == "" {
-		t.Fatal("CLOUDSTACK_NETWORK_1_IPADDRESS must be set for acceptance tests")
+		t.Fatal("CLOUDSTACK_NETWORK_1_IPADDRESS1 must be set for acceptance tests")
+	}
+	if v := os.Getenv("CLOUDSTACK_NETWORK_1_IPADDRESS2"); v == "" {
+		t.Fatal("CLOUDSTACK_NETWORK_1_IPADDRESS2 must be set for acceptance tests")
 	}
 	if v := os.Getenv("CLOUDSTACK_NETWORK_2"); v == "" {
 		t.Fatal("CLOUDSTACK_NETWORK_2 must be set for acceptance tests")
@@ -159,7 +162,10 @@ var CLOUDSTACK_SERVICE_OFFERING_2 = os.Getenv("CLOUDSTACK_SERVICE_OFFERING_2")
 var CLOUDSTACK_NETWORK_1 = os.Getenv("CLOUDSTACK_NETWORK_1")
 
 // A valid IP address in CLOUDSTACK_NETWORK_1
-var CLOUDSTACK_NETWORK_1_IPADDRESS = os.Getenv("CLOUDSTACK_NETWORK_1_IPADDRESS")
+var CLOUDSTACK_NETWORK_1_IPADDRESS1 = os.Getenv("CLOUDSTACK_NETWORK_1_IPADDRESS1")
+
+// A valid IP address in CLOUDSTACK_NETWORK_1
+var CLOUDSTACK_NETWORK_1_IPADDRESS2 = os.Getenv("CLOUDSTACK_NETWORK_1_IPADDRESS2")
 
 // Name for a network that will be created
 var CLOUDSTACK_NETWORK_2 = os.Getenv("CLOUDSTACK_NETWORK_2")

builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go

@@ -1,14 +1,14 @@
 package cloudstack
 
 import (
-	"bytes"
 	"fmt"
 	"regexp"
-	"sort"
 	"strconv"
 	"strings"
+	"sync"
+	"time"
 
-	"github.com/hashicorp/terraform/helper/hashcode"
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/xanzy/go-cloudstack/cloudstack"
 )
@@ -38,9 +38,17 @@ func resourceCloudStackEgressFirewall() *schema.Resource {
 				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
+						"cidr_list": &schema.Schema{
+							Type:     schema.TypeSet,
+							Optional: true,
+							Elem:     &schema.Schema{Type: schema.TypeString},
+							Set:      schema.HashString,
+						},
+
 						"source_cidr": &schema.Schema{
 							Type:       schema.TypeString,
-							Required: true,
+							Optional:   true,
+							Deprecated: "Please use the `cidr_list` field instead",
 						},
 
 						"protocol": &schema.Schema{
@@ -64,9 +72,7 @@ func resourceCloudStackEgressFirewall() *schema.Resource {
 							Type:     schema.TypeSet,
 							Optional: true,
 							Elem:     &schema.Schema{Type: schema.TypeString},
-							Set: func(v interface{}) int {
+							Set:      schema.HashString,
-								return hashcode.String(v.(string))
-							},
 						},
 
 						"uuids": &schema.Schema{
@@ -75,7 +81,6 @@ func resourceCloudStackEgressFirewall() *schema.Resource {
 						},
 					},
 				},
-				Set: resourceCloudStackEgressFirewallRuleHash,
 			},
 		},
 	}
@@ -99,32 +104,66 @@ func resourceCloudStackEgressFirewallCreate(d *schema.ResourceData, meta interfa
 	d.SetId(networkid)
 
 	// Create all rules that are configured
-	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
+	if nrs := d.Get("rule").(*schema.Set); nrs.Len() > 0 {
-
 		// Create an empty schema.Set to hold all rules
-		rules := &schema.Set{
+		rules := resourceCloudStackEgressFirewall().Schema["rule"].ZeroValue().(*schema.Set)
-			F: resourceCloudStackEgressFirewallRuleHash,
-		}
 
-		for _, rule := range rs.List() {
+		err := createEgressFirewallRules(d, meta, rules, nrs)
-			// Create a single rule
-			err := resourceCloudStackEgressFirewallCreateRule(d, meta, rule.(map[string]interface{}))
 
 		// We need to update this first to preserve the correct state
-			rules.Add(rule)
 		d.Set("rule", rules)
 
 		if err != nil {
 			return err
 		}
 	}
-	}
 
 	return resourceCloudStackEgressFirewallRead(d, meta)
 }
 
-func resourceCloudStackEgressFirewallCreateRule(
+func createEgressFirewallRules(
-	d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error {
+	d *schema.ResourceData,
+	meta interface{},
+	rules *schema.Set,
+	nrs *schema.Set) error {
+	var errs *multierror.Error
+
+	var wg sync.WaitGroup
+	wg.Add(nrs.Len())
+
+	sem := make(chan struct{}, 10)
+	for _, rule := range nrs.List() {
+		// Put in a tiny sleep here to avoid DoS'ing the API
+		time.Sleep(500 * time.Millisecond)
+
+		go func(rule map[string]interface{}) {
+			defer wg.Done()
+			sem <- struct{}{}
+
+			// Create a single rule
+			err := createEgressFirewallRule(d, meta, rule)
+
+			// If we have at least one UUID, we need to save the rule
+			if len(rule["uuids"].(map[string]interface{})) > 0 {
+				rules.Add(rule)
+			}
+
+			if err != nil {
+				errs = multierror.Append(errs, err)
+			}
+
+			<-sem
+		}(rule.(map[string]interface{}))
+	}
+
+	wg.Wait()
+
+	return errs.ErrorOrNil()
+}
+func createEgressFirewallRule(
+	d *schema.ResourceData,
+	meta interface{},
+	rule map[string]interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 	uuids := rule["uuids"].(map[string]interface{})
 
@@ -137,7 +176,7 @@ func resourceCloudStackEgressFirewallCreateRule(
 	p := cs.Firewall.NewCreateEgressFirewallRuleParams(d.Id(), rule["protocol"].(string))
 
 	// Set the CIDR list
-	p.SetCidrlist([]string{rule["source_cidr"].(string)})
+	p.SetCidrlist(retrieveCidrList(rule))
 
 	// If the protocol is ICMP set the needed ICMP parameters
 	if rule["protocol"].(string) == "icmp" {
@@ -157,14 +196,18 @@ func resourceCloudStackEgressFirewallCreateRule(
 		if ps := rule["ports"].(*schema.Set); ps.Len() > 0 {
 
 			// Create an empty schema.Set to hold all processed ports
-			ports := &schema.Set{
+			ports := &schema.Set{F: schema.HashString}
-				F: func(v interface{}) int {
+
-					return hashcode.String(v.(string))
+			// Define a regexp for parsing the port
-				},
+			re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`)
-			}
 
 			for _, port := range ps.List() {
-				re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`)
+				if _, ok := uuids[port.(string)]; ok {
+					ports.Add(port)
+					rule["ports"] = ports
+					continue
+				}
+
 				m := re.FindStringSubmatch(port.(string))
 
 				startPort, err := strconv.Atoi(m[1])
@@ -220,9 +263,7 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 	}
 
 	// Create an empty schema.Set to hold all rules
-	rules := &schema.Set{
+	rules := resourceCloudStackEgressFirewall().Schema["rule"].ZeroValue().(*schema.Set)
-		F: resourceCloudStackEgressFirewallRuleHash,
-	}
 
 	// Read all rules that are configured
 	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
@@ -247,10 +288,10 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 				delete(ruleMap, id.(string))
 
 				// Update the values
-				rule["source_cidr"] = r.Cidrlist
 				rule["protocol"] = r.Protocol
 				rule["icmp_type"] = r.Icmptype
 				rule["icmp_code"] = r.Icmpcode
+				setCidrList(rule, r.Cidrlist)
 				rules.Add(rule)
 			}
 
@@ -259,11 +300,7 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 				if ps := rule["ports"].(*schema.Set); ps.Len() > 0 {
 
 					// Create an empty schema.Set to hold all ports
-					ports := &schema.Set{
+					ports := &schema.Set{F: schema.HashString}
-						F: func(v interface{}) int {
-							return hashcode.String(v.(string))
-						},
-					}
 
 					// Loop through all ports and retrieve their info
 					for _, port := range ps.List() {
@@ -283,8 +320,8 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 						delete(ruleMap, id.(string))
 
 						// Update the values
-						rule["source_cidr"] = r.Cidrlist
 						rule["protocol"] = r.Protocol
+						setCidrList(rule, r.Cidrlist)
 						ports.Add(port)
 					}
 
@@ -302,9 +339,14 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 	managed := d.Get("managed").(bool)
 	if managed && len(ruleMap) > 0 {
 		for uuid := range ruleMap {
+			// We need to create and add a dummy value to a schema.Set as the
+			// cidr_list is a required field and thus needs a value
+			cidrs := &schema.Set{F: schema.HashString}
+			cidrs.Add(uuid)
+
 			// Make a dummy rule to hold the unknown UUID
 			rule := map[string]interface{}{
-				"source_cidr": uuid,
+				"cidr_list": uuid,
 				"protocol":  uuid,
 				"uuids":     map[string]interface{}{uuid: uuid},
 			}
@@ -335,27 +377,29 @@ func resourceCloudStackEgressFirewallUpdate(d *schema.ResourceData, meta interfa
 		ors := o.(*schema.Set).Difference(n.(*schema.Set))
 		nrs := n.(*schema.Set).Difference(o.(*schema.Set))
 
-		// Now first loop through all the old rules and delete any obsolete ones
+		// We need to start with a rule set containing all the rules we
-		for _, rule := range ors.List() {
+		// already have and want to keep. Any rules that are not deleted
-			// Delete the rule as it no longer exists in the config
+		// correctly and any newly created rules, will be added to this
-			err := resourceCloudStackEgressFirewallDeleteRule(d, meta, rule.(map[string]interface{}))
+		// set to make sure we end up in a consistent state
+		rules := o.(*schema.Set).Intersection(n.(*schema.Set))
+
+		// First loop through all the old rules and delete them
+		if ors.Len() > 0 {
+			err := deleteEgressFirewallRules(d, meta, rules, ors)
+
+			// We need to update this first to preserve the correct state
+			d.Set("rule", rules)
+
 			if err != nil {
 				return err
 			}
 		}
 
-		// Make sure we save the state of the currently configured rules
+		// Then loop through all the new rules and create them
-		rules := o.(*schema.Set).Intersection(n.(*schema.Set))
+		if nrs.Len() > 0 {
-		d.Set("rule", rules)
+			err := createEgressFirewallRules(d, meta, rules, nrs)
-
-		// Then loop through all the currently configured rules and create the new ones
-		for _, rule := range nrs.List() {
-			// When successfully deleted, re-create it again if it still exists
-			err := resourceCloudStackEgressFirewallCreateRule(
-				d, meta, rule.(map[string]interface{}))
 
 			// We need to update this first to preserve the correct state
-			rules.Add(rule)
 			d.Set("rule", rules)
 
 			if err != nil {
@@ -368,26 +412,69 @@ func resourceCloudStackEgressFirewallUpdate(d *schema.ResourceData, meta interfa
 }
 
 func resourceCloudStackEgressFirewallDelete(d *schema.ResourceData, meta interface{}) error {
+	// Create an empty rule set to hold all rules that where
+	// not deleted correctly
+	rules := resourceCloudStackEgressFirewall().Schema["rule"].ZeroValue().(*schema.Set)
+
 	// Delete all rules
-	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
+	if ors := d.Get("rule").(*schema.Set); ors.Len() > 0 {
-		for _, rule := range rs.List() {
+		err := deleteEgressFirewallRules(d, meta, rules, ors)
-			// Delete a single rule
-			err := resourceCloudStackEgressFirewallDeleteRule(d, meta, rule.(map[string]interface{}))
 
 		// We need to update this first to preserve the correct state
-			d.Set("rule", rs)
+		d.Set("rule", rules)
 
 		if err != nil {
 			return err
 		}
 	}
-	}
 
 	return nil
 }
 
-func resourceCloudStackEgressFirewallDeleteRule(
+func deleteEgressFirewallRules(
-	d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error {
+	d *schema.ResourceData,
+	meta interface{},
+	rules *schema.Set,
+	ors *schema.Set) error {
+	var errs *multierror.Error
+
+	var wg sync.WaitGroup
+	wg.Add(ors.Len())
+
+	sem := make(chan struct{}, 10)
+	for _, rule := range ors.List() {
+		// Put a sleep here to avoid DoS'ing the API
+		time.Sleep(500 * time.Millisecond)
+
+		go func(rule map[string]interface{}) {
+			defer wg.Done()
+			sem <- struct{}{}
+
+			// Delete a single rule
+			err := deleteEgressFirewallRule(d, meta, rule)
+
+			// If we have at least one UUID, we need to save the rule
+			if len(rule["uuids"].(map[string]interface{})) > 0 {
+				rules.Add(rule)
+			}
+
+			if err != nil {
+				errs = multierror.Append(errs, err)
+			}
+
+			<-sem
+		}(rule.(map[string]interface{}))
+	}
+
+	wg.Wait()
+
+	return errs.ErrorOrNil()
+}
+
+func deleteEgressFirewallRule(
+	d *schema.ResourceData,
+	meta interface{},
+	rule map[string]interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 	uuids := rule["uuids"].(map[string]interface{})
 
@@ -416,47 +503,12 @@ func resourceCloudStackEgressFirewallDeleteRule(
 
 		// Delete the UUID of this rule
 		delete(uuids, k)
-	}
-
-	// Update the UUIDs
 		rule["uuids"] = uuids
+	}
 
 	return nil
 }
 
-func resourceCloudStackEgressFirewallRuleHash(v interface{}) int {
-	var buf bytes.Buffer
-	m := v.(map[string]interface{})
-	buf.WriteString(fmt.Sprintf(
-		"%s-%s-", m["source_cidr"].(string), m["protocol"].(string)))
-
-	if v, ok := m["icmp_type"]; ok {
-		buf.WriteString(fmt.Sprintf("%d-", v.(int)))
-	}
-
-	if v, ok := m["icmp_code"]; ok {
-		buf.WriteString(fmt.Sprintf("%d-", v.(int)))
-	}
-
-	// We need to make sure to sort the strings below so that we always
-	// generate the same hash code no matter what is in the set.
-	if v, ok := m["ports"]; ok {
-		vs := v.(*schema.Set).List()
-		s := make([]string, len(vs))
-
-		for i, raw := range vs {
-			s[i] = raw.(string)
-		}
-		sort.Strings(s)
-
-		for _, v := range s {
-			buf.WriteString(fmt.Sprintf("%s-", v))
-		}
-	}
-
-	return hashcode.String(buf.String())
-}
-
 func verifyEgressFirewallParams(d *schema.ResourceData) error {
 	managed := d.Get("managed").(bool)
 	_, rules := d.GetOk("rule")
@@ -470,6 +522,17 @@ func verifyEgressFirewallParams(d *schema.ResourceData) error {
 }
 
 func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{}) error {
+	cidrList := rule["cidr_list"].(*schema.Set)
+	sourceCidr := rule["source_cidr"].(string)
+	if cidrList.Len() == 0 && sourceCidr == "" {
+		return fmt.Errorf(
+			"Parameter cidr_list is a required parameter")
+	}
+	if cidrList.Len() > 0 && sourceCidr != "" {
+		return fmt.Errorf(
+			"Parameter source_cidr is deprecated and cannot be used together with cidr_list")
+	}
+
 	protocol := rule["protocol"].(string)
 	if protocol != "tcp" && protocol != "udp" && protocol != "icmp" {
 		return fmt.Errorf(

builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go

@@ -2,19 +2,15 @@ package cloudstack
 
 import (
 	"fmt"
-	"strconv"
 	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/resource"
-	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/xanzy/go-cloudstack/cloudstack"
 )
 
 func TestAccCloudStackEgressFirewall_basic(t *testing.T) {
-	hash := makeTestCloudStackEgressFirewallRuleHash([]interface{}{"1000-2000", "80"})
-
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
@@ -26,18 +22,26 @@ func TestAccCloudStackEgressFirewall_basic(t *testing.T) {
 					testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_egress_firewall.foo", "network", CLOUDSTACK_NETWORK_1),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.#", "2"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_egress_firewall.foo",
-						"rule."+hash+".source_cidr",
+						"rule.1081385056.cidr_list.3378711023",
-						CLOUDSTACK_NETWORK_1_IPADDRESS+"/32"),
+						CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash+".protocol", "tcp"),
+						"cloudstack_egress_firewall.foo", "rule.1081385056.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash+".ports.#", "2"),
+						"cloudstack_egress_firewall.foo", "rule.1081385056.ports.32925333", "8080"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash+".ports.1209010669", "1000-2000"),
+						"cloudstack_egress_firewall.foo",
+						"rule.1129999216.source_cidr",
+						CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash+".ports.1889509032", "80"),
+						"cloudstack_egress_firewall.foo", "rule.1129999216.protocol", "tcp"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1209010669", "1000-2000"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1889509032", "80"),
 				),
 			},
 		},
@@ -45,9 +49,6 @@ func TestAccCloudStackEgressFirewall_basic(t *testing.T) {
 }
 
 func TestAccCloudStackEgressFirewall_update(t *testing.T) {
-	hash1 := makeTestCloudStackEgressFirewallRuleHash([]interface{}{"1000-2000", "80"})
-	hash2 := makeTestCloudStackEgressFirewallRuleHash([]interface{}{"443"})
-
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
@@ -60,19 +61,25 @@ func TestAccCloudStackEgressFirewall_update(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_egress_firewall.foo", "network", CLOUDSTACK_NETWORK_1),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule.#", "1"),
+						"cloudstack_egress_firewall.foo", "rule.#", "2"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_egress_firewall.foo",
-						"rule."+hash1+".source_cidr",
+						"rule.1081385056.cidr_list.3378711023",
-						CLOUDSTACK_NETWORK_1_IPADDRESS+"/32"),
+						CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash1+".protocol", "tcp"),
+						"cloudstack_egress_firewall.foo", "rule.1081385056.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash1+".ports.#", "2"),
+						"cloudstack_egress_firewall.foo", "rule.1081385056.ports.32925333", "8080"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash1+".ports.1209010669", "1000-2000"),
+						"cloudstack_egress_firewall.foo",
+						"rule.1129999216.source_cidr",
+						CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash1+".ports.1889509032", "80"),
+						"cloudstack_egress_firewall.foo", "rule.1129999216.protocol", "tcp"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1209010669", "1000-2000"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1889509032", "80"),
 				),
 			},
 
@@ -83,29 +90,37 @@ func TestAccCloudStackEgressFirewall_update(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_egress_firewall.foo", "network", CLOUDSTACK_NETWORK_1),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule.#", "2"),
+						"cloudstack_egress_firewall.foo", "rule.#", "3"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_egress_firewall.foo",
-						"rule."+hash1+".source_cidr",
+						"rule.59731059.cidr_list.1910468234",
-						CLOUDSTACK_NETWORK_1_IPADDRESS+"/32"),
+						CLOUDSTACK_NETWORK_1_IPADDRESS2+"/32"),
-					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash1+".protocol", "tcp"),
-					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash1+".ports.#", "2"),
-					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash1+".ports.1209010669", "1000-2000"),
-					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash1+".ports.1889509032", "80"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_egress_firewall.foo",
-						"rule."+hash2+".source_cidr",
+						"rule.59731059.cidr_list.3378711023",
-						CLOUDSTACK_NETWORK_1_IPADDRESS+"/32"),
+						CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash2+".protocol", "tcp"),
+						"cloudstack_egress_firewall.foo", "rule.59731059.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash2+".ports.#", "1"),
+						"cloudstack_egress_firewall.foo", "rule.59731059.ports.32925333", "8080"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_egress_firewall.foo", "rule."+hash2+".ports.3638101695", "443"),
+						"cloudstack_egress_firewall.foo",
+						"rule.1052669680.source_cidr",
+						CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.1052669680.protocol", "tcp"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.1052669680.ports.3638101695", "443"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo",
+						"rule.1129999216.source_cidr",
+						CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.1129999216.protocol", "tcp"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1209010669", "1000-2000"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1889509032", "80"),
 				),
 			},
 		},
@@ -171,20 +186,16 @@ func testAccCheckCloudStackEgressFirewallDestroy(s *terraform.State) error {
 	return nil
 }
 
-func makeTestCloudStackEgressFirewallRuleHash(ports []interface{}) string {
-	return strconv.Itoa(resourceCloudStackEgressFirewallRuleHash(map[string]interface{}{
-		"source_cidr": CLOUDSTACK_NETWORK_1_IPADDRESS + "/32",
-		"protocol":    "tcp",
-		"ports":       schema.NewSet(schema.HashString, ports),
-		"icmp_type":   0,
-		"icmp_code":   0,
-	}))
-}
-
 var testAccCloudStackEgressFirewall_basic = fmt.Sprintf(`
 resource "cloudstack_egress_firewall" "foo" {
   network = "%s"
 
+  rule {
+    cidr_list = ["%s/32"]
+    protocol = "tcp"
+    ports = ["8080"]
+  }
+
   rule {
     source_cidr = "%s/32"
     protocol = "tcp"
@@ -192,12 +203,19 @@ resource "cloudstack_egress_firewall" "foo" {
   }
 }`,
 	CLOUDSTACK_NETWORK_1,
-	CLOUDSTACK_NETWORK_1_IPADDRESS)
+	CLOUDSTACK_NETWORK_1_IPADDRESS1,
+	CLOUDSTACK_NETWORK_1_IPADDRESS1)
 
 var testAccCloudStackEgressFirewall_update = fmt.Sprintf(`
 resource "cloudstack_egress_firewall" "foo" {
   network = "%s"
 
+  rule {
+    cidr_list = ["%s/32", "%s/32"]
+    protocol = "tcp"
+    ports = ["8080"]
+  }
+
   rule {
     source_cidr = "%s/32"
     protocol = "tcp"
@@ -211,5 +229,7 @@ resource "cloudstack_egress_firewall" "foo" {
   }
 }`,
 	CLOUDSTACK_NETWORK_1,
-	CLOUDSTACK_NETWORK_1_IPADDRESS,
+	CLOUDSTACK_NETWORK_1_IPADDRESS1,
-	CLOUDSTACK_NETWORK_1_IPADDRESS)
+	CLOUDSTACK_NETWORK_1_IPADDRESS2,
+	CLOUDSTACK_NETWORK_1_IPADDRESS1,
+	CLOUDSTACK_NETWORK_1_IPADDRESS1)

builtin/providers/cloudstack/resource_cloudstack_firewall.go

@@ -1,14 +1,14 @@
 package cloudstack
 
 import (
-	"bytes"
 	"fmt"
 	"regexp"
-	"sort"
 	"strconv"
 	"strings"
+	"sync"
+	"time"
 
-	"github.com/hashicorp/terraform/helper/hashcode"
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/xanzy/go-cloudstack/cloudstack"
 )
@@ -38,9 +38,17 @@ func resourceCloudStackFirewall() *schema.Resource {
 				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
+						"cidr_list": &schema.Schema{
+							Type:     schema.TypeSet,
+							Optional: true,
+							Elem:     &schema.Schema{Type: schema.TypeString},
+							Set:      schema.HashString,
+						},
+
 						"source_cidr": &schema.Schema{
 							Type:       schema.TypeString,
-							Required: true,
+							Optional:   true,
+							Deprecated: "Please use the `cidr_list` field instead",
 						},
 
 						"protocol": &schema.Schema{
@@ -64,9 +72,7 @@ func resourceCloudStackFirewall() *schema.Resource {
 							Type:     schema.TypeSet,
 							Optional: true,
 							Elem:     &schema.Schema{Type: schema.TypeString},
-							Set: func(v interface{}) int {
+							Set:      schema.HashString,
-								return hashcode.String(v.(string))
-							},
 						},
 
 						"uuids": &schema.Schema{
@@ -75,7 +81,6 @@ func resourceCloudStackFirewall() *schema.Resource {
 						},
 					},
 				},
-				Set: resourceCloudStackFirewallRuleHash,
 			},
 		},
 	}
@@ -99,32 +104,66 @@ func resourceCloudStackFirewallCreate(d *schema.ResourceData, meta interface{})
 	d.SetId(ipaddressid)
 
 	// Create all rules that are configured
-	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
+	if nrs := d.Get("rule").(*schema.Set); nrs.Len() > 0 {
-
 		// Create an empty schema.Set to hold all rules
-		rules := &schema.Set{
+		rules := resourceCloudStackFirewall().Schema["rule"].ZeroValue().(*schema.Set)
-			F: resourceCloudStackFirewallRuleHash,
-		}
 
-		for _, rule := range rs.List() {
+		err := createFirewallRules(d, meta, rules, nrs)
-			// Create a single rule
-			err := resourceCloudStackFirewallCreateRule(d, meta, rule.(map[string]interface{}))
 
 		// We need to update this first to preserve the correct state
-			rules.Add(rule)
 		d.Set("rule", rules)
 
 		if err != nil {
 			return err
 		}
 	}
-	}
 
 	return resourceCloudStackFirewallRead(d, meta)
 }
+func createFirewallRules(
+	d *schema.ResourceData,
+	meta interface{},
+	rules *schema.Set,
+	nrs *schema.Set) error {
+	var errs *multierror.Error
 
-func resourceCloudStackFirewallCreateRule(
+	var wg sync.WaitGroup
-	d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error {
+	wg.Add(nrs.Len())
+
+	sem := make(chan struct{}, 10)
+	for _, rule := range nrs.List() {
+		// Put in a tiny sleep here to avoid DoS'ing the API
+		time.Sleep(500 * time.Millisecond)
+
+		go func(rule map[string]interface{}) {
+			defer wg.Done()
+			sem <- struct{}{}
+
+			// Create a single rule
+			err := createFirewallRule(d, meta, rule)
+
+			// If we have at least one UUID, we need to save the rule
+			if len(rule["uuids"].(map[string]interface{})) > 0 {
+				rules.Add(rule)
+			}
+
+			if err != nil {
+				errs = multierror.Append(errs, err)
+			}
+
+			<-sem
+		}(rule.(map[string]interface{}))
+	}
+
+	wg.Wait()
+
+	return errs.ErrorOrNil()
+}
+
+func createFirewallRule(
+	d *schema.ResourceData,
+	meta interface{},
+	rule map[string]interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 	uuids := rule["uuids"].(map[string]interface{})
 
@@ -137,7 +176,7 @@ func resourceCloudStackFirewallCreateRule(
 	p := cs.Firewall.NewCreateFirewallRuleParams(d.Id(), rule["protocol"].(string))
 
 	// Set the CIDR list
-	p.SetCidrlist([]string{rule["source_cidr"].(string)})
+	p.SetCidrlist(retrieveCidrList(rule))
 
 	// If the protocol is ICMP set the needed ICMP parameters
 	if rule["protocol"].(string) == "icmp" {
@@ -148,6 +187,7 @@ func resourceCloudStackFirewallCreateRule(
 		if err != nil {
 			return err
 		}
+
 		uuids["icmp"] = r.Id
 		rule["uuids"] = uuids
 	}
@@ -157,14 +197,18 @@ func resourceCloudStackFirewallCreateRule(
 		if ps := rule["ports"].(*schema.Set); ps.Len() > 0 {
 
 			// Create an empty schema.Set to hold all processed ports
-			ports := &schema.Set{
+			ports := &schema.Set{F: schema.HashString}
-				F: func(v interface{}) int {
+
-					return hashcode.String(v.(string))
+			// Define a regexp for parsing the port
-				},
+			re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`)
-			}
 
 			for _, port := range ps.List() {
-				re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`)
+				if _, ok := uuids[port.(string)]; ok {
+					ports.Add(port)
+					rule["ports"] = ports
+					continue
+				}
+
 				m := re.FindStringSubmatch(port.(string))
 
 				startPort, err := strconv.Atoi(m[1])
@@ -220,9 +264,7 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 	}
 
 	// Create an empty schema.Set to hold all rules
-	rules := &schema.Set{
+	rules := resourceCloudStackFirewall().Schema["rule"].ZeroValue().(*schema.Set)
-		F: resourceCloudStackFirewallRuleHash,
-	}
 
 	// Read all rules that are configured
 	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
@@ -247,10 +289,10 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 				delete(ruleMap, id.(string))
 
 				// Update the values
-				rule["source_cidr"] = r.Cidrlist
 				rule["protocol"] = r.Protocol
 				rule["icmp_type"] = r.Icmptype
 				rule["icmp_code"] = r.Icmpcode
+				setCidrList(rule, r.Cidrlist)
 				rules.Add(rule)
 			}
 
@@ -259,11 +301,7 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 				if ps := rule["ports"].(*schema.Set); ps.Len() > 0 {
 
 					// Create an empty schema.Set to hold all ports
-					ports := &schema.Set{
+					ports := &schema.Set{F: schema.HashString}
-						F: func(v interface{}) int {
-							return hashcode.String(v.(string))
-						},
-					}
 
 					// Loop through all ports and retrieve their info
 					for _, port := range ps.List() {
@@ -283,8 +321,8 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 						delete(ruleMap, id.(string))
 
 						// Update the values
-						rule["source_cidr"] = r.Cidrlist
 						rule["protocol"] = r.Protocol
+						setCidrList(rule, r.Cidrlist)
 						ports.Add(port)
 					}
 
@@ -302,9 +340,14 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 	managed := d.Get("managed").(bool)
 	if managed && len(ruleMap) > 0 {
 		for uuid := range ruleMap {
+			// We need to create and add a dummy value to a schema.Set as the
+			// cidr_list is a required field and thus needs a value
+			cidrs := &schema.Set{F: schema.HashString}
+			cidrs.Add(uuid)
+
 			// Make a dummy rule to hold the unknown UUID
 			rule := map[string]interface{}{
-				"source_cidr": uuid,
+				"cidr_list": cidrs,
 				"protocol":  uuid,
 				"uuids":     map[string]interface{}{uuid: uuid},
 			}
@@ -335,27 +378,29 @@ func resourceCloudStackFirewallUpdate(d *schema.ResourceData, meta interface{})
 		ors := o.(*schema.Set).Difference(n.(*schema.Set))
 		nrs := n.(*schema.Set).Difference(o.(*schema.Set))
 
-		// Now first loop through all the old rules and delete any obsolete ones
+		// We need to start with a rule set containing all the rules we
-		for _, rule := range ors.List() {
+		// already have and want to keep. Any rules that are not deleted
-			// Delete the rule as it no longer exists in the config
+		// correctly and any newly created rules, will be added to this
-			err := resourceCloudStackFirewallDeleteRule(d, meta, rule.(map[string]interface{}))
+		// set to make sure we end up in a consistent state
+		rules := o.(*schema.Set).Intersection(n.(*schema.Set))
+
+		// First loop through all the old rules and delete them
+		if ors.Len() > 0 {
+			err := deleteFirewallRules(d, meta, rules, ors)
+
+			// We need to update this first to preserve the correct state
+			d.Set("rule", rules)
+
 			if err != nil {
 				return err
 			}
 		}
 
-		// Make sure we save the state of the currently configured rules
+		// Then loop through all the new rules and create them
-		rules := o.(*schema.Set).Intersection(n.(*schema.Set))
+		if nrs.Len() > 0 {
-		d.Set("rule", rules)
+			err := createFirewallRules(d, meta, rules, nrs)
-
-		// Then loop through all the currently configured rules and create the new ones
-		for _, rule := range nrs.List() {
-			// When successfully deleted, re-create it again if it still exists
-			err := resourceCloudStackFirewallCreateRule(
-				d, meta, rule.(map[string]interface{}))
 
 			// We need to update this first to preserve the correct state
-			rules.Add(rule)
 			d.Set("rule", rules)
 
 			if err != nil {
@@ -368,26 +413,69 @@ func resourceCloudStackFirewallUpdate(d *schema.ResourceData, meta interface{})
 }
 
 func resourceCloudStackFirewallDelete(d *schema.ResourceData, meta interface{}) error {
+	// Create an empty rule set to hold all rules that where
+	// not deleted correctly
+	rules := resourceCloudStackFirewall().Schema["rule"].ZeroValue().(*schema.Set)
+
 	// Delete all rules
-	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
+	if ors := d.Get("rule").(*schema.Set); ors.Len() > 0 {
-		for _, rule := range rs.List() {
+		err := deleteFirewallRules(d, meta, rules, ors)
-			// Delete a single rule
-			err := resourceCloudStackFirewallDeleteRule(d, meta, rule.(map[string]interface{}))
 
 		// We need to update this first to preserve the correct state
-			d.Set("rule", rs)
+		d.Set("rule", rules)
 
 		if err != nil {
 			return err
 		}
 	}
-	}
 
 	return nil
 }
 
-func resourceCloudStackFirewallDeleteRule(
+func deleteFirewallRules(
-	d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error {
+	d *schema.ResourceData,
+	meta interface{},
+	rules *schema.Set,
+	ors *schema.Set) error {
+	var errs *multierror.Error
+
+	var wg sync.WaitGroup
+	wg.Add(ors.Len())
+
+	sem := make(chan struct{}, 10)
+	for _, rule := range ors.List() {
+		// Put a sleep here to avoid DoS'ing the API
+		time.Sleep(500 * time.Millisecond)
+
+		go func(rule map[string]interface{}) {
+			defer wg.Done()
+			sem <- struct{}{}
+
+			// Delete a single rule
+			err := deleteFirewallRule(d, meta, rule)
+
+			// If we have at least one UUID, we need to save the rule
+			if len(rule["uuids"].(map[string]interface{})) > 0 {
+				rules.Add(rule)
+			}
+
+			if err != nil {
+				errs = multierror.Append(errs, err)
+			}
+
+			<-sem
+		}(rule.(map[string]interface{}))
+	}
+
+	wg.Wait()
+
+	return errs.ErrorOrNil()
+}
+
+func deleteFirewallRule(
+	d *schema.ResourceData,
+	meta interface{},
+	rule map[string]interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 	uuids := rule["uuids"].(map[string]interface{})
 
@@ -416,47 +504,12 @@ func resourceCloudStackFirewallDeleteRule(
 
 		// Delete the UUID of this rule
 		delete(uuids, k)
-	}
-
-	// Update the UUIDs
 		rule["uuids"] = uuids
+	}
 
 	return nil
 }
 
-func resourceCloudStackFirewallRuleHash(v interface{}) int {
-	var buf bytes.Buffer
-	m := v.(map[string]interface{})
-	buf.WriteString(fmt.Sprintf(
-		"%s-%s-", m["source_cidr"].(string), m["protocol"].(string)))
-
-	if v, ok := m["icmp_type"]; ok {
-		buf.WriteString(fmt.Sprintf("%d-", v.(int)))
-	}
-
-	if v, ok := m["icmp_code"]; ok {
-		buf.WriteString(fmt.Sprintf("%d-", v.(int)))
-	}
-
-	// We need to make sure to sort the strings below so that we always
-	// generate the same hash code no matter what is in the set.
-	if v, ok := m["ports"]; ok {
-		vs := v.(*schema.Set).List()
-		s := make([]string, len(vs))
-
-		for i, raw := range vs {
-			s[i] = raw.(string)
-		}
-		sort.Strings(s)
-
-		for _, v := range s {
-			buf.WriteString(fmt.Sprintf("%s-", v))
-		}
-	}
-
-	return hashcode.String(buf.String())
-}
-
 func verifyFirewallParams(d *schema.ResourceData) error {
 	managed := d.Get("managed").(bool)
 	_, rules := d.GetOk("rule")
@@ -470,6 +523,17 @@ func verifyFirewallParams(d *schema.ResourceData) error {
 }
 
 func verifyFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{}) error {
+	cidrList := rule["cidr_list"].(*schema.Set)
+	sourceCidr := rule["source_cidr"].(string)
+	if cidrList.Len() == 0 && sourceCidr == "" {
+		return fmt.Errorf(
+			"Parameter cidr_list is a required parameter")
+	}
+	if cidrList.Len() > 0 && sourceCidr != "" {
+		return fmt.Errorf(
+			"Parameter source_cidr is deprecated and cannot be used together with cidr_list")
+	}
+
 	protocol := rule["protocol"].(string)
 	if protocol != "tcp" && protocol != "udp" && protocol != "icmp" {
 		return fmt.Errorf(

builtin/providers/cloudstack/resource_cloudstack_firewall_test.go

@@ -23,15 +23,21 @@ func TestAccCloudStackFirewall_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_firewall.foo", "ipaddress", CLOUDSTACK_PUBLIC_IPADDRESS),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.source_cidr", "10.0.0.0/24"),
+						"cloudstack_firewall.foo", "rule.#", "2"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.protocol", "tcp"),
+						"cloudstack_firewall.foo", "rule.60926170.cidr_list.3482919157", "10.0.0.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.ports.#", "2"),
+						"cloudstack_firewall.foo", "rule.60926170.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.ports.1209010669", "1000-2000"),
+						"cloudstack_firewall.foo", "rule.60926170.ports.32925333", "8080"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.ports.1889509032", "80"),
+						"cloudstack_firewall.foo", "rule.716592205.source_cidr", "10.0.0.0/24"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_firewall.foo", "rule.716592205.protocol", "tcp"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_firewall.foo", "rule.716592205.ports.1209010669", "1000-2000"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_firewall.foo", "rule.716592205.ports.1889509032", "80"),
 				),
 			},
 		},
@@ -51,17 +57,21 @@ func TestAccCloudStackFirewall_update(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_firewall.foo", "ipaddress", CLOUDSTACK_PUBLIC_IPADDRESS),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.#", "1"),
+						"cloudstack_firewall.foo", "rule.#", "2"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.source_cidr", "10.0.0.0/24"),
+						"cloudstack_firewall.foo", "rule.60926170.cidr_list.3482919157", "10.0.0.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.protocol", "tcp"),
+						"cloudstack_firewall.foo", "rule.60926170.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.ports.#", "2"),
+						"cloudstack_firewall.foo", "rule.60926170.ports.32925333", "8080"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.ports.1209010669", "1000-2000"),
+						"cloudstack_firewall.foo", "rule.716592205.source_cidr", "10.0.0.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.ports.1889509032", "80"),
+						"cloudstack_firewall.foo", "rule.716592205.protocol", "tcp"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_firewall.foo", "rule.716592205.ports.1209010669", "1000-2000"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_firewall.foo", "rule.716592205.ports.1889509032", "80"),
 				),
 			},
 
@@ -72,27 +82,31 @@ func TestAccCloudStackFirewall_update(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_firewall.foo", "ipaddress", CLOUDSTACK_PUBLIC_IPADDRESS),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.#", "2"),
+						"cloudstack_firewall.foo", "rule.#", "3"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.source_cidr", "10.0.0.0/24"),
+						"cloudstack_firewall.foo", "rule.2207610982.cidr_list.80081744", "10.0.1.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.protocol", "tcp"),
+						"cloudstack_firewall.foo", "rule.2207610982.cidr_list.3482919157", "10.0.0.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.ports.#", "2"),
+						"cloudstack_firewall.foo", "rule.2207610982.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.ports.1209010669", "1000-2000"),
+						"cloudstack_firewall.foo", "rule.2207610982.ports.32925333", "8080"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.1702320581.ports.1889509032", "80"),
+						"cloudstack_firewall.foo", "rule.716592205.source_cidr", "10.0.0.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.3779782959.source_cidr", "172.16.100.0/24"),
+						"cloudstack_firewall.foo", "rule.716592205.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.3779782959.protocol", "tcp"),
+						"cloudstack_firewall.foo", "rule.716592205.ports.1209010669", "1000-2000"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.3779782959.ports.#", "2"),
+						"cloudstack_firewall.foo", "rule.716592205.ports.1889509032", "80"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.3779782959.ports.1889509032", "80"),
+						"cloudstack_firewall.foo", "rule.4449157.source_cidr", "172.16.100.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_firewall.foo", "rule.3779782959.ports.3638101695", "443"),
+						"cloudstack_firewall.foo", "rule.4449157.protocol", "tcp"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_firewall.foo", "rule.4449157.ports.1889509032", "80"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_firewall.foo", "rule.4449157.ports.3638101695", "443"),
 				),
 			},
 		},
@@ -162,6 +176,12 @@ var testAccCloudStackFirewall_basic = fmt.Sprintf(`
 resource "cloudstack_firewall" "foo" {
   ipaddress = "%s"
 
+  rule {
+    cidr_list = ["10.0.0.0/24"]
+    protocol = "tcp"
+    ports = ["8080"]
+  }
+
   rule {
     source_cidr = "10.0.0.0/24"
     protocol = "tcp"
@@ -173,6 +193,12 @@ var testAccCloudStackFirewall_update = fmt.Sprintf(`
 resource "cloudstack_firewall" "foo" {
   ipaddress = "%s"
 
+  rule {
+    cidr_list = ["10.0.0.0/24", "10.0.1.0/24"]
+    protocol = "tcp"
+    ports = ["8080"]
+  }
+
   rule {
     source_cidr = "10.0.0.0/24"
     protocol = "tcp"

builtin/providers/cloudstack/resource_cloudstack_instance_test.go

@@ -80,7 +80,7 @@ func TestAccCloudStackInstance_fixedIP(t *testing.T) {
 					testAccCheckCloudStackInstanceExists(
 						"cloudstack_instance.foobar", &instance),
 					resource.TestCheckResourceAttr(
-						"cloudstack_instance.foobar", "ipaddress", CLOUDSTACK_NETWORK_1_IPADDRESS),
+						"cloudstack_instance.foobar", "ipaddress", CLOUDSTACK_NETWORK_1_IPADDRESS1),
 				),
 			},
 		},
@@ -268,7 +268,7 @@ resource "cloudstack_instance" "foobar" {
 }`,
 	CLOUDSTACK_SERVICE_OFFERING_1,
 	CLOUDSTACK_NETWORK_1,
-	CLOUDSTACK_NETWORK_1_IPADDRESS,
+	CLOUDSTACK_NETWORK_1_IPADDRESS1,
 	CLOUDSTACK_TEMPLATE,
 	CLOUDSTACK_ZONE)
 
@@ -290,7 +290,7 @@ resource "cloudstack_instance" "foobar" {
 }`,
 	CLOUDSTACK_SERVICE_OFFERING_1,
 	CLOUDSTACK_NETWORK_1,
-	CLOUDSTACK_NETWORK_1_IPADDRESS,
+	CLOUDSTACK_NETWORK_1_IPADDRESS1,
 	CLOUDSTACK_TEMPLATE,
 	CLOUDSTACK_ZONE)
 

builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go

@@ -1,17 +1,14 @@
 package cloudstack
 
 import (
-	"bytes"
 	"fmt"
 	"regexp"
-	"sort"
 	"strconv"
 	"strings"
 	"sync"
 	"time"
 
 	"github.com/hashicorp/go-multierror"
-	"github.com/hashicorp/terraform/helper/hashcode"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/xanzy/go-cloudstack/cloudstack"
 )
@@ -47,9 +44,17 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 							Default:  "allow",
 						},
 
+						"cidr_list": &schema.Schema{
+							Type:     schema.TypeSet,
+							Optional: true,
+							Elem:     &schema.Schema{Type: schema.TypeString},
+							Set:      schema.HashString,
+						},
+
 						"source_cidr": &schema.Schema{
 							Type:       schema.TypeString,
-							Required: true,
+							Optional:   true,
+							Deprecated: "Please use the `cidr_list` field instead",
 						},
 
 						"protocol": &schema.Schema{
@@ -73,9 +78,7 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 							Type:     schema.TypeSet,
 							Optional: true,
 							Elem:     &schema.Schema{Type: schema.TypeString},
-							Set: func(v interface{}) int {
+							Set:      schema.HashString,
-								return hashcode.String(v.(string))
-							},
 						},
 
 						"traffic_type": &schema.Schema{
@@ -90,7 +93,6 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 						},
 					},
 				},
-				Set: resourceCloudStackNetworkACLRuleHash,
 			},
 		},
 	}
@@ -108,11 +110,9 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa
 	// Create all rules that are configured
 	if nrs := d.Get("rule").(*schema.Set); nrs.Len() > 0 {
 		// Create an empty rule set to hold all newly created rules
-		rules := &schema.Set{
+		rules := resourceCloudStackNetworkACLRule().Schema["rule"].ZeroValue().(*schema.Set)
-			F: resourceCloudStackNetworkACLRuleHash,
-		}
 
-		err := resourceCloudStackNetworkACLRuleCreateRules(d, meta, rules, nrs)
+		err := createNetworkACLRules(d, meta, rules, nrs)
 
 		// We need to update this first to preserve the correct state
 		d.Set("rule", rules)
@@ -125,7 +125,7 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa
 	return resourceCloudStackNetworkACLRuleRead(d, meta)
 }
 
-func resourceCloudStackNetworkACLRuleCreateRules(
+func createNetworkACLRules(
 	d *schema.ResourceData,
 	meta interface{},
 	rules *schema.Set,
@@ -145,7 +145,7 @@ func resourceCloudStackNetworkACLRuleCreateRules(
 			sem <- struct{}{}
 
 			// Create a single rule
-			err := resourceCloudStackNetworkACLRuleCreateRule(d, meta, rule)
+			err := createNetworkACLRule(d, meta, rule)
 
 			// If we have at least one UUID, we need to save the rule
 			if len(rule["uuids"].(map[string]interface{})) > 0 {
@@ -162,13 +162,10 @@ func resourceCloudStackNetworkACLRuleCreateRules(
 
 	wg.Wait()
 
-	// We need to update this first to preserve the correct state
-	d.Set("rule", rules)
-
 	return errs.ErrorOrNil()
 }
 
-func resourceCloudStackNetworkACLRuleCreateRule(
+func createNetworkACLRule(
 	d *schema.ResourceData,
 	meta interface{},
 	rule map[string]interface{}) error {
@@ -190,7 +187,7 @@ func resourceCloudStackNetworkACLRuleCreateRule(
 	p.SetAction(rule["action"].(string))
 
 	// Set the CIDR list
-	p.SetCidrlist([]string{rule["source_cidr"].(string)})
+	p.SetCidrlist(retrieveCidrList(rule))
 
 	// Set the traffic type
 	p.SetTraffictype(rule["traffic_type"].(string))
@@ -225,11 +222,7 @@ func resourceCloudStackNetworkACLRuleCreateRule(
 		if ps := rule["ports"].(*schema.Set); ps.Len() > 0 {
 
 			// Create an empty schema.Set to hold all processed ports
-			ports := &schema.Set{
+			ports := &schema.Set{F: schema.HashString}
-				F: func(v interface{}) int {
-					return hashcode.String(v.(string))
-				},
-			}
 
 			// Define a regexp for parsing the port
 			re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`)
@@ -296,9 +289,7 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 	}
 
 	// Create an empty schema.Set to hold all rules
-	rules := &schema.Set{
+	rules := resourceCloudStackNetworkACLRule().Schema["rule"].ZeroValue().(*schema.Set)
-		F: resourceCloudStackNetworkACLRuleHash,
-	}
 
 	// Read all rules that are configured
 	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
@@ -324,11 +315,11 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 
 				// Update the values
 				rule["action"] = strings.ToLower(r.Action)
-				rule["source_cidr"] = r.Cidrlist
 				rule["protocol"] = r.Protocol
 				rule["icmp_type"] = r.Icmptype
 				rule["icmp_code"] = r.Icmpcode
 				rule["traffic_type"] = strings.ToLower(r.Traffictype)
+				setCidrList(rule, r.Cidrlist)
 				rules.Add(rule)
 			}
 
@@ -350,9 +341,9 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 
 				// Update the values
 				rule["action"] = strings.ToLower(r.Action)
-				rule["source_cidr"] = r.Cidrlist
 				rule["protocol"] = r.Protocol
 				rule["traffic_type"] = strings.ToLower(r.Traffictype)
+				setCidrList(rule, r.Cidrlist)
 				rules.Add(rule)
 			}
 
@@ -361,11 +352,7 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 				if ps := rule["ports"].(*schema.Set); ps.Len() > 0 {
 
 					// Create an empty schema.Set to hold all ports
-					ports := &schema.Set{
+					ports := &schema.Set{F: schema.HashString}
-						F: func(v interface{}) int {
-							return hashcode.String(v.(string))
-						},
-					}
 
 					// Loop through all ports and retrieve their info
 					for _, port := range ps.List() {
@@ -386,9 +373,9 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 
 						// Update the values
 						rule["action"] = strings.ToLower(r.Action)
-						rule["source_cidr"] = r.Cidrlist
 						rule["protocol"] = r.Protocol
 						rule["traffic_type"] = strings.ToLower(r.Traffictype)
+						setCidrList(rule, r.Cidrlist)
 						ports.Add(port)
 					}
 
@@ -402,13 +389,18 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 		}
 	}
 
-	// If this is a managed firewall, add all unknown rules into a single dummy rule
+	// If this is a managed firewall, add all unknown rules into dummy rules
 	managed := d.Get("managed").(bool)
 	if managed && len(ruleMap) > 0 {
 		for uuid := range ruleMap {
+			// We need to create and add a dummy value to a schema.Set as the
+			// cidr_list is a required field and thus needs a value
+			cidrs := &schema.Set{F: schema.HashString}
+			cidrs.Add(uuid)
+
 			// Make a dummy rule to hold the unknown UUID
 			rule := map[string]interface{}{
-				"source_cidr": uuid,
+				"cidr_list": cidrs,
 				"protocol":  uuid,
 				"uuids":     map[string]interface{}{uuid: uuid},
 			}
@@ -445,9 +437,9 @@ func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interfa
 		// set to make sure we end up in a consistent state
 		rules := o.(*schema.Set).Intersection(n.(*schema.Set))
 
-		// Now first loop through all the old rules and delete them
+		// First loop through all the new rules and create (before destroy) them
-		if ors.Len() > 0 {
+		if nrs.Len() > 0 {
-			err := resourceCloudStackNetworkACLRuleDeleteRules(d, meta, rules, ors)
+			err := createNetworkACLRules(d, meta, rules, nrs)
 
 			// We need to update this first to preserve the correct state
 			d.Set("rule", rules)
@@ -457,9 +449,9 @@ func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interfa
 			}
 		}
 
-		// Then loop through all the new rules and create them
+		// Then loop through all the old rules and delete them
-		if nrs.Len() > 0 {
+		if ors.Len() > 0 {
-			err := resourceCloudStackNetworkACLRuleCreateRules(d, meta, rules, nrs)
+			err := deleteNetworkACLRules(d, meta, rules, ors)
 
 			// We need to update this first to preserve the correct state
 			d.Set("rule", rules)
@@ -476,13 +468,11 @@ func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interfa
 func resourceCloudStackNetworkACLRuleDelete(d *schema.ResourceData, meta interface{}) error {
 	// Create an empty rule set to hold all rules that where
 	// not deleted correctly
-	rules := &schema.Set{
+	rules := resourceCloudStackNetworkACLRule().Schema["rule"].ZeroValue().(*schema.Set)
-		F: resourceCloudStackNetworkACLRuleHash,
-	}
 
 	// Delete all rules
-	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
+	if ors := d.Get("rule").(*schema.Set); ors.Len() > 0 {
-		err := resourceCloudStackNetworkACLRuleDeleteRules(d, meta, rules, rs)
+		err := deleteNetworkACLRules(d, meta, rules, ors)
 
 		// We need to update this first to preserve the correct state
 		d.Set("rule", rules)
@@ -495,7 +485,7 @@ func resourceCloudStackNetworkACLRuleDelete(d *schema.ResourceData, meta interfa
 	return nil
 }
 
-func resourceCloudStackNetworkACLRuleDeleteRules(
+func deleteNetworkACLRules(
 	d *schema.ResourceData,
 	meta interface{},
 	rules *schema.Set,
@@ -515,7 +505,7 @@ func resourceCloudStackNetworkACLRuleDeleteRules(
 			sem <- struct{}{}
 
 			// Delete a single rule
-			err := resourceCloudStackNetworkACLRuleDeleteRule(d, meta, rule)
+			err := deleteNetworkACLRule(d, meta, rule)
 
 			// If we have at least one UUID, we need to save the rule
 			if len(rule["uuids"].(map[string]interface{})) > 0 {
@@ -535,7 +525,7 @@ func resourceCloudStackNetworkACLRuleDeleteRules(
 	return errs.ErrorOrNil()
 }
 
-func resourceCloudStackNetworkACLRuleDeleteRule(
+func deleteNetworkACLRule(
 	d *schema.ResourceData,
 	meta interface{},
 	rule map[string]interface{}) error {
@@ -574,58 +564,6 @@ func resourceCloudStackNetworkACLRuleDeleteRule(
 	return nil
 }
 
-func resourceCloudStackNetworkACLRuleHash(v interface{}) int {
-	var buf bytes.Buffer
-	m := v.(map[string]interface{})
-
-	// This is a little ugly, but it's needed because these arguments have
-	// a default value that needs to be part of the string to hash
-	var action, trafficType string
-	if a, ok := m["action"]; ok {
-		action = a.(string)
-	} else {
-		action = "allow"
-	}
-	if t, ok := m["traffic_type"]; ok {
-		trafficType = t.(string)
-	} else {
-		trafficType = "ingress"
-	}
-
-	buf.WriteString(fmt.Sprintf(
-		"%s-%s-%s-%s-",
-		action,
-		m["source_cidr"].(string),
-		m["protocol"].(string),
-		trafficType))
-
-	if v, ok := m["icmp_type"]; ok {
-		buf.WriteString(fmt.Sprintf("%d-", v.(int)))
-	}
-
-	if v, ok := m["icmp_code"]; ok {
-		buf.WriteString(fmt.Sprintf("%d-", v.(int)))
-	}
-
-	// We need to make sure to sort the strings below so that we always
-	// generate the same hash code no matter what is in the set.
-	if v, ok := m["ports"]; ok {
-		vs := v.(*schema.Set).List()
-		s := make([]string, len(vs))
-
-		for i, raw := range vs {
-			s[i] = raw.(string)
-		}
-		sort.Strings(s)
-
-		for _, v := range s {
-			buf.WriteString(fmt.Sprintf("%s-", v))
-		}
-	}
-
-	return hashcode.String(buf.String())
-}
-
 func verifyNetworkACLParams(d *schema.ResourceData) error {
 	managed := d.Get("managed").(bool)
 	_, rules := d.GetOk("rule")
@@ -644,6 +582,17 @@ func verifyNetworkACLRuleParams(d *schema.ResourceData, rule map[string]interfac
 		return fmt.Errorf("Parameter action only accepts 'allow' or 'deny' as values")
 	}
 
+	cidrList := rule["cidr_list"].(*schema.Set)
+	sourceCidr := rule["source_cidr"].(string)
+	if cidrList.Len() == 0 && sourceCidr == "" {
+		return fmt.Errorf(
+			"Parameter cidr_list is a required parameter")
+	}
+	if cidrList.Len() > 0 && sourceCidr != "" {
+		return fmt.Errorf(
+			"Parameter source_cidr is deprecated and cannot be used together with cidr_list")
+	}
+
 	protocol := rule["protocol"].(string)
 	switch protocol {
 	case "icmp":

builtin/providers/cloudstack/resource_cloudstack_network_acl_rule_test.go

@@ -23,19 +23,31 @@ func TestAccCloudStackNetworkACLRule_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_network_acl_rule.foo", "rule.#", "3"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.action", "allow"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.action", "allow"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.source_cidr", "172.16.100.0/24"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.source_cidr", "172.16.100.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.protocol", "tcp"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.ports.#", "2"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.ports.#", "2"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.ports.1889509032", "80"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.ports.1889509032", "80"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.ports.3638101695", "443"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.ports.3638101695", "443"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.traffic_type", "ingress"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.traffic_type", "ingress"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.action", "allow"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.cidr_list.#", "1"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.cidr_list.3056857544", "172.18.100.0/24"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.icmp_code", "-1"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.icmp_type", "-1"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.traffic_type", "ingress"),
 				),
 			},
 		},
@@ -55,19 +67,31 @@ func TestAccCloudStackNetworkACLRule_update(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_network_acl_rule.foo", "rule.#", "3"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.action", "allow"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.action", "allow"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.source_cidr", "172.16.100.0/24"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.source_cidr", "172.16.100.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.protocol", "tcp"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.ports.#", "2"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.ports.#", "2"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.ports.1889509032", "80"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.ports.1889509032", "80"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.ports.3638101695", "443"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.ports.3638101695", "443"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.traffic_type", "ingress"),
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.traffic_type", "ingress"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.action", "allow"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.cidr_list.#", "1"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.cidr_list.3056857544", "172.18.100.0/24"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.icmp_code", "-1"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.icmp_type", "-1"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.4029966697.traffic_type", "ingress"),
 				),
 			},
 
@@ -78,33 +102,47 @@ func TestAccCloudStackNetworkACLRule_update(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_network_acl_rule.foo", "rule.#", "4"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.action", "allow"),
+						"cloudstack_network_acl_rule.foo", "rule.2254982534.action", "deny"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.source_cidr", "172.16.100.0/24"),
+						"cloudstack_network_acl_rule.foo", "rule.2254982534.source_cidr", "10.0.0.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.protocol", "tcp"),
+						"cloudstack_network_acl_rule.foo", "rule.2254982534.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.ports.#", "2"),
+						"cloudstack_network_acl_rule.foo", "rule.2254982534.ports.#", "2"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.ports.1889509032", "80"),
+						"cloudstack_network_acl_rule.foo", "rule.2254982534.ports.1209010669", "1000-2000"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.ports.3638101695", "443"),
+						"cloudstack_network_acl_rule.foo", "rule.2254982534.ports.1889509032", "80"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.3247834462.traffic_type", "ingress"),
+						"cloudstack_network_acl_rule.foo", "rule.2254982534.traffic_type", "egress"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.4267872693.action", "deny"),
+						"cloudstack_network_acl_rule.foo", "rule.2704020556.action", "deny"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.4267872693.source_cidr", "10.0.0.0/24"),
+						"cloudstack_network_acl_rule.foo", "rule.2704020556.cidr_list.#", "2"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.4267872693.protocol", "tcp"),
+						"cloudstack_network_acl_rule.foo", "rule.2704020556.cidr_list.2104435309", "172.18.101.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.4267872693.ports.#", "2"),
+						"cloudstack_network_acl_rule.foo", "rule.2704020556.cidr_list.3056857544", "172.18.100.0/24"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.4267872693.ports.1209010669", "1000-2000"),
+						"cloudstack_network_acl_rule.foo", "rule.2704020556.icmp_code", "-1"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.4267872693.ports.1889509032", "80"),
+						"cloudstack_network_acl_rule.foo", "rule.2704020556.icmp_type", "-1"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_network_acl_rule.foo", "rule.4267872693.traffic_type", "egress"),
+						"cloudstack_network_acl_rule.foo", "rule.2704020556.traffic_type", "ingress"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.action", "allow"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.source_cidr", "172.16.100.0/24"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.protocol", "tcp"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.ports.#", "2"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.ports.1889509032", "80"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.ports.3638101695", "443"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.2792403380.traffic_type", "ingress"),
 				),
 			},
 		},
@@ -196,7 +234,7 @@ resource "cloudstack_network_acl_rule" "foo" {
 
   rule {
   	action = "allow"
-    source_cidr = "172.18.100.0/24"
+    cidr_list = ["172.18.100.0/24"]
     protocol = "icmp"
     icmp_type = "-1"
     icmp_code = "-1"
@@ -240,7 +278,7 @@ resource "cloudstack_network_acl_rule" "foo" {
 
   rule {
   	action = "deny"
-    source_cidr = "172.18.100.0/24"
+		cidr_list = ["172.18.100.0/24", "172.18.101.0/24"]
     protocol = "icmp"
     icmp_type = "-1"
     icmp_code = "-1"

builtin/providers/cloudstack/resource_cloudstack_port_forward.go

@@ -1,13 +1,14 @@
 package cloudstack
 
 import (
-	"bytes"
 	"fmt"
+	"sync"
+	"time"
 
 	"strconv"
 	"strings"
 
-	"github.com/hashicorp/terraform/helper/hashcode"
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/xanzy/go-cloudstack/cloudstack"
 )
@@ -63,7 +64,6 @@ func resourceCloudStackPortForward() *schema.Resource {
 						},
 					},
 				},
-				Set: resourceCloudStackPortForwardHash,
 			},
 		},
 	}
@@ -82,32 +82,66 @@ func resourceCloudStackPortForwardCreate(d *schema.ResourceData, meta interface{
 	d.SetId(ipaddressid)
 
 	// Create all forwards that are configured
-	if rs := d.Get("forward").(*schema.Set); rs.Len() > 0 {
+	if nrs := d.Get("forward").(*schema.Set); nrs.Len() > 0 {
-
 		// Create an empty schema.Set to hold all forwards
-		forwards := &schema.Set{
+		forwards := resourceCloudStackPortForward().Schema["forward"].ZeroValue().(*schema.Set)
-			F: resourceCloudStackPortForwardHash,
-		}
 
-		for _, forward := range rs.List() {
+		err := createPortForwards(d, meta, forwards, nrs)
-			// Create a single forward
-			err := resourceCloudStackPortForwardCreateForward(d, meta, forward.(map[string]interface{}))
 
 		// We need to update this first to preserve the correct state
-			forwards.Add(forward)
 		d.Set("forward", forwards)
 
 		if err != nil {
 			return err
 		}
 	}
-	}
 
 	return resourceCloudStackPortForwardRead(d, meta)
 }
 
-func resourceCloudStackPortForwardCreateForward(
+func createPortForwards(
-	d *schema.ResourceData, meta interface{}, forward map[string]interface{}) error {
+	d *schema.ResourceData,
+	meta interface{},
+	forwards *schema.Set,
+	nrs *schema.Set) error {
+	var errs *multierror.Error
+
+	var wg sync.WaitGroup
+	wg.Add(nrs.Len())
+
+	sem := make(chan struct{}, 10)
+	for _, forward := range nrs.List() {
+		// Put in a tiny sleep here to avoid DoS'ing the API
+		time.Sleep(500 * time.Millisecond)
+
+		go func(forward map[string]interface{}) {
+			defer wg.Done()
+			sem <- struct{}{}
+
+			// Create a single forward
+			err := createPortForward(d, meta, forward)
+
+			// If we have a UUID, we need to save the forward
+			if forward["uuid"].(string) != "" {
+				forwards.Add(forward)
+			}
+
+			if err != nil {
+				errs = multierror.Append(errs, err)
+			}
+
+			<-sem
+		}(forward.(map[string]interface{}))
+	}
+
+	wg.Wait()
+
+	return errs.ErrorOrNil()
+}
+func createPortForward(
+	d *schema.ResourceData,
+	meta interface{},
+	forward map[string]interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 
 	// Make sure all required parameters are there
@@ -167,9 +201,7 @@ func resourceCloudStackPortForwardRead(d *schema.ResourceData, meta interface{})
 	}
 
 	// Create an empty schema.Set to hold all forwards
-	forwards := &schema.Set{
+	forwards := resourceCloudStackPortForward().Schema["forward"].ZeroValue().(*schema.Set)
-		F: resourceCloudStackPortForwardHash,
-	}
 
 	// Read all forwards that are configured
 	if rs := d.Get("forward").(*schema.Set); rs.Len() > 0 {
@@ -250,26 +282,29 @@ func resourceCloudStackPortForwardUpdate(d *schema.ResourceData, meta interface{
 		ors := o.(*schema.Set).Difference(n.(*schema.Set))
 		nrs := n.(*schema.Set).Difference(o.(*schema.Set))
 
-		// Now first loop through all the old forwards and delete any obsolete ones
+		// We need to start with a rule set containing all the rules we
-		for _, forward := range ors.List() {
+		// already have and want to keep. Any rules that are not deleted
-			// Delete the forward as it no longer exists in the config
+		// correctly and any newly created rules, will be added to this
-			err := resourceCloudStackPortForwardDeleteForward(d, meta, forward.(map[string]interface{}))
+		// set to make sure we end up in a consistent state
+		forwards := o.(*schema.Set).Intersection(n.(*schema.Set))
+
+		// First loop through all the new forwards and create (before destroy) them
+		if nrs.Len() > 0 {
+			err := createPortForwards(d, meta, forwards, nrs)
+
+			// We need to update this first to preserve the correct state
+			d.Set("forward", forwards)
+
 			if err != nil {
 				return err
 			}
 		}
 
-		// Make sure we save the state of the currently configured forwards
+		// Then loop through all the old forwards and delete them
-		forwards := o.(*schema.Set).Intersection(n.(*schema.Set))
+		if ors.Len() > 0 {
-		d.Set("forward", forwards)
+			err := deletePortForwards(d, meta, forwards, ors)
-
-		// Then loop through all the currently configured forwards and create the new ones
-		for _, forward := range nrs.List() {
-			err := resourceCloudStackPortForwardCreateForward(
-				d, meta, forward.(map[string]interface{}))
 
 			// We need to update this first to preserve the correct state
-			forwards.Add(forward)
 			d.Set("forward", forwards)
 
 			if err != nil {
@@ -282,26 +317,69 @@ func resourceCloudStackPortForwardUpdate(d *schema.ResourceData, meta interface{
 }
 
 func resourceCloudStackPortForwardDelete(d *schema.ResourceData, meta interface{}) error {
+	// Create an empty rule set to hold all rules that where
+	// not deleted correctly
+	forwards := resourceCloudStackPortForward().Schema["forward"].ZeroValue().(*schema.Set)
+
 	// Delete all forwards
-	if rs := d.Get("forward").(*schema.Set); rs.Len() > 0 {
+	if ors := d.Get("forward").(*schema.Set); ors.Len() > 0 {
-		for _, forward := range rs.List() {
+		err := deletePortForwards(d, meta, forwards, ors)
-			// Delete a single forward
-			err := resourceCloudStackPortForwardDeleteForward(d, meta, forward.(map[string]interface{}))
 
 		// We need to update this first to preserve the correct state
-			d.Set("forward", rs)
+		d.Set("forward", forwards)
 
 		if err != nil {
 			return err
 		}
 	}
-	}
 
 	return nil
 }
 
-func resourceCloudStackPortForwardDeleteForward(
+func deletePortForwards(
-	d *schema.ResourceData, meta interface{}, forward map[string]interface{}) error {
+	d *schema.ResourceData,
+	meta interface{},
+	forwards *schema.Set,
+	ors *schema.Set) error {
+	var errs *multierror.Error
+
+	var wg sync.WaitGroup
+	wg.Add(ors.Len())
+
+	sem := make(chan struct{}, 10)
+	for _, forward := range ors.List() {
+		// Put a sleep here to avoid DoS'ing the API
+		time.Sleep(500 * time.Millisecond)
+
+		go func(forward map[string]interface{}) {
+			defer wg.Done()
+			sem <- struct{}{}
+
+			// Delete a single forward
+			err := deletePortForward(d, meta, forward)
+
+			// If we have a UUID, we need to save the forward
+			if forward["uuid"].(string) != "" {
+				forwards.Add(forward)
+			}
+
+			if err != nil {
+				errs = multierror.Append(errs, err)
+			}
+
+			<-sem
+		}(forward.(map[string]interface{}))
+	}
+
+	wg.Wait()
+
+	return errs.ErrorOrNil()
+}
+
+func deletePortForward(
+	d *schema.ResourceData,
+	meta interface{},
+	forward map[string]interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 
 	// Create the parameter struct
@@ -323,19 +401,6 @@ func resourceCloudStackPortForwardDeleteForward(
 	return nil
 }
 
-func resourceCloudStackPortForwardHash(v interface{}) int {
-	var buf bytes.Buffer
-	m := v.(map[string]interface{})
-	buf.WriteString(fmt.Sprintf(
-		"%s-%d-%d-%s",
-		m["protocol"].(string),
-		m["private_port"].(int),
-		m["public_port"].(int),
-		m["virtual_machine"].(string)))
-
-	return hashcode.String(buf.String())
-}
-
 func verifyPortForwardParams(d *schema.ResourceData, forward map[string]interface{}) error {
 	protocol := forward["protocol"].(string)
 	if protocol != "tcp" && protocol != "udp" {

builtin/providers/cloudstack/resource_cloudstack_port_forward_test.go

@@ -23,13 +23,13 @@ func TestAccCloudStackPortForward_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_port_forward.foo", "ipaddress", CLOUDSTACK_PUBLIC_IPADDRESS),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.protocol", "tcp"),
+						"cloudstack_port_forward.foo", "forward.952396423.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.private_port", "443"),
+						"cloudstack_port_forward.foo", "forward.952396423.private_port", "443"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.public_port", "8443"),
+						"cloudstack_port_forward.foo", "forward.952396423.public_port", "8443"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.virtual_machine", "terraform-test"),
+						"cloudstack_port_forward.foo", "forward.952396423.virtual_machine", "terraform-test"),
 				),
 			},
 		},
@@ -51,13 +51,13 @@ func TestAccCloudStackPortForward_update(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_port_forward.foo", "forward.#", "1"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.protocol", "tcp"),
+						"cloudstack_port_forward.foo", "forward.952396423.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.private_port", "443"),
+						"cloudstack_port_forward.foo", "forward.952396423.private_port", "443"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.public_port", "8443"),
+						"cloudstack_port_forward.foo", "forward.952396423.public_port", "8443"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.virtual_machine", "terraform-test"),
+						"cloudstack_port_forward.foo", "forward.952396423.virtual_machine", "terraform-test"),
 				),
 			},
 
@@ -70,21 +70,21 @@ func TestAccCloudStackPortForward_update(t *testing.T) {
 					resource.TestCheckResourceAttr(
 						"cloudstack_port_forward.foo", "forward.#", "2"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.8416686.protocol", "tcp"),
+						"cloudstack_port_forward.foo", "forward.260687715.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.8416686.private_port", "80"),
+						"cloudstack_port_forward.foo", "forward.260687715.private_port", "80"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.8416686.public_port", "8080"),
+						"cloudstack_port_forward.foo", "forward.260687715.public_port", "8080"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.8416686.virtual_machine", "terraform-test"),
+						"cloudstack_port_forward.foo", "forward.260687715.virtual_machine", "terraform-test"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.protocol", "tcp"),
+						"cloudstack_port_forward.foo", "forward.952396423.protocol", "tcp"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.private_port", "443"),
+						"cloudstack_port_forward.foo", "forward.952396423.private_port", "443"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.public_port", "8443"),
+						"cloudstack_port_forward.foo", "forward.952396423.public_port", "8443"),
 					resource.TestCheckResourceAttr(
-						"cloudstack_port_forward.foo", "forward.1537694805.virtual_machine", "terraform-test"),
+						"cloudstack_port_forward.foo", "forward.952396423.virtual_machine", "terraform-test"),
 				),
 			},
 		},

builtin/providers/cloudstack/resource_cloudstack_secondary_ipaddress_test.go

@@ -43,7 +43,7 @@ func TestAccCloudStackSecondaryIPAddress_fixedIP(t *testing.T) {
 						"cloudstack_secondary_ipaddress.foo", &ip),
 					testAccCheckCloudStackSecondaryIPAddressAttributes(&ip),
 					resource.TestCheckResourceAttr(
-						"cloudstack_secondary_ipaddress.foo", "ipaddress", CLOUDSTACK_NETWORK_1_IPADDRESS),
+						"cloudstack_secondary_ipaddress.foo", "ipaddress", CLOUDSTACK_NETWORK_1_IPADDRESS1),
 				),
 			},
 		},
@@ -117,7 +117,7 @@ func testAccCheckCloudStackSecondaryIPAddressAttributes(
 	ip *cloudstack.AddIpToNicResponse) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 
-		if ip.Ipaddress != CLOUDSTACK_NETWORK_1_IPADDRESS {
+		if ip.Ipaddress != CLOUDSTACK_NETWORK_1_IPADDRESS1 {
 			return fmt.Errorf("Bad IP address: %s", ip.Ipaddress)
 		}
 		return nil
@@ -222,4 +222,4 @@ resource "cloudstack_secondary_ipaddress" "foo" {
 	CLOUDSTACK_NETWORK_1,
 	CLOUDSTACK_TEMPLATE,
 	CLOUDSTACK_ZONE,
-	CLOUDSTACK_NETWORK_1_IPADDRESS)
+	CLOUDSTACK_NETWORK_1_IPADDRESS1)

builtin/providers/cloudstack/resources.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"log"
 	"regexp"
+	"strings"
 	"time"
 
 	"github.com/hashicorp/terraform/helper/schema"
@@ -145,3 +146,36 @@ func Retry(n int, f RetryFunc) (interface{}, error) {
 
 	return nil, lastErr
 }
+
+// This is a temporary helper function to support both the new
+// cidr_list and the deprecated source_cidr parameter
+func retrieveCidrList(rule map[string]interface{}) []string {
+	sourceCidr := rule["source_cidr"].(string)
+	if sourceCidr != "" {
+		return []string{sourceCidr}
+	}
+
+	var cidrList []string
+	for _, cidr := range rule["cidr_list"].(*schema.Set).List() {
+		cidrList = append(cidrList, cidr.(string))
+	}
+
+	return cidrList
+}
+
+// This is a temporary helper function to support both the new
+// cidr_list and the deprecated source_cidr parameter
+func setCidrList(rule map[string]interface{}, cidrList string) {
+	sourceCidr := rule["source_cidr"].(string)
+	if sourceCidr != "" {
+		rule["source_cidr"] = cidrList
+		return
+	}
+
+	cidrs := &schema.Set{F: schema.HashString}
+	for _, cidr := range strings.Split(cidrList, ",") {
+		cidrs.Add(cidr)
+	}
+
+	rule["cidr_list"] = cidrs
+}

website/source/docs/providers/cloudstack/r/egress_firewall.html.markdown

@@ -17,7 +17,7 @@ resource "cloudstack_egress_firewall" "default" {
   network = "test-network"
 
   rule {
-    source_cidr = "10.0.0.0/8"
+    cidr_list = ["10.0.0.0/8"]
     protocol = "tcp"
     ports = ["80", "1000-2000"]
   }
@@ -40,7 +40,10 @@ The following arguments are supported:
 
 The `rule` block supports:
 
-* `source_cidr` - (Required) The source CIDR to allow access to the given ports.
+* `cidr_list` - (Required) A CIDR list to allow access to the given ports.
+
+* `source_cidr` - (Optional, Deprecated) The source CIDR to allow access to the
+    given ports. This attribute is deprecated, please use `cidr_list` instead.
 
 * `protocol` - (Required) The name of the protocol to allow. Valid options are:
     `tcp`, `udp` and `icmp`.

website/source/docs/providers/cloudstack/r/firewall.html.markdown

@@ -17,7 +17,7 @@ resource "cloudstack_firewall" "default" {
   ipaddress = "192.168.0.1"
 
   rule {
-    source_cidr = "10.0.0.0/8"
+    cidr_list = ["10.0.0.0/8"]
     protocol = "tcp"
     ports = ["80", "1000-2000"]
   }
@@ -40,7 +40,10 @@ The following arguments are supported:
 
 The `rule` block supports:
 
-* `source_cidr` - (Required) The source CIDR to allow access to the given ports.
+* `cidr_list` - (Required) A CIDR list to allow access to the given ports.
+
+* `source_cidr` - (Optional, Deprecated) The source CIDR to allow access to the
+    given ports. This attribute is deprecated, please use `cidr_list` instead.
 
 * `protocol` - (Required) The name of the protocol to allow. Valid options are:
     `tcp`, `udp` and `icmp`.

website/source/docs/providers/cloudstack/r/network_acl_rule.html.markdown

@@ -18,7 +18,7 @@ resource "cloudstack_network_acl_rule" "default" {
 
   rule {
     action = "allow"
-    source_cidr = "10.0.0.0/8"
+    cidr_list = ["10.0.0.0/8"]
     protocol = "tcp"
     ports = ["80", "1000-2000"]
     traffic_type = "ingress"
@@ -45,7 +45,10 @@ The `rule` block supports:
 * `action` - (Optional) The action for the rule. Valid options are: `allow` and
     `deny` (defaults allow).
 
-* `source_cidr` - (Required) The source CIDR to allow access to the given ports.
+* `cidr_list` - (Required) A CIDR list to allow access to the given ports.
+
+* `source_cidr` - (Optional, Deprecated) The source CIDR to allow access to the
+    given ports. This attribute is deprecated, please use `cidr_list` instead.
 
 * `protocol` - (Required) The name of the protocol to allow. Valid options are:
     `tcp`, `udp`, `icmp`, `all` or a valid protocol number.