files and directories are now created as the correct user and group if they are set

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@727 980ebf18-57e1-0310-9a29-db15c13687c0

7 files changed, 88 insertions, 46 deletions

diff --git a/lib/puppet/type/pfile/create.rb b/lib/puppet/type/pfile/create.rb
index f8b2f3818..9ed9b19af 100755
--- a/lib/puppet/type/pfile/create.rb
+++ b/lib/puppet/type/pfile/create.rb
@@ -38,29 +38,50 @@ module Puppet
             def sync
                 event = nil
                 mode = @parent.should(:mode)
+
+                # First, determine if a user has been specified and if so if
+                # that user has write access to the parent dir
+                asuser = nil
+                if @parent.should(:owner) and ! @parent.should(:owner).is_a?(Symbol)
+                    writeable = Puppet::Util.asuser(@parent.should(:owner)) {
+                        FileTest.writable?(File.dirname(@parent[:path]))
+                    }
+
+                    # If the parent directory is writeable, then we execute
+                    # as the user in question.  Otherwise we'll rely on
+                    # the 'owner' state to do things.
+                    if writeable
+                        asuser = @parent.should(:owner)
+                    end
+                end
                 begin
                     case self.should
                     when "file":
                         # just create an empty file
-                        if mode
-                            File.open(@parent[:path],"w", mode) {
-                            }
-                            @parent.delete(:mode)
-                        else
-                            File.open(@parent[:path],"w") {
-                            }
-                        end
+                        Puppet::Util.asuser(asuser, @parent.should(:group)) {
+                            if mode
+                                File.open(@parent[:path],"w", mode) {
+                                }
+                            else
+                                File.open(@parent[:path],"w") {
+                                }
+                            end
+                        }
                         event = :file_created
                     when "directory":
-                        if mode
-                            Dir.mkdir(@parent.name,mode)
-                            @parent.delete(:mode)
-                        else
-                            Dir.mkdir(@parent.name)
-                        end
+                        Puppet::Util.asuser(asuser) {
+                            if mode
+                                Dir.mkdir(@parent.name,mode)
+                            else
+                                Dir.mkdir(@parent.name)
+                            end
+                        }
                         event = :directory_created
                     when :notfound:
                         # this is where the file should be deleted...
