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

Added support to modify service startup #130

Merged
merged 11 commits into from
Aug 5, 2015
52 changes: 52 additions & 0 deletions manifests/server/default.pp
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# == Class: dns::server::default
#
class dns::server::default (

$default_file = $dns::server::params::default_file,
$default_template = $dns::server::params::default_template,

$resolvconf = undef,
$options = undef,
$rootdir = undef,
$enable_zone_write = undef,
$enable_sdb = undef,
$disable_named_dbus = undef,
$keytab_file = undef,
$disable_zone_checking = undef,

) inherits dns::server::params {

if ! defined(Class['::dns::server']) {
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 incorrect now, this class should be safe to include no matter what

fail('You must include the ::dns::server base class before using any dns default defined resources')
}

validate_absolute_path( $default_file )

if $resolvconf != '' {
validate_re( $resolvconf, '^(yes|no)$', 'The resolvconf value is not type of a string yes / no.' )
}

if $rootdir != '' {
validate_absolute_path( $rootdir )
}

validate_re( $enable_zone_write, '^(yes|no|\s*)$', 'The enable_zone_write value is not type of a string yes / no or empty.' )

validate_re( $enable_sdb, '^(yes|no|1|0|\s*)$', 'The enable_sdb value is not type of a string yes / no / 1 / 0 or empty.' )

if $keytab_file != '' {
validate_absolute_path( $keytab_file )
}

validate_re( $disable_zone_checking, '^(yes|no|\s*)$', 'The disable_zone_checking value is not type of a string yes / no or empty.' )

file { $default_file:
ensure => present,
owner => $::dns::server::params::owner,
group => $::dns::server::params::group,
mode => '0644',
content => template("${module_name}/${default_template}"),
notify => Class['dns::server::service'],
}

}
2 changes: 2 additions & 0 deletions manifests/server/install.pp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# == Class dns::server
#
include dns::server::default
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 not correct, you can't put an include outside of a class definition

Copy link
Collaborator

Choose a reason for hiding this comment

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

The default class should be chained after the package.
The package should be installed first, then the default class will replace the stock defaults file in that order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would now solve it like this in the file server.pp:

class dns::server {
  class { 'dns::server::install': } ->
  class { 'dns::server::config': } ->
  class { 'dns::server::default': } ~>
  class { 'dns::server::service': }
}

Do you agree with this changement?

Copy link

Choose a reason for hiding this comment

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

You can do this in the puppet 4 / future parser, does this not work in the traditional parser?

Kyle Anderson wrote:

In manifests/server/install.pp #130 (comment):

@@ -1,5 +1,7 @@

== Class dns::server

+include dns::server::default

This is not correct, you can't put an include outside of a class definition


Reply to this email directly or view it on GitHub https://github.com/ajjahn/puppet-dns/pull/130/files#r33468120.

Johnson Earls
darkfoxprime@gmail.com

Copy link
Collaborator

Choose a reason for hiding this comment

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

It does, but you shouldn't. All resources need to be contained in some sort of class, not just setting out there for the parser to suck up as it reads the file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Cicco0 yea I think that would work, but I think I actually prefer it to be in the config class, as it is a config file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@solarkennedy I've included it:
include dns::server::default

Now everything should be fine. If so, you need a rebase?

Copy link
Collaborator

Choose a reason for hiding this comment

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

instlal.pp:3 still needs to be removed. It is a include line outside a class definition and will be included by simply parsing this file, which is surprising and impossible to test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was removed and tests were positive.


