diff --git a/lib/puppet/network/http/connection.rb b/lib/puppet/network/http/connection.rb index 8ca35e0bd..a6e5499c5 100644 --- a/lib/puppet/network/http/connection.rb +++ b/lib/puppet/network/http/connection.rb @@ -1,253 +1,237 @@ require 'net/https' require 'puppet/ssl/host' require 'puppet/ssl/configuration' require 'puppet/ssl/validator' require 'puppet/network/authentication' require 'puppet/network/http' require 'uri' module Puppet::Network::HTTP # This will be raised if too many redirects happen for a given HTTP request class RedirectionLimitExceededException < Puppet::Error ; end # This class provides simple methods for issuing various types of HTTP # requests. It's interface is intended to mirror Ruby's Net::HTTP # object, but it provides a few important bits of additional # functionality. Notably: # # * Any HTTPS requests made using this class will use Puppet's SSL # certificate configuration for their authentication, and # * Provides some useful error handling for any SSL errors that occur # during a request. # @api public class Connection include Puppet::Network::Authentication OPTION_DEFAULTS = { :use_ssl => true, :verify => nil, :redirect_limit => 10, } # Creates a new HTTP client connection to `host`:`port`. # @param host [String] the host to which this client will connect to # @param port [Fixnum] the port to which this client will connect to # @param options [Hash] options influencing the properties of the created # connection, # @option options [Boolean] :use_ssl true to connect with SSL, false # otherwise, defaults to true # @option options [#setup_connection] :verify An object that will configure # any verification to do on the connection # @option options [Fixnum] :redirect_limit the number of allowed # redirections, defaults to 10 passing any other option in the options # hash results in a Puppet::Error exception # # @note the HTTP connection itself happens lazily only when {#request}, or # one of the {#get}, {#post}, {#delete}, {#head} or {#put} is called # @note The correct way to obtain a connection is to use one of the factory # methods on {Puppet::Network::HttpPool} # @api private def initialize(host, port, options = {}) @host = host @port = port unknown_options = options.keys - OPTION_DEFAULTS.keys raise Puppet::Error, "Unrecognized option(s): #{unknown_options.map(&:inspect).sort.join(', ')}" unless unknown_options.empty? options = OPTION_DEFAULTS.merge(options) @use_ssl = options[:use_ssl] @verify = options[:verify] @redirect_limit = options[:redirect_limit] @site = Puppet::Network::HTTP::Site.new(@use_ssl ? 'https' : 'http', host, port) @pool = Puppet.lookup(:http_pool) end # @!macro [new] common_options # @param options [Hash] options influencing the request made # @option options [Hash{Symbol => String}] :basic_auth The basic auth # :username and :password to use for the request # @param path [String] # @param headers [Hash{String => String}] # @!macro common_options # @api public def get(path, headers = {}, options = {}) request_with_redirects(Net::HTTP::Get.new(path, headers), options) end # @param path [String] # @param data [String] # @param headers [Hash{String => String}] # @!macro common_options # @api public def post(path, data, headers = nil, options = {}) request = Net::HTTP::Post.new(path, headers) request.body = data request_with_redirects(request, options) end # @param path [String] # @param headers [Hash{String => String}] # @!macro common_options # @api public def head(path, headers = {}, options = {}) request_with_redirects(Net::HTTP::Head.new(path, headers), options) end # @param path [String] # @param headers [Hash{String => String}] # @!macro common_options # @api public def delete(path, headers = {'Depth' => 'Infinity'}, options = {}) request_with_redirects(Net::HTTP::Delete.new(path, headers), options) end # @param path [String] # @param data [String] # @param headers [Hash{String => String}] # @!macro common_options # @api public def put(path, data, headers = nil, options = {}) request = Net::HTTP::Put.new(path, headers) request.body = data request_with_redirects(request, options) end def request(method, *args) self.send(method, *args) end # TODO: These are proxies for the Net::HTTP#request_* methods, which are # almost the same as the "get", "post", etc. methods that we've ported above, # but they are able to accept a code block and will yield to it, which is # necessary to stream responses, e.g. file content. For now # we're not funneling these proxy implementations through our #request # method above, so they will not inherit the same error handling. In the # future we may want to refactor these so that they are funneled through # that method and do inherit the error handling. def request_get(*args, &block) with_connection(@site) do |connection| connection.request_get(*args, &block) end end def request_head(*args, &block) with_connection(@site) do |connection| connection.request_head(*args, &block) end end def request_post(*args, &block) with_connection(@site) do |connection| connection.request_post(*args, &block) end end # end of Net::HTTP#request_* proxies def address @site.host end def port @site.port end def use_ssl? @site.use_ssl? end - def site - @site - end - - def initialize_ssl(http) - @verify.setup_connection(http) - end - private def request_with_redirects(request, options) current_request = request current_site = @site response = nil @redirect_limit.times do |redirection| return response if response with_connection(current_site) do |connection| apply_options_to(current_request, options) current_response = execute_request(connection, current_request) if [301, 302, 307].include?(current_response.code.to_i) # handle the redirection location = URI.parse(current_response['location']) current_site = current_site.move_to(location) # update to the current request path current_request = current_request.class.new(location.path) current_request.body = request.body request.each do |header, value| current_request[header] = value end else response = current_response - - # We used to overwrite the @connection variable during - # each redirect related request. This has the side effect - # of changing the host & port for this connection. That - # makes sense for 301 Moved Permanently, but not 302 Temporary - # or 307 Temporary Redirect, as we should preserve our - # original site for future requests... - @site = current_site end end # and try again... end raise RedirectionLimitExceededException, "Too many HTTP redirections for #{@host}:#{@port}" end def apply_options_to(request, options) if options[:basic_auth] request.basic_auth(options[:basic_auth][:user], options[:basic_auth][:password]) end end def execute_request(connection, request) response = connection.request(request) # Check the peer certs and warn if they're nearing expiration. warn_if_near_expiration(*@verify.peer_certs) response rescue OpenSSL::SSL::SSLError => error if error.message.include? "certificate verify failed" msg = error.message msg << ": [" + @verify.verify_errors.join('; ') + "]" raise Puppet::Error, msg, error.backtrace elsif error.message =~ /hostname.*not match.*server certificate/ leaf_ssl_cert = @verify.peer_certs.last valid_certnames = [leaf_ssl_cert.name, *leaf_ssl_cert.subject_alt_names].uniq msg = valid_certnames.length > 1 ? "one of #{valid_certnames.join(', ')}" : valid_certnames.first msg = "Server hostname '#{connection.address}' did not match server certificate; expected #{msg}" raise Puppet::Error, msg, error.backtrace else raise end end def with_connection(site, &block) response = nil - @pool.with_connection(self) do |conn| + @pool.with_connection(site, @verify) do |conn| response = yield conn end response end end end diff --git a/lib/puppet/network/http/nocache_pool.rb b/lib/puppet/network/http/nocache_pool.rb index f605d0a29..bce2ed398 100644 --- a/lib/puppet/network/http/nocache_pool.rb +++ b/lib/puppet/network/http/nocache_pool.rb @@ -1,17 +1,15 @@ class Puppet::Network::HTTP::NoCachePool - attr_reader :factory - - def initialize - @factory = Puppet::Network::HTTP::Factory.new + def initialize(factory = Puppet::Network::HTTP::Factory.new) + @factory = factory end - def with_connection(conn, &block) - http = @factory.create_connection(conn.site) - conn.initialize_ssl(http) + def with_connection(site, verify, &block) + http = @factory.create_connection(site) + verify.setup_connection(http) yield http end def close # do nothing end end diff --git a/lib/puppet/network/http/pool.rb b/lib/puppet/network/http/pool.rb index bd5b35028..8b33b7364 100644 --- a/lib/puppet/network/http/pool.rb +++ b/lib/puppet/network/http/pool.rb @@ -1,92 +1,91 @@ class Puppet::Network::HTTP::Pool FIFTEEN_SECONDS = 15 attr_reader :factory def initialize(keepalive_timeout = FIFTEEN_SECONDS) @pool = {} @factory = Puppet::Network::HTTP::Factory.new @keepalive_timeout = keepalive_timeout end - def with_connection(conn, &block) + def with_connection(site, verify, &block) reuse = true - http = borrow(conn) + http = borrow(site, verify) begin yield http rescue => detail reuse = false - close_connection(conn.site, http) + close_connection(site, http) raise detail ensure - release(conn.site, http) if reuse + release(site, http) if reuse end end def close @pool.each_pair do |site, sessions| sessions.each do |session| close_connection(site, session.connection) end end @pool.clear end # api private def pool @pool end - def close_connection(site, connection) + def close_connection(site, http) Puppet.debug("Closing connection for #{site}") - connection.finish + http.finish rescue => detail Puppet.log_exception(detail, "Failed to close connection for #{site}: #{detail}") end - def borrow(conn) - site = conn.site + def borrow(site, verify) @pool[site] = active_sessions(site) session = @pool[site].shift if session Puppet.debug("Using cached connection for #{site}") session.connection else http = @factory.create_connection(site) - conn.initialize_ssl(http) + verify.setup_connection(http) Puppet.debug("Starting connection for #{site}") http.start http end end - def release(site, connection) + def release(site, http) expiration = Time.now + @keepalive_timeout - session = Puppet::Network::HTTP::Session.new(connection, expiration) + session = Puppet::Network::HTTP::Session.new(http, expiration) Puppet.debug("Caching connection for #{site}") sessions = @pool[site] if sessions sessions.unshift(session) else @pool[site] = [session] end end def active_sessions(site) now = Time.now sessions = @pool[site] || [] sessions.select do |session| if session.expired?(now) close_connection(site, session.connection) false else true end end end end diff --git a/spec/unit/network/http/connection_spec.rb b/spec/unit/network/http/connection_spec.rb index b217cf12e..7252a00ea 100755 --- a/spec/unit/network/http/connection_spec.rb +++ b/spec/unit/network/http/connection_spec.rb @@ -1,251 +1,271 @@ #! /usr/bin/env ruby require 'spec_helper' require 'puppet/network/http/connection' require 'puppet/network/authentication' describe Puppet::Network::HTTP::Connection do let (:host) { "me" } let (:port) { 54321 } subject { Puppet::Network::HTTP::Connection.new(host, port, :verify => Puppet::SSL::Validator.no_validator) } let (:httpok) { Net::HTTPOK.new('1.1', 200, '') } context "when providing HTTP connections" do context "when initializing http instances" do it "should return an http instance created with the passed host and port" do conn = Puppet::Network::HTTP::Connection.new(host, port, :verify => Puppet::SSL::Validator.no_validator) expect(conn.address).to eq(host) expect(conn.port).to eq(port) end it "should enable ssl on the http instance by default" do conn = Puppet::Network::HTTP::Connection.new(host, port, :verify => Puppet::SSL::Validator.no_validator) expect(conn).to be_use_ssl end it "can disable ssl using an option" do conn = Puppet::Network::HTTP::Connection.new(host, port, :use_ssl => false, :verify => Puppet::SSL::Validator.no_validator) expect(conn).to_not be_use_ssl end it "can enable ssl using an option" do conn = Puppet::Network::HTTP::Connection.new(host, port, :use_ssl => true, :verify => Puppet::SSL::Validator.no_validator) expect(conn).to be_use_ssl end it "should raise Puppet::Error when invalid options are specified" do expect { Puppet::Network::HTTP::Connection.new(host, port, :invalid_option => nil) }.to raise_error(Puppet::Error, 'Unrecognized option(s): :invalid_option') end end end context "when methods that accept a block are called with a block" do let (:host) { "my_server" } let (:port) { 8140 } let (:subject) { Puppet::Network::HTTP::Connection.new(host, port, :use_ssl => false, :verify => Puppet::SSL::Validator.no_validator) } before :each do httpok.stubs(:body).returns "" # This stubbing relies a bit more on knowledge of the internals of Net::HTTP # than I would prefer, but it works on ruby 1.8.7 and 1.9.3, and it seems # valuable enough to have tests for blocks that this is probably warranted. socket = stub_everything("socket") TCPSocket.stubs(:open).returns(socket) Net::HTTP::Post.any_instance.stubs(:exec).returns("") Net::HTTP::Head.any_instance.stubs(:exec).returns("") Net::HTTP::Get.any_instance.stubs(:exec).returns("") Net::HTTPResponse.stubs(:read_new).returns(httpok) end [:request_get, :request_head, :request_post].each do |method| context "##{method}" do it "should yield to the block" do block_executed = false subject.send(method, "/foo", {}) do |response| block_executed = true end block_executed.should == true end end end end context "when validating HTTPS requests" do include PuppetSpec::Files let (:host) { "my_server" } let (:port) { 8140 } it "should provide a useful error message when one is available and certificate validation fails", :unless => Puppet.features.microsoft_windows? do connection = Puppet::Network::HTTP::Connection.new( host, port, :verify => ConstantErrorValidator.new(:fails_with => 'certificate verify failed', :error_string => 'shady looking signature')) expect do connection.get('request') end.to raise_error(Puppet::Error, "certificate verify failed: [shady looking signature]") end it "should provide a helpful error message when hostname was not match with server certificate", :unless => Puppet.features.microsoft_windows? do Puppet[:confdir] = tmpdir('conf') connection = Puppet::Network::HTTP::Connection.new( host, port, :verify => ConstantErrorValidator.new( :fails_with => 'hostname was not match with server certificate', :peer_certs => [Puppet::SSL::CertificateAuthority.new.generate( 'not_my_server', :dns_alt_names => 'foo,bar,baz')])) expect do connection.get('request') end.to raise_error(Puppet::Error) do |error| error.message =~ /Server hostname 'my_server' did not match server certificate; expected one of (.+)/ $1.split(', ').should =~ %w[DNS:foo DNS:bar DNS:baz DNS:not_my_server not_my_server] end end it "should pass along the error message otherwise" do connection = Puppet::Network::HTTP::Connection.new( host, port, :verify => ConstantErrorValidator.new(:fails_with => 'some other message')) expect do connection.get('request') end.to raise_error(/some other message/) end it "should check all peer certificates for upcoming expiration", :unless => Puppet.features.microsoft_windows? do Puppet[:confdir] = tmpdir('conf') cert = Puppet::SSL::CertificateAuthority.new.generate( 'server', :dns_alt_names => 'foo,bar,baz') connection = Puppet::Network::HTTP::Connection.new( host, port, :verify => NoProblemsValidator.new(cert)) Net::HTTP.any_instance.stubs(:request).returns(httpok) connection.expects(:warn_if_near_expiration).with(cert) connection.get('request') end class ConstantErrorValidator def initialize(args) @fails_with = args[:fails_with] @error_string = args[:error_string] || "" @peer_certs = args[:peer_certs] || [] end def setup_connection(connection) connection.stubs(:request).with do true end.raises(OpenSSL::SSL::SSLError.new(@fails_with)) end def peer_certs @peer_certs end def verify_errors [@error_string] end end class NoProblemsValidator def initialize(cert) @cert = cert end def setup_connection(connection) end def peer_certs [@cert] end def verify_errors [] end end end context "when response is a redirect" do - let (:other_host) { "redirected" } - let (:other_port) { 9292 } + let (:site) { Puppet::Network::HTTP::Site.new('http', 'my_server', 8140) } + let (:other_site) { Puppet::Network::HTTP::Site.new('http', 'redirected', 9292) } let (:other_path) { "other-path" } - let (:subject) { Puppet::Network::HTTP::Connection.new("my_server", 8140, :use_ssl => false, :verify => Puppet::SSL::Validator.no_validator) } - let (:httpredirection) { Net::HTTPFound.new('1.1', 302, 'Moved Temporarily') } - - before :each do - httpredirection['location'] = "http://#{other_host}:#{other_port}/#{other_path}" - httpredirection.stubs(:read_body).returns("This resource has moved") - - socket = stub_everything("socket") - TCPSocket.stubs(:open).returns(socket) + let (:verify) { Puppet::SSL::Validator.no_validator } + let (:subject) { Puppet::Network::HTTP::Connection.new(site.host, site.port, :use_ssl => false, :verify => verify) } + let (:httpredirection) do + response = Net::HTTPFound.new('1.1', 302, 'Moved Temporarily') + response['location'] = "#{other_site.addr}/#{other_path}" + response.stubs(:read_body).returns("This resource has moved") + response + end - Net::HTTP::Get.any_instance.stubs(:exec).returns("") - Net::HTTP::Post.any_instance.stubs(:exec).returns("") + def create_connection(site, options) + options[:use_ssl] = site.use_ssl? + Puppet::Network::HTTP::Connection.new(site.host, site.port, options) end it "should redirect to the final resource location" do - httpok.stubs(:read_body).returns(:body) - Net::HTTPResponse.stubs(:read_new).returns(httpredirection).then.returns(httpok) + http = stub('http') + http.stubs(:request).returns(httpredirection).then.returns(httpok) - subject.get("/foo").body.should == :body - subject.port.should == other_port - subject.address.should == other_host + seq = sequence('redirection') + pool = Puppet.lookup(:http_pool) + pool.expects(:with_connection).with(site, anything).yields(http).in_sequence(seq) + pool.expects(:with_connection).with(other_site, anything).yields(http).in_sequence(seq) + + conn = create_connection(site, :verify => verify) + conn.get('/foo') end - it "should raise an error after too many redirections" do - Net::HTTPResponse.stubs(:read_new).returns(httpredirection) + def expects_redirection(conn, &block) + http = stub('http') + http.stubs(:request).returns(httpredirection) + + pool = Puppet.lookup(:http_pool) + pool.expects(:with_connection).with(site, anything).yields(http) + pool + end + def expects_limit_exceeded(conn) expect { - subject.get("/foo") + conn.get('/') }.to raise_error(Puppet::Network::HTTP::RedirectionLimitExceededException) end + + it "should raise an exception when the redirect limit is exceeded" do + conn = create_connection(site, :verify => verify, :redirect_limit => 3) + + pool = expects_redirection(conn) + pool.expects(:with_connection).with(other_site, anything).times(2) + + expects_limit_exceeded(conn) + end end it "allows setting basic auth on get requests" do expect_request_with_basic_auth subject.get('/path', nil, :basic_auth => { :user => 'user', :password => 'password' }) end it "allows setting basic auth on post requests" do expect_request_with_basic_auth subject.post('/path', 'data', nil, :basic_auth => { :user => 'user', :password => 'password' }) end it "allows setting basic auth on head requests" do expect_request_with_basic_auth subject.head('/path', nil, :basic_auth => { :user => 'user', :password => 'password' }) end it "allows setting basic auth on delete requests" do expect_request_with_basic_auth subject.delete('/path', nil, :basic_auth => { :user => 'user', :password => 'password' }) end it "allows setting basic auth on put requests" do expect_request_with_basic_auth subject.put('/path', 'data', nil, :basic_auth => { :user => 'user', :password => 'password' }) end def expect_request_with_basic_auth Net::HTTP.any_instance.expects(:request).with do |request| expect(request['authorization']).to match(/^Basic/) end.returns(httpok) end end diff --git a/spec/unit/network/http/nocache_pool_spec.rb b/spec/unit/network/http/nocache_pool_spec.rb index d5999e980..69e2d2e9a 100755 --- a/spec/unit/network/http/nocache_pool_spec.rb +++ b/spec/unit/network/http/nocache_pool_spec.rb @@ -1,27 +1,43 @@ #! /usr/bin/env ruby require 'spec_helper' require 'puppet/network/http' require 'puppet/network/http/connection' describe Puppet::Network::HTTP::NoCachePool do - it 'returns a new connection' do - http = stub('http') - verify = stub('verify', :setup_connection => nil) + let(:site) { Puppet::Network::HTTP::Site.new('https', 'rubygems.org', 443) } + let(:verify) { stub('verify', :setup_connection => nil) } - site = Puppet::Network::HTTP::Site.new('https', 'rubygems.org', 443) - pool = Puppet::Network::HTTP::NoCachePool.new - pool.factory.expects(:create_connection).with(site).returns(http) + it 'yields a connection' do + http = stub('http') - conn = Puppet::Network::HTTP::Connection.new(site.host, site.port, :use_ssl => site.use_ssl?, :verify => verify) + factory = Puppet::Network::HTTP::Factory.new + factory.stubs(:create_connection).returns(http) + pool = Puppet::Network::HTTP::NoCachePool.new(factory) - yielded_http = nil - pool.with_connection(conn) { |h| yielded_http = h } + expect { |b| + pool.with_connection(site, verify, &b) + }.to yield_with_args(http) + end + + it 'yields a new connection each time' do + http1 = stub('http1') + http2 = stub('http2') + + factory = Puppet::Network::HTTP::Factory.new + factory.stubs(:create_connection).returns(http1).then.returns(http2) + pool = Puppet::Network::HTTP::NoCachePool.new(factory) + + expect { |b| + pool.with_connection(site, verify, &b) + }.to yield_with_args(http1) - expect(yielded_http).to eq(http) + expect { |b| + pool.with_connection(site, verify, &b) + }.to yield_with_args(http2) end it 'has a close method' do Puppet::Network::HTTP::NoCachePool.new.close end end diff --git a/spec/unit/network/http/pool_spec.rb b/spec/unit/network/http/pool_spec.rb index 604107e4d..86f079bb6 100755 --- a/spec/unit/network/http/pool_spec.rb +++ b/spec/unit/network/http/pool_spec.rb @@ -1,232 +1,227 @@ #! /usr/bin/env ruby require 'spec_helper' require 'puppet/network/http' require 'puppet/network/http_pool' describe Puppet::Network::HTTP::Pool do before :each do Puppet::SSL::Key.indirection.terminus_class = :memory Puppet::SSL::CertificateRequest.indirection.terminus_class = :memory end let(:site) do Puppet::Network::HTTP::Site.new('https', 'rubygems.org', 443) end let(:different_site) do Puppet::Network::HTTP::Site.new('https', 'github.com', 443) end let(:verify) do stub('verify', :setup_connection => nil) end - let(:puppet_conn) do - Puppet::Network::HTTP::Connection.new(site.host, site.port, :use_ssl => site.use_ssl?, :verify => verify) - end - def create_pool Puppet::Network::HTTP::Pool.new end def create_pool_with_connections(site, *connections) pool = Puppet::Network::HTTP::Pool.new connections.each do |conn| pool.release(site, conn) end pool end def create_pool_with_expired_connections(site, *connections) # setting keepalive timeout to -1 ensures any newly added # connections have already expired pool = Puppet::Network::HTTP::Pool.new(-1) connections.each do |conn| pool.release(site, conn) end pool end def create_connection(site) stub(site.addr, :started? => false, :start => nil, :finish => nil) end context 'when yielding a connection' do it 'yields a connection' do conn = create_connection(site) pool = create_pool_with_connections(site, conn) - yielded_conn = nil - pool.with_connection(puppet_conn) { |c| yielded_conn = c } - - expect(yielded_conn).to eq(conn) + expect { |b| + pool.with_connection(site, verify, &b) + }.to yield_with_args(conn) end it 'returns the connection to the pool' do conn = create_connection(site) pool = create_pool pool.release(site, conn) - pool.with_connection(puppet_conn) do |c| + pool.with_connection(site, verify) do |c| expect(pool.pool[site]).to eq([]) end expect(pool.pool[site].first.connection).to eq(conn) end it 'propagates exceptions' do conn = create_connection(site) pool = create_pool pool.release(site, conn) expect { - pool.with_connection(puppet_conn) do |c| + pool.with_connection(site, verify) do |c| raise IOError, 'connection reset' end }.to raise_error(IOError, 'connection reset') end it 'closes the yielded connection when an error occurs' do # we're not distinguishing between network errors that would # suggest we close the socket, and other errors conn = create_connection(site) pool = create_pool pool.release(site, conn) pool.expects(:release).with(site, conn).never - pool.with_connection(puppet_conn) do |c| + pool.with_connection(site, verify) do |c| raise IOError, 'connection reset' end rescue nil expect(pool.pool[site]).to eq([]) end end context 'when borrowing' do it 'returns a new connection if the pool is empty' do conn = create_connection(site) pool = create_pool pool.factory.expects(:create_connection).with(site).returns(conn) - expect(pool.borrow(puppet_conn)).to eq(conn) + expect(pool.borrow(site, verify)).to eq(conn) end it 'returns a matching connection' do conn = create_connection(site) pool = create_pool_with_connections(site, conn) pool.factory.expects(:create_connection).never - expect(pool.borrow(puppet_conn)).to eq(conn) + expect(pool.borrow(site, verify)).to eq(conn) end it 'returns a new connection if there are no matching sites' do different_conn = create_connection(different_site) pool = create_pool_with_connections(different_site, different_conn) conn = create_connection(site) pool.factory.expects(:create_connection).with(site).returns(conn) - expect(pool.borrow(puppet_conn)).to eq(conn) + expect(pool.borrow(site, verify)).to eq(conn) end it 'returns started connections' do conn = create_connection(site) conn.expects(:start) pool = create_pool pool.factory.expects(:create_connection).with(site).returns(conn) - expect(pool.borrow(puppet_conn)).to eq(conn) + expect(pool.borrow(site, verify)).to eq(conn) end it "doesn't start a cached connection" do conn = create_connection(site) conn.expects(:start).never pool = create_pool_with_connections(site, conn) - pool.borrow(puppet_conn) + pool.borrow(site, verify) end it 'returns the most recently used connection from the pool' do least_recently_used = create_connection(site) most_recently_used = create_connection(site) pool = create_pool_with_connections(site, least_recently_used, most_recently_used) - expect(pool.borrow(puppet_conn)).to eq(most_recently_used) + expect(pool.borrow(site, verify)).to eq(most_recently_used) end it 'finishes expired connections' do conn = create_connection(site) conn.expects(:finish) pool = create_pool_with_expired_connections(site, conn) pool.factory.expects(:create_connection => stub('conn', :start => nil)) - pool.borrow(puppet_conn) + pool.borrow(site, verify) end it 'logs an exception if it fails to close an expired connection' do Puppet.expects(:log_exception).with(is_a(IOError), "Failed to close connection for #{site}: read timeout") conn = create_connection(site) conn.expects(:finish).raises(IOError, 'read timeout') pool = create_pool_with_expired_connections(site, conn) pool.factory.expects(:create_connection => stub('open_conn', :start => nil)) - pool.borrow(puppet_conn) + pool.borrow(site, verify) end end context 'when releasing a connection' do it 'adds the connection to an empty pool' do conn = create_connection(site) pool = create_pool pool.release(site, conn) expect(pool.pool[site].first.connection).to eq(conn) end it 'adds the connection to a pool with a connection for the same site' do pool = create_pool pool.release(site, create_connection(site)) pool.release(site, create_connection(site)) expect(pool.pool[site].count).to eq(2) end it 'adds the connection to a pool with a connection for a different site' do pool = create_pool pool.release(site, create_connection(site)) pool.release(different_site, create_connection(different_site)) expect(pool.pool[site].count).to eq(1) expect(pool.pool[different_site].count).to eq(1) end it 'should ignore expired connections' do pending("No way to know if client is releasing an expired connection") end end context 'when closing' do it 'clears the pool' do pool = create_pool pool.close expect(pool.pool).to be_empty end it 'closes all cached connections' do conn = create_connection(site) conn.expects(:finish) pool = create_pool_with_connections(site, conn) pool.close end end end