Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T5729: firewall: switch to valueless in #2471

Merged
merged 1 commit into from
Nov 11, 2023
Merged

Conversation

nicolas-fort
Copy link
Contributor

Change Summary

Use valueless in leafNode whenever is possible.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

Component(s) name

firewall
policy route
nat

Proposed changes

Remove unnecessary options for commands <enable|disable>
Commands improve:

  • log
  • state

How to test

vyos@Valueless:~$ show config comm | grep "policy\|firewall"
set firewall ipv4 name FOO rule 10 action 'reject'
set firewall ipv4 name FOO rule 10 log
set firewall ipv4 name FOO rule 10 log-options group '123'
set firewall ipv4 name FOO rule 10 protocol 'gre'
set firewall ipv4 name FOO rule 10 state 'related'
set firewall ipv4 name FOO rule 10 state 'established'
set firewall ipv4 name FOO rule 20 action 'accept'
set firewall ipv4 name FOO rule 20 log
set policy route FOO rule 10 action 'accept'
set policy route FOO rule 10 log
set policy route FOO rule 10 state 'related'
set policy route6 FOO6 rule 10 action 'accept'
set policy route6 FOO6 rule 10 log
vyos@Valueless:~$ 

vyos@Valueless:~$ sudo nft -s list ruleset | grep "log\|state"
                ct state { established, related } meta l4proto gre log prefix "[ipv4-NAM-FOO-10-R]" log group 123 counter reject comment "ipv4-NAM-FOO-10"
                log prefix "[ipv4-NAM-FOO-20-A]" counter accept comment "ipv4-NAM-FOO-20"
                ct state related log prefix "[ipv4-route-FOO-10-A]" counter accept comment "ipv4-route-FOO-10"
                log prefix "[ipv6-route6-FOO6-10-A]" counter accept comment "ipv6-route6-FOO6-10"
vyos@Valueless:~$ 


Smoketest result

root@Valueless:/usr/libexec/vyos/tests/smoke/cli# ./test_firewall.py 
test_bridge_basic_rules (__main__.TestFirewall.test_bridge_basic_rules) ... ok
test_flow_offload (__main__.TestFirewall.test_flow_offload) ... 
Interface "eth0" does not support hardware offload

ok
test_geoip (__main__.TestFirewall.test_geoip) ... Updating GeoIP. Please wait...
Updating GeoIP. Please wait...
ok
test_groups (__main__.TestFirewall.test_groups) ... ok
test_ipv4_advanced (__main__.TestFirewall.test_ipv4_advanced) ... ok
test_ipv4_basic_rules (__main__.TestFirewall.test_ipv4_basic_rules) ... ok
test_ipv4_mask (__main__.TestFirewall.test_ipv4_mask) ... ok
test_ipv4_state_and_status_rules (__main__.TestFirewall.test_ipv4_state_and_status_rules) ... ok
test_ipv4_synproxy (__main__.TestFirewall.test_ipv4_synproxy) ... 
"synproxy" option allowed only for action synproxy

ok
test_ipv6_advanced (__main__.TestFirewall.test_ipv6_advanced) ... ok
test_ipv6_basic_rules (__main__.TestFirewall.test_ipv6_basic_rules) ... ok
test_ipv6_mask (__main__.TestFirewall.test_ipv6_mask) ... ok
test_nested_groups (__main__.TestFirewall.test_nested_groups) ... 
Group "smoketest_network1" has a circular reference

ok
test_source_validation (__main__.TestFirewall.test_source_validation) ... ok
test_sysfs (__main__.TestFirewall.test_sysfs) ... ok
test_zone_basic (__main__.TestFirewall.test_zone_basic) ... ok

----------------------------------------------------------------------
Ran 16 tests in 59.292s

OK
root@Valueless:/usr/libexec/vyos/tests/smoke/cli#
root@Valueless:/usr/libexec/vyos/tests/smoke/cli# ./test_policy_route.py 
test_pbr_group (__main__.TestPolicyRoute.test_pbr_group) ... ok
test_pbr_mark (__main__.TestPolicyRoute.test_pbr_mark) ... ok
test_pbr_mark_connection (__main__.TestPolicyRoute.test_pbr_mark_connection) ... ok
test_pbr_matching_criteria (__main__.TestPolicyRoute.test_pbr_matching_criteria) ... ok
test_pbr_table (__main__.TestPolicyRoute.test_pbr_table) ... ok

----------------------------------------------------------------------
Ran 5 tests in 25.125s

OK
root@Valueless:/usr/libexec/vyos/tests/smoke/cli#
root@Valueless:/usr/libexec/vyos/tests/smoke/cli# ./test_nat.py 
test_dnat (__main__.TestNAT.test_dnat) ... ok
test_dnat_negated_addresses (__main__.TestNAT.test_dnat_negated_addresses) ... ok
test_dnat_redirect (__main__.TestNAT.test_dnat_redirect) ... ok
test_dnat_without_translation_address (__main__.TestNAT.test_dnat_without_translation_address) ... ok
test_nat_balance (__main__.TestNAT.test_nat_balance) ... ok
test_nat_no_rules (__main__.TestNAT.test_nat_no_rules) ... ok
test_snat (__main__.TestNAT.test_snat) ... ok
test_snat_groups (__main__.TestNAT.test_snat_groups) ... ok
test_snat_required_translation_address (__main__.TestNAT.test_snat_required_translation_address) ... 
Source NAT configuration error in rule 5: translation requires address
and/or port

ok
test_static_nat (__main__.TestNAT.test_static_nat) ... ok

----------------------------------------------------------------------
Ran 10 tests in 30.995s

OK
root@Valueless:/usr/libexec/vyos/tests/smoke/cli# ./test_nat66.py 
test_destination_nat66 (__main__.TestNAT66.test_destination_nat66) ... ok
test_destination_nat66_prefix (__main__.TestNAT66.test_destination_nat66_prefix) ... ok
test_destination_nat66_protocol (__main__.TestNAT66.test_destination_nat66_protocol) ... ok
test_destination_nat66_without_translation_address (__main__.TestNAT66.test_destination_nat66_without_translation_address) ... ok
test_nat66_no_rules (__main__.TestNAT66.test_nat66_no_rules) ... ok
test_source_nat66 (__main__.TestNAT66.test_source_nat66) ... ok
test_source_nat66_address (__main__.TestNAT66.test_source_nat66_address) ... ok
test_source_nat66_protocol (__main__.TestNAT66.test_source_nat66_protocol) ... ok
test_source_nat66_required_translation_prefix (__main__.TestNAT66.test_source_nat66_required_translation_prefix) ... 
Source NAT66 configuration error in rule 5: translation address not
specified


Source NAT66 configuration error in rule 5: translation address not
specified

ok

----------------------------------------------------------------------
Ran 9 tests in 25.442s

OK
root@Valueless:/usr/libexec/vyos/tests/smoke/cli# 

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

…enable|disable> commands; log and state moved to new syntax.
@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro, sever-sever and c-po and removed request for a team November 10, 2023 19:38
@c-po c-po merged commit e5a53d4 into vyos:current Nov 11, 2023
@nicolas-fort
Copy link
Contributor Author

I think this should be back ported to 1.4

@c-po
Copy link
Member

c-po commented Nov 13, 2023

@Mergifyio backport sagitta

Copy link
Contributor

mergify bot commented Nov 13, 2023

backport sagitta

✅ Backports have been created

@c-po
Copy link
Member

c-po commented Nov 13, 2023

I think this should be back ported to 1.4

definately

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants