Skip to content

Commit

Permalink
Support ports as integers (#315)
Browse files Browse the repository at this point in the history 
Without this patch applied, compilation fails if integer data for ports
(proxy_port, pup_port etc) is passed in.

This is a problem, because it requires us to quote ports in Hiera, and
is a surprising and unexpected behaviour.

After this patch is applied, it is possible to specify these ports
either as a string or an integer.

Tests are added for each port showing that integer or string inputs both
lead to the same configuration in the catalog.
  • Loading branch information
@alex-harvey-z3q @truthbk
alex-harvey-z3q authored and truthbk committed Mar 22, 2017
1 parent b645dad commit 2b3248a
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 43 deletions.
22 changes: 16 additions & 6 deletions manifests/init.pp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,16 @@
$apm_env = '',
) inherits datadog_agent::params {

# Allow ports to be passed as integers or strings.
# lint:ignore:only_variable_string
$_statsd_forward_port = "${statsd_forward_port}"
$_proxy_port = "${proxy_port}"
$_graphite_listen_port = "${graphite_listen_port}"
$_listen_port = "${listen_port}"
$_pup_port = "${pup_port}"
$_syslog_port = "${syslog_port}"
# lint:endignore

validate_string($dd_url)
validate_string($host)
validate_string($api_key)
Expand All @@ -263,12 +273,12 @@
validate_string($log_level)
validate_integer($dogstatsd_port)
validate_string($statsd_histogram_percentiles)
validate_string($statsd_forward_port)
validate_re($_statsd_forward_port, '^\d*$')
validate_string($proxy_host)
validate_string($proxy_port)
validate_re($_proxy_port, '^\d*$')
validate_string($proxy_user)
validate_string($proxy_password)
validate_string($graphite_listen_port)
validate_re($_graphite_listen_port, '^\d*$')
validate_string($extra_template)
validate_string($ganglia_host)
validate_integer($ganglia_port)
Expand All @@ -278,11 +288,11 @@
validate_bool($collect_ec2_tags)
validate_bool($collect_instance_metadata)
validate_string($recent_point_threshold)
validate_string($listen_port)
validate_re($_listen_port, '^\d*$')
validate_string($additional_checksd)
validate_string($bind_host)
validate_bool($use_pup)
validate_string($pup_port)
validate_re($_pup_port, '^\d*$')
validate_string($pup_interface)
validate_string($pup_url)
validate_bool($use_dogstatsd)
Expand All @@ -297,7 +307,7 @@
validate_string($dogstatsd_log_file)
validate_string($pup_log_file)
validate_string($syslog_host)
validate_string($syslog_port)
validate_re($_syslog_port, '^\d*$')
validate_string($service_discovery_backend)
validate_string($sd_config_backend)
validate_string($sd_backend_host)
Expand Down
79 changes: 54 additions & 25 deletions spec/classes/datadog_agent_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,21 +218,24 @@
'content' => /^dd_url: https:\/\/notaurl.datadoghq.com\n/,
)}
end

context 'with a custom proxy_host' do
let(:params) {{:proxy_host => 'localhost'}}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^proxy_host: localhost\n/,
)}
end

context 'with a custom proxy_port' do
let(:params) {{:proxy_port => '1234'}}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^proxy_port: 1234\n/,
)}
end

context 'with a custom proxy_port, specified as an integer' do
let(:params) {{:proxy_port => 1234}}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^proxy_port: 1234\n/,
)}
end
context 'with a custom proxy_user' do
let(:params) {{:proxy_user => 'notauser'}}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
Expand All @@ -245,7 +248,6 @@
'content' => /^api_key: notakey\n/,
)}
end

context 'with a custom hostname' do
let(:params) {{:host => 'notahost'}}

