Actually, the issue is:
*  when the web server gets the request, it creates an indirection
request, filling attributes like ip or node from the HTTP request.
To do this, all the interesting attributes are given in a hash
(called options, see P::I::Request#new).
Once the request is properly initialized the options hash doesn't
contain the ip or node information (see set_attributes)

* the request is then transmitted to the file_serving layer,
which happily wants to use the node attribute to find environments or
perform authorization.
Unfortunately it fetches the node value from the request options hash,
not the request itself.
Since this node information is empty, puppet fails to find the
proper mount point, and fails the download.

This change makes sure we pass all the way down the node and fix
the authorization check.

Signed-off-by: Brice Figureau <[email protected]>
---
 lib/puppet/file_serving/configuration.rb |    2 +-
 lib/puppet/indirector/file_server.rb     |    4 ++--
 spec/unit/file_serving/configuration.rb  |    4 ++--
 spec/unit/indirector/file_server.rb      |    4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/puppet/file_serving/configuration.rb 
b/lib/puppet/file_serving/configuration.rb
index cb6cc36..6f36d27 100644
--- a/lib/puppet/file_serving/configuration.rb
+++ b/lib/puppet/file_serving/configuration.rb
@@ -72,7 +72,7 @@ class Puppet::FileServing::Configuration
 
         raise(ArgumentError, "Cannot find file: Invalid path '%s'" % 
mount_name) unless mount_name =~ %r{^[-\w]+$}
 
-        return nil unless mount = find_mount(mount_name, 
request.options[:node])
+        return nil unless mount = find_mount(mount_name, request.node)
         if mount.name == "modules" and mount_name != "modules"
             # yay backward-compatibility
             path = "%s/%s" % [mount_name, path]
diff --git a/lib/puppet/indirector/file_server.rb 
b/lib/puppet/indirector/file_server.rb
index e3bde15..5fe744a 100644
--- a/lib/puppet/indirector/file_server.rb
+++ b/lib/puppet/indirector/file_server.rb
@@ -19,7 +19,7 @@ class Puppet::Indirector::FileServer < 
Puppet::Indirector::Terminus
 
         # If we're not serving this mount, then access is denied.
         return false unless mount
-        return mount.allowed?(request.options[:node], 
request.options[:ipaddress])
+        return mount.allowed?(request.node, request.ip)
     end
 
     # Find our key using the fileserver.
@@ -30,7 +30,7 @@ class Puppet::Indirector::FileServer < 
Puppet::Indirector::Terminus
 
         # The mount checks to see if the file exists, and returns nil
         # if not.
-        return nil unless path = mount.find(relative_path, request.options)
+        return nil unless path = mount.find(relative_path, :node => 
request.node)
         result = model.new(path)
         result.links = request.options[:links] if request.options[:links]
         result.collect
diff --git a/spec/unit/file_serving/configuration.rb 
b/spec/unit/file_serving/configuration.rb
index 60515bf..9545f01 100755
--- a/spec/unit/file_serving/configuration.rb
+++ b/spec/unit/file_serving/configuration.rb
@@ -150,7 +150,7 @@ describe Puppet::FileServing::Configuration do
             @config = Puppet::FileServing::Configuration.create
             @config.stubs(:find_mount)
 
-            @request = stub 'request', :key => "foo/bar/baz", :options => {}
+            @request = stub 'request', :key => "foo/bar/baz", :options => {}, 
:node => nil
         end
 
         it "should reread the configuration" do
@@ -179,7 +179,7 @@ describe Puppet::FileServing::Configuration do
 
         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" }
-            @request.options[:node] = "mynode"
+            @request.stubs(:node).returns("mynode")
 
             @config.split_path(@request)
         end
diff --git a/spec/unit/indirector/file_server.rb 
b/spec/unit/indirector/file_server.rb
index a80f5ae..7dab320 100755
--- a/spec/unit/indirector/file_server.rb
+++ b/spec/unit/indirector/file_server.rb
@@ -241,8 +241,8 @@ describe Puppet::Indirector::FileServer do
             @mount = stub 'mount'
             
@configuration.expects(:split_path).with(@request).returns([...@mount, 
"rel/path"])
 
-            @request.options[:node] = "mynode"
-            @request.options[:ipaddress] = "myip"
+            @request.stubs(:node).returns("mynode")
+            @request.stubs(:ip).returns("myip")
             @mount.expects(:allowed?).with("mynode", "myip").returns 
"something"
 
             @file_server.authorized?(@request).should == "something"
-- 
1.6.0.2


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to