Merge pull request #7856 from melissa/ticket/master/pup-10039-serverlist-resolver

(PUP-10039) Add ServerList resolver

Author: joshcooper

lib/puppet/http.rb

@@ -20,6 +20,7 @@ module HTTP
     require 'puppet/http/service/ca'
     require 'puppet/http/session'
     require 'puppet/http/resolver'
+    require 'puppet/http/resolver/server_list'
     require 'puppet/http/resolver/settings'
     require 'puppet/http/resolver/srv'
     require 'puppet/http/client'

lib/puppet/http/client.rb

@@ -147,10 +147,10 @@ def build_resolvers
     resolvers = []
 
     if Puppet[:use_srv_records]
-      resolvers << Puppet::HTTP::Resolver::SRV.new(domain: Puppet[:srv_domain])
+      resolvers << Puppet::HTTP::Resolver::SRV.new(self, domain: Puppet[:srv_domain])
     end
 
-    resolvers << Puppet::HTTP::Resolver::Settings.new
+    resolvers << Puppet::HTTP::Resolver::Settings.new(self)
     resolvers.freeze
   end
 end

lib/puppet/http/resolver.rb

@@ -1,5 +1,18 @@
 class Puppet::HTTP::Resolver
-  def resolve(session, name, &block)
+  def initialize(client)
+    @client = client
+  end
+
+  def resolve(session, name, ssl_context: nil)
     raise NotImplementedError
   end
+
+  def check_connection?(session, service, ssl_context: nil)
+    service.connect(ssl_context: ssl_context)
+    return true
+  rescue Puppet::HTTP::ConnectionError => e
+    session.add_exception(e)
+    Puppet.debug("Connection to #{service.url} failed, trying next route: #{e.message}")
+    return false
+  end
 end

lib/puppet/http/resolver/server_list.rb

@@ -0,0 +1,34 @@
+class Puppet::HTTP::Resolver::ServerList < Puppet::HTTP::Resolver
+  def initialize(client, server_list:, default_port:)
+    @client = client
+    @server_list = server_list
+    @default_port = default_port
+  end
+
+  def resolve(session, name, ssl_context: nil)
+    @server_list.each do |server|
+      host = server[0]
+      port = server[1] || @default_port
+      uri = URI("https://#{host}:#{port}/status/v1/simple/master")
+      if get_success?(uri, session, ssl_context: ssl_context)
+        return Puppet::HTTP::Service.create_service(@client, name, host, port)
+      end
+    end
+
+    raise Puppet::Error, _("Could not select a functional puppet master from server_list: '%{server_list}'") % { server_list: Puppet.settings.value(:server_list, Puppet[:environment].to_sym, true) }
+  end
+
+  def get_success?(uri, session, ssl_context: nil)
+    response = @client.get(uri, ssl_context: ssl_context)
+    return true if response.success?
+
+    Puppet.debug(_("Puppet server %{host}:%{port} is unavailable: %{code} %{reason}") %
+                 { host: host, port: port, code: response.code, reason: response.message })
+    return false
+  rescue => detail
+    session.add_exception(detail)
+    #TRANSLATORS 'server_list' is the name of a setting and should not be translated
+    Puppet.debug _("Unable to connect to server from server_list setting: %{detail}") % {detail: detail}
+    return false
+  end
+end

lib/puppet/http/resolver/settings.rb

@@ -1,5 +1,6 @@
 class Puppet::HTTP::Resolver::Settings < Puppet::HTTP::Resolver
-  def resolve(session, name, &block)
-    yield session.create_service(name)
+  def resolve(session, name, ssl_context: nil)
+    service = Puppet::HTTP::Service.create_service(@client, name)
+    check_connection?(session, service, ssl_context: ssl_context) ? service : nil
   end
 end

lib/puppet/http/resolver/srv.rb

@@ -1,15 +1,19 @@
 class Puppet::HTTP::Resolver::SRV < Puppet::HTTP::Resolver
