#2317 - provide role fileserver for eyeofiles.com

modules/fileserver/manifests/init.pp

+# This class is a quickfix for solving http://hub.eyeo.com/issues/2295
+class fileserver(

[mathias, 2017/09/21 11:43:20]

This should be an entity within namespace adblockplus::web, e.g.
adblockplus::web::fileserver. My further feedback will assume it is (you'll see
why).

[f.nicolaisen, 2017/09/21 14:17:13]

Acknowledged.

+    $domain,
+    $certificate,
+    $private_key,
+    $is_default=false
+  ) {
+
+  class {'nginx':

[mathias, 2017/09/21 11:43:20]

Please use `include` when there are no explicit parameters required.

[f.nicolaisen, 2017/09/21 14:17:13]

Acknowledged.

+  }
+
+  File {
+    owner => 'root',
+    group => 'root',
+    mode => '0755',
+  }
+
+  # Root directory for serving files:
+  file {'/var/files':

[mathias, 2017/09/21 11:43:20]

Probably obsolete, see below.

[f.nicolaisen, 2017/09/21 14:17:13]

Acknowledged.

+    ensure => directory,
+  }
+
+  file {'/var/www':

[mathias, 2017/09/21 11:43:20]

You can drop this one in favor of including the adblockplus::web class.

[f.nicolaisen, 2017/09/21 14:17:13]

Acknowledged.

+    ensure => directory,
+  }
+
+  # Directory with new group www-v8 where uploaders can be made members of:

[mathias, 2017/09/21 11:43:20]

What for? That should be documented.

[f.nicolaisen, 2017/09/21 14:17:13]

Acknowledged.

+  # Members are handled manually on-demand for now!
+  group {'www-v8':
+    ensure => present,
+  }->

[mathias, 2017/09/21 11:43:20]

Please avoid using this form of relationship notation, it's easily overseen and
hard to grep for. You can always use Explicit[foo] -> Bar[baz] notation after
the resource definition (recommended), or just the plain old parameters where
applicable (outside the ensure_resource stuff).

[f.nicolaisen, 2017/09/21 14:17:13]

Acknowledged.

+
+  file {'/var/files/v8':

[mathias, 2017/09/21 11:43:20]

What for? This should be something with a more meaningful basename, and it
should be a sub-directory of $adblockplus::directory (requires inclusion of
class adblockplus and explicit realization).

[f.nicolaisen, 2017/09/21 14:17:13]

Acknowledged.

+    ensure => directory,
+    group => 'www-v8',
+    mode => '0775',
+  }
+
+  #Symlink above directory into www:
+  file {'/var/www/v8':

[mathias, 2017/09/21 11:43:19]

What for? That should be documented.

[f.nicolaisen, 2017/09/21 14:17:13]

Acknowledged.

+    ensure => link,
+    target => '/var/files/v8',
+    require => [
+      File['/var/files/v8'],
+      Package['nginx'],
+    ],
+  }
+
+  nginx::hostconfig{$domain:
+    source => 'puppet:///modules/fileserver/site.conf',
+    is_default => $is_default,
+    certificate => $certificate,
+    private_key => $private_key,
+    log => 'access_log_update'

[mathias, 2017/09/21 11:43:20]

Why *_update?

[f.nicolaisen, 2017/09/21 14:17:13]

Acknowledged.

+  }
+}
+