Skip to content

Commit 07fb8b9

Browse files
lgarber-akamailgarber-akamai
authored
Add support for firewall protocol ALL and numeric (#2408)
1 parent 4bf9193 commit 07fb8b9

13 files changed

Lines changed: 340 additions & 17 deletions

‎docs/data-sources/firewall.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ The following arguments are supported in the inbound and outbound rule blocks:
6363

6464
* `action` - Controls whether traffic is accepted or dropped by this rule. Overrides the Firewall’s inbound_policy if this is an inbound rule, or the outbound_policy if this is an outbound rule.
6565

66-
* `protocol` - The network protocol this rule controls. (`TCP`, `UDP`, `ICMP`)
66+
* `protocol` - The network protocol this rule controls. Possible values include `ALL`, `TCP`, `UDP`, `ICMP`, `IPENCAP`, or a protocol number from `0` to `255`.
6767

6868
* `ports` - A string representation of ports and/or port ranges (i.e. "443" or "80-90, 91").
6969

‎docs/data-sources/firewalls.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ Each Linode firewall will be stored in the `firewalls` attribute and will export
9999

100100
* `action` - Controls whether traffic is accepted or dropped by this rule (ACCEPT, DROP).
101101

102-
* `protocol` - The network protocol this rule controls. (TCP, UDP, ICMP)
102+
* `protocol` - The network protocol this rule controls. Possible values include `ALL`, `TCP`, `UDP`, `ICMP`, `IPENCAP`, or a protocol number from `0` to `255`.
103103

104104
* `ports` - A string representation of ports and/or port ranges (i.e. "443" or "80-90, 91").
105105

‎docs/resources/firewall.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ The following arguments are supported in the inbound and outbound rule blocks:
104104

105105
* `action` - (required) Controls whether traffic is accepted or dropped by this rule (`ACCEPT`, `DROP`). Overrides the Firewall’s inbound_policy if this is an inbound rule, or the outbound_policy if this is an outbound rule.
106106

107-
* `protocol` - (Required) The network protocol this rule controls. (`TCP`, `UDP`, `ICMP`)
107+
* `protocol` - (Required) The network protocol this rule controls. Accepted values are `ALL`, `TCP`, `UDP`, `ICMP`, `IPENCAP`, or a protocol number from `0` to `255`.
108108

109109
* `ports` - (Optional) A string representation of ports and/or port ranges (i.e. "443" or "80-90, 91").
110110

‎linode/firewall/framework_resource_test.go‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ import (
1010

1111
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
1212
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
13+
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
14+
"github.com/hashicorp/terraform-plugin-testing/statecheck"
15+
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
1316
"github.com/linode/linodego/v2"
1417
"github.com/linode/terraform-provider-linode/v4/linode/acceptance"
1518
acceptanceTmpl "github.com/linode/terraform-provider-linode/v4/linode/acceptance/tmpl"
@@ -126,6 +129,67 @@ func TestAccLinodeFirewall_basic(t *testing.T) {
126129
})
127130
}
128131

