Committing the last changes, for now, to handling links. You still cannot copy remote links, but you can either ignore or follow them. I do not think we will be able to copy remote links until I have merged symlinks and files to be the same object type again.

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

8 files changed, 137 insertions, 68 deletions

diff --git a/lib/puppet/server/fileserver.rb b/lib/puppet/server/fileserver.rb
index 4fd238892..5b27f2fcb 100755
--- a/lib/puppet/server/fileserver.rb
+++ b/lib/puppet/server/fileserver.rb
@@ -3,13 +3,11 @@ require 'webrick/httpstatus'
 require 'cgi'
 
 module Puppet
+class FileServerError < Puppet::Error; end
 class Server
-    class FileServerError < Puppet::Error; end
     class FileServer < Handler
         attr_accessor :local
 
-        Puppet::Util.logmethods(self, true)
-
         Puppet.setdefaults("fileserver",
             :fileserverconfig => ["$confdir/fileserver.conf",
                 "Where the fileserver configuration is stored."])
@@ -18,9 +16,9 @@ class Server
         CHECKPARAMS = [:mode, :type, :owner, :group, :checksum]
 
         @interface = XMLRPC::Service::Interface.new("fileserver") { |iface|
-            iface.add_method("string describe(string)")
-            iface.add_method("string list(string, boolean, array)")
-            iface.add_method("string retrieve(string)")
+            iface.add_method("string describe(string, string)")
+            iface.add_method("string list(string, string, boolean, array)")
+            iface.add_method("string retrieve(string, string)")
         }
 
         def authcheck(file, mount, client, clientip)
@@ -33,14 +31,21 @@ class Server
 
         # Describe a given file.  This returns all of the manageable aspects
         # of that file.
-        def describe(file, client = nil, clientip = nil)
+        def describe(file, links = :ignore, client = nil, clientip = nil)
             readconfig
+
+            links = links.intern if links.is_a? String
+
+            if links == :manage
+                raise Puppet::FileServerError, "Cannot currently copy links"
+            end
+
             mount, path = splitpath(file)
 
             authcheck(file, mount, client, clientip)
 
             if client
-                self.debug "Describing %s for %s" % [file, client]
+                mount.debug "Describing %s for %s" % [file, client]
             end
 
             sdir = nil
@@ -51,10 +56,15 @@ class Server
             end
 
             obj = nil
-            unless obj = mount.check(sdir)
+            unless obj = mount.check(sdir, links)
                 return ""
             end
 
+            #if links == :ignore and obj[:type] == "link"
+            #    mount.info "Ignoring link %s" % obj.name
+            #    return ""
+            #end
+
             desc = []
             CHECKPARAMS.each { |check|
                 if state = obj.state(check)
@@ -126,7 +136,7 @@ class Server
         end
 
         # List a specific directory's contents.
-        def list(dir, recurse = false, ignore = false, client = nil, clientip = nil)
+        def list(dir, links = :ignore, recurse = false, ignore = false, client = nil, clientip = nil)
             readconfig
             mount, path = splitpath(dir)
 
@@ -289,8 +299,9 @@ class Server
 
         # Retrieve a file from the local disk and pass it to the remote
         # client.
-        def retrieve(file, client = nil, clientip = nil)
+        def retrieve(file, links = :ignore, client = nil, clientip = nil)
             readconfig
+            links = links.intern if links.is_a? String
             mount, path = splitpath(file)
 
             authcheck(file, mount, client, clientip)
@@ -310,7 +321,18 @@ class Server
                 return ""
             end
 
-            str = File.read(fpath)
+            links = links.intern if links.is_a? String
+
+            if links == :ignore and FileTest.symlink?(fpath)
+                return ""
+            end
+
+            str = nil
+            if links == :manage
+                raise Puppet::Error, "Cannot copy links yet."
+            else
+                str = File.read(fpath)
+            end
 
             if @local
                 return str
@@ -331,7 +353,8 @@ class Server
             )
         end
 
-        # Recursively list the directory.
+        # Recursively list the directory. FIXME This should be using
+        # puppet objects, not directly listing.
         def reclist(mount, root, path, recurse, ignore)
             # Take out the root of the path.
             name = path.sub(root, '')
@@ -433,21 +456,13 @@ class Server
 
             # Run 'retrieve' on a file.  This gets the actual parameters, so
             # we can pass them to the client.
-            def check(dir)
+            def check(dir, links)
                 unless FileTest.exists?(dir)
                     self.notice "File source %s does not exist" % dir
                     return nil
                 end
 
