From 78be30d4579f22ac9896de5ae0b2b1d76d8c1c6c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 13 Oct 2015 14:38:31 -0700
Subject: [PATCH 01/29] Basic support for --apache and --nginx

 - Also begin to clean up the code that integrates --configurator,
   --installer, and --authenticator inputs.
---
 letsencrypt/cli.py | 65 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 20 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 64cba508d..5c74e1ea9 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -304,31 +304,54 @@ def _auth_from_domains(le_client, config, domains, plugins):
 
     return lineage
 
+class ConfiguratorError(TypeError):
+    pass
+
+def set_configurator(previously, now):
+    """Setting configurators multiple ways is okay, as long as they all agree"""
+    if previously:
+        if previously != now:
+            raise ConfiguratorError, "Too many flags setting configurators/installers/authenticators %s -> %s" % (`previously`,`now`)
+    return now
+
+def pick_configurator(args, config, plugins, verb):
+    """Figure out which configurator we're going to use"""
+    installer = authenticator = args.configurator
+    print "args.configurator", args.configurator
+    installer = set_configurator(installer, args.installer)
+    authenticator = set_configurator(authenticator, args.authenticator)
+    print "installer", installer
+    if args.nginx:
+        installer = set_configurator(installer, nginx)
+        authenticator = set_configurator(authenticator, nginx)
+    if args.apache:
+        installer = set_configurator(installer, apache)
+        authenticator = set_configurator(authenticator, apache)
+
+    if authenticator == installer:
+        # TODO: this assumes that user doesn't want to pick authenticator
+        #       and installer separately...
+        authenticator = installer = display_ops.pick_configurator(config, args.installer, plugins)
+        #print "ainstaller", installer
+    else:
+        installer = display_ops.pick_installer(config, installer, plugins)
+        authenticator = display_ops.pick_authenticator(config, authenticator, plugins)
+        #print "binstaller", installer
+
+    if installer is None or authenticator is None:
+        #print installer, authenticator
+        raise ConfiguratorError, "Configurator could not be determined"
+    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 = pick_configurator(args, config, plugins, "run")
+    except ConfiguratorError, e:
+        return e.message
 
     domains = _find_domains(args, installer)
 
@@ -656,6 +679,8 @@ 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", help="Obtain and install certs using Apache")
+    helpful.add(None, "--nginx", 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:

From 9bde3b7084635588c73c2c8d7451ffd12a2932bc Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 16 Oct 2015 10:34:29 -0700
Subject: [PATCH 02/29] Support for --apache and --nginx now working.

 - Along with much better error messages for misconfigured plugins
---
 letsencrypt/cli.py | 83 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 59 insertions(+), 24 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 5c74e1ea9..af0f58378 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -314,33 +314,66 @@ def set_configurator(previously, now):
             raise ConfiguratorError, "Too many flags setting configurators/installers/authenticators %s -> %s" % (`previously`,`now`)
     return now
 
-def pick_configurator(args, config, plugins, verb):
+def diagnose_configurator_problem(cfg_type, requested):
+    """
+    Raise the most helpful error message about a plugin being unavailable
+    @cfg_type -- "installer" or "authenticator"
+    @requested -- which plugin the user wanted
+    """
+    plugins = plugins_disco.PluginsRegistry.find_all()
+
+    if requested:
+        if requested not in plugins:
+            msg = "The requested " + requested + " plugin does not appear to be installed"
+            raise ConfiguratorError, msg
+        else:
+            msg = "The " + requested + " plugin is not working; there may be problems "
+            msg += "with your existing configuration"
+            raise ConfiguratorError, msg
+    raise ConfiguratorError, "Installer could not be determined or is not installed"
+
+
+
+def choose_configurator_plugins(args, config, plugins, verb):
     """Figure out which configurator we're going to use"""
-    installer = authenticator = args.configurator
-    print "args.configurator", args.configurator
-    installer = set_configurator(installer, args.installer)
-    authenticator = set_configurator(authenticator, args.authenticator)
-    print "installer", installer
+
+    # Which plugins do we need?
+    need_inst = need_auth = (verb == "run")
+    if verb == "auth":
+        need_auth = True
+    if verb == "install":
+        need_install = True
+
+    # 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:
-        installer = set_configurator(installer, nginx)
-        authenticator = set_configurator(authenticator, nginx)
+        req_inst = set_configurator(req_inst, "nginx")
+        req_auth = set_configurator(req_auth, "nginx")
     if args.apache:
-        installer = set_configurator(installer, apache)
-        authenticator = set_configurator(authenticator, apache)
+        req_inst = set_configurator(req_inst, "apache")
+        req_auth = set_configurator(req_auth, "apache")
 
-    if authenticator == installer:
-        # TODO: this assumes that user doesn't want to pick authenticator
-        #       and installer separately...
-        authenticator = installer = display_ops.pick_configurator(config, args.installer, plugins)
-        #print "ainstaller", installer
+    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
+    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:
-        installer = display_ops.pick_installer(config, installer, plugins)
-        authenticator = display_ops.pick_authenticator(config, authenticator, plugins)
-        #print "binstaller", installer
+        if need_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)
+    if need_auth and not authenticator:
+        diagnose_configurator_problem("authenticator", req_auth)
 
-    if installer is None or authenticator is None:
-        #print installer, authenticator
-        raise ConfiguratorError, "Configurator could not be determined"
     return installer, authenticator
 
 
