Skip to content

Bound the fqdn lookup in the hostname resolvers #177

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.

Sign up for GitHub

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

Open
silug wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Bound the fqdn lookup in the hostname resolvers #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 22 additions & 4 deletions lib/facter/resolvers/hostname.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ class Hostname < BaseResolver

init_resolver

# Upper bound, in seconds, for the fqdn lookup. A hung name service
# otherwise stalls fact collection until the agent's runtimeout fires
Comment thread
silug marked this conversation as resolved.
# (OpenVoxProject/openvox#485). Only applied on Rubies that honour it.
FQDN_LOOKUP_TIMEOUT = 10

class << self
private

Expand Down Expand Up @@ -48,14 +53,27 @@ def retrieve_from_fqdn(output)

def retrieve_with_addrinfo(host)
begin
name = Socket.getaddrinfo(host, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME)[0]
addr = Addrinfo.getaddrinfo(host, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME,
**addrinfo_options)[0]
rescue StandardError => e
@log.debug("Socket.getaddrinfo failed to retrieve fqdn for hostname #{host} with: #{e}")
@log.debug("Addrinfo.getaddrinfo failed to retrieve fqdn for hostname #{host} with: #{e}")
return
end
return if name.nil? || name.empty? || host == name[2] || name[2] == name[3]

name[2]
name = addr&.canonname
return if name.nil? || name.empty? || host == name || name == addr.ip_address

name
end

# Addrinfo.getaddrinfo only honours the timeout keyword from Ruby 4.0
# on. Older MRI accepts and ignores it; JRuby does not accept it.
def addrinfo_options
addrinfo_timeout_supported? ? { timeout: FQDN_LOOKUP_TIMEOUT } : {}
end

def addrinfo_timeout_supported?
RUBY_ENGINE == 'ruby' && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('4.0')
end

def exists_and_valid_fqdn?(fqdn, hostname)
Expand Down
30 changes: 27 additions & 3 deletions lib/facter/resolvers/linux/hostname.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ class Hostname < BaseResolver

init_resolver

# Upper bound, in seconds, for the fqdn lookup. A hung name service
# otherwise stalls fact collection until the agent's runtimeout fires
Comment thread
silug marked this conversation as resolved.
# (OpenVoxProject/openvox#485). Only applied on Rubies that honour it.
FQDN_LOOKUP_TIMEOUT = 10

class << self
private

Expand Down Expand Up @@ -65,16 +70,35 @@ def parse_fqdn(output)

def retrieve_fqdn_for_host(host)
begin
name = Socket.getaddrinfo(host, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME)[0]
addr = Addrinfo.getaddrinfo(host, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME,
**addrinfo_options)[0]
rescue StandardError => e
log.debug("Socket.getaddrinfo failed to retrieve fqdn for hostname #{host} with: #{e}")
log.debug("Addrinfo.getaddrinfo failed to retrieve fqdn for hostname #{host} with: #{e}")
# The name service is not answering. The FFI lookup below has no
# timeout and holds the GVL while it waits, so do not try it.
return if lookup_timed_out?(e)
end

return name[2] if !name.nil? && !name.empty? && host != name[2] && name[2] != name[3]
name = addr&.canonname
return name if exists_and_not_empty?(name) && host != name && name != addr.ip_address

retrieve_fqdn_for_host_with_ffi(host)
end

def lookup_timed_out?(error)
error.is_a?(Errno::ETIMEDOUT) || (defined?(IO::TimeoutError) && error.is_a?(IO::TimeoutError))
end

# Addrinfo.getaddrinfo only honours the timeout keyword from Ruby 4.0
# on. Older MRI accepts and ignores it; JRuby does not accept it.
def addrinfo_options
addrinfo_timeout_supported? ? { timeout: FQDN_LOOKUP_TIMEOUT } : {}
end

def addrinfo_timeout_supported?
RUBY_ENGINE == 'ruby' && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('4.0')
end

def retrieve_fqdn_for_host_with_ffi(host)
require_relative '../../../facter/util/resolvers/ffi/hostname'

Expand Down
61 changes: 58 additions & 3 deletions spec/facter/resolvers/hostname_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
let(:fqdn) { "#{hostname}.#{domain}" }

before do
allow(Socket).to receive(:getaddrinfo).and_return(domain)
allow(Addrinfo).to receive(:getaddrinfo).and_return([])
end

it 'detects hostname' do
Expand All @@ -57,6 +57,61 @@
end
end

context 'when hostname returns host and addrinfo returns the fqdn' do
let(:hostname) { 'foo' }
let(:domain) { 'bar' }
let(:host) { hostname }
let(:fqdn) { "#{hostname}.#{domain}" }

before do
allow(Addrinfo).to receive(:getaddrinfo)
.and_return([instance_double(Addrinfo, canonname: fqdn, ip_address: '10.0.0.1')])
end

it 'bounds the lookup with a timeout when Ruby honours it' do
stub_const('RUBY_ENGINE', 'ruby')
stub_const('RUBY_VERSION', '4.0.0')

hostname_resolver.resolve(:fqdn)

expect(Addrinfo).to have_received(:getaddrinfo)
.with(hostname, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME,
timeout: Facter::Resolvers::Hostname::FQDN_LOOKUP_TIMEOUT)
end

