Fixing #1544 - plugins in modules now works again

We had to fix the fileserving plumbing to use the request
environment instead of trying to use the node environment.
This was apparently never fixed after we added the environment
to the URI in REST calls.

There's still a bit of refactoring left to clean up the APIs used
in some of this code.

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

4 files changed, 42 insertions, 51 deletions

diff --git a/spec/unit/file_serving/mount.rb b/spec/unit/file_serving/mount.rb
index e9e1f0860..cbb97b0d9 100755
--- a/spec/unit/file_serving/mount.rb
+++ b/spec/unit/file_serving/mount.rb
@@ -4,26 +4,8 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 require 'puppet/file_serving/mount'
 
 describe Puppet::FileServing::Mount do
-    before do
-        @mount = Puppet::FileServing::Mount.new("foo")
-    end
-
-    it "should be able to look up a node's environment" do
-        Puppet::Node.expects(:find).with("mynode").returns mock('node', :environment => "myenv")
-        Puppet::Node::Environment.expects(:new).with("myenv").returns "eh"
-
-        @mount.environment("mynode").should == "eh"
-    end
-
-    it "should use the default environment if no node information is provided" do
-        Puppet::Node.expects(:find).with("mynode").returns nil
-        Puppet::Node::Environment.expects(:new).with(nil).returns "eh"
-
-        @mount.environment("mynode").should == "eh"
-    end
-
     it "should use 'mount[$name]' as its string form" do
-        @mount.to_s.should == "mount[foo]"
+        Puppet::FileServing::Mount.new("foo").to_s.should == "mount[foo]"
     end
 end
 
diff --git a/spec/unit/file_serving/mount/plugins.rb b/spec/unit/file_serving/mount/plugins.rb
index ef842d12b..b3a32b70d 100755
--- a/spec/unit/file_serving/mount/plugins.rb
+++ b/spec/unit/file_serving/mount/plugins.rb
@@ -6,65 +6,57 @@ require 'puppet/file_serving/mount/plugins'
 describe Puppet::FileServing::Mount::Plugins, "when finding files" do
     before do
         @mount = Puppet::FileServing::Mount::Plugins.new("modules")
-
-        @environment = stub 'environment', :module => nil
-        @mount.stubs(:environment).returns @environment
     end
 
-    it "should use the node's environment to find the modules" do
+    it "should use the provided environment to find the modules" do
         env = mock 'env'
-        @mount.expects(:environment).with("mynode").returns env
         env.expects(:modules).returns []
 
-        @mount.find("foo", :node => "mynode")
+        @mount.find("foo", env)
     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
 
-        @environment.expects(:modules).returns [mod]
-        @mount.find("foo/bar").should be_nil
+        env = stub 'env', :modules => []
+        @mount.find("foo/bar", env).should be_nil
     end
 
     it "should return the file path from the module" do
         mod = mock 'module'
         mod.stubs(:plugin).with("foo/bar").returns "eh"
 
-        @environment.expects(:modules).returns [mod]
-        @mount.find("foo/bar").should == "eh"
+        env = stub 'env', :modules => [mod]
+        @mount.find("foo/bar", env).should == "eh"
     end
 end
 
 describe Puppet::FileServing::Mount::Plugins, "when searching for files" do
     before do
         @mount = Puppet::FileServing::Mount::Plugins.new("modules")
-
-        @environment = stub 'environment', :module => nil
-        @mount.stubs(:environment).returns @environment
     end
 
     it "should use the node's environment to find the modules" do
         env = mock 'env'
-        @mount.expects(:environment).with("mynode").returns env
         env.expects(:modules).returns []
 
-        @mount.search("foo", :node => "mynode")
+        @mount.search("foo", env)
     end
 
     it "should return nil if no modules can be found that have plugins" do
         mod = mock 'module'
         mod.stubs(:plugins?).returns false
 
-        @environment.expects(:modules).returns [mod]
-        @mount.search("foo/bar").should be_nil
+        env = stub 'env', :modules => []
+        @mount.search("foo/bar", env).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"
 
-        @environment.expects(:modules).returns [one, two]
-        @mount.search("foo/bar").should == %w{/one /two}
+        env = stub 'env', :modules => [one, two]
+        @mount.search("foo/bar", env).should == %w{/one /two}
     end
 end
