Feature/rebuild fully tested

[xorima]

Description

The current iteration of the cookbook does not expose all the common parts of a rule and has used -m comment while match does not prefix with the -m.

This commit totally rewrites the cookbook and is still a work in progress, opening for comments and thoughts

This will also migrate the cookbook to github actions for testing

Things still to do:

• Have resource to handle the install of iptables (default recipe)
• Have a resource to clear the rules (disabled recipe)
• See if we can use the cli instead of a rewrite to disk and reload as a better option (Parsing issues maybe)
• Write some other resources which are wrappers on iptables_rule, such as a iptables_tcp_rule which will add properties for the options the tcp matcher provides
• Update the readme with all this documentation (What would your thoughts be on having a documentation folder similar to sous-chefs with each resources documented individually?)

  Issues Resolved

[List any existing issues this PR resolves]

Check List

• [ ] All tests pass. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/TESTING.MD
• [ ] New functionality includes testing.
• [ ] New functionality has been documented in the README if applicable
• [ ] All commits have been signed for the Developer Certificate of Origin. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/CONTRIBUTING.MD

[xorima]

@tas50 will write some docs around this tomorrow if I get time, What is odd is that while dokken and kitchen pass on my local they do not pass on github actions (Local is fedora) so I am just leaving centos-8 for now, we also have unit tests for this

Documentation plans are:

• Changelog
• Upgrading notes
• Resources documented individually

I have restored as well travis for now (Note, travis also only ran delivery so there has never been testing on this cookbook for kitchen/integration testing)

[xorima]

@tas50 ready for a review on this one, will sort out resources for the other recipes if this goes ahead

[xorima]

@tas50 this is all ready for review, note I tried to get deprecated_property_alias 'target', 'Jump', 'The target property was renamed jump in 7.0.0 and will be removed in 8.0.0' but it blew up so if you can take a look inside iptables_rule I would appreciate it

[xorima]

Notes to self, set deprecated_property_alias 'target', 'jump', 'The target property was renamed jump in 7.0.0 and will be removed in 8.0.0' ... jump should be lower case

Get more testing in this!

[xorima]

Left to fix is Centos-6 as some packages do not exist there we use, and any other tests that break, hopefully I can get this all green tomorrow and ready to merge in with full testing supported.

Also want to extend the unit tests on the helper library for the few additional functions now in there

[xorima]

@bmhughes do you have any idea how to get this working on centos 6, I keep getting modules cannot be loaded errors (note dokken does not seem to work, but kitchen we get that far ...)

[bmhughes]

@Xorima ignore the last comment I'd help if I was on the right branch 🤦🏻‍♂️.

It converges for me on vagrant, it fails on stopping when trying to unload the iptables kernel modules which I presume is the same error. It failed with module in use so I'm wondering if it's possible to do in docker? There are a few google hits around this.

[xorima]

Yeah, the tests also fail mate, just banging my head against a wall, centos 6 support ends in november

[bmhughes]

On vagrant it's failing the test because the iptable modules are missing and on docker it won't unload them, it's catch 22.

The CentOS 6 vagrant image is missing the ip_conntrack module being loaded by default if you modprobe ip_conntrack then service iptables restart the rules will be loaded and the inspec test will then pass. Not sure how best to work around this with, might be easiest to add an execute resource to the test cookbook to load it as it's going to be a bento PR otherwise.

Docker I'm looking into but that seems more difficult with how the networking side of it works.

[xorima]

If you want to take a stab at getting it in you have write access to my repo :)

[bmhughes]

If you want to take a stab at getting it in you have write access to my repo :)

Will do, I have something that may work. I want to test with it a bit more tomorrow and I'll push it, still haven't given up on docker yet.

[bmhughes]

It's not perfect but that passes for me on Vagrant and Dokken now.

edit: Hmm doesn't work on azure though with the image they have, I guess if we aren't super fussed about Vagrant testing we can remove the module part.

[bmhughes]

That needs a serious squash but it passes on CI/dokken/vagrant now.

[xorima]

@bmhughes AMAZING!

Now should that functionality be automatically done by our resources?

@tas50 enjoy the review ;)

[bmhughes]

I don't think so as it's all a workaround for Kitchen on various platforms and I believe it shouldn't be relevant for a 'proper' system. Without spinning up a full CentOS 6 VM to test with I never remember having to manually load the modules.

1. Vagrant/Bento the modules aren't loaded (but should be, there's a quirk there with the packer template I guess). I have no idea why the init script doesn't load them if missing, even more helpful that is successfully exits without actually loading the rules.

2. The dokken and azure's images load them correctly but for some reason the CentOS 6 init scripts default to unloading the modules as well as flushing the rules which I don't think will be possible with docker as it's dependant on netfilter for networking afaik so there's various differences there between dokken and a vagrant VM. In a container there's always a dependency on at least one of the modules even with all the rules flushed so they won't unload.

I don't feel bad changing the default behaviour on CentOS 6 a bit as they removed unloading the modules upstream going forward anyway.

[tas50]

I'm ok with retaining the history. It's huge, but this way we don't squash out valuable information later. Thanks for doing all this @Xorima.