vyos
Impove support for 1.4 Firewall Rules Syntax
Change Summary
A big changed happened in 1.4-rolling-202308040557, and this collection currently cannot handle the syntax after that. That means one cannot use current rolling releases and expect firewall rules setting to work.
I am certain this is not exhaustive, but it is a big improvement in what actually works!
Types of changes
- [x] 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)
- [ ] Other (please describe):
Related Task(s)
https://vyos.dev/T6706
Related PR(s)
Proposed changes
This is a non-breaking change for anything that isn't newer than 1.4-rolling-202308040557.
How to test
The regular expression used in this change was tested at https://regex101.com/r/gDIVEI/1
Checklist:
- [x] I have read the CONTRIBUTING document
- [x] I have linked this PR to one or more Phabricator Task(s)
- [x] My commit headlines contain a valid Task id
Hi @sdwilsh Can you try using the code from #352 and see if that resolves the problems you are seeing with the 1.4 firewall rules? I'm working on getting this approved, but there were a lot of changes, so we're moving carefully.
The intention here was to keep compatibility for as far back as possible (1.3 realistically) and push forward to 1.5 rolling, including handling 1.4 as the current release.
I believe this should encompass the same issues that you were encountering.
@gaige, I left a comment there on at least one thing that this fixes that #352 does not fix, with the line that needs to be updated.
@gaige, I left a comment there on at least one thing that this fixes that #352 does not fix, with the line that needs to be updated.
thanks, I’ll look into that on the weekend.
@sdwilsh I think I've got the bulk of your changes in the PR we've been working on. I'm awaiting final review, we've tested against 1.2, 1.3, 1.4, and 1.5 rolling and things seem to be better than they were before, but I won't guarantee that everything is fixed. However, we've now got a few people working on this, so hopefully things will move a bit more quickly #352 is merged.
@andamasov I've asked @sdwilsh to look at this and make sure we're not leaving anything unfixed based on his checks. However, I don't think we need to bring this over unless he indicates a problem, in which case we should probably file a new issue for that specific to the problems he finds.
The specific change in here to address the new address-family specific naming is already handled in the previously-merged PR. I'm going to close this as redundant.