Skip to content

v2.4.2 — Rule reordering for chains and filters - #111

Merged
ibehren1 merged 2 commits into
masterfrom
v2.4.2
Sep 6, 2026
Merged

ibehren1 merged 2 commits into
masterfrom
v2.4.2

Conversation

@ibehren1

@ibehren1 ibehren1 commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Rule numbers determine evaluation order on VyOS, but there was no way to change a rule number after creation — the only option was to delete the rule and re-enter every field under a new number. A reorder backend and routes already existed but had been orphaned since the UI rewrite in 61849de removed the forms that posted to them.

All changes verified: 447 tests pass, bandit clean.

Reordering controls

Chain and filter rule cards now offer three controls:

  • Up/down arrows — swap a rule's number with its neighbour's. The arrow is hidden on the first and last rule of a chain.
  • Number field — assign an explicit rule number. If the number is already in use, that rule and any rules directly following it with no gap in between are each shifted up by one to make room; rules past the first gap are left alone. Example: in 10, 20, 21, 30, moving 10 to 20 yields 20, 21, 22, 30.
  • Resequence (per chain/filter) — rewrite every rule number to 10, 20, 30, ... while preserving relative order, to restore gaps in a chain whose numbers have been used up.

Both IPv4 and IPv6, chains and filters. Every new form carries a CSRF token like the existing ones.

Implementation

  • New package/rule_order_functions.py holds the shared numbering logic — the swap/renumber/resequence map builders and a single mutation primitive. Both chain_functions and filter_functions call it with their own container, since chains store rules flat while filters nest them under rules. This addresses the reorder_* half of the duplication noted in the code review; delete_*_rule and assemble_* are still duplicated.
  • All rules being moved are removed before any are re-inserted, so swaps and cascading shifts cannot overwrite one another.
  • New routes /chain_rule_move, /chain_rules_resequence, /filter_rule_move, /filter_rules_resequence — POST-only, @login_required + @requires_firewall, redirecting back to the chain's anchor like the existing reorder routes.
  • Redirects no longer pass an empty anchor when validation fails.

Behavior change

reorder_chain_rule_in_data and reorder_filter_rule_in_data keep their names, signatures and form contract, but a taken rule number now shifts the occupant instead of being rejected. Their test_duplicate_number_error tests are rewritten to assert the shift.

Notes for existing installs

  • No schema change and no migration. rule-order keeps its current meaning as a numerically sorted index of the rules that exist, so existing configs are untouched until someone uses a control.
  • A config push does not remove old rule numbers from the device. Pushes use load_merge_candidate and generate_config emits only set commands, so renumbering adds the new rule number and leaves the old numbered rule in place. This already applied to rule deletion, but resequencing makes it much easier to hit, so the Resequence confirm dialog spells it out. Emitting matching delete firewall ... commands is left as follow-up work.

Tests

  • tests/test_rule_order_functions.py — the pure map builders and the mutation primitive, including the cascade, the 999999 bound and both container layouts.
  • tests/test_rule_reorder_integration.py — drives the real routes and templates against an in-memory store (no MongoDB), covering render, move, renumber-with-shift and resequence for chains and filters.
  • Move/resequence/cascade/bound cases added to tests/test_chain_functions.py and tests/test_filter_functions.py; route tests added to tests/test_app_routes.py.

Rule numbers determine evaluation order on VyOS, but there was no way to
change a rule number after creation — the only option was to delete the
rule and re-enter every field under a new number. A reorder backend and
routes already existed but had been orphaned since the UI rewrite in
61849de removed the forms that posted to them.

Chain and filter rule cards now offer three controls:

- Up/down arrows, which swap a rule's number with its neighbour's. The
  arrow is hidden on the first and last rule of a chain.
- A number field, which assigns an explicit rule number. If the number
  is already in use, that rule and any rules directly following it with
  no gap in between are each shifted up by one to make room; rules past
  the first gap are left alone.
- A per-chain Resequence button, which rewrites every rule number to
  10, 20, 30, ... while preserving relative order.

The shared numbering logic lives in the new rule_order_functions module,
which both chain_functions and filter_functions call with their own
container — chains store rules flat, filters nest them under "rules".
All rules being moved are removed before any are re-inserted so that
swaps and cascading shifts cannot overwrite one another.

reorder_chain_rule_in_data and reorder_filter_rule_in_data keep their
names, signatures and form contract. Their behavior on a taken rule
number changes from rejecting the request to shifting the occupant, and
their tests are updated to match. Redirects no longer pass an empty
anchor when validation fails.

Note that a config push uses load_merge_candidate and emits only "set"
commands, so renumbering adds the new rule number on the device without
removing the old one. This already applied to rule deletion, but
resequencing makes it much easier to hit, so the Resequence confirm
dialog spells it out. Emitting matching "delete firewall" commands is
left as follow-up work.

No schema change and no migration; "rule-order" keeps its current
meaning as a numerically sorted index of the rules that exist.
The chain_view and filter_view routes call list_user_files and
list_snapshots, which query MongoDB directly, so the new integration test
only passed where a local MongoDB happened to be running and failed in
CI. Stub both alongside the data read/write patches.

Verified by running the suite against an unreachable MONGODB_URI.
@ibehren1 ibehren1 self-assigned this Sep 6, 2026
@ibehren1
ibehren1 merged commit 1dc4719 into master Sep 6, 2026
1 check passed
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.

1 participant