Adding "links" parameter to files, and adding support for following or ignoring links to all of the states it can matter to. I still need to modify "source" so that it behaves correctly when managing links.

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

9 files changed, 181 insertions, 22 deletions

diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index 4d91805bd..cee53afbf 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -244,7 +244,11 @@ module Puppet
                 # This parameter isn't using defined values to do its work.
                 return value
             end
+
+            # We convert to a string and then a symbol so that things like
+            # booleans work as we expect.
             intern = value.to_s.intern
+
             # If it's a valid value, always return it as a symbol.
             if self.class.values.include?(intern)
                 retval = intern
diff --git a/lib/puppet/storage.rb b/lib/puppet/storage.rb
index a309de63c..2c21f9228 100644
--- a/lib/puppet/storage.rb
+++ b/lib/puppet/storage.rb
@@ -79,23 +79,11 @@ module Puppet
         def self.store
             Puppet.config.use(:puppet)
             Puppet.debug "Storing state"
-#            unless FileTest.directory?(File.dirname(Puppet[:statefile]))
-#                begin
-#                    Puppet.recmkdir(File.dirname(Puppet[:statefile]))
-#                    Puppet.info "Creating state directory %s" %
-#                        File.dirname(Puppet[:statefile])
-#                rescue => detail
-#                    Puppet.err "Could not create state file: %s" % detail
-#                    return
-#                end
-#            end
 
             unless FileTest.exist?(Puppet[:statefile])
                 Puppet.info "Creating state file %s" % Puppet[:statefile]
             end
 
-            # FIXME This should be done as the puppet user, so that it isn't
-            # constantly chowned
             Puppet::Util.writelock(Puppet[:statefile], 0660) do |file|
                 file.print YAML.dump(@@state)
             end
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 9577acb19..9c6e77730 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -94,6 +94,18 @@ module Puppet
             end
         end
 
+        newparam(:links) do
+            desc "How to handle links during file actions.  During file copying,
+                ``follow`` will copy the target file instead of the link, ``copy``
+                will copy the link itself, and ``skip`` will just pass it by.
+                When not doing copying, ``follow`` and ``copy`` behave
+                equivalently."
+
+            newvalues(:follow, :copy, :skip)
+
+            defaultto :skip
+        end
+
         autorequire(:file) do
             cur = []
             pary = self[:path].split(File::SEPARATOR)
@@ -513,15 +525,21 @@ module Puppet
             end
         end
 
+        # Stat our file.  Depending on the value of the 'links' attribute, we use
+        # either 'stat' or 'lstat', and we expect the states to use the resulting
+        # stat object accordingly (mostly by testing the 'ftype' value).
         def stat(refresh = false)
+            method = :lstat
+            unless self[:links] == :skip
+                method = :stat
+            end
             if @stat.nil? or refresh == true
                 begin
-                    @stat = File.lstat(self[:path])
+                    @stat = File.send(method, self[:path])
                 rescue Errno::ENOENT => error
                     @stat = nil
-                rescue => error
-                    self.debug "Failed to stat %s: %s" %
-                        [self.name,error]
+                rescue Errno::EACCES => error
+                    self.warning "Could not stat; permission denied"
                     @stat = nil
                 end
             end
diff --git a/lib/puppet/type/pfile/checksum.rb b/lib/puppet/type/pfile/checksum.rb
index d4089b740..fcbc49c8c 100755
--- a/lib/puppet/type/pfile/checksum.rb
+++ b/lib/puppet/type/pfile/checksum.rb
@@ -156,12 +156,19 @@ module Puppet
                 @checktypes = ["md5"]
             end
 
-            unless FileTest.exists?(@parent[:path])
+            stat = nil
+            unless stat = @parent.stat
                 self.is = :absent
                 return
             end
 
-            if FileTest.directory?(@parent[:path]) and @checktypes[0] =~ /md5/
+            if stat.ftype == "link" and @parent[:links] == :skip
+                self.info "Not checksumming symlink"
+                self.is = self.should
+                return
+            end
+
+            if stat.ftype == "directory" and @checktypes[0] =~ /md5/
                 @checktypes = ["time"]
             end
 
