#301 - Provision formmail templates with absolute paths

Noissue - Provision formmail templates with absolute paths

See http://hub.eyeo.com/issues/301

[f.nicolaisen, 2017-05-16 08:41:54]

Regarding the commit message, please include "See
http://hub.eyeo.com/issues/301" at the bottom.

https://codereview.adblockplus.org/29438564/diff/29438565/hiera/roles/web/acc...
File hiera/roles/web/acceptableads.yaml (right):

https://codereview.adblockplus.org/29438564/diff/29438565/hiera/roles/web/acc...
hiera/roles/web/acceptableads.yaml:37: path: "/var/committee.mail"
See my next comment about dir.

https://codereview.adblockplus.org/29438564/diff/29438565/hiera/roles/web/acc...
hiera/roles/web/acceptableads.yaml:51: committee.template = /var/committee.mail
I would not understand what this file is for when browsing the filesystem. I
would find it clearer if there's a directory for sitescripts/formmail
configuration with the committee.mail file inside.

[f.lopez, 2017-05-16 19:43:24]

What I think can also be discussed here is whether the file should be in /var or
in /etc.

https://codereview.adblockplus.org/29438564/diff/29438565/hiera/roles/web/acc...
File hiera/roles/web/acceptableads.yaml (right):

https://codereview.adblockplus.org/29438564/diff/29438565/hiera/roles/web/acc...
hiera/roles/web/acceptableads.yaml:37: path: "/var/committee.mail"
On 2017/05/16 08:41:54, f.nicolaisen wrote:
> See my next comment about dir.

Acknowledged.

https://codereview.adblockplus.org/29438564/diff/29438565/hiera/roles/web/acc...
hiera/roles/web/acceptableads.yaml:51: committee.template = /var/committee.mail
On 2017/05/16 08:41:54, f.nicolaisen wrote:
> I would not understand what this file is for when browsing the filesystem.
To be fair, a managed system is not supposed to be browsed manually.

> I would find it clearer if there's a directory for sitescripts/formmail
> configuration with the committee.mail file inside.

I can agree with this at some extent, I did this because with puppet is kinda
hard to create a full path in case it doesn't exist, and since the configuration
is in hiera, you don't have to understand the parts from the filesystem but from
hiera itself.

[mathias, 2017-05-16 20:59:48]

I think this should not be a part of the web::server class but a named type
sitescripts::formmail::template with multiple possible instances. Following our
common pattern this implies a $::sitescripts::formmail::templates hash parameter
for the new sitescripts::formmail class, used to create_resources() accordingly,
and that class could be included on demand.

This also allows for following the virtual directory pattern from the
adblockplus::* namespace, i.e. having a virutal @file /var/sitescripts to be a
directory, /var/sitescripts/formmail to be another one, and each instance of the
template type place their stuff in there by default, realizing the former dirs.

https://codereview.adblockplus.org/29438564/diff/29438565/modules/web/manifes...
File modules/web/manifests/server.pp (right):

https://codereview.adblockplus.org/29438564/diff/29438565/modules/web/manifes...
modules/web/manifests/server.pp:18: ensure => absent,
Do you really think ensure => "absent" is a reasonable default value? Also I
think a default path would be quite useful here.

[f.lopez, 2017-05-17 17:04:28]

On 2017/05/16 20:59:48, mathias wrote:
> I think this should not be a part of the web::server class but a named type
> sitescripts::formmail::template with multiple possible instances. Following
our
> common pattern this implies a $::sitescripts::formmail::templates hash
parameter
> for the new sitescripts::formmail class, used to create_resources()
accordingly,
> and that class could be included on demand.
> 
> This also allows for following the virtual directory pattern from the
> adblockplus::* namespace, i.e. having a virutal @file /var/sitescripts to be a
> directory, /var/sitescripts/formmail to be another one, and each instance of
the
> template type place their stuff in there by default, realizing the former
dirs.
> 
>
https://codereview.adblockplus.org/29438564/diff/29438565/modules/web/manifes...
> File modules/web/manifests/server.pp (right):
> 
>
https://codereview.adblockplus.org/29438564/diff/29438565/modules/web/manifes...
> modules/web/manifests/server.pp:18: ensure => absent,
> Do you really think ensure => "absent" is a reasonable default value? Also I
> think a default path would be quite useful here.

Ok I like this, gonna work on it.

[f.lopez, 2018-03-07 17:26:46]

https://codereview.adblockplus.org/29438564/diff/29715793/hiera/roles/web/acc...
File hiera/roles/web/acceptableads.yaml (right):

https://codereview.adblockplus.org/29438564/diff/29715793/hiera/roles/web/acc...
hiera/roles/web/acceptableads.yaml:50: committee.csv_log =
/var/log/committee_email.csv_log
BTW, this is related to http://hub.eyeo.com/issues/446, which I plan to do a
different commit

[mathias, 2018-03-07 17:44:18]

https://codereview.adblockplus.org/29438564/diff/29715793/modules/sitescripts...
File modules/sitescripts/manifests/formmail.pp (right):

https://codereview.adblockplus.org/29438564/diff/29715793/modules/sitescripts...
modules/sitescripts/manifests/formmail.pp:16: #   filtermaster::repo_download
{'exceptionrules':
The example section should either vanish or contain examples matching the
underlying class.

https://codereview.adblockplus.org/29438564/diff/29715793/modules/sitescripts...
modules/sitescripts/manifests/formmail.pp:37: $templates =
parsejson(inline_template('<%= require "json";
I will be so happy when the 4.x / future parser renders these hacks
unnecessary..

[f.lopez, 2018-03-07 23:01:48]

https://codereview.adblockplus.org/29438564/diff/29715793/modules/sitescripts...
File modules/sitescripts/manifests/formmail.pp (right):

https://codereview.adblockplus.org/29438564/diff/29715793/modules/sitescripts...
modules/sitescripts/manifests/formmail.pp:16: #   filtermaster::repo_download
{'exceptionrules':
On 2018/03/07 17:44:17, mathias wrote:
> The example section should either vanish or contain examples matching the
> underlying class.

Acknowledged.

https://codereview.adblockplus.org/29438564/diff/29715793/modules/sitescripts...
modules/sitescripts/manifests/formmail.pp:37: $templates =
parsejson(inline_template('<%= require "json";
On 2018/03/07 17:44:17, mathias wrote:
> I will be so happy when the 4.x / future parser renders these hacks
> unnecessary..

Well I'm working on http://hub.eyeo.com/issues/1254 right now, but the
transition from this hack to a proper loop can be done in a follow up ticket
after said ticket is closed.

[mathias, 2018-03-08 06:57:56]

LGTM.