-                obj = nil
-                unless obj = Puppet.type(:file)[dir]
-                    obj = Puppet.type(:file).create(
-                        :name => dir,
-                        :check => CHECKPARAMS
-                    )
-
-                    @comp.push(obj)
-                end
+                obj = fileobj(dir, links)
 
                 # FIXME we should really have a timeout here -- we don't
                 # want to actually check on every connection, maybe no more
@@ -486,6 +501,30 @@ class Server
                 super()
             end
 
+            def fileobj(path, links)
+                obj = nil
+                unless obj = Puppet.type(:file)[path]
+                    obj = Puppet.type(:file).create(
+                        :name => path,
+                        :check => CHECKPARAMS
+                    )
+
+                    @comp.push(obj)
+                end
+
+                if links == :manage
+                    links = :follow
+                end
+
+                # This, ah, might be completely redundant
+                unless obj[:links] == links
+                    obj.info "setting links to %s" % links.inspect
+                    obj[:links] = links
+                end
+
+                return obj
+            end
+
             # Set the path.
             def path=(path)
                 unless FileTest.exists?(path)
@@ -503,6 +542,9 @@ class Server
                 "mount[#{@name}]"
             end
 
+            def type?(file)
+            end
+
             # Verify our configuration is valid.  This should really check to
             # make sure at least someone will be allowed, but, eh.
             def valid?
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 9c6e77730..ef70d211a 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -96,14 +96,18 @@ module Puppet
 
         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
+                ``follow`` will copy the target file instead of the link, ``manage``
+                will copy the link itself, and ``ignore`` will just pass it by.
+                When not copying, ``manage`` and ``ignore`` behave equivalently
+                (because you cannot really ignore links entirely during local
+                recursion), and ``follow`` will manage the file to which the
+                link points."
+
+            newvalues(:follow, :manage, :ignore)
+
+            # :ignore and :manage behave equivalently on local files,
+            # but don't copy remote links
+            defaultto :ignore
         end
 
         autorequire(:file) do
@@ -462,7 +466,7 @@ module Puppet
 
             #self.warning "Listing path %s with ignore %s" %
             #    [path.inspect, ignore.inspect]
