+1, definitely On Jul 18, 2009, at 6:35 AM, Brice Figureau wrote:
> > 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 > > > > -- I never think of the future. It comes soon enough. --Albert Einstein --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
