Skip to content

Commit 0c30f79

Browse files
alex-harvey-z3qtruthbk
alex-harvey-z3q
authored and
truthbk
committed
Support ports as integers (DataDog#315)
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.
1 parent 9db0d30 commit 0c30f79

3 files changed

Lines changed: 82 additions & 43 deletions

File tree

‎manifests/init.pp‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,16 @@
247247
$apm_env = '',
248248
) inherits datadog_agent::params {
249249

250+
# Allow ports to be passed as integers or strings.
251+
# lint:ignore:only_variable_string
252+
$_statsd_forward_port = "${statsd_forward_port}"
253+
$_proxy_port = "${proxy_port}"
254+
$_graphite_listen_port = "${graphite_listen_port}"
255+
$_listen_port = "${listen_port}"
256+
$_pup_port = "${pup_port}"
257+
$_syslog_port = "${syslog_port}"
258+
# lint:endignore
259+
250260
validate_string($dd_url)
251261
validate_string($host)
252262
validate_string($api_key)
@@ -263,12 +273,12 @@
263273
validate_string($log_level)
264274
validate_integer($dogstatsd_port)
265275
validate_string($statsd_histogram_percentiles)
266-
validate_string($statsd_forward_port)
276+
validate_re($_statsd_forward_port, '^\d*$')
267277
validate_string($proxy_host)
268-
validate_string($proxy_port)
278+
validate_re($_proxy_port, '^\d*$')
269279
validate_string($proxy_user)
270280
validate_string($proxy_password)
271-
validate_string($graphite_listen_port)
281+
validate_re($_graphite_listen_port, '^\d*$')
272282
validate_string($extra_template)
273283
validate_string($ganglia_host)
274284
validate_integer($ganglia_port)
@@ -278,11 +288,11 @@
278288
validate_bool($collect_ec2_tags)
279289
validate_bool($collect_instance_metadata)
280290
validate_string($recent_point_threshold)
281-
validate_string($listen_port)
291+
validate_re($_listen_port, '^\d*$')
282292
validate_string($additional_checksd)
283293
validate_string($bind_host)
284294
validate_bool($use_pup)
285-
validate_string($pup_port)
295+
validate_re($_pup_port, '^\d*$')
286296
validate_string($pup_interface)
287297
validate_string($pup_url)
288298
validate_bool($use_dogstatsd)
@@ -297,7 +307,7 @@
297307
validate_string($dogstatsd_log_file)
298308
validate_string($pup_log_file)
299309
validate_string($syslog_host)
300-
validate_string($syslog_port)
310+
validate_re($_syslog_port, '^\d*$')
301311
validate_string($service_discovery_backend)
302312
validate_string($sd_config_backend)
303313
validate_string($sd_backend_host)

‎spec/classes/datadog_agent_spec.rb‎

Lines changed: 54 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -218,21 +218,24 @@
218218
'content' => /^dd_url: https:\/\/notaurl.datadoghq.com\n/,
219219
)}
220220
end
221-
222221
context 'with a custom proxy_host' do
223222
let(:params) {{:proxy_host => 'localhost'}}
224223
it { should contain_file('/etc/dd-agent/datadog.conf').with(
225224
'content' => /^proxy_host: localhost\n/,
226225
)}
227226
end
228-
229227
context 'with a custom proxy_port' do
230228
let(:params) {{:proxy_port => '1234'}}
231229
it { should contain_file('/etc/dd-agent/datadog.conf').with(
232230
'content' => /^proxy_port: 1234\n/,
233231
)}
234232
end
235-
233+
context 'with a custom proxy_port, specified as an integer' do
234+
let(:params) {{:proxy_port => 1234}}
235+
it { should contain_file('/etc/dd-agent/datadog.conf').with(
236+
'content' => /^proxy_port: 1234\n/,
237+
)}
238+
end
236239
context 'with a custom proxy_user' do
237240
let(:params) {{:proxy_user => 'notauser'}}
238241
it { should contain_file('/etc/dd-agent/datadog.conf').with(
@@ -245,7 +248,6 @@
245248
'content' => /^api_key: notakey\n/,
246249
)}
247250
end
248-
249251
context 'with a custom hostname' do
250252
let(:params) {{:host => 'notahost'}}
251253

