Fixing #2558 - propagating recent fileserving changes

I'd made changes to the internals of the fileserving
system to fix #2544 (mostly switched from passing
the node around and then calculating the environment to just
passing the environment around), but those changes weren't consistent
throughout the fileserving code.

In the process of making them consistent, I realized that the
plain file server actually needs the node name rather than
the environment, so I switched to passing the request around,
because it has both pieces of information.

Also added further integration tests which will hopefully keep
this from cropping up again.

Signed-off-by: Luke Kanies <luke@madstop.com>

7 files changed, 163 insertions, 121 deletions

diff --git a/spec/integration/indirector/file_content/file_server.rb b/spec/integration/indirector/file_content/file_server.rb
index b3c63fc33..ea892302e 100755
--- a/spec/integration/indirector/file_content/file_server.rb
+++ b/spec/integration/indirector/file_content/file_server.rb
@@ -19,7 +19,7 @@ describe Puppet::Indirector::FileContent::FileServer, " when finding files" do
         @test_class = Puppet::FileServing::Content
     end
 
-    it "should find file content in the environment specified in the request" do
+    it "should find plugin file content in the environment specified in the request" do
         path = tmpfile("file_content_with_env")
 
         Dir.mkdir(path)
@@ -40,4 +40,54 @@ describe Puppet::Indirector::FileContent::FileServer, " when finding files" do
         result[1].should be_instance_of(Puppet::FileServing::Content)
         result[1].content.should == "1\n"
     end
+
+    it "should find file content in modules" do
+        path = tmpfile("file_content")
+
+        Dir.mkdir(path)
+
+        modpath = File.join(path, "mymod")
+        FileUtils.mkdir_p(File.join(modpath, "files"))
+        file = File.join(modpath, "files", "myfile")
+        File.open(file, "w") { |f| f.puts "1" }
+
+        Puppet.settings[:modulepath] = path
+
+        result = Puppet::FileServing::Content.find("modules/mymod/myfile")
+
+        result.should_not be_nil
+        result.should be_instance_of(Puppet::FileServing::Content)
+        result.content.should == "1\n"
+    end
+
+    it "should find file content in files when node name expansions are used" do
+        Puppet::Util::Cacher.expire
+        FileTest.stubs(:exists?).returns true
+        FileTest.stubs(:exists?).with(Puppet[:fileserverconfig]).returns(true)
+
+        @path = tmpfile("file_server_testing")
+
+        Dir.mkdir(@path)
+        subdir = File.join(@path, "mynode")
+        Dir.mkdir(subdir)
+        File.open(File.join(subdir, "myfile"), "w") { |f| f.puts "1" }
+
+        # Use a real mount, so the integration is a bit deeper.
+        @mount1 = Puppet::FileServing::Configuration::Mount::File.new("one")
+        @mount1.stubs(:allowed?).returns true
+        @mount1.path = File.join(@path, "%h")
+
+        @parser = stub 'parser', :changed? => false
+        @parser.stubs(:parse).returns("one" => @mount1)
+
+        Puppet::FileServing::Configuration::Parser.stubs(:new).returns(@parser)
+
+        path = File.join(@path, "myfile")
+
+        result = Puppet::FileServing::Content.find("one/myfile", :environment => "foo", :node => "mynode")
+
+        result.should_not be_nil
+        result.should be_instance_of(Puppet::FileServing::Content)
+        result.content.should == "1\n"
+    end
 end
diff --git a/spec/unit/file_serving/configuration.rb b/spec/unit/file_serving/configuration.rb
index 57ae83a14..c2f386f14 100755
--- a/spec/unit/file_serving/configuration.rb
+++ b/spec/unit/file_serving/configuration.rb
@@ -128,21 +128,20 @@ describe Puppet::FileServing::Configuration do
         it "should choose the named mount if one exists" do
             config = Puppet::FileServing::Configuration.create
             config.expects(:mounts).returns("one" => "foo")
-            config.find_mount("one", "mynode").should == "foo"
+            config.find_mount("one", mock('env')).should == "foo"
         end
 
-        it "should use the environment of the module mount to find a matching module if the named module cannot be found" do
+        it "should use the provided environment to find a matching module if the named module cannot be found" do
             config = Puppet::FileServing::Configuration.create
 
             mod = mock 'module'
             env = mock 'environment'
             env.expects(:module).with("foo").returns mod
             mount = mock 'mount'
