Skip to content

Commit 56b0cc3

Browse files
committed
Apply code review fixes: improve documentation and comments
- Fix comment grammar in isAllPortsTCPUDP function - Fix documentation grammar in egress_firewall.html.markdown - Move inline comments to function-level documentation - Address all Copilot review suggestions
1 parent 987dc7b commit 56b0cc3

3 files changed

Lines changed: 4 additions & 4 deletions

File tree

cloudstack/resource_cloudstack_egress_firewall.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import (
3232
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
3333
)
3434

35-
// treats 'all ports' for tcp/udp across CS versions returning 0/0, -1/-1, or 1/65535
35+
// isAllPortsTCPUDP determines if a rule represents all ports for TCP/UDP protocols across CloudStack versions that may return 0/0, -1/-1, or 1/65535
3636
func isAllPortsTCPUDP(protocol string, start, end int) bool {
3737
p := strings.ToLower(protocol)
3838
if p != "tcp" && p != "udp" {
@@ -732,6 +732,8 @@ func verifyEgressFirewallParams(d *schema.ResourceData) error {
732732
return nil
733733
}
734734

735+
// verifyEgressFirewallRuleParams validates egress firewall rule parameters.
736+
// Note: ports parameter is optional for TCP/UDP protocols - when omitted, the rule will encompass all ports
735737
func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{}) error {
736738
protocol := rule["protocol"].(string)
737739
if strings.ToLower(protocol) != "all" && protocol != "tcp" && protocol != "udp" && protocol != "icmp" {
@@ -758,8 +760,6 @@ func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]inte
758760
}
759761
}
760762
}
761-
// Note: ports parameter is optional for TCP/UDP protocols
762-
// When omitted, the rule will encompass all ports
763763
} else if strings.ToLower(protocol) == "all" {
764764
if ports, _ := rule["ports"].(*schema.Set); ports.Len() > 0 {
765765
return fmt.Errorf(

terraform-provider-cloudstack

40.4 MB
Binary file not shown.

website/docs/r/egress_firewall.html.markdown

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ The `rule` block supports:
105105
the protocol is ICMP.
106106

107107
* `ports` - (Optional) List of ports and/or port ranges to allow. This can only
108-
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.
108+
be specified if the protocol is TCP or UDP. For TCP/UDP protocols, omitting `ports` creates an all-ports rule. CloudStack may represent this as empty start/end ports, `0/0`, or `1/65535`; the provider handles all formats.
109109

110110
## Attributes Reference
111111

0 commit comments

Comments
 (0)