Fix ruleset field change detection using TypeSet for cidr_list

- Changed cidr_list from TypeList to TypeSet in ruleset schema to match
  other CloudStack resources (security_group_rule, firewall, egress_firewall)
- Removed custom resourceCloudStackNetworkACLRulesetHash function in favor
  of default schema.HashResource for consistency
- TypeSet for cidr_list provides automatic ordering independence, eliminating
  need for manual CIDR sorting in hash function
- Updated all code paths to handle cidr_list as *schema.Set for ruleset
  and []interface{} for legacy rule field
- Since ruleset is a new feature, no migration path needed

This ensures that changes to cidr, port, protocol, action, ICMP fields,
and traffic_type are properly detected when using ruleset blocks.

Tests:
- All 18 ACL tests pass
- Comprehensive field change test validates CIDR, port, action, and ICMP changes

Author: bhouse-nexthop

cloudstack/resource_cloudstack_network_acl_rule.go

@@ -20,7 +20,6 @@
 package cloudstack
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"log"
@@ -192,7 +191,6 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 				Type:          schema.TypeSet,
 				Optional:      true,
 				ConflictsWith: []string{"rule"},
-				Set:           resourceCloudStackNetworkACLRulesetHash,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"rule_number": {
@@ -207,9 +205,10 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 						},
 
 						"cidr_list": {
-							Type:     schema.TypeList,
+							Type:     schema.TypeSet,
 							Required: true,
 							Elem:     &schema.Schema{Type: schema.TypeString},
+							Set:      schema.HashString,
 						},
 
 						"protocol": {
@@ -272,21 +271,6 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 	}
 }
 
-// resourceCloudStackNetworkACLRulesetHash computes the hash for a ruleset element
-// Only the rule_number is used for hashing since it uniquely identifies a rule
-// This prevents spurious diffs when fields with defaults are populated in state but not in config
-func resourceCloudStackNetworkACLRulesetHash(v interface{}) int {
-	var buf bytes.Buffer
-	m := v.(map[string]interface{})
-
-	// Only hash on rule_number since it's the unique identifier
-	if v, ok := m["rule_number"]; ok {
-		buf.WriteString(fmt.Sprintf("%d-", v.(int)))
-	}
-
-	return schema.HashString(buf.String())
-}
-
 // Helper functions for UUID handling to abstract differences between
 // 'rule' (uses uuids map) and 'ruleset' (uses uuid string)
 
@@ -608,8 +592,15 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str
 
 	// Set the CIDR list
 	var cidrList []string