-            mount.expects(:environment).with("mynode").returns env
 
             config.stubs(:mounts).returns("modules" => mount)
             Puppet::Util::Warnings.expects(:warnonce)
-            config.find_mount("foo", "mynode").should equal(mount)
+            config.find_mount("foo", env).should equal(mount)
         end
 
         it "should return nil if there is no such named mount and no module with the same name exists" do
@@ -150,11 +149,10 @@ describe Puppet::FileServing::Configuration do
 
             env = mock 'environment'
             env.expects(:module).with("foo").returns nil
-            mount = mock 'mount'
-            mount.expects(:environment).with("mynode").returns env
 
+            mount = mock 'mount'
             config.stubs(:mounts).returns("modules" => mount)
-            config.find_mount("foo", "mynode").should be_nil
+            config.find_mount("foo", env).should be_nil
         end
     end
 
@@ -163,7 +161,7 @@ describe Puppet::FileServing::Configuration do
             @config = Puppet::FileServing::Configuration.create
             @config.stubs(:find_mount)
 
-            @request = stub 'request', :key => "foo/bar/baz", :options => {}, :node => nil
+            @request = stub 'request', :key => "foo/bar/baz", :options => {}, :node => nil, :environment => mock("env")
         end
 
         it "should reread the configuration" do
@@ -190,8 +188,8 @@ describe Puppet::FileServing::Configuration do
             lambda { @config.split_path(@request) }.should_not raise_error(ArgumentError)
         end
 
-        it "should use the mount name and node to find the mount" do
-            @config.expects(:find_mount).with { |name, node| name == "foo" and node == "mynode" }
+        it "should use the mount name and environment to find the mount" do
+            @config.expects(:find_mount).with { |name, env| name == "foo" and env == @request.environment }
             @request.stubs(:node).returns("mynode")
 
             @config.split_path(@request)
diff --git a/spec/unit/file_serving/mount/file.rb b/spec/unit/file_serving/mount/file.rb
index 499a0357d..837fe8e62 100755
--- a/spec/unit/file_serving/mount/file.rb
+++ b/spec/unit/file_serving/mount/file.rb
@@ -104,17 +104,17 @@ describe Puppet::FileServing::Mount::File, "when determining the complete file p
 
     it "should return nil if the file is absent" do
         FileTest.stubs(:exist?).returns(false)
-        @mount.complete_path("/my/path").should be_nil
+        @mount.complete_path("/my/path", nil).should be_nil
     end
 
     it "should return the file path if the file is present" do
         FileTest.stubs(:exist?).with("/my/path").returns(true)
-        @mount.complete_path("/my/path").should == "/mount/my/path"
+        @mount.complete_path("/my/path", nil).should == "/mount/my/path"
     end
 
     it "should treat a nil file name as the path to the mount itself" do
         FileTest.stubs(:exist?).returns(true)
-        @mount.complete_path(nil).should == "/mount"
+        @mount.complete_path(nil, nil).should == "/mount"
     end
 
     it "should use the client host name if provided in the options" do
@@ -148,12 +148,14 @@ describe Puppet::FileServing::Mount::File, "when finding files" do
         @mount.path = "/mount"
         stub_facter("myhost.mydomain.com")
         @host = "host.domain.com"
+
+        @request = stub 'request', :node => "foo"
     end
 
     it "should return the results of the complete file path" do
         FileTest.stubs(:exist?).returns(false)
         @mount.expects(:complete_path).with("/my/path", "foo").returns "eh"
-        @mount.find("/my/path", :node => "foo").should == "eh"
+        @mount.find("/my/path", @request).should == "eh"
     end
 end
 
@@ -168,17 +170,19 @@ describe Puppet::FileServing::Mount::File, "when searching for files" do
         @mount.path = "/mount"
         stub_facter("myhost.mydomain.com")
         @host = "host.domain.com"
+
+        @request = stub 'request', :node => "foo"
     end
 
     it "should return the results of the complete file path as an array" do
         FileTest.stubs(:exist?).returns(false)
         @mount.expects(:complete_path).with("/my/path", "foo").returns "eh"
-        @mount.search("/my/path", :node => "foo").should == ["eh"]
+        @mount.search("/my/path", @request).should == ["eh"]
     end
 
     it "should return nil if the complete path is nil" do
         FileTest.stubs(:exist?).returns(false)
         @mount.expects(:complete_path).with("/my/path", "foo").returns nil
-        @mount.search("/my/path", :node => "foo").should be_nil
+        @mount.search("/my/path", @request).should be_nil
     end
 end
diff --git a/spec/unit/file_serving/mount/modules.rb b/spec/unit/file_serving/mount/modules.rb
index 6861e94a5..eeecc9aae 100755
--- a/spec/unit/file_serving/mount/modules.rb
+++ b/spec/unit/file_serving/mount/modules.rb
@@ -3,70 +3,61 @@
 require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/file_serving/mount/modules'
 
-describe Puppet::FileServing::Mount::Modules, "when finding files" do
+describe Puppet::FileServing::Mount::Modules do
     before do
         @mount = Puppet::FileServing::Mount::Modules.new("modules")
 
         @environment = stub 'environment', :module => nil
-        @mount.stubs(:environment).returns @environment
+        @request = stub 'request', :environment => @environment
     end
 
-    it "should use the node's environment to find the module" do
-        env = mock 'env'
-        @mount.expects(:environment).with("mynode").returns env
-        env.expects(:module)
+    describe "when finding files" do
+        it "should use the provided environment to find the module" do
+            @environment.expects(:module)
 
-        @mount.find("foo", :node => "mynode")
-    end
+            @mount.find("foo", @request)
+        end
 
-    it "should treat the first field of the relative path as the module name" do
-        @environment.expects(:module).with("foo")
-        @mount.find("foo/bar/baz")
-    end
+        it "should treat the first field of the relative path as the module name" do
+            @environment.expects(:module).with("foo")
+            @mount.find("foo/bar/baz", @request)
+        end
 
-    it "should return nil if the specified module does not exist" do
-        @environment.expects(:module).with("foo").returns nil
-        @mount.find("foo/bar/baz")
-    end
+        it "should return nil if the specified module does not exist" do
+            @environment.expects(:module).with("foo").returns nil
+            @mount.find("foo/bar/baz", @request)
+        end
 
-    it "should return the file path from the module" do
-        mod = mock 'module'
-        mod.expects(:file).with("bar/baz").returns "eh"
-        @environment.expects(:module).with("foo").returns mod
-        @mount.find("foo/bar/baz").should == "eh"
+        it "should return the file path from the module" do
+            mod = mock 'module'
+            mod.expects(:file).with("bar/baz").returns "eh"
+            @environment.expects(:module).with("foo").returns mod
+            @mount.find("foo/bar/baz", @request).should == "eh"
+        end
     end
-end
 
-describe Puppet::FileServing::Mount::Modules, "when searching for files" do
-    before do
-        @mount = Puppet::FileServing::Mount::Modules.new("modules")
+    describe "when searching for files" do
+        it "should use the node's environment to search the module" do
+            @environment.expects(:module)
 
-        @environment = stub 'environment', :module => nil
-        @mount.stubs(:environment).returns @environment
-    end
-
-    it "should use the node's environment to search the module" do
-        env = mock 'env'
-        @mount.expects(:environment).with("mynode").returns env
-        env.expects(:module)
-
-        @mount.search("foo", :node => "mynode")
-    end
+            @mount.search("foo", @request)
+        end
 
-    it "should treat the first field of the relative path as the module name" do
-        @environment.expects(:module).with("foo")
-        @mount.search("foo/bar/baz")
-    end
+        it "should treat the first field of the relative path as the module name" do
+            @environment.expects(:module).with("foo")
+            @mount.search("foo/bar/baz", @request)
+        end
 
-    it "should return nil if the specified module does not exist" do
-        @environment.expects(:module).with("foo").returns nil
-        @mount.search("foo/bar/baz")
-    end
+        it "should return nil if the specified module does not exist" do
+            @environment.expects(:module).with("foo").returns nil
+            @mount.search("foo/bar/baz", @request)
+        end
 
-    it "should return the file path as an array from the module" do
-        mod = mock 'module'
-        mod.expects(:file).with("bar/baz").returns "eh"
-        @environment.expects(:module).with("foo").returns mod
-        @mount.search("foo/bar/baz").should == ["eh"]
+        it "should return the file path as an array from the module" do
+            mod = mock 'module'
+            mod.expects(:file).with("bar/baz").returns "eh"
+            @environment.expects(:module).with("foo").returns mod
+            @mount.search("foo/bar/baz", @request).should == ["eh"]
+        end
     end
 end
