diff --git a/.github/actions/setup-cloudstack/action.yml b/.github/actions/setup-cloudstack/action.yml index 7d53ab80..ef303fbc 100644 --- a/.github/actions/setup-cloudstack/action.yml +++ b/.github/actions/setup-cloudstack/action.yml @@ -43,18 +43,30 @@ runs: run: | echo "Starting Cloudstack health check" T=0 - until [ $T -gt 20 ] || curl -sfL http://localhost:8080 --output /dev/null + MAX_WAIT=40 + SLEEP_INTERVAL=30 + + echo "Will wait up to $((MAX_WAIT * SLEEP_INTERVAL / 60)) minutes for CloudStack to be ready" + + until [ $T -gt $MAX_WAIT ] || curl -sfL http://localhost:8080 --output /dev/null do - echo "Waiting for Cloudstack to be ready..." + echo "Waiting for Cloudstack to be ready... (attempt $((T+1))/$((MAX_WAIT+1)), elapsed: $((T * SLEEP_INTERVAL / 60))m $((T * SLEEP_INTERVAL % 60))s)" ((T+=1)) - sleep 30 + sleep $SLEEP_INTERVAL done # After loop, check if Cloudstack is up if ! curl -sfSL http://localhost:8080 --output /dev/null; then - echo "Cloudstack did not become ready in time" + echo "Cloudstack did not become ready in time after $((MAX_WAIT * SLEEP_INTERVAL / 60)) minutes" + echo "Checking CloudStack container status:" + docker ps -a | grep cloudstack || echo "No CloudStack containers found" + echo "Checking CloudStack logs:" + docker logs $(docker ps -q --filter "ancestor=apache/cloudstack-simulator") --tail 50 || echo "Could not retrieve logs" + echo "Testing connectivity:" curl -v http://localhost:8080 || true exit 22 + else + echo "CloudStack is ready after $((T * SLEEP_INTERVAL / 60))m $((T * SLEEP_INTERVAL % 60))s" fi - name: Setting up Cloudstack id: setup-cloudstack @@ -64,8 +76,23 @@ runs: set -euo pipefail echo "Deploying Data Center..." - docker exec $(docker container ls --format=json -l | jq -r .ID) \ - python /root/tools/marvin/marvin/deployDataCenter.py -i /root/setup/dev/advanced.cfg + # Retry data center deployment up to 3 times + for attempt in 1 2 3; do + echo "Data center deployment attempt $attempt/3" + if docker exec $(docker container ls --format=json -l | jq -r .ID) \ + python /root/tools/marvin/marvin/deployDataCenter.py -i /root/setup/dev/advanced.cfg; then + echo "Data center deployment successful on attempt $attempt" + break + else + echo "Data center deployment failed on attempt $attempt" + if [ $attempt -eq 3 ]; then + echo "All data center deployment attempts failed" + exit 1 + fi + echo "Waiting 30 seconds before retry..." + sleep 30 + fi + done # Get the container ID of the running simulator CONTAINER_ID=$(docker ps --filter "ancestor=apache/cloudstack-simulator:${{ matrix.cloudstack-version }}" --format "{{.ID}}" | head -n1) diff --git a/.github/workflows/acceptance.yml b/.github/workflows/acceptance.yml index 7b3d178a..239fc6cf 100644 --- a/.github/workflows/acceptance.yml +++ b/.github/workflows/acceptance.yml @@ -47,6 +47,7 @@ jobs: name: Terraform ${{ matrix.terraform-version }} with Cloudstack ${{ matrix.cloudstack-version }} needs: [prepare-matrix] runs-on: ubuntu-latest + timeout-minutes: 90 steps: - uses: actions/checkout@v4 - name: Set up Go @@ -86,6 +87,7 @@ jobs: name: OpenTofu ${{ matrix.opentofu-version }} with Cloudstack ${{ matrix.cloudstack-version }} needs: [prepare-matrix] runs-on: ubuntu-latest + timeout-minutes: 90 steps: - uses: actions/checkout@v4 - name: Set up Go diff --git a/GNUmakefile b/GNUmakefile index 58026716..5a5ffde6 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -31,7 +31,7 @@ test: fmtcheck xargs -t -n4 go test $(TESTARGS) -timeout=30s -parallel=4 testacc: fmtcheck - TF_ACC=1 go test $(TEST) -v $(TESTARGS) -timeout 30m + TF_ACC=1 go test $(TEST) -v $(TESTARGS) -timeout 60m vet: @echo "go vet ." diff --git a/cloudstack/resource_cloudstack_egress_firewall.go b/cloudstack/resource_cloudstack_egress_firewall.go index e2a83e4c..67bb08b7 100644 --- a/cloudstack/resource_cloudstack_egress_firewall.go +++ b/cloudstack/resource_cloudstack_egress_firewall.go @@ -21,6 +21,7 @@ package cloudstack import ( "fmt" + "sort" "strconv" "strings" "sync" @@ -31,6 +32,81 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) +// treats 'all ports' for tcp/udp across CS versions returning 0/0, -1/-1, or 1/65535 +func isAllPortsTCPUDP(protocol string, start, end int) bool { + p := strings.ToLower(protocol) + if p != "tcp" && p != "udp" { + return false + } + // handle various representations of all ports across CloudStack versions + if (start == 0 && end == 0) || + (start == -1 && end == -1) || + (start == 1 && end == 65535) { + return true + } + return false +} + +// normalizeRemoteCIDRs normalizes a comma-separated CIDR string from CloudStack API +func normalizeRemoteCIDRs(cidrList string) []string { + if cidrList == "" { + return []string{} + } + + cidrs := strings.Split(cidrList, ",") + normalized := make([]string, 0, len(cidrs)) + + for _, cidr := range cidrs { + trimmed := strings.TrimSpace(cidr) + if trimmed != "" { + normalized = append(normalized, trimmed) + } + } + + sort.Strings(normalized) + return normalized +} + +// normalizeLocalCIDRs normalizes a Terraform schema.Set of CIDRs +func normalizeLocalCIDRs(cidrSet *schema.Set) []string { + if cidrSet == nil { + return []string{} + } + + normalized := make([]string, 0, cidrSet.Len()) + for _, cidr := range cidrSet.List() { + if cidrStr, ok := cidr.(string); ok { + trimmed := strings.TrimSpace(cidrStr) + if trimmed != "" { + normalized = append(normalized, trimmed) + } + } + } + + sort.Strings(normalized) + return normalized +} + +// cidrSetsEqual compares normalized CIDR sets for equality (order/whitespace agnostic) +func cidrSetsEqual(remoteCidrList string, localCidrSet *schema.Set) bool { + remoteCidrs := normalizeRemoteCIDRs(remoteCidrList) + localCidrs := normalizeLocalCIDRs(localCidrSet) + + // Compare lengths first + if len(remoteCidrs) != len(localCidrs) { + return false + } + + // Compare each element (both are already sorted) + for i, remoteCidr := range remoteCidrs { + if remoteCidr != localCidrs[i] { + return false + } + } + + return true +} + func resourceCloudStackEgressFirewall() *schema.Resource { return &schema.Resource{ Create: resourceCloudStackEgressFirewallCreate, @@ -250,6 +326,17 @@ func createEgressFirewallRule(d *schema.ResourceData, meta interface{}, rule map uuids[port.(string)] = r.Id rule["uuids"] = uuids } + } else { + // No ports specified - create a rule that encompasses all ports + // by not setting startport and endport parameters + r, err := cs.Firewall.CreateEgressFirewallRule(p) + if err != nil { + return fmt.Errorf("failed to create all-ports egress firewall rule: %w", err) + } + uuids["all"] = r.Id + rule["uuids"] = uuids + // Remove the ports field since we're creating an all-ports rule + delete(rule, "ports") } } @@ -315,8 +402,13 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface // Create a set with all CIDR's cidrs := &schema.Set{F: schema.HashString} - for _, cidr := range strings.Split(r.Cidrlist, ",") { - cidrs.Add(cidr) + if r.Cidrlist != "" { + for _, cidr := range strings.Split(r.Cidrlist, ",") { + cidr = strings.TrimSpace(cidr) + if cidr != "" { + cidrs.Add(cidr) + } + } } // Update the values @@ -353,8 +445,13 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface // Create a set with all CIDR's cidrs := &schema.Set{F: schema.HashString} - for _, cidr := range strings.Split(r.Cidrlist, ",") { - cidrs.Add(cidr) + if r.Cidrlist != "" { + for _, cidr := range strings.Split(r.Cidrlist, ",") { + cidr = strings.TrimSpace(cidr) + if cidr != "" { + cidrs.Add(cidr) + } + } } // Update the values @@ -368,8 +465,88 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface rule["ports"] = ports rules.Add(rule) } + } else { + // No ports specified - check if we have an "all" rule + id, ok := uuids["all"] + if !ok { + continue + } + + // Get the rule + r, ok := ruleMap[id.(string)] + if !ok { + delete(uuids, "all") + continue + } + + // Verify this is actually an all-ports rule using our helper + if !isAllPortsTCPUDP(r.Protocol, r.Startport, r.Endport) { + // This rule has specific ports, but we expected all-ports + // This might happen if CloudStack behavior changed + continue + } + + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id.(string)) + + // Create a set with all CIDR's + cidrs := &schema.Set{F: schema.HashString} + if r.Cidrlist != "" { + for _, cidr := range strings.Split(r.Cidrlist, ",") { + cidr = strings.TrimSpace(cidr) + if cidr != "" { + cidrs.Add(cidr) + } + } + } + + // Update the values + rule["protocol"] = r.Protocol + rule["cidr_list"] = cidrs + // Remove ports field for all-ports rules + delete(rule, "ports") + rules.Add(rule) } } + + // Fallback: Check if any remaining rules in ruleMap match our expected all-ports pattern + // This handles cases where CloudStack might return all-ports rules in unexpected formats + if rule["protocol"].(string) != "icmp" && strings.ToLower(rule["protocol"].(string)) != "all" { + // Look for any remaining rules that might be our all-ports rule + for ruleID, r := range ruleMap { + // Get local CIDR set for comparison + localCidrSet, ok := rule["cidr_list"].(*schema.Set) + if !ok { + continue + } + + if isAllPortsTCPUDP(r.Protocol, r.Startport, r.Endport) && + strings.EqualFold(r.Protocol, rule["protocol"].(string)) && + cidrSetsEqual(r.Cidrlist, localCidrSet) { + // This looks like our all-ports rule, add it to state + cidrs := &schema.Set{F: schema.HashString} + if r.Cidrlist != "" { + for _, cidr := range strings.Split(r.Cidrlist, ",") { + cidr = strings.TrimSpace(cidr) + if cidr != "" { + cidrs.Add(cidr) + } + } + } + + rule["protocol"] = r.Protocol + rule["cidr_list"] = cidrs + // Remove ports field for all-ports rules + delete(rule, "ports") + rules.Add(rule) + + // Remove from ruleMap so it's not processed again + delete(ruleMap, ruleID) + break + } + } + } + if strings.ToLower(rule["protocol"].(string)) == "all" { id, ok := uuids["all"] if !ok { @@ -389,8 +566,13 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface // Create a set with all CIDR's if _, ok := rule["cidr_list"]; ok { cidrs := &schema.Set{F: schema.HashString} - for _, cidr := range strings.Split(r.Cidrlist, ",") { - cidrs.Add(cidr) + if r.Cidrlist != "" { + for _, cidr := range strings.Split(r.Cidrlist, ",") { + cidr = strings.TrimSpace(cidr) + if cidr != "" { + cidrs.Add(cidr) + } + } } rule["cidr_list"] = cidrs } @@ -575,7 +757,7 @@ func verifyEgressFirewallParams(d *schema.ResourceData) error { if !rules && !managed { return fmt.Errorf( - "You must supply at least one 'rule' when not using the 'managed' firewall feature") + "you must supply at least one 'rule' when not using the 'managed' firewall feature") } return nil @@ -591,11 +773,11 @@ func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]inte if protocol == "icmp" { if _, ok := rule["icmp_type"]; !ok { return fmt.Errorf( - "Parameter icmp_type is a required parameter when using protocol 'icmp'") + "parameter icmp_type is a required parameter when using protocol 'icmp'") } if _, ok := rule["icmp_code"]; !ok { return fmt.Errorf( - "Parameter icmp_code is a required parameter when using protocol 'icmp'") + "parameter icmp_code is a required parameter when using protocol 'icmp'") } } else if strings.ToLower(protocol) != "all" { if ports, ok := rule["ports"].(*schema.Set); ok { @@ -606,14 +788,13 @@ func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]inte "%q is not a valid port value. Valid options are '80' or '80-90'", port.(string)) } } - } else { - return fmt.Errorf( - "Parameter ports is a required parameter when *not* using protocol 'icmp'") } + // Note: ports parameter is optional for TCP/UDP protocols + // When omitted, the rule will encompass all ports } else if strings.ToLower(protocol) == "all" { if ports, _ := rule["ports"].(*schema.Set); ports.Len() > 0 { return fmt.Errorf( - "Parameter ports is not required when using protocol 'ALL'") + "parameter ports is not required when using protocol 'ALL'") } } diff --git a/cloudstack/resource_cloudstack_egress_firewall_test.go b/cloudstack/resource_cloudstack_egress_firewall_test.go index 28b664f7..bbf825bf 100644 --- a/cloudstack/resource_cloudstack_egress_firewall_test.go +++ b/cloudstack/resource_cloudstack_egress_firewall_test.go @@ -202,3 +202,197 @@ resource "cloudstack_egress_firewall" "foo" { ports = ["80", "1000-2000"] } }` + +func TestAccCloudStackEgressFirewall_allPortsTCP(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackEgressFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackEgressFirewall_allPorts, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.#", "1"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.0.cidr_list.0", "10.1.1.10/32"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.0.protocol", "tcp"), + // No ports should be set when omitting the ports parameter + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.0.ports.#", "0"), + ), + }, + }, + }) +} + +const testAccCloudStackEgressFirewall_allPorts = ` +resource "cloudstack_network" "foo" { + name = "terraform-network-tcp" + display_text = "terraform-network-tcp" + cidr = "10.1.1.0/24" + network_offering = "DefaultIsolatedNetworkOfferingWithSourceNatService" + zone = "Sandbox-simulator" +} + +resource "cloudstack_egress_firewall" "foo" { + network_id = cloudstack_network.foo.id + + rule { + cidr_list = ["10.1.1.10/32"] + protocol = "tcp" + # No ports specified - should create a rule for all ports + } +}` + +func TestAccCloudStackEgressFirewall_allPortsUDP(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackEgressFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackEgressFirewall_allPortsUDP, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.foo", "rule.#", "1"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.foo", "rule.0.protocol", "udp"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.foo", "rule.0.ports.#", "0"), + ), + }, + }, + }) +} + +const testAccCloudStackEgressFirewall_allPortsUDP = ` +resource "cloudstack_network" "foo" { + name = "tf-egress-udp-all" + display_text = "tf-egress-udp-all" + cidr = "10.8.0.0/24" + network_offering = "DefaultIsolatedNetworkOfferingWithSourceNatService" + zone = "Sandbox-simulator" +} + +resource "cloudstack_egress_firewall" "foo" { + network_id = cloudstack_network.foo.id + + rule { + cidr_list = ["10.8.0.10/32"] + protocol = "udp" + # no ports => all ports + } +}` + +func TestAccCloudStackEgressFirewall_allPortsCombined(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackEgressFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackEgressFirewall_allPortsCombined, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.mixed"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.mixed", "rule.#", "2"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.mixed", "rule.0.protocol", "tcp"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.mixed", "rule.0.ports.#", "2"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.mixed", "rule.1.protocol", "udp"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.mixed", "rule.1.ports.#", "0"), + ), + }, + }, + }) +} + +const testAccCloudStackEgressFirewall_allPortsCombined = ` +resource "cloudstack_network" "foo" { + name = "terraform-network-mixed" + display_text = "terraform-network-mixed" + cidr = "10.1.3.0/24" + network_offering = "DefaultIsolatedNetworkOfferingWithSourceNatService" + zone = "Sandbox-simulator" +} + +resource "cloudstack_egress_firewall" "mixed" { + network_id = cloudstack_network.foo.id + + rule { + cidr_list = [cloudstack_network.foo.cidr] + protocol = "tcp" + ports = ["80", "443"] + } + + rule { + cidr_list = ["${cidrhost(cloudstack_network.foo.cidr, 10)}"] + protocol = "udp" + # no ports => all ports + } +}` + +func TestAccCloudStackEgressFirewall_portsToAllPorts(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackEgressFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackEgressFirewall_specificPorts, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.foo", "rule.#", "1"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.foo", "rule.0.ports.#", "2"), + ), + }, + { + Config: testAccCloudStackEgressFirewall_allPortsTransition, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.foo", "rule.#", "1"), + resource.TestCheckResourceAttr("cloudstack_egress_firewall.foo", "rule.0.ports.#", "0"), + ), + }, + }, + }) +} + +const testAccCloudStackEgressFirewall_specificPorts = ` +resource "cloudstack_network" "foo" { + name = "terraform-network-transition" + display_text = "terraform-network-transition" + cidr = "10.1.4.0/24" + network_offering = "DefaultIsolatedNetworkOfferingWithSourceNatService" + zone = "Sandbox-simulator" +} + +resource "cloudstack_egress_firewall" "foo" { + network_id = cloudstack_network.foo.id + + rule { + cidr_list = ["${cidrhost(cloudstack_network.foo.cidr, 10)}"] + protocol = "tcp" + ports = ["80", "1000-2000"] + } +} +` + +const testAccCloudStackEgressFirewall_allPortsTransition = ` +resource "cloudstack_network" "foo" { + name = "terraform-network-transition" + display_text = "terraform-network-transition" + cidr = "10.1.4.0/24" + network_offering = "DefaultIsolatedNetworkOfferingWithSourceNatService" + zone = "Sandbox-simulator" +} + +resource "cloudstack_egress_firewall" "foo" { + network_id = cloudstack_network.foo.id + + rule { + cidr_list = ["${cidrhost(cloudstack_network.foo.cidr, 10)}"] + protocol = "tcp" + # no ports => all ports + } +} +` diff --git a/cloudstack/resource_cloudstack_egress_firewall_unit_test.go b/cloudstack/resource_cloudstack_egress_firewall_unit_test.go new file mode 100644 index 00000000..f300e720 --- /dev/null +++ b/cloudstack/resource_cloudstack_egress_firewall_unit_test.go @@ -0,0 +1,155 @@ +// +// 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 ( + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func TestIsAllPortsTCPUDP(t *testing.T) { + tests := []struct { + protocol string + start int + end int + expected bool + name string + }{ + {"tcp", 0, 0, true, "TCP with 0/0"}, + {"TCP", 0, 0, true, "TCP uppercase with 0/0"}, + {"udp", -1, -1, true, "UDP with -1/-1"}, + {"UDP", -1, -1, true, "UDP uppercase with -1/-1"}, + {"tcp", 1, 65535, true, "TCP with 1/65535"}, + {"udp", 1, 65535, true, "UDP with 1/65535"}, + {"tcp", 80, 80, false, "TCP with specific port"}, + {"udp", 53, 53, false, "UDP with specific port"}, + {"icmp", 0, 0, false, "ICMP protocol"}, + {"all", 0, 0, false, "ALL protocol"}, + {"tcp", 1, 1000, false, "TCP with port range"}, + {"tcp", 0, 1, false, "TCP with 0/1"}, + {"tcp", -1, 0, false, "TCP with -1/0"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := isAllPortsTCPUDP(test.protocol, test.start, test.end) + if result != test.expected { + t.Errorf("isAllPortsTCPUDP(%q, %d, %d) = %v, expected %v", + test.protocol, test.start, test.end, result, test.expected) + } + }) + } +} + +func TestNormalizeRemoteCIDRs(t *testing.T) { + tests := []struct { + input string + expected []string + name string + }{ + {"", []string{}, "empty string"}, + {"10.0.0.0/8", []string{"10.0.0.0/8"}, "single CIDR"}, + {"10.0.0.0/8,192.168.1.0/24", []string{"10.0.0.0/8", "192.168.1.0/24"}, "two CIDRs"}, + {"10.0.0.0/8, 192.168.1.0/24", []string{"10.0.0.0/8", "192.168.1.0/24"}, "two CIDRs with space"}, + {" 10.0.0.0/8 , 192.168.1.0/24 ", []string{"10.0.0.0/8", "192.168.1.0/24"}, "CIDRs with extra spaces"}, + {"192.168.1.0/24,10.0.0.0/8", []string{"10.0.0.0/8", "192.168.1.0/24"}, "unsorted CIDRs (should be sorted)"}, + {"10.0.0.0/8,,192.168.1.0/24", []string{"10.0.0.0/8", "192.168.1.0/24"}, "empty CIDR in middle"}, + {" , , ", []string{}, "only commas and spaces"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := normalizeRemoteCIDRs(test.input) + if len(result) != len(test.expected) { + t.Errorf("normalizeRemoteCIDRs(%q) length = %d, expected %d", + test.input, len(result), len(test.expected)) + return + } + for i, v := range result { + if v != test.expected[i] { + t.Errorf("normalizeRemoteCIDRs(%q)[%d] = %q, expected %q", + test.input, i, v, test.expected[i]) + } + } + }) + } +} + +func TestNormalizeLocalCIDRs(t *testing.T) { + tests := []struct { + input *schema.Set + expected []string + name string + }{ + {nil, []string{}, "nil set"}, + {schema.NewSet(schema.HashString, []interface{}{}), []string{}, "empty set"}, + {schema.NewSet(schema.HashString, []interface{}{"10.0.0.0/8"}), []string{"10.0.0.0/8"}, "single CIDR"}, + {schema.NewSet(schema.HashString, []interface{}{"10.0.0.0/8", "192.168.1.0/24"}), []string{"10.0.0.0/8", "192.168.1.0/24"}, "two CIDRs"}, + {schema.NewSet(schema.HashString, []interface{}{"192.168.1.0/24", "10.0.0.0/8"}), []string{"10.0.0.0/8", "192.168.1.0/24"}, "unsorted CIDRs"}, + {schema.NewSet(schema.HashString, []interface{}{" 10.0.0.0/8 ", " 192.168.1.0/24 "}), []string{"10.0.0.0/8", "192.168.1.0/24"}, "CIDRs with spaces"}, + {schema.NewSet(schema.HashString, []interface{}{"10.0.0.0/8", "", "192.168.1.0/24"}), []string{"10.0.0.0/8", "192.168.1.0/24"}, "with empty string"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := normalizeLocalCIDRs(test.input) + if len(result) != len(test.expected) { + t.Errorf("normalizeLocalCIDRs() length = %d, expected %d", + len(result), len(test.expected)) + return + } + for i, v := range result { + if v != test.expected[i] { + t.Errorf("normalizeLocalCIDRs()[%d] = %q, expected %q", + i, v, test.expected[i]) + } + } + }) + } +} + +func TestCidrSetsEqual(t *testing.T) { + tests := []struct { + remote string + local *schema.Set + expected bool + name string + }{ + {"", schema.NewSet(schema.HashString, []interface{}{}), true, "both empty"}, + {"10.0.0.0/8", schema.NewSet(schema.HashString, []interface{}{"10.0.0.0/8"}), true, "single matching CIDR"}, + {"10.0.0.0/8,192.168.1.0/24", schema.NewSet(schema.HashString, []interface{}{"192.168.1.0/24", "10.0.0.0/8"}), true, "multiple CIDRs different order"}, + {"10.0.0.0/8, 192.168.1.0/24", schema.NewSet(schema.HashString, []interface{}{"10.0.0.0/8", "192.168.1.0/24"}), true, "remote with spaces"}, + {"10.0.0.0/8", schema.NewSet(schema.HashString, []interface{}{"192.168.1.0/24"}), false, "different CIDRs"}, + {"10.0.0.0/8,192.168.1.0/24", schema.NewSet(schema.HashString, []interface{}{"10.0.0.0/8"}), false, "different count"}, + {"", schema.NewSet(schema.HashString, []interface{}{"10.0.0.0/8"}), false, "remote empty, local not"}, + {"10.0.0.0/8", schema.NewSet(schema.HashString, []interface{}{}), false, "local empty, remote not"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := cidrSetsEqual(test.remote, test.local) + if result != test.expected { + t.Errorf("cidrSetsEqual(%q, %v) = %v, expected %v", + test.remote, test.local.List(), result, test.expected) + } + }) + } +} diff --git a/website/docs/r/egress_firewall.html.markdown b/website/docs/r/egress_firewall.html.markdown index 10badd17..3b13e983 100644 --- a/website/docs/r/egress_firewall.html.markdown +++ b/website/docs/r/egress_firewall.html.markdown @@ -24,6 +24,56 @@ resource "cloudstack_egress_firewall" "default" { } ``` +### Example: All Ports Rule + +To create a rule that encompasses all ports for a protocol, simply omit the `ports` parameter: + +```hcl +resource "cloudstack_egress_firewall" "all_ports" { + network_id = "6eb22f91-7454-4107-89f4-36afcdf33021" + + rule { + cidr_list = ["10.0.0.0/8"] + protocol = "tcp" + # No ports specified - rule will encompass all TCP ports + } +} +``` + +### Example: UDP All Ports + +```hcl +resource "cloudstack_egress_firewall" "all_ports_udp" { + network_id = "6eb22f91-7454-4107-89f4-36afcdf33021" + + rule { + cidr_list = ["10.1.0.0/16"] + protocol = "udp" + # No ports => all UDP ports + } +} +``` + +### Example: Mixed Rules (specific + all-ports) + +```hcl +resource "cloudstack_egress_firewall" "mixed_rules" { + network_id = "6eb22f91-7454-4107-89f4-36afcdf33021" + + rule { + cidr_list = ["10.0.0.0/8"] + protocol = "tcp" + ports = ["80", "443"] + } + + rule { + cidr_list = ["10.1.0.0/16"] + protocol = "udp" + # No ports => all UDP ports + } +} +``` + ## Argument Reference The following arguments are supported: @@ -55,7 +105,7 @@ The `rule` block supports: the protocol is ICMP. * `ports` - (Optional) List of ports and/or port ranges to allow. This can only - be specified if the protocol is TCP or UDP. + be specified if the protocol is TCP or UDP. For TCP/UDP, omitting `ports` creates an all-ports rule. CloudStack may represent this as empty start/end, `0/0`, or `1/65535`; the provider handles all. ## Attributes Reference