-            desc = server.list(path, r, ignore)
+            desc = server.list(path, self[:links], r, ignore)
            
             desc.split("\n").each { |line|
                 file, type = line.split("\t")
@@ -529,9 +533,10 @@ module Puppet
         # 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
+            method = :stat
+            # Files are the only types that support links
+            if self.class.name == :file and self[:links] != :follow
+                method = :lstat
             end
             if @stat.nil? or refresh == true
                 begin
diff --git a/lib/puppet/type/pfile/checksum.rb b/lib/puppet/type/pfile/checksum.rb
index fcbc49c8c..ae4fb6639 100755
--- a/lib/puppet/type/pfile/checksum.rb
+++ b/lib/puppet/type/pfile/checksum.rb
@@ -162,7 +162,7 @@ module Puppet
                 return
             end
 
-            if stat.ftype == "link" and @parent[:links] == :skip
+            if stat.ftype == "link" and @parent[:links] != :follow
                 self.info "Not checksumming symlink"
                 self.is = self.should
                 return
diff --git a/lib/puppet/type/pfile/content.rb b/lib/puppet/type/pfile/content.rb
index dea4a562c..42596f1ed 100755
--- a/lib/puppet/type/pfile/content.rb
+++ b/lib/puppet/type/pfile/content.rb
@@ -33,7 +33,7 @@ module Puppet
                 return
             end
 
-            if stat.ftype == "link" and @parent[:links] == :skip
+            if stat.ftype == "link" and @parent[:links] == :ignore
                 self.info "Not changing the content of symlink"
                 self.is = self.should
                 return
diff --git a/lib/puppet/type/pfile/group.rb b/lib/puppet/type/pfile/group.rb
index 82c8e1fea..7fde3c657 100755
--- a/lib/puppet/type/pfile/group.rb
+++ b/lib/puppet/type/pfile/group.rb
@@ -41,7 +41,7 @@ module Puppet
             stat = @parent.stat(false)
 
             # Set our method appropriately, depending on links.
-            if stat.ftype == "link" and @parent[:links] == :skip
+            if stat.ftype == "link" and @parent[:links] != :follow
                 @method = :lchown
             else
                 @method = :chown
diff --git a/lib/puppet/type/pfile/mode.rb b/lib/puppet/type/pfile/mode.rb
index 70328bee2..974b4a453 100755
--- a/lib/puppet/type/pfile/mode.rb
+++ b/lib/puppet/type/pfile/mode.rb
@@ -40,7 +40,8 @@ module Puppet
             value = should
             if value.is_a?(String)
                 unless value =~ /^[0-9]+$/
-                    raise Puppet::Error, "File modes can only be numbers"
+                    raise Puppet::Error, "File modes can only be numbers, not %s" %
+                        value.inspect
                 end
                 unless value =~ /^0/
                     value = "0" + value
@@ -74,16 +75,20 @@ module Puppet
             return value
         end
 
+        def insync?
+            if stat = @parent.stat and stat.ftype == "link" and @parent[:links] != :follow
+                self.info "Not managing symlink mode"
+                return true
+            else
+                return super
+            end
+        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/source.rb b/lib/puppet/type/pfile/source.rb
index b33bdb38f..391101e96 100755
--- a/lib/puppet/type/pfile/source.rb
+++ b/lib/puppet/type/pfile/source.rb
@@ -4,7 +4,7 @@ module Puppet
         PINPARAMS = [:mode, :type, :owner, :group, :checksum]
 
         attr_accessor :source, :local
-        desc "Copy a file over the current file.  Uses `checksum` to
+        desc "Copy a file over the current file.  Uses ``checksum`` to
             determine when a file should be copied.  Valid values are either
             fully qualified paths to files, or URIs.  Currently supported URI
             types are *puppet* and *file*.
@@ -34,7 +34,7 @@ module Puppet
             server = sourceobj.server
 
             begin
-                desc = server.describe(path)
+                desc = server.describe(path, @parent[:links])
             rescue NetworkClientError => detail
                 self.err "Could not describe %s: %s" %
                     [path, detail]
@@ -94,23 +94,6 @@ module Puppet
                 return nil
             end
 
-            # Take each of the stats and set them as states on the local file
-            # if a value has not already been provided.
-            @stats.each { |stat, value|
-                next if stat == :checksum
-                next if stat == :type
-
-                # was the stat already specified, or should the value
-                # be inherited from the source?
-                unless @parent.argument?(stat)
-                    if state = @parent.state(stat)
-                        state.should = value
-                    else
-                        @parent[stat] = value
-                    end
-                end
-            }
-
             # If we're a normal file, then set things up to copy the file down.
             case @stats[:type]
             when "file":
@@ -143,13 +126,44 @@ module Puppet
                 # we'll let the :ensure state do our work
                 @should.clear
                 @is = true
-            # FIXME We should at least support symlinks, I would think...
+            when "link":
+                case @parent[:links]
+                when :ignore
+                    @is = :nocopy
+                    @should = [:nocopy]
+                    self.info "Ignoring link %s" % @source
+                    return
+                when :follow
+                    @stats = self.describe(source, :follow)
+                    if @stats.empty?
+                        raise Puppet::Error, "Could not follow link %s" % @source
+                    end
+                when :copy
+                    raise Puppet::Error, "Cannot copy links yet"
+                end
             else
                 self.err "Cannot use files of type %s as sources" %
                     @stats[:type]
-                @should = nil
-                @is = true
+                @should = [:nocopy]
+                @is = :nocopy
             end
+
+            # Take each of the stats and set them as states on the local file
+            # if a value has not already been provided.
+            @stats.each { |stat, value|
+                next if stat == :checksum
+                next if stat == :type
+
+                # was the stat already specified, or should the value
+                # be inherited from the source?
+                unless @parent.argument?(stat)
+                    if state = @parent.state(stat)
+                        state.should = value
+                    else
+                        @parent[stat] = value
+                    end
+                end
+            }
         end
 
         # The special thing here is that we need to make sure that 'should'
@@ -189,6 +203,9 @@ module Puppet
                 end
             end
 
+            case @stats[:type]
+            when "link":
+            end
             unless @stats[:type] == "file"
                 #if @stats[:type] == "directory"
                         #[@parent.name, @is.inspect, @should.inspect]
@@ -204,7 +221,7 @@ module Puppet
             sourceobj, path = @parent.uri2obj(@source)
 
             begin
-                contents = sourceobj.server.retrieve(path)
+                contents = sourceobj.server.retrieve(path, @parent[:links])
             rescue NetworkClientError => detail
                 self.err "Could not retrieve %s: %s" %
                     [path, detail]
diff --git a/lib/puppet/type/pfile/uid.rb b/lib/puppet/type/pfile/uid.rb
index 25e738780..d2f6566f4 100755
--- a/lib/puppet/type/pfile/uid.rb
+++ b/lib/puppet/type/pfile/uid.rb
@@ -86,7 +86,7 @@ module Puppet
             end
 
             # Set our method appropriately, depending on links.
-            if stat.ftype == "link" and @parent[:links] == :skip
+            if stat.ftype == "link" and @parent[:links] != :follow
                 @method = :lchown
             else
                 @method = :chown