From 08ccc64cd1dc0c852ad4189bbc37aa451e7a611c Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 6 Jun 2016 12:04:44 +0300 Subject: [PATCH 1/4] Initialize augeas in a new method --- certbot-apache/certbot_apache/augeas_configurator.py | 4 +++- certbot-apache/certbot_apache/configurator.py | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/certbot-apache/certbot_apache/augeas_configurator.py b/certbot-apache/certbot_apache/augeas_configurator.py index 12753541c..820a58438 100644 --- a/certbot-apache/certbot_apache/augeas_configurator.py +++ b/certbot-apache/certbot_apache/augeas_configurator.py @@ -1,7 +1,6 @@ """Class of Augeas Configurators.""" import logging -import augeas from certbot import errors from certbot import reverter @@ -29,6 +28,9 @@ class AugeasConfigurator(common.Plugin): def __init__(self, *args, **kwargs): super(AugeasConfigurator, self).__init__(*args, **kwargs) + def init_augeas(self): + """ Initialize the actual Augeas instance """ + import augeas self.aug = augeas.Augeas( # specify a directory to load our preferred lens from loadpath=constants.AUGEAS_LENS_DIR, diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index e4c06ba7e..9caa4a764 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -150,6 +150,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :raises .errors.PluginError: If there is any other error """ + # Perform the actual Augeas initialization to be able to react + try: + self.init_augeas() + except ImportError: + raise errors.NoInstallationError("Problem in Augeas installation") + # Verify Apache is installed if not util.exe_exists(constants.os_constant("restart_cmd")[0]): raise errors.NoInstallationError From 7239361342e0001cfc8ad958250564e50e8bd0cb Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 6 Jun 2016 12:36:54 +0300 Subject: [PATCH 2/4] Test coverage for NoInstallationError --- certbot-apache/certbot_apache/tests/configurator_test.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index a2e39de47..57344bbb6 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -55,6 +55,14 @@ class MultipleVhostsTest(util.ApacheTest): self.assertRaises( errors.NoInstallationError, self.config.prepare) + @mock.patch("certbot_apache.augeas_configurator.AugeasConfigurator.init_augeas") + def test_prepare_no_augeas(self, mock_init_augeas): + def side_effect_error(*args, **kwargs): + raise ImportError + mock_init_augeas.side_effect = side_effect_error + self.assertRaises( + errors.NoInstallationError, self.config.prepare) + @mock.patch("certbot_apache.parser.ApacheParser") @mock.patch("certbot_apache.configurator.util.exe_exists") def test_prepare_version(self, mock_exe_exists, _): From e2631322839e4a3b5117fb4581da72fa4c2eddc5 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 6 Jun 2016 12:44:49 +0300 Subject: [PATCH 3/4] Refactor and lint fixes --- .../certbot_apache/augeas_configurator.py | 20 +++++++++++-------- .../certbot_apache/tests/configurator_test.py | 4 +++- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/certbot-apache/certbot_apache/augeas_configurator.py b/certbot-apache/certbot_apache/augeas_configurator.py index 820a58438..478efb099 100644 --- a/certbot-apache/certbot_apache/augeas_configurator.py +++ b/certbot-apache/certbot_apache/augeas_configurator.py @@ -28,6 +28,18 @@ class AugeasConfigurator(common.Plugin): def __init__(self, *args, **kwargs): super(AugeasConfigurator, self).__init__(*args, **kwargs) + # Placeholder for augeas + self.aug = None + + self.save_notes = "" + + # See if any temporary changes need to be recovered + # This needs to occur before VirtualHost objects are setup... + # because this will change the underlying configuration and potential + # vhosts + self.reverter = reverter.Reverter(self.config) + self.recovery_routine() + def init_augeas(self): """ Initialize the actual Augeas instance """ import augeas @@ -37,14 +49,6 @@ class AugeasConfigurator(common.Plugin): # Do not save backup (we do it ourselves), do not load # anything by default flags=(augeas.Augeas.NONE | augeas.Augeas.NO_MODL_AUTOLOAD)) - self.save_notes = "" - - # See if any temporary changes need to be recovered - # This needs to occur before VirtualHost objects are setup... - # because this will change the underlying configuration and potential - # vhosts - self.reverter = reverter.Reverter(self.config) - self.recovery_routine() def check_parsing_errors(self, lens): """Verify Augeas can parse all of the lens files. diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 57344bbb6..e5c09fd1d 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -57,7 +57,9 @@ class MultipleVhostsTest(util.ApacheTest): @mock.patch("certbot_apache.augeas_configurator.AugeasConfigurator.init_augeas") def test_prepare_no_augeas(self, mock_init_augeas): - def side_effect_error(*args, **kwargs): + """ Test augeas initialization ImportError """ + def side_effect_error(): + """ Side effect error for the test """ raise ImportError mock_init_augeas.side_effect = side_effect_error self.assertRaises( From 1f6e999153be1347d92eef884a297966c0e15801 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 6 Jun 2016 13:10:34 +0300 Subject: [PATCH 4/4] Move recovery_routine() to augeas init --- certbot-apache/certbot_apache/augeas_configurator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-apache/certbot_apache/augeas_configurator.py b/certbot-apache/certbot_apache/augeas_configurator.py index 478efb099..6999120d6 100644 --- a/certbot-apache/certbot_apache/augeas_configurator.py +++ b/certbot-apache/certbot_apache/augeas_configurator.py @@ -38,7 +38,6 @@ class AugeasConfigurator(common.Plugin): # because this will change the underlying configuration and potential # vhosts self.reverter = reverter.Reverter(self.config) - self.recovery_routine() def init_augeas(self): """ Initialize the actual Augeas instance """ @@ -49,6 +48,7 @@ class AugeasConfigurator(common.Plugin): # Do not save backup (we do it ourselves), do not load # anything by default flags=(augeas.Augeas.NONE | augeas.Augeas.NO_MODL_AUTOLOAD)) + self.recovery_routine() def check_parsing_errors(self, lens): """Verify Augeas can parse all of the lens files.