Skip to content

Commit ba2c378

Browse files
david22swandavid22swan
authored
Merge pull request #863 from puppetlabs/maint-codebase-hardening
(maint) Hardening manifests and tasks
2 parents 4625798 + 40f1a09 commit ba2c378

29 files changed

+167
-178
lines changed

‎manifests/compose.pp

+2-3
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,6 @@
7171
}
7272

7373
if $facts['os']['family'] == 'windows' {
74-
$docker_download_command = "if (Invoke-WebRequest ${docker_compose_url} ${proxy_opt} -UseBasicParsing -OutFile \"${docker_compose_location_versioned}\") { exit 0 } else { exit 1}" # lint:ignore:140chars
75-
7674
exec { "Install Docker Compose ${version}":
7775
command => template('docker/windows/download_docker_compose.ps1.erb'),
7876
provider => powershell,
@@ -89,10 +87,11 @@
8987
ensure_packages(['curl'])
9088
}
9189

90+
$compose_install = "curl -s -S -L ${proxy_opt} ${docker_compose_url} -o ${docker_compose_location_versioned}"
9291
exec { "Install Docker Compose ${version}":
9392
path => '/usr/bin/',
9493
cwd => '/tmp',
95-
command => "curl -s -S -L ${proxy_opt} ${docker_compose_url} -o ${docker_compose_location_versioned}",
94+
command => $compose_install,
9695
creates => $docker_compose_location_versioned,
9796
require => Package['curl'],
9897
}

‎manifests/machine.pp

+2-3
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@
5656
}
5757

5858
if $facts['os']['family'] == 'windows' {
59-
$docker_download_command = "if (Invoke-WebRequest ${docker_machine_url} ${proxy_opt} -UseBasicParsing -OutFile \"${docker_machine_location_versioned}\") { exit 0 } else { exit 1}" # lint:ignore:140chars
60-
6159
exec { "Install Docker Machine ${version}":
6260
command => template('docker/windows/download_docker_machine.ps1.erb'),
6361
provider => powershell,
@@ -74,10 +72,11 @@
7472
ensure_packages(['curl'])
7573
}
7674

75+
$install_command = ['curl', '-s', '-S', '-L', $proxy_opt, $docker_machine_url, '-o', $docker_machine_location_versioned] # lint:ignore:140chars
7776
exec { "Install Docker Machine ${version}":
7877
path => '/usr/bin/',
7978
cwd => '/tmp',
80-
command => "curl -s -S -L ${proxy_opt} ${docker_machine_url} -o ${docker_machine_location_versioned}",
79+
command => $install_command,
8180
creates => $docker_machine_location_versioned,
8281
require => Package['curl'],
8382
}

‎manifests/plugin.pp

+7-4
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@
8585
}
8686
)
8787

88-
$exec_rm = "${docker_command} rm ${docker_plugin_remove_flags}"
88+
$exec_rm = [$docker_command, 'rm', $docker_plugin_remove_flags]
8989
$onlyif_rm = "${docker_command} ls --format='{{.PluginReference}}' | grep -w ${plugin_name}"
9090

9191
exec { "plugin remove ${plugin_name}":
@@ -105,7 +105,7 @@
105105
}
106106
)
107107

108-
$exec_enable = "${docker_command} enable ${docker_plugin_enable_flags}"
108+
$exec_enable = [$docker_command, 'enable', $docker_plugin_enable_flags]
109109
$onlyif_enable = "${docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w ${plugin_name}"
110110

111111
exec { "plugin enable ${plugin_name}":
@@ -116,12 +116,15 @@
116116
onlyif => $onlyif_enable,
117117
}
118118
} elsif $enabled == false {
119+
$else_command = [$docker_command, 'disable', $plugin_name]
120+
$else_unless = "${docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w ${plugin_name}"
121+
119122
exec { "disable ${plugin_name}":
120-
command => "${docker_command} disable ${plugin_name}",
123+
command => $else_command,
121124
environment => 'HOME=/root',
122125
path => ['/bin', '/usr/bin',],
123126
timeout => 0,
124-
unless => "${docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w ${plugin_name}",
127+
unless => $else_unless,
125128
}
126129
}
127130
}

‎manifests/registry.pp

+2-2
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@
112112
default => $pass_hash
113113
}
114114

115-
$_auth_command = "${auth_cmd} || (rm -f \"/${_local_user_home}/registry-auth-puppet_receipt_${server_strip}_${local_user}\"; exit 1;)"
115+
$_auth_command = [$auth_cmd, '||', "(rm -f \"/${_local_user_home}/registry-auth-puppet_receipt_${server_strip}_${local_user}\"; exit 1;)"] # lint:ignore:140chars
116116

