From 65f4332798a8a32c60ff55817454264a49565187 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 31 Jul 2015 12:49:39 -0700 Subject: [PATCH] Update IPlugin docs, make augeas conform --- .../letsencrypt_apache/augeas_configurator.py | 56 ++++++++++++------- .../tests/augeas_configurator_test.py | 38 +++++++++++++ letsencrypt/interfaces.py | 35 ++++++++++-- letsencrypt/reverter.py | 2 + 4 files changed, 107 insertions(+), 24 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py index 4b61b9cc4..7557a27c6 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py @@ -38,7 +38,7 @@ class AugeasConfigurator(common.Plugin): # because this will change the underlying configuration and potential # vhosts self.reverter = reverter.Reverter(self.config) - self.reverter.recovery_routine() + self.recovery_routine() def check_parsing_errors(self, lens): """Verify Augeas can parse all of the lens files. @@ -63,6 +63,7 @@ class AugeasConfigurator(common.Plugin): path[13:len(path) - 6], self.aug.get(path + "/message")) raise errors.PluginError(msg) + # TODO: Cleanup this function def save(self, title=None, temporary=False): """Saves all changes to the configuration files. @@ -78,8 +79,7 @@ class AugeasConfigurator(common.Plugin): be quickly reversed in the future (ie. challenges) :raises .errors.PluginError: If there was an error in Augeas, in an - attempt to save the configuration. - :raises .errors.ReverterError: If unable to create the checkpoint + attempt to save the configuration, or an error creating a checkpoint """ save_state = self.aug.get("/augeas/save") @@ -108,22 +108,26 @@ class AugeasConfigurator(common.Plugin): for path in save_paths: save_files.add(self.aug.get(path)[6:]) - # Create Checkpoint - if temporary: - self.reverter.add_to_temp_checkpoint( - save_files, self.save_notes) - else: - self.reverter.add_to_checkpoint(save_files, self.save_notes) + try: + # Create Checkpoint + if temporary: + self.reverter.add_to_temp_checkpoint( + save_files, self.save_notes) + else: + self.reverter.add_to_checkpoint(save_files, self.save_notes) + except errors.ReverterError as err: + raise errors.PluginError(str(err)) if title and not temporary: - self.reverter.finalize_checkpoint(title) + try: + self.reverter.finalize_checkpoint(title) + except errors.ReverterError as err: + raise errors.PluginError(str(err)) self.aug.set("/augeas/save", save_state) self.save_notes = "" self.aug.save() - return True - def _log_save_errors(self, ex_errs): """Log errors due to bad Augeas save. @@ -144,20 +148,26 @@ class AugeasConfigurator(common.Plugin): Reverts all modified files that have not been saved as a checkpoint - :raises .errors.ReverterError: If unable to recover the configuration + :raises .errors.PluginError: If unable to recover the configuration """ - self.reverter.recovery_routine() + try: + self.reverter.recovery_routine() + except errors.ReverterError as err: + raise errors.PluginError(str(err)) # Need to reload configuration after these changes take effect self.aug.load() def revert_challenge_config(self): """Used to cleanup challenge configurations. - :raises .errors.ReverterError: If unable to revert the challenge config. + :raises .errors.PluginError: If unable to revert the challenge config. """ - self.reverter.revert_temporary_config() + try: + self.reverter.revert_temporary_config() + except errors.ReverterError as err: + raise errors.PluginError(str(err)) self.aug.load() def rollback_checkpoints(self, rollback=1): @@ -165,18 +175,24 @@ class AugeasConfigurator(common.Plugin): :param int rollback: Number of checkpoints to revert - :raises .errors.ReverterError: If there is a problem with the input or + :raises .errors.PluginError: If there is a problem with the input or the function is unable to correctly revert the configuration """ - self.reverter.rollback_checkpoints(rollback) + try: + self.reverter.rollback_checkpoints(rollback) + except errors.ReverterError as err: + raise errors.PluginError(str(err)) self.aug.load() def view_config_changes(self): """Show all of the configuration changes that have taken place. - :raises .errors.ReverterError: If there is a problem while processing + :raises .errors.PluginError: If there is a problem while processing the checkpoints directories. """ - self.reverter.view_config_changes() + try: + self.reverter.view_config_changes() + except errors.ReverterError as err: + raise errors.PluginError(str(err)) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py index 8cb1fb3a8..815e6fc44 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py @@ -41,6 +41,20 @@ class AugeasConfiguratorTest(util.ApacheTest): self.assertRaises(errors.PluginError, self.config.save) + def test_bad_save_checkpoint(self): + self.config.reverter.add_to_checkpoint = mock.Mock( + side_effect=errors.ReverterError) + self.config.parser.add_dir( + self.vh_truth[0].path, "Test", "bad_save_ckpt") + self.assertRaises(errors.PluginError, self.config.save) + + def test_bad_save_finalize_checkpoint(self): + self.config.reverter.finalize_checkpoint = mock.Mock( + side_effect=errors.ReverterError) + self.config.parser.add_dir( + self.vh_truth[0].path, "Test", "bad_save_ckpt") + self.assertRaises(errors.PluginError, self.config.save, "Title") + def test_finalize_save(self): mock_finalize = mock.Mock() self.config.reverter = mock_finalize @@ -55,6 +69,13 @@ class AugeasConfiguratorTest(util.ApacheTest): self.config.recovery_routine() self.assertEqual(mock_load.call_count, 1) + def test_recovery_routine_error(self): + self.config.reverter.recovery_routine = mock.Mock( + side_effect=errors.ReverterError) + + self.assertRaises( + errors.PluginError, self.config.recovery_routine) + def test_revert_challenge_config(self): mock_load = mock.Mock() self.config.aug.load = mock_load @@ -62,6 +83,13 @@ class AugeasConfiguratorTest(util.ApacheTest): self.config.revert_challenge_config() self.assertEqual(mock_load.call_count, 1) + def test_revert_challenge_config_error(self): + self.config.reverter.revert_temporary_config = mock.Mock( + side_effect=errors.ReverterError) + + self.assertRaises( + errors.PluginError, self.config.revert_challenge_config) + def test_rollback_checkpoints(self): mock_load = mock.Mock() self.config.aug.load = mock_load @@ -69,9 +97,19 @@ class AugeasConfiguratorTest(util.ApacheTest): self.config.rollback_checkpoints() self.assertEqual(mock_load.call_count, 1) + def test_rollback_error(self): + self.config.reverter.rollback_checkpoints = mock.Mock( + side_effect=errors.ReverterError) + self.assertRaises(errors.PluginError, self.config.rollback_checkpoints) + def test_view_config_changes(self): self.config.view_config_changes() + def test_view_config_changes_error(self): + self.config.reverter.view_config_changes = mock.Mock( + side_effect=errors.ReverterError) + self.assertRaises(errors.PluginError, self.config.view_config_changes) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index 3a0732b64..3cb7270b4 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -171,6 +171,8 @@ class IAuthenticator(IPlugin): :rtype: :class:`list` of :class:`acme.challenges.ChallengeResponse` + :raises .PluginError: If challenges cannot be performed + """ def cleanup(achalls): @@ -180,6 +182,8 @@ class IAuthenticator(IPlugin): :class:`~letsencrypt.achallenges.AnnotatedChallenge` instances, a subset of those previously passed to :func:`perform`. + :raises PluginError: if original configuration cannot be restored + """ @@ -253,6 +257,8 @@ class IInstaller(IPlugin): :param str key_path: absolute path to the private key file :param str chain_path: absolute path to the certificate chain file + :raises .PluginError: when cert cannot be deployed + """ def enhance(domain, enhancement, options=None): @@ -266,6 +272,9 @@ class IInstaller(IPlugin): :const:`~letsencrypt.constants.ENHANCEMENTS` for expected options for each enhancement. + :raises .PluginError: If Enhancement is not supported, or if + an error occurs during the enhancement. + """ def supported_enhancements(): @@ -304,19 +313,37 @@ class IInstaller(IPlugin): :param bool temporary: Indicates whether the changes made will be quickly reversed in the future (challenges) + :raises .PluginError: when save is unsuccessful + """ def rollback_checkpoints(rollback=1): - """Revert `rollback` number of configuration checkpoints.""" + """Revert `rollback` number of configuration checkpoints. + + :raises .PluginError: when configuration cannot be fully reverted + + """ def view_config_changes(): - """Display all of the LE config changes.""" + """Display all of the LE config changes. + + :raises .PluginError: when config changes cannot be parsed + + """ def config_test(): - """Make sure the configuration is valid.""" + """Make sure the configuration is valid. + + :raises .MisconfigurationError: when the config is not in a usable state + + """ def restart(): - """Restart or refresh the server content.""" + """Restart or refresh the server content. + + :raises .PluginError: when server cannot be restarted + + """ class IDisplay(zope.interface.Interface): diff --git a/letsencrypt/reverter.py b/letsencrypt/reverter.py index bddf52b8d..87b3301d9 100644 --- a/letsencrypt/reverter.py +++ b/letsencrypt/reverter.py @@ -97,6 +97,8 @@ class Reverter(object): .. todo:: Decide on a policy for error handling, OSError IOError... + :raises .errors.ReverterError: If invalid directory structure. + """ backups = os.listdir(self.config.backup_dir) backups.sort(reverse=True)