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

(#9362) Create action property and perform transformation for accept, dro #15

Merged
merged 1 commit into from
Oct 18, 2011

Conversation

kbarber
Copy link
Contributor

@kbarber kbarber commented Oct 12, 2011

(#9362) Create action property and perform transformation for accept, drop, reject value for iptables jump parameter.

This commit introduces the new 'action' parameter which is designed to designate
the action to take when a match succeeds. This is a cross-platform parameter and
for the values 'accept','drop','reject' it will take the place of the existing
jump parameter.

The jump parameter is deemed as an iptables specific parameter so by splitting
out this parameter for common actions it allows us to extend the firewall resource
to include other providers much more easily in the future. By having such a common
parameter we will be able to compare resources between boxes that may have different
firewall implementations.

The new behaviour is to force the usage for action parameter, and using 'accept',
'drop' or 'reject' for jump will now no longer work.

To aid in the testing of this new property I've added new ways to test converting
iptables rules to hashes and hashes to general_args. This should simplify the
testing of new bugs as well.

@saysjonathan
Copy link
Contributor

This is great work. There's only two things that came to mind, neither a huge issue:

  • We should probably change the ip6tables examples to use the action property as well.
  • Should we move the test_data hashes in our tests to fixtures?

@kbarber
Copy link
Contributor Author

kbarber commented Oct 14, 2011

  • We should probably change the ip6tables examples to use the action property as well.

Is the ip6tables stuff working? Okay - let me see what needs to be done.

  • Should we move the test_data hashes in our tests to fixtures?

What format do you think? Just .rb files with hashes in them? ... I was pondering this actually with YAML before you mentioned it but that seems ugly for this kind of thing. I don't conceptually mind the idea of having this all in its own file as it may get larger over time.

@kbarber
Copy link
Contributor Author

kbarber commented Oct 14, 2011

IPv6 works ... although we lack specific tests for ip6tables. I've raised another task for that:

http://projects.puppetlabs.com/issues/10086

Since I need to think specifically about what needs tests without doing test duplication.

But I tested it using the folllowing and it seems to work:

firewall { "123 foo":
  ensure => present,
  action => accept,
  source => "::1/128",
  provider => "ip6tables",
}

firewall { "123 foo bar":
  ensure => present,
  jump => "foo",
  source => "::1/128",
  provider => "ip6tables",
}

firewall { "123 foo bar baz":
  ensure => present,
  source => "::1/128",
  provider => "ip6tables",
}

@kbarber
Copy link
Contributor Author

kbarber commented Oct 14, 2011

Okay - I've just submitted a second commit that moves the hashes into a separate file. Can you take a look at it and see if it looks okay?

Don't merge as is - I'll squash if you're happy.

@saysjonathan
Copy link
Contributor

This looks great. Let's squash and merge.

… drop, reject value for iptables jump parameter.

This commit introduces the new 'action' parameter which is designed to designate
the action to take when a match succeeds. This is a cross-platform parameter and
for the values 'accept','drop','reject' it will take the place of the existing
jump parameter.

The jump parameter is deemed as an iptables specific parameter so by splitting
out this parameter for common actions it allows us to extend the firewall
resource to include other providers much more easily in the future. By having
such a common parameter we will be able to compare resources between boxes that
may have different firewall implementations.

The new behaviour is to force the usage for action parameter, and using
'accept', 'drop' or 'reject' for jump will now no longer work.

Also - the default of 'accept' for jump has been removed which means you MUST
specify an action if you want your rule to do something. Without an action the
rule will match, but do nothing (so only useful for keeping counters generally).

To aid in the testing of this new property I've added new ways to test converting
iptables rules to hashes and hashes to general_args. This should simplify the
testing of new bugs as well.
saysjonathan added a commit that referenced this pull request Oct 18, 2011
(#9362) Create action property and perform transformation for accept, dro
@saysjonathan saysjonathan merged commit 86a4491 into puppetlabs:master Oct 18, 2011
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