Skip to content
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

fix: ensure uds always takes precedence over http if both configurations are defined #4024

Merged
merged 7 commits into from
Nov 1, 2024
+55 −66
Merged

fix: ensure uds always takes precedence over http if both configurations are defined #4024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fb6183a
use uds
mabdinur Oct 24, 2024
8a873af
update tests to match this new world order
mabdinur Oct 24, 2024
a1460a9
fix test
mabdinur Oct 24, 2024
f1a79bc
add back mixed_http_and_uds
mabdinur Oct 25, 2024
0b83cab
fix unix path regex
mabdinur Oct 25, 2024
dfc600f
update method
mabdinur Oct 28, 2024
ebaa0bf
lint
mabdinur Nov 1, 2024
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
51 changes: 26 additions & 25 deletions lib/datadog/core/configuration/agent_settings_resolver.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,32 @@ def uds_fallback
end

def should_use_uds?
can_use_uds? && !mixed_http_and_uds?
# When we have mixed settings for http/https and uds, we print a warning
# and use the uds settings.
mixed_http_and_uds
can_use_uds?
end

def mixed_http_and_uds
return @mixed_http_and_uds if defined?(@mixed_http_and_uds)

@mixed_http_and_uds = (configured_hostname || configured_port) && can_use_uds?
if @mixed_http_and_uds
warn_if_configuration_mismatch(
[
DetectedConfiguration.new(
friendly_name: 'configuration for unix domain socket',
value: parsed_url.to_s,
),
DetectedConfiguration.new(
friendly_name: 'configuration of hostname/port for http/https use',
value: "hostname: '#{hostname}', port: '#{port}'",
),
]
)
end

@mixed_http_and_uds
end

def can_use_uds?
Expand Down Expand Up @@ -307,30 +332,6 @@ def unix_scheme?(uri)
uri.scheme == 'unix'
end

# When we have mixed settings for http/https and uds, we print a warning and ignore the uds settings
def mixed_http_and_uds?
return @mixed_http_and_uds if defined?(@mixed_http_and_uds)

@mixed_http_and_uds = (configured_hostname || configured_port) && can_use_uds?

if @mixed_http_and_uds
warn_if_configuration_mismatch(
[
DetectedConfiguration.new(
friendly_name: 'configuration of hostname/port for http/https use',
value: "hostname: '#{hostname}', port: '#{port}'",
),
DetectedConfiguration.new(
friendly_name: 'configuration for unix domain socket',
value: parsed_url.to_s,
),
]
)
end

@mixed_http_and_uds
end

# Represents a given configuration value and where we got it from
class DetectedConfiguration
attr_reader :friendly_name, :value
Expand Down
70 changes: 29 additions & 41 deletions spec/datadog/core/configuration/agent_settings_resolver_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,32 +170,29 @@
context 'when there is a hostname specified along with uds configuration' do
let(:with_agent_host) { 'custom-hostname' }

it 'prioritizes the http configuration' do
expect(resolver).to have_attributes(hostname: 'custom-hostname', adapter: :net_http)
it 'prioritizes the uds configuration' do
expect(resolver).to have_attributes(adapter: :unix)
end

it 'logs a warning including the uds path' do
it 'logs a warning including the mismatching hostname' do
expect(logger).to receive(:warn)
.with(%r{Configuration mismatch.*configuration for unix domain socket \("unix:.*/some/path"\)})
.with(/Configuration mismatch:.*hostname: 'custom-hostname'.*/)

resolver
end

it 'does not include a uds_path in the configuration' do
expect(resolver).to have_attributes(uds_path: nil)
it 'includes a uds_path in the configuration' do
expect(resolver).to have_attributes(uds_path: '/some/path')
end

context 'when there is no port specified' do
it 'prioritizes the http configuration and uses the default port' do
expect(resolver).to have_attributes(port: 8126, hostname: 'custom-hostname', adapter: :net_http)
it 'prioritizes the uds configuration and ignores the default port' do
expect(resolver).to have_attributes(adapter: :unix)
end