@@ -259,7 +261,7 @@
259261
'content' => /^non_local_traffic: true\n/,
260262
)}
261263
end
262-
#Should expand testing to cover changes to the case upcase
264+
#Should expand testing to cover changes to the case upcase
263265
context 'with log level set to critical' do
264266
let(:params) {{:log_level => 'critical'}}
265267
it { should contain_file('/etc/dd-agent/datadog.conf').with(
@@ -279,56 +281,67 @@
279281
)}
280282
end
281283
context 'with skip_ssl_validation set to true' do
282-
let(:params) {{:skip_ssl_validation => true }}
284+
let(:params) {{:skip_ssl_validation => true }}
283285
it { should contain_file('/etc/dd-agent/datadog.conf').with(
284286
'content' => /^skip_ssl_validation: true\n/,
285287
)}
286288
end
287289
context 'with collect_ec2_tags set to yes' do
288-
let(:params) {{:collect_ec2_tags => true }}
290+
let(:params) {{:collect_ec2_tags => true }}
289291
it { should contain_file('/etc/dd-agent/datadog.conf').with(
290292
'content' => /^collect_ec2_tags: true\n/,
291293
)}
292294
end
293295
context 'with collect_instance_metadata set to no' do
294-
let(:params) {{:collect_instance_metadata => false }}
296+
let(:params) {{:collect_instance_metadata => false }}
295297
it { should contain_file('/etc/dd-agent/datadog.conf').with(
296298
'content' => /^collect_instance_metadata: false\n/,
297299
)}
298300
end
299301
context 'with recent_point_threshold set to 60' do
300-
let(:params) {{:recent_point_threshold => '60' }}
302+
let(:params) {{:recent_point_threshold => '60' }}
301303
it { should contain_file('/etc/dd-agent/datadog.conf').with(
302304
'content' => /^recent_point_threshold: 60\n/,
303305
)}
304306
end
305307
context 'with a custom port set to 17125' do
306-
let(:params) {{:listen_port => '17125' }}
308+
let(:params) {{:listen_port => '17125' }}
307309
it { should contain_file('/etc/dd-agent/datadog.conf').with(
308310
'content' => /^listen_port: 17125\n/,
309311
)}
310312
end
311-
context 'litstening for graphite data on port 17124' do
312-
let(:params) {{:graphite_listen_port => '17124' }}
313+
context 'with a custom port set to 17125, specified as an integer' do
314+
let(:params) {{:listen_port => 17125 }}
315+
it { should contain_file('/etc/dd-agent/datadog.conf').with(
316+
'content' => /^listen_port: 17125\n/,
317+
)}
318+
end
319+
context 'listening for graphite data on port 17124' do
320+
let(:params) {{:graphite_listen_port => '17124' }}
321+
it { should contain_file('/etc/dd-agent/datadog.conf').with(
322+
'content' => /^graphite_listen_port: 17124\n/,
323+
)}
324+
end
325+
context 'listening for graphite data on port 17124, port specified as an integer' do
326+
let(:params) {{:graphite_listen_port => 17124 }}
313327
it { should contain_file('/etc/dd-agent/datadog.conf').with(
314328
'content' => /^graphite_listen_port: 17124\n/,
315329
)}
316330
end
317331
context 'with configuration for a custom checks.d' do
318-
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
332+
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
319333
it { should contain_file('/etc/dd-agent/datadog.conf').with(
320334
'content' => /^additional_checksd: \/etc\/dd-agent\/checks_custom.d\n/,
321335
)}
322336
end
323337
context 'with configuration for a custom checks.d' do
324-
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
338+
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
325339
it { should contain_file('/etc/dd-agent/datadog.conf').with(
326340
'content' => /^additional_checksd: \/etc\/dd-agent\/checks_custom.d\n/,
327341
)}
328342
end
329-
330343
context 'with configuration for a custom checks.d' do
331-
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
344+
let(:params) {{:additional_checksd => '/etc/dd-agent/checks_custom.d' }}
332345
it { should contain_file('/etc/dd-agent/datadog.conf').with(
333346
'content' => /^additional_checksd: \/etc\/dd-agent\/checks_custom.d\n/,
334347
)}
@@ -357,28 +370,32 @@
357370
'content' => /^pup_port: 17126\n/,
358371
)}
359372
end
373+
context 'with a custom pup_port, specified as an integer' do
374+
let(:params) {{:pup_port => 17126 }}
375+
it { should contain_file('/etc/dd-agent/datadog.conf').with(
376+
'content' => /^pup_port: 17126\n/,
377+
)}
378+
end
360379
context 'with a custom pup_interface' do
361380
let(:params) {{:pup_interface => 'notalocalhost' }}
362381
it { should contain_file('/etc/dd-agent/datadog.conf').with(
363382
'content' => /^pup_interface: notalocalhost\n/,
364383
)}
365384
end
366-
367385
context 'with a custom pup_url' do
368386
let(:params) {{:pup_url => 'http://localhost:17126' }}
369387
it { should contain_file('/etc/dd-agent/datadog.conf').with(
370388
'content' => /^pup_url: http:\/\/localhost:17126\n/,
371389
)}
372390
end
373-
374391
context 'with use_dogstatsd set to no' do
375392
let(:params) {{:use_dogstatsd => false}}
376393
it { should contain_file('/etc/dd-agent/datadog.conf').with(
377394
'content' => /^use_dogstatsd: no\n/,
378395
)}
379396
end
380-
context 'with dogstatsd_port set to 8126' do
381-
let(:params) {{:dogstatsd_port => 8126}}
397+
context 'with dogstatsd_port set to 8126 - must be specified as an integer!' do
398+
let(:params) {{:dogstatsd_port => 8126 }}
382399
it { should contain_file('/etc/dd-agent/datadog.conf').with(
383400
'content' => /^dogstatsd_port: 8126\n/,
384401
)}
@@ -414,13 +431,13 @@
414431
)}
415432
end
416433
context 'with statsd_forward_port set to 8126' do
417-
let(:params) {{:statsd_forward_port => '8126' }}
434+
let(:params) {{:statsd_forward_port => '8126' }}
418435
it { should contain_file('/etc/dd-agent/datadog.conf').with(
419436
'content' => /^statsd_forward_port: 8126\n/,
420437
)}
421438
end
422-
context 'with statsd_forward_port set to 8126' do
423-
let(:params) {{:statsd_forward_port => '8126' }}
439+
context 'with statsd_forward_port set to 8126, specified as an integer' do
440+
let(:params) {{:statsd_forward_port => 8126 }}
424441
it { should contain_file('/etc/dd-agent/datadog.conf').with(
425442
'content' => /^statsd_forward_port: 8126\n/,
426443
)}
@@ -438,14 +455,20 @@
438455
)}
439456
end
440457
context 'with ganglia_host set to localhost and ganglia_port set to 12345' do
441-
let(:params) {{:ganglia_host => 'testhost', :ganglia_port => '12345' }}
458+
let(:params) {{:ganglia_host => 'testhost', :ganglia_port => '12345' }}
442459
it { should contain_file('/etc/dd-agent/datadog.conf').with(
443460
'content' => /^ganglia_port: 12345\n/,
444461
)}
445462
it { should contain_file('/etc/dd-agent/datadog.conf').with(
446463
'content' => /^ganglia_host: testhost\n/,
447464
)}
448465
end
466+
context 'with ganglia_host set to localhost and ganglia_port set to 12345, port specified as an integer' do
467+
let(:params) {{:ganglia_host => 'testhost', :ganglia_port => 12345 }}
468+
it { should contain_file('/etc/dd-agent/datadog.conf').with(
469+
'content' => /^ganglia_port: 12345\n/,
470+
)}
471+
end
449472
context 'with dogstreams set to /path/to/log1:/path/to/parser' do
450473
let(:params) {{:dogstreams => ['/path/to/log1:/path/to/parser'] }}
451474
it { should contain_file('/etc/dd-agent/datadog.conf').with(
@@ -501,7 +524,13 @@
501524
)}
502525
end
503526
context 'with syslog port set to 8080' do
504-
let(:params) {{:syslog_port => '8080' }}
527+
let(:params) {{:syslog_port => '8080' }}
528+
it { should contain_file('/etc/dd-agent/datadog.conf').with(
529+
'content' => /^syslog_port: 8080\n/,
530+
)}
531+
end
532+
context 'with syslog port set to 8080, specified as an integer' do
533+
let(:params) {{:syslog_port => 8080 }}
505534
it { should contain_file('/etc/dd-agent/datadog.conf').with(
506535
'content' => /^syslog_port: 8080\n/,
507536
)}

