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.

Sign up for GitHub

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

Update zone-serial only on changing zone-records #42

Closed
wants to merge 1 commit into from
Closed

Update zone-serial only on changing zone-records #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions manifests/record.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
$order = 9
) {

$zone_file = "/etc/bind/zones/db.${zone}"
$cfg_dir = $dns::server::params::cfg_dir

$zone_file_data = "${cfg_dir}/zones/db.${zone}.data"

concat::fragment{"db.${zone}.${name}.record":
target => $zone_file,
target => $zone_file_data,
order => $order,
content => template("${module_name}/zone_record.erb")
}
Expand Down
24 changes: 21 additions & 3 deletions manifests/server/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,38 @@
owner => $owner,
group => $group,
mode => '0755',
require => File["${cfg_dir}"],
}
file { "${cfg_dir}/bind.keys.d/":
ensure => directory,
owner => $owner,
group => $group,
mode => '0755',
require => File["${cfg_dir}"],
}

file { "${cfg_dir}/puppet-scripts":
ensure => directory,
owner => $owner,
group => $group,
mode => '0755',
require => File["${cfg_dir}"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't have to have all these requires. Puppet will auto-require a parent directory.

}
file { "${cfg_dir}/puppet-scripts/zone_soa.sh":
ensure => present,
owner => $owner,
group => $group,
mode => '0755',
content => template("${module_name}/zone_soa.sh.erb"),
require => File["${cfg_dir}/puppet-scripts"],
}

file { "${cfg_dir}/named.conf":
ensure => present,
owner => $owner,
group => $group,
mode => '0644',
require => [File['/etc/bind'], Class['dns::server::install']],
require => [File["${cfg_dir}"], Class['dns::server::install']],
notify => Class['dns::server::service'],
}

Expand All @@ -33,14 +51,14 @@
group => $group,
mode => '0644',
require => Class['concat::setup'],
notify => Class['dns::server::service']
notify => Class['dns::server::service'],
}

concat::fragment{'named.conf.local.header':
ensure => present,
target => "${cfg_dir}/named.conf.local",
order => 1,
content => "// File managed by Puppet.\n"
content => "// File managed by Puppet.\n",
}

}
53 changes: 39 additions & 14 deletions manifests/zone.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

validate_array($allow_transfer)

$cfg_dir = $dns::server::params::cfg_dir

$zone_serial = $serial ? {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can set serial of zone manually (compatibility issue), but serial number will be updated only if records changed.

Probably we need to drop support of setting serial number by hand in future.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea, in this PR you can drop support for setting the serial by hand. It doesn't make sense to have that when under puppet control.

false => inline_template('<%= Time.now.to_i %>'),
default => $serial
Expand All @@ -28,26 +30,49 @@
default => $name
}

$zone_file = "/etc/bind/zones/db.${name}"
$zone_dir = "${cfg_dir}/zones"
$zone_file = "${zone_dir}/db.${name}"
$zone_file_data = "${zone_file}.data"