diff --git a/spec/unit/file_serving/mount/plugins.rb b/spec/unit/file_serving/mount/plugins.rb
index b3a32b70d..d8c05a2bd 100755
--- a/spec/unit/file_serving/mount/plugins.rb
+++ b/spec/unit/file_serving/mount/plugins.rb
@@ -3,60 +3,59 @@
 require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/file_serving/mount/plugins'
 
-describe Puppet::FileServing::Mount::Plugins, "when finding files" do
+describe Puppet::FileServing::Mount::Plugins do
     before do
-        @mount = Puppet::FileServing::Mount::Plugins.new("modules")
-    end
-
-    it "should use the provided environment to find the modules" do
-        env = mock 'env'
-        env.expects(:modules).returns []
+        @mount = Puppet::FileServing::Mount::Plugins.new("plugins")
 
-        @mount.find("foo", env)
+        @environment = stub 'environment', :module => nil
+        @request = stub 'request', :environment => @environment
     end
 
-    it "should return nil if no module can be found with a matching plugin" do
-        mod = mock 'module'
-        mod.stubs(:plugin).with("foo/bar").returns nil
+    describe  "when finding files" do
+        it "should use the provided environment to find the modules" do
+            @environment.expects(:modules).returns []
 
-        env = stub 'env', :modules => []
-        @mount.find("foo/bar", env).should be_nil
-    end
+            @mount.find("foo", @request)
+        end
 
-    it "should return the file path from the module" do
-        mod = mock 'module'
-        mod.stubs(:plugin).with("foo/bar").returns "eh"
+        it "should return nil if no module can be found with a matching plugin" do
+            mod = mock 'module'
+            mod.stubs(:plugin).with("foo/bar").returns nil
 
-        env = stub 'env', :modules => [mod]
-        @mount.find("foo/bar", env).should == "eh"
-    end
-end
+            @environment.stubs(:modules).returns [mod]
+            @mount.find("foo/bar", @request).should be_nil
+        end
 
-describe Puppet::FileServing::Mount::Plugins, "when searching for files" do
-    before do
-        @mount = Puppet::FileServing::Mount::Plugins.new("modules")
+        it "should return the file path from the module" do
+            mod = mock 'module'
+            mod.stubs(:plugin).with("foo/bar").returns "eh"
+
+            @environment.stubs(:modules).returns [mod]
+            @mount.find("foo/bar", @request).should == "eh"
+        end
     end
 
-    it "should use the node's environment to find the modules" do
-        env = mock 'env'
-        env.expects(:modules).returns []
+    describe "when searching for files" do
+        it "should use the node's environment to find the modules" do
+            @environment.expects(:modules).returns []
 
-        @mount.search("foo", env)
-    end
+            @mount.search("foo", @request)
+        end
 
-    it "should return nil if no modules can be found that have plugins" do
-        mod = mock 'module'
-        mod.stubs(:plugins?).returns false
+        it "should return nil if no modules can be found that have plugins" do
+            mod = mock 'module'
+            mod.stubs(:plugins?).returns false
 
-        env = stub 'env', :modules => []
-        @mount.search("foo/bar", env).should be_nil
-    end
+            @environment.stubs(:modules).returns []
+            @mount.search("foo/bar", @request).should be_nil
+        end
 
-    it "should return the plugin paths for each module that has plugins" do
-        one = stub 'module', :plugins? => true, :plugin_directory => "/one"
-        two = stub 'module', :plugins? => true, :plugin_directory => "/two"
+        it "should return the plugin paths for each module that has plugins" do
+            one = stub 'module', :plugins? => true, :plugin_directory => "/one"
+            two = stub 'module', :plugins? => true, :plugin_directory => "/two"
 
-        env = stub 'env', :modules => [one, two]
-        @mount.search("foo/bar", env).should == %w{/one /two}
+            @environment.stubs(:modules).returns [one, two]
+            @mount.search("foo/bar", @request).should == %w{/one /two}
+        end
     end
 end
diff --git a/spec/unit/indirector/file_server.rb b/spec/unit/indirector/file_server.rb
index a2e2ff811..912695e27 100755
--- a/spec/unit/indirector/file_server.rb
+++ b/spec/unit/indirector/file_server.rb
@@ -52,15 +52,15 @@ describe Puppet::Indirector::FileServer do
         it "should use the mount to find the full path" do
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:find).with { |key, env| key == "rel/path" }
+            @mount.expects(:find).with { |key, request| key == "rel/path" }
 
             @file_server.find(@request)
         end
 