@@ -349,7 +382,7 @@ def pick_configurator(args, config, plugins, verb):
 def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-locals
     """Obtain a certificate and install."""
     try:
-        installer, authenticator = pick_configurator(args, config, plugins, "run")
+        installer, authenticator = choose_configurator_plugins(args, config, plugins, "run")
     except ConfiguratorError, e:
         return e.message
 
@@ -679,8 +712,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", help="Obtain and install certs using Apache")
-    helpful.add(None, "--nginx", help="Obtain and install certs using Nginx")
+    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:

From 3d6ecc114b5cf11c29ed96a23655153f864079d4 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 16 Oct 2015 10:41:05 -0700
Subject: [PATCH 03/29] lintmonster

---
 letsencrypt/cli.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index af0f58378..f8675b3b4 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -304,14 +304,15 @@ def _auth_from_domains(le_client, config, domains, plugins):
 
     return lineage
 
-class ConfiguratorError(TypeError):
+class ConfiguratorError(TypeError): # pylint: disable=missing-docstring
     pass
 
 def set_configurator(previously, now):
     """Setting configurators multiple ways is okay, as long as they all agree"""
     if previously:
         if previously != now:
-            raise ConfiguratorError, "Too many flags setting configurators/installers/authenticators %s -> %s" % (`previously`,`now`)
+            msg = "Too many flags setting configurators/installers/authenticators %s -> %s"
+            raise ConfiguratorError, msg % (`previously`, `now`)
     return now
 
 def diagnose_configurator_problem(cfg_type, requested):
@@ -330,7 +331,7 @@ def diagnose_configurator_problem(cfg_type, requested):
             msg = "The " + requested + " plugin is not working; there may be problems "
             msg += "with your existing configuration"
             raise ConfiguratorError, msg
-    raise ConfiguratorError, "Installer could not be determined or is not installed"
+    raise ConfiguratorError, cfg_type + " could not be determined or is not installed"
 
 
 
@@ -342,7 +343,7 @@ def choose_configurator_plugins(args, config, plugins, verb):
     if verb == "auth":
         need_auth = True
     if verb == "install":
-        need_install = True
+        need_inst = True
 
     # Which plugins did the user request?
     req_inst = req_auth = args.configurator
@@ -355,7 +356,7 @@ def choose_configurator_plugins(args, config, plugins, verb):
         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))
+    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
     if verb == "run" and req_auth == req_inst:
@@ -367,7 +368,7 @@ def choose_configurator_plugins(args, config, plugins, verb):
             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))
+    logger.debug("Selected authenticator %s and installer %s", authenticator, installer)
 
     if need_inst and not installer:
         diagnose_configurator_problem("installer", req_inst)

From dcf8ea4e97e63fafe0e9ddf75ca6e72a69a0b040 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 16 Oct 2015 17:38:17 -0700
Subject: [PATCH 04/29] Logic for setting configurators assumed they were
 really being set

(They might not be)
---
 letsencrypt/cli.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index f8675b3b4..77480f69f 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -309,6 +309,9 @@ class ConfiguratorError(TypeError): # pylint: disable=missing-docstring
 
 def set_configurator(previously, now):
     """Setting configurators multiple ways is okay, as long as they all agree"""
+    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"

From 36432e35f728d28d4f772f3f139558325bf80545 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 16 Oct 2015 18:19:22 -0700
Subject: [PATCH 05/29] keep cover happy

---
 letsencrypt/tests/cli_test.py | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index f690e77f9..42faad245 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -15,6 +15,7 @@ from letsencrypt import errors
 
 from letsencrypt.tests import renewer_test
 from letsencrypt.tests import test_util
+from letsencrypt.plugins import disco
 
 
 CSR = test_util.vector_path('csr.der')
@@ -75,7 +76,6 @@ class CLITest(unittest.TestCase):
             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 +93,20 @@ class CLITest(unittest.TestCase):
             from letsencrypt import cli
             self.assertTrue(cli.USAGE in out)
 
+    def test_configurator_selection(self):
+        output = StringIO.StringIO()
+        plugins = disco.PluginsRegistry.find_all()
+        if "apache" in plugins:
+ 
+            with mock.patch('letsencrypt.cli.sys.stdout', new=output):
+                self.assertRaises(SystemExit, self._call_stdout,
+                                  ['--agree-eula', '--apache', '--authenticator', 'standalone'])
+                #import sys
+                #sys.stderr.write(repr(self._call_stdout(['--agree-eula', '--apache', '--authenticator', 'standalone'])))
+                out = output.getvalue()
+                self.assertTrue("Too many flags setting" in out)
+                # TODO add tests with a broken plugin, a missing plugin, etc
+
     def test_rollback(self):
         _, _, _, client = self._call(['rollback'])
         self.assertEqual(1, client.rollback.call_count)

From 57e15c52d7895431740b4b868c614f62ef519e16 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 16 Oct 2015 18:28:38 -0700
Subject: [PATCH 06/29] Test actually works now :)

---
 letsencrypt/tests/cli_test.py | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 42faad245..830a73956 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -97,15 +97,11 @@ class CLITest(unittest.TestCase):
         output = StringIO.StringIO()
         plugins = disco.PluginsRegistry.find_all()
         if "apache" in plugins:
- 
-            with mock.patch('letsencrypt.cli.sys.stdout', new=output):
-                self.assertRaises(SystemExit, self._call_stdout,
-                                  ['--agree-eula', '--apache', '--authenticator', 'standalone'])
-                #import sys
-                #sys.stderr.write(repr(self._call_stdout(['--agree-eula', '--apache', '--authenticator', 'standalone'])))
-                out = output.getvalue()
-                self.assertTrue("Too many flags setting" in out)
-                # TODO add tests with a broken plugin, a missing plugin, etc
+            from letsencrypt import cli 
+            args = ['--agree-eula', '--apache', '--authenticator', 'standalone']
+            ret, _, _, _ = self._call(args)
+            self.assertTrue("Too many flags setting" in ret)
+            # TODO add tests with a broken plugin, a missing plugin, etc
 
     def test_rollback(self):
         _, _, _, client = self._call(['rollback'])

From f0faf91b822cac464f2e963d8160f6ebb36de032 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 16 Oct 2015 18:34:57 -0700
Subject: [PATCH 07/29] lint!

---
 letsencrypt/tests/cli_test.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 830a73956..4ab912ad4 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -94,10 +94,8 @@ class CLITest(unittest.TestCase):
             self.assertTrue(cli.USAGE in out)
 
     def test_configurator_selection(self):
-        output = StringIO.StringIO()
         plugins = disco.PluginsRegistry.find_all()
         if "apache" in plugins:
-            from letsencrypt import cli 
             args = ['--agree-eula', '--apache', '--authenticator', 'standalone']
             ret, _, _, _ = self._call(args)
             self.assertTrue("Too many flags setting" in ret)

From 8d4f414e09c0c1adb89c1b802932ad67775f423a Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 17 Oct 2015 12:47:05 -0700
Subject: [PATCH 08/29] Move ConfiguratorError to errors.py

---
 letsencrypt/cli.py    | 13 +++++--------
 letsencrypt/errors.py |  2 ++
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 9a0dd9108..a1fe27e08 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -304,9 +304,6 @@ def _auth_from_domains(le_client, config, domains, plugins):
 
     return lineage
 
-class ConfiguratorError(TypeError): # pylint: disable=missing-docstring
-    pass
-
 def set_configurator(previously, now):
     """Setting configurators multiple ways is okay, as long as they all agree"""
     if now is None:
@@ -315,7 +312,7 @@ def set_configurator(previously, now):
     if previously:
         if previously != now:
             msg = "Too many flags setting configurators/installers/authenticators %s -> %s"
-            raise ConfiguratorError, msg % (`previously`, `now`)
+            raise errors.ConfiguratorError, msg % (`previously`, `now`)
     return now
 
 def diagnose_configurator_problem(cfg_type, requested):
@@ -329,12 +326,12 @@ def diagnose_configurator_problem(cfg_type, requested):
     if requested:
         if requested not in plugins:
             msg = "The requested " + requested + " plugin does not appear to be installed"
-            raise ConfiguratorError, msg
+            raise errors.ConfiguratorError, msg
         else:
             msg = "The " + requested + " plugin is not working; there may be problems "
             msg += "with your existing configuration"
-            raise ConfiguratorError, msg
-    raise ConfiguratorError, cfg_type + " could not be determined or is not installed"
+            raise errors.ConfiguratorError, msg
+    raise errors.ConfiguratorError, cfg_type + " could not be determined or is not installed"
 
 
 
@@ -387,7 +384,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     """Obtain a certificate and install."""
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "run")
-    except ConfiguratorError, e:
+    except errors.ConfiguratorError, e:
         return e.message
 
     domains = _find_domains(args, installer)
diff --git a/letsencrypt/errors.py b/letsencrypt/errors.py
index ba0601d29..def8fc20d 100644
--- a/letsencrypt/errors.py
+++ b/letsencrypt/errors.py
@@ -24,6 +24,8 @@ class SubprocessError(Error):
 class CertStorageError(Error):
     """Generic `.CertStorage` error."""
 
+class ConfiguratorError(Error):
+    """A problem with plugin/configurator selection or setup"""
 
 # Auth Handler Errors
 class AuthorizationError(Error):

From f7bfb5ba8d6e3cf47e2ce5c11fd49aedfecbd912 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 17 Oct 2015 12:54:56 -0700
Subject: [PATCH 09/29] Sphinxify docstrings, plugins plumbing

---
 letsencrypt/cli.py | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index a1fe27e08..4f619b03a 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -315,13 +315,16 @@ def set_configurator(previously, now):
             raise errors.ConfiguratorError, msg % (`previously`, `now`)
     return now
 
-def diagnose_configurator_problem(cfg_type, requested):
+def diagnose_configurator_problem(cfg_type, requested, plugins):
     """
     Raise the most helpful error message about a plugin being unavailable
-    @cfg_type -- "installer" or "authenticator"
-    @requested -- which plugin the user wanted
+
+    :param string cfg_type: either "installer" or "authenticator"
+    :param string requested: the plugin that was requested
+    :param PluginRegistry plugins: available plugins
+
+    :raises error.ConfiguratorError if there was a problem
     """
-    plugins = plugins_disco.PluginsRegistry.find_all()
 
     if requested:
         if requested not in plugins:
@@ -336,7 +339,11 @@ def diagnose_configurator_problem(cfg_type, requested):
 
 
 def choose_configurator_plugins(args, config, plugins, verb):