diff --git a/lib/puppet/type/pfile/content.rb b/lib/puppet/type/pfile/content.rb
index 88b59e0f5..dea4a562c 100755
--- a/lib/puppet/type/pfile/content.rb
+++ b/lib/puppet/type/pfile/content.rb
@@ -27,10 +27,18 @@ module Puppet
         # We should probably take advantage of existing md5 sums if they're there,
         # but I really don't feel like dealing with the complexity right now.
         def retrieve
-            unless FileTest.exists?(@parent[:path])
+            stat = nil
+            unless stat = @parent.stat
                 @is = :absent
                 return
             end
+
+            if stat.ftype == "link" and @parent[:links] == :skip
+                self.info "Not changing the content of symlink"
+                self.is = self.should
+                return
+            end
+
             begin
                 @is = File.read(@parent[:path])
             rescue => detail
diff --git a/lib/puppet/type/pfile/group.rb b/lib/puppet/type/pfile/group.rb
index f01c69a1a..82c8e1fea 100755
--- a/lib/puppet/type/pfile/group.rb
+++ b/lib/puppet/type/pfile/group.rb
@@ -40,6 +40,13 @@ module Puppet
         def retrieve
             stat = @parent.stat(false)
 
+            # Set our method appropriately, depending on links.
+            if stat.ftype == "link" and @parent[:links] == :skip
+                @method = :lchown
+            else
+                @method = :chown
+            end
+
             if stat
                 self.is = stat.gid
             else
@@ -98,7 +105,7 @@ module Puppet
 
             begin
                 # set owner to nil so it's ignored
-                File.chown(nil,gid,@parent[:path])
+                File.send(@method,nil,gid,@parent[:path])
             rescue => detail
                 error = Puppet::Error.new( "failed to chgrp %s to %s: %s" %
                     [@parent[:path], self.should, detail.message])
diff --git a/lib/puppet/type/pfile/mode.rb b/lib/puppet/type/pfile/mode.rb
index 26c1c2449..70328bee2 100755
--- a/lib/puppet/type/pfile/mode.rb
+++ b/lib/puppet/type/pfile/mode.rb
@@ -75,7 +75,15 @@ module Puppet
         end
 
         def retrieve
+            # If we're not following links and we're a link, then we just turn
+            # off mode management entirely.
+
             if stat = @parent.stat(false)
+                if stat.ftype == "link" and @parent[:links] == :skip
+                    self.info "Not managing symlink mode"
+                    self.is = self.should
+                    return
+                end
                 self.is = stat.mode & 007777
                 unless defined? @fixed
                     if defined? @should and @should
diff --git a/lib/puppet/type/pfile/uid.rb b/lib/puppet/type/pfile/uid.rb
index 006712c1b..25e738780 100755
--- a/lib/puppet/type/pfile/uid.rb
+++ b/lib/puppet/type/pfile/uid.rb
@@ -85,6 +85,13 @@ module Puppet
                 return
             end
 
+            # Set our method appropriately, depending on links.
+            if stat.ftype == "link" and @parent[:links] == :skip
+                @method = :lchown
+            else
+                @method = :chown
+            end
+
             self.is = stat.uid
 
             # On OS X, files that are owned by -2 get returned as really
@@ -149,7 +156,7 @@ module Puppet
             end
 
             begin
-                File.chown(user, nil, @parent[:path])
+                File.send(@method, user, nil, @parent[:path])
             rescue => detail
                 raise Puppet::Error, "Failed to set owner to '%s': %s" %
                     [user, detail]
diff --git a/test/types/file.rb b/test/types/file.rb
index 513d7cca8..d9d3b5a23 100644
--- a/test/types/file.rb
+++ b/test/types/file.rb
@@ -41,7 +41,6 @@ class TestFile < Test::Unit::TestCase
     end
 
     def teardown
