Introduces a new implicit 'modules' fileserver module, whose allow/deny can

be set from the fileserver.conf, but whose path is ignored and can
therefore not be used directly in puppet:// URL's.

When the fileserver looks for a file/directory, it first checks if the
first part of the URL references an existing module. If one is found, a new
temporary mount for that module is generated with the same permissions as
the 'modules' module. If no matching puppet module is found, the fileserver
behaves as it always has.


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

lib/puppet/modules.rb

@@ -2,6 +2,7 @@
 class Puppet::Module
 
     TEMPLATES = "templates"
+    FILES = "files"
 
     # Return an array of paths by splitting the +modulepath+ config
     # parameter. Only consider paths that are absolute and existing
@@ -68,5 +69,9 @@ class Puppet::Module
         return File::join(path, TEMPLATES, strip(file))
     end
 
+    def files
+        return File::join(path, FILES)
+    end
+
     private :initialize
 end

lib/puppet/network/handler/fileserver.rb

@@ -11,6 +11,9 @@ class Puppet::Network::Handler
 
         CHECKPARAMS = [:mode, :type, :owner, :group, :checksum]
 
+        # Special filserver module for puppet's module system
+        MODULES = "modules"
+
         @interface = XMLRPC::Service::Interface.new("fileserver") { |iface|
             iface.add_method("string describe(string, string)")
             iface.add_method("string list(string, string, boolean, array)")
@@ -263,12 +266,16 @@ class Puppet::Network::Handler
                             value = $2
                             case var
                             when "path":
-                                begin
+                                if mount.name == MODULES
-                                    mount.path = value
+                                    Puppet.warning "The '#{MODULES}' module can not have a path. Ignoring attempt to set it"
-                                rescue FileServerError => detail
+                                else
-                                    Puppet.err "Removing mount %s: %s" %
+                                    begin
-                                        [mount.name, detail]
+                                        mount.path = value
-                                    newmounts.delete(mount.name)
+                                    rescue FileServerError => detail
+                                        Puppet.err "Removing mount %s: %s" %
+                                            [mount.name, detail]
+                                        newmounts.delete(mount.name)
+                                    end
                                 end
                             when "allow":
                                 value.split(/\s*,\s*/).each { |val|
@@ -312,6 +319,12 @@ class Puppet::Network::Handler
             #    Puppet.err "FileServer error: %s" % detail
             end
 
+            unless newmounts[MODULES]
+                mount = Mount.new(MODULES)
+                mount.allow("*")
+                newmounts[MODULES] = mount
+            end
+
             # Verify each of the mounts are valid.
             # We let the check raise an error, so that it can raise an error
             # pointing to the specific problem.
@@ -375,8 +388,13 @@ class Puppet::Network::Handler
                 tmp = $1
                 path = dir.sub(%r{/#{tmp}/?}, '')
 
-                unless mount = @mounts[tmp]
+                mod = Puppet::Module::find(tmp)
-                    raise FileServerError, "Fileserver module '%s' not mounted" % tmp
+                if mod
+                    mount = @mounts[MODULES].copy(mod, mod.files)
+                else
+                    unless mount = @mounts[tmp]
+                        raise FileServerError, "Fileserver module '%s' not mounted" % tmp
+                    end
                 end
             else
                 raise FileServerError, "Fileserver error: Invalid path '%s'" % dir
@@ -579,9 +597,20 @@ class Puppet::Network::Handler
             # Verify our configuration is valid.  This should really check to
             # make sure at least someone will be allowed, but, eh.
             def valid?
-                return false unless @path
+                if name == MODULES
+                    return @path.nil?
+                else
+                    return ! @path.nil?
+                end
+            end
 
-                return true
+            # Return a new mount with the same properties as +self+, except
+            # with a different name and path.
+            def copy(name, path)
+                result = self.clone
+                result.path = path
+                result.instance_variable_set(:@name, name)
+                return result
             end
         end
     end

test/network/handler/fileserver.rb

@@ -1018,6 +1018,122 @@ allow *
             File.unlink(file)
         end
     end
+
+    # Test the default modules fileserving
+    def test_modules_default
+        moddir = tempfile
+        Dir.mkdir(moddir)
+        mounts = {}
+        Puppet[:modulepath] = moddir
+
+        mods = %w{green red}.collect do |name|
+            path = File::join(moddir, name, Puppet::Module::FILES)
+            FileUtils::mkdir_p(path)
+            if name == "green"
+                file = File::join(path, "test.txt")
+                File::open(file, "w") { |f| f.print name }
+            end
+
+           Puppet::Module::find(name)
+        end
+
+        conffile = tempfile
+        @@tmpfiles << conffile
+
+        File.open(conffile, "w") { |f| f.puts "# a test config file" }
+
+        # create a server with the file
+        server = nil
+        assert_nothing_raised {
+            server = Puppet::Network::Handler::FileServer.new(
+                :Local => false ,
+                :Config => conffile
+            )
+        }
+
+        mods.each do |mod|
+            mount = "/#{mod.name}/"
+            list = nil
+            assert_nothing_raised {
+                list = server.list(mount, :ignore, true, false)
+            }
+            list = list.split("\n")
+            if mod.name == "green"
+                assert_equal(2, list.size)
+                assert_equal("/\tdirectory", list[0])
+                assert_equal("/test.txt\tfile", list[1])
+            else
+                assert_equal(1, list.size)
+                assert_equal("/\tdirectory", list[0])
+            end
+
+            assert_nothing_raised("Host 'allow' denied #{mount}") {
+                server.list(mount, :ignore, true, false,
+                            'allow.example.com', "192.168.0.1")
+            }
+        end
+    end
+
+    # Test that configuring deny/allow for modules works
+    def test_modules_config
+        moddir = tempfile
+        Dir.mkdir(moddir)
+        mounts = {}
+        Puppet[:modulepath] = moddir
+
+        path = File::join(moddir, "amod", Puppet::Module::FILES)
+        file = File::join(path, "test.txt")
+        FileUtils::mkdir_p(path)
+        File::open(file, "w") { |f| f.print "Howdy" }
+
+        mod = Puppet::Module::find("amod")
+
+        conffile = tempfile
+        @@tmpfiles << conffile
+
+        File.open(conffile, "w") { |f|
+            f.print "# a test config file
+[modules]
+    path #{basedir}/thing
+    allow 192.168.0.*
+"
+        }
+
+        # create a server with the file
+        server = nil
+        assert_nothing_raised {
+            server = Puppet::Network::Handler::FileServer.new(
+                :Local => false,
+                :Config => conffile
+            )
+        }
+
+        list = nil
+        mount = "/#{mod.name}/"
+        assert_nothing_raised {
+            list = server.list(mount, :ignore, true, false)
+        }
+
+        assert_nothing_raised {
+            list.split("\n").each { |line|
+                file, type = line.split("\t")
+                server.describe(mount + file)
+            }
+        }
+
+        assert_describe(mount, file, server)
+
+        # now let's check that things are being correctly forbidden
+        assert_raise(Puppet::AuthorizationError,
+                     "Host 'deny' allowed #{mount}") {
+            server.list(mount, :ignore, true, false,
+                        'deny.example.com', "192.168.1.1")
+        }
+        assert_nothing_raised("Host 'allow' denied #{mount}") {
+            server.list(mount, :ignore, true, false,
+                        'allow.example.com', "192.168.0.1")
+        }
+    end
 end
 
 # $Id$