Merge pull request redhat-openstack#230 from duritong/set_group_for_f…

…ragments set group of fragments to gid
xbezdick · Mar 2, 2015 · a3d8632 · a3d8632
2 parents eb04c55 + 0aa6c63
commit a3d8632
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 5 deletions.
diff --git a/manifests/fragment.pp b/manifests/fragment.pp
@@ -72,8 +72,9 @@
   $safe_target_name = regsubst($target, '[/:\n]', '_', 'GM')
   $concatdir        = $concat::setup::concatdir
   $fragdir          = "${concatdir}/${safe_target_name}"
-  $fragowner            = $concat::setup::fragment_owner
-  $fragmode             = $concat::setup::fragment_mode
+  $fragowner        = $concat::setup::fragment_owner
+  $fraggroup        = $concat::setup::fragment_group
+  $fragmode         = $concat::setup::fragment_mode
 
   # The file type's semantics are problematic in that ensure => present will
   # not over write a pre-existing symlink.  We are attempting to provide
@@ -110,11 +111,10 @@
     crit('No content, source or symlink specified')
   }
 
-  # punt on group ownership until some point in the distant future when $::gid
-  # can be relied on to be present
   file { "${fragdir}/fragments/${order}_${safe_name}":
     ensure  => $safe_ensure,
     owner   => $fragowner,
+    group   => $fraggroup,
     mode    => $fragmode,
     source  => $source,
     content => $content,

diff --git a/manifests/setup.pp b/manifests/setup.pp
@@ -20,10 +20,16 @@
     fail ('$concat_basedir not defined. Try running again with pluginsync=true on the [master] and/or [main] section of your node\'s \'/etc/puppet/puppet.conf\'.')
   }
 
-  # owner and mode of fragment files (on windows owner and access rights should
+  # owner,group and mode of fragment files (on windows owner and access rights should
   # be inherited from concatdir and not explicitly set to avoid problems)
   $fragment_owner = $::osfamily ? { 'windows' => undef, default => $::id }
   $fragment_mode  = $::osfamily ? { 'windows' => undef, default => '0640' }
+  # test on gid fact availability to support older facter versions
+  if defined('$gid') and $::gid and $::osfamily != 'Windows' {
+    $fragment_group = $::gid
+  } else {
+    $fragment_group = undef
+  }
 
   $script_name = 'concatfragments.rb'
 

diff --git a/spec/unit/defines/concat_fragment_spec.rb b/spec/unit/defines/concat_fragment_spec.rb
@@ -17,6 +17,7 @@
     concatdir        = '/var/lib/puppet/concat'
     fragdir          = "#{concatdir}/#{safe_target_name}"
     id               = 'root'
+    gid              = 'root'
     if p[:ensure] == 'absent'
       safe_ensure = p[:ensure] 
     else
@@ -28,6 +29,7 @@
       {
         :concat_basedir => concatdir,
         :id             => id,
+        :gid            => gid,
         :osfamily       => 'Debian',
         :path           => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin',
         :is_pe          => false,
@@ -44,6 +46,7 @@
       should contain_file("#{fragdir}/fragments/#{p[:order]}_#{safe_name}").with({
         :ensure  => safe_ensure,
         :owner   => id,
+        :group   => gid,
         :mode    => '0640',
         :source  => p[:source],
         :content => p[:content],
@@ -204,6 +207,7 @@
           :osfamily       => 'Debian',
           :id             => 'root',
           :is_pe          => false,
+          :gid            => 'root',
         }
       end
       let(:params) do
@@ -227,6 +231,7 @@
           :osfamily       => 'Debian',
           :id             => 'root',
           :is_pe          => false,
+          :gid            => 'root',
         }
       end
       let(:params) do
@@ -250,6 +255,7 @@
           :osfamily       => 'Debian',
           :id             => 'root',
           :is_pe          => false,
+          :gid            => 'root',
         }
       end
       let(:params) do