-
Notifications
You must be signed in to change notification settings - Fork 262
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
add no_proxy option #549
add no_proxy option #549
Conversation
b744e8e
to
66382dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @stonith for the PR! I've left a few comments, could you have a look?
attributes/default.rb
Outdated
@@ -296,6 +296,7 @@ | |||
default['datadog']['web_proxy']['user'] = nil | |||
default['datadog']['web_proxy']['password'] = nil | |||
default['datadog']['web_proxy']['skip_ssl_validation'] = nil # accepted values 'yes' or 'no' | |||
default['datadog']['web_proxy']['no_proxy'] = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a small comment here to mention it's used for Agent v6+ only?
@@ -37,6 +37,7 @@ if node['datadog']['web_proxy']['host'] | |||
user = node['datadog']['web_proxy']['user'] | |||
password = node['datadog']['web_proxy']['password'] | |||
scheme = "" | |||
no_proxy = node['datadog']['web_proxy']['no_proxy'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you initialize no_proxy
to nil outside of this if
block
templates/default/datadog.yaml.erb
Outdated
@@ -136,6 +137,10 @@ if !http_proxy.nil? | |||
} | |||
end | |||
|
|||
if !no_proxy.nil? | |||
agent_config['proxy']['no_proxy'] = no_proxy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agent_config['proxy']
should be initialized to a hash here, but the logic is a bit brittle. Could you either check that the key is indeed present in agent_config
or move this block inside the !http_proxy.nil?
conditional above?
- adds comment to the attributes to identify it as v6+ - ensures no_proxy variable has as default value - ensures the proxy key exists in the config:
@olivielpeau comments addressed. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
TL:DR Adds no_proxy option
The behaviour in agent v6 seems to behave differently than v5 when a web proxy is configured. With v5, the agent would connect directly to 127.0.0.1 whereas with v6, the agent will try to connect to 127.0.0.1 via the proxy. In v6, there is a no_proxy option which is not supported the chef datadog cookbook.