Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Chain6 directive #694

Merged
merged 3 commits into from
Sep 19, 2017
Merged

Add support for Chain6 directive #694

merged 3 commits into from
Sep 19, 2017

Conversation

herver
Copy link
Contributor

@herver herver commented Sep 15, 2017

This PR is related to issue #693 and adds support for the Chain6 configuration directive of the iptables Collectd plugin.

Closes GH-693

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it looks good, I'm just nitpicking over names.

The test was killed. I don't know this repo that well but usually it means parallel_tests was too aggressive for Travis' liking so I'll consider it an unrelated failure.

'nat' => 'In_SSH',
'filter' => 'HTTP',
},
chains6 => {
'nat' => 'In6_SSH',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nat example for IPv6 is odd. I'd just leave this line out

@@ -26,6 +26,20 @@
end
end

context ':ensure => present and :chains6 => { \'nat\' => \'In6_SSH\' }' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also use filter instead of nat? I know it doesn't matter that much, but it feels wrong :)

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let Travis finish before merging.

@traylenator
Copy link
Contributor

@herver the example in the README could also be updated, ... two places for examples with this module.

@traylenator traylenator merged commit 64744f5 into voxpupuli:master Sep 19, 2017
@traylenator traylenator added the enhancement New feature or request label Sep 19, 2017
@traylenator
Copy link
Contributor

Sorry should have squashed commits first, to late now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants