Issue 1094 - Generate update manifests on the update server

modules/updateserver/manifests/init.pp

 class updateserver(
     $domain,
     $certificate,
     $private_key,
     $is_default=false
   ) {
   class {'nginx':
     worker_processes => 2,
     worker_connections => 4000,
     ssl_session_cache => off,
   }
 
   File {
     owner => root,
     group => root
   }
 
   file {'/var/www':
     ensure => directory,
     mode => 0755,
     require => Package['nginx']
   }
 
-  file {'/var/www/update':
+  $update_dir = '/var/www/update'
+
+  file {$update_dir:
     ensure => directory,
     mode => 0755
   }
 
-  $update_manifest_dirs = ['/var/www/update/adblockplus',

[mathias, 2014/07/25 07:04:28]

Now that /var/www/update is used over and over, I'd prefer using a variable for
this prefix, much like $base_dir below.

[Felix Dahlke, 2014/07/25 07:17:45]

Done.

-                           '/var/www/update/adblockplusandroid',
-                           '/var/www/update/adblockplussafari']
+  $sitescripts_var_dir = '/var/lib/sitescripts'

[Felix Dahlke, 2014/07/25 14:21:01]

This was /var/lib/adblockplus before. This is now more confusing since we have
/opt/sitescripts, which suggests it's a all-in-one package, and then we have
these runtime files here in the proper FHS place.

I don't see a better way without changing the sitescripts module though.
/opt/sitescripts is a hg repository, adding files not under version control
there seems worse. I suppose the cleanest way would be to move /opt/sitescripts
to /usr/lib/sitescripts.

+
+  user {'sitescripts':
+    ensure => present,
+    home => $sitescripts_var_dir
+  }
+
+  file {$sitescripts_var_dir:
+    ensure => directory,
+    mode => 0755,
+    owner => 'sitescripts',
+    group => 'sitescripts'
+  }
+
+  $update_manifest_dirs = ["${update_dir}/gecko",
+                           "${update_dir}/adblockplusandroid",
+                           "${update_dir}/adblockplussafari"]
 
   file {$update_manifest_dirs:
+    ensure => directory,
+    mode => 0755,
+    owner => 'sitescripts',
+    group => 'sitescripts'
+  }
+
+  file {"${update_dir}/adblockplusie":
     ensure => directory,
     mode => 0755
   }
 
-  file {'/var/www/update/adblockplusie':

[mathias, 2014/07/25 07:04:28]

I assume it's intentional that adblockplusie is not included in the
$update_manifest_dirs? (I don't know these parts of our infrastructure, software
and requirements well enough yet to answer that myself..)

[Felix Dahlke, 2014/07/25 07:17:45]

Yup, we can't generate that one yet with how things currently are in
sitescripts. Working on that in parallel, will be a separate patch.

-    ensure => directory,
-    mode => 0755
-  }
-
-  file {'/var/www/update/adblockplusie/update.json':
+  file {"${update_dir}/adblockplusie/update.json":
     ensure => file,
     source => 'puppet:///modules/updateserver/adblockplusie/update.json',
     mode => 0644
   }
 
   nginx::hostconfig{$domain:
     source => 'puppet:///modules/updateserver/site.conf',
     is_default => $is_default,
     certificate => $certificate,
     private_key => $private_key,
     log => 'access_log_update'
   }
 
   class {'sitescripts':
     sitescriptsini_source => 'puppet:///modules/updateserver/sitescripts'
   }
 
-  $base_dir = '/var/lib/adblockplus'
-
-  file {$base_dir:
-    ensure => directory,
-    mode => 0755
-  }
-
-  $safari_certificate_path = "${base_dir}/adblockplussafari.pem"
+  $safari_certificate_path = "${sitescripts_var_dir}/adblockplussafari.pem"
 
   file {$safari_certificate_path:
     source => 'puppet:///modules/private/adblockplussafari.pem'
   }
 
   $repositories_to_sync = ['downloads', 'adblockplus', 'adblockplusandroid',
                            'adblockpluschrome', 'elemhidehelper', 'abpwatcher',
                            'abpcustomization', 'urlfixer']
 
   define fetch_repository() {
-    $repository_path = "${updateserver::base_dir}/${title}"
+    $repository_path = "${updateserver::sitescripts_var_dir}/${title}"
     exec {"fetch_repository_${title}":
-      command => "hg clone https://hg.adblockplus.org/${title} ${repository_path}",
-      path => '/usr/bin',

[mathias, 2014/07/25 07:04:28]

Are you sure that hg(1) is available in /usr/bin?

[Felix Dahlke, 2014/07/25 07:17:45]

Yes.

[Wladimir Palant, 2014/07/25 09:54:39]

Thinking a bit more about it - this does look like a bad assumption. What if we
at some point decide to install Mercurial via pip? I'd rather go with a standard
search path like '/bin:/usr/bin:/usr/sbin:/usr/local/bin'.

[Felix Dahlke, 2014/07/25 10:31:15]

Done. Left sbin out since I really wouldn't expect hg there.

+      command => "hg clone -U https://hg.adblockplus.org/${title} ${repository_path}",
+      path => '/usr/local/bin:/usr/bin:/bin',
+      user => 'sitescripts',
       timeout => 0,
       onlyif => "test ! -d ${repository_path}",
-      require => [Package['mercurial'], File[$updateserver::base_dir]]
+      require => [Package['mercurial'], File[$updateserver::sitescripts_var_dir]]
     }
   }
 
   fetch_repository {$repositories_to_sync: }
 
   $update_update_manifests_script = '/usr/local/bin/update_update_manifests'

[mathias, 2014/07/25 07:04:28]

Is the update_update prefix intentional?

[Felix Dahlke, 2014/07/25 07:17:45]

Yes, we're updating update manifests here.

 
   file {$update_update_manifests_script:
     mode => '0755',
     content => template('updateserver/update_update_manifests.erb')
   }
 
   $update_update_manifests_dependencies = ['python-m2crypto', 'python-jinja2']

[mathias, 2014/07/25 07:04:28]

Is the update_update prefix intentional?

[Felix Dahlke, 2014/07/25 07:17:45]

Yes.

 
   package {$update_update_manifests_dependencies:}
 
   exec {'update_update_manifests':
     command => $update_update_manifests_script,
+    user => 'sitescripts',
     timeout => 0,
     require => [Exec['fetch_sitescripts'],
                 Fetch_repository[$repositories_to_sync],
                 File[$update_update_manifests_script],
                 File[$update_manifest_dirs], File[$safari_certificate_path],
                 Package[$update_update_manifests_dependencies]]
   }
 
   cron {'update_update_manifests':
     ensure => present,
     environment => ['MAILTO=admins@adblockplus.org'],
     command => $update_update_manifests_script,
+    user => 'sitescripts',
     minute => '*/10',
     require => Exec['update_update_manifests']
   }
 }