-  def initialize(domain: srv_domain, dns: Resolv::DNS.new)
+  def initialize(client, domain:, dns: Resolv::DNS.new)
+    @client = client
     @srv_domain = domain
     @delegate = Puppet::Network::Resolver.new(dns)
   end
 
-  def resolve(session, name, &block)
+  def resolve(session, name, ssl_context: nil)
     # Here we pass our HTTP service name as the DNS SRV service name
     # This is fine for :ca, but note that :puppet and :file are handled
     # specially in `each_srv_record`.
     @delegate.each_srv_record(@srv_domain, name) do |server, port|
-      yield session.create_service(name, server, port)
+      service = Puppet::HTTP::Service.create_service(@client, name, server, port)
+      return service if check_connection?(session, service, ssl_context: ssl_context)
     end
+
+    return nil
   end
 end

lib/puppet/http/session.rb

@@ -3,6 +3,7 @@ def initialize(client, resolvers)
     @client = client
     @resolvers = resolvers
     @resolved_services = {}
+    @resolution_exceptions = []
   end
 
   def route_to(name, ssl_context: nil)
@@ -11,29 +12,23 @@ def route_to(name, ssl_context: nil)
     cached = @resolved_services[name]
     return cached if cached
 
-    errors = []
+    @resolution_exceptions = []
 
     @resolvers.each do |resolver|
       Puppet.debug("Resolving service '#{name}' using #{resolver.class}")
-      resolver.resolve(self, name) do |service|
-        begin
-          service.connect(ssl_context: ssl_context)
-          @resolved_services[name] = service
-          Puppet.debug("Resolved service '#{name}' to #{service.url}")
-          return service
-        rescue Puppet::HTTP::ConnectionError => e
-          errors << e
-          Puppet.debug("Connection to #{service.url} failed, trying next route: #{e.message}")
-        end
+      service = resolver.resolve(self, name, ssl_context: ssl_context)
+      if service
+        @resolved_services[name] = service
+        Puppet.debug("Resolved service '#{name}' to #{service.url}")
+        return service
       end
     end
 
-    errors.each { |e| Puppet.log_exception(e) }
-
+    @resolution_exceptions.each { |e| Puppet.log_exception(e) }
     raise Puppet::HTTP::RouteError, "No more routes to #{name}"
   end
 
-  def create_service(name, server = nil, port = nil)
-    Puppet::HTTP::Service.create_service(@client, name, server, port)
+  def add_exception(exception)
+    @resolution_exceptions << exception
   end
 end

spec/unit/http/resolver_spec.rb

@@ -9,22 +9,59 @@
   let(:uri) { URI.parse('https://www.example.com') }
 
   context 'when resolving using settings' do
-    let(:subject) { Puppet::HTTP::Resolver::Settings.new }
+    let(:subject) { Puppet::HTTP::Resolver::Settings.new(client) }
 
-    it 'yields a service based on the current ca_server and ca_port settings' do
+    it 'returns a service based on the current ca_server and ca_port settings' do
       Puppet[:ca_server] = 'ca.example.com'
       Puppet[:ca_port] = 8141
 
