remove usage of resource defaults for file owner/permissions

In addition, the permissions of the fragment directory and files is now fixed and not world readable.

manifests/fragment.pp

@@ -30,7 +30,7 @@ define concat::fragment(
     $source  = undef,
     $order   = 10,
     $ensure  = 'present',
-    $mode    = '0644',
+    $mode    = '0640',
     $owner   = undef,
     $group   = undef,
     $backup  = undef

manifests/init.pp

@@ -125,20 +125,18 @@ define concat(
   }
 
   File {
-    owner   => $owner,
-    group   => $group,
-    mode    => $mode,
-    replace => $replace,
     backup  => false,
   }
 
   if $ensure == 'present' {
     file { $fragdir:
       ensure => directory,
+      mode   => '0750',
     }
 
     file { "${fragdir}/fragments":
       ensure  => directory,
+      mode    => '0750',
       force   => true,
       ignore  => ['.svn', '.git', '.gitignore'],
       notify  => Exec["concat_${name}"],
@@ -148,18 +146,24 @@ define concat(
 
     file { "${fragdir}/fragments.concat":
       ensure => present,
+      mode   => '0640',
     }
 
     file { "${fragdir}/${concat_name}":
       ensure => present,
+      mode   => '0640',
     }
 
     file { $name:
-      ensure => present,
-      path   => $path,
-      alias  => "concat_${name}",
-      source => "${fragdir}/${concat_name}",
-      backup => $backup,
+      ensure  => present,
+      owner   => $owner,
+      group   => $group,
+      mode    => $mode,
+      replace => $replace,
+      path    => $path,
+      alias   => "concat_${name}",
+      source  => "${fragdir}/${concat_name}",
+      backup  => $backup,
     }
 
     # remove extra whitespace from string interpolation to make testing easier

manifests/setup.pp

@@ -37,6 +37,8 @@ class concat::setup {
   }
 
   file { $script_path:
+    ensure => file,
+    owner  => $::id,
     mode   => '0755',
     source => "puppet:///modules/concat/${script_name}",
   }

spec/system/concat_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper_system'
 
 describe 'basic concat test' do
 
-  shared_examples 'concat' do |pp|
+  shared_examples 'successfully_applied' do |pp|
     context puppet_apply(pp) do
       its(:stderr) { should be_empty }
       its(:exit_code) { should_not == 1 }
@@ -11,25 +11,47 @@ describe 'basic concat test' do
       its(:exit_code) { should be_zero }
     end
 
-    describe file('/tmp/file') do
-      it { should be_file }
-      it { should contain '1' }
-      it { should contain '2' }
-    end
-
-    # Test that all the relevant bits exist on disk after it
-    # concats.
     describe file('/var/lib/puppet/concat') do
       it { should be_directory }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 755 }
+    end
+    describe file('/var/lib/puppet/concat/bin') do
+      it { should be_directory }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 755 }
+    end
+    describe file('/var/lib/puppet/concat/bin/concatfragments.sh') do
+      it { should be_file }
+      it { should be_owned_by 'root' }
+      #it { should be_grouped_into 'root' }
+      it { should be_mode 755 }
     end
     describe file('/var/lib/puppet/concat/_tmp_file') do
       it { should be_directory }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 750 }
     end
     describe file('/var/lib/puppet/concat/_tmp_file/fragments') do
       it { should be_directory }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 750 }
     end
     describe file('/var/lib/puppet/concat/_tmp_file/fragments.concat') do
       it { should be_file }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 640 }
+    end
+    describe file('/var/lib/puppet/concat/_tmp_file/fragments.concat.out') do
+      it { should be_file }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 640 }
     end
   end
 
@@ -54,7 +76,28 @@ describe 'basic concat test' do
       }
     "
 
-    it_behaves_like 'concat', pp
+    it_behaves_like 'successfully_applied', pp
+
+    describe file('/tmp/file') do
+      it { should be_file }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 644 }
+      it { should contain '1' }
+      it { should contain '2' }
+    end
+    describe file('/var/lib/puppet/concat/_tmp_file/fragments/01_1') do
+      it { should be_file }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 640 }
+    end
+    describe file('/var/lib/puppet/concat/_tmp_file/fragments/02_2') do
+      it { should be_file }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 640 }
+    end
   end
 
   context 'owner/group non-root' do
@@ -83,6 +126,29 @@ describe 'basic concat test' do
       }
     "
 
-    it_behaves_like 'concat', pp
+    it_behaves_like 'successfully_applied', pp
+
+    describe file('/tmp/file') do
+      it { should be_file }
+      it { should be_owned_by 'bob' }
+      it { should be_grouped_into 'bob' }
+      it { should be_mode 644 }
+      it { should contain '1' }
+      it { should contain '2' }
+    end
+    describe file('/var/lib/puppet/concat/_tmp_file/fragments/01_1') do
+      it { should be_file }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 640 }
+      it { should contain '1' }
+    end
+    describe file('/var/lib/puppet/concat/_tmp_file/fragments/02_2') do
+      it { should be_file }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      it { should be_mode 640 }
+      it { should contain '2' }
+    end
   end
 end

spec/unit/defines/concat_fragment_spec.rb

@@ -10,7 +10,7 @@ describe 'concat::fragment', :type => :define do
       :source  => nil,
       :order   => 10,
       :ensure  => 'present',
-      :mode    => '0644',
+      :mode    => '0640',
       :owner   => nil,
       :group   => nil,
       :backup  => 'puppet',

spec/unit/defines/concat_spec.rb

@@ -29,11 +29,7 @@ describe 'concat', :type => :define do
     default_warn_message = '# This file is managed by Puppet. DO NOT EDIT.'
 
     file_defaults = {
-      :owner   => p[:owner],
-      :group   => p[:group],
-      :mode    => p[:mode],
       :backup  => false,
-      :replace => p[:replace],
     }
 
     let(:title) { title }
@@ -44,12 +40,14 @@ describe 'concat', :type => :define do
       it do
         should contain_file(fragdir).with(file_defaults.merge({
           :ensure => 'directory',
+          :mode   => '0750',
         }))
       end
 
       it do
         should contain_file("#{fragdir}/fragments").with(file_defaults.merge({
           :ensure  => 'directory',
+          :mode    => '0750',
           :force   => true,
           :ignore  => ['.svn', '.git', '.gitignore'],
           :purge   => true,
@@ -64,17 +62,22 @@ describe 'concat', :type => :define do
         it do
           should contain_file(file).with(file_defaults.merge({
             :ensure => 'present',
+            :mode   => '0640',
           }))
         end
       end
 
       it do
         should contain_file(title).with(file_defaults.merge({
-          :ensure => 'present',
-          :path   => p[:path],
-          :alias  => "concat_#{title}",
-          :source => "#{fragdir}/#{concat_name}",
-          :backup => p[:backup],
+          :ensure  => 'present',
+          :owner   => p[:owner],
+          :group   => p[:group],
+          :mode    => p[:mode],
+          :replace => p[:replace],
+          :path    => p[:path],
+          :alias   => "concat_#{title}",
+          :source  => "#{fragdir}/#{concat_name}",
+          :backup  => p[:backup],
         }))
       end