-    """Figure out which configurator we're going to use"""
+    """
+    Figure out which configurator we're going to use
+
+    :raises error.ConfiguratorError if there was a problem
+    """
 
     # Which plugins do we need?
     need_inst = need_auth = (verb == "run")
@@ -371,9 +378,9 @@ def choose_configurator_plugins(args, config, plugins, verb):
     logger.debug("Selected authenticator %s and installer %s", authenticator, installer)
 
     if need_inst and not installer:
-        diagnose_configurator_problem("installer", req_inst)
+        diagnose_configurator_problem("installer", req_inst, plugins)
     if need_auth and not authenticator:
-        diagnose_configurator_problem("authenticator", req_auth)
+        diagnose_configurator_problem("authenticator", req_auth, plugins)
 
     return installer, authenticator
 

From a841c4fc5d2b1a8b1b308e556de1ecc12400ebe3 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 17 Oct 2015 13:13:00 -0700
Subject: [PATCH 10/29] More nits

---
 letsencrypt/cli.py            | 7 +++----
 letsencrypt/tests/cli_test.py | 3 ++-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 4f619b03a..289b14554 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -328,11 +328,11 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
 
     if requested:
         if requested not in plugins:
-            msg = "The requested " + requested + " plugin does not appear to be installed"
+            msg = "The requested {0} plugin does not appear to be installed".format(requested)
             raise errors.ConfiguratorError, msg
         else:
-            msg = "The " + requested + " plugin is not working; there may be problems "
-            msg += "with your existing configuration"
+            msg = ("The {0} plugin is not working; there may be problems with "
+                   "your existing configuration").format(requested)
             raise errors.ConfiguratorError, msg
     raise errors.ConfiguratorError, cfg_type + " could not be determined or is not installed"
 
@@ -362,7 +362,6 @@ def choose_configurator_plugins(args, config, plugins, verb):
     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
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 4ab912ad4..d31bfaff4 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -13,9 +13,10 @@ from letsencrypt import account
 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
-from letsencrypt.plugins import disco
 
 
 CSR = test_util.vector_path('csr.der')

From 7c92db095f9f828fb4085c0d18236cc808461ed1 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 17 Oct 2015 13:43:49 -0700
Subject: [PATCH 11/29] Use new selection logic for "auth"

Also: why was auth selecting installers?
---
 letsencrypt/cli.py | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 289b14554..6e65d4c0d 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -419,15 +419,14 @@ 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"
+    try:
+        installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
+    except errors.ConfiguratorError, e:
+        return e.message
 
-    if args.installer is not None:
-        installer = display_ops.pick_installer(config, args.installer, plugins)
-    else:
-        installer = None
+    installer = None # we're doing auth!
+    if args.installer:
+        return "Specifying an installer doesn't make sense in auth mode!"
 
     # TODO: Handle errors from _init_le_client?
     le_client = _init_le_client(args, config, authenticator, installer)

From f4f6b98f31f43094778018ecebf9b3db3a61ba13 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 17 Oct 2015 13:57:52 -0700
Subject: [PATCH 12/29] Use new plugin selection logic for install

---
 letsencrypt/cli.py | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 6e65d4c0d..6287611a5 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -334,8 +334,8 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
             msg = ("The {0} plugin is not working; there may be problems with "
                    "your existing configuration").format(requested)
             raise errors.ConfiguratorError, msg
-    raise errors.ConfiguratorError, cfg_type + " could not be determined or is not installed"
-
+    raise errors.ConfiguratorError,
+          "{0} could not be determined or is not installed".format(cfg_type)
 
 
 def choose_configurator_plugins(args, config, plugins, verb):
@@ -349,8 +349,14 @@ def choose_configurator_plugins(args, config, plugins, verb):
     need_inst = need_auth = (verb == "run")
     if verb == "auth":
         need_auth = True
+        if args.installer:
+            msg = "Specifying an installer doesn't make sense in auth mode"
+            raise errors.ConfiguratorError, msg
     if verb == "install":
         need_inst = True
+        if args.authenticator:
+            msg = "Specifying an authenticator doesn't make sense in install mode"
+            raise errors.ConfiguratorError, msg
 
     # Which plugins did the user request?
     req_inst = req_auth = args.configurator
@@ -421,13 +427,10 @@ def auth(args, config, plugins):
 
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
+        installer = None # we're doing auth!
     except errors.ConfiguratorError, e:
         return e.message
 
-    installer = None # we're doing auth!
-    if args.installer:
-        return "Specifying an installer doesn't make sense in auth mode!"
-
     # TODO: Handle errors from _init_le_client?
     le_client = _init_le_client(args, config, authenticator, installer)
 
@@ -446,9 +449,16 @@ 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, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
+        authenticator = None # we're doing install!
+    except errors.ConfiguratorError, e:
+        return e.message
+
+    if args.authenticator:
+        return "Specifying an authenticator doesn't make sense in install mode!"
+
     domains = _find_domains(args, installer)
     le_client = _init_le_client(
         args, config, authenticator=None, installer=installer)

From c89e192ee70cc709f5d45829d86d5257052d3cbd Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 17 Oct 2015 14:01:54 -0700
Subject: [PATCH 13/29] Concision is beautiful

