Fixing #796 -- the fileserver can now start with no

configuration file (it creates both default mount points
if it does) and puppetmasterd no longer requires the
configuration file to exist.

3 files changed, 76 insertions, 30 deletions

diff --git a/bin/puppetmasterd b/bin/puppetmasterd
index b782969a8..bae11c831 100755
--- a/bin/puppetmasterd
+++ b/bin/puppetmasterd
@@ -103,7 +103,6 @@ result = GetoptLong.new(*options)
 master = {}
 ca = {}
 report = {}
-fs = {}
 bucket = {}
 
 options = {
@@ -193,6 +192,7 @@ require 'etc'
 
 handlers = {
     :Status => {},
+    :FileServer => {}
 }
 
 if options[:havemaster]
@@ -225,17 +225,6 @@ if Puppet[:parseonly]
     exit(0)
 end
 
-if File.exists?(Puppet[:fileserverconfig])
-    fs[:Config] = Puppet[:fileserverconfig]
-#else
-#    Puppet.notice "File server config %s does not exist; skipping file serving" %
-#        Puppet[:fileserverconfig]
-end
-
-if fs.include?(:Config)
-    handlers[:FileServer] = fs
-end
-
 webserver = server = nil
 begin
     case Puppet[:servertype]
diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb
index 52db3821f..da02a46c5 100755
--- a/lib/puppet/network/handler/fileserver.rb
+++ b/lib/puppet/network/handler/fileserver.rb
@@ -29,6 +29,34 @@ class Puppet::Network::Handler
             CHECKPARAMS.dup
         end
 
+        # If the configuration file exists, then create (if necessary) a LoadedFile
+        # object to manage it; else, return nil.
+        def configuration
+            # Short-circuit the default case.
+            return @configuration if defined?(@configuration)
+
+            config_path = @passed_configuration_path || Puppet[:fileserverconfig]
+            return nil unless FileTest.exist?(config_path)
+
+            # The file exists but we don't have a LoadedFile instance for it.
+            @configuration = Puppet::Util::LoadedFile.new(config_path)
+        end
+
+        # Create our default mounts for modules and plugins.  This is duplicated code,
+        # but I'm not really worried about that.
+        def create_default_mounts
+            @mounts = {}
+            Puppet.debug "No file server configuration file; autocreating #{MODULES} mount with default permissions"
+            mount = Mount.new(MODULES)
+            mount.allow("*")
+            @mounts[MODULES] = mount
+            
+            Puppet.debug "No file server configuration file; autocreating #{PLUGINS} mount with default permissions"
+            mount = PluginMount.new(PLUGINS)
+            mount.allow("*")
+            @mounts[PLUGINS] = mount
+        end
+
         # Describe a given file.  This returns all of the manageable aspects
         # of that file.
         def describe(url, links = :ignore, client = nil, clientip = nil)
@@ -80,13 +108,10 @@ class Puppet::Network::Handler
 
             if hash[:Config] == false
                 @noreadconfig = true
-            else
-                @config = Puppet::Util::LoadedFile.new(
-                    hash[:Config] || Puppet[:fileserverconfig]
-                )
-                @noreadconfig = false
             end
 
+            @passed_configuration_path = hash[:Config]
+
             if hash.include?(:Mount)
                 @passedconfig = true
                 unless hash[:Mount].is_a?(Hash)
@@ -103,7 +128,11 @@ class Puppet::Network::Handler
                 self.mount(nil, PLUGINS)
             else
                 @passedconfig = false
-                readconfig(false) # don't check the file the first time.
+                if configuration
+                    readconfig(false) # don't check the file the first time.
+                else
+                    create_default_mounts()
+                end
             end
         end
 
@@ -137,6 +166,11 @@ class Puppet::Network::Handler
             self.local
         end
 
+        # Is a given mount available?
+        def mounted?(name)
+            @mounts.include?(name)
+        end
+
         # Mount a new directory with a name.
         def mount(path, name)
             if @mounts.include?(name)
@@ -253,13 +287,15 @@ class Puppet::Network::Handler
         def readconfig(check = true)
             return if @noreadconfig
 
-            if check and ! @config.changed?
+            return unless configuration
+
+            if check and ! @configuration.changed?
                 return
             end
 
             newmounts = {}
             begin
-                File.open(@config.file) { |f|
+                File.open(@configuration.file) { |f|
                     mount = nil
                     count = 1
                     f.each { |line|
@@ -270,7 +306,7 @@ class Puppet::Network::Handler
                             name = $1
                             if newmounts.include?(name)
                                 raise FileServerError, "%s is already mounted at %s" %
-                                    [newmounts[name], name], count, @config.file
+                                    [newmounts[name], name], count, @configuration.file
                             end
                             mount = Mount.new(name)
                             newmounts[name] = mount
@@ -301,7 +337,7 @@ class Puppet::Network::Handler
                                         mount.allow(val)
                                     rescue AuthStoreError => detail
                                         raise FileServerError.new(detail.to_s,
-                                            count, @config.file)
+                                            count, @configuration.file)
                                     end
                                 }
                             when "deny":
@@ -311,26 +347,26 @@ class Puppet::Network::Handler
                                         mount.deny(val)
                                     rescue AuthStoreError => detail
                                         raise FileServerError.new(detail.to_s,
-                                            count, @config.file)
+                                            count, @configuration.file)
                                     end
                                 }
                             else
                                 raise FileServerError.new("Invalid argument '%s'" % var,
-                                    count, @config.file)
+                                    count, @configuration.file)
                             end
                         else
                             raise FileServerError.new("Invalid line '%s'" % line.chomp,
-                                count, @config.file)
+                                count, @configuration.file)
                         end
                         count += 1
                     }
                 }
             rescue Errno::EACCES => detail