132+
func TestAccLinodeFirewall_protocolAllNumeric(t *testing.T) {
133+
t.Parallel()
134+
135+
name := acctest.RandomWithPrefix("tf_test")
136+
137+
resource.Test(t, resource.TestCase{
138+
PreCheck: func() { acceptance.PreCheck(t) },
139+
ProtoV6ProviderFactories: acceptance.ProtoV6ProviderFactories,
140+
Steps: []resource.TestStep{
141+
{
142+
Config: acceptanceTmpl.ProviderNoPoll(t) + tmpl.Protocol(t, name, "ALL", ""),
143+
ConfigStateChecks: []statecheck.StateCheck{
144+
statecheck.ExpectKnownValue(testFirewallResName, tfjsonpath.New("label"), knownvalue.StringExact(name)),
145+
statecheck.ExpectKnownValue(
146+
testFirewallResName,
147+
tfjsonpath.New("inbound").AtSliceIndex(0).AtMapKey("protocol"),
148+
knownvalue.StringExact("ALL"),
149+
),
150+
statecheck.ExpectKnownValue(
151+
testFirewallResName,
152+
tfjsonpath.New("inbound").AtSliceIndex(0).AtMapKey("ports"),
153+
knownvalue.Null(),
154+
),
155+
},
156+
},
157+
{
158+
Config: acceptanceTmpl.ProviderNoPoll(t) + tmpl.Protocol(t, name, "50", ""),
159+
ConfigStateChecks: []statecheck.StateCheck{
160+
statecheck.ExpectKnownValue(testFirewallResName, tfjsonpath.New("label"), knownvalue.StringExact(name)),
161+
statecheck.ExpectKnownValue(
162+
testFirewallResName,
163+
tfjsonpath.New("inbound").AtSliceIndex(0).AtMapKey("protocol"),
164+
knownvalue.StringExact("50"),
165+
),
166+
statecheck.ExpectKnownValue(
167+
testFirewallResName,
168+
tfjsonpath.New("inbound").AtSliceIndex(0).AtMapKey("ports"),
169+
knownvalue.Null(),
170+
),
171+
},
172+
},
173+
{
174+
Config: acceptanceTmpl.ProviderNoPoll(t) + tmpl.Protocol(t, name, "6", "443"),
175+
ConfigStateChecks: []statecheck.StateCheck{
176+
statecheck.ExpectKnownValue(testFirewallResName, tfjsonpath.New("label"), knownvalue.StringExact(name)),
177+
statecheck.ExpectKnownValue(
178+
testFirewallResName,
179+
tfjsonpath.New("inbound").AtSliceIndex(0).AtMapKey("protocol"),
180+
knownvalue.StringExact("6"),
181+
),
182+
statecheck.ExpectKnownValue(
183+
testFirewallResName,
184+
tfjsonpath.New("inbound").AtSliceIndex(0).AtMapKey("ports"),
185+
knownvalue.StringExact("443"),
186+
),
187+
},
188+
},
189+
},
190+
})
191+
}
192+
129193
func TestAccLinodeFirewall_minimum(t *testing.T) {
130194
t.Parallel()
131195

‎linode/firewall/framework_schema_resource.go‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
1515
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
1616
"github.com/hashicorp/terraform-plugin-framework/types"
17-
"github.com/linode/linodego/v2"
1817
"github.com/linode/terraform-provider-linode/v4/linode/helper"
1918
linodesetplanmodifiers "github.com/linode/terraform-provider-linode/v4/linode/helper/setplanmodifiers"
2019
)
@@ -38,15 +37,11 @@ var ruleNestedObject = schema.NestedBlockObject{
3837
},
3938
},
4039
"protocol": schema.StringAttribute{
41-
Description: "The network protocol this rule controls.",
42-
Required: true,
40+
Description: "The network protocol this rule controls. Accepted values are ALL, TCP, UDP, " +
41+
"ICMP, IPENCAP, or a protocol number from 0 to 255.",
42+
Required: true,
4343
Validators: []validator.String{
44-
stringvalidator.OneOf(
45-
string(linodego.TCP),
46-
string(linodego.UDP),
47-
string(linodego.ICMP),
48-
string(linodego.IPENCAP),
49-
),
44+
firewallProtocolValidator{},
5045
},
5146
},
5247
"description": schema.StringAttribute{

‎linode/firewall/tmpl/protocol.gotf‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{{ define "firewall_protocol" }}
2+
3+
resource "linode_firewall" "test" {
4+
label = "{{.Label}}"
5+
tags = ["test"]
6+
7+
inbound {
8+
label = "tf-test-in"
9+
action = "ACCEPT"
10+
protocol = "{{.Protocol}}"
11+
{{ if .Ports }}ports = "{{.Ports}}"{{ end }}
12+
ipv4 = ["0.0.0.0/0"]
13+
}
14+
inbound_policy = "DROP"
15+
outbound_policy = "DROP"
16+
}
17+
18+
{{ end }}

‎linode/firewall/tmpl/template.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ type TemplateData struct {
1919
Instances []ResourceTemplateData
2020
NodeBalancers []ResourceTemplateData
2121

22-
Label string
22+
Label string
23+
Protocol string
24+
Ports string
2325
}
2426

2527
func Basic(t testing.TB, label, devicePrefix, region string) string {
@@ -111,6 +113,15 @@ func NoRules(t testing.TB, label string) string {
111113
})
112114
}
113115