---
 letsencrypt/cli.py | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 6287611a5..aff70cca3 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -29,7 +29,6 @@ from letsencrypt import configuration
 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 import storage
 
 from letsencrypt.display import util as display_util
 from letsencrypt.display import ops as display_ops
+from letsencrypt.errors import Error, ConfiguratorError, CertStorageError
 from letsencrypt.plugins import disco as plugins_disco
 
 
@@ -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 _determine_account(args, config):
             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)
 
@@ -312,7 +312,7 @@ def set_configurator(previously, now):
     if previously:
         if previously != now:
             msg = "Too many flags setting configurators/installers/authenticators %s -> %s"
-            raise errors.ConfiguratorError, msg % (`previously`, `now`)
+            raise ConfiguratorError, msg % (`previously`, `now`)
     return now
 
 def diagnose_configurator_problem(cfg_type, requested, plugins):
@@ -329,13 +329,12 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
     if requested:
         if requested not in plugins:
             msg = "The requested {0} plugin does not appear to be installed".format(requested)
-            raise errors.ConfiguratorError, msg
+            raise ConfiguratorError, msg
         else:
             msg = ("The {0} plugin is not working; there may be problems with "
                    "your existing configuration").format(requested)
-            raise errors.ConfiguratorError, msg
-    raise errors.ConfiguratorError,
-          "{0} could not be determined or is not installed".format(cfg_type)
+            raise ConfiguratorError, msg
+    raise ConfiguratorError, "{0} could not be determined or is not installed".format(cfg_type)
 
 
 def choose_configurator_plugins(args, config, plugins, verb):
@@ -350,13 +349,12 @@ def choose_configurator_plugins(args, config, plugins, verb):
     if verb == "auth":
         need_auth = True
         if args.installer:
-            msg = "Specifying an installer doesn't make sense in auth mode"
-            raise errors.ConfiguratorError, msg
+            raise ConfiguratorError, "Specifying an installer doesn't make sense in auth mode"
     if verb == "install":
         need_inst = True
         if args.authenticator:
             msg = "Specifying an authenticator doesn't make sense in install mode"
-            raise errors.ConfiguratorError, msg
+            raise ConfiguratorError, msg
 
     # Which plugins did the user request?
     req_inst = req_auth = args.configurator
@@ -396,7 +394,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     """Obtain a certificate and install."""
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "run")
-    except errors.ConfiguratorError, e:
+    except ConfiguratorError, e:
         return e.message
 
     domains = _find_domains(args, installer)
@@ -428,7 +426,7 @@ def auth(args, config, plugins):
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
         installer = None # we're doing auth!
-    except errors.ConfiguratorError, e:
+    except ConfiguratorError, e:
         return e.message
 
     # TODO: Handle errors from _init_le_client?
@@ -453,7 +451,7 @@ def install(args, config, plugins):
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
         authenticator = None # we're doing install!
-    except errors.ConfiguratorError, e:
+    except ConfiguratorError, e:
         return e.message
 
     if args.authenticator:
@@ -992,7 +990,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
@@ -1056,7 +1054,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(

From 4996e9b678478fd26198febaa9b536d5f694c261 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 17 Oct 2015 14:28:38 -0700
Subject: [PATCH 14/29] Installers are actually useful in auth mode

---
 letsencrypt/cli.py | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index aff70cca3..cdad7d99c 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -348,8 +348,6 @@ def choose_configurator_plugins(args, config, plugins, verb):
     need_inst = need_auth = (verb == "run")
     if verb == "auth":
         need_auth = True
-        if args.installer:
-            raise ConfiguratorError, "Specifying an installer doesn't make sense in auth mode"
     if verb == "install":
         need_inst = True
         if args.authenticator:
@@ -424,8 +422,8 @@ def auth(args, config, plugins):
         return "--domains and --csr are mutually exclusive"
 
     try:
+        # installers are used in auth mode to determine domain names
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
-        installer = None # we're doing auth!
     except ConfiguratorError, e:
         return e.message
 
@@ -449,8 +447,7 @@ def install(args, config, plugins):
     # XXX: Update for renewer/RenewableCert
 
     try:
-        installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
-        authenticator = None # we're doing install!
+        installer, _ = choose_configurator_plugins(args, config, plugins, "auth")
     except ConfiguratorError, e:
         return e.message
 
@@ -903,7 +900,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 "

From d6345a47c51cd94f56fc6c6a3d04634cfdefc823 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 18 Oct 2015 02:34:24 -0700
Subject: [PATCH 15/29] Fix some bugs & immprove test cases

---
 letsencrypt/cli.py            | 3 ++-
 letsencrypt/tests/cli_test.py | 9 +++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index cdad7d99c..4cbfb8a2a 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -367,12 +367,13 @@ def choose_configurator_plugins(args, config, plugins, verb):
     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:
+        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)
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index d31bfaff4..ccc57cb8c 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -96,11 +96,12 @@ class CLITest(unittest.TestCase):
 
     def test_configurator_selection(self):
         plugins = disco.PluginsRegistry.find_all()
+        args = ['--agree-eula', '--apache', '--authenticator', 'standalone']
+        ret, _, _, _ = self._call(args)
         if "apache" in plugins:
-            args = ['--agree-eula', '--apache', '--authenticator', 'standalone']
-            ret, _, _, _ = self._call(args)
             self.assertTrue("Too many flags setting" in ret)
