diff --git a/cloudstack/provider.go b/cloudstack/provider.go index 72090147..1ea5ec34 100644 --- a/cloudstack/provider.go +++ b/cloudstack/provider.go @@ -130,6 +130,7 @@ func Provider() *schema.Provider { "cloudstack_network": resourceCloudStackNetwork(), "cloudstack_network_acl": resourceCloudStackNetworkACL(), "cloudstack_network_acl_rule": resourceCloudStackNetworkACLRule(), + "cloudstack_network_acl_ruleset": resourceCloudStackNetworkACLRuleset(), "cloudstack_nic": resourceCloudStackNIC(), "cloudstack_physical_network": resourceCloudStackPhysicalNetwork(), "cloudstack_pod": resourceCloudStackPod(), diff --git a/cloudstack/resource_cloudstack_network_acl_rule.go b/cloudstack/resource_cloudstack_network_acl_rule.go index 839f3e8a..70f6e720 100644 --- a/cloudstack/resource_cloudstack_network_acl_rule.go +++ b/cloudstack/resource_cloudstack_network_acl_rule.go @@ -44,6 +44,7 @@ func resourceCloudStackNetworkACLRule() *schema.Resource { Importer: &schema.ResourceImporter{ State: resourceCloudStackNetworkACLRuleImport, }, + DeprecationMessage: "cloudstack_network_acl_rule is deprecated. Use cloudstack_network_acl_ruleset instead for better performance and in-place updates.", CustomizeDiff: func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) error { // Force replacement for migration from deprecated 'ports' to 'port' field if diff.HasChange("rule") { diff --git a/cloudstack/resource_cloudstack_network_acl_ruleset.go b/cloudstack/resource_cloudstack_network_acl_ruleset.go new file mode 100644 index 00000000..d9a654fd --- /dev/null +++ b/cloudstack/resource_cloudstack_network_acl_ruleset.go @@ -0,0 +1,1026 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package cloudstack + +import ( + "context" + "fmt" + "log" + "strconv" + "strings" + "sync" + "time" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func resourceCloudStackNetworkACLRuleset() *schema.Resource { + return &schema.Resource{ + Create: resourceCloudStackNetworkACLRulesetCreate, + Read: resourceCloudStackNetworkACLRulesetRead, + Update: resourceCloudStackNetworkACLRulesetUpdate, + Delete: resourceCloudStackNetworkACLRulesetDelete, + Importer: &schema.ResourceImporter{ + State: resourceCloudStackNetworkACLRulesetImport, + }, + // CustomizeDiff is used to eliminate spurious diffs when modifying individual rules. + // Without this, changing a single rule (e.g., port 80->8080) would show ALL rules + // as being replaced in the plan because TypeSet uses hashing and any field change + // changes the hash. This function matches rules by their natural key (rule_number) + // and uses SetNew to suppress diffs for unchanged rules. + CustomizeDiff: resourceCloudStackNetworkACLRulesetCustomizeDiff, + + Schema: map[string]*schema.Schema{ + "acl_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + + "managed": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + + "project": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + + "rule": { + Type: schema.TypeSet, + Optional: true, + // Computed: true is required to allow CustomizeDiff to use SetNew(). + // Without this, we get "Error: SetNew only operates on computed keys". + // This enables CustomizeDiff to suppress spurious diffs by preserving + // the old state for unchanged rules. + Computed: true, + 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.TypeSet, + Required: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + }, + + "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, + }, + + "traffic_type": { + Type: schema.TypeString, + Optional: true, + Default: "ingress", + }, + + "description": { + Type: schema.TypeString, + Optional: true, + }, + + "uuid": { + Type: schema.TypeString, + Computed: true, + }, + }, + }, + }, + }, + } +} + +func resourceCloudStackNetworkACLRulesetCustomizeDiff(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + // Only apply this logic during updates, not creates + if d.Id() == "" { + return nil + } + + old, new := d.GetChange("rule") + oldSet := old.(*schema.Set) + newSet := new.(*schema.Set) + + // If the sets are empty, nothing to do + if oldSet.Len() == 0 || newSet.Len() == 0 { + return nil + } + + // Create maps indexed by rule_number (the natural key) + oldMap := make(map[int]map[string]interface{}) + for _, v := range oldSet.List() { + m := v.(map[string]interface{}) + ruleNum := m["rule_number"].(int) + oldMap[ruleNum] = m + } + + newMap := make(map[int]map[string]interface{}) + for _, v := range newSet.List() { + m := v.(map[string]interface{}) + ruleNum := m["rule_number"].(int) + newMap[ruleNum] = m + } + + // Build a new set that preserves UUIDs for unchanged rules + // and uses new values for changed/added rules + preservedSet := schema.NewSet(newSet.F, []interface{}{}) + + for ruleNum, newRule := range newMap { + oldRule, exists := oldMap[ruleNum] + + if exists && compareACLRules(oldRule, newRule) { + // Rule exists and is functionally identical - preserve the old rule + // (including its UUID) to prevent spurious diff + preservedSet.Add(oldRule) + } else { + // Rule is new or changed - use the new rule + preservedSet.Add(newRule) + } + } + + // Set the preserved set as the new value + // This maintains UUIDs for unchanged rules while allowing changes to show correctly + return d.SetNew("rule", preservedSet) +} + +func compareACLRules(old, new map[string]interface{}) bool { + // Compare all fields except uuid (which is computed and may differ) + fields := []string{"rule_number", "action", "protocol", "icmp_type", "icmp_code", "port", "traffic_type", "description"} + + for _, field := range fields { + oldVal := old[field] + newVal := new[field] + + // Handle nil/empty string equivalence + if oldVal == nil && newVal == "" { + continue + } + if oldVal == "" && newVal == nil { + continue + } + + if oldVal != newVal { + return false + } + } + + // Compare cidr_list (TypeSet) + oldCIDR := old["cidr_list"].(*schema.Set) + newCIDR := new["cidr_list"].(*schema.Set) + + if !oldCIDR.Equal(newCIDR) { + return false + } + + return true +} + +func resourceCloudStackNetworkACLRulesetCreate(d *schema.ResourceData, meta interface{}) error { + // We need to set this upfront in order to be able to save a partial state + d.SetId(d.Get("acl_id").(string)) + + // Create all rules that are configured + if nrs := d.Get("rule").(*schema.Set); nrs.Len() > 0 { + // Create an empty schema.Set to hold all rules + rules := resourceCloudStackNetworkACLRuleset().Schema["rule"].ZeroValue().(*schema.Set) + + err := createACLRules(d, meta, rules, nrs) + if err != nil { + return err + } + + // We need to update this first to preserve the correct state + d.Set("rule", rules) + } + + return resourceCloudStackNetworkACLRulesetRead(d, meta) +} + +func createACLRules(d *schema.ResourceData, meta interface{}, rules *schema.Set, nrs *schema.Set) error { + var errs *multierror.Error + var mu sync.Mutex + + var wg sync.WaitGroup + wg.Add(nrs.Len()) + + sem := make(chan struct{}, 10) + for _, rule := range nrs.List() { + // Put in a tiny sleep here to avoid DoS'ing the API + time.Sleep(500 * time.Millisecond) + + go func(rule map[string]interface{}) { + defer wg.Done() + sem <- struct{}{} + + // Create a single rule + err := createACLRule(d, meta, rule) + + // If we have a UUID, we need to save the rule + if uuid, ok := rule["uuid"].(string); ok && uuid != "" { + mu.Lock() + rules.Add(rule) + mu.Unlock() + } + + if err != nil { + mu.Lock() + errs = multierror.Append(errs, err) + mu.Unlock() + } + + <-sem + }(rule.(map[string]interface{})) + } + + wg.Wait() + + return errs.ErrorOrNil() +} + +func createACLRule(d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error { + cs := meta.(*cloudstack.CloudStackClient) + + // Make sure all required parameters are there + if err := verifyACLRuleParams(d, rule); err != nil { + return err + } + + protocol := rule["protocol"].(string) + action := rule["action"].(string) + trafficType := rule["traffic_type"].(string) + + // Create a new parameter struct + p := cs.NetworkACL.NewCreateNetworkACLParams(protocol) + + // Set the rule number + p.SetNumber(rule["rule_number"].(int)) + + // Set the acl ID + p.SetAclid(d.Get("acl_id").(string)) + + // Set the action + p.SetAction(action) + + // Set the CIDR list + var cidrList []string + for _, cidr := range rule["cidr_list"].(*schema.Set).List() { + cidrList = append(cidrList, cidr.(string)) + } + p.SetCidrlist(cidrList) + + // Set the traffic type + p.SetTraffictype(trafficType) + + // Set the description + if desc, ok := rule["description"].(string); ok && desc != "" { + p.SetReason(desc) + } + + // If the protocol is ICMP set the needed ICMP parameters + if protocol == "icmp" { + p.SetIcmptype(rule["icmp_type"].(int)) + p.SetIcmpcode(rule["icmp_code"].(int)) + + r, err := Retry(4, retryableACLCreationFunc(cs, p)) + if err != nil { + return err + } + + rule["uuid"] = r.(*cloudstack.CreateNetworkACLResponse).Id + return nil + } + + // If the protocol is ALL set the needed parameters + if protocol == "all" { + r, err := Retry(4, retryableACLCreationFunc(cs, p)) + if err != nil { + return err + } + + rule["uuid"] = r.(*cloudstack.CreateNetworkACLResponse).Id + return nil + } + + // If protocol is TCP or UDP, create the rule (with or without port) + if protocol == "tcp" || protocol == "udp" { + portStr, hasPort := rule["port"].(string) + + if hasPort && portStr != "" { + // Handle single port + m := splitPorts.FindStringSubmatch(portStr) + if m == nil { + return fmt.Errorf("%q is not a valid port value. Valid options are '80' or '80-90'", portStr) + } + + startPort, err := strconv.Atoi(m[1]) + if err != nil { + return err + } + + endPort := startPort + if m[2] != "" { + endPort, err = strconv.Atoi(m[2]) + if err != nil { + return err + } + } + + p.SetStartport(startPort) + p.SetEndport(endPort) + } + + r, err := Retry(4, retryableACLCreationFunc(cs, p)) + if err != nil { + return err + } + + rule["uuid"] = r.(*cloudstack.CreateNetworkACLResponse).Id + return nil + } + + // If we reach here, it's an unsupported protocol (should have been caught by validation) + return fmt.Errorf("unsupported protocol %q. Valid protocols are: tcp, udp, icmp, all", protocol) +} + +// buildRuleFromAPI converts a CloudStack NetworkACL API response to a rule map +func buildRuleFromAPI(r *cloudstack.NetworkACL) map[string]interface{} { + cidrs := &schema.Set{F: schema.HashString} + for _, cidr := range strings.Split(r.Cidrlist, ",") { + cidrs.Add(cidr) + } + + rule := map[string]interface{}{ + "cidr_list": cidrs, + "action": strings.ToLower(r.Action), + "protocol": r.Protocol, + "traffic_type": strings.ToLower(r.Traffictype), + "rule_number": r.Number, + "description": r.Reason, + "uuid": r.Id, + } + + // Set ICMP fields + if r.Protocol == "icmp" { + rule["icmp_type"] = r.Icmptype + rule["icmp_code"] = r.Icmpcode + } else { + rule["icmp_type"] = 0 + rule["icmp_code"] = 0 + } + + // Set port if applicable + if r.Protocol == "tcp" || r.Protocol == "udp" { + if r.Startport != "" && r.Endport != "" { + if r.Startport == r.Endport { + rule["port"] = r.Startport + } else { + rule["port"] = fmt.Sprintf("%s-%s", r.Startport, r.Endport) + } + } else { + // Explicitly clear port when no ports are set (all ports) + rule["port"] = "" + } + } else { + // Explicitly clear port when protocol is not tcp/udp + rule["port"] = "" + } + + return rule +} + +func resourceCloudStackNetworkACLRulesetRead(d *schema.ResourceData, meta interface{}) error { + cs := meta.(*cloudstack.CloudStackClient) + + // First check if the ACL itself still exists + _, count, err := cs.NetworkACL.GetNetworkACLListByID( + d.Id(), + cloudstack.WithProject(d.Get("project").(string)), + ) + if err != nil { + if count == 0 { + log.Printf("[DEBUG] Network ACL list %s does not exist", d.Id()) + d.SetId("") + return nil + } + return err + } + + // Get all the rules from the running environment + p := cs.NetworkACL.NewListNetworkACLsParams() + p.SetAclid(d.Id()) + p.SetListall(true) + + if err := setProjectid(p, cs, d); err != nil { + return err + } + + l, err := cs.NetworkACL.ListNetworkACLs(p) + if err != nil { + return err + } + + // Make a map of all the rules so we can easily find a rule + ruleMap := make(map[string]*cloudstack.NetworkACL, l.Count) + for _, r := range l.NetworkACLs { + ruleMap[r.Id] = r + } + + // Create an empty schema.Set to hold all rules + rules := resourceCloudStackNetworkACLRuleset().Schema["rule"].ZeroValue().(*schema.Set) + + // Read all rules that are configured + rs := d.Get("rule").(*schema.Set) + if rs.Len() > 0 { + for _, oldRule := range rs.List() { + oldRule := oldRule.(map[string]interface{}) + + id, ok := oldRule["uuid"] + if !ok || id.(string) == "" { + continue + } + + // Get the rule + r, ok := ruleMap[id.(string)] + if !ok { + // Rule no longer exists in the API, skip it + continue + } + + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id.(string)) + + // Create a NEW map with the updated values (don't mutate the old one) + rule := buildRuleFromAPI(r) + rules.Add(rule) + } + } else { + // If no rules in state (e.g., during import), read all remote rules + for _, r := range ruleMap { + rule := buildRuleFromAPI(r) + rules.Add(rule) + // Remove from ruleMap so we don't add it again as a dummy rule + delete(ruleMap, r.Id) + } + } + + // If this is a managed resource, add all unknown rules to dummy rules + // This allows Terraform to detect them and trigger an update to delete them + managed := d.Get("managed").(bool) + if managed && len(ruleMap) > 0 { + log.Printf("[DEBUG] Found %d out-of-band ACL rules for ACL %s", len(ruleMap), d.Id()) + for uuid, r := range ruleMap { + log.Printf("[DEBUG] Adding dummy rule for out-of-band rule: uuid=%s, rule_number=%d", uuid, r.Number) + + // Build the rule from the API response to preserve actual values + // This ensures the diff shows the real cidr_list and protocol values + // instead of UUIDs, making it clear what's being deleted + rule := buildRuleFromAPI(r) + + // Add the dummy rule to the rules set + rules.Add(rule) + } + } + + if rules.Len() > 0 { + d.Set("rule", rules) + } else if !managed { + d.SetId("") + } + + return nil +} + +func resourceCloudStackNetworkACLRulesetUpdate(d *schema.ResourceData, meta interface{}) error { + // Check if the rule set as a whole has changed + if d.HasChange("rule") { + o, n := d.GetChange("rule") + oldSet := o.(*schema.Set) + newSet := n.(*schema.Set) + + // Build maps of rules by rule_number for efficient lookup + oldRulesByNumber := make(map[int]map[string]interface{}) + newRulesByNumber := make(map[int]map[string]interface{}) + + for _, rule := range oldSet.List() { + ruleMap := rule.(map[string]interface{}) + ruleNum := ruleMap["rule_number"].(int) + oldRulesByNumber[ruleNum] = ruleMap + } + + for _, rule := range newSet.List() { + ruleMap := rule.(map[string]interface{}) + ruleNum := ruleMap["rule_number"].(int) + newRulesByNumber[ruleNum] = ruleMap + } + + // Categorize rules into: update, delete, create + var rulesToUpdate []*ruleUpdatePair + var rulesToDelete []map[string]interface{} + var rulesToCreate []map[string]interface{} + + // Find rules to update or delete + for ruleNum, oldRule := range oldRulesByNumber { + if newRule, exists := newRulesByNumber[ruleNum]; exists { + // Rule exists in both old and new - check if it needs updating + if aclRuleNeedsUpdate(oldRule, newRule) { + rulesToUpdate = append(rulesToUpdate, &ruleUpdatePair{ + oldRule: oldRule, + newRule: newRule, + }) + } + // If no update needed, the rule stays as-is (UUID preserved) + } else { + // Rule only exists in old state - delete it + rulesToDelete = append(rulesToDelete, oldRule) + } + } + + // Find rules to create + for ruleNum, newRule := range newRulesByNumber { + if _, exists := oldRulesByNumber[ruleNum]; !exists { + // Rule only exists in new state - create it + rulesToCreate = append(rulesToCreate, newRule) + } + } + + // We need to start with a rule set containing all the rules we + // already have and want to keep. Any rules that are not deleted + // correctly and any newly created rules, will be added to this + // set to make sure we end up in a consistent state + rules := resourceCloudStackNetworkACLRuleset().Schema["rule"].ZeroValue().(*schema.Set) + + // Add all rules that will remain (either unchanged or updated) + for ruleNum := range newRulesByNumber { + if oldRule, exists := oldRulesByNumber[ruleNum]; exists { + // This rule will either be updated or kept as-is + // Start with the old rule (which has the UUID) + rules.Add(oldRule) + } + } + + // First, delete rules that are no longer needed + if len(rulesToDelete) > 0 { + deleteSet := &schema.Set{F: rules.F} + for _, rule := range rulesToDelete { + deleteSet.Add(rule) + } + err := deleteACLRules(d, meta, rules, deleteSet) + + // We need to update this first to preserve the correct state + d.Set("rule", rules) + + if err != nil { + return err + } + } + + // Second, update rules that have changed + if len(rulesToUpdate) > 0 { + err := updateACLRules(d, meta, rules, rulesToUpdate) + + // We need to update this first to preserve the correct state + d.Set("rule", rules) + + if err != nil { + return err + } + } + + // Finally, create new rules + if len(rulesToCreate) > 0 { + createSet := &schema.Set{F: rules.F} + for _, rule := range rulesToCreate { + createSet.Add(rule) + } + err := createACLRules(d, meta, rules, createSet) + + // We need to update this first to preserve the correct state + d.Set("rule", rules) + + if err != nil { + return err + } + } + } + + return resourceCloudStackNetworkACLRulesetRead(d, meta) +} + +type ruleUpdatePair struct { + oldRule map[string]interface{} + newRule map[string]interface{} +} + +func resourceCloudStackNetworkACLRulesetDelete(d *schema.ResourceData, meta interface{}) error { + // If managed=false, don't delete any rules - just remove from state + managed := d.Get("managed").(bool) + if !managed { + log.Printf("[DEBUG] Managed=false, not deleting ACL rules for %s", d.Id()) + return nil + } + + // Create an empty rule set to hold all rules that where + // not deleted correctly + rules := resourceCloudStackNetworkACLRuleset().Schema["rule"].ZeroValue().(*schema.Set) + + // Delete all rules + if ors := d.Get("rule").(*schema.Set); ors.Len() > 0 { + err := deleteACLRules(d, meta, rules, ors) + + // We need to update this first to preserve the correct state + d.Set("rule", rules) + + if err != nil { + return err + } + } + + return nil +} + +func deleteACLRules(d *schema.ResourceData, meta interface{}, rules *schema.Set, ors *schema.Set) error { + var errs *multierror.Error + var mu sync.Mutex + + var wg sync.WaitGroup + wg.Add(ors.Len()) + + sem := make(chan struct{}, 10) + for _, rule := range ors.List() { + // Put a sleep here to avoid DoS'ing the API + time.Sleep(500 * time.Millisecond) + + go func(rule map[string]interface{}) { + defer wg.Done() + sem <- struct{}{} + + // Delete a single rule + err := deleteACLRule(d, meta, rule) + + // If we have a UUID, we need to save the rule + if uuid, ok := rule["uuid"].(string); ok && uuid != "" { + mu.Lock() + rules.Add(rule) + mu.Unlock() + } + + if err != nil { + mu.Lock() + errs = multierror.Append(errs, err) + mu.Unlock() + } + + <-sem + }(rule.(map[string]interface{})) + } + + wg.Wait() + + return errs.ErrorOrNil() +} + +func deleteACLRule(d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error { + cs := meta.(*cloudstack.CloudStackClient) + + // Create the parameter struct + p := cs.NetworkACL.NewDeleteNetworkACLParams(rule["uuid"].(string)) + + // Delete the rule + 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", rule["uuid"].(string))) { + return err + } + } + + // Empty the UUID of this rule + rule["uuid"] = "" + + return nil +} + +func updateACLRules(d *schema.ResourceData, meta interface{}, rules *schema.Set, updatePairs []*ruleUpdatePair) error { + var errs *multierror.Error + var mu sync.Mutex + + var wg sync.WaitGroup + wg.Add(len(updatePairs)) + + sem := make(chan struct{}, 10) + for _, pair := range updatePairs { + // Put a sleep here to avoid DoS'ing the API + time.Sleep(500 * time.Millisecond) + + go func(pair *ruleUpdatePair) { + defer wg.Done() + sem <- struct{}{} + + // Update a single rule + err := updateACLRule(d, meta, pair.oldRule, pair.newRule) + + // If we have a UUID, we need to save the updated rule + if uuid, ok := pair.oldRule["uuid"].(string); ok && uuid != "" { + mu.Lock() + // Remove the old rule from the set + rules.Remove(pair.oldRule) + // Update the old rule with new values (preserving UUID) + updateRuleValues(pair.oldRule, pair.newRule) + // Add the updated rule back to the set + rules.Add(pair.oldRule) + mu.Unlock() + } + + if err != nil { + mu.Lock() + errs = multierror.Append(errs, err) + mu.Unlock() + } + + <-sem + }(pair) + } + + wg.Wait() + + return errs.ErrorOrNil() +} + +func updateACLRule(d *schema.ResourceData, meta interface{}, oldRule, newRule map[string]interface{}) error { + cs := meta.(*cloudstack.CloudStackClient) + + uuid := oldRule["uuid"].(string) + if uuid == "" { + return fmt.Errorf("cannot update rule without UUID") + } + + log.Printf("[DEBUG] Updating ACL rule with UUID: %s", uuid) + + // If the protocol changed, we need to delete and recreate the rule + // because the CloudStack API doesn't properly clear protocol-specific fields + // (e.g., ports when changing from TCP to ICMP) + if oldRule["protocol"].(string) != newRule["protocol"].(string) { + log.Printf("[DEBUG] Protocol changed, using delete+create approach for rule %s", uuid) + + // Delete the old rule + p := cs.NetworkACL.NewDeleteNetworkACLParams(uuid) + if _, err := cs.NetworkACL.DeleteNetworkACL(p); err != nil { + // Ignore "does not exist" errors + if !strings.Contains(err.Error(), fmt.Sprintf( + "Invalid parameter id value=%s due to incorrect long value format, "+ + "or entity does not exist", uuid)) { + return fmt.Errorf("failed to delete rule during protocol change: %w", err) + } + } + + // Create the new rule with the new protocol + if err := createACLRule(d, meta, newRule); err != nil { + return fmt.Errorf("failed to create rule during protocol change: %w", err) + } + + // The new UUID is now in newRule["uuid"], copy it to oldRule so it gets saved + oldRule["uuid"] = newRule["uuid"] + + return nil + } + + // Create the parameter struct + p := cs.NetworkACL.NewUpdateNetworkACLItemParams(uuid) + + // Set the action + p.SetAction(newRule["action"].(string)) + + // Set the CIDR list + var cidrList []string + for _, cidr := range newRule["cidr_list"].(*schema.Set).List() { + cidrList = append(cidrList, cidr.(string)) + } + p.SetCidrlist(cidrList) + + // Set the description + if desc, ok := newRule["description"].(string); ok && desc != "" { + p.SetReason(desc) + } + + // Set the protocol + p.SetProtocol(newRule["protocol"].(string)) + + // Set the traffic type + p.SetTraffictype(newRule["traffic_type"].(string)) + + // Set the rule number + p.SetNumber(newRule["rule_number"].(int)) + + protocol := newRule["protocol"].(string) + switch protocol { + case "icmp": + p.SetIcmptype(newRule["icmp_type"].(int)) + p.SetIcmpcode(newRule["icmp_code"].(int)) + // Don't set ports for ICMP - CloudStack API will handle this + case "all": + // Don't set ports or ICMP fields for "all" protocol + case "tcp", "udp": + if portStr, hasPort := newRule["port"].(string); hasPort && portStr != "" { + m := splitPorts.FindStringSubmatch(portStr) + if m != nil { + startPort, err := strconv.Atoi(m[1]) + if err == nil { + endPort := startPort + if m[2] != "" { + if ep, err := strconv.Atoi(m[2]); err == nil { + endPort = ep + } + } + p.SetStartport(startPort) + p.SetEndport(endPort) + } + } + } + // If port is empty, don't set start/end port - CloudStack will handle "all ports" + } + + // Execute the update + _, err := cs.NetworkACL.UpdateNetworkACLItem(p) + if err != nil { + log.Printf("[ERROR] Failed to update ACL rule %s: %v", uuid, err) + return err + } + + log.Printf("[DEBUG] Successfully updated ACL rule %s", uuid) + return nil +} + +func verifyACLRuleParams(d *schema.ResourceData, rule map[string]interface{}) error { + ruleNumber := rule["rule_number"].(int) + if ruleNumber < 1 || ruleNumber > 65535 { + return fmt.Errorf("rule_number must be between 1 and 65535, got: %d", ruleNumber) + } + + action := rule["action"].(string) + if action != "allow" && action != "deny" { + return fmt.Errorf("action must be 'allow' or 'deny', got: %s", action) + } + + protocol := rule["protocol"].(string) + switch protocol { + case "icmp": + if _, ok := rule["icmp_type"]; !ok { + return fmt.Errorf("icmp_type is required when protocol is 'icmp'") + } + if _, ok := rule["icmp_code"]; !ok { + return fmt.Errorf("icmp_code is required when protocol is 'icmp'") + } + case "all": + // No additional validation needed + case "tcp", "udp": + // Port is optional + if portStr, ok := rule["port"].(string); ok && portStr != "" { + m := splitPorts.FindStringSubmatch(portStr) + if m == nil { + return fmt.Errorf("%q is not a valid port value. Valid options are '80' or '80-90'", portStr) + } + } + default: + // Reject numeric protocols - CloudStack API expects protocol names + if _, err := strconv.Atoi(protocol); err == nil { + return fmt.Errorf("numeric protocols are not supported, use protocol names instead (tcp, udp, icmp, all). Got: %s", protocol) + } + // If not a number, it's an unsupported protocol name + return fmt.Errorf("%q is not a valid protocol. Valid options are 'tcp', 'udp', 'icmp', 'all'", protocol) + } + + return nil +} + +func aclRuleNeedsUpdate(oldRule, newRule map[string]interface{}) bool { + // Check basic attributes + if oldRule["action"].(string) != newRule["action"].(string) { + return true + } + + if oldRule["protocol"].(string) != newRule["protocol"].(string) { + return true + } + + if oldRule["traffic_type"].(string) != newRule["traffic_type"].(string) { + return true + } + + // Check description + oldDesc, _ := oldRule["description"].(string) + newDesc, _ := newRule["description"].(string) + if oldDesc != newDesc { + return true + } + + // Check CIDR list + oldCidrs := oldRule["cidr_list"].(*schema.Set) + newCidrs := newRule["cidr_list"].(*schema.Set) + if !oldCidrs.Equal(newCidrs) { + return true + } + + // Check protocol-specific attributes + protocol := newRule["protocol"].(string) + switch protocol { + case "icmp": + if oldRule["icmp_type"].(int) != newRule["icmp_type"].(int) { + return true + } + if oldRule["icmp_code"].(int) != newRule["icmp_code"].(int) { + return true + } + case "tcp", "udp": + oldPort, _ := oldRule["port"].(string) + newPort, _ := newRule["port"].(string) + if oldPort != newPort { + return true + } + } + + return false +} + +func updateRuleValues(oldRule, newRule map[string]interface{}) { + // Update all values from newRule to oldRule, preserving the UUID + oldRule["action"] = newRule["action"] + oldRule["cidr_list"] = newRule["cidr_list"] + oldRule["protocol"] = newRule["protocol"] + oldRule["icmp_type"] = newRule["icmp_type"] + oldRule["icmp_code"] = newRule["icmp_code"] + oldRule["port"] = newRule["port"] + oldRule["traffic_type"] = newRule["traffic_type"] + oldRule["description"] = newRule["description"] + oldRule["rule_number"] = newRule["rule_number"] + // Note: UUID is NOT updated - it's preserved from oldRule +} + +func resourceCloudStackNetworkACLRulesetImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { + // Parse the import ID to extract optional project name + // Format: acl_id or project/acl_id + s := strings.SplitN(d.Id(), "/", 2) + if len(s) == 2 { + d.Set("project", s[0]) + d.SetId(s[1]) + } + + // Set the acl_id field to match the resource ID + d.Set("acl_id", d.Id()) + + // Don't set managed here - let it use the default value from the schema (false) + // The Read function will be called after this and will populate the rules + + log.Printf("[DEBUG] Imported ACL ruleset with ID: %s", d.Id()) + + return []*schema.ResourceData{d}, nil +} diff --git a/cloudstack/resource_cloudstack_network_acl_ruleset_test.go b/cloudstack/resource_cloudstack_network_acl_ruleset_test.go new file mode 100644 index 00000000..14321bb7 --- /dev/null +++ b/cloudstack/resource_cloudstack_network_acl_ruleset_test.go @@ -0,0 +1,2083 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package cloudstack + +import ( + "fmt" + "regexp" + "strings" + "testing" + + "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 testAccCheckCloudStackNetworkACLRulesetExists(n string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No network ACL ruleset ID is set") + } + + cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) + foundRules := 0 + + for k, id := range rs.Primary.Attributes { + // Check for ruleset format: rule.*.uuid + if strings.Contains(k, "rule.") && strings.HasSuffix(k, ".uuid") && id != "" { + _, count, err := cs.NetworkACL.GetNetworkACLByID(id) + + if err != nil { + // Check if this is a "not found" error + if strings.Contains(err.Error(), "No match found") { + continue + } + return err + } + + if count == 0 { + continue + } + foundRules++ + } + } + + if foundRules == 0 { + return fmt.Errorf("No network ACL rules found in state for %s", n) + } + + return nil + } +} + +func testAccCheckCloudStackNetworkACLRulesetDestroy(s *terraform.State) error { + cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) + + for _, rs := range s.RootModule().Resources { + if rs.Type != "cloudstack_network_acl_ruleset" { + continue + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No network ACL ruleset ID is set") + } + + for k, id := range rs.Primary.Attributes { + // Check for ruleset format: rule.*.uuid + if strings.Contains(k, "rule.") && 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) + } + } + } + } + + return nil +} + +// testAccCreateOutOfBandACLRule creates an ACL rule outside of Terraform +// to simulate an out-of-band change for testing managed=true behavior +func testAccCreateOutOfBandACLRule(t *testing.T, aclID string) { + client := testAccProvider.Meta().(*cloudstack.CloudStackClient) + + p := client.NetworkACL.NewCreateNetworkACLParams("tcp") + p.SetAclid(aclID) + p.SetCidrlist([]string{"10.0.0.0/8"}) + p.SetStartport(443) + p.SetEndport(443) + p.SetTraffictype("ingress") + p.SetAction("allow") + p.SetNumber(30) + + _, err := client.NetworkACL.CreateNetworkACL(p) + if err != nil { + t.Fatalf("Failed to create out-of-band ACL rule: %v", err) + } +} + +// testAccCheckOutOfBandACLRuleExists verifies that the out-of-band rule still exists +func testAccCheckOutOfBandACLRuleExists(aclID string) error { + client := testAccProvider.Meta().(*cloudstack.CloudStackClient) + + p := client.NetworkACL.NewListNetworkACLsParams() + p.SetAclid(aclID) + + resp, err := client.NetworkACL.ListNetworkACLs(p) + if err != nil { + return fmt.Errorf("Failed to list ACL rules: %v", err) + } + + // Check that the out-of-band rule (rule number 30) still exists + for _, rule := range resp.NetworkACLs { + if rule.Number == 30 { + return nil // Found it - success! + } + } + + return fmt.Errorf("Out-of-band rule (number 30) was deleted even though managed=false") +} + +// testAccCheckOutOfBandACLRuleDeleted verifies that the out-of-band rule was deleted +func testAccCheckOutOfBandACLRuleDeleted(aclID string) error { + client := testAccProvider.Meta().(*cloudstack.CloudStackClient) + + p := client.NetworkACL.NewListNetworkACLsParams() + p.SetAclid(aclID) + + resp, err := client.NetworkACL.ListNetworkACLs(p) + if err != nil { + return fmt.Errorf("Failed to list ACL rules: %v", err) + } + + // Check that the out-of-band rule (rule number 30) was deleted + for _, rule := range resp.NetworkACLs { + if rule.Number == 30 { + return fmt.Errorf("Out-of-band rule (number 30) still exists even though managed=true") + } + } + + return nil // Not found - success! +} + +func TestAccCloudStackNetworkACLRuleset_basic(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.bar"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.bar", "rule.#", "8"), + // Check for the expected rules using TypeSet elem matching + // Test minimum rule number (1) + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "1", + "action": "allow", + "protocol": "all", + "traffic_type": "ingress", + "description": "Allow all traffic - min rule number", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", 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_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "40", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + // Test UDP protocol + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "50", + "action": "allow", + "protocol": "udp", + "port": "53", + "traffic_type": "ingress", + "description": "Allow DNS", + }), + // Test optional description field (rule without description) + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "60", + "action": "allow", + "protocol": "udp", + "port": "123", + "traffic_type": "ingress", + }), + // Test maximum port number + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "100", + "action": "allow", + "protocol": "tcp", + "port": "65535", + "traffic_type": "ingress", + "description": "Max port number", + }), + // Test maximum rule number (65535) + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "65535", + "action": "deny", + "protocol": "all", + "traffic_type": "egress", + "description": "Max rule number", + }), + ), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_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_ruleset" "bar" { + acl_id = cloudstack_network_acl.bar.id + + rule { + rule_number = 1 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "all" + traffic_type = "ingress" + description = "Allow all traffic - min rule number" + } + + 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 { + rule_number = 30 + action = "allow" + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + rule_number = 40 + action = "allow" + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } + + rule { + rule_number = 50 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "udp" + port = "53" + traffic_type = "ingress" + description = "Allow DNS" + } + + rule { + rule_number = 60 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "udp" + port = "123" + traffic_type = "ingress" + # No description - testing optional field + } + + rule { + rule_number = 100 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "65535" + traffic_type = "ingress" + description = "Max port number" + } + + rule { + rule_number = 65535 + action = "deny" + cidr_list = ["0.0.0.0/0"] + protocol = "all" + traffic_type = "egress" + description = "Max rule number" + } +}` + +func TestAccCloudStackNetworkACLRuleset_update(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.bar"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.bar", "rule.#", "8"), + ), + }, + { + Config: testAccCloudStackNetworkACLRuleset_update, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.bar"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.bar", "rule.#", "6"), + // Check for the expected rules using TypeSet elem matching + // Rule 10: Changed action from allow to deny AND changed CIDR list from single to multiple + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "10", + "action": "deny", + "protocol": "all", + "traffic_type": "ingress", + "description": "Allow all traffic", + }), + // Rule 20: Changed action and added CIDR + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "20", + "action": "deny", + "protocol": "icmp", + "icmp_type": "-1", + "icmp_code": "-1", + "traffic_type": "ingress", + "description": "Allow ICMP traffic", + }), + // Rule 30: Unchanged + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + // Rule 40: Unchanged + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "40", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + // Rule 50: New egress rule + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "50", + "action": "deny", + "protocol": "tcp", + "port": "80", + "traffic_type": "egress", + "description": "Deny specific TCP ports", + }), + // Rule 60: New egress rule with port range + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.bar", "rule.*", map[string]string{ + "rule_number": "60", + "action": "deny", + "protocol": "tcp", + "port": "1000-2000", + "traffic_type": "egress", + "description": "Deny specific TCP ports", + }), + ), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_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_ruleset" "bar" { + acl_id = cloudstack_network_acl.bar.id + + rule { + rule_number = 10 + action = "deny" + cidr_list = ["172.18.100.0/24", "192.168.1.0/24", "10.0.0.0/8"] # Changed from single to multiple CIDRs + protocol = "all" + traffic_type = "ingress" + description = "Allow all traffic" + } + + rule { + 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 = "Allow ICMP traffic" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + rule_number = 40 + action = "allow" + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } + + rule { + rule_number = 50 + action = "deny" + cidr_list = ["10.0.0.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "egress" + description = "Deny specific TCP ports" + } + + rule { + 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" + } +}` + +func TestAccCloudStackNetworkACLRuleset_managed(t *testing.T) { + var aclID string + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_managed, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.managed"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.managed", "managed", "true"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.managed", "rule.#", "2"), + // Capture the ACL ID for later use + func(s *terraform.State) error { + rs, ok := s.RootModule().Resources["cloudstack_network_acl_ruleset.managed"] + if !ok { + return fmt.Errorf("Not found: cloudstack_network_acl_ruleset.managed") + } + aclID = rs.Primary.ID + return nil + }, + ), + }, + { + // Add an out-of-band rule via the API + PreConfig: func() { + // Create a rule outside of Terraform + testAccCreateOutOfBandACLRule(t, aclID) + }, + Config: testAccCloudStackNetworkACLRuleset_managed, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.managed"), + // With managed=true, the out-of-band rule should be DELETED + // Verify only the 2 configured rules exist + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.managed", "rule.#", "2"), + // Verify the out-of-band rule was deleted + func(s *terraform.State) error { + return testAccCheckOutOfBandACLRuleDeleted(aclID) + }, + ), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_managed = ` +resource "cloudstack_vpc" "managed" { + name = "terraform-vpc-managed" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "managed" { + name = "terraform-acl-managed" + description = "terraform-acl-managed-text" + vpc_id = cloudstack_vpc.managed.id +} + +resource "cloudstack_network_acl_ruleset" "managed" { + acl_id = cloudstack_network_acl.managed.id + managed = true + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH" + } + + rule { + rule_number = 20 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } +}` + +func TestAccCloudStackNetworkACLRuleset_insert(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_insert_initial, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.baz"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.baz", "rule.#", "3"), + // Initial rules: 10, 30, 50 + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.baz", "rule.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "tcp", + "port": "22", + "traffic_type": "ingress", + "description": "Allow SSH", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.baz", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.baz", "rule.*", map[string]string{ + "rule_number": "50", + "action": "allow", + "protocol": "tcp", + "port": "3306", + "traffic_type": "ingress", + "description": "Allow MySQL", + }), + ), + }, + + { + Config: testAccCloudStackNetworkACLRuleset_insert_middle, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.baz"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.baz", "rule.#", "4"), + // After inserting rule 20 in the middle, all original rules should still exist + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.baz", "rule.*", 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_ruleset.baz", "rule.*", map[string]string{ + "rule_number": "20", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.baz", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.baz", "rule.*", map[string]string{ + "rule_number": "50", + "action": "allow", + "protocol": "tcp", + "port": "3306", + "traffic_type": "ingress", + "description": "Allow MySQL", + }), + ), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_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_ruleset" "baz" { + acl_id = cloudstack_network_acl.baz.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } + + rule { + rule_number = 50 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "3306" + traffic_type = "ingress" + description = "Allow MySQL" + } +}` + +const testAccCloudStackNetworkACLRuleset_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_ruleset" "baz" { + acl_id = cloudstack_network_acl.baz.id + + rule { + 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 + rule { + rule_number = 20 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } + + rule { + rule_number = 50 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "3306" + traffic_type = "ingress" + description = "Allow MySQL" + } +}` + +func TestAccCloudStackNetworkACLRuleset_not_managed(t *testing.T) { + var aclID string + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_not_managed, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.not_managed"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.not_managed", "rule.#", "2"), + // Capture the ACL ID for later use + func(s *terraform.State) error { + rs, ok := s.RootModule().Resources["cloudstack_network_acl_ruleset.not_managed"] + if !ok { + return fmt.Errorf("Not found: cloudstack_network_acl_ruleset.not_managed") + } + aclID = rs.Primary.ID + return nil + }, + ), + }, + { + // Add an out-of-band rule via the API + PreConfig: func() { + // Create a rule outside of Terraform + testAccCreateOutOfBandACLRule(t, aclID) + }, + Config: testAccCloudStackNetworkACLRuleset_not_managed, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.not_managed"), + // With managed=false (default), the out-of-band rule should be PRESERVED + // Verify the out-of-band rule still exists + func(s *terraform.State) error { + return testAccCheckOutOfBandACLRuleExists(aclID) + }, + ), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_not_managed = ` +resource "cloudstack_vpc" "not_managed" { + name = "terraform-vpc-not-managed" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "not_managed" { + name = "terraform-acl-not-managed" + description = "terraform-acl-not-managed-text" + vpc_id = cloudstack_vpc.not_managed.id +} + +resource "cloudstack_network_acl_ruleset" "not_managed" { + acl_id = cloudstack_network_acl.not_managed.id + # managed = false is the default, so we don't set it explicitly + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH" + } + + rule { + rule_number = 20 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } +}` + +func TestAccCloudStackNetworkACLRuleset_insert_plan_check(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_plan_check_initial, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.plan_check"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.plan_check", "rule.#", "3"), + // Initial rules: 10, 30, 50 + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.plan_check", "rule.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "tcp", + "port": "22", + "traffic_type": "ingress", + "description": "Allow SSH", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.plan_check", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.plan_check", "rule.*", map[string]string{ + "rule_number": "50", + "action": "allow", + "protocol": "tcp", + "port": "3306", + "traffic_type": "ingress", + "description": "Allow MySQL", + }), + ), + }, + + { + Config: testAccCloudStackNetworkACLRuleset_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_ruleset.plan_check", plancheck.ResourceActionUpdate), + // Verify that rule.# is changing from 3 to 4 (exactly one block added) + plancheck.ExpectKnownValue( + "cloudstack_network_acl_ruleset.plan_check", + tfjsonpath.New("rule"), + knownvalue.SetSizeExact(4), + ), + }, + }, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.plan_check"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.plan_check", "rule.#", "4"), + // After inserting rule 20 in the middle, all original rules should still exist + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.plan_check", "rule.*", 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_ruleset.plan_check", "rule.*", map[string]string{ + "rule_number": "20", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.plan_check", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.plan_check", "rule.*", map[string]string{ + "rule_number": "50", + "action": "allow", + "protocol": "tcp", + "port": "3306", + "traffic_type": "ingress", + "description": "Allow MySQL", + }), + ), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_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" "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_ruleset" "plan_check" { + acl_id = cloudstack_network_acl.plan_check.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } + + rule { + rule_number = 50 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "3306" + traffic_type = "ingress" + description = "Allow MySQL" + } +} +` + +const testAccCloudStackNetworkACLRuleset_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_ruleset" "plan_check" { + acl_id = cloudstack_network_acl.plan_check.id + + rule { + 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 + rule { + rule_number = 20 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } + + rule { + rule_number = 50 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "3306" + traffic_type = "ingress" + description = "Allow MySQL" + } +} +` + +func TestAccCloudStackNetworkACLRuleset_field_changes(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_field_changes_initial, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.field_changes"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.field_changes", "rule.#", "4"), + // Initial rules with specific values + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.field_changes", "rule.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "tcp", + "port": "22", + "traffic_type": "ingress", + "description": "Allow SSH", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.field_changes", "rule.*", map[string]string{ + "rule_number": "20", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.field_changes", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "icmp", + "icmp_type": "8", + "icmp_code": "0", + "traffic_type": "ingress", + "description": "Allow ping", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.field_changes", "rule.*", map[string]string{ + "rule_number": "40", + "action": "allow", + "protocol": "all", + "traffic_type": "egress", + "description": "Allow all egress", + }), + ), + }, + { + Config: testAccCloudStackNetworkACLRuleset_field_changes_updated, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.field_changes"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.field_changes", "rule.#", "4"), + // Same rule numbers but with changed fields + // Rule 10: Changed port and CIDR list + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.field_changes", "rule.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "tcp", + "port": "2222", // Changed port + "traffic_type": "ingress", + "description": "Allow SSH", + }), + // Rule 20: Changed action + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.field_changes", "rule.*", map[string]string{ + "rule_number": "20", + "action": "deny", // Changed action + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + // Rule 30: Changed ICMP type + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.field_changes", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "icmp", + "icmp_type": "0", // Changed ICMP type + "icmp_code": "0", + "traffic_type": "ingress", + "description": "Allow ping", + }), + // Rule 40: Changed action + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.field_changes", "rule.*", map[string]string{ + "rule_number": "40", + "action": "deny", // Changed action + "protocol": "all", + "traffic_type": "egress", + "description": "Allow all egress", + }), + ), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_field_changes_initial = ` +resource "cloudstack_vpc" "field_changes" { + name = "terraform-vpc-field-changes" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "field_changes" { + name = "terraform-acl-field-changes" + description = "terraform-acl-field-changes-text" + vpc_id = cloudstack_vpc.field_changes.id +} + +resource "cloudstack_network_acl_ruleset" "field_changes" { + acl_id = cloudstack_network_acl.field_changes.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH" + } + + rule { + rule_number = 20 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "icmp" + icmp_type = 8 + icmp_code = 0 + traffic_type = "ingress" + description = "Allow ping" + } + + rule { + rule_number = 40 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "all" + traffic_type = "egress" + description = "Allow all egress" + } +} +` + +const testAccCloudStackNetworkACLRuleset_field_changes_updated = ` +resource "cloudstack_vpc" "field_changes" { + name = "terraform-vpc-field-changes" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "field_changes" { + name = "terraform-acl-field-changes" + description = "terraform-acl-field-changes-text" + vpc_id = cloudstack_vpc.field_changes.id +} + +resource "cloudstack_network_acl_ruleset" "field_changes" { + acl_id = cloudstack_network_acl.field_changes.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["192.168.1.0/24", "10.0.0.0/8"] # Changed CIDR list + protocol = "tcp" + port = "2222" # Changed from 22 + traffic_type = "ingress" + description = "Allow SSH" + } + + rule { + rule_number = 20 + action = "deny" # Changed from allow + cidr_list = ["172.18.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["172.18.100.0/24"] + protocol = "icmp" + icmp_type = 0 # Changed from 8 + icmp_code = 0 + traffic_type = "ingress" + description = "Allow ping" + } + + rule { + rule_number = 40 + action = "deny" # Changed from allow + cidr_list = ["172.18.100.0/24"] + protocol = "all" + traffic_type = "egress" + description = "Allow all egress" + } +} +` + +func TestAccCloudStackNetworkACLRuleset_protocol_transitions(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_protocol_tcp, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.protocol_test"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.protocol_test", "rule.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.protocol_test", "rule.*", map[string]string{ + "rule_number": "10", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + }), + ), + }, + { + Config: testAccCloudStackNetworkACLRuleset_protocol_icmp, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.protocol_test"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.protocol_test", "rule.#", "1"), + // Verify protocol changed to ICMP + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.protocol_test", "rule.*", map[string]string{ + "rule_number": "10", + "protocol": "icmp", + "icmp_type": "8", + "icmp_code": "0", + "traffic_type": "ingress", + }), + ), + }, + { + Config: testAccCloudStackNetworkACLRuleset_protocol_all, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.protocol_test"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.protocol_test", "rule.#", "1"), + // Verify protocol changed to all + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.protocol_test", "rule.*", map[string]string{ + "rule_number": "10", + "protocol": "all", + "traffic_type": "ingress", + }), + ), + }, + { + Config: testAccCloudStackNetworkACLRuleset_protocol_udp, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.protocol_test"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.protocol_test", "rule.#", "1"), + // Verify protocol changed back to UDP with port + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.protocol_test", "rule.*", map[string]string{ + "rule_number": "10", + "protocol": "udp", + "port": "53", + "traffic_type": "ingress", + }), + ), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_protocol_tcp = ` +resource "cloudstack_vpc" "protocol_test" { + name = "terraform-vpc-protocol-test" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "protocol_test" { + name = "terraform-acl-protocol-test" + description = "terraform-acl-protocol-test-text" + vpc_id = cloudstack_vpc.protocol_test.id +} + +resource "cloudstack_network_acl_ruleset" "protocol_test" { + acl_id = cloudstack_network_acl.protocol_test.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "TCP with port" + } +} +` + +const testAccCloudStackNetworkACLRuleset_protocol_icmp = ` +resource "cloudstack_vpc" "protocol_test" { + name = "terraform-vpc-protocol-test" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "protocol_test" { + name = "terraform-acl-protocol-test" + description = "terraform-acl-protocol-test-text" + vpc_id = cloudstack_vpc.protocol_test.id +} + +resource "cloudstack_network_acl_ruleset" "protocol_test" { + acl_id = cloudstack_network_acl.protocol_test.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "icmp" + icmp_type = 8 + icmp_code = 0 + traffic_type = "ingress" + description = "ICMP ping" + } +} +` + +const testAccCloudStackNetworkACLRuleset_protocol_all = ` +resource "cloudstack_vpc" "protocol_test" { + name = "terraform-vpc-protocol-test" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "protocol_test" { + name = "terraform-acl-protocol-test" + description = "terraform-acl-protocol-test-text" + vpc_id = cloudstack_vpc.protocol_test.id +} + +resource "cloudstack_network_acl_ruleset" "protocol_test" { + acl_id = cloudstack_network_acl.protocol_test.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "all" + traffic_type = "ingress" + description = "All protocols" + } +} +` + +const testAccCloudStackNetworkACLRuleset_protocol_udp = ` +resource "cloudstack_vpc" "protocol_test" { + name = "terraform-vpc-protocol-test" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "protocol_test" { + name = "terraform-acl-protocol-test" + description = "terraform-acl-protocol-test-text" + vpc_id = cloudstack_vpc.protocol_test.id +} + +resource "cloudstack_network_acl_ruleset" "protocol_test" { + acl_id = cloudstack_network_acl.protocol_test.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "udp" + port = "53" + traffic_type = "ingress" + description = "UDP DNS" + } +} +` + +func TestAccCloudStackNetworkACLRuleset_no_spurious_diff(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_no_spurious_diff_initial, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.no_spurious_diff"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.no_spurious_diff", "rule.#", "3"), + ), + }, + { + Config: testAccCloudStackNetworkACLRuleset_no_spurious_diff_change_one_rule, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + // Verify that only an update occurs (not a replacement) + plancheck.ExpectResourceAction("cloudstack_network_acl_ruleset.no_spurious_diff", plancheck.ResourceActionUpdate), + // Verify that rule.# stays at 3 (no rules added or removed) + // This proves that rules 10 and 30 are not being deleted and recreated + plancheck.ExpectKnownValue( + "cloudstack_network_acl_ruleset.no_spurious_diff", + tfjsonpath.New("rule"), + knownvalue.SetSizeExact(3), + ), + }, + }, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.no_spurious_diff"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.no_spurious_diff", "rule.#", "3"), + // Verify rule 20 was updated + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.no_spurious_diff", "rule.*", map[string]string{ + "rule_number": "20", + "port": "8080", // Changed from 80 + }), + // Verify rules 10 and 30 still exist with their original values + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.no_spurious_diff", "rule.*", map[string]string{ + "rule_number": "10", + "port": "22", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.no_spurious_diff", "rule.*", map[string]string{ + "rule_number": "30", + "port": "443", + }), + ), + }, + }, + }) +} + +// Test that changing the action field on one rule doesn't cause spurious diffs on other rules +func TestAccCloudStackNetworkACLRuleset_no_spurious_diff_action_change(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_action_change_initial, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.action_change"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.action_change", "rule.#", "2"), + ), + }, + { + Config: testAccCloudStackNetworkACLRuleset_action_change_deny, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + // Verify that only an update occurs (not a replacement) + plancheck.ExpectResourceAction("cloudstack_network_acl_ruleset.action_change", plancheck.ResourceActionUpdate), + // Verify that rule.# stays at 2 (no rules added or removed) + // This proves that rule 42002 is not being deleted and recreated + plancheck.ExpectKnownValue( + "cloudstack_network_acl_ruleset.action_change", + tfjsonpath.New("rule"), + knownvalue.SetSizeExact(2), + ), + }, + }, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.action_change"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.action_change", "rule.#", "2"), + // Verify rule 42001 was updated to deny + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.action_change", "rule.*", map[string]string{ + "rule_number": "42001", + "action": "deny", // Changed from allow + "traffic_type": "egress", + }), + // Verify rule 42002 still exists with its original values + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.action_change", "rule.*", map[string]string{ + "rule_number": "42002", + "action": "allow", // Unchanged + "traffic_type": "ingress", + }), + ), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_no_spurious_diff_initial = ` +resource "cloudstack_vpc" "no_spurious_diff" { + name = "terraform-vpc-no-spurious-diff" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "no_spurious_diff" { + name = "terraform-acl-no-spurious-diff" + description = "terraform-acl-no-spurious-diff-text" + vpc_id = cloudstack_vpc.no_spurious_diff.id +} + +resource "cloudstack_network_acl_ruleset" "no_spurious_diff" { + acl_id = cloudstack_network_acl.no_spurious_diff.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "SSH" + } + + rule { + rule_number = 20 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "HTTP" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "HTTPS" + } +} +` + +const testAccCloudStackNetworkACLRuleset_action_change_initial = ` +resource "cloudstack_vpc" "action_change" { + name = "terraform-vpc-action-change" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "action_change" { + name = "terraform-acl-action-change" + description = "terraform-acl-action-change-text" + vpc_id = cloudstack_vpc.action_change.id +} + +resource "cloudstack_network_acl_ruleset" "action_change" { + acl_id = cloudstack_network_acl.action_change.id + + rule { + rule_number = 42001 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "all" + traffic_type = "egress" + description = "to any vpc: allow egress" + } + + rule { + rule_number = 42002 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "all" + traffic_type = "ingress" + description = "from anywhere: allow ingress" + } +} +` + +const testAccCloudStackNetworkACLRuleset_action_change_deny = ` +resource "cloudstack_vpc" "action_change" { + name = "terraform-vpc-action-change" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "action_change" { + name = "terraform-acl-action-change" + description = "terraform-acl-action-change-text" + vpc_id = cloudstack_vpc.action_change.id +} + +resource "cloudstack_network_acl_ruleset" "action_change" { + acl_id = cloudstack_network_acl.action_change.id + + rule { + rule_number = 42001 + action = "deny" + cidr_list = ["0.0.0.0/0"] + protocol = "all" + traffic_type = "egress" + description = "to any vpc: deny egress" + } + + rule { + rule_number = 42002 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "all" + traffic_type = "ingress" + description = "from anywhere: allow ingress" + } +} +` + +const testAccCloudStackNetworkACLRuleset_no_spurious_diff_change_one_rule = ` +resource "cloudstack_vpc" "no_spurious_diff" { + name = "terraform-vpc-no-spurious-diff" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "no_spurious_diff" { + name = "terraform-acl-no-spurious-diff" + description = "terraform-acl-no-spurious-diff-text" + vpc_id = cloudstack_vpc.no_spurious_diff.id +} + +resource "cloudstack_network_acl_ruleset" "no_spurious_diff" { + acl_id = cloudstack_network_acl.no_spurious_diff.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "SSH" + } + + rule { + rule_number = 20 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "8080" # Changed from 80 + traffic_type = "ingress" + description = "HTTP" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "HTTPS" + } +} +` + +func TestAccCloudStackNetworkACLRuleset_import(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_import_config, + }, + { + ResourceName: "cloudstack_network_acl_ruleset.import_test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"managed"}, + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_import_config = ` +resource "cloudstack_vpc" "import_test" { + name = "terraform-vpc-import-test" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "import_test" { + name = "terraform-acl-import-test" + description = "terraform-acl-import-test-text" + vpc_id = cloudstack_vpc.import_test.id +} + +resource "cloudstack_network_acl_ruleset" "import_test" { + acl_id = cloudstack_network_acl.import_test.id + managed = false # Don't delete rules on destroy, so they can be imported + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + rule_number = 20 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "icmp" + icmp_type = 8 + icmp_code = 0 + traffic_type = "ingress" + description = "Allow ping" + } +} +` + +func TestAccCloudStackNetworkACLRuleset_numeric_protocol_error(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + // Don't check destroy since the resource was never created + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_numeric_protocol, + ExpectError: regexp.MustCompile("numeric protocols are not supported"), + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_numeric_protocol = ` +resource "cloudstack_network_acl_ruleset" "numeric_test" { + acl_id = "test-acl-id" + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "6" # Numeric protocol (6 = TCP) + port = "80" + traffic_type = "ingress" + description = "This should fail" + } +} +` + +func TestAccCloudStackNetworkACLRuleset_remove(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackNetworkACLRulesetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackNetworkACLRuleset_remove_initial, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.remove_test"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.remove_test", "rule.#", "4"), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.remove_test", "rule.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "all", + "traffic_type": "ingress", + "description": "Allow all traffic", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.remove_test", "rule.*", 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_ruleset.remove_test", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.remove_test", "rule.*", map[string]string{ + "rule_number": "40", + "action": "allow", + "protocol": "tcp", + "port": "443", + "traffic_type": "ingress", + "description": "Allow HTTPS", + }), + ), + }, + + { + Config: testAccCloudStackNetworkACLRuleset_remove_after, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + // Verify that we're only removing rules, not adding ghost entries + plancheck.ExpectResourceAction("cloudstack_network_acl_ruleset.remove_test", plancheck.ResourceActionUpdate), + // The plan should show exactly 2 rules in the ruleset after removal + // No ghost entries with empty cidr_list should appear + }, + }, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.remove_test"), + resource.TestCheckResourceAttr( + "cloudstack_network_acl_ruleset.remove_test", "rule.#", "2"), + // Only rules 10 and 30 should remain + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.remove_test", "rule.*", map[string]string{ + "rule_number": "10", + "action": "allow", + "protocol": "all", + "traffic_type": "ingress", + "description": "Allow all traffic", + }), + resource.TestCheckTypeSetElemNestedAttrs( + "cloudstack_network_acl_ruleset.remove_test", "rule.*", map[string]string{ + "rule_number": "30", + "action": "allow", + "protocol": "tcp", + "port": "80", + "traffic_type": "ingress", + "description": "Allow HTTP", + }), + ), + }, + { + // Re-apply the same config to verify no permadiff + // This ensures that Computed: true doesn't cause unexpected diffs + Config: testAccCloudStackNetworkACLRuleset_remove_after, + PlanOnly: true, // Should show no changes + }, + }, + }) +} + +const testAccCloudStackNetworkACLRuleset_remove_initial = ` +resource "cloudstack_vpc" "remove_test" { + name = "terraform-vpc-remove-test" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "remove_test" { + name = "terraform-acl-remove-test" + description = "terraform-acl-remove-test-text" + vpc_id = cloudstack_vpc.remove_test.id +} + +resource "cloudstack_network_acl_ruleset" "remove_test" { + acl_id = cloudstack_network_acl.remove_test.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 { + rule_number = 30 + action = "allow" + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + rule_number = 40 + action = "allow" + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } +}` + +const testAccCloudStackNetworkACLRuleset_remove_after = ` +resource "cloudstack_vpc" "remove_test" { + name = "terraform-vpc-remove-test" + cidr = "10.0.0.0/8" + vpc_offering = "Default VPC offering" + zone = "Sandbox-simulator" +} + +resource "cloudstack_network_acl" "remove_test" { + name = "terraform-acl-remove-test" + description = "terraform-acl-remove-test-text" + vpc_id = cloudstack_vpc.remove_test.id +} + +resource "cloudstack_network_acl_ruleset" "remove_test" { + acl_id = cloudstack_network_acl.remove_test.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 = 30 + action = "allow" + cidr_list = ["172.16.100.0/24"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } +} +` diff --git a/website/docs/r/network_acl_rule.html.markdown b/website/docs/r/network_acl_rule.html.markdown index 55bc6c94..2eea9567 100644 --- a/website/docs/r/network_acl_rule.html.markdown +++ b/website/docs/r/network_acl_rule.html.markdown @@ -8,8 +8,19 @@ description: |- # cloudstack_network_acl_rule +!> **WARNING:** This resource is deprecated. Use [`cloudstack_network_acl_ruleset`](/docs/providers/cloudstack/r/network_acl_ruleset.html) instead for better performance and in-place updates. + Creates network ACL rules for a given network ACL. +## Migration to cloudstack_network_acl_ruleset + +The `cloudstack_network_acl_ruleset` resource provides several advantages: +- **In-place updates**: Rule modifications preserve UUIDs and avoid delete+create cycles +- **Better performance**: Optimized concurrent operations with proper thread safety +- **Simpler state management**: More reliable tracking of rule changes + +See the [`cloudstack_network_acl_ruleset`](/docs/providers/cloudstack/r/network_acl_ruleset.html) documentation for migration examples. + ## Example Usage ### Basic Example with Port @@ -127,6 +138,12 @@ resource "cloudstack_network_acl_rule" "web_server" { description = "Allow all outbound TCP" } } +``` + +~> **Note:** For better change management when managing multiple rules, consider using the +[`cloudstack_network_acl_ruleset`](/docs/providers/cloudstack/r/network_acl_ruleset.html) resource +instead. It provides cleaner Terraform plans when inserting or removing rules by identifying rules +by their `rule_number` rather than position in a list. ## Argument Reference @@ -150,7 +167,9 @@ The following arguments are supported: The `rule` block supports: -* `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) 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, + CloudStack will assign a rule number automatically. * `action` - (Optional) The action for the rule. Valid options are: `allow` and `deny` (defaults allow). @@ -166,14 +185,14 @@ 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 +* `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. * `traffic_type` - (Optional) The traffic type for the rule. Valid options are: diff --git a/website/docs/r/network_acl_ruleset.html.markdown b/website/docs/r/network_acl_ruleset.html.markdown new file mode 100644 index 00000000..b394a630 --- /dev/null +++ b/website/docs/r/network_acl_ruleset.html.markdown @@ -0,0 +1,221 @@ +--- +layout: "cloudstack" +page_title: "CloudStack: cloudstack_network_acl_ruleset" +sidebar_current: "docs-cloudstack-resource-network-acl-ruleset" +description: |- + Manages a complete set of network ACL rules for a given network ACL. +--- + +# cloudstack_network_acl_ruleset + +Manages a complete set of network ACL rules for a given network ACL. This resource is designed +for managing all rules in an ACL as a single unit, with efficient handling of rule insertions +and deletions. + +~> **Note:** This resource is recommended over `cloudstack_network_acl_rule` when you need to +manage multiple rules and frequently insert or remove rules. It provides better change management +by identifying rules by their `rule_number` rather than position in a list. + +## Example Usage + +### Basic Example + +```hcl +resource "cloudstack_network_acl_ruleset" "web_server" { + acl_id = cloudstack_network_acl.example.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "80" + traffic_type = "ingress" + description = "Allow HTTP" + } + + rule { + rule_number = 20 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "tcp" + port = "443" + traffic_type = "ingress" + description = "Allow HTTPS" + } + + rule { + rule_number = 30 + action = "allow" + cidr_list = ["192.168.100.0/24"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH from management" + } +} +``` + +### Example with ICMP + +```hcl +resource "cloudstack_network_acl_ruleset" "icmp_example" { + acl_id = cloudstack_network_acl.example.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "icmp" + icmp_type = 8 + icmp_code = 0 + traffic_type = "ingress" + description = "Allow ping" + } +} +``` + +### Example with Managed Mode + +When `managed = true`, the provider will delete any rules not defined in your configuration. +This is useful for ensuring complete control over the ACL. + +```hcl +resource "cloudstack_network_acl_ruleset" "managed_example" { + acl_id = cloudstack_network_acl.example.id + managed = true + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["10.0.0.0/8"] + protocol = "tcp" + port = "22" + traffic_type = "ingress" + description = "Allow SSH" + } +} +``` + +### Example with Port Range + +```hcl +resource "cloudstack_network_acl_ruleset" "port_range" { + acl_id = cloudstack_network_acl.example.id + + rule { + rule_number = 10 + action = "allow" + cidr_list = ["192.168.1.0/24"] + protocol = "tcp" + port = "8000-8010" + traffic_type = "ingress" + description = "Allow port range" + } +} +``` + +### Example with All Protocols + +```hcl +resource "cloudstack_network_acl_ruleset" "all_protocols" { + acl_id = cloudstack_network_acl.example.id + + rule { + rule_number = 100 + action = "allow" + cidr_list = ["0.0.0.0/0"] + protocol = "all" + traffic_type = "egress" + description = "Allow all outbound traffic" + } +} +``` + +## Argument Reference + +The following arguments are supported: + +* `acl_id` - (Required) The network ACL ID for which to create the rules. + Changing this forces a new resource to be created. + +* `managed` - (Optional) USE WITH CAUTION! If enabled all the ACL rules for + this network ACL will be managed by this resource. This means it will delete + all ACL rules that are not in your config! (defaults false) + +* `rule` - (Required) Can be specified multiple times. Each rule block supports + fields documented below. + +* `project` - (Optional) The name or ID of the project to deploy this + instance to. Changing this forces a new resource to be created. + +The `rule` block supports: + +* `rule_number` - (Required) The number of the ACL item used to order the ACL rules. + The ACL rule with the lowest number has the highest priority. Each rule_number + must be unique within the ruleset. + +* `action` - (Optional) The action for the rule. Valid options are: `allow` and + `deny` (defaults allow). + +* `cidr_list` - (Required) A CIDR list to allow access to the given ports. + +* `protocol` - (Required) The name of the protocol to allow. Valid options are: + `tcp`, `udp`, `icmp`, or `all`. + +* `icmp_type` - (Optional) The ICMP type to allow, or `-1` to allow `any`. This + can only be specified if the protocol is ICMP. (defaults 0) + +* `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 + the protocol is TCP or UDP. Valid formats are: + - Single port: `"80"` + - Port range: `"8000-8010"` + - If not specified for TCP/UDP, allows all ports for that protocol + +* `traffic_type` - (Optional) The traffic type for the rule. Valid options are: + `ingress` or `egress` (defaults ingress). + +* `description` - (Optional) A description indicating why the ACL rule is required. + +## Attributes Reference + +The following attributes are exported: + +* `id` - The ACL ID for which the rules are managed. + +## Import + +Network ACL Rulesets can be imported using the ACL ID. For example: + +```shell +terraform import cloudstack_network_acl_ruleset.default e8b5982a-1b50-4ea9-9920-6ea2290c7359 +``` + +When importing into a project you need to prefix the import ID with the project name: + +```shell +terraform import cloudstack_network_acl_ruleset.default my-project/e8b5982a-1b50-4ea9-9920-6ea2290c7359 +``` + +## Comparison with cloudstack_network_acl_rule + +The `cloudstack_network_acl_ruleset` resource is similar to `cloudstack_network_acl_rule` but +with some key differences: + +* **Rule identification**: Uses `rule_number` to identify rules (set-based), rather than position + in a list. This means inserting a rule in the middle only creates that one rule, without + triggering updates to other rules. + +* **Simpler implementation**: Does not support the deprecated `ports` field or auto-numbering + of rules. All rules must have an explicit `rule_number`. + +* **Better for dynamic rulesets**: If you frequently add or remove rules, this resource will + generate cleaner Terraform plans with fewer spurious changes. + +Use `cloudstack_network_acl_rule` if you need auto-numbering or backward compatibility with +the `ports` field. Use `cloudstack_network_acl_ruleset` for cleaner change management when +managing multiple rules. +