‎templates/datadog.conf.erb‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ dd_url: <%= @dd_url %>
1212
<% else -%>
1313
proxy_host: <%= @proxy_host %>
1414
<% end -%>
15-
<% if @proxy_port.empty? -%>
15+
<% if @_proxy_port.empty? -%>
1616
# proxy_port:
1717
<% else -%>
18-
proxy_port: <%= @proxy_port %>
18+
proxy_port: <%= @_proxy_port %>
1919
<% end -%>
2020
<% if @proxy_user.empty? -%>
2121
# proxy_user:
@@ -85,17 +85,17 @@ recent_point_threshold: <%= @recent_point_threshold %>
8585
<% end -%>
8686

8787
# Change port the Agent is listening to
88-
<% if @listen_port.empty? -%>
88+
<% if @_listen_port.empty? -%>
8989
# listen_port: 17123
9090
<% else -%>
91-
listen_port: <%= @listen_port %>
91+
listen_port: <%= @_listen_port %>
9292
<% end -%>
9393

9494
# Start a graphite listener on this port
95-
<% if @graphite_listen_port.empty? -%>
95+
<% if @_graphite_listen_port.empty? -%>
9696
# graphite_listen_port: 17124
9797
<% else -%>
98-
graphite_listen_port: <%= @graphite_listen_port %>
98+
graphite_listen_port: <%= @_graphite_listen_port %>
9999
<% end -%>
100100

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

