Merge pull request #1001 from letsencrypt/cli

Implement --apache and --nginx; improve configurator errors

letsencrypt/cli.py

@@ -29,7 +29,6 @@
 from letsencrypt import constants
 from letsencrypt import client
 from letsencrypt import crypto_util
-from letsencrypt import errors
 from letsencrypt import interfaces
 from letsencrypt import le_util
 from letsencrypt import log
@@ -38,6 +37,7 @@
 
 from letsencrypt.display import util as display_util
 from letsencrypt.display import ops as display_ops
+from letsencrypt.errors import Error, PluginSelectionError, CertStorageError
 from letsencrypt.plugins import disco as plugins_disco
 
 
@@ -50,7 +50,7 @@
 # This is the stub to include in help generated by argparse
 
 SHORT_USAGE = """
-  letsencrypt [SUBCOMMAND] [options] [domains]
+  letsencrypt [SUBCOMMAND] [options] [-d domain] [-d domain] ...
 
 The Let's Encrypt agent can obtain and install HTTPS/TLS/SSL certificates.  By
 default, it will attempt to use a webserver both for obtaining and installing
@@ -92,7 +92,7 @@ def _find_domains(args, installer):
         domains = args.domains
 
     if not domains:
-        raise errors.Error("Please specify --domains, or --installer that "
+        raise Error("Please specify --domains, or --installer that "
                            "will help in domain names autodiscovery")
 
     return domains
@@ -145,9 +145,9 @@ def _tos_cb(regr):
             try:
                 acc, acme = client.register(
                     config, account_storage, tos_cb=_tos_cb)
-            except errors.Error as error:
+            except Error as error:
                 logger.debug(error, exc_info=True)
-                raise errors.Error(
+                raise Error(
                     "Unable to register an account with ACME server")
 
     args.account = acc.id
@@ -185,7 +185,7 @@ def _find_duplicative_certs(domains, config, renew_config):
             rc_config.filename = full_path
             candidate_lineage = storage.RenewableCert(
                 rc_config, config_opts=None, cli_config=cli_config)
-        except (configobj.ConfigObjError, errors.CertStorageError, IOError):
+        except (configobj.ConfigObjError, CertStorageError, IOError):
             logger.warning("Renewal configuration file %s is broken. "
                            "Skipping.", full_path)
             continue
@@ -257,7 +257,7 @@ def _treat_as_renewal(config, domains):
                     br=os.linesep
                 ),
                 reporter_util.HIGH_PRIORITY)
-            raise errors.Error(
+            raise Error(
                 "User did not use proper CLI and would like "
                 "to reinvoke the client.")
 
@@ -298,7 +298,7 @@ def _auth_from_domains(le_client, config, domains, plugins):
         # TREAT AS NEW REQUEST
         lineage = le_client.obtain_and_enroll_certificate(domains, plugins)
         if not lineage:
-            raise errors.Error("Certificate could not be obtained")
+            raise Error("Certificate could not be obtained")
 
     _report_new_cert(lineage.cert)
     reporter_util = zope.component.getUtility(interfaces.IReporter)
@@ -310,31 +310,109 @@ def _auth_from_domains(le_client, config, domains, plugins):
 
     return lineage
 
+def set_configurator(previously, now):
+    """
+    Setting configurators multiple ways is okay, as long as they all agree
+    :param string previously: previously identified request for the installer/authenticator
+    :param string requested: the request currently being processed
+    """
+    if now is None:
+        # we're not actually setting anything
+        return previously
+    if previously:
+        if previously != now:
+            msg = "Too many flags setting configurators/installers/authenticators %s -> %s"
+            raise PluginSelectionError, msg % (`previously`, `now`)
+    return now
+
+def diagnose_configurator_problem(cfg_type, requested, plugins):
+    """
+    Raise the most helpful error message about a plugin being unavailable
+
+    :param string cfg_type: either "installer" or "authenticator"
+    :param string requested: the plugin that was requested
+    :param PluginRegistry plugins: available plugins
+
+    :raises error.PluginSelectionError: if there was a problem
+    """
+
+    if requested:
+        if requested not in plugins:
+            msg = "The requested {0} plugin does not appear to be installed".format(requested)
+        else:
+            msg = ("The {0} plugin is not working; there may be problems with "
+                   "your existing configuration").format(requested)
+    elif cfg_type == "installer":
+        if os.path.exists("/etc/debian_version"):
+            # Debian... installers are at least possible
+            msg = ('No installers seem to be present and working on your system; '
+                   'fix that or try running letsencrypt with the "auth" command')
+        else:
+            # XXX update this logic as we make progress on #788 and nginx support
+            msg = ('No installers are available on your OS yet; try running '
+                   '"letsencrypt-auto auth" to get a cert you can install manually')
+    else:
+        msg = "{0} could not be determined or is not installed".format(cfg_type)
+    raise PluginSelectionError, msg
+
+
+def choose_configurator_plugins(args, config, plugins, verb):
+    """
+    Figure out which configurator we're going to use
+
+    :raises error.PluginSelectionError if there was a problem
+    """
+
+    # Which plugins do we need?
+    need_inst = need_auth = (verb == "run")
+    if verb == "auth":
+        need_auth = True
+    if verb == "install":
+        need_inst = True
+        if args.authenticator:
+            logger.warn("Specifying an authenticator doesn't make sense in install mode")
+
+    # Which plugins did the user request?
+    req_inst = req_auth = args.configurator
+    req_inst = set_configurator(req_inst, args.installer)
+    req_auth = set_configurator(req_auth, args.authenticator)
+    if args.nginx:
+        req_inst = set_configurator(req_inst, "nginx")
+        req_auth = set_configurator(req_auth, "nginx")
+    if args.apache:
+        req_inst = set_configurator(req_inst, "apache")
+        req_auth = set_configurator(req_auth, "apache")
+    logger.debug("Requested authenticator %s and installer %s", req_auth, req_inst)
+
+    # Try to meet the user's request and/or ask them to pick plugins
+    authenticator = installer = None
+    if verb == "run" and req_auth == req_inst:
+        # Unless the user has explicitly asked for different auth/install,
+        # only consider offering a single choice
+        authenticator = installer = display_ops.pick_configurator(config, req_inst, plugins)
+    else:
+        if need_inst or req_inst:
+            installer = display_ops.pick_installer(config, req_inst, plugins)
+        if need_auth:
+            authenticator = display_ops.pick_authenticator(config, req_auth, plugins)
+    logger.debug("Selected authenticator %s and installer %s", authenticator, installer)
+
+    if need_inst and not installer:
+        diagnose_configurator_problem("installer", req_inst, plugins)
+    if need_auth and not authenticator:
+        diagnose_configurator_problem("authenticator", req_auth, plugins)
+
+    return installer, authenticator
+
 
 # TODO: Make run as close to auth + install as possible
 # Possible difficulties: args.csr was hacked into auth
 def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-locals
     """Obtain a certificate and install."""
-    # Begin authenticator and installer setup
-    if args.configurator is not None and (args.installer is not None or
-                                          args.authenticator is not None):
-        return ("Either --configurator or --authenticator/--installer"
-                "pair, but not both, is allowed")
-
-    if args.authenticator is not None or args.installer is not None:
-        installer = display_ops.pick_installer(
-            config, args.installer, plugins)
-        authenticator = display_ops.pick_authenticator(
-            config, args.authenticator, plugins)
-    else:
-        # TODO: this assumes that user doesn't want to pick authenticator
-        #       and installer separately...
-        authenticator = installer = display_ops.pick_configurator(
-            config, args.configurator, plugins)
-
-    if installer is None or authenticator is None:
-        return "Configurator could not be determined"
-    # End authenticator and installer setup
+    try:
+        installer, authenticator = choose_configurator_plugins(args, config, plugins, "run")
+    except PluginSelectionError, e:
+        return e.message
 
     domains = _find_domains(args, installer)
 
@@ -362,15 +440,11 @@ def auth(args, config, plugins):
         # supplied, check if CSR matches given domains?
         return "--domains and --csr are mutually exclusive"
 
-    authenticator = display_ops.pick_authenticator(
-        config, args.authenticator, plugins)
-    if authenticator is None:
-        return "Authenticator could not be determined"
-
-    if args.installer is not None:
-        installer = display_ops.pick_installer(config, args.installer, plugins)
-    else:
-        installer = None
+    try:
+        # installers are used in auth mode to determine domain names
+        installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
+    except PluginSelectionError, e:
+        return e.message
 
     # TODO: Handle errors from _init_le_client?
     le_client = _init_le_client(args, config, authenticator, installer)
@@ -390,9 +464,12 @@ def auth(args, config, plugins):
 def install(args, config, plugins):
     """Install a previously obtained cert in a server."""
     # XXX: Update for renewer/RenewableCert
-    installer = display_ops.pick_installer(config, args.installer, plugins)
-    if installer is None:
-        return "Installer could not be determined"
+
+    try:
+        installer, _ = choose_configurator_plugins(args, config, plugins, "auth")
+    except PluginSelectionError, e:
+        return e.message
+
     domains = _find_domains(args, installer)
     le_client = _init_le_client(
         args, config, authenticator=None, installer=installer)
@@ -663,6 +740,10 @@ def create_parser(plugins, args):
         None, "-t", "--text", dest="text_mode", action="store_true",
         help="Use the text output instead of the curses UI.")
     helpful.add(None, "-m", "--email", help=config_help("email"))
+    helpful.add(None, "--apache", action="store_true",
+                help="Obtain and install certs using Apache")
+    helpful.add(None, "--nginx", action="store_true",
+                help="Obtain and install certs using Nginx")
     # positional arg shadows --domains, instead of appending, and
     # --domains is useful, because it can be stored in config
     #for subparser in parser_run, parser_auth, parser_install:
@@ -835,7 +916,7 @@ def _plugins_parsing(helpful, plugins):
     helpful.add(
         "plugins", "-a", "--authenticator", help="Authenticator plugin name.")
     helpful.add(
-        "plugins", "-i", "--installer", help="Installer plugin name.")
+        "plugins", "-i", "--installer", help="Installer plugin name (also used to find domains).")
     helpful.add(
         "plugins", "--configurator", help="Name of the plugin that is "
         "both an authenticator and an installer. Should not be used "
@@ -922,7 +1003,7 @@ def _handle_exception(exc_type, exc_value, trace, args):
                 sys.exit("".join(
                     traceback.format_exception(exc_type, exc_value, trace)))
 
-        if issubclass(exc_type, errors.Error):
+        if issubclass(exc_type, Error):
             sys.exit(exc_value)
         else:
             # Tell the user a bit about what happened, without overwhelming
@@ -986,7 +1067,7 @@ def main(cli_args=sys.argv[1:]):
         eula = pkg_resources.resource_string("letsencrypt", "EULA")
         if not zope.component.getUtility(interfaces.IDisplay).yesno(
                 eula, "Agree", "Cancel"):
-            raise errors.Error("Must agree to TOS")
+            raise Error("Must agree to TOS")
 
     if not os.geteuid() == 0:
         logger.warning(
@@ -1003,4 +1084,7 @@ def main(cli_args=sys.argv[1:]):
 
 
 if __name__ == "__main__":
-    sys.exit(main())  # pragma: no cover
+    err_string = main()
+    if err_string:
+        logger.warn("Exiting with message %s", err_string)
+    sys.exit(err_string)  # pragma: no cover

letsencrypt/errors.py

@@ -24,7 +24,6 @@ class SubprocessError(Error):
 class CertStorageError(Error):
     """Generic `.CertStorage` error."""
 
-
 # Auth Handler Errors
 class AuthorizationError(Error):
     """Authorization error."""
@@ -65,6 +64,8 @@ class DvsniError(DvAuthError):
 class PluginError(Error):
     """Let's Encrypt Plugin error."""
 
