add support for using rpfilter in rules

[cmusik]

When rules with rpfilter are used, then there are warnings like these in the output:

Warning: Puppet::Type::Firewall::ProviderIptables: Skipping unparsable iptables rule: keys (5) and values (6) count mismatch on line: -A cali-PREROUTING -m comment --comment "cali:mPIOOWmbH3iO0R90" -m mark --mark 0x40000/0x40000 -m rpfilter --validmark --invert -j DROP

[CLAassistant]

All committers have signed the CLA.

[github-actions]

Hello! 👋

This pull request has been open for a while and has had no recent activity. We've labelled it with attention-needed so that we can get a clear view of which PRs need our attention.

If you are waiting on a response from us we will try and address your comments on a future Community Day.

Alternatively, if it is no longer relevant to you please close the PR with a comment.

Please note that if a pull request receives no update for 7 after it has been labelled, it will be closed. We are always happy to re-open pull request if they have been closed in error.

[cmusik]

I have updated the code so that the tests should run fine now

[david22swan]

@cmusik
Look's like you've got a bunch of syntax errors.
The only test failure I see looks to be transient though.

[cmusik]

@david22swan I have fixed them and now there should be no syntax problem

[david22swan]

@cmusik While this looks good to me on a first glance I would prefer that it have at least one test to cover the new functionality.

[cmusik]

right now the rpfilter option allows you only to specify one argument so this fix here is to parse it correctly if there are more arguments set. We have applications which are creating rpfilter rules with more then one argument and this leads to this warning specified in the description. So I'm not sure how to test it without fixing the other problem (right now I'm struggling with this).

[cmusik]

Ok, I have found the missing steps and updated the PR. Now I still need to add a test case for the new functionality.

[jordanbreen28]

@cmusik has there been any update with adding a test for the new functionality?

[cmusik]

@jordanbreen28 is there any guide how to implement and running the acceptance tests? I have worked a lot with pdk and unit tests but struggling with the acceptance tests

[jordanbreen28]

Hi @cmusik - you can test modules using Litmus, check out our Litmus documentation here.

[cmusik]

Hi @jordanbreen28, I have finally managed to get the acceptance to run and extended them for the rpfilter feature

[jordanbreen28]

@cmusik Thank you for all your work on this! It seems that the spec tests are failing with

manifests/linux/debian.pp - ERROR: there should be a single space before an opening brace on line 37 (check: manifest_whitespace_opening_brace_before).

We are aware of this and are working to rectify, it is unrelated to this PR.

Please rebase with the current main so we can proceed.

P.s. we are also aware of the PR testing failures for this module and they are unrelated to this PR. :-)

[cmusik]

@jordanbreen28 I have rebased the branch. And yes - I have already noticed that the tests are failing. I had also some problems with litmus tests from the main branch.

[david22swan]

@cmusik Look's like you've just got a simple syntax failure here:

manifests/linux/debian.pp - ERROR: there should be a single space before an opening brace on line 37 (check: manifest_whitespace_opening_brace_before)

[cmusik]

@david22swan this problem was not introduced by me and was fixed a few days ago in the puppetlabs main branch. I have rebased again and it should work now

[cmusik]

I have somehow missed one rubocop warning. This is fixed now and hopefully this was the last problem with this PR :)

[jordanbreen28]

Hi @cmusik - Apologies for the wait with this one, can you rebase with the current main and I will work on getting this merged in! :-)

[cmusik]

Hi @jordanbreen28, I have rebased it again.

[jordanbreen28]

@cmusik great! Once tests have completed and are green I'll merge in.
Thanks for your work on this one.