Expand All @@ -259,7 +261,7 @@
'content' => /^non_local_traffic: true\n/,
)}
end
#Should expand testing to cover changes to the case upcase
#Should expand testing to cover changes to the case upcase
context 'with log level set to critical' do
let(:params) {{:log_level => 'critical'}}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
Expand All @@ -279,56 +281,67 @@
)}
end
context 'with skip_ssl_validation set to true' do
let(:params) {{:skip_ssl_validation => true }}
let(:params) {{:skip_ssl_validation => true }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^skip_ssl_validation: true\n/,
)}
end
context 'with collect_ec2_tags set to yes' do
let(:params) {{:collect_ec2_tags => true }}
let(:params) {{:collect_ec2_tags => true }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^collect_ec2_tags: true\n/,
)}
end
context 'with collect_instance_metadata set to no' do
let(:params) {{:collect_instance_metadata => false }}
let(:params) {{:collect_instance_metadata => false }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^collect_instance_metadata: false\n/,
)}
end
context 'with recent_point_threshold set to 60' do
let(:params) {{:recent_point_threshold => '60' }}
let(:params) {{:recent_point_threshold => '60' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^recent_point_threshold: 60\n/,
)}
end
context 'with a custom port set to 17125' do
let(:params) {{:listen_port => '17125' }}
let(:params) {{:listen_port => '17125' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^listen_port: 17125\n/,
)}
end
context 'litstening for graphite data on port 17124' do
let(:params) {{:graphite_listen_port => '17124' }}
context 'with a custom port set to 17125, specified as an integer' do
let(:params) {{:listen_port => 17125 }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^listen_port: 17125\n/,
)}
end
context 'listening for graphite data on port 17124' do
let(:params) {{:graphite_listen_port => '17124' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^graphite_listen_port: 17124\n/,
)}
end
context 'listening for graphite data on port 17124, port specified as an integer' do
let(:params) {{:graphite_listen_port => 17124 }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^graphite_listen_port: 17124\n/,
)}
end
context 'with configuration for a custom checks.d' do
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^additional_checksd: \/etc\/dd-agent\/checks_custom.d\n/,
)}
end
context 'with configuration for a custom checks.d' do
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^additional_checksd: \/etc\/dd-agent\/checks_custom.d\n/,
)}
end

context 'with configuration for a custom checks.d' do
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^additional_checksd: \/etc\/dd-agent\/checks_custom.d\n/,
)}
Expand Down Expand Up @@ -357,28 +370,32 @@
'content' => /^pup_port: 17126\n/,
)}
end
context 'with a custom pup_port, specified as an integer' do
let(:params) {{:pup_port => 17126 }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^pup_port: 17126\n/,
)}
end
context 'with a custom pup_interface' do
let(:params) {{:pup_interface => 'notalocalhost' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^pup_interface: notalocalhost\n/,
)}
end

context 'with a custom pup_url' do
let(:params) {{:pup_url => 'http://localhost:17126' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^pup_url: http:\/\/localhost:17126\n/,
)}
end

