Merge pull request #307 from bmjen/ensure-newline

readd ensure_newline param and tests for backwards compatibility

lib/puppet/type/concat_file.rb

@@ -10,13 +10,14 @@
     example:
       Concat_fragment <<| tag == 'unique_tag' |>>
 
-      concat_file { '/tmp/file:
-        tag   => 'unique_tag', # Mandatory
-        path  => '/tmp/file',  # Optional. If given it overrides the resource name
-        owner => 'root',       # Optional. Default to undef
-        group => 'root',       # Optional. Default to undef
-        mode  => '0644'        # Optional. Default to undef
-        order => 'numeric'     # Optional, Default to 'numeric'
+      concat_file { '/tmp/file':
+        tag            => 'unique_tag', # Mandatory
+        path           => '/tmp/file',  # Optional. If given it overrides the resource name
+        owner          => 'root',       # Optional. Default to undef
+        group          => 'root',       # Optional. Default to undef
+        mode           => '0644'        # Optional. Default to undef
+        order          => 'numeric'     # Optional, Default to 'numeric'
+        ensure_newline => false         # Optional, Defaults to false
       }
   "
   ensurable do
@@ -75,6 +76,11 @@ def exists?
     desc "Validates file."
   end
 
+  newparam(:ensure_newline) do
+    desc "Whether to ensure there is a newline after each fragment."
+    defaultto false
+  end
+
   autorequire(:concat_fragment) do
     catalog.resources.collect do |r|
       if r.is_a?(Puppet::Type.type(:concat_fragment)) && r[:tag] == self[:tag]
@@ -134,6 +140,11 @@ def fragment_content(r)
       tmp = Puppet::FileServing::Content.indirection.find(@source, :environment => catalog.environment)
       fragment_content = tmp.content unless tmp.nil?
     end
+
+    if self[:ensure_newline]
+      fragment_content<<"\n"
+    end
+
     fragment_content
   end
 

manifests/init.pp

@@ -16,7 +16,7 @@
 #   Who will own the file
 # [*mode*]
 #   The mode of the final file
-# [*warn_header*]
+# [*warn*]
 #   Adds a normal shell style comment top of the file indicating that it is
 #   built by puppet
 # [*backup*]
@@ -27,6 +27,13 @@
 # [*order*]
 #   Select whether to order associated fragments by 'alpha' or 'numeric'.
 #   Defaults to 'alpha'.
+# [*ensure_newline*]
+#   Specifies whether to ensure there's a new line at the end of each fragment.
+#   Valid options: 'true' and 'false'. Default value: 'false'.
+# [*validate_cmd*]
+#   Specifies a validation command to apply to the destination file.
+#   Requires Puppet version 3.5 or newer. Valid options: a string to be passed
+#   to a file resource. Default value: undefined.
 #
 
 define concat(
@@ -40,6 +47,7 @@
   $backup         = 'puppet',
   $replace        = true,
   $order          = 'alpha',
+  $ensure_newline = false,
   $validate_cmd   = undef,
 ) {
   validate_re($ensure, '^present$|^absent$')
@@ -55,6 +63,8 @@
   }
   validate_bool($replace)
   validate_re($order, '^alpha$|^numeric$')
+  validate_bool($ensure_newline)
+
   if $validate_cmd and ! is_string($validate_cmd) {
     fail('$validate_cmd must be a string')
   }
@@ -83,15 +93,16 @@
 
   if $ensure == 'present' {
     concat_file { $name:
-      tag          => $safe_name,
-      path         => $path,
-      owner        => $owner,
-      group        => $group,
-      mode         => $mode,
-      replace      => $replace,
-      backup       => $backup,
-      order        => $order,
-      validate_cmd => $validate_cmd,
+      tag            => $safe_name,
+      path           => $path,
+      owner          => $owner,
+      group          => $group,
+      mode           => $mode,
+      replace        => $replace,
+      backup         => $backup,
+      order          => $order,
+      ensure_newline => $ensure_newline,
+      validate_cmd   => $validate_cmd,
     }
 
     if $_append_header {

spec/acceptance/newline_spec.rb

@@ -0,0 +1,67 @@
+require 'spec_helper_acceptance'
+
+describe 'concat ensure_newline parameter' do
+  basedir = default.tmpdir('concat')
+  context '=> false' do
+    before(:all) do
+      pp = <<-EOS
+        file { '#{basedir}':
+          ensure => directory
+        }
+      EOS
+
+      apply_manifest(pp)
+    end
+    pp = <<-EOS
+      concat { '#{basedir}/file':
+        ensure_newline => false,
+      }
+      concat::fragment { '1':
+        target  => '#{basedir}/file',
+        content => '1',
+      }
+      concat::fragment { '2':
+        target  => '#{basedir}/file',
+        content => '2',
+      }
+    EOS
+
+    it 'applies the manifest twice with no stderr' do
+      apply_manifest(pp, :catch_failures => true)
+      apply_manifest(pp, :catch_changes => true)
+    end
+
+    describe file("#{basedir}/file") do
+      it { should be_file }
+      its(:content) { should match '12' }
+    end
+  end
+
+  context '=> true' do
+    pp = <<-EOS
+      concat { '#{basedir}/file':
+        ensure_newline => true,
+      }
+      concat::fragment { '1':
+        target  => '#{basedir}/file',
+        content => '1',
+      }
+      concat::fragment { '2':
+        target  => '#{basedir}/file',
+        content => '2',
+      }
+    EOS
+
+    it 'applies the manifest twice with no stderr' do
+      apply_manifest(pp, :catch_failures => true)
+      apply_manifest(pp, :catch_changes => true)
+    end
+
+    describe file("#{basedir}/file") do
+      it { should be_file }
+      its(:content) {
+        should match /1\n2\n/
+      }
+    end
+  end
+end

spec/unit/defines/concat_spec.rb

@@ -248,4 +248,36 @@
       end
     end
   end # order =>
+
+  context 'ensure_newline =>' do
+    [true, false].each do |ensure_newline|
+      context 'true' do
+        it_behaves_like 'concat', '/etc/foo.bar', { :ensure_newline => ensure_newline}
+      end
+    end
+
+    context '123' do
+      let(:title) { '/etc/foo.bar' }
+      let(:params) {{ :ensure_newline => 123 }}
+      it 'should fail' do
+        expect { catalogue }.to raise_error(Puppet::Error, /is not a boolean/)
+      end
+    end
+  end # ensure_newline =>
+
+  context 'validate_cmd =>' do
+    context '/usr/bin/test -e %' do
+      it_behaves_like 'concat', '/etc/foo.bar', { :validate_cmd => '/usr/bin/test -e %' }
+    end
+
+    [ 1234, true ].each do |cmd|
+      context cmd do
+        let(:title) { '/etc/foo.bar' }
+        let(:params) {{ :validate_cmd => cmd }}
+        it 'should fail' do
+          expect { catalogue }.to raise_error(Puppet::Error, /\$validate_cmd must be a string/)
+        end
+      end
+    end
+  end # validate_cmd =>
 end