Bugfix: Implement egress firewall cidr validation

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(

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) },