feat(server): add flexible filtering to firewall rule enable/disable commands

Extends firewall rule enable/disable commands with multiple filter options
beyond just position-based selection, addressing feedback on issue #244.

The same filter options used when creating firewall rules (--direction,
--protocol, --dest-port, --src-address, --comment) can now be used to select
which existing rules to enable or disable.

Filter options (can be combined):
- --comment: Match rules by comment text (partial, case-insensitive)
- --direction: Filter by direction (in/out)
- --protocol: Filter by protocol (tcp/udp/icmp)
- --dest-port: Match destination port
- --src-address: Match source IP address (partial)
- --position: Match exact position (mutually exclusive with others)

Multiple filters use AND logic to narrow results. For example:
  upctl server firewall rule enable <uuid> --comment "Dev" --direction in --protocol tcp

Safety features:
- --skip-confirmation N: Set max rules to modify without confirmation
- Default: confirms if modifying >1 rule
- Lists all matching rules before requiring confirmation

Examples prioritize comment-based selection over position, as position-based
selection is fragile when rules are reordered. Comments provide stable,
human-readable rule identification.

Resolves: #244

Author: mgajda

internal/commands/server/firewall/rule_disable.go

@@ -2,6 +2,7 @@ package serverfirewall
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
 	"github.com/UpCloudLtd/upcloud-cli/v3/internal/completion"
