Configure recipe

[nathwill]

tried to do a couple things here, but definitely looking for a skeptical eye on this (the de-dupe specifically). here's what i changed and why:

• fix the variable names that overlapped with resource attribute methods for clarity
• moved attr-driven configuration into a separate configuration recipe so it's clearer what's going on
• keep non-default recipes limited to single responsibility (e.g install doesn't require manage). this makes it a lot easier for folks to consume specific pieces of a cookbook if they need to do a particular piece of it differently for some reason, and makes it a little easier to intuitively know where to look for something.
• de-dupe the attr-driven config that seemed to be setting default chain policies in default.rb and install.rb from the same attributes via different chain resources? i tried to keep the ones that seemed the most important, but any context around whether that was in fact true duplication or whether i'm off my rocker is appreciated
• update/add specs to match new recipe layout

i did converge the recipe-default-centos-71 suite to make sure a (pseudo-)randomly selected suite was still passing integration tests as well, but to be honest, the number of suite + platform combos at present is a pretty big barrier to full testing. I (and i'd guess most folks looking to contribute) quail at the thought of trying to slog through TK testing for the 117 (!!) current combinations. i'm planning to try to reduce the number of permutation via https://github.com/chr4-cookbooks/iptables-ng/pull/48 (just a couple more pieces to bite off via more focused PRs!), but wanted to keep this one a bit more focused for now.

cheers!

[chr4]

I'll review this in a bit.

Yes, there are TONS of tests. It is really exhausting to run all of them, but I usually do so before releasing a new version. I think it's hard to improve the process without losing importrant checks though.

[chr4]

Unless I'm missing something, removing the duplicate can lead to situations where the required directories are not present. E.g. when using the provider (which calls the instal recipe).

A potential solution would be to create another recipe which takes care of that, and include that in the provider/ and or the other recipes.

[nathwill]

ah, good call, i totally missed that. i'll work on fixing that up and holler when i think there's something workable.

[nathwill]

alrighty; moved that to the provider(s), as it seemed the most appropriate place. thoughts?

[chr4]

Like the approach. Wouldn't calling the scratch directory helpers multiple times also result in "duplicate ressource" warnings?

[nathwill]

ok, tweaked that (dirty hacks are dirty) to avoid the warning; this is still generating dupe warnings, though. i kinda think that should go away and just be a hard fail instead (users should probably be required to explicitly create a chain resource before adding rules to it), but this whole class of issue should disappear if we stop using scratch dirs and start doing atomic rule-generation from the resource list as proposed in #48.

[chr4]

I like the idea of forcing the user to manually create a chain using iptables_ng_chain. This should only be necessary when using a custom chain and could maybe even raise consciousness about what the user is doing. Definitly open for that!

I'm a little annoyed with the testing setup, as I can't properly test the changes automatically (stupid vagrant/ kitchen) :( Is this ready to merge in your opinion as a small step towards the big change?

[nathwill]

uh, if you're down with requiring users to specify custom chains before they use them, I might as well make this PR handle that change as well since it's already mucking about in that part of the code. i'll get that updated, and then i think this is probably good to go

[chr4]

Mmh, well that would require a major version change, as it might break installations. Maybe just do another PR based on this one? Or we decide to work on a new major release anyway.

[nathwill]

well, #48 is definitely going to be a major version change as well, and also accomplishes the "requires users to specify custom chains" goal, but through a totally different approach, so maybe we leave this PR as-is and forgo an intermediate step that only adds that requirement?

[nathwill]

closing this out, will reconsider some of the options in smaller PRs

[chr4]

I'll cherry-pick some of your minor improvements, such as the Berksfile and .kitchen.yml improvements. Thanks for your work! Looking forward to your upcoming pull-requests!