Issue 2600 - Normalize ownership and priviliges for Nginx logs

modules/nginx/manifests/init.pp

 class nginx (
     $worker_processes = $nginx::params::worker_processes,
     $worker_connections = $nginx::params::worker_connections,
     $ssl_session_cache =  $nginx::params::ssl_session_cache
   ) inherits nginx::params {
 
   apt::source {'nginx':
     location => "http://nginx.org/packages/ubuntu",
     repos => "nginx",
     key => "7BD9BF62",
     key_source => "http://nginx.org/keys/nginx_signing.key"
   }
 
   # Ensures that nginx is not installed from the Ubuntu sources
   package {'nginx-common':
     ensure => purged,
     before => Package['nginx']
   }
 
   package {'nginx':
     ensure => '1.8.0-1~precise',
     require => Apt::Source['nginx']
   }
 
   File {
     owner => root,
     group => root,
     mode => 0644,
   }
+
+  Exec {
+    path => '/usr/bin:/bin',
+    logoutput => 'on_failure',
+  }
+
 
   file {'/etc/nginx/nginx.conf':
     content => template('nginx/nginx.conf.erb'),
     require => Package['nginx'],
     notify => Service['nginx']
   }
 
   file {'/etc/nginx/sites-available':
     ensure => directory,
     require => Package['nginx']
@@ skipping 72 matching lines @@
       require => File["/etc/nginx/sites-available/${domain}"],
       content => template('nginx/logrotate.erb')
     }
   }
 
   file {'/etc/logrotate.d/nginx':
     source => 'puppet:///modules/nginx/logrotate',
     require => Package['nginx']
   }
 
-  exec {"set_logfiles_owner_and_permissions":
-    command => '/bin/chown www-data.adm /var/log/nginx/* && /bin/chmod 0640 /var/log/nginx/*',

[mathias, 2015/07/03 13:51:04]

Except for intermediate fixes, please do not use shell commands - especially not
with logic operators (&&). By the way, there's no need to make the chmod
dependent on a successful chown here. Why not just creating two exec's?

Also, the asterisk wildcard can very well lead to more issues; it may expand to
either unrecognized sub-directories or, altogether, in a too long command line.
What about `find /var/log/nginx -mindepth 1 ... -exec ...`?

And please take the user from $nginx::params::user (the group can remain
hard-coded). Have a look at the shellquote() function at
https://docs.puppetlabs.com/references/latest/function.html#shellquote - it's
the best way to integrate parameters (like the user name) with your command.

[Fred, 2015/07/06 16:11:52]

Acknowledged.

-    # this should all be provided by package nginx:
-    # require => [File['/var/log/nginx'], User['www-data'], Group['adm']]

[mathias, 2015/07/03 13:51:04]

This comment is both wrong (neither User['www-data'] nor Group['adm'] is
provided by Package['nginx']) and unnecessary. The require parameter below
should say enough.

[Fred, 2015/07/06 16:11:52]

Acknowledged.

-    require => Package['nginx']

[mathias, 2015/07/03 13:51:04]

Please always use trailing commas, even when defining the last item. (I know
this has been forgotten before, even in this file.)

[Fred, 2015/07/06 16:11:52]

Acknowledged.

+  $find_cmd_base = [
+    'find', '/var/log/nginx',
+    '-mindepth', '1', '-maxdepth', '1', '-type', 'f',
+  ]
+
+  # Kill the find process to force an exit status != 0 by finding the parent pid
+  # of the exec's sh process
+  $find_kill_exec = [
+    '-exec', 'sh', '-c',
+    'ps -p $$ -o ppid= | xargs kill -TERM',
+     ';',
+  ]
+
+  $find_chown_base = [
+    $find_cmd_base,
+    '-not', '(', '-user', $nginx::params::user, '-and', '-group', 'adm', ')',
+  ]
+  $find_chown_exec = [
+    '-ls', '-exec', 'chown',
+    "${nginx::params::user}.adm", '{}', ';',
+  ]
+
+  exec {"set_logfiles_owner":
+    command => shellquote($find_chown_base, $find_chown_exec),
+    unless => shellquote($find_chown_base, $find_kill_exec),
+    subscribe => Service['nginx'], 
+  }
+
+  $find_chmod_base = [$find_cmd_base, '-not', '-perm', '0640']
+  $find_chmod_exec = ['-ls', '-exec', 'chmod', '0640', '{}', ';']
+
+  exec {"set_logfiles_permissions":
+    command => shellquote($find_chmod_base, $find_chmod_exec),
+    unless => shellquote($find_chmod_base, $find_kill_exec),
+    subscribe => Service['nginx'],
   }
 
   service {'nginx':
     ensure => running,
     enable => true,
     restart => '/etc/init.d/nginx reload',
     hasstatus => true,
     require => File['/etc/nginx/nginx.conf']
   }
 }