diff --git a/spec/unit/indirector/file_server.rb b/spec/unit/indirector/file_server.rb
index 7dab320c1..a2e2ff811 100755
--- a/spec/unit/indirector/file_server.rb
+++ b/spec/unit/indirector/file_server.rb
@@ -28,7 +28,7 @@ describe Puppet::Indirector::FileServer do
         @configuration = mock 'configuration'
         Puppet::FileServing::Configuration.stubs(:create).returns(@configuration)
 
-        @request = Puppet::Indirector::Request.new(:myind, :mymethod, @uri)
+        @request = Puppet::Indirector::Request.new(:myind, :mymethod, @uri, :environment => "myenv")
     end
 
     describe "when finding files" do
@@ -52,7 +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, options| key == "rel/path" }
+            @mount.expects(:find).with { |key, env| key == "rel/path" }
+
+            @file_server.find(@request)
+        end
+
+        it "should pass the request's environment when finding a file" do
+            @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
+
+            @mount.expects(:find).with { |key, env| env == @request.environment }
 
             @file_server.find(@request)
         end
@@ -60,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, options| key == "rel/path" }.returns nil
+            @mount.expects(:find).with { |key, env| key == "rel/path" }.returns nil
 
             @file_server.find(@request).should be_nil
         end
@@ -68,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, options| key == "rel/path" }.returns "/my/file"
+            @mount.expects(:find).with { |key, env| key == "rel/path" }.returns "/my/file"
 
             @model.expects(:new).with("/my/file").returns @instance
 
@@ -79,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, options| key == "rel/path" }.returns "/my/file"
+            @mount.expects(:find).with { |key, env| key == "rel/path" }.returns "/my/file"
 
             @model.expects(:new).with("/my/file").returns @instance
 
@@ -92,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, options| key == "rel/path" }.returns "/my/file"
+            @mount.expects(:find).with { |key, env| key == "rel/path" }.returns "/my/file"
 
             @model.expects(:new).with("/my/file").returns @instance
 
@@ -123,7 +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, options| key == "rel/path" }
+            @mount.expects(:search).with { |key, env| key == "rel/path" }
+
+            @file_server.search(@request)
+        end
+
+        it "should pass the request's environment" do
+            @configuration.stubs(:split_path).returns([@mount, "rel/path"])
+
+            @mount.expects(:search).with { |key, env| env == @request.environment }
 
             @file_server.search(@request)
         end
@@ -131,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, options| key == "rel/path" }.returns nil
+            @mount.expects(:search).with { |key, env| key == "rel/path" }.returns nil
 
             @file_server.search(@request).should be_nil
         end
@@ -139,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, options| key == "rel/path" }.returns %w{/one /two}
+            @mount.expects(:search).with { |key, env| key == "rel/path" }.returns %w{/one /two}
 
             FileTest.stubs(:exist?).returns true
 
@@ -156,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, options| key == "rel/path" }.returns []
+            @mount.expects(:search).with { |key, env| key == "rel/path" }.returns []
 
             FileTest.stubs(:exist?).returns true
 
@@ -178,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, options| key == "rel/path" }.returns []
+            @mount.expects(:search).with { |key, env| key == "rel/path" }.returns []
 
             FileTest.stubs(:exist?).returns true
 
diff --git a/spec/unit/node/environment.rb b/spec/unit/node/environment.rb
index f8b2dea7f..18747d1b7 100755
--- a/spec/unit/node/environment.rb
+++ b/spec/unit/node/environment.rb
@@ -105,9 +105,10 @@ describe Puppet::Node::Environment do
             module_path = %w{/one /two}.join(File::PATH_SEPARATOR)
             env.expects(:modulepath).returns module_path
 
-            Puppet::Module.expects(:each_module).with(module_path).multiple_yields("mod1", "mod2")
+            mods = [Puppet::Module.new('mod1'), Puppet::Module.new("mod2")]
+            Puppet::Module.expects(:each_module).with(module_path).multiple_yields(*mods)
 
-            env.modules.should == %w{mod1 mod2}
+            env.modules.should == mods
         end
 
         it "should be able to return an individual module that exists in its module path" do