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

T7147: Import Large Network Sets to Firewall Group From File #4341

Open
wants to merge 5 commits into
base: current
Choose a base branch
from

Conversation

sskaje
Copy link
Contributor

@sskaje sskaje commented Feb 9, 2025

Change summary

  • add source-file to firewall/group/network-group (ipv6-network-group)
  • add op command update firewall-groups to update sets from file

Op command updates:

  • nat (nftables_nat.conf)
  • policy (nftables_policy.conf)

Following with group_tmpl not updated:

  • firewall (nftables.conf)
  • nat66 (nftables_nat66.nft)
  • conntrack (nftables-ct.conf)
  • bridge (nftables-bridge)

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)

https://vyos.dev/T7147

Related PR(s)

How to test / Smoketest result

Passed

  • test_firewall.py
  • test_policy.py
  • test_policy_route.py
  • test_policy_local-route.py
  • test_policy_local-route.py

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

Copy link

github-actions bot commented Feb 9, 2025

👍
No issues in PR Title / Commit Title

@sskaje
Copy link
Contributor Author

sskaje commented Feb 9, 2025

For the op command, I don't add all because:

  1. different templates use different vars for group_tmpl, to build a config tree for all templates but using different array keys looks weird
  2. building config tree for these templates like what I have done is not that good, but I don't find a better way rebuild all nftable files and re-apply in op mode.

@sskaje
Copy link
Contributor Author

sskaje commented Feb 9, 2025

When I make changes to firewall groups in config mode, all nftables using groups are updated. Is there any mechanism I can invoke in op mode?

@c-po
Copy link
Member

c-po commented Feb 9, 2025

How is this different from T5493 ?

@sskaje
Copy link
Contributor Author

sskaje commented Feb 9, 2025

How is this different from T5493 ?

I read the changes in that PR, he added a new type of group, but I don't see how he uses such group.
(also read #1648, T6040, etc.)

I would say, the so-called 'remote' should just be a different source for each type of group(embedded in config tree, local file or remote url), but not a new group type.

In my case I planned to say from file or url, but I see many different file formats for china route lists, and there is no standards.

So I create this PR with least thing which I think VyOS should support, which is loading from a file under /config and update nftables.
Those who need to update, they can write their own scripts and setup their own task schedulers.

Other type of groups can be also added in similar way.

Copy link

github-actions bot commented Feb 9, 2025

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests (no interfaces) ❌ failed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

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