-            # TODO add tests with a broken plugin, a missing plugin, etc
+        else:
+            self.assertTrue("The requested apache plugin does not" in ret)
 
     def test_rollback(self):
         _, _, _, client = self._call(['rollback'])
@@ -126,7 +127,7 @@ class CLITest(unittest.TestCase):
         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):

From 995c1dfb83eba73d3dbffe22adbdeb801aa520b3 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 18 Oct 2015 11:05:46 -0700
Subject: [PATCH 16/29] More sphinxiness, more clarity

---
 letsencrypt/cli.py            | 2 +-
 letsencrypt/tests/cli_test.py | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 4cbfb8a2a..4c4692675 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -323,7 +323,7 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
     :param string requested: the plugin that was requested
     :param PluginRegistry plugins: available plugins
 
-    :raises error.ConfiguratorError if there was a problem
+    :raises error.ConfiguratorError: if there was a problem
     """
 
     if requested:
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index ccc57cb8c..ee847b234 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -98,10 +98,13 @@ class CLITest(unittest.TestCase):
         plugins = disco.PluginsRegistry.find_all()
         args = ['--agree-eula', '--apache', '--authenticator', 'standalone']
         ret, _, _, _ = self._call(args)
+        # TODO replace these cases with .mockery to test both paths regardless
+        # of what's actually installed
         if "apache" in plugins:
             self.assertTrue("Too many flags setting" in ret)
         else:
-            self.assertTrue("The requested apache plugin does not" in ret)
+            self.assertTrue("The requested apache plugin does not appear" in ret)
+
 
     def test_rollback(self):
         _, _, _, client = self._call(['rollback'])

From 63f4f113608cde95e2ab190ea2e3063655a965e5 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 18 Oct 2015 11:05:58 -0700
Subject: [PATCH 17/29] Test "install --nginx" w/ misconfiguration

---
 letsencrypt/tests/cli_test.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index ee847b234..d246f1a9a 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -105,6 +105,15 @@ class CLITest(unittest.TestCase):
         else:
             self.assertTrue("The requested apache plugin does not appear" in ret)
 
+        # Sending nginx a non-existent conf dir will simulate misconfiguration
+        args = ["install", "--nginx", "--cert-path", "/tmp/blah", "--key-path", "/tmp/blah",
+                "--nginx-server-root", "/nonexistent/thing"]
+        ret, _, _, _ = self._call(args)
+
+        if "nginx" in plugins:
+            self.assertTrue("The nginx plugin is not working" in ret)
+        else:
+            self.assertTrue("The requested nginx plugin does not appear" in ret)
 
     def test_rollback(self):
         _, _, _, client = self._call(['rollback'])

From e05073c33eebd377f2ce76478bc4d8a28fef78ca Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 10:37:39 -0700
Subject: [PATCH 18/29] debugging

---
 letsencrypt/cli.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 4c4692675..047d2ae15 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -394,6 +394,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "run")
     except ConfiguratorError, e:
+        logger.warn("Exiting with message {0}".format(e.message))
         return e.message
 
     domains = _find_domains(args, installer)

From 6e69e584027087bde10b739b3d8bcee7b3b04b59 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 12:03:11 -0700
Subject: [PATCH 19/29] Make more tests run regardless of local plugin state

---
 letsencrypt/cli.py            | 32 ++++++++++++++++++--------------
 letsencrypt/errors.py         |  5 ++---
 letsencrypt/tests/cli_test.py | 30 ++++++++++++++++--------------
 3 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 047d2ae15..2883acd1c 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -37,7 +37,7 @@ from letsencrypt import storage
 
 from letsencrypt.display import util as display_util
 from letsencrypt.display import ops as display_ops
-from letsencrypt.errors import Error, ConfiguratorError, CertStorageError
+from letsencrypt.errors import Error, PluginSelectionError, CertStorageError
 from letsencrypt.plugins import disco as plugins_disco
 
 
@@ -312,7 +312,7 @@ def set_configurator(previously, now):
     if previously:
         if previously != now:
             msg = "Too many flags setting configurators/installers/authenticators %s -> %s"
-            raise ConfiguratorError, msg % (`previously`, `now`)
+            raise PluginSelectionError, msg % (`previously`, `now`)
     return now
 
 def diagnose_configurator_problem(cfg_type, requested, plugins):
@@ -323,25 +323,25 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
     :param string requested: the plugin that was requested
     :param PluginRegistry plugins: available plugins
 
-    :raises error.ConfiguratorError: if there was a problem
+    :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)
-            raise ConfiguratorError, msg
+            raise PluginSelectionError, msg
         else:
             msg = ("The {0} plugin is not working; there may be problems with "
                    "your existing configuration").format(requested)
-            raise ConfiguratorError, msg
-    raise ConfiguratorError, "{0} could not be determined or is not installed".format(cfg_type)
+            raise PluginSelectionError, msg
+    raise PluginSelectionError, "{0} could not be determined or is not installed".format(cfg_type)
 
 
 def choose_configurator_plugins(args, config, plugins, verb):
     """
     Figure out which configurator we're going to use
 