if $ensure == absent {
file { $zone_file:
ensure => absent,
}
} else {
# Zone Database
concat { $zone_file:
owner => 'bind',
group => 'bind',
mode => '0644',
require => [Class['concat::setup'], Class['dns::server']],
notify => Class['dns::server::service']
}
concat::fragment{"db.${name}.soa":
target => $zone_file,
order => 1,
content => template("${module_name}/zone_file.erb")
}
# Splitting zone file on two parts: zone-soa (soa record) and zone-data (all other records).
# This is to update zone_serial only by need (then zone-data updated).

$soa_exec_cmdline = template("${module_name}/soa_exec_cmdline.erb")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this place i create command line for zone_soa.sh (it's necessary because we have unknown numbers of nameservers, i.e. different number of arguments for script).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know about this. I kinda liked your previous approach better.

This approach is pretty confusing:

  • Puppet generates a command + args to generate... from a erb template
  • Puppet executes those commands, which the main command is another erb template
  • That templated script is a script that echos out a the zone file components into the real zone file
  • The real zone file is used by bind.

This is pretty complicated.

What if you took a different approach:

  • Use erb + concat to build out the zone file (what we have)
  • Make that zone file zone.data, with the word SERIAL in place of the serial number
  • Make it exec a sed -i 's/SERIAL/$date_in_seconds zone.data' > zone

I think that is more straightforward, still converges, and has only 1 intermediate step?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Puppet generates a command + args to generate... from a erb template
    I did it because zone_soa.sh syntax look like:
zone_soa.sh <many params> <nameserver1> <nameserver2> ... <nameserverN>

I don't know another way to generate cmdline with a different numbers of args.

  • Puppet executes those commands, which the main command is another erb template
    In a template the path to files of zones changes only. As alternative I can pass zone_dir as a parameter to zone_soa.sh.
  • That templated script is a script that echos out a the zone file components into the real zone file
    When I used echo directly in Exec I had a row of problems: different versions of puppet do this different, and i wrote some workarounds. I am afraid that in other Puppet versions workarounds wan't work.

What if you took a different approach:

  • Make it exec a sed -i 's/SERIAL/$date_in_seconds zone.data' > zone

Hmm...
If we create real zone file via concat with SERIAL as template and notify exec with sed then next time Puppet will update zone file to set real serial to SERIAL (because the file will different from concat template).
If we create fake zone file with SERIAL as template and notify exec with sed we will have two identical files (except SERIAL) and one Exec. I don't see big difference between my variant in this case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The big difference is we are not generating a large command line. The only thing we would be executing would be sed.

Se would have concat make zonefile.staging and sed would be creating zonefile.

It is true that both methods achieve the same effect, I'm just suggesting the sed method is much easier to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Main problem in exec with echo was output redirection ">" and pipes "|". Variant with sed has similar problem :(
As variant:

$nameservers_str = inline_template("<% @nameservers.each do |nameserver| -%> <%= nameserver %><% end -%>"),
exec { "soa-${zone}":
  command => "$cfg_dir/puppet-scripts/zone_soa.sh $zone $soa $soa_email $zone_serial $zone_ttl $zone_refresh $zone_retry $zone_expire $zone_minimum $nameservers_str",
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't need a special command based on a template with lots of arguments. We only care about the serial. Other parts can be handled in the ERB like normal.

$zone_file = '/tmp/zone.staging'
$time = inline_template('<%= Time.now.to_i %>')
exec { "Bump serials on ${zone}":
  command  => "/bin/sed 's/SERIAL/${time}/' $zone_file > /tmp/zone",
  provider => 'shell',
}


exec { "soa-${zone}":
command => "$soa_exec_cmdline",
path => ["/bin", "/sbin", "/usr/bin", "/usr/sbin"],
refreshonly => true,
require => Class['dns::server::install'],
notify => Class['dns::server::service'],
}
# Set correct rights on file generated by exec{soa-$zone:}
file { $zone_file:
ensure => file,
owner => $dns::owner,
group => $dns::group,
mode => 0644,
require => Exec["soa-${zone}"],
}

# Generate ${zone_name}.data file for static zones and request refresh exec on changes
concat { $zone_file_data:
owner => $dns::owner,
group => $dns::group,
mode => '644',
require => Class['dns::server::install'],
notify => Exec["soa-${zone}"],
}
concat::fragment{"db.${zone}.data-header":
target => $zone_file_data,
order => 1,
content => template("${module_name}/zone_data_header.erb")
}
}

# Include Zone in named.conf.local
Expand Down
1 change: 1 addition & 0 deletions templates/soa_exec_cmdline.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= @cfg_dir %>/puppet-scripts/zone_soa.sh <%= @zone %> <%= @soa %> <%= @soa_email %> <%= @zone_serial %> <%= @zone_ttl %> <%= @zone_refresh %> <%= @zone_retry %> <%= @zone_expire %> <%= @zone_minimum %><% @nameservers.each do |nameserver| -%> <%= nameserver %><% end -%>
6 changes: 6 additions & 0 deletions templates/zone_data_header.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
; ttl: <%= @zone_ttl %>, soa: <%= @soa %>, soa_email: <%= @soa_email %>
; zone_refresh: <%= @zone_refresh %>, zone_retry: <%= @zone_retry %>
; zone_expire: <%= @zone_expire %>, zone_minimum: <%= @zone_minimum %>
;
; nameservers: <% @nameservers.each do |nameserver| -%><%= nameserver %> <% end %>
;
17 changes: 0 additions & 17 deletions templates/zone_file.erb
View file
Open in desktop

This file was deleted.

44 changes: 44 additions & 0 deletions templates/zone_soa.sh.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#!/bin/sh

# This is "template" for Puppet. It needs to make SOA header of DNS zone.
#
# Usage:
# zone-soa.sh <zone_name> <soa> <soa_email> <zone_serial> <zone_ttl> <zone_refresh> <zone_retry> <zone_expire> <zone_minimum> <ns1> ... <nsN>

ZONE_NAME=$1
SOA=$2
SOA_EMAIL=$3
ZONE_SERIAL=$4
ZONE_TTL=$5
ZONE_REFRESH=$6
ZONE_RETRY=$7
ZONE_EXPIRE=$8
ZONE_MINIMUM=$9

cat > "<%= @cfg_dir %>/zones/db.${ZONE_NAME}" <<EOF
;
; BIND data file for $ZONE_NAME zone.
; File managed by puppet.
;
\$ORIGIN ${ZONE_NAME}.
\$TTL $ZONE_TTL
@ IN SOA ${SOA}. ${SOA_EMAIL}. (
$ZONE_SERIAL ; Serial
$ZONE_REFRESH ; Refresh
$ZONE_RETRY ; Retry
$ZONE_EXPIRE ; Expire
$ZONE_MINIMUM ) ; Negative Cache TTL
;
EOF

shift 9
while [ "$#" -gt "0" ]; do
NAMESERVER=$1
echo "@ IN NS $NAMESERVER." >> "<%= @cfg_dir %>/zones/db.${ZONE_NAME}"
shift
done

cat >> "<%= @cfg_dir %>/zones/db.${ZONE_NAME}" <<EOF
;
\$INCLUDE /etc/bind/zones/db.${ZONE_NAME}.data
EOF