From 9cf2ea8a5742d8868f1f1c47377626a741464bc7 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 30 Sep 2015 17:16:27 -0700 Subject: [PATCH] Report Apache correctly when uninstalled --- .../letsencrypt_apache/configurator.py | 6 +++ .../tests/configurator_test.py | 10 ++++- .../letsencrypt_apache/tests/util.py | 45 ++++++++++--------- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ad3c62d2c..f3d2b5f9a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -137,6 +137,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :raises .errors.PluginError: If there is any other error """ + # Verify Apache is installed + for exe in (self.conf("ctl"), self.conf("enmod"), + self.conf("dismod"), self.conf("init-script")): + if not le_util.exe_exists(exe): + raise errors.NoInstallationError + # Make sure configuration is valid self.config_test() diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 026594a8f..7c2137c45 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -37,8 +37,16 @@ class TwoVhost80Test(util.ApacheTest): shutil.rmtree(self.config_dir) shutil.rmtree(self.work_dir) + @mock.patch("letsencrypt_apache.configurator.le_util.exe_exists") + def test_prepare_no_install(self, mock_exe_exists): + mock_exe_exists.return_value = False + self.assertRaises( + errors.NoInstallationError, self.config.prepare) + @mock.patch("letsencrypt_apache.parser.ApacheParser") - def test_prepare_version(self, _): + @mock.patch("letsencrypt_apache.configurator.le_util.exe_exists") + def test_prepare_version(self, mock_exe_exists, _): + mock_exe_exists.return_value = True self.config.version = None self.config.config_test = mock.Mock() self.config.get_version = mock.Mock(return_value=(1, 1)) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index b544e06ee..2594ba773 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -66,31 +66,34 @@ def get_apache_configurator( """ backups = os.path.join(work_dir, "backups") + mock_le_config = mock.MagicMock( + apache_server_root=config_path, + apache_le_vhost_ext=constants.CLI_DEFAULTS["le_vhost_ext"], + backup_dir=backups, + config_dir=config_dir, + temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), + in_progress_dir=os.path.join(backups, "IN_PROGRESS"), + work_dir=work_dir) with mock.patch("letsencrypt_apache.configurator." "subprocess.Popen") as mock_popen: - with mock.patch("letsencrypt_apache.parser.ApacheParser." - "update_runtime_variables"): - # This indicates config_test passes - mock_popen().communicate.return_value = ("Fine output", "No problems") - mock_popen().returncode = 0 + # This indicates config_test passes + mock_popen().communicate.return_value = ("Fine output", "No problems") + mock_popen().returncode = 0 + with mock.patch("letsencrypt_apache.configurator.le_util." + "exe_exists") as mock_exe_exists: + mock_exe_exists.return_value = True + with mock.patch("letsencrypt_apache.parser.ApacheParser." + "update_runtime_variables"): + config = configurator.ApacheConfigurator( + config=mock_le_config, + name="apache", + version=version) + # This allows testing scripts to set it a bit more quickly + if conf is not None: + config.conf = conf # pragma: no cover - config = configurator.ApacheConfigurator( - config=mock.MagicMock( - apache_server_root=config_path, - apache_le_vhost_ext=constants.CLI_DEFAULTS["le_vhost_ext"], - backup_dir=backups, - config_dir=config_dir, - temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), - in_progress_dir=os.path.join(backups, "IN_PROGRESS"), - work_dir=work_dir), - name="apache", - version=version) - # This allows testing scripts to set it a bit more quickly - if conf is not None: - config.conf = conf # pragma: no cover - - config.prepare() + config.prepare() return config