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 ipset support #383

Merged
merged 3 commits into from
Oct 23, 2014
Merged

add ipset support #383

merged 3 commits into from
Oct 23, 2014

Conversation

vzctl
Copy link
Contributor

@vzctl vzctl commented Jun 26, 2014

This is a rebased version of #348 and a fix for MODULES-1173.

@underscorgan
Copy link
Contributor

Can you please rebase this?

@underscorgan underscorgan mentioned this pull request Jul 31, 2014
@vzctl
Copy link
Contributor Author

vzctl commented Aug 6, 2014

@mhaskel shall we merge it now please ? :) I'm rebasing it 3rd time already.

@@ -217,6 +219,8 @@ def self.rule_to_hash(line, table, counter)
# --tcp-flags takes two values; we cheat by adding " around it
# so it behaves like --comment
values = values.gsub(/(!\s+)?--tcp-flags (\S*) (\S*)/, '--tcp-flags "\1\2 \3"')
# ditto for --match-set
values = values.sub(/--match-set (\S*) (\S*)/, '--match-set "\1 \2"')
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is possible to be inverted, do we want to add (!\s+)? and "\1\2 \3" to the regex, and add it to https://github.com/puppetlabs/puppetlabs-firewall/blob/master/lib/puppet/provider/firewall/iptables.rb#L318 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think that's a good idea.

@underscorgan
Copy link
Contributor

@vzctl - @hunner has a couple of good comments but once you resolve those we should be able to get this merged.

@puppetcla
Copy link

CLA signed by all contributors.

@vzctl
Copy link
Contributor Author

vzctl commented Sep 19, 2014

sorry for the late reply. i resolved all the comments, let me know if there's anything else.

@vzctl
Copy link
Contributor Author

vzctl commented Oct 19, 2014

@mhaskel @hunner could you take a look when you have time?

hunner added a commit that referenced this pull request Oct 23, 2014
@hunner hunner merged commit 4ed1b43 into puppetlabs:master Oct 23, 2014
cegeka-jenkins pushed a commit to cegeka/puppet-firewall that referenced this pull request Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants