#760 - Make our ssh configuration used automatically for all servers

See https://issues.adblockplus.org/ticket/760

[Wladimir Palant, 2014-07-15 11:56:47]

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
File modules/filtermaster/manifests/init.pp (right):

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/filtermaster/manifests/init.pp:8: order => '02',
This doesn't need to be sequential, '50' maybe?

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/filtermaster/manifests/init.pp:17: content => '
order => '99'?

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
File modules/ssh/manifests/init.pp (right):

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/ssh/manifests/init.pp:1: class ssh () {
Nit: parentheses are unnecessary if there are no parameters.

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
File modules/ssh/templates/sshd_config.erb (right):

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/ssh/templates/sshd_config.erb:90: UsePAM yes
This no longer needs to be a template, move is to files/ and use as a static
file?

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
File modules/statsclient/manifests/init.pp (right):

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/statsclient/manifests/init.pp:24: content => '
order => '99'?

[mathias, 2014-07-15 12:03:12]

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
File modules/filtermaster/manifests/init.pp (right):

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/filtermaster/manifests/init.pp:17: content => '
On 2014/07/15 11:56:47, Wladimir Palant wrote:
> order => '99'?
Either the non-block settings (see your comment above) or block settings should
become sequential. I dislike to do that for both types. Which do you prefer? (I
prefer the former option.)

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
File modules/ssh/manifests/init.pp (right):

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/ssh/manifests/init.pp:1: class ssh () {
On 2014/07/15 11:56:47, Wladimir Palant wrote:
> Nit: parentheses are unnecessary if there are no parameters.
Ok.

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
File modules/ssh/templates/sshd_config.erb (right):

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/ssh/templates/sshd_config.erb:90: UsePAM yes
On 2014/07/15 11:56:47, Wladimir Palant wrote:
> This no longer needs to be a template, move is to files/ and use as a static
> file?
Well, I could imagine performing some future global setup by passing class
parameters through to the template (e.g. the Max* directives from
modules/filtermaster/init.pp). Of course one could argue that we can re-convert
it into a template when necessary..

[mathias, 2014-07-15 12:59:35]

See https://issues.adblockplus.org/ticket/760

[Wladimir Palant, 2014-07-15 14:18:35]

LGTM

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
File modules/filtermaster/manifests/init.pp (right):

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/filtermaster/manifests/init.pp:17: content => '
On 2014/07/15 12:03:12, matze wrote:
> Either the non-block settings (see your comment above) or block settings
should
> become sequential. I dislike to do that for both types. Which do you prefer?
(I
> prefer the former option.)

I don't really understand your comment. My understanding of sshd_config is that
ordering matters - Match blocks always have to be placed at the end of the file.
It might be that omitting the order parameter will also place the fragment at
the end but the documentation doesn't really say what will happen in that
scenario so I would consider the behavior as undefined. Hence my suggestion to
always use '99' for blocks.

Using '50' instead of '02' was meant simply in case we need to add more
fragments to the generic part. We can have as convention that anything below
'50' is generic whereas '50' and above marks custom configuration.

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
File modules/ssh/templates/sshd_config.erb (right):

http://codereview.adblockplus.org/5766400081657856/diff/5629499534213120/modu...
modules/ssh/templates/sshd_config.erb:90: UsePAM yes
On 2014/07/15 12:03:12, matze wrote:
> Well, I could imagine performing some future global setup by passing class
> parameters through to the template

Well, I cannot - that fragment is being created via base module, we don't have
any server-specific parameters at this point so I don't see any reason why we
would use templates. Either way, our general approach is not to keep things
around just because we might need it at some point.