Skip to content

Conversation

@mgajda
Copy link
Contributor

@mgajda mgajda commented Dec 13, 2025

Implements upctl server firewall rule enable and upctl server firewall rule disable commands to modify existing firewall rules.

Features:

  • Filter rules by position, comment, direction, protocol, dest-port, or src-address
  • Multiple filters combine with AND logic
  • Confirmation prompt when modifying multiple rules (configurable via --skip-confirmation)
  • Shell completion for ports, IP addresses, and common values

Closes: #244

@mgajda mgajda requested a review from a team as a code owner December 13, 2025 13:19
…content-type flag

- Support IANA-registered content types: application/gzip, application/x-xz,
  application/x-tar, application/x-bzip2, application/x-7z-compressed,
  application/zip, and application/octet-stream
- Add --content-type flag to allow explicit content type specification
- Auto-detect content type from file extension with fallback to octet-stream
- Add getContentType() function for extensible content type mapping
- Add comprehensive unit tests for content type detection

Fixes UpCloudLtd#581
- Move supportedContentTypes map to package level for reusability
- Add getSupportedExtensionsText() to list all supported file extensions
- Add getSupportedContentTypesText() to list all supported content types
- Update --source-location help to show supported extensions dynamically
- Update --content-type help to show supported types dynamically
- Simplify getContentType() to use the package-level map
- Help text now automatically reflects all supported formats
@mgajda
Copy link
Contributor Author

mgajda commented Dec 31, 2025

Summary of changes:

  1. **Enhanced `delete` command**:
     - Now supports the same filters as enable/disable
     - Can delete multiple rules matching filters
     - Confirms before deleting multiple rules

  2. **Reimplemented `enable` command**:
     - Moves rules from AFTER catch-all to BEFORE catch-all
     - Only operates on "disabled" rules (those after catch-all)
     - Maintains rule content, just changes position

  3. **Reimplemented `disable` command**:
     - Moves rules from BEFORE catch-all to AFTER catch-all
     - Only operates on "enabled" rules (those before catch-all)
     - Maintains rule content, just changes position

All three work with filters:

  1. --position (int)
    - Exact rule position (1-1000)
    - When specified, all other filters are ignored
    - Targets a single specific rule
  2. --comment (string)
    - Partial match, case-insensitive
    - Searches within rule comments
    - Example: --comment "SSH" matches rules with "SSH server", "ssh access", etc.
  3. --direction (string)
    - Values: in or out
    - Filters rules by traffic direction
  4. --protocol (string)
    - Values: tcp, udp, or icmp
    - Filters rules by network protocol
  5. --dest-port (string)
    - Matches destination port (either start or end of port range)
    - Example: --dest-port 80 matches rules affecting port 80
  6. --src-address (string)
    - Partial match on source IP address
    - Searches in both start and end addresses of source ranges

Filter Behavior:

  • Position filter: When --position is used, it takes precedence and all other filters are ignored (mutually exclusive)
  • Multiple filters: When using comment/direction/protocol/dest-port/src-address, they work with AND logic (all specified filters must
    match)
  • Batch operations: All three commands can operate on multiple rules when filters match more than one rule

Additional Options:

  • --skip-confirmation (int, default: 1)
    • Maximum number of rules to modify without confirmation prompt
    • Set to 0 to always require confirmation
    • If more rules match than this value, user must confirm the operation

@kangasta Is it what you meant?

@mgajda mgajda requested a review from kangasta January 1, 2026 01:24
@mgajda mgajda force-pushed the feat/firewall-rule-enable-disable branch from 5c1bfce to 00132ed Compare January 1, 2026 01:30
mgajda and others added 22 commits January 9, 2026 21:10
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
…tive to catch-all

- delete command now supports filters to delete multiple rules
- enable command moves rules before catch-all drop rule (activates them)
- disable command moves rules after catch-all drop rule (deactivates them)
- consistent filter flags across delete/enable/disable commands

This addresses PR review feedback to make command semantics clearer:
- create/delete for rule CRUD operations
- enable/disable for rule activation by position relative to catch-all
…oudLtd#611)

See https://github.com/yaml/go-yaml#project-status.

Various dependencies have already switched, most recently [cobra in
1.10.2](https://github.com/spf13/cobra/releases/tag/v1.10.2).

For a diff between the old 3.0.1 and the new 3.0.2, see
https://gist.github.com/scop/6ec72debf62a9603cff9dc97e6814ddd. 3.0.1 are
identical.

Upgrade to 3.0.4 while at it:
yaml/go-yaml@v3.0.2...v3.0.4
Big PR, but the bulk of it are modernize's `interface{}` to `any`
autofixes.

---------

Co-authored-by: Ville Välimäki <ville.valimaki@upcloud.com>
mgajda and others added 11 commits January 9, 2026 21:10
…Ltd#627)

Co-authored-by: Toni Kangas <kangasta@users.noreply.github.com>
…Ltd#649)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Closes UpCloudLtd#339

Co-authored-by: Toni Kangas <toni.kangas@upcloud.com>
…pCloudLtd#652)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…UpCloudLtd#646)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…UpCloudLtd#648)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- 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
- Update mocks to use GetFirewallRules() instead of server.FirewallRules
- Mock DeleteFirewallRule and CreateFirewallRule for enable/disable operations
- Fix expected error messages to match new implementation behavior
- All firewall command tests now pass successfully
@mgajda mgajda force-pushed the feat/firewall-rule-enable-disable branch from 067b041 to 4549fc7 Compare January 9, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants