(MODULES-2159) ignore the --connlimit-saddr switch when parsing rules

[paulseward]

Workaround for https://tickets.puppetlabs.com/browse/MODULES-2159 (as described by Greg Murphy in that ticket)

On some distributions (notably on Ubuntu 14.04 and above, and Centos7 and above) the --connlimit-saddr switch is added after the rule is applied causing rule_to_hash to ignore the rule. Puppet then attempts (and failes) to re-create the rule every time it runs.

[tphoney]

Thanks for this fix. Some tests would be great, to prevent this kind of thing from showing up again. Do you need any help with this ?

[paulseward]

It looks to me like newer versions of iptables add that --connlimit-saddr option when you add the rule, to make what used to be a default more explicit.

I'm no rspec expert, but it looked to me like the connlimit_spec.rb was already spotting and ignoring the --connlimit-saddr parameter (line 34)

I'm not sure what the best way to approach rewriting that test would be, as you're testing for a change in behaviour of iptables rather than a change in behaviour of the module.

I guess what I'm saying is, "yes please!" to the offer of help with the tests!

[bmjen]

Thanks @paulseward , as stated in the JIRA ticket (MODULES-2159) the --connlimit-saddr isn't available on centos6, possibly meaning it isn't available on all Redhat 6 platforms. Is it possible you can validate this against EL6 and possibly add a conditional check to ensure this won't break older versions of centos or redhat?

[paulseward]

@bmjen I'm not sure what you mean. The patch as proposed in that JIRA ticket doesn't affect how puppet specifies the rule to iptables (when creating the rule) - that behaviour hasn't changed

The patch applies where puppet is inspecting the rules currently in iptables to see if "our" rules are present. Without the patch, iptables returns the rule complete with the --connlimit-saddr flag, so it doesn't match the rule that the module was expecting, so the module re-adds the rule - and over time you end up with hundreds of them!

If you look at what the patch does, it uses a regex to strip out the "--connlimit-saddr" flag from the rule as returned by iptables but before we parse the rule to check if it matches one of "our" rules.

If the --connlimit-saddr flag isn't present (because iptables didn't add it) there's nothing to strip out.

I can't see how adding a conditional based on the os version would improve that.

The patched version of the module works fine against centos6 and centos7 in my testing. I don't have any centos5 or ubuntu available to test against.

[bmjen]

Ah. @paulseward thanks for the explanation. Oversight on my part. So really the only thing this needs is test, which we can certainly provide pointers for.

[jonnytdevops]

I am going to merge this now. I will update the current connlimit acceptance test to make sure idempotency is ensured.