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

(#13201) Firewall autorequire Firewallchains #67

Merged
merged 2 commits into from
Mar 19, 2012

Conversation

dcarley
Copy link
Contributor

@dcarley dcarley commented Mar 17, 2012

Autorequire Firewallchain resources for Firewall resources that have jump or
chain parameters. Remove require params from README examples now that
they're not essential.

Only deals with iptables and ip6tables providers, which have support for
chains. Doesn't attempt to weed out chains that might be builtin. Just let
Puppet determine which of the resources are really managed.


Doesn't include tests. Should firewall_spec.rb mock up the appropriate Firewallchain resources?

Autorequire Firewallchain resources for Firewall resources that have jump or
chain parameters. Remove require params from README examples now that
they're not essential.

Only deals with iptables and ip6tables providers, which have support for
chains. Doesn't attempt to weed out chains that might be builtin. Just let
Puppet determine which of the resources are really managed.
@kbarber
Copy link
Contributor

kbarber commented Mar 17, 2012

Looking good,

So there are examples for how to document autorequires in puppet core. Try this one on for size:

https://github.com/puppetlabs/puppet/blob/master/lib/puppet/type/user.rb#L17-20

And there are a few examples around for testing autorequires, on the same type try this one:

https://github.com/puppetlabs/puppet/blob/master/spec/unit/type/user_spec.rb#L327-342

I'd imagine we want to focus on testing not just explicit table/chains - but also any implicit/default settings to make sure the Right Thing (tm) happens :-).

Test for autorequire behaviour on :chain, :jump, and :chain + :jump params.
With both specified and default/undef :table and :provider params.

Document autorequire behaviour in docstring.
@dcarley
Copy link
Contributor Author

dcarley commented Mar 19, 2012

Those tests should cover everything except for the absence of autoreqs on providers other than ip[6]tables, of which we don't have at the moment. Should we test for that?

@kbarber
Copy link
Contributor

kbarber commented Mar 19, 2012

@dcarley umm ... not sure we could even test for that reasonably today. Is this ready to test now?

@dcarley
Copy link
Contributor Author

dcarley commented Mar 19, 2012

Yeah, ready to rock.

kbarber added a commit that referenced this pull request Mar 19, 2012
(#13201) Firewall autorequire Firewallchains
@kbarber kbarber merged commit b5b55dd into puppetlabs:master Mar 19, 2012
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