diff options
| -rw-r--r-- | lib/puppet/indirector/request.rb | 36 | ||||
| -rw-r--r-- | lib/puppet/indirector/rest.rb | 40 | ||||
| -rwxr-xr-x | spec/unit/indirector/request.rb | 31 | ||||
| -rwxr-xr-x | spec/unit/indirector/rest.rb | 80 |
4 files changed, 141 insertions, 46 deletions
diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb index 98fa38885..f1d02cead 100644 --- a/lib/puppet/indirector/request.rb +++ b/lib/puppet/indirector/request.rb @@ -5,6 +5,8 @@ require 'puppet/indirector' class Puppet::Indirector::Request attr_accessor :indirection_name, :key, :method, :options, :instance, :node, :ip, :authenticated + attr_accessor :server, :port, :uri, :protocol + # Is this an authenticated request? def authenticated? # Double negative, so we just get true or false @@ -28,7 +30,15 @@ class Puppet::Indirector::Request end if key.is_a?(String) or key.is_a?(Symbol) - @key = key + # If the request key is a URI, then we need to treat it specially, + # because it rewrites the key. We could otherwise strip server/port/etc + # info out in the REST class, but it seemed bad design for the REST + # class to rewrite the key. + if key.to_s =~ /^\w+:\/\// # it's a URI + set_uri_key(key) + else + @key = key + end else @instance = key @key = @instance.name @@ -39,4 +49,28 @@ class Puppet::Indirector::Request def indirection Puppet::Indirector::Indirection.instance(@indirection_name) end + + private + + # Parse the key as a URI, setting attributes appropriately. + def set_uri_key(key) + @uri = key + begin + uri = URI.parse(URI.escape(key)) + rescue => detail + raise ArgumentError, "Could not understand URL %s: %s" % [source, detail.to_s] + end + @server = uri.host if uri.host + + # If the URI class can look up the scheme, it will provide a port, + # otherwise it will default to '0'. + if uri.port.to_i == 0 and uri.scheme == "puppet" + @port = Puppet.settings[:masterport].to_i + else + @port = uri.port.to_i + end + + @protocol = uri.scheme + @key = uri.path.sub(/^\//, '') + end end diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 4389dfb7e..a2bfc5d9a 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -1,8 +1,33 @@ require 'net/http' require 'uri' +require 'puppet/network/http_pool' + # Access objects via REST class Puppet::Indirector::REST < Puppet::Indirector::Terminus + + class << self + attr_reader :server_setting, :port_setting + end + + # Specify the setting that we should use to get the server name. + def self.use_server_setting(setting) + @server_setting = setting + end + + def self.server + return Puppet.settings[server_setting || :server] + end + + # Specify the setting that we should use to get the port. + def self.use_port_setting(setting) + @port_setting = setting + end + + def self.port + return Puppet.settings[port_setting || :masterport].to_i + end + # Figure out the content type, turn that into a format, and use the format # to extract the body of the response. def deserialize(response, multiple = false) @@ -33,20 +58,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus end def network(request) - if request.key =~ /^\w+:\/\// # it looks like a URI - begin - uri = URI.parse(URI.escape(request.key)) - rescue => detail - raise ArgumentError, "Could not understand URL %s: %s" % [source, detail.to_s] - end - server = uri.host || Puppet[:server] - port = uri.port.to_i == 0 ? Puppet[:masterport].to_i : uri.port.to_i - else - server = Puppet[:server] - port = Puppet[:masterport].to_i - end - - Puppet::Network::HttpPool.http_instance(server, port) + Puppet::Network::HttpPool.http_instance(request.server || self.class.server, request.port || self.class.port) end def find(request) diff --git a/spec/unit/indirector/request.rb b/spec/unit/indirector/request.rb index f7702f821..2476a3d7f 100755 --- a/spec/unit/indirector/request.rb +++ b/spec/unit/indirector/request.rb @@ -75,6 +75,37 @@ describe Puppet::Indirector::Request do it "should keep its options as a hash even if another option is specified" do Puppet::Indirector::Request.new(:ind, :method, :key, :foo => "bar").options.should be_instance_of(Hash) end + + describe "and the request key is a URI" do + it "should set the protocol to the URI scheme" do + Puppet::Indirector::Request.new(:ind, :method, "http://host/stuff").protocol.should == "http" + end + + it "should set the server if a server is provided" do + Puppet::Indirector::Request.new(:ind, :method, "http://host/stuff").server.should == "host" + end + + it "should set the server and port if both are provided" do + Puppet::Indirector::Request.new(:ind, :method, "http://host:543/stuff").port.should == 543 + end + + it "should default to the masterport if the URI scheme is 'puppet'" do + Puppet.settings.expects(:value).with(:masterport).returns "321" + Puppet::Indirector::Request.new(:ind, :method, "puppet://host/stuff").port.should == 321 + end + + it "should use the provided port if the URI scheme is not 'puppet'" do + Puppet::Indirector::Request.new(:ind, :method, "http://host/stuff").port.should == 80 + end + + it "should set the request key to the path from the URI" do + Puppet::Indirector::Request.new(:ind, :method, "http:///stuff").key.should == "stuff" + end + + it "should set the :uri attribute to the full URI" do + Puppet::Indirector::Request.new(:ind, :method, "http:///stuff").uri.should == "http:///stuff" + end + end end it "should look use the Indirection class to return the appropriate indirection" do diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 0c88ea4df..e7afbb5b2 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -45,6 +45,38 @@ describe Puppet::Indirector::REST do @searcher = @rest_class.new end + it "should have a method for specifying what setting a subclass should use to retrieve its server" do + @rest_class.should respond_to(:use_server_setting) + end + + it "should use any specified setting to pick the server" do + @rest_class.expects(:server_setting).returns :servset + Puppet.settings.expects(:value).with(:servset).returns "myserver" + @rest_class.server.should == "myserver" + end + + it "should default to :server for the server setting" do + @rest_class.expects(:server_setting).returns nil + Puppet.settings.expects(:value).with(:server).returns "myserver" + @rest_class.server.should == "myserver" + end + + it "should have a method for specifying what setting a subclass should use to retrieve its port" do + @rest_class.should respond_to(:use_port_setting) + end + + it "should use any specified setting to pick the port" do + @rest_class.expects(:port_setting).returns :servset + Puppet.settings.expects(:value).with(:servset).returns "321" + @rest_class.port.should == 321 + end + + it "should default to :port for the port setting" do + @rest_class.expects(:port_setting).returns nil + Puppet.settings.expects(:value).with(:masterport).returns "543" + @rest_class.port.should == 543 + end + describe "when deserializing responses" do it "should return nil if the response code is 404" do response = mock 'response' @@ -91,40 +123,26 @@ describe Puppet::Indirector::REST do Puppet.settings.stubs(:value).returns("rest_testing") end - describe "and the request key is not a URL" do - it "should use the :server setting as the host and the :masterport setting (as an Integer) as the port" do - @request = stub 'request', :key => "foo" - Puppet.settings.expects(:value).with(:server).returns "myserver" - Puppet.settings.expects(:value).with(:masterport).returns "1234" - Puppet::Network::HttpPool.expects(:http_instance).with("myserver", 1234).returns "myconn" - @searcher.network(@request).should == "myconn" - end + it "should use the class's server and port if the indirection request provides neither" do + @request = stub 'request', :key => "foo", :server => nil, :port => nil + @searcher.class.expects(:port).returns 321 + @searcher.class.expects(:server).returns "myserver" + Puppet::Network::HttpPool.expects(:http_instance).with("myserver", 321).returns "myconn" + @searcher.network(@request).should == "myconn" end - describe "and the request key is a URL" do - it "should use the :server setting and masterport setting, as an Integer, as the host if no values are provided" do - @request = stub 'request', :key => "puppet:///key" - - Puppet.settings.expects(:value).with(:server).returns "myserver" - Puppet.settings.expects(:value).with(:masterport).returns "1234" - Puppet::Network::HttpPool.expects(:http_instance).with("myserver", 1234).returns "myconn" - @searcher.network(@request).should == "myconn" - end - - it "should use the server if one is provided" do - @request.stubs(:key).returns "puppet://host/key" - - Puppet.settings.expects(:value).with(:masterport).returns "1234" - Puppet::Network::HttpPool.expects(:http_instance).with("host", 1234).returns "myconn" - @searcher.network(@request).should == "myconn" - end - - it "should use the server and port if they are provided" do - @request.stubs(:key).returns "puppet://host:123/key" + it "should use the server from the indirection request if one is present" do + @request = stub 'request', :key => "foo", :server => "myserver", :port => nil + @searcher.class.stubs(:port).returns 321 + Puppet::Network::HttpPool.expects(:http_instance).with("myserver", 321).returns "myconn" + @searcher.network(@request).should == "myconn" + end - Puppet::Network::HttpPool.expects(:http_instance).with("host", 123).returns "myconn" - @searcher.network(@request).should == "myconn" - end + it "should use the port from the indirection request if one is present" do + @request = stub 'request', :key => "foo", :server => nil, :port => 321 + @searcher.class.stubs(:server).returns "myserver" + Puppet::Network::HttpPool.expects(:http_instance).with("myserver", 321).returns "myconn" + @searcher.network(@request).should == "myconn" end end |