Skip to content

(PUP-10039) Add ServerList resolver #7856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/puppet/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/http/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 14 additions & 1 deletion lib/puppet/http/resolver.rb
Original file line number Diff line number Diff line change
@@ -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
34 changes: 34 additions & 0 deletions lib/puppet/http/resolver/server_list.rb
Original file line number Diff line number Diff line change
@@ -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
5 changes: 3 additions & 2 deletions lib/puppet/http/resolver/settings.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 7 additions & 3 deletions lib/puppet/http/resolver/srv.rb
Original file line number Diff line number Diff line change
@@ -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
25 changes: 10 additions & 15 deletions lib/puppet/http/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ def initialize(client, resolvers)
@client = client
@resolvers = resolvers
@resolved_services = {}
@resolution_exceptions = []
end

def route_to(name, ssl_context: nil)
Expand All @@ -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}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mucked with the code to play with this a bit more:

I added the ServerList resolver to the CA service, and I removed the raise in the ServerList resolver so that we would fall back to the settings resolver. I also ensured both the SRV and ServerList resolvers return nil if we don't find anything that connects successfully (otherwise we get weird errors when it tries to resolve service.url on line 20, since service is being set to either the array of servers in server_list or an empty array for srv).

be ./bin/puppet agent -t --server_list apple.example.com,google.example.com --use_srv_records --debug
Debug: Resolving service 'ca' using Puppet::HTTP::Resolver::ServerList
Debug: Creating new connection for https://apple.example.com:8140
Debug: Starting connection for https://apple.example.com:8140
Debug: Unable to connect to server from server_list setting: Failed to connect to https://apple.example.com:8140/status/v1/simple/master: Failed to open TCP connection to apple.example.com:8140 (getaddrinfo: nodename nor servname provided, or not known)
Debug: Creating new connection for https://google.example.com:8140
Debug: Starting connection for https://google.example.com:8140
Debug: Unable to connect to server from server_list setting: Failed to connect to https://google.example.com:8140/status/v1/simple/master: Failed to open TCP connection to google.example.com:8140 (getaddrinfo: nodename nor servname provided, or not known)
Debug: Resolving service 'ca' using Puppet::HTTP::Resolver::SRV
Debug: Searching for SRV records for domain: Home
Debug: Found 0 SRV records for: _x-puppet-ca._tcp.Home
Debug: Searching for SRV records for domain: Home
Debug: Found 0 SRV records for: _x-puppet._tcp.Home
Debug: Resolving service 'ca' using Puppet::HTTP::Resolver::Settings
Debug: Creating new connection for https://puppet:8140
Debug: Starting connection for https://puppet:8140
Debug: Connection to https://puppet:8140/puppet-ca/v1 failed, trying next route: Failed to connect to https://puppet:8140/puppet-ca/v1: execution expired
Error: Failed to connect to https://apple.example.com:8140/status/v1/simple/master: Failed to open TCP connection to apple.example.com:8140 (getaddrinfo: nodename nor servname provided, or not known)
Wrapped exception:
Failed to open TCP connection to apple.example.com:8140 (getaddrinfo: nodename nor servname provided, or not known)
Error: Failed to connect to https://google.example.com:8140/status/v1/simple/master: Failed to open TCP connection to google.example.com:8140 (getaddrinfo: nodename nor servname provided, or not known)
Wrapped exception:
Failed to open TCP connection to google.example.com:8140 (getaddrinfo: nodename nor servname provided, or not known)
Error: Failed to connect to https://puppet:8140/puppet-ca/v1: execution expired
Wrapped exception:
execution expired
Error: No more routes to ca
Error: Could not run: No more routes to ca

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
60 changes: 48 additions & 12 deletions spec/unit/http/resolver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
15 changes: 12 additions & 3 deletions spec/unit/http/session_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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

Expand Down