Legacy ERB template support (#7)

* Add legacy ERB template support through introduction of config_epp and step_tickers_epp parameters

* Changes made to the readme. config_template & config_epp tests now test valid EPP and ERB templates with differing content. Fixed some code styling issues.

README.markdown

@@ -109,7 +109,7 @@ class { '::ntp':
   servers         => [ 'ntp1.corp.com', 'ntp2.corp.com' ],
   restrict        => ['127.0.0.1'],
   service_manage  => false,
-  config_template => 'different/module/custom.template.epp',
+  config_epp      => 'different/module/custom.template.epp',
 }
 ```
 
@@ -150,7 +150,11 @@ Specifies a file mode for the ntp configuration file. Valid options: string cont
 
 ####`config_template`
 
-Specifies a file to act as a template for the config file. Valid options: string containing a path (absolute, or relative to the module path). Default value: 'ntp/ntp.conf.epp'
+Specifies a file to act as a ERB template for the config file. Valid options: string containing a path (absolute, or relative to the module path). Example value: 'ntp/ntp.conf.erb'. Validation error will be thrown if this param is supplied as well as the `config_epp` param.
+
+####`config_epp`
+
+Specifies a file to act as a EPP template for the config file. Valid options: string containing a path (absolute, or relative to the module path). Example value: 'ntp/ntp.conf.epp'. Validation error will be thrown if this param is supplied as well as the `config_template` param.
 
 ####`disable_auth`
 
@@ -315,7 +319,11 @@ Location of the step tickers file on the managed system. Default value: varies b
 
 ####`step_tickers_template`
 
-Location of the step tickers template file. Default value: varies by operating system
+Location of the step tickers ERB template file. Default value: varies by operating system. Validation error will be thrown if this is specified as well as the `step_tickers_epp` param.
+
+####`step_tickers_epp`
+
+Location of the step tickers EPP template file. Default value: varies by operating system. Validation error will be thrown if this is specified as well as the `step_tickers_template` param.
 
 ####`stepout`
 

data/common.yaml

@@ -3,8 +3,9 @@ ntp::authprov: ~
 ntp::broadcastclient: false
 ntp::config_dir: ~
 ntp::config_file_mode: '0644'
-ntp::config_template: 'ntp/ntp.conf.epp'
 ntp::config: '/etc/ntp.conf'
+ntp::config_template: ~
+ntp::config_epp: ~
 ntp::disable_auth: false
 ntp::disable_dhclient: false
 ntp::disable_kernel: false
@@ -43,7 +44,8 @@ ntp::service_name: ntpd
 ntp::service_provider: ~
 ntp::stepout: ~
 ntp::step_tickers_file: ~
-ntp::step_tickers_template: 'ntp/step-tickers.epp'
+ntp::step_tickers_template: ~
+ntp::step_tickers_epp: ~
 ntp::tinker: ~
 ntp::tos_ceiling: 15
 ntp::tos_cohort: 0

manifests/config.pp

@@ -31,21 +31,46 @@
     }
   }
 
+  #If both epp and erb are defined, throw validation error.
+  #Otherwise use the defined erb/epp template, or use default
+  if $ntp::config_epp and $ntp::config_template {
+    fail('Cannot supply both config_epp and config_template templates for ntp config file.')
+  }elsif $ntp::config_template {
+    $config_content = template($ntp::config_template)
+  }elsif $ntp::config_epp {
+    $config_content = epp($ntp::config_epp)
+  }else {
+    $config_content = epp('ntp/ntp.conf.epp')
+  }
+
   file { $ntp::config:
     ensure  => file,
     owner   => 0,
     group   => 0,
     mode    => $::ntp::config_file_mode,
-    content => epp($ntp::config_template),
+    content => $config_content,
   }
 
+  #If both epp and erb are defined, throw validation error.
+  #Otherwise use the defined erb/epp template, or use default
+
   if $::ntp::step_tickers_file {
+    if $::ntp::step_tickers_template and $::ntp::step_tickers_epp {
+      fail('Cannot supply both step_tickers_file and step_tickers_epp templates for step ticker file')
+    } elsif $::ntp::step_tickers_template {
+      $step_ticker_content = template($ntp::step_tickers_template)
+    } elsif $::ntp::step_tickers_epp {
+      $step_ticker_content = epp($::ntp::step_tickers_epp)
+    } else{
+      $step_ticker_content = epp('ntp/step-tickers.epp')
+    }
+
     file { $::ntp::step_tickers_file:
       ensure  => file,
       owner   => 0,
       group   => 0,
       mode    => $::ntp::config_file_mode,
-      content => epp($ntp::step_tickers_template),
+      content => $step_ticker_content,
     }
   }
 

manifests/init.pp

@@ -3,7 +3,8 @@
   Tea::Absolutepath $config,
   Optional[Tea::Absolutepath] $config_dir,
   String $config_file_mode,
-  String[1] $config_template,
+  Optional[String] $config_epp,
+  Optional[String] $config_template,
   Boolean $disable_auth,
   Boolean $disable_dhclient,
   Boolean $disable_kernel,
@@ -39,6 +40,7 @@
   Optional[Integer[0, 65535]] $stepout,
   Optional[Tea::Absolutepath] $step_tickers_file,
   Optional[String] $step_tickers_template,
+  Optional[String] $step_tickers_epp,
   Optional[Boolean] $tinker,
   Boolean $tos,
   Optional[Integer[1]] $tos_minclock,

spec/acceptance/ntp_parameters_spec.rb

@@ -74,17 +74,49 @@
     before :all do
       modulepath = default['distmoduledir']
       shell("mkdir -p #{modulepath}/test/templates")
-      shell("echo 'testcontent' >> #{modulepath}/test/templates/ntp.conf.epp")
+      #The looping over an array of length 1 is unnecessary, however the ERB/EPP
+      #Syntax differs for this scenario
+      shell("echo '<% [1].each do |i| %>erbserver<%= i %><%end %>' >> #{modulepath}/test/templates/ntp.conf.erb")
     end
 
-    it 'sets the ntp.conf location' do
-      pp = "class { 'ntp': config_template => 'test/ntp.conf.epp' }"
+    it 'sets the ntp.conf erb template location' do
+      pp = "class { 'ntp': config_template => 'test/ntp.conf.erb' }"
+      apply_manifest(pp, :catch_failures => true)
+    end
+
+    describe file("#{config}") do
+      it { should be_file }
+      its(:content) { should match 'erbserver1' }
+    end
+
+    it 'sets the ntp.conf epp template location and the ntp.conf erb template location which should fail' do
+      pp = "class { 'ntp': config_template => 'test/ntp.conf.erb', config_epp => 'test/ntp.conf.epp' }"
+      expect(apply_manifest(pp, :expect_failures => true).stderr).to match(/Cannot supply both config_epp and config_template/i)
+    end
+  end
+
+  describe 'config_epp' do
+    before :all do
+      modulepath = default['distmoduledir']
+      shell("mkdir -p #{modulepath}/test/templates")
+      #The looping over an array of length 1 is unnecessary, however the ERB/EPP
+      #Syntax differs for this scenario
+      shell("echo '<% [1].each |$i| { -%>eppserver<%= $i %><% } -%>' >> #{modulepath}/test/templates/ntp.conf.epp")
+    end
+
+    it 'sets the ntp.conf epp template location' do
+      pp = "class { 'ntp': config_epp => 'test/ntp.conf.epp' }"
       apply_manifest(pp, :catch_failures => true)
     end
 
     describe file("#{config}") do
       it { should be_file }
-      its(:content) { should match 'testcontent' }
+      its(:content) { should match 'eppserver1' }
+    end
+
+    it 'sets the ntp.conf epp template location and the ntp.conf erb template location which should fail' do
+      pp = "class { 'ntp': config_template => 'test/ntp.conf.erb', config_epp => 'test/ntp.conf.epp' }"
+      expect(apply_manifest(pp, :expect_failures => true).stderr).to match(/Cannot supply both config_epp and config_template/i)
     end
   end
 

spec/classes/ntp_spec.rb

@@ -30,11 +30,14 @@
           it { should contain_file('/var/run/ntp/servers-netconfig').with_ensure_absent }
         end
 
-        describe 'allows template to be overridden' do
-          let(:params) {{ :config_template => 'my_ntp/ntp.conf.epp' }}
-          it { should contain_file('/etc/ntp.conf').with({
-            'content' => /server foobar/})
-          }
+        describe 'allows template to be overridden with erb template' do
+          let(:params) {{ :config_template => 'my_ntp/ntp.conf.erb' }}
+          it { should contain_file('/etc/ntp.conf').with_content(/erbserver1/) }
+        end
+
+        describe 'allows template to be overridden with epp template' do
+          let(:params) {{ :config_epp => 'my_ntp/ntp.conf.epp' }}
+          it { should contain_file('/etc/ntp.conf').with_content(/eppserver1/) }
         end
 
         describe 'keys' do

spec/fixtures/my_ntp/templates/ntp.conf.epp

@@ -1,4 +1 @@
-#my uber ntp config
-#
-
-server foobar
+<% [1].each |$i| { -%>eppserver<%= $i %><% } -%>

spec/fixtures/my_ntp/templates/ntp.conf.erb

@@ -0,0 +1 @@
+<% [1].each do |i| %>erbserver<%= i %><%end %>