saltstack-formulas / shorewall-formula

Saltstack formula for managing shorewall
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
5 stars 21 forks source link

Support Shorewall V5 #19

Closed hwhesselink closed 4 years ago

hwhesselink commented 4 years ago

PR progress checklist (to be filled in by reviewers)


What type of PR is this?

Primary type

Secondary type

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

Describe the changes you're proposing

Update the formula to handle version 5:

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

Testing checklist

Additional context

pull-assistant[bot] commented 4 years ago
Score: 0.95

Best reviewed: commit by commit


Optimal code review plan (2 commits squashed)

     update for Shorewall version 5.x      drop unsupported routestopped, tcrules in >= 5.0
delete options not s... ... use macro for cleane...
> Squashed 2 commits: > - delete options not supported in ipv6 > - use macro for cleaner version of previous delta
     undo overeager part of merge from all_patches      Merge pull request #11 from hwhesselink/python3      Merge branch 'ap' into v5      default to '-' for all unset fields      Merge remote-tracking branch 'upstream/master' into v5

Powered by Pull Assistant. Last update 8acb4cc ... 557287b. Read the comment docs.

myii commented 4 years ago

Thanks for the PR, @hwhesselink -- apologies for the delayed response. Let see if we can get any reviews from previous contributors.

@n-rodriguez You've contributed to this formula before, would you be able to have a quick look at this PR to see all is OK?

If not, perhaps we can find some other method to ensure this formula is maintained.

n-rodriguez commented 4 years ago

@myii sorry for the delayed answer. Actually I don't use shorewall anymore (I use ufw) so I'm not sure I'm the right person to review this PR. Maybe @daks still using it?

daks commented 4 years ago

Nope, I don't use it anymore, using straight iptables now.

myii commented 4 years ago

@hwhesselink Do you have a plan for further PRs? If so, I might consider adding all of the standardised structure and CI to make things easier to review, etc. Otherwise, perhaps we can just merge this as-is.

hwhesselink commented 4 years ago

@myii The core Shorewall developer stepped down last year https://sourceforge.net/p/shorewall/mailman/message/36589783/ and it appears to be in maintenance mode, so I doubt I'll be submitting further (major) PRs. Given that the kernel seems to be moving away from iptables I don't expect anyone else to pick up development. So maybe just merge as-is and I'll fix issues if they arise.

myii commented 4 years ago

@hwhesselink Thanks for the patience -- merged. Thanks to all others for their feedback, as well.