-	for _, cidr := range rule["cidr_list"].([]interface{}) {
-		cidrList = append(cidrList, cidr.(string))
+	if cidrSet, ok := rule["cidr_list"].(*schema.Set); ok {
+		for _, cidr := range cidrSet.List() {
+			cidrList = append(cidrList, cidr.(string))
+		}
+	} else {
+		// Fallback for 'rule' field which uses TypeList
+		for _, cidr := range rule["cidr_list"].([]interface{}) {
+			cidrList = append(cidrList, cidr.(string))
+		}
 	}
 	p.SetCidrlist(cidrList)
 	log.Printf("[DEBUG] Set cidr_list=%v", cidrList)
@@ -868,15 +859,26 @@ func processPortForRuleUnified(portKey string, rule map[string]interface{}, rule
 	// Delete the known rule so only unknown rules remain in the ruleMap
 	delete(ruleMap, id)
 
-	var cidrs []interface{}
-	for _, cidr := range strings.Split(r.Cidrlist, ",") {
-		cidrs = append(cidrs, cidr)
+	// Create a list or set with all CIDR's depending on field type
+	// Check if this is a ruleset rule (has uuid field) vs rule (has uuids field)
+	_, isRuleset := rule["uuid"]
+	if isRuleset {
+		cidrs := &schema.Set{F: schema.HashString}
+		for _, cidr := range strings.Split(r.Cidrlist, ",") {
+			cidrs.Add(cidr)
+		}
+		rule["cidr_list"] = cidrs
+	} else {
+		var cidrs []interface{}
+		for _, cidr := range strings.Split(r.Cidrlist, ",") {
+			cidrs = append(cidrs, cidr)
+		}
+		rule["cidr_list"] = cidrs
 	}
 
 	rule["action"] = strings.ToLower(r.Action)
 	rule["protocol"] = r.Protocol
 	rule["traffic_type"] = strings.ToLower(r.Traffictype)
-	rule["cidr_list"] = cidrs
 	rule["rule_number"] = r.Number
 	rule["description"] = r.Reason
 	// Set ICMP fields to 0 for non-ICMP protocols to avoid spurious diffs
@@ -978,10 +980,19 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 			// Delete the known rule so only unknown rules remain in the ruleMap
 			delete(ruleMap, id)
 
-			// Create a list with all CIDR's
-			var cidrs []interface{}
-			for _, cidr := range strings.Split(r.Cidrlist, ",") {
-				cidrs = append(cidrs, cidr)
+			// Create a list or set with all CIDR's depending on field type
+			if usingRuleset {
+				cidrs := &schema.Set{F: schema.HashString}
+				for _, cidr := range strings.Split(r.Cidrlist, ",") {
+					cidrs.Add(cidr)
+				}
+				rule["cidr_list"] = cidrs
+			} else {
+				var cidrs []interface{}
+				for _, cidr := range strings.Split(r.Cidrlist, ",") {
+					cidrs = append(cidrs, cidr)
+				}
+				rule["cidr_list"] = cidrs
 			}
 
 			// Update the values
@@ -990,7 +1001,6 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 			rule["icmp_type"] = r.Icmptype
 			rule["icmp_code"] = r.Icmpcode
 			rule["traffic_type"] = strings.ToLower(r.Traffictype)
-			rule["cidr_list"] = cidrs
 			rule["rule_number"] = r.Number
 			rule["description"] = r.Reason
 			if usingRuleset {
@@ -1017,17 +1027,25 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 			// Delete the known rule so only unknown rules remain in the ruleMap
 			delete(ruleMap, id)
 
-			// Create a list with all CIDR's
-			var cidrs []interface{}
-			for _, cidr := range strings.Split(r.Cidrlist, ",") {
-				cidrs = append(cidrs, cidr)
+			// Create a list or set with all CIDR's depending on field type
+			if usingRuleset {
+				cidrs := &schema.Set{F: schema.HashString}
+				for _, cidr := range strings.Split(r.Cidrlist, ",") {
+					cidrs.Add(cidr)
+				}
+				rule["cidr_list"] = cidrs
+			} else {
+				var cidrs []interface{}
+				for _, cidr := range strings.Split(r.Cidrlist, ",") {
+					cidrs = append(cidrs, cidr)
+				}
+				rule["cidr_list"] = cidrs
 			}
 
 			// Update the values
 			rule["action"] = strings.ToLower(r.Action)
 			rule["protocol"] = r.Protocol
 			rule["traffic_type"] = strings.ToLower(r.Traffictype)
-			rule["cidr_list"] = cidrs
 			rule["rule_number"] = r.Number
 			rule["description"] = r.Reason
 			delete(rule, "port") // Remove port field for "all" protocol
@@ -1063,15 +1081,15 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 		outOfBandRuleNumber := maxRuleNumber + 1
 
 		for uuid := range ruleMap {
-			// We need to create and add a placeholder value to a list as the
-			// cidr_list is a required field and thus needs a value
-			cidrs := []interface{}{uuid}
-
 			// Make a placeholder rule to hold the unknown UUID
 			// Format differs between 'rule' and 'ruleset'
 			var rule map[string]interface{}
 			if usingRuleset {
 				// For ruleset: use 'uuid' string and include rule_number
+				// cidr_list is a TypeSet for ruleset
+				cidrs := &schema.Set{F: schema.HashString}
+				cidrs.Add(uuid)
+
 				// Include all fields with defaults to avoid spurious diffs
 				rule = map[string]interface{}{
 					"cidr_list":    cidrs,
@@ -1088,6 +1106,8 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 				outOfBandRuleNumber++
 			} else {
 				// For rule: use 'uuids' map
+				// cidr_list is a TypeList for rule
+				cidrs := []interface{}{uuid}
 				rule = map[string]interface{}{
 					"cidr_list": cidrs,
 					"protocol":  uuid,
@@ -1107,8 +1127,9 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 		// For TypeSet, we need to be very careful about state updates
 		// The SDK has issues with properly clearing removed elements from TypeSet
 		// So we explicitly set to empty first, then set the new value
-		// Use the same hash function as defined in the schema to avoid spurious diffs
-		hashFunc := resourceCloudStackNetworkACLRulesetHash
+		// Use schema.HashResource to match the default hash function
+		rulesetResource := resourceCloudStackNetworkACLRule().Schema["ruleset"].Elem.(*schema.Resource)
+		hashFunc := schema.HashResource(rulesetResource)
 
 		// First, clear the ruleset completely
 		emptySet := schema.NewSet(hashFunc, []interface{}{})
@@ -1529,7 +1550,17 @@ func performNormalRuleUpdates(d *schema.ResourceData, meta interface{}, cs *clou
 
 			newRuleMap := newRule.(map[string]interface{})
 			log.Printf("[DEBUG] Comparing old rule %+v with new rule %+v", oldRuleMap, newRuleMap)
-			if rulesMatch(oldRuleMap, newRuleMap) {
+
+			// For ruleset rules, match by rule_number only
+			// For regular rules, use the full rulesMatch function
+			var matched bool
+			if isRulesetRule(oldRuleMap) && isRulesetRule(newRuleMap) {
+				matched = rulesetRulesMatchByNumber(oldRuleMap, newRuleMap)
+			} else {
+				matched = rulesMatch(oldRuleMap, newRuleMap)
+			}
+
+			if matched {
 				log.Printf("[DEBUG] Found matching new rule for old rule")
 
 				// Copy UUID from old rule to new rule
@@ -1609,6 +1640,20 @@ func performNormalRuleUpdates(d *schema.ResourceData, meta interface{}, cs *clou
 	return nil
 }
 
+// rulesetRulesMatchByNumber matches ruleset rules by rule_number only
+// This allows changes to other fields (CIDR, port, protocol, etc.) to be detected as updates
+func rulesetRulesMatchByNumber(oldRule, newRule map[string]interface{}) bool {
+	oldRuleNum, oldHasRuleNum := oldRule["rule_number"].(int)
+	newRuleNum, newHasRuleNum := newRule["rule_number"].(int)
+
+	// Both must have rule_number and they must match
+	if !oldHasRuleNum || !newHasRuleNum {
+		return false
+	}
+
+	return oldRuleNum == newRuleNum
+}
+
 func rulesMatch(oldRule, newRule map[string]interface{}) bool {
 	oldProtocol := oldRule["protocol"].(string)
 	newProtocol := newRule["protocol"].(string)
@@ -1725,20 +1770,34 @@ func ruleNeedsUpdate(oldRule, newRule map[string]interface{}) bool {
 		}
 	}
 
-	oldCidrs := oldRule["cidr_list"].([]interface{})
-	newCidrs := newRule["cidr_list"].([]interface{})
-	if len(oldCidrs) != len(newCidrs) {
-		log.Printf("[DEBUG] CIDR list length changed: %d -> %d", len(oldCidrs), len(newCidrs))
-		return true
+	// Handle cidr_list comparison - can be TypeSet (ruleset) or TypeList (rule)
+	var oldCidrStrs, newCidrStrs []string
+
+	// Extract old CIDRs
+	if oldSet, ok := oldRule["cidr_list"].(*schema.Set); ok {
+		for _, cidr := range oldSet.List() {
+			oldCidrStrs = append(oldCidrStrs, cidr.(string))
+		}
+	} else if oldList, ok := oldRule["cidr_list"].([]interface{}); ok {
+		for _, cidr := range oldList {
+			oldCidrStrs = append(oldCidrStrs, cidr.(string))
+		}
 	}
 
-	oldCidrStrs := make([]string, len(oldCidrs))
-	newCidrStrs := make([]string, len(newCidrs))
-	for i, cidr := range oldCidrs {
-		oldCidrStrs[i] = cidr.(string)
+	// Extract new CIDRs
+	if newSet, ok := newRule["cidr_list"].(*schema.Set); ok {
+		for _, cidr := range newSet.List() {
+			newCidrStrs = append(newCidrStrs, cidr.(string))
+		}
+	} else if newList, ok := newRule["cidr_list"].([]interface{}); ok {
+		for _, cidr := range newList {
+			newCidrStrs = append(newCidrStrs, cidr.(string))
+		}
 	}
-	for i, cidr := range newCidrs {
-		newCidrStrs[i] = cidr.(string)
+
+	if len(oldCidrStrs) != len(newCidrStrs) {
+		log.Printf("[DEBUG] CIDR list length changed: %d -> %d", len(oldCidrStrs), len(newCidrStrs))
+		return true
 	}
 
 	sort.Strings(oldCidrStrs)
@@ -1768,13 +1827,22 @@ func updateNetworkACLRule(cs *cloudstack.CloudStackClient, oldRule, newRule map[
 		p.SetAction(newRule["action"].(string))
 
 		var cidrList []string
-		for _, cidr := range newRule["cidr_list"].([]interface{}) {
-			cidrList = append(cidrList, cidr.(string))
+		if cidrSet, ok := newRule["cidr_list"].(*schema.Set); ok {
+			for _, cidr := range cidrSet.List() {
+				cidrList = append(cidrList, cidr.(string))
+			}
+		} else {
+			for _, cidr := range newRule["cidr_list"].([]interface{}) {
+				cidrList = append(cidrList, cidr.(string))
+			}
 		}
 		p.SetCidrlist(cidrList)
 
-		if desc, ok := newRule["description"].(string); ok && desc != "" {
+		// Set description from the new rule
+		if desc, ok := newRule["description"].(string); ok {
 			p.SetReason(desc)
+		} else {
+			p.SetReason("")
 		}
 
 		p.SetProtocol(newRule["protocol"].(string))