Skip to content

Feat/typed option columns#7

Closed
NavidSassan wants to merge 16 commits intomainfrom
feat/typed-option-columns
Closed

Feat/typed option columns#7
NavidSassan wants to merge 16 commits intomainfrom
feat/typed-option-columns

Conversation

@NavidSassan
Copy link
Copy Markdown
Member

No description provided.

Replace the JSON `options` columns on Host, Interface, and Rule with
individual typed SQLAlchemy columns (opt_xxx) for compile-time type
safety and IDE autocomplete. All consumers now access options as direct
attributes (e.g. fw.opt_accept_established, rule.opt_log).

Core changes:
- Add core/options module with OptionMeta registry mapping yaml keys to
  typed column names, defaults, and Python types
- Add apply_options() and build_options_dict() helpers for dict<->column
  conversion, used by XML/YAML readers and writers
- Add 96 Host, 4 Interface, and 55 Rule typed option columns
- Add legacy key migration for backward compatibility with old .fwf files
- Add dataclass schemas with typed defaults for GUI dialogs
- Use nullable columns (default=None) where "not set" differs from
  "explicitly false/empty"
- Keep backward-compat options property on Host/Interface/Rule for now

Consumer migration:
- Platform compilers (iptables, nftables) use fw.opt_xxx directly
- GUI dialogs read/write typed columns instead of options dicts
- CompRule builds its options dict from typed columns at load time
- Rule.group promoted to a dedicated column (was inside options JSON)

Also fixes empty-string falsy bug in _compiler_driver.py where
`fw.opt_firewall_dir or '/etc/fw'` incorrectly defaulted when the
option was explicitly set to ''.
Remove the backward-compat options property getter/setter from Host,
Interface, and Rule ORM models. Extract the shared coercion logic into
apply_options() and build_options_dict() in _metadata.py, and migrate
the remaining consumers (XML reader, YAML reader/writer, GUI
policy_model) to call these helpers directly.

This prevents future code from accidentally using `obj.options = {...}`
which would silently create a plain instance attribute that is never
persisted to the database.
# Conflicts:
#	src/firewallfabrik/gui/policy_model.py
#	src/firewallfabrik/platforms/iptables/_policy_compiler.py
#	src/firewallfabrik/platforms/iptables/_print_rule.py
#	src/firewallfabrik/platforms/nftables/_policy_compiler.py
#	src/firewallfabrik/platforms/nftables/_print_rule.py
…umns

# Conflicts:
#	src/firewallfabrik/driver/_compiler_driver.py
#	tests/expected-output/nft/objects-for-regression-tests/firewall-server-1-s.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall16.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall18.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall2-5.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall2-6.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall2-7.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall23-2.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall23-3.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall23.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall31.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall4.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall73.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall74.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall80.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall94.fw
#	tests/expected-output/nft/objects-for-regression-tests/firewall95.fw
…nd centralize host defaults

Replace the untyped CompRule.options dict with 54 typed dataclass fields
(bool/int/str), eliminating silent typo failures across 127 call sites in
10 compiler files. Add compiler_default field to OptionMeta and
HOST_COMPILER_DEFAULTS dict as single source of truth for fallback values
(firewall_dir, admuser, prolog_place, ipv4_6_order), replacing scattered
hardcoded 'or' fallbacks in compiler drivers.
…g applied

Use HOST_COMPILER_DEFAULTS for placeholder text and fallback values in
settings dialogs and compile dialog instead of hardcoded strings. Fix
_PLATFORM_DEFAULTS being silently dropped when creating new firewalls
(Host has no 'options' dict column) by converting to typed column names
and applying via setattr. Clean up mangle_only_rule_set dict access by
removing unnecessary isinstance(str) guards.
Update test_store_action_processor to use typed CompRule fields instead of
options dict. Update test_load_save to check typed ORM columns via
RULE_OPTIONS metadata. Regenerate expected output and fixture files.
The YAML writer now omits fields that match their column defaults (from
the typed ORM columns migration). Regenerate fixtures from round-trip
output so test_load_save passes.
@NavidSassan NavidSassan marked this pull request as draft February 26, 2026 12:31
Replace intermediate options dict with direct attribute access on the
Rule ORM object in ActionsPanel._load_options(), eliminating string-keyed
dict lookups that could silently return wrong defaults on typos. Simplify
_save_options() to only pass changed widget values. Remove redundant
`or ''` null coercion in _compiler_driver where the value is immediately
checked in an if-statement.
Remove unnecessary `or ''` where the value is immediately checked in an
if-statement, and simplify `(fw.opt_x if fw else '') or ''` to
`fw.opt_x or ''` since fw cannot be None at that point.
…_DEFAULTS

The '/second' fallback was hardcoded in 3 places (iptables PrintRule,
iptables settings dialog, nftables settings dialog). Moved to the single
source of truth in _metadata.py to prevent drift.
…MPILER_DEFAULTS

Move hardcoded '/usr/sbin/nft', 'ip', and ULOG spin box defaults (cprange=0,
qthreshold=1, nlgroup=1) into the single source of truth in _metadata.py.
Also remove dead 'or 0' on non-nullable int columns (limit_value, cprange).
Platform creation-time defaults are data, not GUI logic. Moved from
new_device_dialog.py to core/options/_metadata.py alongside the other
option metadata (HOST_OPTIONS, HOST_COMPILER_DEFAULTS).
Replace dict-based _read_rule_options() with _get_rule() returning the
ORM object. Load reads typed attributes directly; save builds only the
widget values instead of read-all/mutate/filter. Removes the "clean out
defaults" stripping loop and the unused _to_int() helper.
…lumns

Replace JSON dict columns with first-class typed SQLAlchemy columns for
Address (inet_address, inet_netmask, range_start, range_end), Service
(icmp_type, icmp_code, protocol_num, tcp flags/masks, ip options, tag_code),
Host (platform, host_OS, version, timestamps, inactive, mac_filter_enabled),
Interface (dyn, unnum, label, security_level, management, unprotected,
dedicated_failover), Rule (ipt_use_masq, ipt_use_snat_instead_of_masq,
no_fail), and RuleSet (mangle_only_rule_set).

Update XML/YAML readers to populate typed columns, YAML writer to
reconstruct legacy dict format for backward compatibility, and all GUI
dialogs and compiler code to access typed columns directly.
…lumns

Replace Rule.negations JSON dict with 16 typed neg_* bool columns
(one per SLOT_NAMES entry), giving compile-time safety and eliminating
silent typo bugs from untyped .get() lookups.

Promote the folder string from the data JSON dict to a first-class
column on Address, Service, Interval, Group, Host, and Interface,
replacing ~12 deepcopy-mutate-reassign patterns in the GUI with
direct attribute assignment.
@markuslf markuslf closed this Mar 9, 2026
@markuslf markuslf deleted the feat/typed-option-columns branch March 17, 2026 15:55
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.

2 participants