Add Filtermaster

modules/filtermaster/manifests/init.pp

+class filtermaster {
+  #changing cron default

[Felix Dahlke, 2013/11/08 08:55:58]

I find this rather obvious, same goes for the other comments below like "add
user". Don't think they add anything here.

[christian, 2013/11/08 11:36:56]

Done.

+  Cron{

[Felix Dahlke, 2013/11/08 08:55:58]

Space before "{"

[christian, 2013/11/08 11:36:56]

Done.

+    environment => ['MAILTO=cvervoorts@adblockplus.org', 'PYTHONPATH=/opt/sitescripts'],
+  }
+        
+  class {'ssh':
+    custom_configuration => 'Match User rsync
+    AllowTcpForwarding no
+    X11Forwarding no
+    AllowAgentForwarding no
+    GatewayPorts no
+    ForceCommand rsync --server --sender -vltprz --delete-excluded --exclude CVS . /home/rsync/subscriptions/'
+        }

[Felix Dahlke, 2013/11/08 08:55:58]

Indentation is off here.

[christian, 2013/11/08 11:36:56]

Done.

+  
+  #add user
+  user { 'rsync':

[Felix Dahlke, 2013/11/08 08:55:58]

No space between "{" and "'".

[christian, 2013/11/08 11:36:56]

Done.

+    ensure     => present,

[Felix Dahlke, 2013/11/08 08:55:58]

We never align operators on columns, so just a single space before "=>" please.
Likewise below.

[christian, 2013/11/08 11:36:56]

Done.

+    comment => 'Filter list mirror user',
+    home       => '/home/rsync',
+    managehome => true
+  }
+         
+  file {'/home/rsync/.ssh':
+          ensure => directory,

[Felix Dahlke, 2013/11/08 08:55:58]

Indentation is off here, should be two spaces.

[christian, 2013/11/08 11:36:56]

Done.

+          require => User['rsync'],
+          owner => rsync,
+          mode => 0600;

[Felix Dahlke, 2013/11/08 08:55:58]

No semicolon here.

[christian, 2013/11/08 11:36:56]

Done.

+  }
+         
+  file {'/home/rsync/.ssh/authorized_keys':
+          ensure => file,

[Felix Dahlke, 2013/11/08 08:55:58]

Indentation is off here as well.

[christian, 2013/11/08 11:36:56]

Done.

+          require => [
+          File['/home/rsync/.ssh'],

[Felix Dahlke, 2013/11/08 08:55:58]

Puppet normally requires parent directories automatically, are you sure this is
necessary?

[christian, 2013/11/08 11:36:56]

I think it's needed to set the permissions of the .ssh dir right. SSH don't work
if the permissions are not correct. So yes this is necessary in my opinion.

[Felix Dahlke, 2013/11/08 15:49:39]

I'd be really surprised if it was necessary. Can you try a fresh provision
without this? It should work. If it doesn't, we should figure out why and add a
comment here.

+          User['rsync']
+          ],
+          owner => rsync,
+          mode => 0600,
+          source => 'puppet:///modules/filtermaster/authorized_keys'
+  }
+
+  file {'/etc/sitescripts':

[Felix Dahlke, 2013/11/08 08:55:58]

I think you should pass this to the sitescripts class, instead of
sitescripts.ini which I don't think we need. Or am I missing something?

[christian, 2013/11/08 11:36:56]

The scripts are not taking the sitescripts.ini file. They need a sitescripts
file. Don't know why. Also the content of the file is only need for the
filtermaster

[Felix Dahlke, 2013/11/08 15:49:39]

You use sitescriptsini_source at the bottom of the file - it points to a now
nonexistent file (sitescripts.ini). How I see it, you should be able to get rid
of this file resource here and just point to
puppet:///modules/filtermaster/sitescripts down there.

+          ensure => file,

[Felix Dahlke, 2013/11/08 08:55:58]

Indentation's off again.

[christian, 2013/11/08 11:36:56]

Done.

+          owner => root,
+          mode => 0644,
+          source => 'puppet:///modules/filtermaster/sitescripts'
+  }
+
+#donwload the repos
+
+ exec { "fetch_easylist":

[Felix Dahlke, 2013/11/08 08:55:58]

No space before ", " should actually be '. Likewise below.

More importantly, seeing how the below exec blocks are nearly identical, I think
we should use a class for this to reduce duplication.

+    command => "hg clone https://hg.adblockplus.org/easylist /home/rsync/subscription/easylist",
+    path => ["/usr/bin/", "/bin/"],
+    require => Package['mercurial'],
+    user => rsync,
+    timeout => 0,
+    onlyif => "test ! -d /home/rsync/subscription/easylist"
+  }
+         
+ exec { "fetch_easylist_germany":
+    command => "hg clone https://hg.adblockplus.org/easylistgermany /home/rsync/subscription/easylistgermany",
+    path => ["/usr/bin/", "/bin/"],
+    require => Package['mercurial'],
+    user => rsync,
+    timeout => 0,
+    onlyif => "test ! -d /home/rsync/subscription/easylistgermany"
+  }
+
+ exec { "fetch_easylist_italy":
+    command => "hg clone https://hg.adblockplus.org/easylistitaly /home/rsync/subscription/easylistitaly",
+    path => ["/usr/bin/", "/bin/"],
+    require => Package['mercurial'],
+    user => rsync,
+    timeout => 0,
+    onlyif => "test ! -d /home/rsync/subscription/easylistitaly"
+  } 
+
+ exec { "fetch_easylist_combinations":
+    command => "hg clone https://hg.adblockplus.org/easylistcombinations /home/rsync/subscription/easylistcombinations",
+    path => ["/usr/bin/", "/bin/"],
+    require => Package['mercurial'],
+    user => rsync,
+    timeout => 0,
+    onlyif => "test ! -d /home/rsync/subscription/easylistcombinations"
+  } 
+
+ exec { "fetch_malwaredomains":
+    command => "hg clone https://hg.adblockplus.org/malwaredomains /home/rsync/subscription/malwaredomains",
+    path => ["/usr/bin/", "/bin/"],
+    require => Package['mercurial'],
+    user => rsync,
+    timeout => 0,
+    onlyif => "test ! -d /home/rsync/subscription/malwaredomains"
+  } 
+
+ exec { "fetch_ruadlist":
+    command => "hg clone https://hg.adblockplus.org/ruadlist /home/rsync/subscription/ruadlist",
+    path => ["/usr/bin/", "/bin/"],
+    require => Package['mercurial'],
+    user => rsync,
+    timeout => 0,
+    onlyif => "test ! -d /home/rsync/subscription/ruadlist"
+  }
+
+ exec { "fetch_listefr":
+    command => "hg clone https://hg.adblockplus.org/listefr /home/rsync/subscription/listefr",
+    path => ["/usr/bin/", "/bin/"],
+    require => Package['mercurial'],
+    user => rsync,
+    timeout => 0,
+    onlyif => "test ! -d /home/rsync/subscription/listefr"
+  } 
+
+ exec { "fetch_exceptionrules":
+    command => "hg clone https://hg.adblockplus.org/exceptionrules /home/rsync/subscription/exceptionrules",
+    path => ["/usr/bin/", "/bin/"],
+    require => Package['mercurial'],
+    user => rsync,
+    timeout => 0,
+    onlyif => "test ! -d /home/rsync/subscription/exceptionrules"
+  }
+
+  #cron entrys
+  
+  cron { updateSubscription:

[Felix Dahlke, 2013/11/08 08:55:58]

No space after "{", and updateSubscription should be 'update_subscription' for
consistency. Likewise below ("updateMalware").

[christian, 2013/11/08 11:36:56]

Done.

+    ensure => present,
+    command => "python -m sitescripts.subscriptions.bin.updateSubscriptionDownloads 3>&1 1>/dev/null 2>&3 | perl -pe 's/^/\"[\" . scalar localtime() . \"] \"/e' >> /tmp/subscription_errors && chmod 666 /tmp/subscription_errors 2>/dev/null",
+    user    => rsync,
+    require => [
+    User['rsync']
+    ],
+    minute  => '*/10'
+  }
+         

[Felix Dahlke, 2013/11/08 08:55:58]

One empty line is enough.

[christian, 2013/11/08 11:36:56]

Done.

+         
+  cron { updateMalware:
+    ensure => present,
+    command => "python -m sitescripts.subscriptions.bin.updateMalwareDomainsList",
+    user    => rsync,
+    require => [
+    User['rsync']
+    ],
+    hour    => '*/6',
+    minute  => 15
+  }
+
+#  cron { forwardErrors:

[Felix Dahlke, 2013/11/08 08:55:58]

Please remove this code, it's commented out anyway.

[christian, 2013/11/08 11:36:56]

Done.

+#          ensure => present,
+#         command => "/srv/www/vhosts/adblockplus.org/files/forward_subscription_errors.sh",
+#         user    => rsync,
+#         require => [
+#         User['rsync']
+#         ],
+#         hour    => 0,
+#         minute  => 0
+#  }
+
+  class {'sitescripts':
+    sitescriptsini_source => 'puppet:///modules/filtermaster/sitescripts.ini'
+  }
+
+}