diff options
-rw-r--r-- | cloudinit/config/cc_set_passwords.py | 46 | ||||
-rw-r--r-- | cloudinit/distros/__init__.py | 4 | ||||
-rw-r--r-- | packages/debian/control.in | 2 | ||||
-rw-r--r-- | tests/unittests/config/test_cc_set_passwords.py | 135 |
4 files changed, 171 insertions, 16 deletions
diff --git a/cloudinit/config/cc_set_passwords.py b/cloudinit/config/cc_set_passwords.py index 939e773b..b572b26a 100644 --- a/cloudinit/config/cc_set_passwords.py +++ b/cloudinit/config/cc_set_passwords.py @@ -14,7 +14,7 @@ from textwrap import dedent from cloudinit import log as logging from cloudinit import subp, util from cloudinit.config.schema import MetaSchema, get_meta_doc -from cloudinit.distros import ALL_DISTROS, ug_util +from cloudinit.distros import ALL_DISTROS, Distro, ug_util from cloudinit.settings import PER_INSTANCE from cloudinit.ssh_util import update_ssh_config @@ -79,7 +79,7 @@ LOG = logging.getLogger(__name__) PW_SET = "".join([x for x in ascii_letters + digits if x not in "loLOI01"]) -def handle_ssh_pwauth(pw_auth, distro): +def handle_ssh_pwauth(pw_auth, distro: Distro): """Apply sshd PasswordAuthentication changes. @param pw_auth: config setting from 'pw_auth'. @@ -87,6 +87,39 @@ def handle_ssh_pwauth(pw_auth, distro): @param distro: an instance of the distro class for the target distribution @return: None""" + service = distro.get_option("ssh_svcname", "ssh") + restart_ssh = True + try: + distro.manage_service("status", service) + except subp.ProcessExecutionError as e: + if e.exit_code == 3: + # Service is not running. Write ssh config. + LOG.warning( + "Writing config 'ssh_pwauth: %s'." + " SSH service '%s' will not be restarted because is stopped.", + pw_auth, + service, + ) + restart_ssh = False + elif e.exit_code == 4: + # Service status is unknown + LOG.warning( + "Ignoring config 'ssh_pwauth: %s'." + " SSH service '%s' is not installed.", + pw_auth, + service, + ) + return + else: + LOG.warning( + "Ignoring config 'ssh_pwauth: %s'." + " SSH service '%s' is not available. Error: %s.", + pw_auth, + service, + e, + ) + return + cfg_name = "PasswordAuthentication" if isinstance(pw_auth, str): @@ -112,8 +145,11 @@ def handle_ssh_pwauth(pw_auth, distro): LOG.debug("No need to restart SSH service, %s not updated.", cfg_name) return - distro.manage_service("restart", distro.get_option("ssh_svcname", "ssh")) - LOG.debug("Restarted the SSH daemon.") + if restart_ssh: + distro.manage_service("restart", service) + LOG.debug("Restarted the SSH daemon.") + else: + LOG.debug("Not restarting SSH service: service is stopped.") def handle(_name, cfg, cloud, log, args): @@ -226,7 +262,7 @@ def handle(_name, cfg, cloud, log, args): handle_ssh_pwauth(cfg.get("ssh_pwauth"), cloud.distro) if len(errors): - log.debug("%s errors occured, re-raising the last one", len(errors)) + log.debug("%s errors occurred, re-raising the last one", len(errors)) raise errors[-1] diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index b97ee3ab..b034e2c8 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -850,7 +850,7 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta): args.append(message) return args - def manage_service(self, action, service): + def manage_service(self, action: str, service: str): """ Perform the requested action on a service. This handles the common 'systemctl' and 'service' cases and may be overridden in subclasses @@ -867,6 +867,7 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta): "restart": ["restart", service], "reload": ["reload-or-restart", service], "try-reload": ["reload-or-try-restart", service], + "status": ["status", service], } else: cmds = { @@ -876,6 +877,7 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta): "restart": [service, "restart"], "reload": [service, "restart"], "try-reload": [service, "restart"], + "status": [service, "status"], } cmd = list(init_cmd) + list(cmds[action]) return subp.subp(cmd, capture=True) diff --git a/packages/debian/control.in b/packages/debian/control.in index 7cded051..5bb915a9 100644 --- a/packages/debian/control.in +++ b/packages/debian/control.in @@ -14,7 +14,7 @@ Depends: ${misc:Depends}, iproute2, isc-dhcp-client Recommends: eatmydata, sudo, software-properties-common, gdisk -Suggests: ssh-import-id +Suggests: ssh-import-id, openssh-server Description: Init scripts for cloud instances Cloud instances need special scripts to run during initialisation to retrieve and install ssh keys and to let the user run various scripts. diff --git a/tests/unittests/config/test_cc_set_passwords.py b/tests/unittests/config/test_cc_set_passwords.py index 94c1e8cc..0b3de839 100644 --- a/tests/unittests/config/test_cc_set_passwords.py +++ b/tests/unittests/config/test_cc_set_passwords.py @@ -4,7 +4,7 @@ from unittest import mock import pytest -from cloudinit import util +from cloudinit import subp, util from cloudinit.config import cc_set_passwords as setpass from cloudinit.config.schema import ( SchemaValidationError, @@ -28,7 +28,10 @@ class TestHandleSshPwauth(CiTestCase): self.assertIn( "Unrecognized value: ssh_pwauth=floo", self.logs.getvalue() ) - m_subp.assert_not_called() + self.assertEqual( + [mock.call(["systemctl", "status", "ssh"], capture=True)], + m_subp.call_args_list, + ) @mock.patch(MODPATH + "update_ssh_config", return_value=True) @mock.patch("cloudinit.distros.subp.subp") @@ -40,6 +43,7 @@ class TestHandleSshPwauth(CiTestCase): m_subp.assert_called_with( ["systemctl", "restart", "ssh"], capture=True ) + self.assertIn("DEBUG: Restarted the SSH daemon.", self.logs.getvalue()) @mock.patch(MODPATH + "update_ssh_config", return_value=False) @mock.patch("cloudinit.distros.subp.subp") @@ -47,7 +51,10 @@ class TestHandleSshPwauth(CiTestCase): """If config is not updated, then no system restart should be done.""" cloud = self.tmp_cloud(distro="ubuntu") setpass.handle_ssh_pwauth(True, cloud.distro) - m_subp.assert_not_called() + self.assertEqual( + [mock.call(["systemctl", "status", "ssh"], capture=True)], + m_subp.call_args_list, + ) self.assertIn("No need to restart SSH", self.logs.getvalue()) @mock.patch(MODPATH + "update_ssh_config", return_value=True) @@ -57,7 +64,11 @@ class TestHandleSshPwauth(CiTestCase): cloud = self.tmp_cloud(distro="ubuntu") setpass.handle_ssh_pwauth("unchanged", cloud.distro) m_update_ssh_config.assert_not_called() - m_subp.assert_not_called() + self.assertEqual(m_update_ssh_config.call_count, 0) + self.assertEqual( + [mock.call(["systemctl", "status", "ssh"], capture=True)], + m_subp.call_args_list, + ) @mock.patch("cloudinit.distros.subp.subp") def test_valid_change_values(self, m_subp): @@ -65,12 +76,111 @@ class TestHandleSshPwauth(CiTestCase): cloud = self.tmp_cloud(distro="ubuntu") upname = MODPATH + "update_ssh_config" optname = "PasswordAuthentication" - for value in util.FALSE_STRINGS + util.TRUE_STRINGS: + for n, value in enumerate(util.FALSE_STRINGS + util.TRUE_STRINGS, 1): optval = "yes" if value in util.TRUE_STRINGS else "no" with mock.patch(upname, return_value=False) as m_update: setpass.handle_ssh_pwauth(value, cloud.distro) - m_update.assert_called_with({optname: optval}) - m_subp.assert_not_called() + self.assertEqual( + mock.call({optname: optval}), m_update.call_args_list[-1] + ) + self.assertEqual(m_subp.call_count, n) + self.assertEqual( + mock.call(["systemctl", "status", "ssh"], capture=True), + m_subp.call_args_list[-1], + ) + + @mock.patch(MODPATH + "update_ssh_config", return_value=True) + @mock.patch("cloudinit.distros.subp.subp") + def test_failed_ssh_service_is_not_runing( + self, m_subp, m_update_ssh_config + ): + """If the ssh service is not running, then the config is updated and + no restart. + """ + cloud = self.tmp_cloud(distro="ubuntu") + cloud.distro.init_cmd = ["systemctl"] + cloud.distro.manage_service = mock.Mock( + side_effect=subp.ProcessExecutionError( + stderr="Service is not running.", exit_code=3 + ) + ) + + setpass.handle_ssh_pwauth(True, cloud.distro) + self.assertIn( + r"WARNING: Writing config 'ssh_pwauth: True'." + r" SSH service 'ssh' will not be restarted because is stopped.", + self.logs.getvalue(), + ) + self.assertIn( + r"DEBUG: Not restarting SSH service: service is stopped.", + self.logs.getvalue(), + ) + self.assertEqual( + [mock.call("status", "ssh")], + cloud.distro.manage_service.call_args_list, + ) + self.assertEqual(m_update_ssh_config.call_count, 1) + self.assertEqual(m_subp.call_count, 0) + + @mock.patch(MODPATH + "update_ssh_config", return_value=True) + @mock.patch("cloudinit.distros.subp.subp") + def test_failed_ssh_service_is_not_installed( + self, m_subp, m_update_ssh_config + ): + """If the ssh service is not installed, then no updates config and + no restart. + """ + cloud = self.tmp_cloud(distro="ubuntu") + cloud.distro.init_cmd = ["systemctl"] + cloud.distro.manage_service = mock.Mock( + side_effect=subp.ProcessExecutionError( + stderr="Service is not installed.", exit_code=4 + ) + ) + + setpass.handle_ssh_pwauth(True, cloud.distro) + self.assertIn( + r"WARNING: Ignoring config 'ssh_pwauth: True'." + r" SSH service 'ssh' is not installed.", + self.logs.getvalue(), + ) + self.assertEqual( + [mock.call("status", "ssh")], + cloud.distro.manage_service.call_args_list, + ) + self.assertEqual(m_update_ssh_config.call_count, 0) + self.assertEqual(m_subp.call_count, 0) + + @mock.patch(MODPATH + "update_ssh_config", return_value=True) + @mock.patch("cloudinit.distros.subp.subp") + def test_failed_ssh_service_is_not_available( + self, m_subp, m_update_ssh_config + ): + """If the ssh service is not available, then no updates config and + no restart. + """ + cloud = self.tmp_cloud(distro="ubuntu") + cloud.distro.init_cmd = ["systemctl"] + process_error = "Service is not available." + cloud.distro.manage_service = mock.Mock( + side_effect=subp.ProcessExecutionError( + stderr=process_error, exit_code=2 + ) + ) + + setpass.handle_ssh_pwauth(True, cloud.distro) + self.assertIn( + r"WARNING: Ignoring config 'ssh_pwauth: True'." + r" SSH service 'ssh' is not available. Error: ", + self.logs.getvalue(), + ) + self.assertIn(process_error, self.logs.getvalue()) + self.assertEqual( + [mock.call("status", "ssh")], + cloud.distro.manage_service.call_args_list, + ) + self.assertEqual(m_update_ssh_config.call_count, 0) + self.assertEqual(m_subp.call_count, 0) class TestSetPasswordsHandle(CiTestCase): @@ -78,7 +188,8 @@ class TestSetPasswordsHandle(CiTestCase): with_logs = True - def test_handle_on_empty_config(self, *args): + @mock.patch(MODPATH + "subp.subp") + def test_handle_on_empty_config(self, m_subp): """handle logs that no password has changed when config is empty.""" cloud = self.tmp_cloud(distro="ubuntu") setpass.handle( @@ -89,8 +200,13 @@ class TestSetPasswordsHandle(CiTestCase): "ssh_pwauth=None\n", self.logs.getvalue(), ) + self.assertEqual( + [mock.call(["systemctl", "status", "ssh"], capture=True)], + m_subp.call_args_list, + ) - def test_handle_on_chpasswd_list_parses_common_hashes(self): + @mock.patch(MODPATH + "subp.subp") + def test_handle_on_chpasswd_list_parses_common_hashes(self, m_subp): """handle parses command password hashes.""" cloud = self.tmp_cloud(distro="ubuntu") valid_hashed_pwds = [ @@ -136,6 +252,7 @@ class TestSetPasswordsHandle(CiTestCase): logstring="chpasswd for ubuntu", ), mock.call(["pw", "usermod", "ubuntu", "-p", "01-Jan-1970"]), + mock.call(["systemctl", "status", "sshd"], capture=True), ], m_subp.call_args_list, ) |