validate define concat::fragment params

README.markdown

@@ -154,6 +154,9 @@ Contributors:
 **Joshua Hoblitt**
 
  * Remove requirement to manually include `concat::setup` in the manifest
+ * Style improvements
+ * Parameter validation / refactor parameter handling
+ * Test coverage
 
 Contact:
 --------

manifests/fragment.pp

@@ -36,6 +36,16 @@ define concat::fragment(
     $group   = undef,
     $backup  = 'puppet'
 ) {
+  validate_absolute_path($target)
+  validate_re($ensure, '^$|^present$|^absent$|^file$|^directory$')
+  validate_string($content)
+  validate_string($source)
+  validate_string($order)
+  validate_string($mode)
+  validate_string($owner)
+  validate_string($group)
+  validate_string($backup)
+
   include concat::setup
 
   $safe_name        = regsubst($name, '[/\n]', '_', 'GM')

spec/defines/concat_fragment_spec.rb

@@ -1,37 +1,237 @@
 require 'spec_helper'
 
-describe 'concat::fragment', :type => :defne do
-  let(:title) { 'motd_header' }
+describe 'concat::fragment', :type => :define do
 
-  let(:facts) do
-    {
-     :concat_basedir => '/var/lib/puppet/concat',
-     :id             => 'root',
-    }
-  end
+  shared_examples 'fragment' do |title, params={}|
+    id = 'root'
 
-  let :pre_condition do
-    "concat{ '/etc/motd': }"
-  end
+    p = {
+      :content => nil,
+      :source  => nil,
+      :order   => 10,
+      :ensure  => 'present',
+      :mode    => '0644',
+      :owner   => id,
+      :group   => nil,
+      :backup  => 'puppet',
+    }.merge(params)
+
+    safe_name        = title.gsub(/[\/\n]/, '_')
+    safe_target_name = p[:target].gsub(/[\/\n]/, '_')
+    concatdir        = '/var/lib/puppet/concat'
+    fragdir          = "#{concatdir}/#{safe_target_name}"
+    if p[:group].nil?
+      safe_group = id == 'root' ? 0 : id
+    else
+      safe_group = p[:group]
+    end
+
+    let(:title) { title }
+    let(:facts) do
+      {
+       :concat_basedir => concatdir,
+       :id             => id,
+      }
+    end
+    let(:params) { params }
+    let(:pre_condition) do
+      "concat{ '#{p[:target]}': }"
+    end
 
-  context 'target => /etc/motd' do
-    let(:params) {{ :target => '/etc/motd' }}
     it do
       should contain_class('concat::setup')
-      should contain_concat('/etc/motd')
-      should contain_concat__fragment('motd_header').with({
-        :target => '/etc/motd',
-      })
-      should contain_file('/var/lib/puppet/concat/_etc_motd/fragments/10_motd_header' ).with({
-        :ensure  => 'present',
-        :mode    => '0644',
-        :owner   => 'root',
-        :group   => 0,
-        :source  => nil,
-        :content => nil,
-        :backup  => 'puppet',
-        :alias   => 'concat_fragment_motd_header',
+      should contain_concat(p[:target])
+      should contain_file("#{fragdir}/fragments/#{p[:order]}_#{safe_name}").with({
+        :ensure  => p[:ensure],
+        :mode    => p[:mode],
+        :owner   => p[:owner],
+        :group   => safe_group,
+        :source  => p[:source],
+        :content => p[:content],
+        :backup  => p[:backup],
+        :alias   => "concat_fragment_#{title}",
       })
     end
   end
+
+  context 'target =>' do
+    context '/etc/motd' do
+      it_behaves_like 'fragment', 'motd_header', {
+        :target  => '/etc/motd',
+      }
+    end
+
+    ['./etc/motd', 'etc/motd', false].each do |target|
+      context target do
+        let(:title) { 'motd_header' }
+        let(:facts) {{ :concat_basedir => '/tmp', :id => 'root' }}
+        let(:params) {{ :target => target }}
+
+        it 'should fail' do
+          expect { should }.to raise_error(Puppet::Error, /is not an absolute path/)
+        end
+      end
+    end
+  end # target =>
+
+  context 'ensure =>' do
+    ['', 'present', 'absent', 'file', 'directory'].each do |ens|
+      context ens do
+        it_behaves_like 'fragment', 'motd_header', {
+          :ensure => ens,
+          :target => '/etc/motd',
+        }
+      end
+    end
+
+    context 'invalid' do
+      let(:title) { 'motd_header' }
+      let(:facts) {{ :concat_basedir => '/tmp', :id => 'root' }}
+      let(:params) {{ :ensure => 'invalid', :target => '/etc/motd' }}
+
+      it 'should fail' do
+        expect { should }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^$|^present$|^absent$|^file$|^directory$"')}/)
+      end
+    end
+  end # ensure =>
+
+  context 'content =>' do
+    ['', 'ashp is our hero'].each do |content|
+      context content do
+        it_behaves_like 'fragment', 'motd_header', {
+          :content => content,
+          :target  => '/etc/motd',
+        }
+      end
+    end
+
+    context 'false' do
+      let(:title) { 'motd_header' }
+      let(:facts) {{ :concat_basedir => '/tmp', :id => 'root' }}
+      let(:params) {{ :content => false, :target => '/etc/motd' }}
+
+      it 'should fail' do
+        expect { should }.to raise_error(Puppet::Error, /is not a string/)
+      end
+    end
+  end # content =>
+
+  context 'source =>' do
+    ['', '/foo/bar'].each do |source|
+      context source do
+        it_behaves_like 'fragment', 'motd_header', {
+          :source => source,
+          :target => '/etc/motd',
+        }
+      end
+    end
+
+    context 'false' do
+      let(:title) { 'motd_header' }
+      let(:facts) {{ :concat_basedir => '/tmp', :id => 'root' }}
+      let(:params) {{ :source => false, :target => '/etc/motd' }}
+
+      it 'should fail' do
+        expect { should }.to raise_error(Puppet::Error, /is not a string/)
+      end
+    end
+  end # source =>
+
+  context 'order =>' do
+    ['', '42', 'a', 'z'].each do |order|
+      context '\'\'' do
+        it_behaves_like 'fragment', 'motd_header', {
+          :order  => order,
+          :target => '/etc/motd',
+        }
+      end
+    end
+
+    context 'false' do
+      let(:title) { 'motd_header' }
+      let(:facts) {{ :concat_basedir => '/tmp', :id => 'root' }}
+      let(:params) {{ :order => false, :target => '/etc/motd' }}
+
+      it 'should fail' do
+        expect { should }.to raise_error(Puppet::Error, /is not a string/)
+      end
+    end
+  end # order =>
+
+  context 'mode =>' do
+    context '1755' do
+      it_behaves_like 'fragment', 'motd_header', {
+        :mode => '1755',
+        :target => '/etc/motd',
+      }
+    end
+
+    context 'false' do
+      let(:title) { 'motd_header' }
+      let(:facts) {{ :concat_basedir => '/tmp', :id => 'root' }}
+      let(:params) {{ :mode => false, :target => '/etc/motd' }}
+
+      it 'should fail' do
+        expect { should }.to raise_error(Puppet::Error, /is not a string/)
+      end
+    end
+  end # mode =>
+
+  context 'owner =>' do
+    context 'apenny' do
+      it_behaves_like 'fragment', 'motd_header', {
+        :owner => 'apenny',
+        :target => '/etc/motd',
+      }
+    end
+
+    context 'false' do
+      let(:title) { 'motd_header' }
+      let(:facts) {{ :concat_basedir => '/tmp', :id => 'root' }}
+      let(:params) {{ :owner => false, :target => '/etc/motd' }}
+
+      it 'should fail' do
+        expect { should }.to raise_error(Puppet::Error, /is not a string/)
+      end
+    end
+  end # owner =>
+
+  context 'group =>' do
+    context 'apenny' do
+      it_behaves_like 'fragment', 'motd_header', {
+        :group => 'apenny',
+        :target => '/etc/motd',
+      }
+    end
+
+    context 'false' do
+      let(:title) { 'motd_header' }
+      let(:facts) {{ :concat_basedir => '/tmp', :id => 'root' }}
+      let(:params) {{ :group => false, :target => '/etc/motd' }}
+
+      it 'should fail' do
+        expect { should }.to raise_error(Puppet::Error, /is not a string/)
+      end
+    end
+  end # group =>
+
+  context 'backup =>' do
+    context 'apenny' do
+      it_behaves_like 'fragment', 'motd_header', {
+        :backup => 'apenny',
+        :target => '/etc/motd',
+      }
+    end
+
+    context 'false' do
+      let(:title) { 'motd_header' }
+      let(:facts) {{ :concat_basedir => '/tmp', :id => 'root' }}
+      let(:params) {{ :backup => false, :target => '/etc/motd' }}
+
+      it 'should fail' do
+        expect { should }.to raise_error(Puppet::Error, /is not a string/)
+      end
+    end
+  end # backup =>
+
 end