class dns::server::install (
$necessary_packages = $dns::server::params::necessary_packages,
$ensure_packages = $dns::server::params::ensure_packages,
Expand Down
4 changes: 4 additions & 0 deletions manifests/server/params.pp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
$owner = 'bind'
$package = 'bind9'
$service = 'bind9'
$default_file = '/etc/default/bind9'
$default_template = 'default.debian.erb'
$default_dnssec_validation = 'auto'
case $::operatingsystemmajrelease {
'8': {
Expand All @@ -35,6 +37,8 @@
$package = 'bind'
$service = 'named'
$necessary_packages = [ 'bind', ]
$default_file = '/etc/sysconfig/named'
$default_template = 'default.redhat.erb'
if $::operatingsystemmajrelease =~ /^[1-5]$/ {
$default_dnssec_validation = 'absent'
} else {
Expand Down
156 changes: 156 additions & 0 deletions spec/classes/server/startup_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
require 'spec_helper'

describe 'dns::server::default', :type => :type do

let :pre_condition do
'class { "::dns::server": }'
end

context "on an unsupported OS" do
it{ should raise_error(/dns::server is incompatible with this osfamily/) }
end

context 'by default on debian' do

let(:facts) {{ :osfamily => 'Debian', :concat_basedir => '/tmp' }}

context "passing correct values and paths" do

context 'passing `no` to resolvconf' do
let(:params) {{ :resolvconf => 'no' }}
it { should contain_file('/etc/default/bind9').with_content(/RESOLVCONF=no/) }
end

context 'passing `yes` to resolvconf' do
let(:params) {{ :resolvconf => 'yes' }}
it { should contain_file('/etc/default/bind9').with_content(/RESOLVCONF=yes/) }
end

context 'passing `-u bind -4` to options' do
let(:params) {{ :options => '-u bind -4' }}
it { should contain_file('/etc/default/bind9').with_content(/OPTIONS="-u bind -4"/) }
end

context 'passing `-u bind -6` to options' do
let(:params) {{ :options => '-u bind -6' }}
it { should contain_file('/etc/default/bind9').with_content(/OPTIONS="-u bind -6"/) }
end

end

context "passing wrong values and paths" do

context 'passing wrong value to resolvconf for hit an error' do
let(:params) {{ :resolvconf => 'WrongValue' }}
it{ should raise_error(/The resolvconf value is not type of a string yes \/ no./)}
end

end

end

context 'by default on redhat' do

let(:facts) {{ :osfamily => 'RedHat', :concat_basedir => '/tmp' }}

context "passing correct values and paths" do

context 'passing path `/chroot` to rootdir' do
let(:params) {{ :rootdir => '/chroot' }}
it { should contain_file('/etc/sysconfig/named').with_content(/ROOTDIR="\/chroot"/) }
end

context 'passing `-u named` to options' do
let(:params) {{ :options => '-u named' }}
it { should contain_file('/etc/sysconfig/named').with_content(/OPTIONS="-u named"/) }
end

context 'passing `yes` to enable_zone_write' do
let(:params) {{ :enable_zone_write => 'yes' }}
it { should contain_file('/etc/sysconfig/named').with_content(/ENABLE_ZONE_WRITE=yes/) }
end

context 'passing `no` to enable_zone_write' do
let(:params) {{ :enable_zone_write => 'no' }}
it { should contain_file('/etc/sysconfig/named').with_content(/ENABLE_ZONE_WRITE=no/) }
end

context 'passing `yes` to enable_sdb' do
let(:params) {{ :enable_sdb => 'yes' }}
it { should contain_file('/etc/sysconfig/named').with_content(/ENABLE_SDB=yes/) }
end

context 'passing `no` to enable_sdb' do
let(:params) {{ :enable_sdb => 'no' }}
it { should contain_file('/etc/sysconfig/named').with_content(/ENABLE_SDB=no/) }
end

context 'passing `1` to enable_sdb' do
let(:params) {{ :enable_sdb => '1' }}
it { should contain_file('/etc/sysconfig/named').with_content(/ENABLE_SDB=1/) }
end

context 'passing `0` to enable_sdb' do
let(:params) {{ :enable_sdb => '0' }}
it { should contain_file('/etc/sysconfig/named').with_content(/ENABLE_SDB=0/) }
end

context 'passing `yes` to disable_named_dbus' do
let(:params) {{ :disable_named_dbus => 'yes' }}
it { should contain_file('/etc/sysconfig/named').with_content(/DISABLE_NAMED_DBUS=yes/) }
end

context 'passing `no` to disable_named_dbus' do
let(:params) {{ :disable_named_dbus => 'no' }}
it { should contain_file('/etc/sysconfig/named').with_content(/DISABLE_NAMED_DBUS=no/) }
end

context 'passing path `/usr/local/samba/private/dns.keytab` to keytab_file' do
let(:params) {{ :keytab_file => '/usr/local/samba/private/dns.keytab' }}
it { should contain_file('/etc/sysconfig/named').with_content(/KEYTAB_FILE="\/usr\/local\/samba\/private\/dns.keytab/) }
end

context 'passing `yes` to disable_zone_checking' do
let(:params) {{ :disable_zone_checking => 'yes' }}
it { should contain_file('/etc/sysconfig/named').with_content(/DISABLE_ZONE_CHECKING=yes/) }
end

context 'passing `no` to disable_zone_checking' do
let(:params) {{ :disable_zone_checking => 'no' }}
it { should contain_file('/etc/sysconfig/named').with_content(/DISABLE_ZONE_CHECKING=no/) }
end

end

context "passing wrong values and paths" do

context 'passing wrong value to rootdir for hit an error' do
let(:params) {{ :rootdir => 'chroot' }}
it{ should raise_error(/"chroot" is not an absolute path./)}
end

context 'passing wrong value to enable_zone_write for hit an error' do
let(:params) {{ :enable_zone_write => 'WrongValue' }}
it{ should raise_error(/The enable_zone_write value is not type of a string yes \/ no./)}
end

context 'passing wrong value to enable_sdb for hit an error' do
let(:params) {{ :enable_sdb => 'WrongValue' }}
it{ should raise_error(/The enable_sdb value is not type of a string yes \/ no \/ 1 \/ 0 or empty./)}
end

context 'passing wrong value to keytab_file for hit an error' do
let(:params) {{ :keytab_file => 'usr/local/samba/private/dns.keytab' }}
it{ should raise_error(/"usr\/local\/samba\/private\/dns.keytab" is not an absolute path./)}
end

context 'passing wrong value to disable_zone_checking for hit an error' do
let(:params) {{ :disable_zone_checking => 'chroot' }}
it{ should raise_error(/The disable_zone_checking value is not type of a string yes \/ no or empty./)}
end

end

end

end
13 changes: 13 additions & 0 deletions templates/default.debian.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# run resolvconf?
<% if @resolvconf -%>
RESOLVCONF=<%= @resolvconf %>
<% else %>
RESOLVCONF= no
<% end -%>

# startup options for the server
<% if @options -%>
OPTIONS="<%= @options %>"
<% else %>
OPTIONS="-u bind"
<% end -%>
62 changes: 62 additions & 0 deletions templates/default.redhat.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# BIND named process options
# ~~~~~~~~~~~~~~~~~~~~~~~~~~
# Currently, you can use the following options:
#
# ROOTDIR="/some/where" -- will run named in a chroot environment.
# you must set up the chroot environment
# (install the bind-chroot package) before
# doing this.
#
# OPTIONS="whatever" -- These additional options will be passed to named
# at startup. Don't add -t here, use ROOTDIR instead.
#
# ENABLE_ZONE_WRITE=yes -- If SELinux is disabled, then allow named to write
# its zone files and create files in its $ROOTDIR/var/named
# directory, necessary for DDNS and slave zone transfers.
# Slave zones should reside in the $ROOTDIR/var/named/slaves
# directory, in which case you would not need to enable zone
# writes. If SELinux is enabled, you must use only the
# 'named_write_master_zones' variable to enable zone writes.
#
# ENABLE_SDB=yes -- This enables use of 'named_sdb', which has support
# -- for the ldap, pgsql and dir zone database backends
# -- compiled in, to be used instead of named.
#
# DISABLE_NAMED_DBUS=[1y]-- If NetworkManager is enabled in any runlevel, then
# the initscript will by default enable named's D-BUS
# support with the named -D option. This setting disables
# this behavior.
#
# KEYTAB_FILE="/dir/file" -- Specify named service keytab file (for GSS-TSIG)
#
# DISABLE_ZONE_CHECKING -- By default, initscript calls named-checkzone
# utility for every zone to ensure all zones are
# valid before named starts. If you set this option
# to 'yes' then initscript doesn't perform those checks.
<% if @rootdir -%>
ROOTDIR="<%= @rootdir %>"
<% end -%>

<% if @options -%>
OPTIONS="<%= @options %>"
<% end -%>

<% if @enable_zone_write -%>
ENABLE_ZONE_WRITE=<%= @enable_zone_write %>
<% end -%>

<% if @enable_sdb -%>
ENABLE_SDB=<%= @enable_sdb %>
<% end -%>

<% if @disable_named_dbus -%>
DISABLE_NAMED_DBUS=<%= @disable_named_dbus %>
<% end -%>

<% if @keytab_file -%>
KEYTAB_FILE="<%= @keytab_file %>"
<% end -%>

<% if @disable_zone_checking -%>
DISABLE_ZONE_CHECKING=<%= @disable_zone_checking %>
<% end -%>