From d9d52c657bf477e240be59e9d9a53a2b56767be2 Mon Sep 17 00:00:00 2001 From: Brad House - Nexthop Date: Fri, 6 Mar 2026 13:03:02 +0000 Subject: [PATCH 1/9] Add auto-numbering for ACL rules without explicit rule_number - Implement assignRuleNumbers() function for sequential auto-numbering - Rules without rule_number get assigned sequential numbers starting from 1 - If a rule has explicit rule_number, numbering continues from that value - Integrated into Create, Update, and ports migration flows - Preserves config file order (TypeList maintains order naturally) --- .../resource_cloudstack_network_acl_rule.go | 51 +++++++++++++++++-- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/cloudstack/resource_cloudstack_network_acl_rule.go b/cloudstack/resource_cloudstack_network_acl_rule.go index 839f3e8a..eb97e2d0 100644 --- a/cloudstack/resource_cloudstack_network_acl_rule.go +++ b/cloudstack/resource_cloudstack_network_acl_rule.go @@ -51,7 +51,7 @@ func resourceCloudStackNetworkACLRule() *schema.Resource { oldRulesList := oldRules.([]interface{}) newRulesList := newRules.([]interface{}) - log.Printf("[DEBUG] CustomizeDiff: checking %d old rules -> %d new rules for migration", len(oldRulesList), len(newRulesList)) + log.Printf("[DEBUG] CustomizeDiff: checking %d old rules -> %d new rules", len(oldRulesList), len(newRulesList)) // Check if ANY old rule uses deprecated 'ports' field hasDeprecatedPorts := false @@ -202,6 +202,38 @@ func resourceCloudStackNetworkACLRule() *schema.Resource { } } +// assignRuleNumbers assigns rule numbers to rules that don't have them +// Rules are numbered sequentially starting from 1 +// If a rule has an explicit rule_number, numbering continues from that value +func assignRuleNumbers(rules []interface{}) []interface{} { + result := make([]interface{}, len(rules)) + nextNumber := 1 + + for i, rule := range rules { + ruleMap := make(map[string]interface{}) + // Copy the rule + for k, v := range rule.(map[string]interface{}) { + ruleMap[k] = v + } + + // Check if rule_number is set + if ruleNum, ok := ruleMap["rule_number"].(int); ok && ruleNum > 0 { + // Rule has explicit number, use it and continue from there + nextNumber = ruleNum + 1 + log.Printf("[DEBUG] Rule at index %d has explicit rule_number=%d", i, ruleNum) + } else { + // Auto-assign sequential number + ruleMap["rule_number"] = nextNumber + log.Printf("[DEBUG] Auto-assigned rule_number=%d to rule at index %d", nextNumber, i) + nextNumber++ + } + + result[i] = ruleMap + } + + return result +} + func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interface{}) error { log.Printf("[DEBUG] Entering resourceCloudStackNetworkACLRuleCreate with acl_id=%s", d.Get("acl_id").(string)) @@ -217,7 +249,11 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa rules := make([]interface{}, 0) log.Printf("[DEBUG] Processing %d rules", len(nrs)) - err := createNetworkACLRules(d, meta, &rules, nrs) + + // Assign rule numbers to rules that don't have them + rulesWithNumbers := assignRuleNumbers(nrs) + + err := createNetworkACLRules(d, meta, &rules, rulesWithNumbers) if err != nil { log.Printf("[ERROR] Failed to create network ACL rules: %v", err) return err @@ -740,7 +776,11 @@ func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interfa } log.Printf("[DEBUG] Rule list changed, performing efficient updates") - err := updateNetworkACLRules(d, meta, oldRules, newRules) + + // Assign rule numbers to new rules that don't have them + newRulesWithNumbers := assignRuleNumbers(newRules) + + err := updateNetworkACLRules(d, meta, oldRules, newRulesWithNumbers) if err != nil { return err } @@ -1416,8 +1456,11 @@ func performPortsMigration(d *schema.ResourceData, meta interface{}, oldRules, n rulesToCreate = append(rulesToCreate, cleanRule) } + // Assign rule numbers to new rules that don't have them + rulesToCreateWithNumbers := assignRuleNumbers(rulesToCreate) + var createdRules []interface{} - err := createNetworkACLRules(d, meta, &createdRules, rulesToCreate) + err := createNetworkACLRules(d, meta, &createdRules, rulesToCreateWithNumbers) if err != nil { return fmt.Errorf("failed to create new rules during migration: %v", err) } From 27d5be40f7c8ba3bf39fb40a98e652e84fbb5c01 Mon Sep 17 00:00:00 2001 From: Brad House - Nexthop Date: Fri, 6 Mar 2026 13:11:21 +0000 Subject: [PATCH 2/9] Add 'ruleset' field using TypeSet with mandatory rule_number - Add new 'ruleset' field as TypeSet alternative to 'rule' TypeList - Make rule_number required in ruleset (no auto-numbering needed) - Remove deprecated 'ports' field from ruleset schema - Add ConflictsWith to prevent using both 'rule' and 'ruleset' - Update Create, Read, Update, Delete functions to handle both fields - TypeSet prevents spurious diffs when rules are inserted in the middle - Add comprehensive example showing ruleset usage - Document key differences between rule and ruleset Add test cases for ruleset field - Add TestAccCloudStackNetworkACLRule_ruleset_basic test - Add TestAccCloudStackNetworkACLRule_ruleset_update test - Both tests mirror existing rule tests but use ruleset with explicit rule_number - All rules in ruleset tests have mandatory rule_number values - Tests verify TypeSet behavior and rule attributes - Ensures ruleset field works correctly for create and update operations --- .../resource_cloudstack_network_acl_rule.go | 686 ++++++++--- ...source_cloudstack_network_acl_rule_test.go | 1031 ++++++++++++++++- website/docs/r/network_acl_rule.html.markdown | 82 +- 3 files changed, 1548 insertions(+), 251 deletions(-) diff --git a/cloudstack/resource_cloudstack_network_acl_rule.go b/cloudstack/resource_cloudstack_network_acl_rule.go index eb97e2d0..99fda022 100644 --- a/cloudstack/resource_cloudstack_network_acl_rule.go +++ b/cloudstack/resource_cloudstack_network_acl_rule.go @@ -20,6 +20,7 @@ package cloudstack import ( + "bytes" "context" "fmt" "log" @@ -187,6 +188,75 @@ func resourceCloudStackNetworkACLRule() *schema.Resource { }, }, + "ruleset": { + Type: schema.TypeSet, + Optional: true, + ConflictsWith: []string{"rule"}, + Set: resourceCloudStackNetworkACLRulesetHash, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "rule_number": { + Type: schema.TypeInt, + Required: true, + }, + + "action": { + Type: schema.TypeString, + Optional: true, + Default: "allow", + }, + + "cidr_list": { + Type: schema.TypeList, + Required: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + + "protocol": { + Type: schema.TypeString, + Required: true, + }, + + "icmp_type": { + Type: schema.TypeInt, + Optional: true, + Default: 0, + }, + + "icmp_code": { + Type: schema.TypeInt, + Optional: true, + Default: 0, + }, + + "port": { + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + // Treat empty string as equivalent to not set (for "all" protocol) + return old == "" && new == "" + }, + }, + + "traffic_type": { + Type: schema.TypeString, + Optional: true, + Default: "ingress", + }, + + "description": { + Type: schema.TypeString, + Optional: true, + }, + + "uuid": { + Type: schema.TypeString, + Computed: true, + }, + }, + }, + }, + "project": { Type: schema.TypeString, Optional: true, @@ -202,6 +272,127 @@ 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) + +// getRuleUUID gets the UUID for a rule, handling both formats +// For ruleset: returns the uuid string +// For rule with key: returns the UUID from uuids map for the given key +// For rule without key: returns the first UUID from uuids map +func getRuleUUID(rule map[string]interface{}, key string) (string, bool) { + // Try uuid string first (ruleset format) + if uuidVal, ok := rule["uuid"]; ok && uuidVal != nil { + if uuid, ok := uuidVal.(string); ok && uuid != "" { + return uuid, true + } + } + + // Try uuids map (rule format) + if uuidsVal, ok := rule["uuids"]; ok && uuidsVal != nil { + if uuids, ok := uuidsVal.(map[string]interface{}); ok { + if key != "" { + // Get specific key + if idVal, ok := uuids[key]; ok && idVal != nil { + if id, ok := idVal.(string); ok { + return id, true + } + } + } else { + // Get first non-nil UUID + for _, idVal := range uuids { + if idVal != nil { + if id, ok := idVal.(string); ok { + return id, true + } + } + } + } + } + } + + return "", false +} + +// setRuleUUID sets the UUID for a rule, handling both formats +// For ruleset: sets the uuid string +// For rule: sets the UUID in uuids map with the given key +func setRuleUUID(rule map[string]interface{}, key string, uuid string) { + // Check if this is a ruleset (has uuid field) or rule (has uuids field) + if _, hasUUID := rule["uuid"]; hasUUID { + // Ruleset format + rule["uuid"] = uuid + } else { + // Rule format - ensure uuids map exists + var uuids map[string]interface{} + if uuidsVal, ok := rule["uuids"]; ok && uuidsVal != nil { + uuids = uuidsVal.(map[string]interface{}) + } else { + uuids = make(map[string]interface{}) + rule["uuids"] = uuids + } + uuids[key] = uuid + } +} + +// hasRuleUUID checks if a rule has any UUID set +func hasRuleUUID(rule map[string]interface{}) bool { + // Check uuid string (ruleset format) + if uuidVal, ok := rule["uuid"]; ok && uuidVal != nil { + if uuid, ok := uuidVal.(string); ok && uuid != "" { + return true + } + } + + // Check uuids map (rule format) + if uuidsVal, ok := rule["uuids"]; ok && uuidsVal != nil { + if uuids, ok := uuidsVal.(map[string]interface{}); ok && len(uuids) > 0 { + return true + } + } + + return false +} + +// copyRuleUUIDs copies all UUIDs from source rule to destination rule +func copyRuleUUIDs(dst, src map[string]interface{}) { + // Check if source has uuid string (ruleset format) + if uuidVal, ok := src["uuid"]; ok && uuidVal != nil { + if uuid, ok := uuidVal.(string); ok && uuid != "" { + dst["uuid"] = uuid + return + } + } + + // Check if source has uuids map (rule format) + if uuidsVal, ok := src["uuids"]; ok && uuidsVal != nil { + if uuids, ok := uuidsVal.(map[string]interface{}); ok { + dst["uuids"] = uuids + return + } + } +} + +// isRulesetRule checks if a rule is from a ruleset (has uuid field) vs rule (has uuids field) +func isRulesetRule(rule map[string]interface{}) bool { + _, hasUUID := rule["uuid"] + return hasUUID +} + // assignRuleNumbers assigns rule numbers to rules that don't have them // Rules are numbered sequentially starting from 1 // If a rule has an explicit rule_number, numbering continues from that value @@ -243,12 +434,12 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa return err } - // Create all rules that are configured + // Handle 'rule' (TypeList with auto-numbering) if nrs := d.Get("rule").([]interface{}); len(nrs) > 0 { // Create an empty rule list to hold all newly created rules rules := make([]interface{}, 0) - log.Printf("[DEBUG] Processing %d rules", len(nrs)) + log.Printf("[DEBUG] Processing %d rules from 'rule' field", len(nrs)) // Assign rule numbers to rules that don't have them rulesWithNumbers := assignRuleNumbers(nrs) @@ -268,6 +459,30 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa log.Printf("[ERROR] Failed to set rule attribute: %v", err) return err } + } else if nrs := d.Get("ruleset").(*schema.Set); nrs.Len() > 0 { + // Handle 'ruleset' (TypeSet with mandatory rule_number) + rules := make([]interface{}, 0) + + log.Printf("[DEBUG] Processing %d rules from 'ruleset' field", nrs.Len()) + + // Convert Set to list (no auto-numbering needed, rule_number is required) + rulesList := nrs.List() + + err := createNetworkACLRules(d, meta, &rules, rulesList) + if err != nil { + log.Printf("[ERROR] Failed to create network ACL rules: %v", err) + return err + } + + // Set the resource ID only after successful creation + log.Printf("[DEBUG] Setting resource ID to acl_id=%s", d.Get("acl_id").(string)) + d.SetId(d.Get("acl_id").(string)) + + // Update state with created rules + if err := d.Set("ruleset", rules); err != nil { + log.Printf("[ERROR] Failed to set ruleset attribute: %v", err) + return err + } } else { log.Printf("[DEBUG] No rules provided, setting ID to acl_id=%s", d.Get("acl_id").(string)) d.SetId(d.Get("acl_id").(string)) @@ -305,11 +520,14 @@ func createNetworkACLRules(d *schema.ResourceData, meta interface{}, rules *[]in mu.Lock() errs = multierror.Append(errs, fmt.Errorf("rule #%d: %v", index+1, err)) mu.Unlock() - } else if len(rule["uuids"].(map[string]interface{})) > 0 { - log.Printf("[DEBUG] Successfully created rule #%d, storing at index %d", index+1, index) - results[index] = rule } else { - log.Printf("[WARN] Rule #%d created but has no UUIDs", index+1) + // Check if rule was created successfully (has uuid or uuids) + if hasRuleUUID(rule) { + log.Printf("[DEBUG] Successfully created rule #%d, storing at index %d", index+1, index) + results[index] = rule + } else { + log.Printf("[WARN] Rule #%d created but has no UUID/UUIDs", index+1) + } } <-sem @@ -336,8 +554,12 @@ func createNetworkACLRules(d *schema.ResourceData, meta interface{}, rules *[]in func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - uuids := rule["uuids"].(map[string]interface{}) - log.Printf("[DEBUG] Creating network ACL rule with protocol=%s", rule["protocol"].(string)) + + protocol := rule["protocol"].(string) + action := rule["action"].(string) + trafficType := rule["traffic_type"].(string) + + log.Printf("[DEBUG] Creating network ACL rule with protocol=%s, action=%s, traffic_type=%s", protocol, action, trafficType) // Make sure all required parameters are there if err := verifyNetworkACLRuleParams(d, rule); err != nil { @@ -346,7 +568,7 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str } // Create a new parameter struct - p := cs.NetworkACL.NewCreateNetworkACLParams(rule["protocol"].(string)) + p := cs.NetworkACL.NewCreateNetworkACLParams(protocol) log.Printf("[DEBUG] Initialized CreateNetworkACLParams") // If a rule ID is specified, set it @@ -361,8 +583,8 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str log.Printf("[DEBUG] Set aclid=%s", aclID) // Set the action - p.SetAction(rule["action"].(string)) - log.Printf("[DEBUG] Set action=%s", rule["action"].(string)) + p.SetAction(action) + log.Printf("[DEBUG] Set action=%s", action) // Set the CIDR list var cidrList []string @@ -373,8 +595,8 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str log.Printf("[DEBUG] Set cidr_list=%v", cidrList) // Set the traffic type - p.SetTraffictype(rule["traffic_type"].(string)) - log.Printf("[DEBUG] Set traffic_type=%s", rule["traffic_type"].(string)) + p.SetTraffictype(trafficType) + log.Printf("[DEBUG] Set traffic_type=%s", trafficType) // Set the description if desc, ok := rule["description"].(string); ok && desc != "" { @@ -393,9 +615,9 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str log.Printf("[ERROR] Failed to create ICMP rule: %v", err) return err } - uuids["icmp"] = r.(*cloudstack.CreateNetworkACLResponse).Id - rule["uuids"] = uuids - log.Printf("[DEBUG] Created ICMP rule with ID=%s", r.(*cloudstack.CreateNetworkACLResponse).Id) + ruleID := r.(*cloudstack.CreateNetworkACLResponse).Id + setRuleUUID(rule, "icmp", ruleID) + log.Printf("[DEBUG] Created ICMP rule with ID=%s", ruleID) } // If the protocol is ALL set the needed parameters @@ -405,9 +627,9 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str log.Printf("[ERROR] Failed to create ALL rule: %v", err) return err } - uuids["all"] = r.(*cloudstack.CreateNetworkACLResponse).Id - rule["uuids"] = uuids - log.Printf("[DEBUG] Created ALL rule with ID=%s", r.(*cloudstack.CreateNetworkACLResponse).Id) + ruleID := r.(*cloudstack.CreateNetworkACLResponse).Id + setRuleUUID(rule, "all", ruleID) + log.Printf("[DEBUG] Created ALL rule with ID=%s", ruleID) } // If protocol is TCP or UDP, create the rule (with or without port) @@ -424,7 +646,8 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str // Handle single port log.Printf("[DEBUG] Processing single port for TCP/UDP rule: %s", portStr) - if _, ok := uuids[portStr]; !ok { + // Check if this port already has a UUID (for 'rule' field with uuids map) + if _, hasUUID := getRuleUUID(rule, portStr); !hasUUID { m := splitPorts.FindStringSubmatch(portStr) if m == nil { log.Printf("[ERROR] Invalid port format: %s", portStr) @@ -456,9 +679,9 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str return err } - uuids[portStr] = r.(*cloudstack.CreateNetworkACLResponse).Id - rule["uuids"] = uuids - log.Printf("[DEBUG] Created TCP/UDP rule for port %s with ID=%s", portStr, r.(*cloudstack.CreateNetworkACLResponse).Id) + ruleID := r.(*cloudstack.CreateNetworkACLResponse).Id + setRuleUUID(rule, portStr, ruleID) + log.Printf("[DEBUG] Created TCP/UDP rule for port %s with ID=%s", portStr, ruleID) } else { log.Printf("[DEBUG] Port %s already has UUID, skipping", portStr) } @@ -470,97 +693,81 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str log.Printf("[ERROR] Failed to create TCP/UDP rule for all ports: %v", err) return err } - uuids["all_ports"] = r.(*cloudstack.CreateNetworkACLResponse).Id - rule["uuids"] = uuids - log.Printf("[DEBUG] Created TCP/UDP rule for all ports with ID=%s", r.(*cloudstack.CreateNetworkACLResponse).Id) + ruleID := r.(*cloudstack.CreateNetworkACLResponse).Id + setRuleUUID(rule, "all_ports", ruleID) + log.Printf("[DEBUG] Created TCP/UDP rule for all ports with ID=%s", ruleID) } } - log.Printf("[DEBUG] Successfully created rule with uuids=%+v", uuids) + log.Printf("[DEBUG] Successfully created rule") return nil } -func processTCPUDPRule(rule map[string]interface{}, ruleMap map[string]*cloudstack.NetworkACL, uuids map[string]interface{}, rules *[]interface{}) { +func processTCPUDPRule(rule map[string]interface{}, ruleMap map[string]*cloudstack.NetworkACL, rules *[]interface{}) { // Check for deprecated ports field first (for reading existing state during migration) + // This is only applicable to the legacy 'rule' field, not 'ruleset' ps, hasPortsSet := rule["ports"].(*schema.Set) portStr, hasPort := rule["port"].(string) if hasPortsSet && ps.Len() > 0 { + // Only legacy 'rule' field supports deprecated ports log.Printf("[DEBUG] Processing deprecated ports field with %d ports during state read", ps.Len()) // Process each port in the deprecated ports set during state read for _, port := range ps.List() { portStr := port.(string) - if processPortForRule(portStr, rule, ruleMap, uuids) { + if processPortForRuleUnified(portStr, rule, ruleMap) { log.Printf("[DEBUG] Processed deprecated port %s during state read", portStr) } } // Only add the rule once with all processed ports - if len(uuids) > 0 { - *rules = append(*rules, rule) - log.Printf("[DEBUG] Added TCP/UDP rule with deprecated ports to state during read: %+v", rule) + if uuidsVal, ok := rule["uuids"]; ok { + if uuids, ok := uuidsVal.(map[string]interface{}); ok && len(uuids) > 0 { + *rules = append(*rules, rule) + log.Printf("[DEBUG] Added TCP/UDP rule with deprecated ports to state during read: %+v", rule) + } } } else if hasPort && portStr != "" { + // Handle single port - works for both 'rule' and 'ruleset' log.Printf("[DEBUG] Processing single port for TCP/UDP rule: %s", portStr) - if processPortForRule(portStr, rule, ruleMap, uuids) { + if processPortForRuleUnified(portStr, rule, ruleMap) { rule["port"] = portStr *rules = append(*rules, rule) log.Printf("[DEBUG] Added TCP/UDP rule with single port to state: %+v", rule) } } else { + // No port specified - create rule for all ports + // Works for both 'rule' and 'ruleset' log.Printf("[DEBUG] Processing TCP/UDP rule with no port specified") - id, ok := uuids["all_ports"] - if !ok { - log.Printf("[DEBUG] No UUID for all_ports, skipping rule") - return - } - - r, ok := ruleMap[id.(string)] - if !ok { - log.Printf("[DEBUG] TCP/UDP rule for all_ports with ID %s not found, removing UUID", id.(string)) - delete(uuids, "all_ports") - return - } - - delete(ruleMap, id.(string)) - - var cidrs []interface{} - for _, cidr := range strings.Split(r.Cidrlist, ",") { - cidrs = append(cidrs, cidr) + if processPortForRuleUnified("all_ports", rule, ruleMap) { + *rules = append(*rules, rule) + log.Printf("[DEBUG] Added TCP/UDP rule with no port to state: %+v", rule) } - - 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 - *rules = append(*rules, rule) - log.Printf("[DEBUG] Added TCP/UDP rule with no port to state: %+v", rule) } } -func processPortForRule(portStr string, rule map[string]interface{}, ruleMap map[string]*cloudstack.NetworkACL, uuids map[string]interface{}) bool { - id, ok := uuids[portStr] +func processPortForRuleUnified(portKey string, rule map[string]interface{}, ruleMap map[string]*cloudstack.NetworkACL) bool { + // Get the UUID for this port (handles both 'rule' and 'ruleset' formats) + id, ok := getRuleUUID(rule, portKey) if !ok { - log.Printf("[DEBUG] No UUID for port %s, skipping", portStr) + log.Printf("[DEBUG] No UUID for port %s, skipping", portKey) return false } - r, ok := ruleMap[id.(string)] + r, ok := ruleMap[id] if !ok { - log.Printf("[DEBUG] TCP/UDP rule for port %s with ID %s not found, removing UUID", portStr, id.(string)) - delete(uuids, portStr) + log.Printf("[DEBUG] TCP/UDP rule for port %s with ID %s not found", portKey, id) return false } // Delete the known rule so only unknown rules remain in the ruleMap - delete(ruleMap, id.(string)) + delete(ruleMap, id) var cidrs []interface{} for _, cidr := range strings.Split(r.Cidrlist, ",") { @@ -572,6 +779,10 @@ func processPortForRule(portStr string, rule map[string]interface{}, ruleMap map 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 + rule["icmp_type"] = 0 + rule["icmp_code"] = 0 return true } @@ -632,87 +843,108 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface // Create an empty rule list to hold all rules var rules []interface{} - // Read all rules that are configured - if rs := d.Get("rule").([]interface{}); len(rs) > 0 { - for _, rule := range rs { - rule := rule.(map[string]interface{}) - uuids := rule["uuids"].(map[string]interface{}) - log.Printf("[DEBUG] Processing rule with protocol=%s, uuids=%+v", rule["protocol"].(string), uuids) - - if rule["protocol"].(string) == "icmp" { - id, ok := uuids["icmp"] - if !ok { - log.Printf("[DEBUG] No ICMP UUID found, skipping rule") - continue - } + // Determine which field is being used and get the rules list + var configuredRules []interface{} + usingRuleset := false + if rs := d.Get("ruleset").(*schema.Set); rs != nil && rs.Len() > 0 { + usingRuleset = true + configuredRules = rs.List() + } else if rs := d.Get("rule").([]interface{}); len(rs) > 0 { + configuredRules = rs + } - // Get the rule - r, ok := ruleMap[id.(string)] - if !ok { - log.Printf("[DEBUG] ICMP rule with ID %s not found, removing UUID", id.(string)) - delete(uuids, "icmp") - continue - } + // Process all configured rules (works for both 'rule' and 'ruleset') + for _, rule := range configuredRules { + rule := rule.(map[string]interface{}) - // Delete the known rule so only unknown rules remain in the ruleMap - delete(ruleMap, id.(string)) + fieldName := "rule" + if usingRuleset { + fieldName = "ruleset" + } + log.Printf("[DEBUG] Processing %s with protocol=%s", fieldName, rule["protocol"].(string)) - // Create a list with all CIDR's - var cidrs []interface{} - for _, cidr := range strings.Split(r.Cidrlist, ",") { - cidrs = append(cidrs, cidr) - } + if rule["protocol"].(string) == "icmp" { + id, ok := getRuleUUID(rule, "icmp") + if !ok { + log.Printf("[DEBUG] No ICMP UUID found, skipping rule") + continue + } - // Update the values - rule["action"] = strings.ToLower(r.Action) - rule["protocol"] = r.Protocol - 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 - rules = append(rules, rule) - log.Printf("[DEBUG] Added ICMP rule to state: %+v", rule) + // Get the rule + r, ok := ruleMap[id] + if !ok { + log.Printf("[DEBUG] ICMP rule with ID %s not found", id) + continue } - if rule["protocol"].(string) == "all" { - id, ok := uuids["all"] - if !ok { - log.Printf("[DEBUG] No ALL UUID found, skipping rule") - continue - } + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id) - // Get the rule - r, ok := ruleMap[id.(string)] - if !ok { - log.Printf("[DEBUG] ALL rule with ID %s not found, removing UUID", id.(string)) - delete(uuids, "all") - continue - } + // Create a list with all CIDR's + var cidrs []interface{} + for _, cidr := range strings.Split(r.Cidrlist, ",") { + cidrs = append(cidrs, cidr) + } - // Delete the known rule so only unknown rules remain in the ruleMap - delete(ruleMap, id.(string)) + // Update the values + rule["action"] = strings.ToLower(r.Action) + rule["protocol"] = r.Protocol + 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 { + rule["uuid"] = id + } + rules = append(rules, rule) + log.Printf("[DEBUG] Added ICMP rule to state: %+v", rule) + } - // Create a list with all CIDR's - var cidrs []interface{} - for _, cidr := range strings.Split(r.Cidrlist, ",") { - cidrs = append(cidrs, cidr) - } + if rule["protocol"].(string) == "all" { + id, ok := getRuleUUID(rule, "all") + if !ok { + log.Printf("[DEBUG] No ALL UUID found, skipping rule") + continue + } + + // Get the rule + r, ok := ruleMap[id] + if !ok { + log.Printf("[DEBUG] ALL rule with ID %s not found", id) + continue + } - // 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 - rules = append(rules, rule) - log.Printf("[DEBUG] Added ALL rule to state: %+v", rule) + // 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) } - if rule["protocol"].(string) == "tcp" || rule["protocol"].(string) == "udp" { - uuids := rule["uuids"].(map[string]interface{}) - processTCPUDPRule(rule, ruleMap, uuids, &rules) + // 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 + // Set ICMP fields to 0 for non-ICMP protocols to avoid spurious diffs + rule["icmp_type"] = 0 + rule["icmp_code"] = 0 + if usingRuleset { + rule["uuid"] = id } + rules = append(rules, rule) + log.Printf("[DEBUG] Added ALL rule to state: %+v", rule) + } + + if rule["protocol"].(string) == "tcp" || rule["protocol"].(string) == "udp" { + processTCPUDPRule(rule, ruleMap, &rules) } } @@ -739,9 +971,16 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface if len(rules) > 0 { log.Printf("[DEBUG] Setting %d rules in state", len(rules)) - if err := d.Set("rule", rules); err != nil { - log.Printf("[ERROR] Failed to set rule attribute: %v", err) - return err + if usingRuleset { + if err := d.Set("ruleset", rules); err != nil { + log.Printf("[ERROR] Failed to set ruleset attribute: %v", err) + return err + } + } else { + if err := d.Set("rule", rules); err != nil { + log.Printf("[ERROR] Failed to set rule attribute: %v", err) + return err + } } } else if !managed { log.Printf("[DEBUG] No rules found and not managed, clearing ID") @@ -786,11 +1025,27 @@ func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interfa } } + // Check if the ruleset has changed + if d.HasChange("ruleset") { + o, n := d.GetChange("ruleset") + oldRules := o.(*schema.Set).List() + newRules := n.(*schema.Set).List() + + log.Printf("[DEBUG] Ruleset changed: %d old rules -> %d new rules", len(oldRules), len(newRules)) + + // No migration needed for ruleset (no deprecated 'ports' field) + // No auto-numbering needed (rule_number is required) + err := updateNetworkACLRules(d, meta, oldRules, newRules) + if err != nil { + return err + } + } + return resourceCloudStackNetworkACLRuleRead(d, meta) } func resourceCloudStackNetworkACLRuleDelete(d *schema.ResourceData, meta interface{}) error { - // Delete all rules + // Delete all rules from 'rule' field if ors := d.Get("rule").([]interface{}); len(ors) > 0 { for _, rule := range ors { ruleMap := rule.(map[string]interface{}) @@ -802,52 +1057,80 @@ func resourceCloudStackNetworkACLRuleDelete(d *schema.ResourceData, meta interfa } } + // Delete all rules from 'ruleset' field + if ors := d.Get("ruleset").(*schema.Set); ors != nil && ors.Len() > 0 { + for _, rule := range ors.List() { + ruleMap := rule.(map[string]interface{}) + err := deleteNetworkACLRule(d, meta, ruleMap) + if err != nil { + log.Printf("[ERROR] Failed to delete ruleset rule: %v", err) + return err + } + } + } + return nil } func deleteNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error { cs := meta.(*cloudstack.CloudStackClient) - uuids := rule["uuids"].(map[string]interface{}) - for k, id := range uuids { - // We don't care about the count here, so just continue - if k == "%" { - continue + if isRulesetRule(rule) { + // For ruleset, delete the single UUID + if uuid, ok := getRuleUUID(rule, ""); ok { + if err := deleteSingleACL(cs, uuid); err != nil { + return err + } + rule["uuid"] = "" } + } else { + // For rule, delete all UUIDs from the map + if uuidsVal, ok := rule["uuids"]; ok && uuidsVal != nil { + uuids := uuidsVal.(map[string]interface{}) + for k, id := range uuids { + // Skip the count field + if k == "%" { + continue + } + if idStr, ok := id.(string); ok { + if err := deleteSingleACL(cs, idStr); err != nil { + return err + } + delete(uuids, k) + } + } + rule["uuids"] = uuids + } + } - // Create the parameter struct - p := cs.NetworkACL.NewDeleteNetworkACLParams(id.(string)) - - // Delete the rule - if _, err := cs.NetworkACL.DeleteNetworkACL(p); err != nil { + return nil +} - // This is a very poor way to be told the ID does no longer exist :( - if strings.Contains(err.Error(), fmt.Sprintf( - "Invalid parameter id value=%s due to incorrect long value format, "+ - "or entity does not exist", id.(string))) { - delete(uuids, k) - rule["uuids"] = uuids - continue - } +func deleteSingleACL(cs *cloudstack.CloudStackClient, id string) error { + log.Printf("[DEBUG] Deleting ACL rule with UUID=%s", id) - return err + p := cs.NetworkACL.NewDeleteNetworkACLParams(id) + if _, err := cs.NetworkACL.DeleteNetworkACL(p); err != nil { + // This is a very poor way to be told the ID does no longer exist :( + if strings.Contains(err.Error(), fmt.Sprintf( + "Invalid parameter id value=%s due to incorrect long value format, "+ + "or entity does not exist", id)) { + // ID doesn't exist, which is fine for delete + return nil } - - // Delete the UUID of this rule - delete(uuids, k) - rule["uuids"] = uuids + return err } - return nil } func verifyNetworkACLParams(d *schema.ResourceData) error { managed := d.Get("managed").(bool) _, rules := d.GetOk("rule") + _, ruleset := d.GetOk("ruleset") - if !rules && !managed { + if !rules && !ruleset && !managed { return fmt.Errorf( - "You must supply at least one 'rule' when not using the 'managed' firewall feature") + "You must supply at least one 'rule' or 'ruleset' when not using the 'managed' firewall feature") } return nil @@ -1009,19 +1292,14 @@ func performNormalRuleUpdates(d *schema.ResourceData, meta interface{}, cs *clou if rulesMatch(oldRuleMap, newRuleMap) { log.Printf("[DEBUG] Found matching new rule for old rule") - if oldUUIDs, ok := oldRuleMap["uuids"].(map[string]interface{}); ok { - newRuleMap["uuids"] = oldUUIDs - } + // Copy UUID from old rule to new rule + copyRuleUUIDs(newRuleMap, oldRuleMap) if ruleNeedsUpdate(oldRuleMap, newRuleMap) { log.Printf("[DEBUG] Rule needs updating") - if uuids, ok := oldRuleMap["uuids"].(map[string]interface{}); ok { - for _, uuid := range uuids { - if uuid != nil { - rulesToUpdate[uuid.(string)] = newRuleMap - break - } - } + // Get UUID for update (use empty key to get first UUID) + if updateUUID, ok := getRuleUUID(oldRuleMap, ""); ok { + rulesToUpdate[updateUUID] = newRuleMap } } @@ -1084,13 +1362,20 @@ func performNormalRuleUpdates(d *schema.ResourceData, meta interface{}, cs *clou } func rulesMatch(oldRule, newRule map[string]interface{}) bool { - if oldRule["protocol"].(string) != newRule["protocol"].(string) || - oldRule["traffic_type"].(string) != newRule["traffic_type"].(string) || - oldRule["action"].(string) != newRule["action"].(string) { + oldProtocol := oldRule["protocol"].(string) + newProtocol := newRule["protocol"].(string) + oldTrafficType := oldRule["traffic_type"].(string) + newTrafficType := newRule["traffic_type"].(string) + oldAction := oldRule["action"].(string) + newAction := newRule["action"].(string) + + if oldProtocol != newProtocol || + oldTrafficType != newTrafficType || + oldAction != newAction { return false } - protocol := newRule["protocol"].(string) + protocol := newProtocol if protocol == "tcp" || protocol == "udp" { oldPort, oldHasPort := oldRule["port"].(string) @@ -1121,18 +1406,24 @@ func rulesMatch(oldRule, newRule map[string]interface{}) bool { } func ruleNeedsUpdate(oldRule, newRule map[string]interface{}) bool { - if oldRule["action"].(string) != newRule["action"].(string) { - log.Printf("[DEBUG] Action changed: %s -> %s", oldRule["action"].(string), newRule["action"].(string)) + oldAction := oldRule["action"].(string) + newAction := newRule["action"].(string) + if oldAction != newAction { + log.Printf("[DEBUG] Action changed: %s -> %s", oldAction, newAction) return true } - if oldRule["protocol"].(string) != newRule["protocol"].(string) { - log.Printf("[DEBUG] Protocol changed: %s -> %s", oldRule["protocol"].(string), newRule["protocol"].(string)) + oldProtocol := oldRule["protocol"].(string) + newProtocol := newRule["protocol"].(string) + if oldProtocol != newProtocol { + log.Printf("[DEBUG] Protocol changed: %s -> %s", oldProtocol, newProtocol) return true } - if oldRule["traffic_type"].(string) != newRule["traffic_type"].(string) { - log.Printf("[DEBUG] Traffic type changed: %s -> %s", oldRule["traffic_type"].(string), newRule["traffic_type"].(string)) + oldTrafficType := oldRule["traffic_type"].(string) + newTrafficType := newRule["traffic_type"].(string) + if oldTrafficType != newTrafficType { + log.Printf("[DEBUG] Traffic type changed: %s -> %s", oldTrafficType, newTrafficType) return true } @@ -1151,15 +1442,30 @@ func ruleNeedsUpdate(oldRule, newRule map[string]interface{}) bool { return true } - protocol := newRule["protocol"].(string) - switch protocol { + // Use newProtocol from earlier + switch newProtocol { case "icmp": - if oldRule["icmp_type"].(int) != newRule["icmp_type"].(int) { - log.Printf("[DEBUG] ICMP type changed: %d -> %d", oldRule["icmp_type"].(int), newRule["icmp_type"].(int)) + // Helper function to get int value with default + getInt := func(rule map[string]interface{}, key string, defaultVal int) int { + if val, ok := rule[key]; ok && val != nil { + if i, ok := val.(int); ok { + return i + } + } + return defaultVal + } + + oldIcmpType := getInt(oldRule, "icmp_type", 0) + newIcmpType := getInt(newRule, "icmp_type", 0) + if oldIcmpType != newIcmpType { + log.Printf("[DEBUG] ICMP type changed: %d -> %d", oldIcmpType, newIcmpType) return true } - if oldRule["icmp_code"].(int) != newRule["icmp_code"].(int) { - log.Printf("[DEBUG] ICMP code changed: %d -> %d", oldRule["icmp_code"].(int), newRule["icmp_code"].(int)) + + oldIcmpCode := getInt(oldRule, "icmp_code", 0) + newIcmpCode := getInt(newRule, "icmp_code", 0) + if oldIcmpCode != newIcmpCode { + log.Printf("[DEBUG] ICMP code changed: %d -> %d", oldIcmpCode, newIcmpCode) return true } case "tcp", "udp": diff --git a/cloudstack/resource_cloudstack_network_acl_rule_test.go b/cloudstack/resource_cloudstack_network_acl_rule_test.go index e894a8e3..7a0baf23 100644 --- a/cloudstack/resource_cloudstack_network_acl_rule_test.go +++ b/cloudstack/resource_cloudstack_network_acl_rule_test.go @@ -26,6 +26,7 @@ import ( "github.com/apache/cloudstack-go/v2/cloudstack" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" ) @@ -251,6 +252,369 @@ func testAccCheckCloudStackNetworkACLRuleDestroy(s *terraform.State) error { return nil } +func TestAccCloudStackNetworkACLRule_ruleset_basic(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRule_ruleset_basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.bar"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.bar", "ruleset.#", "4"), + // Check for the expected rules using TypeSet elem matching + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "all", + "traffic_type": "ingress", + "description": "Allow all traffic", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "20", + "action": "allow", + "protocol": "icmp", + "icmp_type": "-1", + "icmp_code": "-1", + "traffic_type": "ingress", + "description": "Allow ICMP traffic", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "40", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + ), + }, + }, + }) +} + +func TestAccCloudStackNetworkACLRule_ruleset_update(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRule_ruleset_basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.bar"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.bar", "ruleset.#", "4"), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "all", + "traffic_type": "ingress", + "description": "Allow all traffic", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "20", + "action": "allow", + "protocol": "icmp", + "icmp_type": "-1", + "icmp_code": "-1", + "traffic_type": "ingress", + "description": "Allow ICMP traffic", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "40", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + ), + }, + + { + Config: testAccCloudStackNetworkACLRule_ruleset_update, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.bar"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.bar", "ruleset.#", "6"), + // Check for the expected rules using TypeSet elem matching + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "10", + "action": "deny", + "protocol": "all", + "traffic_type": "ingress", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "20", + "action": "deny", + "protocol": "icmp", + "icmp_type": "-1", + "icmp_code": "-1", + "traffic_type": "ingress", + "description": "Deny ICMP traffic", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "40", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "50", + "action": "deny", + "protocol": "tcp", + "port": "80", + "traffic_type": "egress", + "description": "Deny specific TCP ports", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.bar", "ruleset.*", map[string]string{ + "rule_number": "60", + "action": "deny", + "protocol": "tcp", + "port": "1000-2000", + "traffic_type": "egress", + "description": "Deny specific TCP ports", + }), + ), + }, + }, + }) +} + +func TestAccCloudStackNetworkACLRule_ruleset_insert(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRule_ruleset_insert_initial, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.baz"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.baz", "ruleset.#", "3"), + // Initial rules: 10, 30, 50 + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.baz", "ruleset.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "tcp", + "port": "22", + "traffic_type": "ingress", + "description": "Allow SSH", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.baz", "ruleset.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.baz", "ruleset.*", map[string]string{ + "rule_number": "50", + "action": "allow", + "protocol": "tcp", + "port": "3306", + "traffic_type": "ingress", + "description": "Allow MySQL", + }), + ), + }, + + { + Config: testAccCloudStackNetworkACLRule_ruleset_insert_middle, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.baz"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.baz", "ruleset.#", "4"), + // After inserting rule 20 in the middle, all original rules should still exist + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.baz", "ruleset.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "tcp", + "port": "22", + "traffic_type": "ingress", + "description": "Allow SSH", + }), + // NEW RULE inserted in the middle + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.baz", "ruleset.*", map[string]string{ + "rule_number": "20", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.baz", "ruleset.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.baz", "ruleset.*", map[string]string{ + "rule_number": "50", + "action": "allow", + "protocol": "tcp", + "port": "3306", + "traffic_type": "ingress", + "description": "Allow MySQL", + }), + ), + }, + }, + }) +} + +func TestAccCloudStackNetworkACLRule_ruleset_insert_plan_check(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRule_ruleset_plan_check_initial, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.plan_check"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.plan_check", "ruleset.#", "3"), + // Initial rules: 10, 30, 50 + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.plan_check", "ruleset.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "tcp", + "port": "22", + "traffic_type": "ingress", + "description": "Allow SSH", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.plan_check", "ruleset.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.plan_check", "ruleset.*", map[string]string{ + "rule_number": "50", + "action": "allow", + "protocol": "tcp", + "port": "3306", + "traffic_type": "ingress", + "description": "Allow MySQL", + }), + ), + }, + + { + Config: testAccCloudStackNetworkACLRule_ruleset_plan_check_insert, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + // Verify that only 1 rule is being added (the new rule 20) + // and the existing rules (10, 30, 50) are not being modified + plancheck.ExpectResourceAction("cloudstack_network_acl_rule.plan_check", plancheck.ResourceActionUpdate), + }, + }, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.plan_check"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.plan_check", "ruleset.#", "4"), + // After inserting rule 20 in the middle, all original rules should still exist + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.plan_check", "ruleset.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "tcp", + "port": "22", + "traffic_type": "ingress", + "description": "Allow SSH", + }), + // NEW RULE inserted in the middle + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.plan_check", "ruleset.*", map[string]string{ + "rule_number": "20", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.plan_check", "ruleset.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_rule.plan_check", "ruleset.*", map[string]string{ + "rule_number": "50", + "action": "allow", + "protocol": "tcp", + "port": "3306", + "traffic_type": "ingress", + "description": "Allow MySQL", + }), + ), + }, + }, + }) +} + const testAccCloudStackNetworkACLRule_basic = ` resource "cloudstack_vpc" "foo" { name = "terraform-vpc" @@ -259,116 +623,675 @@ resource "cloudstack_vpc" "foo" { zone = "Sandbox-simulator" } -resource "cloudstack_network_acl" "foo" { - name = "terraform-acl" - description = "terraform-acl-text" - vpc_id = cloudstack_vpc.foo.id +resource "cloudstack_network_acl" "foo" { + name = "terraform-acl" + description = "terraform-acl-text" + vpc_id = cloudstack_vpc.foo.id +} + +resource "cloudstack_network_acl_rule" "foo" { + acl_id = cloudstack_network_acl.foo.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "all" + traffic_type = "ingress" + description = "Allow all traffic" + } + + rule { + rule_number = 20 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "icmp" + icmp_type = "-1" + icmp_code = "-1" + traffic_type = "ingress" + description = "Allow ICMP traffic" + } + + rule { + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } +}` + +const testAccCloudStackNetworkACLRule_update = ` +resource "cloudstack_vpc" "foo" { + name = "terraform-vpc" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "foo" { + name = "terraform-acl" + description = "terraform-acl-text" + vpc_id = cloudstack_vpc.foo.id +} + +resource "cloudstack_network_acl_rule" "foo" { + acl_id = cloudstack_network_acl.foo.id + + rule { + action = "deny" + cidr_list = ["172.18.100.0/24"] + protocol = "all" + traffic_type = "ingress" + } + + rule { + action = "deny" + cidr_list = ["172.18.100.0/24", "172.18.101.0/24"] + protocol = "icmp" + icmp_type = "-1" + icmp_code = "-1" + traffic_type = "ingress" + description = "Deny ICMP traffic" + } + + rule { + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } + + rule { + action = "deny" + cidr_list = ["10.0.0.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "egress" + description = "Deny specific TCP ports" + } + + rule { + action = "deny" + cidr_list = ["10.0.0.0/24"] + protocol = "tcp" + port = "1000-2000" + traffic_type = "egress" + description = "Deny specific TCP ports" + } +}` + +const testAccCloudStackNetworkACLRule_ruleset_basic = ` +resource "cloudstack_vpc" "bar" { + name = "terraform-vpc-ruleset" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "bar" { + name = "terraform-acl-ruleset" + description = "terraform-acl-ruleset-text" + vpc_id = cloudstack_vpc.bar.id +} + +resource "cloudstack_network_acl_rule" "bar" { + acl_id = cloudstack_network_acl.bar.id + + ruleset { + rule_number = 10 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "all" + traffic_type = "ingress" + description = "Allow all traffic" + } + + ruleset { + rule_number = 20 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "icmp" + icmp_type = "-1" + icmp_code = "-1" + traffic_type = "ingress" + description = "Allow ICMP traffic" + } + + ruleset { + rule_number = 30 + action = "allow" + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + ruleset { + rule_number = 40 + action = "allow" + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } +}` + +const testAccCloudStackNetworkACLRule_ruleset_update = ` +resource "cloudstack_vpc" "bar" { + name = "terraform-vpc-ruleset" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "bar" { + name = "terraform-acl-ruleset" + description = "terraform-acl-ruleset-text" + vpc_id = cloudstack_vpc.bar.id +} + +resource "cloudstack_network_acl_rule" "bar" { + acl_id = cloudstack_network_acl.bar.id + + ruleset { + rule_number = 10 + action = "deny" + cidr_list = ["172.18.100.0/24"] + protocol = "all" + traffic_type = "ingress" + } + + ruleset { + rule_number = 20 + action = "deny" + cidr_list = ["172.18.100.0/24", "172.18.101.0/24"] + protocol = "icmp" + icmp_type = "-1" + icmp_code = "-1" + traffic_type = "ingress" + description = "Deny ICMP traffic" + } + + ruleset { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + } + + ruleset { + rule_number = 40 + action = "allow" + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + } + + ruleset { + rule_number = 50 + action = "deny" + cidr_list = ["10.0.0.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "egress" + description = "Deny specific TCP ports" + } + + ruleset { + rule_number = 60 + action = "deny" + cidr_list = ["10.0.0.0/24"] + protocol = "tcp" + port = "1000-2000" + traffic_type = "egress" + description = "Deny specific TCP ports" + } +}` + +const testAccCloudStackNetworkACLRule_ruleset_insert_initial = ` +resource "cloudstack_vpc" "baz" { + name = "terraform-vpc-ruleset-insert" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "baz" { + name = "terraform-acl-ruleset-insert" + description = "terraform-acl-ruleset-insert-text" + vpc_id = cloudstack_vpc.baz.id } -resource "cloudstack_network_acl_rule" "foo" { - acl_id = cloudstack_network_acl.foo.id +resource "cloudstack_network_acl_rule" "baz" { + acl_id = cloudstack_network_acl.baz.id - rule { + ruleset { rule_number = 10 action = "allow" cidr_list = ["172.18.100.0/24"] - protocol = "all" + protocol = "tcp" + port = "22" traffic_type = "ingress" - description = "Allow all traffic" + description = "Allow SSH" } - rule { - rule_number = 20 + ruleset { + rule_number = 30 action = "allow" cidr_list = ["172.18.100.0/24"] - protocol = "icmp" - icmp_type = "-1" - icmp_code = "-1" + protocol = "tcp" + port = "443" traffic_type = "ingress" - description = "Allow ICMP traffic" + description = "Allow HTTPS" } - rule { - cidr_list = ["172.16.100.0/24"] + ruleset { + rule_number = 50 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "3306" + traffic_type = "ingress" + description = "Allow MySQL" + } +}` + +const testAccCloudStackNetworkACLRule_ruleset_insert_middle = ` +resource "cloudstack_vpc" "baz" { + name = "terraform-vpc-ruleset-insert" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "baz" { + name = "terraform-acl-ruleset-insert" + description = "terraform-acl-ruleset-insert-text" + vpc_id = cloudstack_vpc.baz.id +} + +resource "cloudstack_network_acl_rule" "baz" { + acl_id = cloudstack_network_acl.baz.id + + ruleset { + rule_number = 10 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH" + } + + # NEW RULE INSERTED IN THE MIDDLE + ruleset { + rule_number = 20 + action = "allow" + cidr_list = ["172.18.100.0/24"] protocol = "tcp" port = "80" traffic_type = "ingress" description = "Allow HTTP" } - rule { - cidr_list = ["172.16.100.0/24"] + ruleset { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] protocol = "tcp" port = "443" traffic_type = "ingress" description = "Allow HTTPS" } + + ruleset { + rule_number = 50 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "3306" + traffic_type = "ingress" + description = "Allow MySQL" + } }` -const testAccCloudStackNetworkACLRule_update = ` -resource "cloudstack_vpc" "foo" { - name = "terraform-vpc" +const testAccCloudStackNetworkACLRule_ruleset_plan_check_initial = ` +resource "cloudstack_vpc" "plan_check" { + name = "terraform-vpc-ruleset-plan-check" cidr = "10.0.0.0/8" vpc_offering = "Default VPC offering" zone = "Sandbox-simulator" } -resource "cloudstack_network_acl" "foo" { - name = "terraform-acl" - description = "terraform-acl-text" - vpc_id = cloudstack_vpc.foo.id +resource "cloudstack_network_acl" "plan_check" { + name = "terraform-acl-ruleset-plan-check" + description = "terraform-acl-ruleset-plan-check-text" + vpc_id = cloudstack_vpc.plan_check.id } -resource "cloudstack_network_acl_rule" "foo" { - acl_id = cloudstack_network_acl.foo.id +resource "cloudstack_network_acl_rule" "plan_check" { + acl_id = cloudstack_network_acl.plan_check.id - rule { - action = "deny" + ruleset { + rule_number = 10 + action = "allow" cidr_list = ["172.18.100.0/24"] - protocol = "all" + protocol = "tcp" + port = "22" traffic_type = "ingress" + description = "Allow SSH" } - rule { - action = "deny" - cidr_list = ["172.18.100.0/24", "172.18.101.0/24"] - protocol = "icmp" - icmp_type = "-1" - icmp_code = "-1" + ruleset { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "443" traffic_type = "ingress" - description = "Deny ICMP traffic" + description = "Allow HTTPS" } - rule { - action = "allow" + ruleset { + rule_number = 50 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "3306" + traffic_type = "ingress" + description = "Allow MySQL" + } +} +` + +const testAccCloudStackNetworkACLRule_ruleset_plan_check_insert = ` +resource "cloudstack_vpc" "plan_check" { + name = "terraform-vpc-ruleset-plan-check" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "plan_check" { + name = "terraform-acl-ruleset-plan-check" + description = "terraform-acl-ruleset-plan-check-text" + vpc_id = cloudstack_vpc.plan_check.id +} + +resource "cloudstack_network_acl_rule" "plan_check" { + acl_id = cloudstack_network_acl.plan_check.id + + ruleset { + rule_number = 10 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH" + } + + # NEW RULE INSERTED IN THE MIDDLE + ruleset { + rule_number = 20 + action = "allow" cidr_list = ["172.18.100.0/24"] protocol = "tcp" port = "80" traffic_type = "ingress" + description = "Allow HTTP" } - rule { - cidr_list = ["172.16.100.0/24"] + ruleset { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] protocol = "tcp" port = "443" traffic_type = "ingress" + description = "Allow HTTPS" } - rule { - action = "deny" - cidr_list = ["10.0.0.0/24"] + ruleset { + rule_number = 50 + action = "allow" + cidr_list = ["172.18.100.0/24"] protocol = "tcp" - port = "80" + port = "3306" + traffic_type = "ingress" + description = "Allow MySQL" + } +} +` + +func TestAccCloudStackNetworkACLRule_icmp_fields_no_spurious_diff(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRule_icmp_fields_config, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.foo", "ruleset.#", "3"), + ), + }, + { + // Second apply with same config should show no changes + Config: testAccCloudStackNetworkACLRule_icmp_fields_config, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + }, + }, + }) +} + +func TestAccCloudStackNetworkACLRule_icmp_fields_add_remove_rule(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // Step 1: Create with 2 rules + Config: testAccCloudStackNetworkACLRule_icmp_fields_two_rules, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.foo", "ruleset.#", "2"), + ), + }, + { + // Step 2: Add a third rule + Config: testAccCloudStackNetworkACLRule_icmp_fields_three_rules, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.foo", "ruleset.#", "3"), + ), + }, + { + // Step 3: Remove the third rule - should not cause spurious diff on remaining rules + Config: testAccCloudStackNetworkACLRule_icmp_fields_two_rules, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_rule.foo", "ruleset.#", "2"), + ), + }, + { + // Step 4: Plan should be empty after removing the rule + Config: testAccCloudStackNetworkACLRule_icmp_fields_two_rules, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + }, + }, + }) +} + +const testAccCloudStackNetworkACLRule_icmp_fields_config = ` +resource "cloudstack_vpc" "foo" { + name = "terraform-vpc" + display_text = "terraform-vpc" + cidr = "10.0.0.0/16" + zone = "Sandbox-simulator" + vpc_offering = "Default VPC offering" +} + +resource "cloudstack_network_acl" "foo" { + name = "terraform-acl" + vpc_id = cloudstack_vpc.foo.id +} + +resource "cloudstack_network_acl_rule" "foo" { + acl_id = cloudstack_network_acl.foo.id + + ruleset { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "all" + traffic_type = "ingress" + description = "Allow all ingress - protocol all with icmp_type=0, icmp_code=0 in config" + } + + ruleset { + rule_number = 20 + action = "allow" + cidr_list = ["10.0.0.0/8"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH - protocol tcp with icmp_type=0, icmp_code=0 in config" + } + + ruleset { + rule_number = 30 + action = "allow" + cidr_list = ["10.0.0.0/8"] + protocol = "icmp" + icmp_type = 8 + icmp_code = 0 + traffic_type = "ingress" + description = "Allow ICMP echo - protocol icmp with explicit icmp_type and icmp_code" + } +} +` + +const testAccCloudStackNetworkACLRule_icmp_fields_two_rules = ` +resource "cloudstack_vpc" "foo" { + name = "terraform-vpc-add-remove" + display_text = "terraform-vpc-add-remove" + cidr = "10.0.0.0/16" + zone = "Sandbox-simulator" + vpc_offering = "Default VPC offering" +} + +resource "cloudstack_network_acl" "foo" { + name = "terraform-acl-add-remove" + vpc_id = cloudstack_vpc.foo.id +} + +resource "cloudstack_network_acl_rule" "foo" { + acl_id = cloudstack_network_acl.foo.id + + ruleset { + rule_number = 10 + action = "allow" + cidr_list = ["10.0.0.0/8"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH ingress" + } + + ruleset { + rule_number = 100 + action = "allow" + cidr_list = ["10.0.0.0/8"] + protocol = "tcp" + port = "443" traffic_type = "egress" - description = "Deny specific TCP ports" + description = "Allow HTTPS egress" } +} +` - rule { - action = "deny" - cidr_list = ["10.0.0.0/24"] - protocol = "tcp" - port = "1000-2000" +const testAccCloudStackNetworkACLRule_icmp_fields_three_rules = ` +resource "cloudstack_vpc" "foo" { + name = "terraform-vpc-add-remove" + display_text = "terraform-vpc-add-remove" + cidr = "10.0.0.0/16" + zone = "Sandbox-simulator" + vpc_offering = "Default VPC offering" +} + +resource "cloudstack_network_acl" "foo" { + name = "terraform-acl-add-remove" + vpc_id = cloudstack_vpc.foo.id +} + +resource "cloudstack_network_acl_rule" "foo" { + acl_id = cloudstack_network_acl.foo.id + + ruleset { + rule_number = 10 + action = "allow" + cidr_list = ["10.0.0.0/8"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH ingress" + } + + ruleset { + rule_number = 20 + action = "allow" + cidr_list = ["10.0.0.0/8"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP ingress" + } + + ruleset { + rule_number = 100 + action = "allow" + cidr_list = ["10.0.0.0/8"] + protocol = "tcp" + port = "443" traffic_type = "egress" - description = "Deny specific TCP ports" + description = "Allow HTTPS egress" } -}` +} +` diff --git a/website/docs/r/network_acl_rule.html.markdown b/website/docs/r/network_acl_rule.html.markdown index 55bc6c94..fa487f9c 100644 --- a/website/docs/r/network_acl_rule.html.markdown +++ b/website/docs/r/network_acl_rule.html.markdown @@ -127,6 +127,61 @@ resource "cloudstack_network_acl_rule" "web_server" { description = "Allow all outbound TCP" } } +``` + +### Using `ruleset` for Better Change Management + +The `ruleset` field is recommended when you need to insert or remove rules without +triggering unnecessary updates to other rules. Unlike `rule` (which uses a list), +`ruleset` uses a set that identifies rules by their content rather than position. + +**Key differences:** +- `ruleset` requires `rule_number` on all rules (no auto-numbering) +- `ruleset` does not support the deprecated `ports` field (use `port` instead) +- Inserting a rule in the middle only creates that one rule, without updating others + +```hcl +resource "cloudstack_network_acl_rule" "web_server_set" { + acl_id = "f3843ce0-334c-4586-bbd3-0c2e2bc946c6" + + # HTTP traffic + ruleset { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + # HTTPS traffic + ruleset { + rule_number = 20 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } + + # SSH from management network + ruleset { + rule_number = 30 + action = "allow" + cidr_list = ["192.168.100.0/24"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH from management" + } +} +``` + +**Note:** You cannot use both `rule` and `ruleset` in the same resource. Choose one based on your needs: +- Use `rule` if you want auto-numbering and don't mind Terraform showing updates when inserting rules +- Use `ruleset` if you frequently insert/remove rules and want minimal plan changes ## Argument Reference @@ -140,7 +195,15 @@ The following arguments are supported: all firewall rules that are not in your config! (defaults false) * `rule` - (Optional) Can be specified multiple times. Each rule block supports - fields documented below. If `managed = false` at least one rule is required! + fields documented below. If `managed = false` at least one rule or ruleset is required! + **Cannot be used together with `ruleset`.** + +* `ruleset` - (Optional) Can be specified multiple times. Similar to `rule` but uses + a set instead of a list, which prevents spurious updates when inserting rules. + Each ruleset block supports the same fields as `rule` (documented below), with these differences: + - `rule_number` is **required** (no auto-numbering) + - `ports` field is not supported (use `port` instead) + **Cannot be used together with `rule`.** * `project` - (Optional) The name or ID of the project to deploy this instance to. Changing this forces a new resource to be created. @@ -148,9 +211,14 @@ The following arguments are supported: * `parallelism` (Optional) Specifies how much rules will be created or deleted concurrently. (defaults 2) -The `rule` block supports: +The `rule` and `ruleset` blocks support: -* `rule_number` - (Optional) The number of the ACL item used to order the ACL rules. The ACL rule with the lowest number has the highest priority. If not specified, the ACL item will be created with a number one greater than the highest numbered rule. +* `rule_number` - (Optional for `rule`, **Required** for `ruleset`) The number of the ACL + item used to order the ACL rules. The ACL rule with the lowest number has the highest + priority. + - For `rule`: If not specified, the ACL item will be created with a number one greater + than the highest numbered rule or the previous rule in the list. + - For `ruleset`: Must be specified for all rules (no auto-numbering). * `action` - (Optional) The action for the rule. Valid options are: `allow` and `deny` (defaults allow). @@ -166,15 +234,15 @@ The `rule` block supports: * `icmp_code` - (Optional) The ICMP code to allow, or `-1` to allow `any`. This can only be specified if the protocol is ICMP. (defaults 0) -* `port` - (Optional) Port or port range to allow. This can only be specified if +* `port` - (Optional) Port or port range to allow. This can only be specified if the protocol is TCP, UDP, ALL or a valid protocol number. Valid formats are: - Single port: `"80"` - Port range: `"8000-8010"` - If not specified for TCP/UDP, allows all ports for that protocol -* `ports` - (Optional) **DEPRECATED**: Use `port` instead. List of ports and/or - port ranges to allow. This field is deprecated and will be removed in a future - version. For backward compatibility only. +* `ports` - (Optional) **DEPRECATED**: Use `port` instead. List of ports and/or + port ranges to allow. This field is deprecated and will be removed in a future + version. For backward compatibility only. **Not available in `ruleset`.** * `traffic_type` - (Optional) The traffic type for the rule. Valid options are: `ingress` or `egress` (defaults ingress). From 7b69a116e77616489c735120e908216e14950bb5 Mon Sep 17 00:00:00 2001 From: Brad House - Nexthop Date: Tue, 10 Mar 2026 14:39:32 +0000 Subject: [PATCH 3/9] Fix documentation: clarify ruleset identifies by rule_number, not content - Update docs to state ruleset identifies rules by rule_number (not full content) - Add explicit requirement that each rule_number must be unique - Addresses review comment about misleading documentation --- website/docs/r/network_acl_rule.html.markdown | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/docs/r/network_acl_rule.html.markdown b/website/docs/r/network_acl_rule.html.markdown index fa487f9c..716a1109 100644 --- a/website/docs/r/network_acl_rule.html.markdown +++ b/website/docs/r/network_acl_rule.html.markdown @@ -133,10 +133,11 @@ resource "cloudstack_network_acl_rule" "web_server" { The `ruleset` field is recommended when you need to insert or remove rules without triggering unnecessary updates to other rules. Unlike `rule` (which uses a list), -`ruleset` uses a set that identifies rules by their content rather than position. +`ruleset` uses a set that identifies rules by their `rule_number` rather than position. **Key differences:** - `ruleset` requires `rule_number` on all rules (no auto-numbering) +- Each `rule_number` must be unique within the ruleset - `ruleset` does not support the deprecated `ports` field (use `port` instead) - Inserting a rule in the middle only creates that one rule, without updating others From d65a84515055e5ed839a9d469a8c3945a448f9ad Mon Sep 17 00:00:00 2001 From: Brad House - Nexthop Date: Tue, 10 Mar 2026 14:42:23 +0000 Subject: [PATCH 4/9] Document TypeSet deduplication behavior for ruleset - Clarify that duplicate rule_number values will result in last-one-wins behavior - This is standard TypeSet behavior in Terraform, not a bug - Validation cannot prevent this as TypeSet deduplicates before CustomizeDiff runs Review Comment 1 Rebuttal: The review suggests adding CustomizeDiff validation to detect duplicate rule_number values. However, this is not possible because Terraform's TypeSet automatically deduplicates entries based on the hash function BEFORE CustomizeDiff runs. Since we hash on rule_number (the unique identifier), duplicate rule_numbers are automatically deduplicated with last-one-wins behavior. This is the INTENDED behavior of TypeSet - rule_number acts as a primary key, and having multiple rules with the same rule_number should result in one overwriting the other. This is analogous to how a map works in most programming languages. The documentation has been updated to make this behavior explicit and warn users that duplicate rule_numbers will result in only the last one being kept. --- website/docs/r/network_acl_rule.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/network_acl_rule.html.markdown b/website/docs/r/network_acl_rule.html.markdown index 716a1109..fd1ea0f5 100644 --- a/website/docs/r/network_acl_rule.html.markdown +++ b/website/docs/r/network_acl_rule.html.markdown @@ -137,7 +137,7 @@ triggering unnecessary updates to other rules. Unlike `rule` (which uses a list) **Key differences:** - `ruleset` requires `rule_number` on all rules (no auto-numbering) -- Each `rule_number` must be unique within the ruleset +- Each `rule_number` must be unique within the ruleset; if you define multiple rules with the same `rule_number`, only the last one will be kept (Terraform's TypeSet behavior) - `ruleset` does not support the deprecated `ports` field (use `port` instead) - Inserting a rule in the middle only creates that one rule, without updating others From 20b8359ff9c278cb8aca7a43d68d330614cdca36 Mon Sep 17 00:00:00 2001 From: Brad House - Nexthop Date: Tue, 10 Mar 2026 14:43:41 +0000 Subject: [PATCH 5/9] Fix assignRuleNumbers to prevent duplicate/decreasing rule numbers - Ensure nextNumber never moves backwards when encountering explicit rule_number - Use max(nextNumber, ruleNum+1) logic to prevent duplicates - Prevents scenario where auto-assigned rule gets same number as later explicit rule - Addresses review comment about potential duplicate rule numbers --- cloudstack/resource_cloudstack_network_acl_rule.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cloudstack/resource_cloudstack_network_acl_rule.go b/cloudstack/resource_cloudstack_network_acl_rule.go index 99fda022..59410e71 100644 --- a/cloudstack/resource_cloudstack_network_acl_rule.go +++ b/cloudstack/resource_cloudstack_network_acl_rule.go @@ -395,7 +395,7 @@ func isRulesetRule(rule map[string]interface{}) bool { // assignRuleNumbers assigns rule numbers to rules that don't have them // Rules are numbered sequentially starting from 1 -// If a rule has an explicit rule_number, numbering continues from that value +// If a rule has an explicit rule_number, nextNumber advances to ensure no duplicates func assignRuleNumbers(rules []interface{}) []interface{} { result := make([]interface{}, len(rules)) nextNumber := 1 @@ -409,9 +409,12 @@ func assignRuleNumbers(rules []interface{}) []interface{} { // Check if rule_number is set if ruleNum, ok := ruleMap["rule_number"].(int); ok && ruleNum > 0 { - // Rule has explicit number, use it and continue from there - nextNumber = ruleNum + 1 - log.Printf("[DEBUG] Rule at index %d has explicit rule_number=%d", i, ruleNum) + // Rule has explicit number, ensure nextNumber never decreases + // to prevent duplicate or decreasing rule numbers + if ruleNum >= nextNumber { + nextNumber = ruleNum + 1 + } + log.Printf("[DEBUG] Rule at index %d has explicit rule_number=%d, nextNumber=%d", i, ruleNum, nextNumber) } else { // Auto-assign sequential number ruleMap["rule_number"] = nextNumber From bdfa4306d369bfef173e7e9c3faee9e74870daab Mon Sep 17 00:00:00 2001 From: Brad House - Nexthop Date: Tue, 10 Mar 2026 14:46:02 +0000 Subject: [PATCH 6/9] Fix managed=true unknown-rule handling for ruleset - Adjust dummy rule format for ruleset to use 'uuid' string instead of 'uuids' map - Add rule_number to dummy rules for ruleset (required field) - Find highest existing rule_number and assign dummy rules starting from max+1 - Prevents conflicts between dummy rules and user-defined rules - Addresses review comment about managed mode incompatibility with ruleset --- .../resource_cloudstack_network_acl_rule.go | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/cloudstack/resource_cloudstack_network_acl_rule.go b/cloudstack/resource_cloudstack_network_acl_rule.go index 59410e71..56136c1d 100644 --- a/cloudstack/resource_cloudstack_network_acl_rule.go +++ b/cloudstack/resource_cloudstack_network_acl_rule.go @@ -954,21 +954,48 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface // If this is a managed firewall, add all unknown rules into dummy rules managed := d.Get("managed").(bool) if managed && len(ruleMap) > 0 { + // Find the highest rule_number to avoid conflicts when creating dummy rules + maxRuleNumber := 0 + for _, rule := range rules { + if ruleMap, ok := rule.(map[string]interface{}); ok { + if ruleNum, ok := ruleMap["rule_number"].(int); ok && ruleNum > maxRuleNumber { + maxRuleNumber = ruleNum + } + } + } + + // Start assigning dummy rule numbers after the highest existing rule_number + dummyRuleNumber := maxRuleNumber + 1 + for uuid := range ruleMap { // We need to create and add a dummy value to a list as the // cidr_list is a required field and thus needs a value cidrs := []interface{}{uuid} // Make a dummy rule to hold the unknown UUID - rule := map[string]interface{}{ - "cidr_list": cidrs, - "protocol": uuid, - "uuids": map[string]interface{}{uuid: uuid}, + // Format differs between 'rule' and 'ruleset' + var rule map[string]interface{} + if usingRuleset { + // For ruleset: use 'uuid' string and include rule_number + rule = map[string]interface{}{ + "cidr_list": cidrs, + "protocol": uuid, + "uuid": uuid, + "rule_number": dummyRuleNumber, + } + dummyRuleNumber++ + } else { + // For rule: use 'uuids' map + rule = map[string]interface{}{ + "cidr_list": cidrs, + "protocol": uuid, + "uuids": map[string]interface{}{uuid: uuid}, + } } // Add the dummy rule to the rules list rules = append(rules, rule) - log.Printf("[DEBUG] Added managed dummy rule for UUID %s", uuid) + log.Printf("[DEBUG] Added managed dummy rule for UUID %s (usingRuleset=%t)", uuid, usingRuleset) } } From 4efb17529e4d1cdebd5db0c49f4d563b5cdf790b Mon Sep 17 00:00:00 2001 From: Brad House - Nexthop Date: Tue, 10 Mar 2026 14:48:50 +0000 Subject: [PATCH 7/9] Strengthen plan check assertions for ruleset insertion test - Add granular plan check using plancheck.ExpectKnownValue to verify that exactly one nested block is added (ruleset.# changes from 3 to 4) - This provides more specific validation than just checking for Update action - Addresses review comment about strengthening plan check assertions --- cloudstack/resource_cloudstack_network_acl_rule_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cloudstack/resource_cloudstack_network_acl_rule_test.go b/cloudstack/resource_cloudstack_network_acl_rule_test.go index 7a0baf23..928e0bbb 100644 --- a/cloudstack/resource_cloudstack_network_acl_rule_test.go +++ b/cloudstack/resource_cloudstack_network_acl_rule_test.go @@ -26,8 +26,10 @@ import ( "github.com/apache/cloudstack-go/v2/cloudstack" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" ) func TestAccCloudStackNetworkACLRule_basic(t *testing.T) { @@ -565,6 +567,12 @@ func TestAccCloudStackNetworkACLRule_ruleset_insert_plan_check(t *testing.T) { // Verify that only 1 rule is being added (the new rule 20) // and the existing rules (10, 30, 50) are not being modified plancheck.ExpectResourceAction("cloudstack_network_acl_rule.plan_check", plancheck.ResourceActionUpdate), + // Verify that ruleset.# is changing from 3 to 4 (exactly one block added) + plancheck.ExpectKnownValue( + "cloudstack_network_acl_rule.plan_check", + tfjsonpath.New("ruleset"), + knownvalue.SetSizeExact(4), + ), }, }, Check: resource.ComposeTestCheckFunc( From 3777e462b9283498e3aa575a8b16a6d14c0d1086 Mon Sep 17 00:00:00 2001 From: Brad House - Nexthop Date: Wed, 11 Mar 2026 10:06:40 -0400 Subject: [PATCH 8/9] Fix rule_number documentation For the legacy rule list, better describe how the rule number is generated. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- website/docs/r/network_acl_rule.html.markdown | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/website/docs/r/network_acl_rule.html.markdown b/website/docs/r/network_acl_rule.html.markdown index fd1ea0f5..b1dd5da6 100644 --- a/website/docs/r/network_acl_rule.html.markdown +++ b/website/docs/r/network_acl_rule.html.markdown @@ -217,8 +217,9 @@ The `rule` and `ruleset` blocks support: * `rule_number` - (Optional for `rule`, **Required** for `ruleset`) The number of the ACL item used to order the ACL rules. The ACL rule with the lowest number has the highest priority. - - For `rule`: If not specified, the ACL item will be created with a number one greater - than the highest numbered rule or the previous rule in the list. + - For `rule`: If not specified, the provider will auto-assign rule numbers starting at 1, + increasing sequentially in the order the rules are defined and filling any gaps, rather + than basing the number on the highest existing rule in the ACL. - For `ruleset`: Must be specified for all rules (no auto-numbering). * `action` - (Optional) The action for the rule. Valid options are: `allow` and From bf3db663abebb70d6616a79d0a38600f78e9fd3f Mon Sep 17 00:00:00 2001 From: Brad House - Nexthop Date: Wed, 11 Mar 2026 14:25:04 +0000 Subject: [PATCH 9/9] Fix test helpers to properly validate ruleset format The testAccCheckCloudStackNetworkACLRulesExist and testAccCheckCloudStackNetworkACLRuleDestroy helper functions were only checking for legacy 'rule' format UUIDs (stored in rule.*.uuids.). This commit updates both helpers to: 1. Check both legacy 'rule' format (rule.*.uuids.) and new 'ruleset' format (ruleset.*.uuid) UUIDs 2. Verify that at least one rule UUID was found and validated 3. Fix all test cases to check the correct resource (cloudstack_network_acl_rule.* instead of cloudstack_network_acl.*) This ensures that ruleset tests actually verify rule existence in CloudStack, catching create/read failures that were previously missed. --- ...source_cloudstack_network_acl_rule_test.go | 83 ++++++++++++------- 1 file changed, 55 insertions(+), 28 deletions(-) diff --git a/cloudstack/resource_cloudstack_network_acl_rule_test.go b/cloudstack/resource_cloudstack_network_acl_rule_test.go index 928e0bbb..9b9a711a 100644 --- a/cloudstack/resource_cloudstack_network_acl_rule_test.go +++ b/cloudstack/resource_cloudstack_network_acl_rule_test.go @@ -42,7 +42,7 @@ func TestAccCloudStackNetworkACLRule_basic(t *testing.T) { Config: testAccCloudStackNetworkACLRule_basic, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.foo"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.foo", "rule.#", "4"), // Don't rely on specific rule ordering as TypeSet doesn't guarantee order @@ -96,7 +96,7 @@ func TestAccCloudStackNetworkACLRule_update(t *testing.T) { { Config: testAccCloudStackNetworkACLRule_basic, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.foo"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.foo", "rule.#", "4"), // Don't rely on specific rule ordering as TypeSet doesn't guarantee order @@ -140,7 +140,7 @@ func TestAccCloudStackNetworkACLRule_update(t *testing.T) { { Config: testAccCloudStackNetworkACLRule_update, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.foo"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.foo", "rule.#", "6"), // Check for the expected rules using TypeSet elem matching @@ -206,23 +206,43 @@ func testAccCheckCloudStackNetworkACLRulesExist(n string) resource.TestCheckFunc return fmt.Errorf("No network ACL rule ID is set") } + cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) + foundRules := 0 + for k, id := range rs.Primary.Attributes { - if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.%") { - continue + // Check for legacy 'rule' format: rule.*.uuids. + if strings.Contains(k, ".uuids.") && !strings.HasSuffix(k, ".uuids.%") { + _, count, err := cs.NetworkACL.GetNetworkACLByID(id) + + if err != nil { + return err + } + + if count == 0 { + return fmt.Errorf("Network ACL rule %s not found", k) + } + foundRules++ } - cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) - _, count, err := cs.NetworkACL.GetNetworkACLByID(id) + // Check for new 'ruleset' format: ruleset.*.uuid + if strings.Contains(k, "ruleset.") && strings.HasSuffix(k, ".uuid") && id != "" { + _, count, err := cs.NetworkACL.GetNetworkACLByID(id) - if err != nil { - return err - } + if err != nil { + return err + } - if count == 0 { - return fmt.Errorf("Network ACL rule %s not found", k) + if count == 0 { + return fmt.Errorf("Network ACL rule %s not found", k) + } + foundRules++ } } + if foundRules == 0 { + return fmt.Errorf("No network ACL rules found in state for %s", n) + } + return nil } } @@ -240,13 +260,20 @@ func testAccCheckCloudStackNetworkACLRuleDestroy(s *terraform.State) error { } for k, id := range rs.Primary.Attributes { - if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.%") { - continue + // Check for legacy 'rule' format: rule.*.uuids. + if strings.Contains(k, ".uuids.") && !strings.HasSuffix(k, ".uuids.%") { + _, _, err := cs.NetworkACL.GetNetworkACLByID(id) + if err == nil { + return fmt.Errorf("Network ACL rule %s still exists", rs.Primary.ID) + } } - _, _, err := cs.NetworkACL.GetNetworkACLByID(id) - if err == nil { - return fmt.Errorf("Network ACL rule %s still exists", rs.Primary.ID) + // Check for new 'ruleset' format: ruleset.*.uuid + if strings.Contains(k, "ruleset.") && strings.HasSuffix(k, ".uuid") && id != "" { + _, _, err := cs.NetworkACL.GetNetworkACLByID(id) + if err == nil { + return fmt.Errorf("Network ACL rule %s still exists", rs.Primary.ID) + } } } } @@ -263,7 +290,7 @@ func TestAccCloudStackNetworkACLRule_ruleset_basic(t *testing.T) { { Config: testAccCloudStackNetworkACLRule_ruleset_basic, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.bar"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.bar"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.bar", "ruleset.#", "4"), // Check for the expected rules using TypeSet elem matching @@ -318,7 +345,7 @@ func TestAccCloudStackNetworkACLRule_ruleset_update(t *testing.T) { { Config: testAccCloudStackNetworkACLRule_ruleset_basic, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.bar"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.bar"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.bar", "ruleset.#", "4"), resource.TestCheckTypeSetElemNestedAttrs( @@ -363,7 +390,7 @@ func TestAccCloudStackNetworkACLRule_ruleset_update(t *testing.T) { { Config: testAccCloudStackNetworkACLRule_ruleset_update, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.bar"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.bar"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.bar", "ruleset.#", "6"), // Check for the expected rules using TypeSet elem matching @@ -433,7 +460,7 @@ func TestAccCloudStackNetworkACLRule_ruleset_insert(t *testing.T) { { Config: testAccCloudStackNetworkACLRule_ruleset_insert_initial, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.baz"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.baz"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.baz", "ruleset.#", "3"), // Initial rules: 10, 30, 50 @@ -470,7 +497,7 @@ func TestAccCloudStackNetworkACLRule_ruleset_insert(t *testing.T) { { Config: testAccCloudStackNetworkACLRule_ruleset_insert_middle, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.baz"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.baz"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.baz", "ruleset.#", "4"), // After inserting rule 20 in the middle, all original rules should still exist @@ -526,7 +553,7 @@ func TestAccCloudStackNetworkACLRule_ruleset_insert_plan_check(t *testing.T) { { Config: testAccCloudStackNetworkACLRule_ruleset_plan_check_initial, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.plan_check"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.plan_check"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.plan_check", "ruleset.#", "3"), // Initial rules: 10, 30, 50 @@ -576,7 +603,7 @@ func TestAccCloudStackNetworkACLRule_ruleset_insert_plan_check(t *testing.T) { }, }, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.plan_check"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.plan_check"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.plan_check", "ruleset.#", "4"), // After inserting rule 20 in the middle, all original rules should still exist @@ -1104,7 +1131,7 @@ func TestAccCloudStackNetworkACLRule_icmp_fields_no_spurious_diff(t *testing.T) { Config: testAccCloudStackNetworkACLRule_icmp_fields_config, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.foo"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.foo", "ruleset.#", "3"), ), @@ -1131,7 +1158,7 @@ func TestAccCloudStackNetworkACLRule_icmp_fields_add_remove_rule(t *testing.T) { // Step 1: Create with 2 rules Config: testAccCloudStackNetworkACLRule_icmp_fields_two_rules, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.foo"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.foo", "ruleset.#", "2"), ), @@ -1140,7 +1167,7 @@ func TestAccCloudStackNetworkACLRule_icmp_fields_add_remove_rule(t *testing.T) { // Step 2: Add a third rule Config: testAccCloudStackNetworkACLRule_icmp_fields_three_rules, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.foo"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.foo", "ruleset.#", "3"), ), @@ -1149,7 +1176,7 @@ func TestAccCloudStackNetworkACLRule_icmp_fields_add_remove_rule(t *testing.T) { // Step 3: Remove the third rule - should not cause spurious diff on remaining rules Config: testAccCloudStackNetworkACLRule_icmp_fields_two_rules, Check: resource.ComposeTestCheckFunc( - testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"), + testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl_rule.foo"), resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.foo", "ruleset.#", "2"), ),