Fix ACL rule validation and port ordering issues

This commit addresses three code review comments:

1. Add validation for ruleset create path
   - The ruleset create path was skipping verifyNetworkACLRuleParams
     before calling createNetworkACLRules
   - Added validation loop to verify each ruleset element before creation
   - This prevents panics from unchecked type assertions in
     createNetworkACLRule

2. Add validation for all rule creation paths
   - createNetworkACLRule is called from Create and Update paths for
     both 'rule' and 'ruleset' fields
   - Added validation in Update path for both 'rule' and 'ruleset'
     before calling updateNetworkACLRules
   - Ensures all new rules are validated before creation, preventing
     less actionable CloudStack API errors

3. Sort ports for deterministic rule numbering
   - When expanding deprecated 'ports' field, iteration used
     portsSet.List() which is unordered
   - Rule numbers were derived as baseRuleNum + portIndex, causing
     non-deterministic numbering across runs
   - Added sort.Strings() to ensure stable, deterministic rule number
     assignment

Additionally, consolidated duplicate validation loops into a helper
function validateRulesList() to improve code maintainability and
reduce duplication across Create and Update paths. The helper
correctly skips validation for out-of-band rule placeholders
(created by managed=true) as they are markers for deletion.

All changes tested with full ACL acceptance test suite (15 tests):
- TestAccCloudStackNetworkACLRule_basic
- TestAccCloudStackNetworkACLRule_update
- TestAccCloudStackNetworkACLRule_ruleset_basic
- TestAccCloudStackNetworkACLRule_ruleset_update
- TestAccCloudStackNetworkACLRule_ruleset_insert
- TestAccCloudStackNetworkACLRule_ruleset_insert_plan_check
- TestAccCloudStackNetworkACLRule_ruleset_managed
- TestAccCloudStackNetworkACLRule_ruleset_not_managed
- TestAccCloudStackNetworkACLRule_rule_managed
- TestAccCloudStackNetworkACLRule_rule_not_managed
- TestAccCloudStackNetworkACLRule_icmp_fields_no_spurious_diff
- TestAccCloudStackNetworkACLRule_icmp_fields_add_remove_rule
- TestAccCloudStackNetworkACLRule_deprecated_ports
- TestAccCloudStackNetworkACLRule_deprecated_ports_managed
- TestAccCloudStackNetworkACLRule_deprecated_ports_not_managed

Author: bhouse-nexthop

cloudstack/resource_cloudstack_network_acl_rule.go

@@ -456,11 +456,8 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa
 		log.Printf("[DEBUG] Processing %d rules from 'rule' field", len(nrs))
 
 		// Validate rules BEFORE assigning numbers, so we can detect user-provided rule_number
-		for _, rule := range nrs {
-			if err := verifyNetworkACLRuleParams(d, rule.(map[string]interface{})); err != nil {
-				log.Printf("[ERROR] Failed to verify rule parameters: %v", err)
-				return err
-			}
+		if err := validateRulesList(d, nrs, "rule"); err != nil {
+			return err
 		}
 
 		// Assign rule numbers to rules that don't have them
@@ -490,6 +487,11 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa
 		// Convert Set to list (no auto-numbering needed, rule_number is required)
 		rulesList := nrs.List()
 
+		// Validate rules BEFORE creating them
+		if err := validateRulesList(d, rulesList, "ruleset"); err != nil {
+			return err
+		}
+
 		err := createNetworkACLRules(d, meta, &rules, rulesList)
 		if err != nil {
 			log.Printf("[ERROR] Failed to create network ACL rules: %v", err)
@@ -667,9 +669,18 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str
 				baseRuleNum = ruleNum
 			}
 
+			// Convert TypeSet to sorted list for deterministic rule number assignment
+			// This ensures that rule numbers are stable across runs
+			portsList := portsSet.List()
+			portsStrings := make([]string, len(portsList))
+			for i, port := range portsList {
+				portsStrings[i] = port.(string)
+			}
+			sort.Strings(portsStrings)
+			log.Printf("[DEBUG] Sorted ports for deterministic numbering: %v", portsStrings)
+
 			portIndex := 0
-			for _, port := range portsSet.List() {
-				portValue := port.(string)
+			for _, portValue := range portsStrings {
 
 				// Check if this port already has a UUID
 				if _, hasUUID := getRuleUUID(rule, portValue); !hasUUID {
@@ -1153,6 +1164,11 @@ func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interfa
 
 		log.Printf("[DEBUG] Rule list changed, performing efficient updates")
 
+		// Validate new rules BEFORE assigning numbers
+		if err := validateRulesList(d, newRules, "rule"); err != nil {
+			return err
+		}
+
 		// Assign rule numbers to new rules that don't have them
 		newRulesWithNumbers := assignRuleNumbers(newRules)
 
@@ -1170,6 +1186,11 @@ func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interfa
 
 		log.Printf("[DEBUG] Ruleset changed: %d old rules -> %d new rules", len(oldRules), len(newRules))
 
+		// Validate new rules BEFORE updating
+		if err := validateRulesList(d, newRules, "ruleset"); err != nil {
+			return err
+		}
+
 		// No migration needed for ruleset (no deprecated 'ports' field)
 		// No auto-numbering needed (rule_number is required)
 		err := updateNetworkACLRules(d, meta, oldRules, newRules)
@@ -1273,6 +1294,31 @@ func verifyNetworkACLParams(d *schema.ResourceData) error {
 	return nil
 }
 
+// validateRulesList validates all rules in a list by calling verifyNetworkACLRuleParams on each
+// This helper consolidates the validation logic used in Create and Update paths for both 'rule' and 'ruleset' fields
+// Out-of-band rule placeholders (created by managed=true) are skipped as they are markers for deletion
+func validateRulesList(d *schema.ResourceData, rules []interface{}, fieldName string) error {
+	validatedCount := 0
+	for i, rule := range rules {
+		ruleMap := rule.(map[string]interface{})
+
+		// Skip validation for out-of-band rule placeholders
+		// These are created by managed=true and are just markers for deletion
+		if isOutOfBandRulePlaceholder(ruleMap) {
+			log.Printf("[DEBUG] Skipping validation for out-of-band rule placeholder at index %d", i)
+			continue
+		}
+
+		if err := verifyNetworkACLRuleParams(d, ruleMap); err != nil {
+			log.Printf("[ERROR] Failed to verify %s rule %d parameters: %v", fieldName, i, err)
+			return fmt.Errorf("validation failed for %s rule %d: %w", fieldName, i, err)
+		}
+		validatedCount++
+	}
+	log.Printf("[DEBUG] Successfully validated %d %s rules (skipped %d out-of-band placeholders)", validatedCount, fieldName, len(rules)-validatedCount)
+	return nil
+}
+
 func verifyNetworkACLRuleParams(d *schema.ResourceData, rule map[string]interface{}) error {
 	log.Printf("[DEBUG] Verifying parameters for rule: %+v", rule)