-        it "should pass the request's environment when finding a file" do
+        it "should pass the request when finding a file" do
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:find).with { |key, env| env == @request.environment }
+            @mount.expects(:find).with { |key, request| request == @request }
 
             @file_server.find(@request)
         end
@@ -68,7 +68,7 @@ describe Puppet::Indirector::FileServer do
         it "should return nil if it cannot find a full path" do
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:find).with { |key, env| key == "rel/path" }.returns nil
+            @mount.expects(:find).with { |key, request| key == "rel/path" }.returns nil
 
             @file_server.find(@request).should be_nil
         end
@@ -76,7 +76,7 @@ describe Puppet::Indirector::FileServer do
         it "should create an instance with the found path" do
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:find).with { |key, env| key == "rel/path" }.returns "/my/file"
+            @mount.expects(:find).with { |key, request| key == "rel/path" }.returns "/my/file"
 
             @model.expects(:new).with("/my/file").returns @instance
 
@@ -87,7 +87,7 @@ describe Puppet::Indirector::FileServer do
             @request.options[:links] = true
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:find).with { |key, env| key == "rel/path" }.returns "/my/file"
+            @mount.expects(:find).with { |key, request| key == "rel/path" }.returns "/my/file"
 
             @model.expects(:new).with("/my/file").returns @instance
 
@@ -100,7 +100,7 @@ describe Puppet::Indirector::FileServer do
             @request.options[:links] = true
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:find).with { |key, env| key == "rel/path" }.returns "/my/file"
+            @mount.expects(:find).with { |key, request| key == "rel/path" }.returns "/my/file"
 
             @model.expects(:new).with("/my/file").returns @instance
 
@@ -131,15 +131,15 @@ describe Puppet::Indirector::FileServer do
         it "should use the mount to search for the full paths" do
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:search).with { |key, env| key == "rel/path" }
+            @mount.expects(:search).with { |key, request| key == "rel/path" }
 
             @file_server.search(@request)
         end
 
-        it "should pass the request's environment" do
+        it "should pass the request" do
             @configuration.stubs(:split_path).returns([@mount, "rel/path"])
 
-            @mount.expects(:search).with { |key, env| env == @request.environment }
+            @mount.expects(:search).with { |key, request| request == @request }
 
             @file_server.search(@request)
         end
@@ -147,7 +147,7 @@ describe Puppet::Indirector::FileServer do
         it "should return nil if searching does not find any full paths" do
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:search).with { |key, env| key == "rel/path" }.returns nil
+            @mount.expects(:search).with { |key, request| key == "rel/path" }.returns nil
 
             @file_server.search(@request).should be_nil
         end
@@ -155,7 +155,7 @@ describe Puppet::Indirector::FileServer do
         it "should create a fileset with each returned path and merge them" do
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:search).with { |key, env| key == "rel/path" }.returns %w{/one /two}
+            @mount.expects(:search).with { |key, request| key == "rel/path" }.returns %w{/one /two}
 
             FileTest.stubs(:exist?).returns true
 
@@ -172,7 +172,7 @@ describe Puppet::Indirector::FileServer do
         it "should create an instance with each path resulting from the merger of the filesets" do
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:search).with { |key, env| key == "rel/path" }.returns []
+            @mount.expects(:search).with { |key, request| key == "rel/path" }.returns []
 
             FileTest.stubs(:exist?).returns true
 
@@ -194,7 +194,7 @@ describe Puppet::Indirector::FileServer do
         it "should set 'links' on the instances if it is set in the request options" do
             @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
 
-            @mount.expects(:search).with { |key, env| key == "rel/path" }.returns []
+            @mount.expects(:search).with { |key, request| key == "rel/path" }.returns []
 
             FileTest.stubs(:exist?).returns true
 
diff --git a/spec/unit/network/authstore.rb b/spec/unit/network/authstore.rb
index 224d67130..bc42e2ce1 100644..100755
--- a/spec/unit/network/authstore.rb
+++ b/spec/unit/network/authstore.rb
@@ -57,7 +57,7 @@ describe Puppet::Network::AuthStore::Declaration do
             @declaration.should be_match(@host,'200.101.99.98')
         end
         it "should not match a similar PQDN" do
-            pending "FQDN consensus"
+            #pending "FQDN consensus"
             @declaration.should_not be_match(@host[0..-2],'200.101.99.98')
         end
     end