-        clearstorage
         Puppet::Storage.clear
         system("rm -rf %s" % Puppet[:statefile])
         super
@@ -134,6 +133,61 @@ class TestFile < Test::Unit::TestCase
             assert_events([:file_created], comp)
         end
 
+        def test_nofollowlinks
+            basedir = tempfile()
+            Dir.mkdir(basedir)
+            file = File.join(basedir, "file")
+            link = File.join(basedir, "link")
+
+            File.open(file, "w", 0644) { |f| f.puts "yayness"; f.flush }
+            File.symlink(file, link)
+
+            user = nonrootuser()
+
+            obj = nil
+            assert_nothing_raised {
+                obj = Puppet.type(:file).create(
+                    :name => link,
+                    :owner => user.name
+                )
+            }
+            obj.retrieve
+
+            assert_events([:file_changed], obj)
+
+            assert_equal(0, File.stat(file).uid)
+
+            obj[:links] = :follow
+            assert_events([:file_changed], obj)
+            assert_equal(user.uid, File.stat(file).uid)
+            File.chown(0, nil, file)
+
+            obj[:links] = :copy
+            assert_events([:file_changed], obj)
+            assert_equal(user.uid, File.stat(file).uid)
+
+            obj.delete(:owner)
+            obj[:links] = :skip
+
+            group = nonrootgroup
+
+            initgroup = File.stat(file).gid
+            obj[:group] = group.name
+
+            assert_events([:file_changed], obj)
+
+            assert_equal(initgroup, File.stat(file).gid)
+
+            obj[:links] = :follow
+            assert_events([:file_changed], obj)
+            assert_equal(group.gid, File.stat(file).gid)
+            File.chown(nil, initgroup, file)
+
+            obj[:links] = :copy
+            assert_events([:file_changed], obj)
+            assert_equal(group.gid, File.stat(file).gid)
+        end
+
         def test_ownerasroot
             file = mktestfile()
 
@@ -781,6 +835,64 @@ class TestFile < Test::Unit::TestCase
         assert_apply(file)
         assert_equal(0777, File.stat(path).mode & 007777)
     end
+
+    def test_followlinks
+        basedir = tempfile()
+        Dir.mkdir(basedir)
+        file = File.join(basedir, "file")
+        link = File.join(basedir, "link")
+
+        File.open(file, "w", 0644) { |f| f.puts "yayness"; f.flush }
+        File.symlink(file, link)
+
+        obj = nil
+        assert_nothing_raised {
+            obj = Puppet.type(:file).create(
+                :name => link,
+                :mode => "755"
+            )
+        }
+        obj.retrieve
+
+        assert_events([], obj)
+
+        # Assert that we default to not following links
+        assert_equal("%o" % 0644, "%o" % (File.stat(file).mode & 007777))
+
+        obj[:links] = :follow
+        assert_events([:file_changed], obj)
+
+        assert_equal("%o" % 0755, "%o" % (File.stat(file).mode & 007777))
+
+        File.chmod(0644, file)
+        obj[:links] = :copy
+        assert_events([:file_changed], obj)
+
+        assert_equal("%o" % 0755, "%o" % (File.stat(file).mode & 007777))
+
+        # Now verify that content and checksum don't update, either
+        obj.delete(:mode)
+        obj[:checksum] = "md5"
+        obj[:links] = :skip
+
+        assert_events([], obj)
+        File.open(file, "w") { |f| f.puts "more text" }
+        assert_events([], obj)
+        obj[:links] = :follow
+        assert_events([], obj)
+        File.open(file, "w") { |f| f.puts "even more text" }
+        assert_events([:file_changed], obj)
+
+        obj.delete(:checksum)
+        obj[:content] = "this is some content"
+        obj[:links] = :skip
+
+        assert_events([], obj)
+        File.open(file, "w") { |f| f.puts "more text" }
+        assert_events([], obj)
+        obj[:links] = :follow
+        assert_events([:file_changed], obj)
+    end
 end
 
 # $Id$