From 8a5be3ee3ab01d578812872d3c48e0ce687cded1 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 2 May 2015 13:14:03 +0000 Subject: [PATCH] Remove determine_{authenticator,installer} --- letsencrypt/client/cli.py | 5 +- letsencrypt/client/client.py | 84 +++----------------- letsencrypt/client/display/ops.py | 38 ++------- letsencrypt/client/plugins/disco.py | 26 +++--- letsencrypt/client/tests/client_test.py | 74 ++--------------- letsencrypt/client/tests/display/ops_test.py | 44 ---------- 6 files changed, 42 insertions(+), 229 deletions(-) diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py index c4ede223f..b10868b3e 100644 --- a/letsencrypt/client/cli.py +++ b/letsencrypt/client/cli.py @@ -153,12 +153,13 @@ def revoke(args, config, plugins): # This depends on the renewal config and cannot be completed yet. zope.component.getUtility(interfaces.IDisplay).notification( "Revocation is not available with the new Boulder server yet.") - #client.revoke(config, args.no_confirm, args.rev_cert, args.rev_key) + #client.revoke(args.installer, config, plugins, args.no_confirm, + # args.rev_cert, args.rev_key) def rollback(args, config, plugins): """Rollback.""" - client.rollback(args.checkpoints, config) + client.rollback(args.installer, args.checkpoints, config, plugins) def config_changes(args, config, plugins): diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 9d1083e46..a98de272d 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -20,7 +20,10 @@ from letsencrypt.client import network2 from letsencrypt.client import reverter from letsencrypt.client import revoker +from letsencrypt.client.plugins import disco as plugins_disco + from letsencrypt.client.plugins.apache import configurator + from letsencrypt.client.display import ops as display_ops from letsencrypt.client.display import enhancements @@ -314,49 +317,6 @@ def validate_key_csr(privkey, csr=None): "The key and CSR do not match") -# This should be controlled by commandline parameters -def determine_authenticator(all_auths): - """Returns a valid IAuthenticator. - - :param list all_auths: Where each is a - :class:`letsencrypt.client.interfaces.IAuthenticator` object - - :returns: Valid Authenticator object or None - - :raises letsencrypt.client.errors.LetsEncryptClientError: If no - authenticator is available. - - """ - # Available Authenticator objects - avail_auths = [] - # Error messages for misconfigured authenticators - errs = {} - - for pot_auth in all_auths: - try: - pot_auth.prepare() - except errors.LetsEncryptMisconfigurationError as err: - errs[pot_auth] = err - except errors.LetsEncryptNoInstallationError: - continue - avail_auths.append(pot_auth) - - if len(avail_auths) > 1: - auth = display_ops.choose_authenticator(avail_auths, errs) - elif len(avail_auths) == 1: - auth = avail_auths[0] - else: - raise errors.LetsEncryptClientError("No Authenticators available.") - - if auth is not None and auth in errs: - logging.error("Please fix the configuration for the Authenticator. " - "The following error message was received: " - "%s", errs[auth]) - return - - return auth - - def determine_account(config): """Determine which account to use. @@ -379,29 +339,7 @@ def determine_account(config): return account.Account.from_prompts(config) -def determine_installer(config): - """Returns a valid installer if one exists. - - :param config: Configuration. - :type config: :class:`letsencrypt.client.interfaces.IConfig` - - :returns: IInstaller or `None` - :rtype: :class:`~letsencrypt.client.interfaces.IInstaller` or `None` - - """ - installer = configurator.ApacheConfigurator(config) - try: - installer.prepare() - return installer - except errors.LetsEncryptNoInstallationError: - logging.info("Unable to find a way to install the certificate.") - return - except errors.LetsEncryptMisconfigurationError: - # This will have to be changed in the future... - return installer - - -def rollback(checkpoints, config): +def rollback(default_installer, checkpoints, config, plugins): """Revert configuration the specified number of checkpoints. :param int checkpoints: Number of checkpoints to revert. @@ -411,7 +349,9 @@ def rollback(checkpoints, config): """ # Misconfigurations are only a slight problems... allow the user to rollback - installer = determine_installer(config) + installer = plugins_disco.pick_installer( + config, default_installer, plugins, question="Which installer " + "should be used for rollback?") # No Errors occurred during init... proceed normally # If installer is None... couldn't find an installer... there shouldn't be @@ -421,18 +361,16 @@ def rollback(checkpoints, config): installer.restart() -def revoke(config, no_confirm, cert, authkey): +def revoke(default_installer, config, plugins, no_confirm, cert, authkey): """Revoke certificates. :param config: Configuration. :type config: :class:`letsencrypt.client.interfaces.IConfig` """ - # Misconfigurations don't really matter. Determine installer better choose - # correctly though. - # This will need some better prepared or properly configured parameter... - # I will figure it out later... - installer = determine_installer(config) + installer = plugins_disco.pick_installer( + config, default_installer, plugins, question="Which installer " + "should be used for certificate revocation?") revoc = revoker.Revoker(installer, config, no_confirm) # Cert is most selective, so it is chosen first. diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py index 6582035bd..d59c1ceb1 100644 --- a/letsencrypt/client/display/ops.py +++ b/letsencrypt/client/display/ops.py @@ -12,6 +12,11 @@ util = zope.component.getUtility # pylint: disable=invalid-name def choose_plugin(prepared, question): + """Allow the user to choose ther plugin. + + :param list prepared: + + """ opts = [plugin_ep.name_with_description if error is None else "%s (Misconfigured)" % plugin_ep.name_with_description for (plugin_ep, error) in prepared] @@ -33,39 +38,6 @@ def choose_plugin(prepared, question): return None -def choose_authenticator(auths, errs): - """Allow the user to choose their authenticator. - - :param list auths: Where each of type - :class:`letsencrypt.client.interfaces.IAuthenticator` object - :param dict errs: Mapping IAuthenticator objects to error messages - - :returns: Authenticator selected - :rtype: :class:`letsencrypt.client.interfaces.IAuthenticator` or `None` - - """ - descs = [auth.description if auth not in errs - else "%s (Misconfigured)" % auth.description - for auth in auths] - - while True: - code, index = util(interfaces.IDisplay).menu( - "How would you like to authenticate with the Let's Encrypt CA?", - descs, help_label="More Info") - - if code == display_util.OK: - return auths[index] - elif code == display_util.HELP: - if auths[index] in errs: - msg = "Reported Error: %s" % errs[auths[index]] - else: - msg = auths[index].more_info() - util(interfaces.IDisplay).notification( - msg, height=display_util.HEIGHT) - else: - return - - def choose_account(accounts): """Choose an account. diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py index 37d60cbc8..b60c836d6 100644 --- a/letsencrypt/client/plugins/disco.py +++ b/letsencrypt/client/plugins/disco.py @@ -131,7 +131,7 @@ def prepare_plugins(initialized): return prepared # succefully prepared + misconfigured -def _pick_plugin(config, default, plugins, ifaces, question): +def _pick_plugin(config, default, plugins, question, ifaces): if default is not None: filtered = {default: plugins[default]} else: @@ -154,22 +154,26 @@ def _pick_plugin(config, default, plugins, ifaces, question): return None -def pick_authenticator(config, default, plugins): +def pick_authenticator( + config, default, plugins, question="How would you " + "like to authenticate with Let's Encrypt CA?"): """Pick authentication plugin.""" return _pick_plugin( - config, default, plugins, (interfaces.IAuthenticator,), - "How would you like to authenticate with Let's Encrypt CA?") + config, default, plugins, question, (interfaces.IAuthenticator,)) -def pick_installer(config, default, plugins): +def pick_installer(config, default, plugins, + question="How would you like to install certificates?"): """Pick installer plugin.""" - return _pick_plugin(config, default, plugins, (interfaces.IInstaller,), - "How would you like to install certificates?") + return _pick_plugin( + config, default, plugins, question, (interfaces.IInstaller,)) -def pick_configurator(config, default, plugins): +def pick_configurator( + config, default, plugins, + question="How would you like to authenticate and install " + "certificates?"): """Pick configurator plugin.""" return _pick_plugin( - config, default, plugins, - (interfaces.IAuthenticator, interfaces.IInstaller), - "How would you like to install certificates?") + config, default, plugins, question + (interfaces.IAuthenticator, interfaces.IInstaller)) diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py index b5ccf099b..b4595f257 100644 --- a/letsencrypt/client/tests/client_test.py +++ b/letsencrypt/client/tests/client_test.py @@ -54,63 +54,6 @@ class DetermineAccountTest(unittest.TestCase): self.assertTrue(chosen_acc.email, test_acc.email) -class DetermineAuthenticatorTest(unittest.TestCase): - def setUp(self): - from letsencrypt.client.plugins.apache.configurator import ( - ApacheConfigurator) - from letsencrypt.client.plugins.standalone.authenticator import ( - StandaloneAuthenticator) - - self.mock_stand = mock.MagicMock( - spec=StandaloneAuthenticator, description="Apache Web Server") - self.mock_apache = mock.MagicMock( - spec=ApacheConfigurator, description="Standalone Authenticator") - - self.mock_config = mock.Mock() - - self.all_auths = [self.mock_apache, self.mock_stand] - - @classmethod - def _call(cls, all_auths): - from letsencrypt.client.client import determine_authenticator - return determine_authenticator(all_auths) - - @mock.patch("letsencrypt.client.client.display_ops.choose_authenticator") - def test_accept_two(self, mock_choose): - mock_choose.return_value = self.mock_stand() - self.assertEqual(self._call(self.all_auths), self.mock_stand()) - - def test_accept_one(self): - self.mock_apache.prepare.return_value = self.mock_apache - self.assertEqual( - self._call(self.all_auths[:1]), self.mock_apache) - - def test_no_installation_one(self): - self.mock_apache.prepare.side_effect = ( - errors.LetsEncryptNoInstallationError) - - self.assertEqual(self._call(self.all_auths), self.mock_stand) - - def test_no_installations(self): - self.mock_apache.prepare.side_effect = ( - errors.LetsEncryptNoInstallationError) - self.mock_stand.prepare.side_effect = ( - errors.LetsEncryptNoInstallationError) - - self.assertRaises(errors.LetsEncryptClientError, - self._call, - self.all_auths) - - @mock.patch("letsencrypt.client.client.logging") - @mock.patch("letsencrypt.client.client.display_ops.choose_authenticator") - def test_misconfigured(self, mock_choose, unused_log): - self.mock_apache.prepare.side_effect = ( - errors.LetsEncryptMisconfigurationError) - mock_choose.return_value = self.mock_apache - - self.assertTrue(self._call(self.all_auths) is None) - - class RollbackTest(unittest.TestCase): """Test the rollback function.""" def setUp(self): @@ -121,23 +64,22 @@ class RollbackTest(unittest.TestCase): @classmethod def _call(cls, checkpoints): from letsencrypt.client.client import rollback - rollback(checkpoints, mock.MagicMock()) + rollback(None, checkpoints, {}, mock.MagicMock()) - @mock.patch("letsencrypt.client.client.determine_installer") - def test_no_problems(self, mock_det): - mock_det.side_effect = self.m_install + @mock.patch("letsencrypt.client.client.plugins_disco.pick_installer") + def test_no_problems(self, mock_pick_installer): + mock_pick_installer.side_effect = self.m_install self._call(1) self.assertEqual(self.m_install().rollback_checkpoints.call_count, 1) self.assertEqual(self.m_install().restart.call_count, 1) - @mock.patch("letsencrypt.client.client.determine_installer") - def test_no_installer(self, mock_det): - mock_det.return_value = None - - # Just make sure no exceptions are raised + @mock.patch("letsencrypt.client.client.plugins_disco.pick_installer") + def test_no_installer(self, mock_pick_installer): + mock_pick_installer.return_value = None self._call(1) + # Just make sure no exceptions are raised if __name__ == "__main__": diff --git a/letsencrypt/client/tests/display/ops_test.py b/letsencrypt/client/tests/display/ops_test.py index de5745e8e..2da411b6b 100644 --- a/letsencrypt/client/tests/display/ops_test.py +++ b/letsencrypt/client/tests/display/ops_test.py @@ -11,50 +11,6 @@ from letsencrypt.client import account from letsencrypt.client import le_util from letsencrypt.client.display import util as display_util -class ChooseAuthenticatorTest(unittest.TestCase): - """Test choose_authenticator function.""" - def setUp(self): - zope.component.provideUtility(display_util.FileDisplay(sys.stdout)) - self.mock_apache = mock.Mock() - self.mock_stand = mock.Mock() - self.mock_apache().more_info.return_value = "Apache Info" - self.mock_stand().more_info.return_value = "Standalone Info" - - self.auths = [self.mock_apache, self.mock_stand] - - self.errs = {self.mock_apache: "This is an error message."} - - @classmethod - def _call(cls, auths, errs): - from letsencrypt.client.display.ops import choose_authenticator - return choose_authenticator(auths, errs) - - @mock.patch("letsencrypt.client.display.ops.util") - def test_successful_choice(self, mock_util): - mock_util().menu.return_value = (display_util.OK, 0) - - ret = self._call(self.auths, {}) - - self.assertEqual(ret, self.mock_apache) - - @mock.patch("letsencrypt.client.display.ops.util") - def test_more_info(self, mock_util): - mock_util().menu.side_effect = [ - (display_util.HELP, 0), - (display_util.HELP, 1), - (display_util.OK, 1), - ] - - ret = self._call(self.auths, self.errs) - - self.assertEqual(mock_util().notification.call_count, 2) - self.assertEqual(ret, self.mock_stand) - - @mock.patch("letsencrypt.client.display.ops.util") - def test_no_choice(self, mock_util): - mock_util().menu.return_value = (display_util.CANCEL, 0) - self.assertTrue(self._call(self.auths, {}) is None) - class ChooseAccountTest(unittest.TestCase): """Test choose_account."""