Skip to content

Commit bb09664

Browse files
committed
Fix egress firewall rules to support all-ports without explicit ports
- Allow omitting ports parameter for TCP/UDP protocols to create rules that encompass all ports - Update validation to make ports parameter optional for TCP/UDP protocols - Add support for reading rules created without explicit ports - Add test case for all-ports functionality - Update documentation with example and clarification Fixes #202
1 parent 3021ce2 commit bb09664

File tree

3 files changed

+100
-4
lines changed

3 files changed

+100
-4
lines changed

cloudstack/resource_cloudstack_egress_firewall.go

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,15 @@ func createEgressFirewallRule(d *schema.ResourceData, meta interface{}, rule map
250250
uuids[port.(string)] = r.Id
251251
rule["uuids"] = uuids
252252
}
253+
} else {
254+
// No ports specified - create a rule that encompasses all ports
255+
// by not setting startport and endport parameters
256+
r, err := cs.Firewall.CreateEgressFirewallRule(p)
257+
if err != nil {
258+
return err
259+
}
260+
uuids["all"] = r.Id
261+
rule["uuids"] = uuids
253262
}
254263
}
255264

@@ -368,6 +377,33 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
368377
rule["ports"] = ports
369378
rules.Add(rule)
370379
}
380+
} else {
381+
// No ports specified - check if we have an "all" rule
382+
id, ok := uuids["all"]
383+
if !ok {
384+
continue
385+
}
386+
387+
// Get the rule
388+
r, ok := ruleMap[id.(string)]
389+
if !ok {
390+
delete(uuids, "all")
391+
continue
392+
}
393+
394+
// Delete the known rule so only unknown rules remain in the ruleMap
395+
delete(ruleMap, id.(string))
396+
397+
// Create a set with all CIDR's
398+
cidrs := &schema.Set{F: schema.HashString}
399+
for _, cidr := range strings.Split(r.Cidrlist, ",") {
400+
cidrs.Add(cidr)
401+
}
402+
403+
// Update the values
404+
rule["protocol"] = r.Protocol
405+
rule["cidr_list"] = cidrs
406+
rules.Add(rule)
371407
}
372408
}
373409
if strings.ToLower(rule["protocol"].(string)) == "all" {
@@ -606,10 +642,9 @@ func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]inte
606642
"%q is not a valid port value. Valid options are '80' or '80-90'", port.(string))
607643
}
608644
}
609-
} else {
610-
return fmt.Errorf(
611-
"Parameter ports is a required parameter when *not* using protocol 'icmp'")
612645
}
646+
// Note: ports parameter is optional for TCP/UDP protocols
647+
// When omitted, the rule will encompass all ports
613648
} else if strings.ToLower(protocol) == "all" {
614649
if ports, _ := rule["ports"].(*schema.Set); ports.Len() > 0 {
615650
return fmt.Errorf(

cloudstack/resource_cloudstack_egress_firewall_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,3 +202,47 @@ resource "cloudstack_egress_firewall" "foo" {
202202
ports = ["80", "1000-2000"]
203203
}
204204
}`
205+
206+
func TestAccCloudStackEgressFirewall_allPorts(t *testing.T) {
207+
resource.Test(t, resource.TestCase{
208+
PreCheck: func() { testAccPreCheck(t) },
209+
Providers: testAccProviders,
210+
CheckDestroy: testAccCheckCloudStackEgressFirewallDestroy,
211+
Steps: []resource.TestStep{
212+
{
213+
Config: testAccCloudStackEgressFirewall_allPorts,
214+
Check: resource.ComposeTestCheckFunc(
215+
testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"),
216+
resource.TestCheckResourceAttr(
217+
"cloudstack_egress_firewall.foo", "rule.#", "1"),
218+
resource.TestCheckResourceAttr(
219+
"cloudstack_egress_firewall.foo", "rule.0.cidr_list.0", "10.1.1.10/32"),
220+
resource.TestCheckResourceAttr(
221+
"cloudstack_egress_firewall.foo", "rule.0.protocol", "tcp"),
222+
// No ports should be set when omitting the ports parameter
223+
resource.TestCheckNoResourceAttr(
224+
"cloudstack_egress_firewall.foo", "rule.0.ports"),
225+
),
226+
},
227+
},
228+
})
229+
}
230+
231+
const testAccCloudStackEgressFirewall_allPorts = `
232+
resource "cloudstack_network" "foo" {
233+
name = "terraform-network"
234+
display_text = "terraform-network"
235+
cidr = "10.1.1.0/24"
236+
network_offering = "DefaultIsolatedNetworkOfferingWithSourceNatService"
237+
zone = "Sandbox-simulator"
238+
}
239+
240+
resource "cloudstack_egress_firewall" "foo" {
241+
network_id = cloudstack_network.foo.id
242+
243+
rule {
244+
cidr_list = ["10.1.1.10/32"]
245+
protocol = "tcp"
246+
# No ports specified - should create a rule for all ports
247+
}
248+
}`

website/docs/r/egress_firewall.html.markdown

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,22 @@ resource "cloudstack_egress_firewall" "default" {
2424
}
2525
```
2626

27+
### Example: All Ports Rule
28+
29+
To create a rule that encompasses all ports for a protocol, simply omit the `ports` parameter:
30+
31+
```hcl
32+
resource "cloudstack_egress_firewall" "all_ports" {
33+
network_id = "6eb22f91-7454-4107-89f4-36afcdf33021"
34+
35+
rule {
36+
cidr_list = ["10.0.0.0/8"]
37+
protocol = "tcp"
38+
# No ports specified - rule will encompass all TCP ports
39+
}
40+
}
41+
```
42+
2743
## Argument Reference
2844

2945
The following arguments are supported:
@@ -55,7 +71,8 @@ The `rule` block supports:
5571
the protocol is ICMP.
5672

5773
* `ports` - (Optional) List of ports and/or port ranges to allow. This can only
58-
be specified if the protocol is TCP or UDP.
74+
be specified if the protocol is TCP or UDP. When omitted for TCP or UDP protocols,
75+
the rule will encompass all ports.
5976

6077
## Attributes Reference
6178

0 commit comments

Comments
 (0)