-    :raises error.ConfiguratorError if there was a problem
+    :raises error.PluginSelectionError if there was a problem
     """
 
     # Which plugins do we need?
@@ -352,7 +352,7 @@ def choose_configurator_plugins(args, config, plugins, verb):
         need_inst = True
         if args.authenticator:
             msg = "Specifying an authenticator doesn't make sense in install mode"
-            raise ConfiguratorError, msg
+            raise PluginSelectionError, msg
 
     # Which plugins did the user request?
     req_inst = req_auth = args.configurator
@@ -393,8 +393,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     """Obtain a certificate and install."""
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "run")
-    except ConfiguratorError, e:
-        logger.warn("Exiting with message {0}".format(e.message))
+    except PluginSelectionError, e:
         return e.message
 
     domains = _find_domains(args, installer)
@@ -426,7 +425,7 @@ def auth(args, config, plugins):
     try:
         # installers are used in auth mode to determine domain names
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
-    except ConfiguratorError, e:
+    except PluginSelectionError, e:
         return e.message
 
     # TODO: Handle errors from _init_le_client?
@@ -450,7 +449,7 @@ def install(args, config, plugins):
 
     try:
         installer, _ = choose_configurator_plugins(args, config, plugins, "auth")
-    except ConfiguratorError, e:
+    except PluginSelectionError, e:
         return e.message
 
     if args.authenticator:
@@ -1007,6 +1006,8 @@ def _handle_exception(exc_type, exc_value, trace, args):
             traceback.format_exception(exc_type, exc_value, trace)))
 
 
+# this copy of plugins can be mocked out
+plugins_testable = plugins_disco.PluginsRegistry.find_all()
 def main(cli_args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     sys.excepthook = functools.partial(_handle_exception, args=None)
@@ -1066,8 +1067,11 @@ def main(cli_args=sys.argv[1:]):
         #    "{0}Root is required to run letsencrypt.  Please use sudo.{0}"
         #    .format(os.linesep))
 
-    return args.func(args, config, plugins)
+    return args.func(args, config, plugins_testable)
 
 
 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
diff --git a/letsencrypt/errors.py b/letsencrypt/errors.py
index def8fc20d..fd737bc81 100644
--- a/letsencrypt/errors.py
+++ b/letsencrypt/errors.py
@@ -24,9 +24,6 @@ class SubprocessError(Error):
 class CertStorageError(Error):
     """Generic `.CertStorage` error."""
 
-class ConfiguratorError(Error):
-    """A problem with plugin/configurator selection or setup"""
-
 # Auth Handler Errors
 class AuthorizationError(Error):
     """Authorization error."""
@@ -67,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."""
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index d246f1a9a..e42ca3e92 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -95,24 +95,26 @@ class CLITest(unittest.TestCase):
             self.assertTrue(cli.USAGE in out)
 
     def test_configurator_selection(self):
-        plugins = disco.PluginsRegistry.find_all()
+        real_plugins = disco.PluginsRegistry.find_all()
         args = ['--agree-eula', '--apache', '--authenticator', 'standalone']
-        ret, _, _, _ = self._call(args)
-        # TODO replace these cases with .mockery to test both paths regardless
-        # of what's actually installed
-        if "apache" in plugins:
+
+        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)
-        else:
-            self.assertTrue("The requested apache plugin does not appear" in ret)
 
-        # Sending nginx a non-existent conf dir will simulate misconfiguration
-        args = ["install", "--nginx", "--cert-path", "/tmp/blah", "--key-path", "/tmp/blah",
-                "--nginx-server-root", "/nonexistent/thing"]
-        ret, _, _, _ = self._call(args)
-
-        if "nginx" in plugins:
+        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)
+            args = ["install", "--nginx", "--cert-path", "/tmp/blah", "--key-path", "/tmp/blah",
+                    "--nginx-server-root", "/nonexistent/thing"]
+            ret, _, _, _ = self._call(args)
             self.assertTrue("The nginx plugin is not working" in ret)
-        else:
+
+        # But we can pretend that nginx is uninstalled, even if it is
+        with mock.patch('letsencrypt.cli.plugins_testable') as plugins:
+            plugins.return_value = {}
+            ret, _, _, _ = self._call(args)
             self.assertTrue("The requested nginx plugin does not appear" in ret)
 
     def test_rollback(self):

From 39ae9bdf43c27aaa16886bfd63db965415a4718c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 12:06:39 -0700
Subject: [PATCH 20/29] Document a somewhat cryptic function

---
 letsencrypt/cli.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 2883acd1c..dad486c2c 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -305,7 +305,12 @@ 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"""
+    """
+    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

From 26e703779468dedc556d0fc67dac079cd68d8306 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 12:15:11 -0700
Subject: [PATCH 21/29] Fix a mock type error

---
 letsencrypt/cli.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index dad486c2c..4368db7f7 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1012,7 +1012,7 @@ def _handle_exception(exc_type, exc_value, trace, args):
 
 
 # this copy of plugins can be mocked out
