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

Adding a forward option for a zone. #46

Merged
merged 20 commits into from
Apr 14, 2014
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Modulefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ author 'Adam Jahn'
project_page 'https://github.com/ajjahn/puppet-dns'
source 'https://github.com/ajjahn/puppet-dns'
dependency 'puppetlabs/concat', '>=1.0.0'
dependency 'puppetlabs/stdlib', '>= 2.4.0'
10 changes: 10 additions & 0 deletions manifests/zone.pp
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,22 @@
$reverse = false,
$zone_type = 'master',
$allow_transfer = [],
$allow_forwarder = [],
$forward_policy = 'first',
$slave_masters = undef,
$zone_notify = false,
$ensure = present
) {

validate_array($allow_transfer)
validate_array($allow_forwarder)
if $dns::options::forwarder and $allow_forwarder {
fatal("You cannot specify a global forwarder and \
a zone forwarder for zone ${soa}")
}
if !member(['first', 'only'], $forward_policy) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is one way to do it, I would have used validate_re, but this works.

fatal('The forward policy can only be set to either first or only')
}

$zone_serial = $serial ? {
false => inline_template('<%= Time.now.to_i %>'),
Expand Down
21 changes: 19 additions & 2 deletions spec/defines/dns_zone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
context 'passing an array to data' do
let :facts do { :concat_basedir => '/dne', } end
let :params do
{ :allow_transfer => [ '192.0.2.0', '2001:db8::/32' ] }
{ :allow_transfer => [ '192.0.2.0', '2001:db8::/32' ],
:allow_forwarder => ['8.8.8.8', '208.67.222.222']
}
end

it 'should pass input validation' do
Expand All @@ -31,11 +33,26 @@
should contain_concat__fragment('named.conf.local.test.com.include').
with_content(/192\.0\.2\.0/)
}

it {
should contain_concat__fragment('named.conf.local.test.com.include').
with_content(/2001:db8::\/32/)
with_content(/forwarders/)
}

it {
should contain_concat__fragment('named.conf.local.test.com.include').
with_content(/forward only;/)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test if failing.

I would like to see this in a context of "By default" => it should have content "forward only"

And then another context of "When asked to have a forward first policy" => it should have content forward first

In order to test both entry points.

Bonus points for at test like
context "When given a bogus forward policy" (params forward_policy => bogus) it should fail

}

it {
should contain_concat__fragment('named.conf.local.test.com.include').
with_content(/8.8.8.8/)
}

it {
should contain_concat__fragment('named.conf.local.test.com.include').
with_content(/2001:db8::\/32/)
}
end

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you give me another context, something like "When allow_forwarder is not defined" and test that it does not have content with the "forward" stuff?

In erb I think that if @allow_forwarder will be true on an empty array, and I would like a test to prove that it does not. (and that the forward stuff does not get leaked into the zone file when it is not specified. Make sense?

end
Expand Down
10 changes: 9 additions & 1 deletion templates/zone.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ zone "<%= @zone %>" {
<%= ip %>;
<% end -%>
};
<% end -%>
<% end -%>
<% if @allow_forwarder -%>
forward <%= @forward_policy %>;
forwarders {
<% @allow_forwarder.each do |ip| -%>
<%= ip %>;
<% end -%>
};
<% end -%>
<% end -%>
};