it 'omits the timeout when Ruby does not honour it' do
stub_const('RUBY_ENGINE', 'ruby')
stub_const('RUBY_VERSION', '3.4.0')

hostname_resolver.resolve(:fqdn)

expect(Addrinfo).to have_received(:getaddrinfo)
.with(hostname, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME)
end

it 'returns networking Domain' do
expect(hostname_resolver.resolve(:domain)).to eq(domain)
end

it 'returns fqdn' do
expect(hostname_resolver.resolve(:fqdn)).to eq(fqdn)
end
end

context 'when hostname returns host and addrinfo times out' do
let(:host) { 'foo' }

let(:timeout_error) { defined?(IO::TimeoutError) ? IO::TimeoutError : Errno::ETIMEDOUT }

before do
allow(Addrinfo).to receive(:getaddrinfo).and_raise(timeout_error, 'user specified timeout')
end

it 'detects domain from resolv.conf' do
expect(hostname_resolver.resolve(:domain)).to eq('baz')
end
end

context 'when hostname could not be retrieved' do
let(:host) { nil }

Expand All @@ -71,7 +126,7 @@

before do
allow(Facter::Util::FileHelper).to receive(:safe_read).with('/etc/resolv.conf').and_return('')
allow(Socket).to receive(:getaddrinfo).and_return(domain)
allow(Addrinfo).to receive(:getaddrinfo).and_return([])
end

it 'detects that domain is nil' do
Expand All @@ -83,7 +138,7 @@
let(:host) { 'foo' }

before do
allow(Socket).to receive(:getaddrinfo).and_raise('socket exception')
allow(Addrinfo).to receive(:getaddrinfo).and_raise('socket exception')
end

it 'detects domain from resolv.conf' do
Expand Down
52 changes: 46 additions & 6 deletions spec/facter/resolvers/linux/hostname_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,50 @@

context 'when it returns only the hostname and ruby addrinfo works' do
let(:host) { hostname }
let(:addr_info) { [['', '', "#{hostname}.#{domain}", '']] }
let(:addr_info) { [instance_double(Addrinfo, canonname: "#{hostname}.#{domain}", ip_address: '10.0.0.1')] }

before do
allow(Socket).to receive(:getaddrinfo).and_return(addr_info)
allow(Addrinfo).to receive(:getaddrinfo).and_return(addr_info)
end

it_behaves_like 'detects values'
end

context 'when it returns only the hostname and ruby addrinfo times out' do
let(:host) { hostname }
let(:domain) { 'baz' }
let(:timeout_error) { defined?(IO::TimeoutError) ? IO::TimeoutError : Errno::ETIMEDOUT }

before do
allow(Addrinfo).to receive(:getaddrinfo).and_raise(timeout_error, 'user specified timeout')
allow(Facter::Util::Resolvers::Ffi::Hostname).to receive(:getffiaddrinfo)
end

it 'bounds the lookup with a timeout when Ruby honours it' do
stub_const('RUBY_ENGINE', 'ruby')
stub_const('RUBY_VERSION', '4.0.0')

hostname_resolver.resolve(:fqdn)

expect(Addrinfo).to have_received(:getaddrinfo)
.with(hostname, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME,
timeout: Facter::Resolvers::Linux::Hostname::FQDN_LOOKUP_TIMEOUT)
end

it 'omits the timeout when Ruby does not honour it' do
stub_const('RUBY_ENGINE', 'ruby')
stub_const('RUBY_VERSION', '3.4.0')

hostname_resolver.resolve(:fqdn)

expect(Addrinfo).to have_received(:getaddrinfo)
.with(hostname, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME)
end

it 'does not fall back to the FFI lookup' do
hostname_resolver.resolve(:fqdn)

expect(Facter::Util::Resolvers::Ffi::Hostname).not_to have_received(:getffiaddrinfo)
end

it_behaves_like 'detects values'
Expand All @@ -56,7 +96,7 @@
let(:output) { fqdn }

before do
allow(Socket).to receive(:getaddrinfo).and_return([])
allow(Addrinfo).to receive(:getaddrinfo).and_return([])
allow(Facter::Util::Resolvers::Ffi::Hostname).to receive(:getffiaddrinfo).and_return(output)
end

Expand Down Expand Up @@ -153,11 +193,11 @@
end

context 'when it returns only the hostname and ruby addrinfo works' do
let(:addr_info) { [['', '', "#{hostname}.#{domain}", '']] }
let(:addr_info) { [instance_double(Addrinfo, canonname: "#{hostname}.#{domain}", ip_address: '10.0.0.1')] }
let(:ffi_host) { hostname }

before do
allow(Socket).to receive(:getaddrinfo).and_return(addr_info)
allow(Addrinfo).to receive(:getaddrinfo).and_return(addr_info)
end

it_behaves_like 'detects values'
Expand All @@ -168,7 +208,7 @@
let(:output) { fqdn }

before do
allow(Socket).to receive(:getaddrinfo).and_return([])
allow(Addrinfo).to receive(:getaddrinfo).and_return([])
allow(Facter::Util::Resolvers::Ffi::Hostname).to receive(:getffiaddrinfo).and_return(output)
end

Expand Down