Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#760 closed change (fixed)

Make our ssh configuration used automatically for all servers

Reported by: trev Assignee: matze
Priority: P2 Milestone:
Module: Infrastructure Keywords:
Cc: fhd, matze Blocked By: #765, #766
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/5766400081657856
http://codereview.adblockplus.org/5955657815556096

Description

Background

It seems that not all servers are currently using our sshd_config. The ssh module isn't being used consistently, currently it is used by filtermaster and statsclient modules (with some custom configuration) and in the monitoring server configuration. Other servers still use the default configuration which accepts passwords for example.

What to change

Make sure the ssh module is included by the base module, so that it is used by all servers consistently. We won't be able to pass in class parameters then, since the base module cannot know whether custom configuration is necessary. We can add custom configuration directives using the concat module however, we've already implemented it this way for the sitescripts.ini file.

Change History (12)

comment:1 Changed 5 years ago by matze

  • Owner set to matze

comment:2 Changed 5 years ago by matze

Please explain the issue you've mentioned in the "What to change" section above.

As far as I can see, custom configuration is already implemented in the ssh module, using templating (modules/ssh/templates/sshd_config.erb, last line) and an empty default parameter (modules/ssh/manifests/init.pp), and applied as parameter in e.g. modules/statsclient/manifests/init.pp.

Although I understand that this is more a hack than a clean solution (like the concat module would have been), I do not see an upcoming issue - except maybe in case of multiple roles and inheritance. So, before I convert this to use concat, I just want to make sure we're on the same page --

comment:3 Changed 5 years ago by trev

The issue is really that the ssh module can only be included once. If we include it from the base module then we won't be able to invoke it later with different parameters. The sitescripts module solves this issue by using the concat class - one fragment is defined when the sitescripts module is included, other modules can define additional fragments as needed. We can use the same approach for the ssh module, meaning that filtermaster or statsclient will define concat fragments for their server-specific SSH directives.

comment:4 Changed 5 years ago by matze

OK, got it, thank you.

comment:5 Changed 5 years ago by matze

  • Blocked By 765 added
Version 0, edited 5 years ago by matze (next)

comment:6 Changed 5 years ago by matze

  • Blocked By 766 added; 765 removed

comment:7 Changed 5 years ago by matze

  • Blocked By 772 added; 766 removed

comment:8 Changed 5 years ago by trev

  • Blocked By 765, 766 added

comment:9 Changed 5 years ago by matze

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

One question: Is the "base" relation configured via manifests/*server.pp enough?

mhennig@kali:~/AdBlockPlus/infrastructure$ grep base manifests/*
manifests/codereviewserver.pp:  include base
manifests/downloadserver.pp:  include base, statsclient
manifests/filtermasterserver.pp:  include base, filtermaster
manifests/filterserver.pp:  include base, statsclient
manifests/intraforumserver.pp:  include base, statsclient
manifests/issuesserver.pp:  include base
manifests/monitoringserver.pp:  include base, ssh, puppetmaster
manifests/statsserver.pp:  include base
manifests/updateserver.pp:  include base, statsclient
manifests/webserver.pp:  include base, statsclient

Especially when using concat::fragment for sshd_config (which is set up in modules/ssh and then included in modules/base), wouldn't it be more clean if we include base or include ssh explicitly?

(See also the review link.)

comment:10 Changed 5 years ago by matze

I've just pushed the changes, after the review was successful. How do we proceed? Who's going to deploy the modifications? And do I need to re-assign the ticket or just close it?

comment:11 Changed 5 years ago by trev

  • Blocked By 772 removed
  • Resolution set to fixed
  • Status changed from reviewing to closed

Fixed: https://hg.adblockplus.org/infrastructure/rev/8d7c8df29eba. Please close the review as well.

Let's have a brief screen sharing session tomorrow so that you can see how deployment works. Normally we deploy immediately after the changes landed.

comment:12 Changed 5 years ago by matze

  • Review URL(s) modified (diff)
Note: See TracTickets for help on using tickets.