context 'with use_dogstatsd set to no' do
let(:params) {{:use_dogstatsd => false}}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^use_dogstatsd: no\n/,
)}
end
context 'with dogstatsd_port set to 8126' do
let(:params) {{:dogstatsd_port => 8126}}
context 'with dogstatsd_port set to 8126 - must be specified as an integer!' do
let(:params) {{:dogstatsd_port => 8126 }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^dogstatsd_port: 8126\n/,
)}
Expand Down Expand Up @@ -414,13 +431,13 @@
)}
end
context 'with statsd_forward_port set to 8126' do
let(:params) {{:statsd_forward_port => '8126' }}
let(:params) {{:statsd_forward_port => '8126' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^statsd_forward_port: 8126\n/,
)}
end
context 'with statsd_forward_port set to 8126' do
let(:params) {{:statsd_forward_port => '8126' }}
context 'with statsd_forward_port set to 8126, specified as an integer' do
let(:params) {{:statsd_forward_port => 8126 }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^statsd_forward_port: 8126\n/,
)}
Expand All @@ -438,14 +455,20 @@
)}
end
context 'with ganglia_host set to localhost and ganglia_port set to 12345' do
let(:params) {{:ganglia_host => 'testhost', :ganglia_port => '12345' }}
let(:params) {{:ganglia_host => 'testhost', :ganglia_port => '12345' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^ganglia_port: 12345\n/,
)}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^ganglia_host: testhost\n/,
)}
end
context 'with ganglia_host set to localhost and ganglia_port set to 12345, port specified as an integer' do
let(:params) {{:ganglia_host => 'testhost', :ganglia_port => 12345 }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^ganglia_port: 12345\n/,
)}
end
context 'with dogstreams set to /path/to/log1:/path/to/parser' do
let(:params) {{:dogstreams => ['/path/to/log1:/path/to/parser'] }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
Expand Down Expand Up @@ -501,7 +524,13 @@
)}
end
context 'with syslog port set to 8080' do
let(:params) {{:syslog_port => '8080' }}
let(:params) {{:syslog_port => '8080' }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^syslog_port: 8080\n/,
)}
end
context 'with syslog port set to 8080, specified as an integer' do
let(:params) {{:syslog_port => 8080 }}
it { should contain_file('/etc/dd-agent/datadog.conf').with(
'content' => /^syslog_port: 8080\n/,
)}
Expand Down
24 changes: 12 additions & 12 deletions templates/datadog.conf.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ dd_url: <%= @dd_url %>
<% else -%>
proxy_host: <%= @proxy_host %>
<% end -%>
<% if @proxy_port.empty? -%>
<% if @_proxy_port.empty? -%>
# proxy_port:
<% else -%>
proxy_port: <%= @proxy_port %>
proxy_port: <%= @_proxy_port %>
<% end -%>
<% if @proxy_user.empty? -%>
# proxy_user:
Expand Down Expand Up @@ -85,17 +85,17 @@ recent_point_threshold: <%= @recent_point_threshold %>
<% end -%>

# Change port the Agent is listening to
<% if @listen_port.empty? -%>
<% if @_listen_port.empty? -%>
# listen_port: 17123
<% else -%>
listen_port: <%= @listen_port %>
listen_port: <%= @_listen_port %>
<% end -%>

# Start a graphite listener on this port
<% if @graphite_listen_port.empty? -%>
<% if @_graphite_listen_port.empty? -%>
# graphite_listen_port: 17124
<% else -%>
graphite_listen_port: <%= @graphite_listen_port %>
graphite_listen_port: <%= @_graphite_listen_port %>
<% end -%>

# Additional directory to look for Datadog checks
Expand Down Expand Up @@ -134,10 +134,10 @@ bind_host: <%= @bind_host %>

use_pup: <%= @use_pup ? "yes" : "no" %>
<% if @pup_port.empty? -%>
<% if @_pup_port.empty? -%>
# pup_port: 17125
<% else -%>
pup_port: <%= @pup_port %>
pup_port: <%= @_pup_port %>
<% end -%>
<% if @pup_interface.empty? -%>
Expand Down Expand Up @@ -200,10 +200,10 @@ histogram_percentiles: <%= @statsd_histogram_percentiles %>
<% else -%>
statsd_forward_host: <%= @statsd_forward_host %>
<% end -%>
<% if @statsd_forward_port.empty? -%>
<% if @_statsd_forward_port.empty? -%>
# statsd_forward_port: 8125
<% else -%>
statsd_forward_port: <%= @statsd_forward_port %>
statsd_forward_port: <%= @_statsd_forward_port %>
<% end -%>

# ========================================================================== #
Expand Down Expand Up @@ -340,10 +340,10 @@ log_to_syslog: <%= @log_to_syslog ? "yes" : "no" %>
syslog_host: <%= @syslog_host %>
<% end -%>
<% if @syslog_port.empty? -%>
<% if @_syslog_port.empty? -%>
# syslog_port:
<% else -%>
syslog_port: <%= @syslog_port %>
syslog_port: <%= @_syslog_port %>
<% end -%>


Expand Down

0 comments on commit 2b3248a

Please sign in to comment.