Skip to content

Commit 4e10645

Browse files
mgajdamgajda
committed
fix: resolve compilation errors in firewall commands
- Replace server.FirewallRules with GetFirewallRules API call - Fix CreateFirewallRule to use upcloud.FirewallRule type - Replace ui.Confirm with error-based confirmation pattern - Fix PushProgress method calls to match Executor interface - Add strings import for string operations
1 parent 4368947 commit 4e10645

3 files changed

Lines changed: 64 additions & 62 deletions

File tree

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

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ package serverfirewall
33
import (
44
"fmt"
55
"sort"
6+
"strings"
67

78
"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
89
"github.com/UpCloudLtd/upcloud-cli/v3/internal/completion"
910
"github.com/UpCloudLtd/upcloud-cli/v3/internal/output"
1011
"github.com/UpCloudLtd/upcloud-cli/v3/internal/resolver"
11-
"github.com/UpCloudLtd/upcloud-cli/v3/internal/ui"
1212
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud"
1313
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud/request"
1414
"github.com/spf13/pflag"
@@ -48,33 +48,36 @@ func (s *deleteCommand) InitCommand() {
4848
// Execute implements commands.MultipleArgumentCommand
4949
func (s *deleteCommand) Execute(exec commands.Executor, arg string) (output.Output, error) {
5050
// Get current firewall rules
51-
server, err := exec.Server().GetServerDetails(exec.Context(), &request.GetServerDetailsRequest{
52-
UUID: arg,
51+
rulesResponse, err := exec.Firewall().GetFirewallRules(exec.Context(), &request.GetFirewallRulesRequest{
52+
ServerUUID: arg,
5353
})
5454
if err != nil {
5555
return nil, err
5656
}
5757

5858
// Find matching rules
59-
matchedIndices := findMatchingRules(server.FirewallRules, &s.params)
59+
matchedIndices := findMatchingRules(rulesResponse.FirewallRules, &s.params)
6060

6161
if len(matchedIndices) == 0 {
6262
return nil, fmt.Errorf("no firewall rules matched the specified filters")
6363
}
6464

6565
// Confirm if multiple rules or if confirmation required
6666
if len(matchedIndices) > s.params.skipConfirmation {
67-
exec.PushProgressUpdate(fmt.Sprintf("Found %d matching firewall rules:", len(matchedIndices)))
67+
var ruleDescriptions []string
6868
for _, idx := range matchedIndices {
69-
rule := &server.FirewallRules[idx]
70-
exec.PushProgressUpdate(fmt.Sprintf(" Position %d: %s %s %s -> %s",
69+
rule := &rulesResponse.FirewallRules[idx]
70+
desc := fmt.Sprintf(" Position %d: %s %s %s -> %s",
7171
rule.Position, rule.Direction, rule.Protocol,
72-
formatRuleAddress(rule, true), formatRuleAddress(rule, false)))
72+
formatRuleAddress(rule, true), formatRuleAddress(rule, false))
73+
if rule.Comment != "" {
74+
desc += fmt.Sprintf(" (comment: %q)", rule.Comment)
75+
}
76+
ruleDescriptions = append(ruleDescriptions, desc)
7377
}
7478

75-
if !ui.Confirm(fmt.Sprintf("Delete %d firewall rules?", len(matchedIndices))) {
76-
return output.None{}, nil
77-
}
79+
return nil, fmt.Errorf("would delete %d firewall rules (exceeds skip-confirmation=%d). Matching rules:\n%s\n\nIncrease --skip-confirmation to proceed",
80+
len(matchedIndices), s.params.skipConfirmation, strings.Join(ruleDescriptions, "\n"))
7881
}
7982

8083
// Sort indices in descending order to delete from highest position first
@@ -84,7 +87,7 @@ func (s *deleteCommand) Execute(exec commands.Executor, arg string) (output.Outp
8487
// Delete each matched rule
8588
deletedCount := 0
8689
for _, idx := range matchedIndices {
87-
rule := &server.FirewallRules[idx]
90+
rule := &rulesResponse.FirewallRules[idx]
8891
msg := fmt.Sprintf("Deleting firewall rule at position %d", rule.Position)
8992
exec.PushProgressStarted(msg)
9093

@@ -93,20 +96,19 @@ func (s *deleteCommand) Execute(exec commands.Executor, arg string) (output.Outp
9396
Position: rule.Position,
9497
})
9598
if err != nil {
96-
exec.PushProgressFailed(msg)
9799
if deletedCount > 0 {
98-
exec.PushProgressUpdate(fmt.Sprintf("Successfully deleted %d rules before error", deletedCount))
100+
msg = fmt.Sprintf("Successfully deleted %d rules before error: %v", deletedCount, err)
99101
}
100-
return nil, err
102+
return commands.HandleError(exec, msg, err)
101103
}
102104

103105
exec.PushProgressSuccess(msg)
104106
deletedCount++
105107

106108
// Adjust positions of remaining rules in our local copy
107-
for i := range server.FirewallRules {
108-
if server.FirewallRules[i].Position > rule.Position {
109-
server.FirewallRules[i].Position--
109+
for i := range rulesResponse.FirewallRules {
110+
if rulesResponse.FirewallRules[i].Position > rule.Position {
111+
rulesResponse.FirewallRules[i].Position--
110112
}
111113
}
112114
}

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

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ package serverfirewall
33
import (
44
"fmt"
55
"sort"
6+
"strings"
67

78
"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
89
"github.com/UpCloudLtd/upcloud-cli/v3/internal/completion"
910
"github.com/UpCloudLtd/upcloud-cli/v3/internal/output"
1011
"github.com/UpCloudLtd/upcloud-cli/v3/internal/resolver"
11-
"github.com/UpCloudLtd/upcloud-cli/v3/internal/ui"
1212
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud"
1313
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud/request"
1414
"github.com/spf13/pflag"
@@ -48,24 +48,24 @@ func (s *ruleDisableCommand) InitCommand() {
4848
// Execute implements commands.MultipleArgumentCommand
4949
func (s *ruleDisableCommand) Execute(exec commands.Executor, arg string) (output.Output, error) {
5050
// Get current firewall rules
51-
server, err := exec.Server().GetServerDetails(exec.Context(), &request.GetServerDetailsRequest{
52-
UUID: arg,
51+
rulesResponse, err := exec.Firewall().GetFirewallRules(exec.Context(), &request.GetFirewallRulesRequest{
52+
ServerUUID: arg,
5353
})
5454
if err != nil {
5555
return nil, err
5656
}
5757

5858
// Find the catch-all drop rule position
59-
catchAllPosition := findCatchAllDropRule(server.FirewallRules)
59+
catchAllPosition := findCatchAllDropRule(rulesResponse.FirewallRules)
6060
if catchAllPosition == 0 {
6161
return nil, fmt.Errorf("no catch-all drop rule found in firewall rules")
6262
}
6363

6464
// Find matching rules that are currently before the catch-all
65-
matchedIndices := findMatchingRules(server.FirewallRules, &s.params)
65+
matchedIndices := findMatchingRules(rulesResponse.FirewallRules, &s.params)
6666
var rulesToMove []int
6767
for _, idx := range matchedIndices {
68-
if server.FirewallRules[idx].Position < catchAllPosition {
68+
if rulesResponse.FirewallRules[idx].Position < catchAllPosition {
6969
rulesToMove = append(rulesToMove, idx)
7070
}
7171
}
@@ -76,27 +76,29 @@ func (s *ruleDisableCommand) Execute(exec commands.Executor, arg string) (output
7676

7777
// Confirm if multiple rules or if confirmation required
7878
if len(rulesToMove) > s.params.skipConfirmation {
79-
exec.PushProgressUpdate(fmt.Sprintf("Found %d enabled firewall rules to disable:", len(rulesToMove)))
79+
var ruleDescriptions []string
8080
for _, idx := range rulesToMove {
81-
rule := &server.FirewallRules[idx]
82-
exec.PushProgressUpdate(fmt.Sprintf(" Position %d: %s %s %s",
83-
rule.Position, rule.Direction, rule.Protocol, rule.Comment))
81+
rule := &rulesResponse.FirewallRules[idx]
82+
desc := fmt.Sprintf(" Position %d: %s %s", rule.Position, rule.Direction, rule.Protocol)
83+
if rule.Comment != "" {
84+
desc += fmt.Sprintf(" (comment: %q)", rule.Comment)
85+
}
86+
ruleDescriptions = append(ruleDescriptions, desc)
8487
}
8588

86-
if !ui.Confirm(fmt.Sprintf("Disable %d firewall rules?", len(rulesToMove))) {
87-
return output.None{}, nil
88-
}
89+
return nil, fmt.Errorf("would disable %d firewall rules (exceeds skip-confirmation=%d). Matching rules:\n%s\n\nIncrease --skip-confirmation to proceed",
90+
len(rulesToMove), s.params.skipConfirmation, strings.Join(ruleDescriptions, "\n"))
8991
}
9092

9193
// Sort by position (descending) to move rules from highest position first
9294
sort.Slice(rulesToMove, func(i, j int) bool {
93-
return server.FirewallRules[rulesToMove[i]].Position > server.FirewallRules[rulesToMove[j]].Position
95+
return rulesResponse.FirewallRules[rulesToMove[i]].Position > rulesResponse.FirewallRules[rulesToMove[j]].Position
9496
})
9597

9698
// Move each rule to after the catch-all
9799
movedCount := 0
98100
for _, idx := range rulesToMove {
99-
rule := &server.FirewallRules[idx]
101+
rule := &rulesResponse.FirewallRules[idx]
100102
oldPosition := rule.Position
101103
// Move to just after catch-all
102104
newPosition := catchAllPosition + 1
@@ -114,17 +116,16 @@ func (s *ruleDisableCommand) Execute(exec commands.Executor, arg string) (output
114116
Position: oldPosition,
115117
})
116118
if err != nil {
117-
exec.PushProgressFailed(msg)
118119
if movedCount > 0 {
119-
exec.PushProgressUpdate(fmt.Sprintf("Successfully disabled %d rules before error", movedCount))
120+
msg = fmt.Sprintf("Successfully disabled %d rules before error: %v", movedCount, err)
120121
}
121-
return nil, err
122+
return commands.HandleError(exec, msg, err)
122123
}
123124

124125
// Create the rule at the new position (after catch-all)
125126
_, err = exec.Firewall().CreateFirewallRule(exec.Context(), &request.CreateFirewallRuleRequest{
126-
ServerUUID: arg,
127-
FirewallRule: request.FirewallRule{
127+
ServerUUID: arg,
128+
FirewallRule: upcloud.FirewallRule{
128129
Direction: newRule.Direction,
129130
Action: newRule.Action,
130131
Family: newRule.Family,
@@ -139,11 +140,10 @@ func (s *ruleDisableCommand) Execute(exec commands.Executor, arg string) (output
139140
SourcePortStart: newRule.SourcePortStart,
140141
SourcePortEnd: newRule.SourcePortEnd,
141142
Comment: newRule.Comment,
143+
Position: newPosition,
142144
},
143-
Position: newPosition,
144145
})
145146
if err != nil {
146-
exec.PushProgressFailed(msg)
147147
return nil, fmt.Errorf("failed to recreate rule at position %d: %w", newPosition, err)
148148
}
149149

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

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ package serverfirewall
33
import (
44
"fmt"
55
"sort"
6+
"strings"
67

78
"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
89
"github.com/UpCloudLtd/upcloud-cli/v3/internal/completion"
910
"github.com/UpCloudLtd/upcloud-cli/v3/internal/output"
1011
"github.com/UpCloudLtd/upcloud-cli/v3/internal/resolver"
11-
"github.com/UpCloudLtd/upcloud-cli/v3/internal/ui"
1212
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud"
1313
"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud/request"
1414
"github.com/spf13/pflag"
@@ -48,24 +48,24 @@ func (s *ruleEnableCommand) InitCommand() {
4848
// Execute implements commands.MultipleArgumentCommand
4949
func (s *ruleEnableCommand) Execute(exec commands.Executor, arg string) (output.Output, error) {
5050
// Get current firewall rules
51-
server, err := exec.Server().GetServerDetails(exec.Context(), &request.GetServerDetailsRequest{
52-
UUID: arg,
51+
rulesResponse, err := exec.Firewall().GetFirewallRules(exec.Context(), &request.GetFirewallRulesRequest{
52+
ServerUUID: arg,
5353
})
5454
if err != nil {
5555
return nil, err
5656
}
5757

5858
// Find the catch-all drop rule position
59-
catchAllPosition := findCatchAllDropRule(server.FirewallRules)
59+
catchAllPosition := findCatchAllDropRule(rulesResponse.FirewallRules)
6060
if catchAllPosition == 0 {
6161
return nil, fmt.Errorf("no catch-all drop rule found in firewall rules")
6262
}
6363

6464
// Find matching rules that are currently after the catch-all
65-
matchedIndices := findMatchingRules(server.FirewallRules, &s.params)
65+
matchedIndices := findMatchingRules(rulesResponse.FirewallRules, &s.params)
6666
var rulesToMove []int
6767
for _, idx := range matchedIndices {
68-
if server.FirewallRules[idx].Position > catchAllPosition {
68+
if rulesResponse.FirewallRules[idx].Position > catchAllPosition {
6969
rulesToMove = append(rulesToMove, idx)
7070
}
7171
}
@@ -76,27 +76,29 @@ func (s *ruleEnableCommand) Execute(exec commands.Executor, arg string) (output.
7676

7777
// Confirm if multiple rules or if confirmation required
7878
if len(rulesToMove) > s.params.skipConfirmation {
79-
exec.PushProgressUpdate(fmt.Sprintf("Found %d disabled firewall rules to enable:", len(rulesToMove)))
79+
var ruleDescriptions []string
8080
for _, idx := range rulesToMove {
81-
rule := &server.FirewallRules[idx]
82-
exec.PushProgressUpdate(fmt.Sprintf(" Position %d: %s %s %s",
83-
rule.Position, rule.Direction, rule.Protocol, rule.Comment))
81+
rule := &rulesResponse.FirewallRules[idx]
82+
desc := fmt.Sprintf(" Position %d: %s %s", rule.Position, rule.Direction, rule.Protocol)
83+
if rule.Comment != "" {
84+
desc += fmt.Sprintf(" (comment: %q)", rule.Comment)
85+
}
86+
ruleDescriptions = append(ruleDescriptions, desc)
8487
}
8588

86-
if !ui.Confirm(fmt.Sprintf("Enable %d firewall rules?", len(rulesToMove))) {
87-
return output.None{}, nil
88-
}
89+
return nil, fmt.Errorf("would enable %d firewall rules (exceeds skip-confirmation=%d). Matching rules:\n%s\n\nIncrease --skip-confirmation to proceed",
90+
len(rulesToMove), s.params.skipConfirmation, strings.Join(ruleDescriptions, "\n"))
8991
}
9092

9193
// Sort by position (ascending) to move rules in order
9294
sort.Slice(rulesToMove, func(i, j int) bool {
93-
return server.FirewallRules[rulesToMove[i]].Position < server.FirewallRules[rulesToMove[j]].Position
95+
return rulesResponse.FirewallRules[rulesToMove[i]].Position < rulesResponse.FirewallRules[rulesToMove[j]].Position
9496
})
9597

9698
// Move each rule to just before the catch-all
9799
movedCount := 0
98100
for _, idx := range rulesToMove {
99-
rule := &server.FirewallRules[idx]
101+
rule := &rulesResponse.FirewallRules[idx]
100102
oldPosition := rule.Position
101103
// Move to just before catch-all (which may have shifted)
102104
newPosition := catchAllPosition
@@ -118,17 +120,16 @@ func (s *ruleEnableCommand) Execute(exec commands.Executor, arg string) (output.
118120
Position: oldPosition,
119121
})
120122
if err != nil {
121-
exec.PushProgressFailed(msg)
122123
if movedCount > 0 {
123-
exec.PushProgressUpdate(fmt.Sprintf("Successfully enabled %d rules before error", movedCount))
124+
msg = fmt.Sprintf("Successfully enabled %d rules before error: %v", movedCount, err)
124125
}
125-
return nil, err
126+
return commands.HandleError(exec, msg, err)
126127
}
127128

128129
// Create the rule at the new position
129130
_, err = exec.Firewall().CreateFirewallRule(exec.Context(), &request.CreateFirewallRuleRequest{
130-
ServerUUID: arg,
131-
FirewallRule: request.FirewallRule{
131+
ServerUUID: arg,
132+
FirewallRule: upcloud.FirewallRule{
132133
Direction: newRule.Direction,
133134
Action: newRule.Action,
134135
Family: newRule.Family,
@@ -143,11 +144,10 @@ func (s *ruleEnableCommand) Execute(exec commands.Executor, arg string) (output.
143144
SourcePortStart: newRule.SourcePortStart,
144145
SourcePortEnd: newRule.SourcePortEnd,
145146
Comment: newRule.Comment,
147+
Position: newPosition,
146148
},
147-
Position: newPosition,
148149
})
149150
if err != nil {
150-
exec.PushProgressFailed(msg)
151151
return nil, fmt.Errorf("failed to recreate rule at position %d: %w", newPosition, err)
152152
}
153153

0 commit comments

Comments
 (0)