@@ -15,7 +16,13 @@ import (
 
 type ruleDisableCommand struct {
 	*commands.BaseCommand
-	rulePosition int
+	rulePosition      int
+	ruleComment       string
+	ruleDirection     string
+	ruleProtocol      string
+	ruleDestPort      string
+	ruleSrcAddress    string
+	skipConfirmation  int
 	completion.Server
 	resolver.CachingServer
 }
@@ -25,29 +32,58 @@ func RuleDisableCommand() commands.Command {
 	return &ruleDisableCommand{
 		BaseCommand: commands.New(
 			"disable",
-			"Disable a specific firewall rule by changing its action to drop",
+			"Disable firewall rules by changing their action to drop",
+			"upctl server firewall rule disable 00038afc-d526-4148-af0e-d2f1eeaded9b --comment \"Dev ports\"",
+			"upctl server firewall rule disable 00038afc-d526-4148-af0e-d2f1eeaded9b --direction in --protocol tcp --dest-port 8080",
+			"upctl server firewall rule disable 00038afc-d526-4148-af0e-d2f1eeaded9b --comment \"Test\" --direction in --skip-confirmation 10",
 			"upctl server firewall rule disable 00038afc-d526-4148-af0e-d2f1eeaded9b --position 5",
 		),
+		skipConfirmation: 1,
 	}
 }
 
 // InitCommand implements Command.InitCommand
 func (s *ruleDisableCommand) InitCommand() {
+	directions := []string{upcloud.FirewallRuleDirectionIn, upcloud.FirewallRuleDirectionOut}
+	protocols := []string{upcloud.FirewallRuleProtocolTCP, upcloud.FirewallRuleProtocolUDP, upcloud.FirewallRuleProtocolICMP}
+
 	flagSet := &pflag.FlagSet{}
 	flagSet.IntVar(&s.rulePosition, "position", 0, "Rule position. Available: 1-1000")
+	flagSet.StringVar(&s.ruleComment, "comment", "", "Filter by comment (partial match, case-insensitive)")
+	flagSet.StringVar(&s.ruleDirection, "direction", "", "Filter by direction. Available: "+strings.Join(directions, ", "))
+	flagSet.StringVar(&s.ruleProtocol, "protocol", "", "Filter by protocol. Available: "+strings.Join(protocols, ", "))
+	flagSet.StringVar(&s.ruleDestPort, "dest-port", "", "Filter by destination port (matches both start and end)")
+	flagSet.StringVar(&s.ruleSrcAddress, "src-address", "", "Filter by source address (partial match)")
+	flagSet.IntVar(&s.skipConfirmation, "skip-confirmation", 1, "Maximum rules to modify without confirmation (0 = always confirm)")
 	s.AddFlags(flagSet)
 
-	commands.Must(s.Cobra().MarkFlagRequired("position"))
+	s.Cobra().MarkFlagsMutuallyExclusive("position", "comment")
+	s.Cobra().MarkFlagsMutuallyExclusive("position", "direction")
+	s.Cobra().MarkFlagsMutuallyExclusive("position", "protocol")
+	s.Cobra().MarkFlagsMutuallyExclusive("position", "dest-port")
+	s.Cobra().MarkFlagsMutuallyExclusive("position", "src-address")
 	commands.Must(s.Cobra().RegisterFlagCompletionFunc("position", cobra.NoFileCompletions))
+	commands.Must(s.Cobra().RegisterFlagCompletionFunc("comment", cobra.NoFileCompletions))
+	commands.Must(s.Cobra().RegisterFlagCompletionFunc("direction", cobra.FixedCompletions(directions, cobra.ShellCompDirectiveNoFileComp)))
+	commands.Must(s.Cobra().RegisterFlagCompletionFunc("protocol", cobra.FixedCompletions(protocols, cobra.ShellCompDirectiveNoFileComp)))
+	commands.Must(s.Cobra().RegisterFlagCompletionFunc("dest-port", cobra.NoFileCompletions))
+	commands.Must(s.Cobra().RegisterFlagCompletionFunc("src-address", cobra.NoFileCompletions))
+	commands.Must(s.Cobra().RegisterFlagCompletionFunc("skip-confirmation", cobra.NoFileCompletions))
 }
 
 // Execute implements commands.MultipleArgumentCommand
 func (s *ruleDisableCommand) Execute(exec commands.Executor, arg string) (output.Output, error) {
-	if s.rulePosition < 1 || s.rulePosition > 1000 {
+	// Validation
+	hasFilters := s.rulePosition != 0 || s.ruleComment != "" || s.ruleDirection != "" ||
+		s.ruleProtocol != "" || s.ruleDestPort != "" || s.ruleSrcAddress != ""
+	if !hasFilters {
+		return nil, fmt.Errorf("at least one filter must be specified (--comment, --direction, --protocol, --dest-port, --src-address, or --position)")
+	}
+	if s.rulePosition != 0 && (s.rulePosition < 1 || s.rulePosition > 1000) {
 		return nil, fmt.Errorf("invalid position (1-1000 allowed)")
 	}
 
-	msg := fmt.Sprintf("Disabling firewall rule %d on server %v", s.rulePosition, arg)
+	msg := fmt.Sprintf("Disabling firewall rules on server %v", arg)
 	exec.PushProgressStarted(msg)
 
 	// Fetch current firewall rules
@@ -58,18 +94,95 @@ func (s *ruleDisableCommand) Execute(exec commands.Executor, arg string) (output
 		return commands.HandleError(exec, msg, err)
 	}
 
-	// Find and modify the target rule
-	ruleFound := false
+	// Find matching rules
+	var matchedIndices []int
 	for i := range currentRules.FirewallRules {
-		if currentRules.FirewallRules[i].Position == s.rulePosition {
-			currentRules.FirewallRules[i].Action = upcloud.FirewallRuleActionDrop
-			ruleFound = true
-			break
+		rule := &currentRules.FirewallRules[i]
+
+		// Position-based filter (exact match, exclusive)
+		if s.rulePosition != 0 {
+			if rule.Position == s.rulePosition {
+				matchedIndices = append(matchedIndices, i)
+			}
+			continue
+		}
+
+		// Apply all specified filters (AND logic)
+		match := true
+
+		if s.ruleComment != "" {
+			if !strings.Contains(strings.ToLower(rule.Comment), strings.ToLower(s.ruleComment)) {
+				match = false
+			}
+		}
+
+		if s.ruleDirection != "" {
+			if !strings.EqualFold(rule.Direction, s.ruleDirection) {
+				match = false
+			}
+		}
+
+		if s.ruleProtocol != "" {
+			if !strings.EqualFold(rule.Protocol, s.ruleProtocol) {
+				match = false
+			}
+		}
+
+		if s.ruleDestPort != "" {
+			// Match if either start or end matches the specified port
+			if rule.DestinationPortStart != s.ruleDestPort && rule.DestinationPortEnd != s.ruleDestPort {
+				match = false
+			}
+		}
+
+		if s.ruleSrcAddress != "" {
+			// Partial match on either start or end address
+			addrLower := strings.ToLower(s.ruleSrcAddress)
+			if !strings.Contains(strings.ToLower(rule.SourceAddressStart), addrLower) &&
+				!strings.Contains(strings.ToLower(rule.SourceAddressEnd), addrLower) {
+				match = false
+			}
+		}
+
+		if match {
+			matchedIndices = append(matchedIndices, i)
+		}
+	}
+
+	if len(matchedIndices) == 0 {
+		if s.rulePosition != 0 {
+			return nil, fmt.Errorf("firewall rule at position %d not found on server %s", s.rulePosition, arg)
+		}
+		return nil, fmt.Errorf("no firewall rules matching the specified filters found on server %s", arg)
+	}
+
+	// Confirmation check
+	if len(matchedIndices) > s.skipConfirmation {
+		var ruleDescriptions []string
+		for _, idx := range matchedIndices {
+			rule := currentRules.FirewallRules[idx]
+			desc := fmt.Sprintf("  - Position %d: %s %s", rule.Position, rule.Direction, rule.Protocol)
+			if rule.Comment != "" {
+				desc += fmt.Sprintf(" (comment: %q)", rule.Comment)
+			}
+			ruleDescriptions = append(ruleDescriptions, desc)
+		}
+
+		return nil, fmt.Errorf("would disable %d rules (exceeds skip-confirmation=%d). Matching rules:\n%s\n\nIncrease --skip-confirmation to proceed",
+			len(matchedIndices), s.skipConfirmation, strings.Join(ruleDescriptions, "\n"))
+	}
+
+	// Modify matched rules
+	modifiedCount := 0
+	for _, idx := range matchedIndices {
+		if currentRules.FirewallRules[idx].Action != upcloud.FirewallRuleActionDrop {
+			currentRules.FirewallRules[idx].Action = upcloud.FirewallRuleActionDrop
+			modifiedCount++
 		}
 	}
 
-	if !ruleFound {
-		return nil, fmt.Errorf("firewall rule at position %d not found on server %s", s.rulePosition, arg)
+	if modifiedCount == 0 {
+		return nil, fmt.Errorf("all %d matching rules already disabled", len(matchedIndices))
 	}
 
 	// Replace entire ruleset atomically
@@ -81,6 +194,7 @@ func (s *ruleDisableCommand) Execute(exec commands.Executor, arg string) (output
 		return commands.HandleError(exec, msg, err)
 	}
 
+	msg = fmt.Sprintf("Disabled %d firewall rule(s) on server %v", modifiedCount, arg)
 	exec.PushProgressSuccess(msg)
 
 	return output.None{}, nil

internal/commands/server/firewall/rule_disable_test.go

@@ -51,13 +51,13 @@ func TestRuleDisableCommand(t *testing.T) {
 			name:        "Missing position flag",
 			flags:       []string{},
 			arg:         serverUUID,
-			expectedErr: `required flag(s) "position" not set`,
+			expectedErr: "at least one filter must be specified",
 		},
 		{
 			name:        "Invalid position - too low",
 			flags:       []string{"--position", "0"},
 			arg:         serverUUID,
-			expectedErr: "invalid position (1-1000 allowed)",
+			expectedErr: "at least one filter must be specified",
 		},
 		{
 			name:        "Invalid position - too high",