Skip to content

Commit 08f34be

Browse files
mgajdamgajda
committed
feat(server): add firewall rule enable/disable commands
Implements #244 - Enable/disable specific firewall rules via CLI UpCloud API does not support modifying individual firewall rules directly. Instead, this implementation: 1. Fetches all firewall rules for the server 2. Modifies the target rule's action field (accept/drop) 3. Replaces the entire ruleset atomically using CreateFirewallRules This pattern follows the Terraform provider implementation and is the only viable approach with current UpCloud API design. Changes: - Add `upctl server firewall rule enable <uuid> --position <N>` command - Add `upctl server firewall rule disable <uuid> --position <N>` command - Comprehensive tests for both commands - Input validation for position (1-1000) - Clear error messages when rule position not found Usage Examples: # Create firewall rules first upctl server firewall create my-server --direction in --action drop --comment "Block SSH" --protocol tcp --destination-port-start 22 --destination-port-end 22 upctl server firewall create my-server --direction in --action accept --comment "Allow HTTP" --protocol tcp --destination-port-start 80 --destination-port-end 80 upctl server firewall create my-server --direction in --action accept --comment "Allow HTTPS" --protocol tcp --destination-port-start 443 --destination-port-end 443 # View current rules to get positions upctl server firewall show my-server # Enable a specific rule (sets action to "accept") upctl server firewall rule enable my-server --position 1 # Disable a specific rule (sets action to "drop") upctl server firewall rule disable my-server --position 3 Note: Rules are identified by position (1-1000). Use the comment field when creating rules to help identify them later.
1 parent a2ac26d commit 08f34be

6 files changed

Lines changed: 474 additions & 0 deletions

File tree

‎internal/commands/base/base.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ func BuildCommands(rootCmd *cobra.Command, conf *config.Config) {
8181
commands.BuildCommand(serverfirewall.DeleteCommand(), serverFirewallCommand.Cobra(), conf)
8282
commands.BuildCommand(serverfirewall.ShowCommand(), serverFirewallCommand.Cobra(), conf)
8383

84+
// Server firewall rule operations
85+
serverFirewallRuleCommand := commands.BuildCommand(serverfirewall.BaseServerFirewallRuleCommand(), serverFirewallCommand.Cobra(), conf)
86+
commands.BuildCommand(serverfirewall.RuleEnableCommand(), serverFirewallRuleCommand.Cobra(), conf)
87+
commands.BuildCommand(serverfirewall.RuleDisableCommand(), serverFirewallRuleCommand.Cobra(), conf)
88+
8489
// Storages
8590
storageCommand := commands.BuildCommand(storage.BaseStorageCommand(), rootCmd, conf)
8691
commands.BuildCommand(storage.ListCommand(), storageCommand.Cobra(), conf)

‎internal/commands/server/firewall/rule.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package serverfirewall
2+
3+
import (
4+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
5+
)
6+
7+
// BaseServerFirewallRuleCommand is the root command for all 'server firewall rule' commands
8+
func BaseServerFirewallRuleCommand() commands.Command {
9+
return &serverFirewallRuleCommand{
10+
commands.New(
11+
"rule",
12+
"Manage individual firewall rules. Enable or disable specific rules by position.",
13+
),
14+
}
15+
}
16+
17+
type serverFirewallRuleCommand struct {
18+
*commands.BaseCommand
19+
}

‎internal/commands/server/firewall/rule_disable.go‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package serverfirewall
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
7+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/completion"
8+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/output"
9+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/resolver"
10+
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud"
11+
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud/request"
12+
"github.com/spf13/cobra"
13+
"github.com/spf13/pflag"
14+
)
15+
16+
type ruleDisableCommand struct {
17+
*commands.BaseCommand
18+
rulePosition int
19+
completion.Server
20+
resolver.CachingServer
21+
}
22+
23+
// RuleDisableCommand creates the "server firewall rule disable" command
24+
func RuleDisableCommand() commands.Command {
25+
return &ruleDisableCommand{
26+
BaseCommand: commands.New(
27+
"disable",
28+
"Disable a specific firewall rule by changing its action to drop",
29+
"upctl server firewall rule disable 00038afc-d526-4148-af0e-d2f1eeaded9b --position 5",
30+
),
31+
}
32+
}
33+
34+
// InitCommand implements Command.InitCommand
35+
func (s *ruleDisableCommand) InitCommand() {
36+
flagSet := &pflag.FlagSet{}
37+
flagSet.IntVar(&s.rulePosition, "position", 0, "Rule position. Available: 1-1000")
38+
s.AddFlags(flagSet)
39+
40+
commands.Must(s.Cobra().MarkFlagRequired("position"))
41+
commands.Must(s.Cobra().RegisterFlagCompletionFunc("position", cobra.NoFileCompletions))
42+
}
43+
44+
// Execute implements commands.MultipleArgumentCommand
45+
func (s *ruleDisableCommand) Execute(exec commands.Executor, arg string) (output.Output, error) {
46+
if s.rulePosition < 1 || s.rulePosition > 1000 {
47+
return nil, fmt.Errorf("invalid position (1-1000 allowed)")
48+
}
49+
50+
msg := fmt.Sprintf("Disabling firewall rule %d on server %v", s.rulePosition, arg)
51+
exec.PushProgressStarted(msg)
52+
53+
// Fetch current firewall rules
54+
currentRules, err := exec.Firewall().GetFirewallRules(exec.Context(), &request.GetFirewallRulesRequest{
55+
ServerUUID: arg,
56+
})
57+
if err != nil {
58+
return commands.HandleError(exec, msg, err)
59+
}
60+
61+
// Find and modify the target rule
62+
ruleFound := false
63+
for i := range currentRules.FirewallRules {
64+
if currentRules.FirewallRules[i].Position == s.rulePosition {
65+
currentRules.FirewallRules[i].Action = upcloud.FirewallRuleActionDrop
66+
ruleFound = true
67+
break
68+
}
69+
}
70+
71+
if !ruleFound {
72+
return nil, fmt.Errorf("firewall rule at position %d not found on server %s", s.rulePosition, arg)
73+
}
74+
75+
// Replace entire ruleset atomically
76+
err = exec.Firewall().CreateFirewallRules(exec.Context(), &request.CreateFirewallRulesRequest{
77+
ServerUUID: arg,
78+
FirewallRules: currentRules.FirewallRules,
79+
})
80+
if err != nil {
81+
return commands.HandleError(exec, msg, err)
82+
}
83+
84+
exec.PushProgressSuccess(msg)
85+
86+
return output.None{}, nil
87+
}