135135
use_pup: <%= @use_pup ? "yes" : "no" %>
136136

137-
<% if @pup_port.empty? -%>
137+
<% if @_pup_port.empty? -%>
138138
# pup_port: 17125
139139
<% else -%>
140-
pup_port: <%= @pup_port %>
140+
pup_port: <%= @_pup_port %>
141141
<% end -%>
142142

143143
<% if @pup_interface.empty? -%>
@@ -200,10 +200,10 @@ histogram_percentiles: <%= @statsd_histogram_percentiles %>
200200
<% else -%>
201201
statsd_forward_host: <%= @statsd_forward_host %>
202202
<% end -%>
203-
<% if @statsd_forward_port.empty? -%>
203+
<% if @_statsd_forward_port.empty? -%>
204204
# statsd_forward_port: 8125
205205
<% else -%>
206-
statsd_forward_port: <%= @statsd_forward_port %>
206+
statsd_forward_port: <%= @_statsd_forward_port %>
207207
<% end -%>
208208

209209
# ========================================================================== #
@@ -340,10 +340,10 @@ log_to_syslog: <%= @log_to_syslog ? "yes" : "no" %>
340340
syslog_host: <%= @syslog_host %>
341341
<% end -%>
342342

343-
<% if @syslog_port.empty? -%>
343+
<% if @_syslog_port.empty? -%>
344344
# syslog_port:
345345
<% else -%>
346-
syslog_port: <%= @syslog_port %>
346+
syslog_port: <%= @_syslog_port %>
347347
<% end -%>
348348

349349

0 commit comments

Comments
 (0)
⚡