116+
func Protocol(t testing.TB, label, protocol, ports string) string {
117+
return acceptance.ExecuteTemplate(t,
118+
"firewall_protocol", TemplateData{
119+
Label: label,
120+
Protocol: protocol,
121+
Ports: ports,
122+
})
123+
}
124+
114125
func DataBasic(t testing.TB, label, devicePrefix, region string) string {
115126
resources := []ResourceTemplateData{
116127
{

‎linode/firewall/validation.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package firewall
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"strconv"
7+
8+
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
9+
)
10+
11+
var firewallProtocolKeywords = map[string]struct{}{
12+
"ALL": {},
13+
"TCP": {},
14+
"UDP": {},
15+
"ICMP": {},
16+
"IPENCAP": {},
17+
}
18+
19+
var _ validator.String = firewallProtocolValidator{}
20+
21+
type firewallProtocolValidator struct{}
22+
23+
func (v firewallProtocolValidator) Description(ctx context.Context) string {
24+
return "value must be ALL, TCP, UDP, ICMP, IPENCAP, or a protocol number from 0 to 255"
25+
}
26+
27+
func (v firewallProtocolValidator) MarkdownDescription(ctx context.Context) string {
28+
return v.Description(ctx)
29+
}
30+
31+
func (v firewallProtocolValidator) ValidateString(
32+
ctx context.Context,
33+
req validator.StringRequest,
34+
resp *validator.StringResponse,
35+
) {
36+
if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() {
37+
return
38+
}
39+
40+
protocol := req.ConfigValue.ValueString()
41+
if isValidFirewallProtocol(protocol) {
42+
return
43+
}
44+
45+
resp.Diagnostics.AddAttributeError(
46+
req.Path,
47+
"Invalid Firewall Rule Protocol",
48+
fmt.Sprintf(
49+
"Expected ALL, TCP, UDP, ICMP, IPENCAP, or a protocol number from 0 to 255 without leading zeros, got %q.",
50+
protocol,
51+
),
52+
)
53+
}
54+
55+
func isValidFirewallProtocol(protocol string) bool {
56+
if _, ok := firewallProtocolKeywords[protocol]; ok {
57+
return true
58+
}
59+
60+
value, err := strconv.Atoi(protocol)
61+
if err != nil {
62+
return false
63+
}
64+
65+
return value >= 0 && value <= 255 && strconv.Itoa(value) == protocol
66+
}

‎linode/firewall/validation_unit_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
//go:build unit
2+
3+
package firewall
4+
5+
import (
6+
"context"
7+
"encoding/json"
8+
"testing"
9+
10+
"github.com/hashicorp/terraform-plugin-framework-nettypes/cidrtypes"
11+
"github.com/hashicorp/terraform-plugin-framework/attr"
12+
"github.com/hashicorp/terraform-plugin-framework/diag"
13+
"github.com/hashicorp/terraform-plugin-framework/path"
14+
fwvalidator "github.com/hashicorp/terraform-plugin-framework/schema/validator"
15+
"github.com/hashicorp/terraform-plugin-framework/types"
16+
"github.com/linode/linodego/v2"
17+
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
19+
)
20+
21+
func TestFirewallProtocolValidator(t *testing.T) {
22+
testCases := []struct {
23+
name string
24+
protocol string
25+
wantError bool
26+
}{
27+
{name: "zero", protocol: "0"},
28+
{name: "max", protocol: "255"},
29+
{name: "all keyword", protocol: "ALL"},
30+
{name: "tcp keyword", protocol: "TCP"},
31+
{name: "too large", protocol: "256", wantError: true},
32+
{name: "leading zero", protocol: "06", wantError: true},
33+
{name: "invalid keyword", protocol: "any", wantError: true},
34+
{name: "lowercase all", protocol: "all", wantError: true},
35+
}
36+
37+
for _, tc := range testCases {
38+
t.Run(tc.name, func(t *testing.T) {
39+
var resp fwvalidator.StringResponse
40+
41+
firewallProtocolValidator{}.ValidateString(
42+
context.Background(),
43+
fwvalidator.StringRequest{
44+
Path: path.Root("protocol"),
45+
ConfigValue: types.StringValue(tc.protocol),
46+
},
47+
&resp,
48+
)
49+
50+
assert.Equal(t, tc.wantError, resp.Diagnostics.HasError())
51+
})
52+
}
53+
}
54+
55+
func TestExpandRuleOmitsNullPortsFromJSON(t *testing.T) {
56+
ruleModel := RuleModel{
57+
Label: types.StringValue("tf-test-in"),
58+
Action: types.StringValue("ACCEPT"),
59+
Protocol: types.StringValue("ALL"),
60+
Ports: types.StringNull(),
61+
Description: types.StringNull(),
62+
IPv4: types.ListValueMust(
63+
cidrtypes.IPv4PrefixType{},
64+
[]attr.Value{
65+
cidrtypes.NewIPv4PrefixValue("0.0.0.0/0"),
66+
},
67+
),
68+
IPv6: types.ListValueMust(
69+
cidrtypes.IPv6PrefixType{},
70+
[]attr.Value{},
71+
),
72+
}
73+
74+
var diags diag.Diagnostics
75+
rule := ExpandRule[linodego.FirewallRuleInbound](context.Background(), ruleModel, &diags)
76+
77+
require.False(t, diags.HasError())
78+
require.Empty(t, rule.Ports)
79+
80+
payload, err := json.Marshal(rule)
81+
require.NoError(t, err)
82+
83+
assert.Contains(t, string(payload), `"protocol":"ALL"`)
84+
assert.NotContains(t, string(payload), `"ports"`)
85+
}

‎linode/firewalls/framework_datasource_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,3 +231,40 @@ func TestAccDataSourceFirewalls_basic(t *testing.T) {
231231
})
232232
})
233233
}
234+
235+
func TestAccDataSourceFirewalls_protocolAllNumeric(t *testing.T) {
236+
t.Parallel()
237+
238+
firewallName := acctest.RandomWithPrefix("tf_test")
239+
resource.Test(t, resource.TestCase{
240+
PreCheck: func() { acceptance.PreCheck(t) },
241+
ProtoV6ProviderFactories: acceptance.ProtoV6ProviderFactories,
242+
Steps: []resource.TestStep{
243+
{
244+
Config: tmpl.DataProtocolAllNumeric(t, firewallName),
245+
ConfigStateChecks: []statecheck.StateCheck{
246+
statecheck.ExpectKnownValue(
247+
testFirewallDataName,
248+
tfjsonpath.New("firewalls"),
249+
knownvalue.ListSizeExact(1),
250+
),
251+
statecheck.ExpectKnownValue(
252+
testFirewallDataName,
253+
tfjsonpath.New("firewalls").AtSliceIndex(0).AtMapKey("label"),
254+
knownvalue.StringExact(firewallName),
255+
),
256+
statecheck.ExpectKnownValue(
257+
testFirewallDataName,
258+
tfjsonpath.New("firewalls").AtSliceIndex(0).AtMapKey("inbound").AtSliceIndex(0).AtMapKey("protocol"),
259+
knownvalue.StringExact("ALL"),
260+
),
261+
statecheck.ExpectKnownValue(
262+
testFirewallDataName,
263+
tfjsonpath.New("firewalls").AtSliceIndex(0).AtMapKey("outbound").AtSliceIndex(0).AtMapKey("protocol"),
264+
knownvalue.StringExact("50"),
265+
),
266+
},
267+
},
268+
},
269+
})
270+
}

0 commit comments

Comments
 (0)