-                Puppet.err "FileServer error: Cannot read %s; cannot serve" % @config
-                #raise Puppet::Error, "Cannot read %s" % @config
+                Puppet.err "FileServer error: Cannot read %s; cannot serve" % @configuration
+                #raise Puppet::Error, "Cannot read %s" % @configuration
             rescue Errno::ENOENT => detail
                 Puppet.err "FileServer error: '%s' does not exist; cannot serve" %
-                    @config
+                    @configuration
             end
 
             unless newmounts[MODULES]
diff --git a/test/network/handler/fileserver.rb b/test/network/handler/fileserver.rb
index 962d35e65..233e705c6 100755
--- a/test/network/handler/fileserver.rb
+++ b/test/network/handler/fileserver.rb
@@ -970,12 +970,16 @@ allow *
         }
 
         dir = tempfile()
+        Facter.stubs(:value).with(:ipaddress).returns("127.0.0.1")
+        Facter.stubs(:value).with { |v| v.to_s == "hostname" }.returns("myhost")
+        Facter.stubs(:value).with { |v| v.to_s == "domain" }.returns("mydomain.com")
+        Facter.stubs(:value).with(:domain).returns("mydomain.com")
         ip = Facter.value(:ipaddress)
 
         Dir.mkdir(dir)
-        host = "host.domain.com"
+        host = "myhost.mydomain.com"
         {
-            "%H" => "host.domain.com", "%h" => "host", "%d" => "domain.com"
+            "%H" => "myhost.mydomain.com", "%h" => "myhost", "%d" => "mydomain.com"
         }.each do |pattern, string|
             file = File.join(dir, string)
             mount = File.join(dir, pattern)
@@ -1147,6 +1151,23 @@ allow *
             }
         end
     end
+
+    def test_can_start_without_configuration
+        Puppet[:fileserverconfig] = tempfile
+        assert_nothing_raised("Could not create fileserver when configuration is absent") do
+                server = Puppet::Network::Handler::FileServer.new(
+                    :Local => false
+                )
+        end
+    end
+
+    def test_creates_default_mounts_when_no_configuration_is_available
+        Puppet[:fileserverconfig] = tempfile
+        server = Puppet::Network::Handler::FileServer.new(:Local => false)
+
+        assert(server.mounted?("plugins"), "Did not create default plugins mount when missing configuration file")
+        assert(server.mounted?("modules"), "Did not create default modules mount when missing configuration file")
+    end
 end