117117
file { "/${_local_user_home}/registry-auth-puppet_receipt_${server_strip}_${local_user}":
118118
ensure => $ensure,
@@ -125,7 +125,7 @@
125125
# server may be an URI, which can contain /
126126
$server_strip = regsubst($server, '[/:]', '_', 'G')
127127
$passfile = "${::docker_user_temp_path}/registry-auth-puppet_receipt_${server_strip}_${local_user}"
128-
$_auth_command = "if (-not (${auth_cmd})) { Remove-Item -Path ${passfile} -Force -Recurse -EA SilentlyContinue; exit 1 } else { exit 0 }" # lint:ignore:140chars
128+
$_auth_command = ["if (-not (${auth_cmd}))", "{ Remove-Item -Path ${passfile}", '-Force', '-Recurse', '-EA', 'SilentlyContinue; exit 1 } else { exit 0 }'] # lint:ignore:140chars
129129

130130
if $ensure == 'absent' {
131131
file { $passfile:

‎manifests/run.pp

+41-58
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,10 @@
373373
}
374374

375375
if $restart_on_unhealthy {
376+
$unhealthy_command = [$docker_command, 'restart', $sanitised_title]
377+
376378
exec { "Restart unhealthy container ${title} with docker":
377-
command => "${docker_command} restart ${sanitised_title}",
379+
command => $unhealthy_command,
378380
onlyif => $restart_check,
379381
environment => $exec_environment,
380382
path => $exec_path,
@@ -385,18 +387,24 @@
385387

386388
if $restart {
387389
if $ensure == 'absent' {
390+
$restart_stop_command = [$docker_command, 'stop', '--time', $stop_wait_time, $sanitised_title]
391+
$restart_stop_onlyif = [[$docker_command, 'inspect', $sanitised_title]]
392+
388393
exec { "stop ${title} with docker":
389-
command => "${docker_command} stop --time=${stop_wait_time} ${sanitised_title}",
390-
onlyif => "${docker_command} inspect ${sanitised_title}",
394+
command => $restart_stop_command,
395+
onlyif => $restart_stop_onlyif,
391396
environment => $exec_environment,
392397
path => $exec_path,
393398
provider => $exec_provider,
394399
timeout => $exec_timeout,
395400
}
396401

402+
$restart_remove_command = "${docker_command} rm -v ${sanitised_title}"
403+
$restart_remove_onlyif = [[$docker_command, 'inspect', $sanitised_title]]
404+
397405
exec { "remove ${title} with docker":
398-
command => "${docker_command} rm -v ${sanitised_title}",
399-
onlyif => "${docker_command} inspect ${sanitised_title}",
406+
command => $restart_remove_command,
407+
onlyif => $restart_remove_onlyif,
400408
environment => $exec_environment,
401409
path => $exec_path,
402410
provider => $exec_provider,
@@ -421,57 +429,26 @@
421429
$exec_unless = $inspect
422430
}
423431

424-
if versioncmp($facts['puppetversion'], '6') < 0 {
425-
exec { "run ${title} with docker":
426-
command => join($run_with_docker_command, ' '),
427-
unless => $exec_unless,
428-
environment => $exec_environment,
429-
path => $exec_path,
430-
provider => $exec_provider,
431-
timeout => $exec_timeout,
432-
}
433-
434-
if $running == false {
435-
exec { "stop ${title} with docker":
436-
command => "${docker_command} stop --time=${stop_wait_time} ${sanitised_title}",
437-
onlyif => $container_running_check,
438-
environment => $exec_environment,
439-
path => $exec_path,
440-
provider => $exec_provider,
441-
timeout => $exec_timeout,
442-
}
443-
} else {
444-
exec { "start ${title} with docker":
445-
command => "${docker_command} start ${sanitised_title}",
446-
unless => $container_running_check,
447-
environment => $exec_environment,
448-
path => $exec_path,
449-
provider => $exec_provider,
450-
timeout => $exec_timeout,
451-
}
452-
}
453-
} else {
454-
$docker_params_changed_args = {
455-
sanitised_title => $sanitised_title,
456-
osfamily => $facts['os']['family'],
457-
command => join($run_with_docker_command, ' '),
458-
cidfile => $cidfile,
459-
image => $image,
460-
volumes => $volumes,
461-
ports => $ports,
462-
stop_wait_time => $stop_wait_time,
463-
container_running => $running,
464-
# logfile_path => ($facts['os']['family'] == 'windows') ? {
465-
# true => ::docker_user_temp_path,
466-
# default => '/tmp',
467-
# },
468-
}
432+
$docker_params_changed_args = {
433+
sanitised_title => $sanitised_title,
434+
osfamily => $facts['os']['family'],
435+
command => join($run_with_docker_command, ' '),
436+
cidfile => $cidfile,
437+
image => $image,
438+
volumes => $volumes,
439+
ports => $ports,
440+
stop_wait_time => $stop_wait_time,
441+
container_running => $running,
442+
# logfile_path => ($facts['os']['family'] == 'windows') ? {
443+
# true => ::docker_user_temp_path,
444+
# default => '/tmp',
445+
# },
446+
}
469447

470-
$detect_changes = Deferred('docker_params_changed', [$docker_params_changed_args])
448+
$detect_changes = Deferred('docker_params_changed', [$docker_params_changed_args])
471449

472-
notify { "${title}_docker_params_changed":
473-
message => $detect_changes,
474-
}
450+
notify { "${title}_docker_params_changed":
451+
message => $detect_changes,
475452
}
476453
}
477454
} else {
@@ -517,9 +494,12 @@
517494

518495
if $ensure == 'absent' {
519496
if $facts['os']['family'] == 'windows' {
497+
$absent_stop_command = "${docker_command} stop --time ${stop_wait_time} ${sanitised_title}"
498+
$absent_stop_onlyif = "${docker_command} inspect ${sanitised_title}"
499+
520500
exec { "stop container ${service_prefix}${sanitised_title}":
521-
command => "${docker_command} stop --time=${stop_wait_time} ${sanitised_title}",
522-
onlyif => "${docker_command} inspect ${sanitised_title}",
501+
command => $absent_stop_command,
502+
onlyif => $absent_stop_onlyif,
523503
environment => $exec_environment,
524504
path => $exec_path,
525505
provider => $exec_provider,
@@ -536,9 +516,12 @@
536516
notify => Exec["remove container ${service_prefix}${sanitised_title}"],
537517
}
538518
}
519+
$absent_remove_command = "${docker_command} rm -v ${sanitised_title}"
520+
$absent_remove_onlyif = "${docker_command} inspect ${sanitised_title}"
521+
539522
exec { "remove container ${service_prefix}${sanitised_title}":
540-
command => "${docker_command} rm -v ${sanitised_title}",
541-
onlyif => "${docker_command} inspect ${sanitised_title}",
523+
command => $absent_remove_command,
524+
onlyif => $absent_remove_onlyif,
542525
environment => $exec_environment,
543526
path => $exec_path,
544527
refreshonly => true,

‎manifests/secrets.pp

+7-4
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
}
2828
)
2929

30-
$exec_secret = "${docker_command} ${docker_secrets_flags}"
31-
$unless_secret = "${docker_command} inspect ${secret_name}"
30+
$exec_secret = [$docker_command, $docker_secrets_flags]
31+
$unless_secret = [$docker_command, 'inspect', $secret_name]
3232

3333
exec { "${title} docker secret create":
3434
command => $exec_secret,
@@ -38,9 +38,12 @@
3838
}
3939

4040
if $ensure == 'absent' {
41+
$absent_secret_command = [$docker_command, 'rm', $secret_name]
42+
$absent_secret_onlyif = [$docker_command, 'inspect', $secret_name]
43+
4144
exec { "${title} docker secret rm":
42-
command => "${docker_command} rm ${secret_name}",
43-
onlyif => "${docker_command} inspect ${secret_name}",
45+
command => $absent_secret_command,
46+
onlyif => $absent_secret_onlyif,
4447
path => ['/bin', '/usr/bin',],
4548
}
4649
}

‎manifests/services.pp

+8-5
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@
132132
}
133133
)
134134

135-
$exec_create = "${docker_command} create --name ${docker_service_create_flags}"
135+
$exec_create = [$docker_command, 'create', '--name', $docker_service_create_flags]
136136
$unless_create = "docker service ps ${service_name}"
137137

138138
exec { "${title} docker service create":
@@ -163,7 +163,7 @@
163163
}
164164
)
165165

166-
$exec_update = "${docker_command} update ${docker_service_flags}"
166+
$exec_update = [$docker_command, 'update', $docker_service_flags]
167167

168168
exec { "${title} docker service update":
169169
command => $exec_update,
@@ -182,7 +182,7 @@
182182
}
183183
)
184184

185-
$exec_scale = "${docker_command} scale ${service_name}=${replicas}"
185+
$exec_scale = [$docker_command, 'scale', "${service_name}=${replicas}"]
186186

187187
exec { "${title} docker service scale":
188188
command => $exec_scale,
@@ -194,9 +194,12 @@
194194
}
195195

196196
if $ensure == 'absent' {
197+
$service_command = ['docker', 'service', 'rm', $service_name]
198+
$service_onlyif = ['docker', 'service', 'ps', $service_name]
199+
197200
exec { "${title} docker service remove":
198-
command => "docker service rm ${service_name}",
199-
onlyif => "docker service ps ${service_name}",
201+
command => $service_command,
202+
onlyif => $service_onlyif,
200203
path => $exec_path,
201204
provider => $exec_provider,
202205
timeout => $exec_timeout,

‎manifests/stack.pp

+4-2
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
}
6060
)
6161

62-
$exec_stack = "${docker_command} deploy ${docker_stack_flags} ${stack_name}"
62+
$exec_stack = [$docker_command, 'deploy', $docker_stack_flags, $stack_name]
6363

6464
exec { "docker stack create ${stack_name}":
6565
command => $exec_stack,
@@ -70,8 +70,10 @@
7070
}
7171

7272
if $ensure == 'absent' {
73+
$destroy_command = [$docker_command, 'rm', $stack_name]
74+
7375
exec { "docker stack destroy ${stack_name}":
74-
command => "${docker_command} rm ${stack_name}",
76+
command => $destroy_command,
7577
onlyif => $check_stack,
7678
path => $exec_path,
7779
provider => $provider,

‎manifests/swarm.pp

+2-2
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@
117117
}
118118
)
119119

120-
$exec_init = "${docker_command} ${docker_swarm_init_flags}"
120+
$exec_init = [$docker_command, $docker_swarm_init_flags]
121121

122122
exec { 'Swarm init':
123123
command => $exec_init,
@@ -138,7 +138,7 @@
138138
}
139139
)
140140

141-
$exec_join = "${docker_command} ${docker_swarm_join_flags} ${manager_ip}"
141+
$exec_join = [$docker_command, $docker_swarm_join_flags, $manager_ip]
142142

143143
exec { 'Swarm join':
144144
command => $exec_join,

‎spec/shared_examples/machine.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
is_expected.to contain_exec("Install Docker Machine #{version}").with(
6363
'path' => '/usr/bin/',
6464
'cwd' => '/tmp',
65-
'command' => "curl -s -S -L #{proxy_opt} #{docker_machine_url} -o #{docker_machine_location_versioned}",
65+
'command' => ['curl', '-s', '-S', '-L', proxy_opt, docker_machine_url, '-o', docker_machine_location_versioned],
6666
'creates' => docker_machine_location_versioned,
6767
).that_requires(
6868
'Package[curl]',

‎spec/shared_examples/plugin.rb

+7-4
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
'force_remove' => force_remove,
4444
)
4545

46-
exec_rm = "#{docker_command} rm #{docker_plugin_remove_flags}"
46+
exec_rm = [docker_command, 'rm', docker_plugin_remove_flags]
4747
onlyif_rm = "#{docker_command} ls --format='{{.PluginReference}}' | grep -w #{plugin_name}"
4848

4949
it {
@@ -64,7 +64,7 @@
6464
'timeout' => timeout,
6565
)
6666

67-
exec_enable = "#{docker_command} enable #{docker_plugin_enable_flags}"
67+
exec_enable = [docker_command, 'enable', docker_plugin_enable_flags]
6868
onlyif_enable = "#{docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w #{plugin_name}"
6969

7070
it {
@@ -77,13 +77,16 @@
7777
)
7878
}
7979
else
80+
else_command = [docker_command, 'disable', plugin_name]
81+
else_unless = "#{docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w #{plugin_name}"
82+
8083
it {
8184
is_expected.to contain_exec("disable #{plugin_name}").with(
82-
'command' => "#{docker_command} disable #{plugin_name}",
85+
'command' => else_command,
8386
'environment' => 'HOME=/root',
8487
'path' => ['/bin', '/usr/bin'],
8588
'timeout' => 0,
86-
'unless' => "#{docker_command} ls -f enabled=false --format='{{.PluginReference}}' | grep -w #{plugin_name}",
89+
'unless' => else_unless,
8790
)
8891
}
8992
end

0 commit comments

Comments
 (0)