+class PluginSelectionError(Error):
+    """A problem with plugin/configurator selection or setup"""
 
 class NoInstallationError(PluginError):
     """Let's Encrypt No Installation error."""

letsencrypt/tests/cli_test.py

@@ -13,6 +13,8 @@
 from letsencrypt import configuration
 from letsencrypt import errors
 
+from letsencrypt.plugins import disco
+
 from letsencrypt.tests import renewer_test
 from letsencrypt.tests import test_util
 
@@ -75,7 +77,6 @@ def test_help(self):
             output.truncate(0)
             self.assertRaises(SystemExit, self._call_stdout, ['-h', 'nginx'])
             out = output.getvalue()
-            from letsencrypt.plugins import disco
             if "nginx" in disco.PluginsRegistry.find_all():
                 # may be false while building distributions without plugins
                 self.assertTrue("--nginx-ctl" in out)
@@ -93,6 +94,27 @@ def test_help(self):
             from letsencrypt import cli
             self.assertTrue(cli.USAGE in out)
 
+    def test_configurator_selection(self):
+        real_plugins = disco.PluginsRegistry.find_all()
+        args = ['--agree-eula', '--apache', '--authenticator', 'standalone']
+
+        # This needed two calls to find_all(), which we're avoiding for now
+        # because of possible side effects:
+        # https://github.com/letsencrypt/letsencrypt/commit/51ed2b681f87b1eb29088dd48718a54f401e4855
+        #with mock.patch('letsencrypt.cli.plugins_testable') as plugins:
+        #    plugins.return_value = {"apache": True, "nginx": True}
+        #    ret, _, _, _ = self._call(args)
+        #    self.assertTrue("Too many flags setting" in ret)
+
+        args = ["install", "--nginx", "--cert-path", "/tmp/blah", "--key-path", "/tmp/blah",
+                "--nginx-server-root", "/nonexistent/thing", "-d",
+                "example.com", "--debug"]
+        if "nginx" in real_plugins:
+            # Sending nginx a non-existent conf dir will simulate misconfiguration
+            # (we can only do that if letsencrypt-nginx is actually present)
+            ret, _, _, _ = self._call(args)
+            self.assertTrue("The nginx plugin is not working" in ret)
+
     def test_rollback(self):
         _, _, _, client = self._call(['rollback'])
         self.assertEqual(1, client.rollback.call_count)
@@ -117,7 +139,7 @@ def test_auth_bad_args(self):
         self.assertEqual(ret, '--domains and --csr are mutually exclusive')
 
         ret, _, _, _ = self._call(['-a', 'bad_auth', 'auth'])
-        self.assertEqual(ret, 'Authenticator could not be determined')
+        self.assertEqual(ret, 'The requested bad_auth plugin does not appear to be installed')
 
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     def test_auth_new_request_success(self, mock_get_utility):