From feca20f7a5432629a5c354698e2af4349af313da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elias=20Fl=C3=B6tzinger?= Date: Wed, 19 Aug 2026 09:56:39 +0200 Subject: [PATCH] feat!: disallow 0.0.0.0/0 and empty cidr_list in egress firewall rules CloudStack silently rewrites an egress firewall rule's cidr_list to the network's CIDR whenever it is empty or contains 0.0.0.0/0: Collections.replaceAll(sourceCidrs, "0.0.0.0/0", network.getCidr()); Since Terraform reads this rewritten value back from CloudStack, a config that omits cidr_list or sets 0.0.0.0/0 causes a perpetual diff on every plan/apply, as Terraform keeps trying to reconcile state with the rewritten CIDR CloudStack actually stored. To prevent this, cidr_list is now required and must contain at least one entry, and 0.0.0.0/0 is explicitly rejected so that CIDR values must be within the network's own subnet. BREAKING CHANGE: cidr_list is now required and must be a non-empty list of valid subnet CIDRs. 0.0.0.0/0 is no longer accepted. --- .../resource_cloudstack_egress_firewall.go | 17 ++++- ...esource_cloudstack_egress_firewall_test.go | 64 +++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/cloudstack/resource_cloudstack_egress_firewall.go b/cloudstack/resource_cloudstack_egress_firewall.go index beec0bd0..ec703e0b 100644 --- a/cloudstack/resource_cloudstack_egress_firewall.go +++ b/cloudstack/resource_cloudstack_egress_firewall.go @@ -65,7 +65,7 @@ func resourceCloudStackEgressFirewall() *schema.Resource { Schema: map[string]*schema.Schema{ "cidr_list": { Type: schema.TypeSet, - Optional: true, + Required: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, @@ -542,7 +542,7 @@ func deleteEgressFirewallRules(d *schema.ResourceData, meta interface{}, rules * return errs.ErrorOrNil() } -func deleteEgressFirewallRule(d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error { +func deleteEgressFirewallRule(_ *schema.ResourceData, meta interface{}, rule map[string]interface{}) error { cs := meta.(*cloudstack.CloudStackClient) uuids := rule["uuids"].(map[string]interface{}) @@ -589,7 +589,18 @@ func verifyEgressFirewallParams(d *schema.ResourceData) error { return nil } -func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{}) error { +func verifyEgressFirewallRuleParams(_ *schema.ResourceData, rule map[string]interface{}) error { + cidrList, ok := rule["cidr_list"].(*schema.Set) + if !ok { + return fmt.Errorf("cidr_list must be a set of CIDR strings") + } + if cidrList.Len() == 0 { + return fmt.Errorf("cidr_list must not be empty in egress firewall rules") + } + if cidrList.Contains("0.0.0.0/0") { + return fmt.Errorf("CIDR 0.0.0.0/0 is not allowed in egress firewall rules, as CloudStack silently rewrites it to the network CIDR, causing perpetual Terraform diffs") + } + protocol := rule["protocol"].(string) if strings.ToLower(protocol) != "all" && protocol != "tcp" && protocol != "udp" && protocol != "icmp" { return fmt.Errorf( diff --git a/cloudstack/resource_cloudstack_egress_firewall_test.go b/cloudstack/resource_cloudstack_egress_firewall_test.go index 28b664f7..806741b9 100644 --- a/cloudstack/resource_cloudstack_egress_firewall_test.go +++ b/cloudstack/resource_cloudstack_egress_firewall_test.go @@ -25,10 +25,74 @@ import ( "testing" "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" ) +func TestVerifyEgressFirewallRuleParams_disallowAnyIP(t *testing.T) { + cidrSet := schema.NewSet(schema.HashString, []interface{}{"0.0.0.0/0"}) + rule := map[string]interface{}{ + "cidr_list": cidrSet, + "protocol": "tcp", + "ports": schema.NewSet(schema.HashString, []interface{}{"80"}), + "uuids": map[string]interface{}{}, + } + + err := verifyEgressFirewallRuleParams(nil, rule) + if err == nil { + t.Fatal("expected error for cidr 0.0.0.0/0, got nil") + } + if !strings.Contains(err.Error(), "0.0.0.0/0") { + t.Fatalf("expected error message to mention 0.0.0.0/0, got: %s", err.Error()) + } +} + +func TestVerifyEgressFirewallRuleParams_disallowEmptyCIDR(t *testing.T) { + rule := map[string]interface{}{ + "cidr_list": schema.NewSet(schema.HashString, []interface{}{}), + "protocol": "tcp", + "ports": schema.NewSet(schema.HashString, []interface{}{"80"}), + "uuids": map[string]interface{}{}, + } + + err := verifyEgressFirewallRuleParams(nil, rule) + if err == nil { + t.Fatal("expected error for empty cidr_list, got nil") + } + if !strings.Contains(err.Error(), "empty") { + t.Fatalf("expected error message to mention empty, got: %s", err.Error()) + } +} + +func TestVerifyEgressFirewallRuleParams_allowValidCIDR(t *testing.T) { + cidrSet := schema.NewSet(schema.HashString, []interface{}{"10.1.1.0/24"}) + rule := map[string]interface{}{ + "cidr_list": cidrSet, + "protocol": "tcp", + "ports": schema.NewSet(schema.HashString, []interface{}{"80"}), + "uuids": map[string]interface{}{}, + } + + err := verifyEgressFirewallRuleParams(nil, rule) + if err != nil { + t.Fatalf("expected no error for valid cidr, got: %s", err.Error()) + } +} + +func TestEgressFirewallRuleSchema_cidrListIsRequired(t *testing.T) { + res := resourceCloudStackEgressFirewall() + ruleElem := res.Schema["rule"].Elem.(*schema.Resource) + cidrSchema := ruleElem.Schema["cidr_list"] + + if !cidrSchema.Required { + t.Error("expected cidr_list to be Required") + } + if cidrSchema.Optional { + t.Error("expected cidr_list not to be Optional") + } +} + func TestAccCloudStackEgressFirewall_basic(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) },