-      subject.resolve(session, :ca) do |service|
-        expect(service).to be_an_instance_of(Puppet::HTTP::Service::Ca)
-        expect(service.url.to_s).to eq("https://ca.example.com:8141/puppet-ca/v1")
-      end
+      service = subject.resolve(session, :ca)
+      expect(service).to be_an_instance_of(Puppet::HTTP::Service::Ca)
+      expect(service.url.to_s).to eq("https://ca.example.com:8141/puppet-ca/v1")
+    end
+  end
+
+  context 'when resolving using server_list' do
+    let(:server_list) { [["ca.example.com", "8141"], ["apple.example.com"]] }
+    let(:default_port) { '8142' }
+    let(:subject) { Puppet::HTTP::Resolver::ServerList.new(client, server_list: server_list, default_port: default_port) }
+
+    it 'returns a service based on the current server_list setting' do
+      stub_request(:get, "https://ca.example.com:8141/status/v1/simple/master").to_return(status: 200)
+
+      service = subject.resolve(session, :ca)
+      expect(service).to be_an_instance_of(Puppet::HTTP::Service::Ca)
+      expect(service.url.to_s).to eq("https://ca.example.com:8141/puppet-ca/v1")
+    end
+
+    it 'returns a service based on the current server_list setting if the server returns any success codes' do
+      stub_request(:get, "https://ca.example.com:8141/status/v1/simple/master").to_return(status: 202)
+
+      service = subject.resolve(session, :ca)
+      expect(service).to be_an_instance_of(Puppet::HTTP::Service::Ca)
+      expect(service.url.to_s).to eq("https://ca.example.com:8141/puppet-ca/v1")
+    end
+
+    it 'falls fails if no servers in server_list are accessible' do
+      stub_request(:get, "https://ca.example.com:8141/status/v1/simple/master").to_return(status: 503)
+      stub_request(:get, "https://apple.example.com:8142/status/v1/simple/master").to_return(status: 503)
+
+      expect { subject.resolve(session, :ca) }.to raise_error(Puppet::Error, /^Could not select a functional puppet master from server_list:/)
+    end
+
+    it 'cycles through server_list until a valid server is found' do
+      stub_request(:get, "https://ca.example.com:8141/status/v1/simple/master").to_return(status: 503)
+      stub_request(:get, "https://apple.example.com:8142/status/v1/simple/master").to_return(status: 200)
+
+      service = subject.resolve(session, :ca)
+      expect(service).to be_an_instance_of(Puppet::HTTP::Service::Ca)
+      expect(service.url.to_s).to eq("https://apple.example.com:8142/puppet-ca/v1")
     end
   end
 
   context 'when resolving using SRV' do
     let(:dns) { double('dns') }
-    let(:subject) { Puppet::HTTP::Resolver::SRV.new(domain: 'example.com', dns: dns) }
+    let(:subject) { Puppet::HTTP::Resolver::SRV.new(client, domain: 'example.com', dns: dns) }
 
     def stub_srv(host, port)
       srv = Resolv::DNS::Resource::IN::SRV.new(0, 0, port, host)
@@ -33,13 +70,12 @@ def stub_srv(host, port)
       allow(dns).to receive(:getresources).with("_x-puppet-ca._tcp.example.com", Resolv::DNS::Resource::IN::SRV).and_return([srv])
     end
 
-    it 'yields a service based on an SRV record' do
+    it 'returns a service based on an SRV record' do
       stub_srv('ca1.example.com', 8142)
 
-      subject.resolve(session, :ca) do |service|
-        expect(service).to be_an_instance_of(Puppet::HTTP::Service::Ca)
-        expect(service.url.to_s).to eq("https://ca1.example.com:8142/puppet-ca/v1")
-      end
+      service = subject.resolve(session, :ca)
+      expect(service).to be_an_instance_of(Puppet::HTTP::Service::Ca)
+      expect(service.url.to_s).to eq("https://ca1.example.com:8142/puppet-ca/v1")
     end
   end
 end

spec/unit/http/session_spec.rb

@@ -10,7 +10,7 @@
     double('good', url: uri, connect: nil)
   }
   let(:bad_service) {
-    service = double('good', url: uri)
+    service = double('bad', url: uri)
     allow(service).to receive(:connect).and_raise(Puppet::HTTP::ConnectionError, 'whoops')
     service
   }
@@ -23,9 +23,18 @@ def initialize(service)
       @count = 0
     end
 
-    def resolve(session, name, &block)
+    def resolve(session, name, ssl_context: nil)
       @count += 1
-      yield @service
+      return @service if check_connection?(session, @service, ssl_context: ssl_context)
+    end
+
+    def check_connection?(session, service, ssl_context: nil)
+      service.connect(ssl_context: ssl_context)
+      return true
+    rescue Puppet::HTTP::ConnectionError => e
+      session.add_exception(e)
+      Puppet.debug("Connection to #{service.url} failed, trying next route: #{e.message}")
+      return false
     end
   end