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

(MODULES-439) Work around existing rules #286

Merged
merged 1 commit into from
Jan 29, 2014

Conversation

hunner
Copy link
Contributor

@hunner hunner commented Jan 28, 2014

The firewall resource is not intended to be used with rules that are not
also managed by puppet; the behavior when doing so was undefined. This
is an attempt to make it more defined.

The behavior is that any rule added by puppet will be inserted in its
given order in relation to the other rules managed by puppet, but ahead
of any rules not managed by puppet.

The firewall resource is not intended to be used with rules that are not
also managed by puppet; the behavior when doing so was undefined. This
is an attempt to make it more defined.

The behavior is that any rule added by puppet will be inserted in its
given order in relation to the other rules managed by puppet, but ahead
of any rules not managed by puppet.
@@ -408,8 +408,39 @@ def insert_order
# No rules at all? Just bail now.
return 1 if rules.empty?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if the rule to edit is on a different chain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved. Firewall doesn't support moving rules between chains.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hahaha, I love this "Resolved: WONTFIX." :D

@ghost ghost assigned hunner Jan 28, 2014
apenney pushed a commit that referenced this pull request Jan 29, 2014
(MODULES-439) Work around existing rules
@apenney apenney merged commit a848cfc into puppetlabs:master Jan 29, 2014
@hunner hunner deleted the fix_source branch January 29, 2014 19:01
hunner added a commit to hunner/puppetlabs-firewall that referenced this pull request Feb 6, 2014
In puppetlabs#286 we fixed rule offset detection for existing managed and
unmanaged rules, but in the case where the first rule in a chain was
unmanaged, managed rules were still being inserted under it.

This patch changes it so that if the first rule detected for offset is
unmanaged, then we should insert before that for more consistent
behavior.
apenney pushed a commit that referenced this pull request Feb 8, 2014
Fix for #286 for pre-existing rules at the start of a chain
@mrwacky42
Copy link

I don't understand how this change is supposed to benefit me.
If I'm running fail2ban, it adds a rule at the front of my INPUT chain, something like: -p tcp -m tcp --dport 22 -j fail2ban-SSH. If I have Puppet manage some firewall rules, but ignore this one, it's supposed to end up as the last rule in my INPUT chain. Suppose that my firewall rules have a default drop, which they do; now my fail2ban filtering is completely neutered.

cegeka-jenkins pushed a commit to cegeka/puppet-firewall that referenced this pull request Oct 23, 2017
In puppetlabs#286 we fixed rule offset detection for existing managed and
unmanaged rules, but in the case where the first rule in a chain was
unmanaged, managed rules were still being inserted under it.

This patch changes it so that if the first rule detected for offset is
unmanaged, then we should insert before that for more consistent
behavior.
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.

3 participants