feat: Add firewall rule enable/disable commands #630
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implements UpCloudLtd#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.
…commands Extends firewall rule enable/disable commands with multiple filter options beyond just position-based selection, addressing feedback on issue UpCloudLtd#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: UpCloudLtd#244
Verifies that --skip-confirmation 0 correctly requires confirmation even for a single rule modification, ensuring users can always opt into manual confirmation for safety.
Improve documentation for --skip-confirmation flag to clearly explain that setting it to 0 will always require confirmation, even for a single rule modification.
Extract common filter flag definitions, matching logic, and rule modification code into rule_modify_shared.go to eliminate duplication between enable and disable commands. Benefits: - Reduces code from ~200 lines each to ~45 lines per command - Single source of truth for filter logic and validation - Easier to maintain and extend with new features - All tests pass, no behavior changes The shared functions: - addRuleFilterFlags: Defines all filter flags - configureRuleFilterFlagsPostAdd: Sets up mutual exclusivity and completion - findMatchingRules: Filters rules based on criteria - modifyFirewallRules: Main modification logic with confirmation
Implements intelligent shell completion for: - --dest-port: Suggests common ports (SSH, HTTP, HTTPS, databases) and parses /etc/services for well-known TCP/UDP ports - --src-address: Suggests common IP ranges (private networks, localhost, any IPv4/IPv6) - --skip-confirmation: Suggests 0 (always confirm) and 10 (batch operations) This improves UX by helping users discover valid values without consulting documentation.
- Remove FTP (21) - not recommended for production use - Remove all Dev-Server entries (3000, 5000) - not relevant for firewall rules - Update descriptions for ports 80, 8000, 8080 to clarify HTTP/HTTPS usage Changed from 'HTTP-Alt' to 'HTTP (or HTTPS w/TLS)' for clarity
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements
upctl server firewall rule enableandupctl server firewall rule disablecommands to modify existing firewall rules.Features:
Closes: #244