‎internal/commands/server/firewall/rule_disable_test.go‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
package serverfirewall
2+
3+
import (
4+
"testing"
5+
6+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
7+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/config"
8+
smock "github.com/UpCloudLtd/upcloud-cli/v3/internal/mock"
9+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/mockexecute"
10+
11+
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud"
12+
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud/request"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/mock"
15+
)
16+
17+
func TestRuleDisableCommand(t *testing.T) {
18+
serverUUID := "1fdfda29-ead1-4855-b71f-1e33eb2ca9de"
19+
20+
currentRules := &upcloud.FirewallRules{
21+
FirewallRules: []upcloud.FirewallRule{
22+
{
23+
Position: 1,
24+
Direction: "in",
25+
Action: "accept",
26+
Protocol: "tcp",
27+
},
28+
{
29+
Position: 2,
30+
Direction: "in",
31+
Action: "accept",
32+
Protocol: "tcp",
33+
},
34+
{
35+
Position: 5,
36+
Direction: "out",
37+
Action: "accept",
38+
Protocol: "udp",
39+
},
40+
},
41+
}
42+
43+
for _, test := range []struct {
44+
name string
45+
flags []string
46+
arg string
47+
expectedErr string
48+
checkMocks func(*testing.T, *smock.Service)
49+
}{
50+
{
51+
name: "Missing position flag",
52+
flags: []string{},
53+
arg: serverUUID,
54+
expectedErr: `required flag(s) "position" not set`,
55+
},
56+
{
57+
name: "Invalid position - too low",
58+
flags: []string{"--position", "0"},
59+
arg: serverUUID,
60+
expectedErr: "invalid position (1-1000 allowed)",
61+
},
62+
{
63+
name: "Invalid position - too high",
64+
flags: []string{"--position", "1001"},
65+
arg: serverUUID,
66+
expectedErr: "invalid position (1-1000 allowed)",
67+
},
68+
{
69+
name: "Successfully disable rule at position 2",
70+
flags: []string{"--position", "2"},
71+
arg: serverUUID,
72+
checkMocks: func(t *testing.T, mService *smock.Service) {
73+
mService.AssertCalled(t, "GetFirewallRules", &request.GetFirewallRulesRequest{
74+
ServerUUID: serverUUID,
75+
})
76+
77+
mService.AssertCalled(t, "CreateFirewallRules", mock.MatchedBy(func(req interface{}) bool {
78+
r, ok := req.(*request.CreateFirewallRulesRequest)
79+
if !ok {
80+
return false
81+
}
82+
if r.ServerUUID != serverUUID {
83+
return false
84+
}
85+
for _, rule := range r.FirewallRules {
86+
if rule.Position == 2 {
87+
return rule.Action == upcloud.FirewallRuleActionDrop
88+
}
89+
}
90+
return false
91+
}))
92+
},
93+
},
94+
{
95+
name: "Successfully disable rule at position 1",
96+
flags: []string{"--position", "1"},
97+
arg: serverUUID,
98+
checkMocks: func(t *testing.T, mService *smock.Service) {
99+
mService.AssertCalled(t, "CreateFirewallRules", mock.MatchedBy(func(req interface{}) bool {
100+
r, ok := req.(*request.CreateFirewallRulesRequest)
101+
if !ok {
102+
return false
103+
}
104+
if r.ServerUUID != serverUUID {
105+
return false
106+
}
107+
for _, rule := range r.FirewallRules {
108+
if rule.Position == 1 {
109+
return rule.Action == upcloud.FirewallRuleActionDrop
110+
}
111+
}
112+
return false
113+
}))
114+
},
115+
},
116+
{
117+
name: "Rule position not found",
118+
flags: []string{"--position", "99"},
119+
arg: serverUUID,
120+
expectedErr: "firewall rule at position 99 not found on server",
121+
},
122+
} {
123+
t.Run(test.name, func(t *testing.T) {
124+
mService := smock.Service{}
125+
126+
mService.On("GetFirewallRules", mock.MatchedBy(func(req interface{}) bool {
127+
r, ok := req.(*request.GetFirewallRulesRequest)
128+
return ok && r.ServerUUID == serverUUID
129+
})).Return(currentRules, nil)
130+
131+
mService.On("CreateFirewallRules", mock.Anything).Return(nil)
132+
133+
conf := config.New()
134+
cc := commands.BuildCommand(RuleDisableCommand(), nil, conf)
135+
136+
cc.Cobra().SetArgs(append(test.flags, test.arg))
137+
_, err := mockexecute.MockExecute(cc, &mService, conf)
138+
139+
if test.expectedErr != "" {
140+
assert.ErrorContains(t, err, test.expectedErr)
141+
} else {
142+
assert.NoError(t, err)
143+
if test.checkMocks != nil {
144+
test.checkMocks(t, &mService)
145+
}
146+
}
147+
})
148+
}
149+
}