-plugins_testable = plugins_disco.PluginsRegistry.find_all()
+plugins_testable = plugins_disco.PluginsRegistry.find_all
 def main(cli_args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     sys.excepthook = functools.partial(_handle_exception, args=None)
@@ -1072,7 +1072,7 @@ def main(cli_args=sys.argv[1:]):
         #    "{0}Root is required to run letsencrypt.  Please use sudo.{0}"
         #    .format(os.linesep))
 
-    return args.func(args, config, plugins_testable)
+    return args.func(args, config, plugins_testable())
 
 
 if __name__ == "__main__":

From eac513d73b204c6f31c807b53851f4d65ece0497 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 17:21:47 -0700
Subject: [PATCH 22/29] Offer more specific help if no installer is present

Fixes: #896
---
 letsencrypt/cli.py | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 4368db7f7..5f4f8102a 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -334,12 +334,21 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
     if requested:
         if requested not in plugins:
             msg = "The requested {0} plugin does not appear to be installed".format(requested)
-            raise PluginSelectionError, msg
         else:
             msg = ("The {0} plugin is not working; there may be problems with "
                    "your existing configuration").format(requested)
-            raise PluginSelectionError, msg
-    raise PluginSelectionError, "{0} could not be determined or is not installed".format(cfg_type)
+    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):

From 484e8ef6fece5d94f7d98bc955cd12b79dff13ad Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 19:11:05 -0700
Subject: [PATCH 23/29] Not trying to mock out plugins just yet

---
 letsencrypt/tests/cli_test.py | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index e42ca3e92..93947c2f1 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -103,20 +103,15 @@ class CLITest(unittest.TestCase):
             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)
-            args = ["install", "--nginx", "--cert-path", "/tmp/blah", "--key-path", "/tmp/blah",
-                    "--nginx-server-root", "/nonexistent/thing"]
             ret, _, _, _ = self._call(args)
             self.assertTrue("The nginx plugin is not working" in ret)
 
-        # But we can pretend that nginx is uninstalled, even if it is
-        with mock.patch('letsencrypt.cli.plugins_testable') as plugins:
-            plugins.return_value = {}
-            ret, _, _, _ = self._call(args)
-            self.assertTrue("The requested nginx plugin does not appear" in ret)
-
     def test_rollback(self):
         _, _, _, client = self._call(['rollback'])
         self.assertEqual(1, client.rollback.call_count)

From eeeb66e58a3f8c475cf5f95c5b9aa04a371dd581 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 19:15:11 -0700
Subject: [PATCH 24/29] document -d domain

Closes: #285 #787

we may want to implement the other thing soon, but for now documenting what we
have
---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 5f4f8102a..6af6a45c6 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -50,7 +50,7 @@ logger = logging.getLogger(__name__)
 # 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

From 6460a3b50ef3b14f5ad2d2fc98a0ae683e131c6f Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 19:27:36 -0700
Subject: [PATCH 25/29] Dial back the "authenticator doesn't make sense in
 install mode" message

Since it was breaking integration tests
---
 letsencrypt/cli.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 6af6a45c6..dd0623757 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -365,8 +365,7 @@ def choose_configurator_plugins(args, config, plugins, verb):
     if verb == "install":
         need_inst = True
         if args.authenticator:
-            msg = "Specifying an authenticator doesn't make sense in install mode"
-            raise PluginSelectionError, msg
+            logger.warn("Specifying an authenticator doesn't make sense in install mode")
 
     # Which plugins did the user request?
     req_inst = req_auth = args.configurator

From 72589bcae3506d71d5f85250edbb3ef9756a9828 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 19:31:09 -0700
Subject: [PATCH 26/29] Lint

---
 letsencrypt/cli.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index dd0623757..fb564c630 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -307,7 +307,6 @@ def _auth_from_domains(le_client, config, domains, plugins):
 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
     """

From e384a1fe2857e53abfabbe03175ed52b7af61575 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 19:35:53 -0700
Subject: [PATCH 27/29] Catch some excessive policing of installers

---
 letsencrypt/cli.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index fb564c630..f0e1d3a4d 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -464,9 +464,6 @@ def install(args, config, plugins):
     except PluginSelectionError, e:
         return e.message
 
-    if args.authenticator:
-        return "Specifying an authenticator doesn't make sense in install mode!"
-
     domains = _find_domains(args, installer)
     le_client = _init_le_client(
         args, config, authenticator=None, installer=installer)

From 51ed2b681f87b1eb29088dd48718a54f401e4855 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 19:41:37 -0700
Subject: [PATCH 28/29] We're not mocking out plugins

So we don't need infrastructure for it
---
 letsencrypt/cli.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index f0e1d3a4d..54168ef87 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1015,8 +1015,6 @@ def _handle_exception(exc_type, exc_value, trace, args):
             traceback.format_exception(exc_type, exc_value, trace)))
 
 
-# this copy of plugins can be mocked out
-plugins_testable = plugins_disco.PluginsRegistry.find_all
 def main(cli_args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     sys.excepthook = functools.partial(_handle_exception, args=None)
@@ -1076,7 +1074,7 @@ def main(cli_args=sys.argv[1:]):
         #    "{0}Root is required to run letsencrypt.  Please use sudo.{0}"
         #    .format(os.linesep))
 
-    return args.func(args, config, plugins_testable())
+    return args.func(args, config, plugins)
 
 
 if __name__ == "__main__":

From 363dffd8a644ae42bed9689b133ceda42f78f034 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Oct 2015 19:51:02 -0700
Subject: [PATCH 29/29] Comment out a test that requires slightly doubtful
 infrastructure

---
 letsencrypt/tests/cli_test.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 93947c2f1..2b8be5b1e 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -98,10 +98,13 @@ class CLITest(unittest.TestCase):
         real_plugins = disco.PluginsRegistry.find_all()
         args = ['--agree-eula', '--apache', '--authenticator', 'standalone']
 
-        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)
+        # 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",