+
+                        # This value is only valid when we're rolling back a creation,
+                        # so we verify that the file has not been modified since then.
                         unless FileTest.size(@parent.name) == 0
                             raise Puppet::Error.new(
                                 "Created file %s has since been modified; cannot roll back."
diff --git a/lib/puppet/type/pfile/group.rb b/lib/puppet/type/pfile/group.rb
index 0e16af4ad..65acb3d00 100755
--- a/lib/puppet/type/pfile/group.rb
+++ b/lib/puppet/type/pfile/group.rb
@@ -97,29 +97,19 @@ module Puppet
             # we'll just let it fail, but we should probably set things up so
             # that users get warned if they try to change to an unacceptable group.
             def sync
-                # now make sure the user is allowed to change to that group
-                # We don't do this in the should section, so it can still be used
-                # for noop.
-                unless Process.uid == 0
-                    unless defined? @@notifiedgroup
-                        Puppet.notice(  
-                            "Cannot manage group ownership unless running as root"
-                        )
-                        @@notifiedgroup = true
-                    end
-                    return nil
-                end
-
                 if @is == :notfound
                     @parent.stat(true)
                     self.retrieve
-                    #Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
-                end
 
-                unless @parent.stat
-                    Puppet.err "File '%s' does not exist; cannot chgrp" %
-                        @parent[:path]
-                    return nil
+                    if @is == :notfound
+                        Puppet.err "File '%s' does not exist; cannot chgrp" %
+                            @parent[:path]
+                        return nil
+                    end
+
+                    if self.insync?
+                        return nil
+                    end
                 end
 
                 begin
diff --git a/lib/puppet/type/pfile/uid.rb b/lib/puppet/type/pfile/uid.rb
index 4ac7523bf..4a03530b0 100755
--- a/lib/puppet/type/pfile/uid.rb
+++ b/lib/puppet/type/pfile/uid.rb
@@ -100,15 +100,17 @@ module Puppet
                 if @is == :notfound
                     @parent.stat(true)
                     self.retrieve
+                    if @is == :notfound
+                        Puppet.err "File '%s' does not exist; cannot chown" %
+                            @parent[:path]
+                        return nil
+                    end
+                    if self.insync?
+                        return nil
+                    end
                     #Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
                 end
 
-                unless @parent.stat
-                    Puppet.err "File '%s' does not exist; cannot chown" %
-                        @parent[:path]
-                    return nil
-                end
-
                 begin
                     File.chown(self.should,nil,@parent[:path])
                 rescue => detail
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 6dd408cc5..da52791cb 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -2,9 +2,8 @@
 
 module Puppet
 module Util
-    # Execute a block as a given user, and optionally as a group
-    def self.asuser(user, group = nil)
-        # FIXME This needs to allow user, group, or both to be optional.
+    # Execute a block as a given user or group
+    def self.asuser(user = nil, group = nil)
         require 'etc'
 
         uid = nil
diff --git a/test/other/transactions.rb b/test/other/transactions.rb
index 338bb6844..738337dc4 100644
--- a/test/other/transactions.rb
+++ b/test/other/transactions.rb
@@ -114,8 +114,16 @@ class TestTransactions < Test::Unit::TestCase
 
 
         component = newcomp("file",file)
+        require 'etc'
+        groupname = Etc.getgrgid(File.stat(file.name).gid).name
         assert_nothing_raised() {
-            file[:group] = @groups[1]
+            # Find a group that it's not set to
+            group = @groups.find { |group| group != groupname }
+            unless group
+                raise "Could not find suitable group"
+            end
+            file[:group] = group
+
             file[:mode] = "755"
         }
         trans = assert_events(component, [:inode_changed, :inode_changed], "file")
diff --git a/test/puppettest.rb b/test/puppettest.rb
index cfd0b9342..3d483d4de 100644
--- a/test/puppettest.rb
+++ b/test/puppettest.rb
@@ -119,8 +119,9 @@ module TestPuppet
         run_events(:rollback, trans, events, msg)
     end
 
-    def assert_events(comp, events, msg)
+    def assert_events(comp, events, msg = nil)
         trans = nil
+        msg ||= comp.name
         assert_nothing_raised("Component %s failed" % [msg]) {
             trans = comp.evaluate
         }
diff --git a/test/types/file.rb b/test/types/file.rb
index 713dd96c1..54b290fc7 100644
--- a/test/types/file.rb
+++ b/test/types/file.rb
@@ -104,7 +104,7 @@ class TestFile < Test::Unit::TestCase
         }
     end
 
-    def test_zgroup
+    def test_group
         file = mktestfile()
         [%x{groups}.chomp.split(/ /), Process.groups].flatten.each { |group|
             assert_nothing_raised() {
@@ -116,6 +116,28 @@ class TestFile < Test::Unit::TestCase
     end
 
     if Process.uid == 0
+        def test_zcreateasuser
+            dir = tmpdir()
+
+            user = nonrootuser()
+            path = File.join(tmpdir, "createusertesting")
+            @@tmpfiles << path
+
+            file = nil
+            assert_nothing_raised {
+                file = Puppet::Type::PFile.create(
+                    :path => path,
+                    :owner => user.name,
+                    :create => true,
+                    :mode => "755"
+                )
+            }
+
+            comp = newcomp("createusertest", file)
+
+            assert_events(comp, [:file_created])
+        end
+
         def test_ownerasroot
             file = mktestfile()
 
@@ -460,7 +482,6 @@ class TestFile < Test::Unit::TestCase
     def test_filetype_retrieval
         file = nil
 
-        Puppet.err tmpdir()
         assert_nothing_raised {
             file = Puppet::Type::PFile.create(
                 :name => tmpdir(),