‎internal/commands/server/firewall/rule_enable.go‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package serverfirewall
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
7+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/completion"
8+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/output"
9+
"github.com/UpCloudLtd/upcloud-cli/v3/internal/resolver"
10+
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud"
11+
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud/request"
12+
"github.com/spf13/cobra"
13+
"github.com/spf13/pflag"
14+
)
15+
16+
type ruleEnableCommand struct {
17+
*commands.BaseCommand
18+
rulePosition int
19+
completion.Server
20+
resolver.CachingServer
21+
}
22+
23+
// RuleEnableCommand creates the "server firewall rule enable" command
24+
func RuleEnableCommand() commands.Command {
25+
return &ruleEnableCommand{
26+
BaseCommand: commands.New(
27+
"enable",
28+
"Enable a specific firewall rule by changing its action to accept",
29+
"upctl server firewall rule enable 00038afc-d526-4148-af0e-d2f1eeaded9b --position 5",
30+
),
31+
}
32+
}
33+
34+
// InitCommand implements Command.InitCommand
35+
func (s *ruleEnableCommand) InitCommand() {
36+
flagSet := &pflag.FlagSet{}
37+
flagSet.IntVar(&s.rulePosition, "position", 0, "Rule position. Available: 1-1000")
38+
s.AddFlags(flagSet)
39+
40+
commands.Must(s.Cobra().MarkFlagRequired("position"))
41+
commands.Must(s.Cobra().RegisterFlagCompletionFunc("position", cobra.NoFileCompletions))
42+
}
43+
44+
// Execute implements commands.MultipleArgumentCommand
45+
func (s *ruleEnableCommand) Execute(exec commands.Executor, arg string) (output.Output, error) {
46+
if s.rulePosition < 1 || s.rulePosition > 1000 {
47+
return nil, fmt.Errorf("invalid position (1-1000 allowed)")
48+
}
49+
50+
msg := fmt.Sprintf("Enabling firewall rule %d on server %v", s.rulePosition, arg)
51+
exec.PushProgressStarted(msg)
52+
53+
// Fetch current firewall rules
54+
currentRules, err := exec.Firewall().GetFirewallRules(exec.Context(), &request.GetFirewallRulesRequest{
55+
ServerUUID: arg,
56+
})
57+
if err != nil {
58+
return commands.HandleError(exec, msg, err)
59+
}
60+
61+
// Find and modify the target rule
62+
ruleFound := false
63+
for i := range currentRules.FirewallRules {
64+
if currentRules.FirewallRules[i].Position == s.rulePosition {
65+
currentRules.FirewallRules[i].Action = upcloud.FirewallRuleActionAccept
66+
ruleFound = true
67+
break
68+
}
69+
}
70+
71+
if !ruleFound {
72+
return nil, fmt.Errorf("firewall rule at position %d not found on server %s", s.rulePosition, arg)
73+
}
74+
75+
// Replace entire ruleset atomically
76+
err = exec.Firewall().CreateFirewallRules(exec.Context(), &request.CreateFirewallRulesRequest{
77+
ServerUUID: arg,
78+
FirewallRules: currentRules.FirewallRules,
79+
})
80+
if err != nil {
81+
return commands.HandleError(exec, msg, err)
82+
}
83+
84+
exec.PushProgressSuccess(msg)
85+
86+
return output.None{}, nil
87+
}

0 commit comments

Comments
 (0)