So the missing piece with this new setup seems to be that you *must* create a fileserver.conf entry of:
[modules] allow * for modules in environments to work correctly, whether or not you represent the URI in the old style: puppet:///modulename/filename.txt or the new style: puppet:///modules/modulename/filename.txt Patch working happily after that. On Sun, Jul 19, 2009 at 11:59 PM, Luke Kanies<[email protected]> wrote: > > +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" } >> - �[email protected][:node] = "mynode" >> + �[email protected](: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"]) >> >> - �[email protected][:node] = "mynode" >> - �[email protected][:ipaddress] = "myip" >> + �[email protected](:node).returns("mynode") >> + �[email protected](: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 > > > > > -- Nigel Kersten [email protected] System Administrator Google, Inc. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