it 'logs a warning including the hostname and default port' do
it 'logs a warning including the uds path' do
expect(logger).to receive(:warn)
.with(/
Configuration\ mismatch:\ values\ differ\ between\ configuration.*
Using\ "hostname:\ 'custom-hostname',\ port:\ '8126'".*
/x)
.with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) # rubocop:disable Style/edundantRegexpCharacterClass

resolver
end
Expand All @@ -204,51 +201,45 @@
context 'when there is a port specified' do
let(:with_agent_port) { 1234 }

it 'prioritizes the http configuration and uses the specified port' do
expect(resolver).to have_attributes(port: 1234, hostname: 'custom-hostname', adapter: :net_http)
it 'prioritizes the uds path' do
expect(resolver).to have_attributes(adapter: :unix)
end

it 'logs a warning including the hostname and port' do
it 'logs a warning including the uds configuration' do
expect(logger).to receive(:warn)
.with(/
Configuration\ mismatch:\ values\ differ\ between\ configuration.*
Using\ "hostname:\ 'custom-hostname',\ port:\ '1234'".*
/x)
.with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) # rubocop:disable Style/edundantRegexpCharacterClass

resolver
end
end
end

context 'when there is a port specified along with uds configuration' do
context 'when there is a port specified along with a uds configuration' do
let(:with_agent_port) { 5678 }

it 'prioritizes the http configuration' do
expect(resolver).to have_attributes(port: 5678, adapter: :net_http)
it 'prioritizes the uds configuration' do
expect(resolver).to have_attributes(port: 5678, adapter: :unix)
end

it 'logs a warning including the uds path' do
it 'logs a warning including the mismatching port' do
expect(logger).to receive(:warn)
.with(%r{Configuration mismatch.*configuration for unix domain socket \("unix:.*/some/path"\)})
.with(/Configuration mismatch:.*port: '5678'.*/)

resolver
end

it 'does not include a uds_path in the configuration' do
expect(resolver).to have_attributes(uds_path: nil)
it 'includes the uds path in the configuration' do
expect(resolver).to have_attributes(uds_path: '/some/path')
end

context 'when there is no hostname specified' do
it 'prioritizes the http configuration and uses the default hostname' do
expect(resolver).to have_attributes(port: 5678, hostname: '127.0.0.1', adapter: :net_http)
it 'prioritizes the uds configuration' do
expect(resolver).to have_attributes(port: 5678, hostname: nil, adapter: :unix)
end

it 'logs a warning including the default hostname and port' do
it 'logs a warning including the uds configuration' do
expect(logger).to receive(:warn)
.with(/
Configuration\ mismatch:\ values\ differ\ between\ configuration.*
Using\ "hostname:\ '127.0.0.1',\ port:\ '5678'".*
/x)
.with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) # rubocop:disable Style/edundantRegexpCharacterClass

resolver
end
Expand All @@ -257,16 +248,13 @@
context 'when there is a hostname specified' do
let(:with_agent_host) { 'custom-hostname' }

it 'prioritizes the http configuration and uses the specified hostname' do
expect(resolver).to have_attributes(port: 5678, hostname: 'custom-hostname', adapter: :net_http)
it 'prioritizes the uds configuration' do
expect(resolver).to have_attributes(adapter: :unix)
end

it 'logs a warning including the hostname and port' do
it 'logs a warning including the uds configuration' do
expect(logger).to receive(:warn)
.with(/
Configuration\ mismatch:\ values\ differ\ between\ configuration.*
Using\ "hostname:\ 'custom-hostname',\ port:\ '5678'".*
/x)
.with(%r{Configuration mismatch.*Using "unix:[\/]{1,3}some/path"}) # rubocop:disable Style/edundantRegexpCharacterClass

resolver
end
Expand Down
Loading