From 8faa877c45e319b53e82258338ba7e07320856b1 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 30 Mar 2015 12:33:49 +0000
Subject: [PATCH 01/43] plugins.disco

---
 letsencrypt/client/constants.py     |   3 +
 letsencrypt/client/display/ops.py   |  22 +++++
 letsencrypt/client/interfaces.py    |  50 +++++------
 letsencrypt/client/plugins/disco.py | 123 ++++++++++++++++++++++++++++
 4 files changed, 170 insertions(+), 28 deletions(-)
 create mode 100644 letsencrypt/client/plugins/disco.py

diff --git a/letsencrypt/client/constants.py b/letsencrypt/client/constants.py
index 43cf5e8a0..8f2d083ef 100644
--- a/letsencrypt/client/constants.py
+++ b/letsencrypt/client/constants.py
@@ -4,6 +4,9 @@ import pkg_resources
 from letsencrypt.acme import challenges
 
 
+SETUPTOOLS_PLUGINS_ENTRY_POINT = "letsencrypt.plugins"
+"""Setuptools entry point group name for plugins."""
+
 S_SIZE = 32
 """Size (in bytes) of secret base64-encoded octet string "s" used in
 challenges."""
diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py
index 1cffe2846..8c97aadd5 100644
--- a/letsencrypt/client/display/ops.py
+++ b/letsencrypt/client/display/ops.py
@@ -10,6 +10,28 @@ from letsencrypt.client.display import util as display_util
 util = zope.component.getUtility  # pylint: disable=invalid-name
 
 
+def choose_plugin(prepared, question):
+    descs = [plugin.description if error is None
+             else "%s (Misconfigured)" % plugin.description
+             for (plugin, error) in prepared]
+
+    while True:
+        code, index = util(interfaces.IDisplay).menu(
+            question, descs, help_label="More Info")
+
+        if code == display_util.OK:
+            return prepared[index][0]
+        elif code == display_util.HELP:
+            if prepared[index][1] is not None:
+                msg = "Reported Error: %s" % prepared[index][1]
+            else:
+                msg = prepared[index][0].more_info()
+            util(interfaces.IDisplay).notification(
+                msg, height=display_util.HEIGHT)
+        else:
+            return
+
+
 def choose_authenticator(auths, errs):
     """Allow the user to choose their authenticator.
 
diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py
index 6779d4e1e..2390330b6 100644
--- a/letsencrypt/client/interfaces.py
+++ b/letsencrypt/client/interfaces.py
@@ -5,16 +5,13 @@ import zope.interface
 # pylint: disable=too-few-public-methods
 
 
-class IAuthenticator(zope.interface.Interface):
-    """Generic Let's Encrypt Authenticator.
+class IPlugin(zope.interface.Interface):
+    """Let's Encrypt plugin."""
 
-    Class represents all possible tools processes that have the
-    ability to perform challenges and attain a certificate.
-
-    """
+    description = zope.interface.Attribute("Short plugin description")
 
     def prepare():
-        """Prepare the authenticator.
+        """Prepare the plugin.
 
          Finish up any additional initialization.
 
@@ -25,6 +22,23 @@ class IAuthenticator(zope.interface.Interface):
 
         """
 
+    def more_info():
+        """Human-readable string to help the user.
+
+        Should describe the steps taken and any relevant info to help the user
+        decide which plugin to use.
+
+        """
+
+
+class IAuthenticator(IPlugin):
+    """Generic Let's Encrypt Authenticator.
+
+    Class represents all possible tools processes that have the
+    ability to perform challenges and attain a certificate.
+
+    """
+
     def get_chall_pref(domain):
         """Return list of challenge preferences.
 
@@ -70,14 +84,6 @@ class IAuthenticator(zope.interface.Interface):
 
         """
 
-    def more_info():
-        """Human-readable string to help the user.
-
-        Should describe the steps taken and any relevant info to help the user
-        decide which Authenticator to use.
-
-        """
-
 
 class IConfig(zope.interface.Interface):
     """Let's Encrypt user-supplied configuration.
@@ -124,25 +130,13 @@ class IConfig(zope.interface.Interface):
         "Contains standard Apache SSL directives.")
 
 
-class IInstaller(zope.interface.Interface):
+class IInstaller(IPlugin):
     """Generic Let's Encrypt Installer Interface.
 
     Represents any server that an X509 certificate can be placed.
 
     """
 
-    def prepare():
-        """Prepare the installer.
-
-         Finish up any additional initialization.
-
-         :raises letsencrypt.client.errors.LetsEncryptMisconfigurationError`:
-             when full initialization cannot be completed.
-         :raises letsencrypt.errors.LetsEncryptNoInstallationError`:
-             when the necessary programs/files cannot be located.
-
-        """
-
     def get_all_names():
         """Returns all names that may be authenticated."""
 
diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
new file mode 100644
index 000000000..d2526d583
--- /dev/null
+++ b/letsencrypt/client/plugins/disco.py
@@ -0,0 +1,123 @@
+"""Utilities for plugins discovery and selection."""
+import collections
+import logging
+import pkg_resources
+
+import zope.interface
+
+from letsencrypt.client import constants
+from letsencrypt.client import errors
+from letsencrypt.client import interfaces
+
+from letsencrypt.client.display import ops as display_ops
+
+
+def name_plugins(plugins):
+    # TODO: actually make it unambiguous...
+    names = {}
+    for plugin_cls, entry_points in plugins.iteritems():
+        entry_point = next(iter(entry_points))  # entry_points.peek()
+        names[plugin_cls] = entry_point.name
+    return names
+
+
+def find_plugins():
+    """Find plugins using setuptools entry points."""
+    plugins = collections.defaultdict(set)
+    for entry_point in pkg_resources.iter_entry_points(
+            constants.SETUPTOOLS_PLUGINS_ENTRY_POINT):
+        plugin_cls = entry_point.load()
+        plugins[plugin_cls].add(entry_point)
+    return plugins
+
+
+def filter_plugins(plugins, *ifaces_groups):
+    """Filter plugins based on interfaces."""
+    return dict(
+        (plugin_cls, entry_points)
+        for plugin_cls, entry_points in plugins.iteritems()
+        if not ifaces_groups or any(
+            all(iface.implementedBy(plugin_cls) for iface in ifaces)
+            for ifaces in ifaces_groups))
+
+
+def verify_plugins(initialized, ifaces):
+    """Verify plugin objects."""
+    verified = {}
+    for plugin_cls, plugin in initialized.iteritems():
+        verifies = True
+        for iface in ifaces:  # zope.interface.providedBy(plugin)
+            try:
+                zope.interface.verify.verifyObject(iface, plugin)
+            except zope.interface.exceptions.BrokenImplementation:
+                if iface.implementedBy(plugin_cls):
+                    logging.debug(
+                        "%s implements %s but object does "
+                        "not verify", plugin_cls, iface.__name__)
+                verifies = False
+                break
+        if verifies:
+            verified[plugin_cls] = plugin
+    return verified
+
+
+def prepare_plugins(initialized):
+    """Prepare plugins."""
+    prepared = {}
+
+    for plugin_cls, plugin in initialized.iteritems():
+        error = None
+        try:
+            plugin.prepare()
+        except errors.LetsEncryptMisconfigurationError as error:
+            logging.debug("Misconfigured %s: %s", plugin, error)
+        except errors.LetsEncryptNoInstallationError as error:
+            logging.debug("No installation (%s): %s", plugin, error)
+            continue
+        prepared[plugin_cls] = (plugin, error)
+
+    return prepared  # succefully prepared + misconfigured
+
+
+def pick_plugin(config, default, ifaces, question):
+    plugins = find_plugins()
+    names = name_plugins(plugins)
+
+    if default is not None:
+        filtered = [names[default]]
+    else:
+        filtered = filter_plugins(plugins, ifaces)
+
+    initialized = dict((plugin_cls, plugin_cls(config))
+                       for plugin_cls in filtered)
+    verified = verify_plugins(initialized, ifaces)
+    prepared = prepare_plugins(initialized)
+
+    if len(prepared) > 1:
+        logging.debug("Multiple candidate plugins: %s", prepared)
+        return display_ops.choose_plugin(prepared.values(), question)
+    elif len(prepared) == 1:
+        logging.debug("Single candidate plugin: %s", prepared)
+        return prepared.values()[0]
+    else:
+        logging.debug("No candidate plugin")
+        return None
+
+
+def pick_authenticator(config, default):
+    """Pick authentication plugin."""
+    return pick_plugin(
+            config, default, (interfaces.IAuthenticator,),
+            "How would you like to authenticate with Let's Encrypt CA?")
+
+
+def pick_installer(config, default):
+    """Pick installer plugin."""
+    return pick_plugin(config, default, (interfaces.IInstaller,),
+                       "How would you like to install certificates?")
+
+def pick_configurator(config, default):
+    """Pick configurator plugin."""
+    return pick_plugin(
+        config, default, (interfaces.IAuthenticator, interfaces.IInstaller),
+        "How would you like to install certificates?")

From 8bc55899e64a7b20ba6d8e2e00063865ee9f2d88 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 30 Mar 2015 12:34:22 +0000
Subject: [PATCH 02/43] Subparsers CLI

---
 examples/plugins/setup.py       |   2 +-
 letsencrypt/client/constants.py |   4 +
 letsencrypt/scripts/main.py     | 378 +++++++++++++++++++++-----------
 setup.py                        |   2 +-
 4 files changed, 254 insertions(+), 132 deletions(-)

diff --git a/examples/plugins/setup.py b/examples/plugins/setup.py
index 845d6eb66..599d57020 100644
--- a/examples/plugins/setup.py
+++ b/examples/plugins/setup.py
@@ -9,7 +9,7 @@ setup(
         'zope.interface',
     ],
     entry_points={
-        'letsencrypt.authenticators': [
+        'letsencrypt.plugins': [
             'example = letsencrypt_example_plugins:Authenticator',
         ],
     },
diff --git a/letsencrypt/client/constants.py b/letsencrypt/client/constants.py
index 8f2d083ef..9541aacac 100644
--- a/letsencrypt/client/constants.py
+++ b/letsencrypt/client/constants.py
@@ -1,4 +1,5 @@
 """Let's Encrypt constants."""
+import logging
 import pkg_resources
 
 from letsencrypt.acme import challenges
@@ -7,6 +8,9 @@ from letsencrypt.acme import challenges
 SETUPTOOLS_PLUGINS_ENTRY_POINT = "letsencrypt.plugins"
 """Setuptools entry point group name for plugins."""
 
+DEFAULT_VERBOSE_COUNT = -(logging.WARNING / 10)
+
+
 S_SIZE = 32
 """Size (in bytes) of secret base64-encoded octet string "s" used in
 challenges."""
diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py
index 3b4b7c10d..cc89510cf 100644
--- a/letsencrypt/scripts/main.py
+++ b/letsencrypt/scripts/main.py
@@ -1,11 +1,8 @@
-"""Parse command line and call the appropriate functions.
-
-.. todo:: Sanity check all input.  Be sure to avoid shell code etc...
-
-"""
+"""Let's Encrypt Client."""
+# TODO: Sanity check all input.  Be sure to avoid shell code etc...
 import argparse
+import collections
 import logging
-import os
 import pkg_resources
 import sys
 
@@ -17,46 +14,254 @@ import zope.interface.verify
 import letsencrypt
 
 from letsencrypt.client import configuration
+from letsencrypt.client import constants
 from letsencrypt.client import client
 from letsencrypt.client import errors
 from letsencrypt.client import interfaces
 from letsencrypt.client import le_util
 from letsencrypt.client import log
+
 from letsencrypt.client.display import util as display_util
 from letsencrypt.client.display import ops as display_ops
 
-
-SETUPTOOLS_AUTHENTICATORS_ENTRY_POINT = "letsencrypt.authenticators"
-"""Setuptools entry point group name for Authenticator plugins."""
+from letsencrypt.client.plugins import disco as plugins_disco
 
 
-def init_auths(config):
-    """Find (setuptools entry points) and initialize Authenticators."""
-    auths = {}
-    for entrypoint in pkg_resources.iter_entry_points(
-            SETUPTOOLS_AUTHENTICATORS_ENTRY_POINT):
-        auth_cls = entrypoint.load()
-        auth = auth_cls(config)
-        try:
-            zope.interface.verify.verifyObject(interfaces.IAuthenticator, auth)
-        except zope.interface.exceptions.BrokenImplementation:
-            logging.debug(
-                "%r object does not provide IAuthenticator, skipping",
-                entrypoint.name)
+def _common_run(args, config, authenticator, installer):
+    if args.domains is None:
+        doms = display_ops.choose_names(installer)
+    else:
+        doms = args.domains
+
+    if not doms:
+        return
+
+    # Prepare for init of Client
+    if args.authkey is None:
+        authkey = client.init_key(config.rsa_key_size, config.key_dir)
+    else:
+        authkey = le_util.Key(args.authkey[0], args.authkey[1])
+
+    acme = client.Client(config, authkey, authenticator, installer)
+
+    # Validate the key and csr
+    client.validate_key_csr(authkey)
+
+    return acme, doms, authkey
+
+
+def run(args, config):
+    """Obtain a certificate and install."""
+    if not args.eula:
+        display_eula()
+
+    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 = plugins_disco.pick_installer(
+            config, args.installer)
+        authenticator = plugins_disco.pick_authenticator(
+            config, args.authenticator)
+    else:
+        authenticator = installer = plugins_disco.pick_configurator(
+            config, args.configurator)
+
+    if installer is None or authenticator is None:
+        return "Configurator could not be determined"
+
+    acme, auth, installer, doms, auth_key = _common_run(args, config)
+    cert_file, chain_file = acme.obtain_certificate(doms)
+    acme.deploy_certificate(doms, authkey, cert_file, chain_file)
+    acme.enhance_config(doms, args.redirect)
+
+
+def auth(args, config):
+    """Obtain a certificate (no install)."""
+    authenticator = plugins_disco.pick_authenticator(config, args.authenticator)
+    if authenticator is None:
+        return "Authenticator could not be determined"
+
+    if args.installer is not None:
+        installer = plugins_disco.pick_installer(config, args.installer)
+    else:
+        installer = None
+
+    if args.domains is None:
+        if args.installer is not None:
+            return ("--domains not set and provided --installer does not "
+                    "help in autodiscovery")
         else:
-            auths[auth] = entrypoint.name
-    return auths
+           return ("Please specify --domains, or --installer that will "
+                   "help in domain names autodiscovery")
+
+    acme, doms, _ = _common_run(
+        args, config, authenticator=authenticator, installer=None)
+    acme.obtain_certificate(doms)
+
+
+def install(args, config):
+    """Install (no auth)."""
+    installer = plugins_disco.pick_installer(config, args.installer)
+    if installer is None:
+        return "Installer could not be determined"
+    acme, doms, authkey = _common_run(
+        args, config, authenticator=None, installer=installer)
+    assert args.cert_file is not None and args.chain_file is not None
+    acme.deploy_certificate(doms, authkey, args.cert_file, args.chain_file)
+    acme.enhance_config(doms, args.redirect)
+
+
+def revoke(args, config):
+    """Revoke."""
+    if args.rev_cert is None and args.rev_key is None:
+        return "At least one of --certificate or --key is required"
+    client.revoke(config, args.no_confirm, args.rev_cert, args.rev_key)
+
+
+def rollback(args, config):
+    """Rollback."""
+    client.rollback(args.checkpoints, config)
+
+
+def config_changes(args, config):
+    """View config changes.
+
+    View checkpoints and associated configuration changes.
+
+    """
+    print args, config
+    client.config_changes(config)
+
+
+def _print_plugins(filtered, plugins, names):
+    if not filtered:
+        print "No plugins found"
+
+    for plugin_cls, content in filtered.iteritems():
+        print "* {0}".format(names[plugin_cls])
+        print "Description: {0}".format(plugin_cls.description)
+        print "Interfaces: {0}".format(", ".join(
+            iface.__name__ for iface in zope.interface.implementedBy(
+                plugin_cls)))
+        print "Entry points:"
+        for entry_point in plugins[plugin_cls]:
+            print "- {0.dist}: {0}".format(entry_point)
+
+        # if filtered == prepared:
+        if isinstance(content, tuple) and content[1] is not None:
+            print content[1]  # error
+        print
+
+
+def plugins(args, config):
+    """List plugins."""
+    plugins = plugins_disco.find_plugins()
+    logging.debug("Discovered plugins: %s", plugins)
+
+    names = plugins_disco.name_plugins(plugins)
+
+    ifaces = [] if args.ifaces is None else args.ifaces
+    filtered = plugins_disco.filter_plugins(
+        plugins, *((iface,) for iface in ifaces))
+    logging.debug("Filtered plugins: %s", filtered)
+
+    if not args.init and not args.prepare:
+        return _print_plugins(filtered, plugins, names)
+
+    initialized = dict((plugin_cls, plugin_cls(config))
+                       for plugin_cls in filtered)
+    verified = plugins_disco.verify_plugins(initialized, ifaces)
+    logging.debug("Verified plugins: %s", initialized)
+
+    if not args.prepare:
+        return _print_plugins(initialized, plugins, names)
+
+    prepared = plugins_disco.prepare_plugins(initialized)
+    logging.debug("Prepared plugins: %s", plugins)
+
+    _print_plugins(prepared, plugins, names)
+    plugins_disco
+
+
+def display_eula():
+    """Displays the end user agreement."""
+    eula = pkg_resources.resource_string("letsencrypt", "EULA")
+    if not zope.component.getUtility(interfaces.IDisplay).yesno(
+            eula, "Agree", "Cancel"):
+        sys.exit(0)
+
+
+def read_file(filename):
+    """Returns the given file's contents with universal new line support.
+
+    :param str filename: Filename
+
+    :returns: A tuple of filename and its contents
+    :rtype: tuple
+
+    :raises argparse.ArgumentTypeError: File does not exist or is not readable.
+
+    """
+    try:
+        return filename, open(filename, "rU").read()
+    except IOError as exc:
+        raise argparse.ArgumentTypeError(exc.strerror)
 
 
 def create_parser():
     """Create parser."""
-    parser = confargparse.ConfArgParser(
-        description="letsencrypt client %s" % letsencrypt.__version__)
+    parser = confargparse.ConfArgParser(description=__doc__)
+
+    # --help is automatically provided by argparse
+    parser.add_argument(
+        "--version", action="version", version="%(prog)s {0}".format(
+            letsencrypt.__version__))
+    parser.add_argument(
+        "-v", "--verbose", dest="verbose_count", action="count",
+        default=constants.DEFAULT_VERBOSE_COUNT)
+
+    subparsers = parser.add_subparsers(metavar="SUBCOMMAND")
+    def add_subparser(name, func):
+        subparser = subparsers.add_parser(
+            name, help=func.__doc__.splitlines()[0], description=func.__doc__)
+        subparser.set_defaults(func=func)
+        return subparser
+
+    parser_run = add_subparser("run", run)
+    parser_auth = add_subparser("auth", auth)
+    parser_install = add_subparser("install", install)
+    parser_revoke = add_subparser("revoke", revoke)
+    parser_rollback = add_subparser("rollback", rollback)
+    parrser_config_changes = add_subparser("config_changes", config_changes)
+
+    parser_plugins = add_subparser("plugins", plugins)
+    parser_plugins.add_argument("--init", action="store_true")
+    parser_plugins.add_argument("--prepare", action="store_true")
+    parser_plugins.add_argument(
+        "--authenticators", action="append_const", dest="ifaces",
+        const=interfaces.IAuthenticator)
+    parser_plugins.add_argument(
+        "--installers", action="append_const", dest="ifaces",
+        const=interfaces.IInstaller)
 
     add = parser.add_argument
     config_help = lambda name: interfaces.IConfig[name].__doc__
 
-    add("-d", "--domains", metavar="DOMAIN", nargs="+")
+    parser_run.add_argument("--configurator")
+    for subparser in parser_run, parser_auth:
+        subparser.add_argument("-a", "--authenticator")
+    for subparser in parser_run, parser_auth, parser_install:
+        # parser_auth uses --installer for domains autodiscovery
+        subparser.add_argument("-i", "--installer")
+    # 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:
+    #    subparser.add_argument("domains", nargs="*", metavar="domain")
+
+    add("-d", "--domains", metavar="DOMAIN", action="append")
     add("-s", "--server", default="letsencrypt-demo.org:443",
         help=config_help("server"))
 
@@ -65,17 +270,16 @@ def create_parser():
     add("-B", "--rsa-key-size", type=int, default=2048, metavar="N",
         help=config_help("rsa_key_size"))
 
-    add("-R", "--revoke", action="store_true",
-        help="Revoke a certificate from a menu.")
-    add("--revoke-certificate", dest="rev_cert", type=read_file,
+    parser_revoke.add_argument(
+        "--certificate", dest="rev_cert", type=read_file, metavar="CERT_PATH",
         help="Revoke a specific certificate.")
-    add("--revoke-key", dest="rev_key", type=read_file,
+    parser_revoke.add_argument(
+        "--key", dest="rev_key", type=read_file, metavar="KEY_PATH",
         help="Revoke all certs generated by the provided authorized key.")
 
-    add("-b", "--rollback", type=int, default=0, metavar="N",
+    parser_rollback.add_argument(
+        "--checkpoints", type=int, default=0, metavar="N",
         help="Revert configuration N number of checkpoints.")
-    add("-v", "--view-config-changes", action="store_true",
-        help="View checkpoints and associated configuration changes.")
 
     # TODO: resolve - assumes binary logic while client.py assumes ternary.
     add("-r", "--redirect", action="store_true",
@@ -127,112 +331,26 @@ def main():  # pylint: disable=too-many-branches, too-many-statements
     config = configuration.NamespaceConfig(args)
 
     # note: check is done after arg parsing as --help should work w/o root also.
-    if not os.geteuid() == 0:
-        sys.exit(
-            "{0}Root is required to run letsencrypt.  Please use sudo.{0}"
-            .format(os.linesep))
+    #if not os.geteuid() == 0:
+    #    return (
+    #        "{0}Root is required to run letsencrypt.  Please use sudo.{0}"
+    #        .format(os.linesep))
 
     # Set up logging
+    level = -args.verbose_count * 10
     logger = logging.getLogger()
-    logger.setLevel(logging.INFO)
+    logger.setLevel(level)
+    logging.debug("Logging level set at %d", level)
+    # displayer
     if args.use_curses:
         logger.addHandler(log.DialogHandler())
         displayer = display_util.NcursesDisplay()
     else:
         displayer = display_util.FileDisplay(sys.stdout)
-
     zope.component.provideUtility(displayer)
 
-    if args.view_config_changes:
-        client.view_config_changes(config)
-        sys.exit()
-
-    if args.revoke or args.rev_cert is not None or args.rev_key is not None:
-        client.revoke(config, args.no_confirm, args.rev_cert, args.rev_key)
-        sys.exit()
-
-    if args.rollback > 0:
-        client.rollback(args.rollback, config)
-        sys.exit()
-
-    if not args.eula:
-        display_eula()
-
-    all_auths = init_auths(config)
-    logging.debug('Initialized authenticators: %s', all_auths.values())
-    try:
-        auth = client.determine_authenticator(all_auths.keys())
-    except errors.LetsEncryptClientError:
-        logging.critical("No authentication mechanisms were found on your "
-                         "system.")
-        sys.exit(1)
-
-    if auth is None:
-        sys.exit(0)
-
-    # Use the same object if possible
-    if interfaces.IInstaller.providedBy(auth):  # pylint: disable=no-member
-        installer = auth
-    else:
-        # This is simple and avoids confusion right now.
-        installer = None
-
-    if args.domains is None:
-        doms = display_ops.choose_names(installer)
-    else:
-        doms = args.domains
-
-    if not doms:
-        sys.exit(0)
-
-    # Prepare for init of Client
-    if args.authkey is None:
-        authkey = client.init_key(args.rsa_key_size, config.key_dir)
-    else:
-        authkey = le_util.Key(args.authkey[0], args.authkey[1])
-
-    acme = client.Client(config, authkey, auth, installer)
-
-    # Validate the key and csr
-    client.validate_key_csr(authkey)
-
-    # This more closely mimics the capabilities of the CLI
-    # It should be possible for reconfig only, install-only, no-install
-    # I am not sure the best way to handle all of the unimplemented abilities,
-    # but this code should be safe on all environments.
-    cert_file = None
-    if auth is not None:
-        cert_file, chain_file = acme.obtain_certificate(doms)
-    if installer is not None and cert_file is not None:
-        acme.deploy_certificate(doms, authkey, cert_file, chain_file)
-    if installer is not None:
-        acme.enhance_config(doms, args.redirect)
-
-
-def display_eula():
-    """Displays the end user agreement."""
-    eula = pkg_resources.resource_string("letsencrypt", "EULA")
-    if not zope.component.getUtility(interfaces.IDisplay).yesno(
-            eula, "Agree", "Cancel"):
-        sys.exit(0)
-
-
-def read_file(filename):
-    """Returns the given file's contents with universal new line support.
-
-    :param str filename: Filename
-
-    :returns: A tuple of filename and its contents
-    :rtype: tuple
-
-    :raises argparse.ArgumentTypeError: File does not exist or is not readable.
-
-    """
-    try:
-        return filename, open(filename, "rU").read()
-    except IOError as exc:
-        raise argparse.ArgumentTypeError(exc.strerror)
+    return args.func(args, config)
 
 
 if __name__ == "__main__":
-    main()
+    sys.exit(main())
diff --git a/setup.py b/setup.py
index ca7de3abb..413345125 100644
--- a/setup.py
+++ b/setup.py
@@ -122,7 +122,7 @@ setup(
             'letsencrypt = letsencrypt.scripts.main:main',
             'jws = letsencrypt.acme.jose.jws:CLI.run',
         ],
-        'letsencrypt.authenticators': [
+        'letsencrypt.plugins': [
             'apache = letsencrypt.client.plugins.apache.configurator'
             ':ApacheConfigurator',
             'standalone = letsencrypt.client.plugins.standalone.authenticator'

From b76542afb3acf1903a4a993f7c9207f090af63a2 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Thu, 2 Apr 2015 06:55:50 +0000
Subject: [PATCH 03/43] constants:DEFAULT_*

---
 letsencrypt/client/constants.py | 17 +++++++++++++++++
 letsencrypt/scripts/main.py     | 34 ++++++++++++++++++---------------
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/letsencrypt/client/constants.py b/letsencrypt/client/constants.py
index 9541aacac..727404030 100644
--- a/letsencrypt/client/constants.py
+++ b/letsencrypt/client/constants.py
@@ -8,7 +8,24 @@ from letsencrypt.acme import challenges
 SETUPTOOLS_PLUGINS_ENTRY_POINT = "letsencrypt.plugins"
 """Setuptools entry point group name for plugins."""
 
+
+# CLI/IConfig defaults
 DEFAULT_VERBOSE_COUNT = -(logging.WARNING / 10)
+DEFAULT_SERVER = "letsencrypt-demo.org:443"
+DEFAULT_RSA_KEY_SIZE = 2048
+DEFAULT_ROLLBACK_CHECKPOINTS = 0
+DEFAULT_CONFIG_DIR = "/etc/letsencrypt"
+DEFAULT_WORK_DIR = "/var/lib/letsencrypt"
+DEFAULT_BACKUP_DIR = "/var/lib/letsencrypt/backups"
+DEFAULT_KEY_DIR = "/etc/letsencrypt/keys"
+DEFAULT_CERTS_DIR = "/etc/letsencrypt/certs"
+DEFAULT_CERT_PATH = "/etc/letsencrypt/certs/cert-letsencrypt.pem"
+DEFAULT_CHAIN_PATH = "/etc/letsencrypt/certs/chain-letsencrypt.pem"
+DEFAULT_APACHE_SERVER_ROOT = "/etc/apache2"
+DEFAULT_APACHE_MOD_SSL_CONF = "/etc/letsencrypt/options-ssl.conf"
+DEFAULT_APACHE_CTL = "apache2ctl"
+DEFAULT_APACHE_ENMOD = "a2enmod"
+DEFAULT_APACHE_INIT_SCRIPT = "/etc/init.d/apache2"
 
 
 S_SIZE = 32
diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py
index cc89510cf..e421e7ac2 100644
--- a/letsencrypt/scripts/main.py
+++ b/letsencrypt/scripts/main.py
@@ -262,12 +262,13 @@ def create_parser():
     #    subparser.add_argument("domains", nargs="*", metavar="domain")
 
     add("-d", "--domains", metavar="DOMAIN", action="append")
-    add("-s", "--server", default="letsencrypt-demo.org:443",
+    add("-s", "--server", default=constants.DEFAULT_SERVER,
         help=config_help("server"))
 
     add("-k", "--authkey", type=read_file,
         help="Path to the authorized key file")
-    add("-B", "--rsa-key-size", type=int, default=2048, metavar="N",
+    add("-B", "--rsa-key-size", type=int, metavar="N",
+        default=constants.DEFAULT_RSA_KEY_SIZE,
         help=config_help("rsa_key_size"))
 
     parser_revoke.add_argument(
@@ -278,7 +279,8 @@ def create_parser():
         help="Revoke all certs generated by the provided authorized key.")
 
     parser_rollback.add_argument(
-        "--checkpoints", type=int, default=0, metavar="N",
+        "--checkpoints", type=int, metavar="N",
+        default=constants.DEFAULT_ROLLBACK_CHECKPOINTS,
         help="Revert configuration N number of checkpoints.")
 
     # TODO: resolve - assumes binary logic while client.py assumes ternary.
@@ -294,31 +296,33 @@ def create_parser():
     add("-t", "--text", dest="use_curses", action="store_false",
         help="Use the text output instead of the curses UI.")
 
-    add("--config-dir", default="/etc/letsencrypt",
+    add("--config-dir", default=constants.DEFAULT_CONFIG_DIR,
         help=config_help("config_dir"))
-    add("--work-dir", default="/var/lib/letsencrypt",
+    add("--work-dir", default=constants.DEFAULT_WORK_DIR,
         help=config_help("work_dir"))
-    add("--backup-dir", default="/var/lib/letsencrypt/backups",
+    add("--backup-dir", default=constants.DEFAULT_BACKUP_DIR,
         help=config_help("backup_dir"))
-    add("--key-dir", default="/etc/letsencrypt/keys",
+    add("--key-dir", default=constants.DEFAULT_KEY_DIR,
         help=config_help("key_dir"))
-    add("--cert-dir", default="/etc/letsencrypt/certs",
+    add("--cert-dir", default=constants.DEFAULT_CERTS_DIR,
         help=config_help("cert_dir"))
 
     add("--le-vhost-ext", default="-le-ssl.conf",
         help=config_help("le_vhost_ext"))
-    add("--cert-path", default="/etc/letsencrypt/certs/cert-letsencrypt.pem",
+    add("--cert-path", default=constants.DEFAULT_CERT_PATH,
         help=config_help("cert_path"))
-    add("--chain-path", default="/etc/letsencrypt/certs/chain-letsencrypt.pem",
+    add("--chain-path", default=constants.DEFAULT_CHAIN_PATH,
         help=config_help("chain_path"))
 
-    add("--apache-server-root", default="/etc/apache2",
+    add("--apache-server-root", default=constants.DEFAULT_APACHE_SERVER_ROOT,
         help=config_help("apache_server_root"))
-    add("--apache-mod-ssl-conf", default="/etc/letsencrypt/options-ssl.conf",
+    add("--apache-mod-ssl-conf", default=constants.DEFAULT_APACHE_MOD_SSL_CONF,
         help=config_help("apache_mod_ssl_conf"))
-    add("--apache-ctl", default="apache2ctl", help=config_help("apache_ctl"))
-    add("--apache-enmod", default="a2enmod", help=config_help("apache_enmod"))
-    add("--apache-init-script", default="/etc/init.d/apache2",
+    add("--apache-ctl", default=constants.DEFAULT_APACHE_CTL,
+        help=config_help("apache_ctl"))
+    add("--apache-enmod", default=constants.DEFAULT_APACHE_ENMOD,
+        help=config_help("apache_enmod"))
+    add("--apache-init-script", default=constants.DEFAULT_APACHE_INIT_SCRIPT,
         help=config_help("apache_init_script"))
 
     return parser

From 975fe1c65b0f5e7c6b6b97b04de646a0115e59c9 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 06:23:25 +0000
Subject: [PATCH 04/43] Move scripts/main to client/cli.py

---
 letsencrypt/{scripts/main.py => client/cli.py} | 2 +-
 letsencrypt/scripts/__init__.py                | 1 -
 setup.py                                       | 3 +--
 3 files changed, 2 insertions(+), 4 deletions(-)
 rename letsencrypt/{scripts/main.py => client/cli.py} (99%)
 delete mode 100644 letsencrypt/scripts/__init__.py

diff --git a/letsencrypt/scripts/main.py b/letsencrypt/client/cli.py
similarity index 99%
rename from letsencrypt/scripts/main.py
rename to letsencrypt/client/cli.py
index e421e7ac2..47bd9189a 100644
--- a/letsencrypt/scripts/main.py
+++ b/letsencrypt/client/cli.py
@@ -1,4 +1,4 @@
-"""Let's Encrypt Client."""
+"""Let's Encrypt CLI."""
 # TODO: Sanity check all input.  Be sure to avoid shell code etc...
 import argparse
 import collections
diff --git a/letsencrypt/scripts/__init__.py b/letsencrypt/scripts/__init__.py
deleted file mode 100644
index 3860534ca..000000000
--- a/letsencrypt/scripts/__init__.py
+++ /dev/null
@@ -1 +0,0 @@
-"""Let's Encrypt scripts."""
diff --git a/setup.py b/setup.py
index 413345125..2f9e85526 100644
--- a/setup.py
+++ b/setup.py
@@ -104,7 +104,6 @@ setup(
         'letsencrypt.client.plugins.standalone.tests',
         'letsencrypt.client.tests',
         'letsencrypt.client.tests.display',
-        'letsencrypt.scripts',
     ],
 
     install_requires=install_requires,
@@ -119,7 +118,7 @@ setup(
 
     entry_points={
         'console_scripts': [
-            'letsencrypt = letsencrypt.scripts.main:main',
+            'letsencrypt = letsencrypt.client.cli:main',
             'jws = letsencrypt.acme.jose.jws:CLI.run',
         ],
         'letsencrypt.plugins': [

From 1001b027cb47096a2fc7a78ad812679beafafd3d Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 06:47:27 +0000
Subject: [PATCH 05/43] ConfArgParse -> ConfigArgParse

ConfArgParse + subparsers = config scoped only to subcommand
ConfArgParse doesn't seem to respect formatter_class (no defaults)

ConfigArgParse works with ENV variables

https://github.com/bw2/ConfigArgParse#design-notes
---
 letsencrypt/client/cli.py | 7 +++++--
 setup.py                  | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index 47bd9189a..d3465469e 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -6,7 +6,7 @@ import logging
 import pkg_resources
 import sys
 
-import confargparse
+import configargparse
 import zope.component
 import zope.interface.exceptions
 import zope.interface.verify
@@ -213,7 +213,10 @@ def read_file(filename):
 
 def create_parser():
     """Create parser."""
-    parser = confargparse.ConfArgParser(description=__doc__)
+    parser = configargparse.ArgParser(
+        description=__doc__,
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter,
+        args_for_setting_config_path=["-c", "--config"])
 
     # --help is automatically provided by argparse
     parser.add_argument(
diff --git a/setup.py b/setup.py
index 2f9e85526..6e0ce0aa6 100644
--- a/setup.py
+++ b/setup.py
@@ -28,7 +28,7 @@ changes = read_file(os.path.join(here, 'CHANGES.rst'))
 
 install_requires = [
     'argparse',
-    'ConfArgParse',
+    'ConfigArgParse',
     'jsonschema',
     'mock',
     'ndg-httpsclient',  # urllib3 InsecurePlatformWarning (#304)

From 048876a1dff47646ae996071b8f7620d5475c5a9 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 07:09:23 +0000
Subject: [PATCH 06/43] default_config_files

---
 letsencrypt/client/cli.py       | 3 ++-
 letsencrypt/client/constants.py | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index d3465469e..600d6791b 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -216,7 +216,8 @@ def create_parser():
     parser = configargparse.ArgParser(
         description=__doc__,
         formatter_class=argparse.ArgumentDefaultsHelpFormatter,
-        args_for_setting_config_path=["-c", "--config"])
+        args_for_setting_config_path=["-c", "--config"],
+        default_config_files=constants.DEFAULT_CONFIG_FILES)
 
     # --help is automatically provided by argparse
     parser.add_argument(
diff --git a/letsencrypt/client/constants.py b/letsencrypt/client/constants.py
index 727404030..5c13ea1a3 100644
--- a/letsencrypt/client/constants.py
+++ b/letsencrypt/client/constants.py
@@ -10,6 +10,7 @@ SETUPTOOLS_PLUGINS_ENTRY_POINT = "letsencrypt.plugins"
 
 
 # CLI/IConfig defaults
+DEFAULT_CONFIG_FILES = ["/etc/letsencrypt/cli.ini"]
 DEFAULT_VERBOSE_COUNT = -(logging.WARNING / 10)
 DEFAULT_SERVER = "letsencrypt-demo.org:443"
 DEFAULT_RSA_KEY_SIZE = 2048

From 0e3504cecc33a5f691865c87e3fd94cf12da6b59 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 07:20:45 +0000
Subject: [PATCH 07/43] stub cli_test

---
 letsencrypt/client/cli.py            |  2 +-
 letsencrypt/client/tests/cli_test.py | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 letsencrypt/client/tests/cli_test.py

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index 600d6791b..1be10d75b 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -361,4 +361,4 @@ def main():  # pylint: disable=too-many-branches, too-many-statements
 
 
 if __name__ == "__main__":
-    sys.exit(main())
+    sys.exit(main())  # pragma: no cover
diff --git a/letsencrypt/client/tests/cli_test.py b/letsencrypt/client/tests/cli_test.py
new file mode 100644
index 000000000..8afede3bc
--- /dev/null
+++ b/letsencrypt/client/tests/cli_test.py
@@ -0,0 +1,11 @@
+import unittest
+
+
+class CLITest(unittest.TestCase):
+
+    def test_it(self):
+        from letsencrypt.client import cli
+
+
+if __name__ == '__main__':
+    unittest.main()

From bad3a959d43bee869f3ad6021e1e2312a1526fc1 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 07:22:15 +0000
Subject: [PATCH 08/43] random cli cleanup

---
 letsencrypt/client/cli.py | 85 +++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 35 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index 1be10d75b..f49918bf2 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -3,6 +3,7 @@
 import argparse
 import collections
 import logging
+import os
 import pkg_resources
 import sys
 
@@ -210,6 +211,8 @@ def read_file(filename):
     except IOError as exc:
         raise argparse.ArgumentTypeError(exc.strerror)
 
+def config_help(name):
+    return interfaces.IConfig[name].__doc__
 
 def create_parser():
     """Create parser."""
@@ -218,14 +221,19 @@ def create_parser():
         formatter_class=argparse.ArgumentDefaultsHelpFormatter,
         args_for_setting_config_path=["-c", "--config"],
         default_config_files=constants.DEFAULT_CONFIG_FILES)
+    add = parser.add_argument
 
     # --help is automatically provided by argparse
-    parser.add_argument(
-        "--version", action="version", version="%(prog)s {0}".format(
+    add("--version", action="version", version="%(prog)s {0}".format(
             letsencrypt.__version__))
-    parser.add_argument(
-        "-v", "--verbose", dest="verbose_count", action="count",
+    add("-v", "--verbose", dest="verbose_count", action="count",
         default=constants.DEFAULT_VERBOSE_COUNT)
+    add("--no-confirm", dest="no_confirm", action="store_true",
+        help="Turn off confirmation screens, currently used for --revoke")
+    add("-e", "--agree-tos", dest="eula", action="store_true",
+        help="Skip the end user license agreement screen.")
+    add("-t", "--text", dest="use_curses", action="store_false",
+        help="Use the text output instead of the curses UI.")
 
     subparsers = parser.add_subparsers(metavar="SUBCOMMAND")
     def add_subparser(name, func):
@@ -251,9 +259,6 @@ def create_parser():
         "--installers", action="append_const", dest="ifaces",
         const=interfaces.IInstaller)
 
-    add = parser.add_argument
-    config_help = lambda name: interfaces.IConfig[name].__doc__
-
     parser_run.add_argument("--configurator")
     for subparser in parser_run, parser_auth:
         subparser.add_argument("-a", "--authenticator")
@@ -268,12 +273,15 @@ def create_parser():
     add("-d", "--domains", metavar="DOMAIN", action="append")
     add("-s", "--server", default=constants.DEFAULT_SERVER,
         help=config_help("server"))
-
     add("-k", "--authkey", type=read_file,
         help="Path to the authorized key file")
     add("-B", "--rsa-key-size", type=int, metavar="N",
         default=constants.DEFAULT_RSA_KEY_SIZE,
         help=config_help("rsa_key_size"))
+    # TODO: resolve - assumes binary logic while client.py assumes ternary.
+    add("-r", "--redirect", action="store_true",
+        help="Automatically redirect all HTTP traffic to HTTPS for the newly "
+             "authenticated vhost.")
 
     parser_revoke.add_argument(
         "--certificate", dest="rev_cert", type=read_file, metavar="CERT_PATH",
@@ -287,19 +295,13 @@ def create_parser():
         default=constants.DEFAULT_ROLLBACK_CHECKPOINTS,
         help="Revert configuration N number of checkpoints.")
 
-    # TODO: resolve - assumes binary logic while client.py assumes ternary.
-    add("-r", "--redirect", action="store_true",
-        help="Automatically redirect all HTTP traffic to HTTPS for the newly "
-             "authenticated vhost.")
+    paths_parser(parser.add_argument_group("paths"))
+    apache_parser(parser.add_argument_group("apache"))
+    return parser
 
-    add("--no-confirm", dest="no_confirm", action="store_true",
-        help="Turn off confirmation screens, currently used for --revoke")
-
-    add("-e", "--agree-tos", dest="eula", action="store_true",
-        help="Skip the end user license agreement screen.")
-    add("-t", "--text", dest="use_curses", action="store_false",
-        help="Use the text output instead of the curses UI.")
 
+def paths_parser(parser):
+    add = parser.add_argument
     add("--config-dir", default=constants.DEFAULT_CONFIG_DIR,
         help=config_help("config_dir"))
     add("--work-dir", default=constants.DEFAULT_WORK_DIR,
@@ -318,6 +320,13 @@ def create_parser():
     add("--chain-path", default=constants.DEFAULT_CHAIN_PATH,
         help=config_help("chain_path"))
 
+    return parser
+
+
+def apache_parser(parser):
+    # TODO: this should probably be moved to plugins/apache, in
+    # general all plugins should be able to inject config options
+    add = parser.add_argument
     add("--apache-server-root", default=constants.DEFAULT_APACHE_SERVER_ROOT,
         help=config_help("apache_server_root"))
     add("--apache-mod-ssl-conf", default=constants.DEFAULT_APACHE_MOD_SSL_CONF,
@@ -328,35 +337,41 @@ def create_parser():
         help=config_help("apache_enmod"))
     add("--apache-init-script", default=constants.DEFAULT_APACHE_INIT_SCRIPT,
         help=config_help("apache_init_script"))
-
     return parser
 
 
-def main():  # pylint: disable=too-many-branches, too-many-statements
+def main(args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     # note: arg parser internally handles --help (and exits afterwards)
-    args = create_parser().parse_args()
+    args = create_parser().parse_args(args)
     config = configuration.NamespaceConfig(args)
 
-    # note: check is done after arg parsing as --help should work w/o root also.
-    #if not os.geteuid() == 0:
-    #    return (
-    #        "{0}Root is required to run letsencrypt.  Please use sudo.{0}"
-    #        .format(os.linesep))
-
-    # Set up logging
-    level = -args.verbose_count * 10
-    logger = logging.getLogger()
-    logger.setLevel(level)
-    logging.debug("Logging level set at %d", level)
-    # displayer
+    # Displayer
     if args.use_curses:
-        logger.addHandler(log.DialogHandler())
         displayer = display_util.NcursesDisplay()
     else:
         displayer = display_util.FileDisplay(sys.stdout)
     zope.component.provideUtility(displayer)
 
+    # Logging
+    level = -args.verbose_count * 10
+    logger = logging.getLogger()
+    logger.setLevel(level)
+    logging.debug("Logging level set at %d", level)
+    if args.use_curses:
+        logger.addHandler(log.DialogHandler())
+
+    if not os.geteuid() == 0:
+        logging.warning(
+            "Root (sudo) is required to run most of letsencrypt functionality.")
+        # check must be done after arg parsing as --help should work
+        # w/o root; on the other hand, e.g. "letsencrypt run
+        # --authenticator dns" or "letsencrypt plugins" does not
+        # require root as well
+        #return (
+        #    "{0}Root is required to run letsencrypt.  Please use sudo.{0}"
+        #    .format(os.linesep))
+
     return args.func(args, config)
 
 

From 88dc56186ef931a998e9741bd1f8c9e08a18514e Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 08:32:34 +0000
Subject: [PATCH 09/43] Move apache constants/args/IConfig to its subdirectory

---
 letsencrypt/client/cli.py                     | 27 +++++++++----------
 letsencrypt/client/constants.py               | 15 -----------
 letsencrypt/client/interfaces.py              | 12 ---------
 .../client/plugins/apache/configurator.py     | 26 +++++++++++++-----
 .../client/plugins/apache/constants.py        | 20 ++++++++++++++
 .../client/plugins/apache/tests/util.py       |  4 +--
 6 files changed, 54 insertions(+), 50 deletions(-)
 create mode 100644 letsencrypt/client/plugins/apache/constants.py

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index f49918bf2..221c51969 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -27,6 +27,8 @@ from letsencrypt.client.display import ops as display_ops
 
 from letsencrypt.client.plugins import disco as plugins_disco
 
+from letsencrypt.client.plugins.apache import configurator as apache_configurator
+
 
 def _common_run(args, config, authenticator, installer):
     if args.domains is None:
@@ -296,7 +298,11 @@ def create_parser():
         help="Revert configuration N number of checkpoints.")
 
     paths_parser(parser.add_argument_group("paths"))
-    apache_parser(parser.add_argument_group("apache"))
+
+    # TODO: plugin_parser should be called for every detected plugin
+    plugin_parser(
+        parser.add_argument_group("apache"), prefix="apache",
+        plugin_cls=apache_configurator.ApacheConfigurator)
     return parser
 
 
@@ -323,20 +329,11 @@ def paths_parser(parser):
     return parser
 
 
-def apache_parser(parser):
-    # TODO: this should probably be moved to plugins/apache, in
-    # general all plugins should be able to inject config options
-    add = parser.add_argument
-    add("--apache-server-root", default=constants.DEFAULT_APACHE_SERVER_ROOT,
-        help=config_help("apache_server_root"))
-    add("--apache-mod-ssl-conf", default=constants.DEFAULT_APACHE_MOD_SSL_CONF,
-        help=config_help("apache_mod_ssl_conf"))
-    add("--apache-ctl", default=constants.DEFAULT_APACHE_CTL,
-        help=config_help("apache_ctl"))
-    add("--apache-enmod", default=constants.DEFAULT_APACHE_ENMOD,
-        help=config_help("apache_enmod"))
-    add("--apache-init-script", default=constants.DEFAULT_APACHE_INIT_SCRIPT,
-        help=config_help("apache_init_script"))
+def plugin_parser(parser, prefix, plugin_cls):
+    def add(arg_name_no_prefix, *args, **kwargs):
+        parser.add_argument(
+            "--{0}-{1}".format(prefix, arg_name_no_prefix), *args, **kwargs)
+    plugin_cls.add_parser_arguments(add)
     return parser
 
 
diff --git a/letsencrypt/client/constants.py b/letsencrypt/client/constants.py
index 5c13ea1a3..6c8519196 100644
--- a/letsencrypt/client/constants.py
+++ b/letsencrypt/client/constants.py
@@ -22,11 +22,6 @@ DEFAULT_KEY_DIR = "/etc/letsencrypt/keys"
 DEFAULT_CERTS_DIR = "/etc/letsencrypt/certs"
 DEFAULT_CERT_PATH = "/etc/letsencrypt/certs/cert-letsencrypt.pem"
 DEFAULT_CHAIN_PATH = "/etc/letsencrypt/certs/chain-letsencrypt.pem"
-DEFAULT_APACHE_SERVER_ROOT = "/etc/apache2"
-DEFAULT_APACHE_MOD_SSL_CONF = "/etc/letsencrypt/options-ssl.conf"
-DEFAULT_APACHE_CTL = "apache2ctl"
-DEFAULT_APACHE_ENMOD = "a2enmod"
-DEFAULT_APACHE_INIT_SCRIPT = "/etc/init.d/apache2"
 
 
 S_SIZE = 32
@@ -55,16 +50,6 @@ List of expected options parameters:
 """
 
 
-APACHE_MOD_SSL_CONF = pkg_resources.resource_filename(
-    "letsencrypt.client.plugins.apache", "options-ssl.conf")
-"""Path to the Apache mod_ssl config file found in the Let's Encrypt
-distribution."""
-
-APACHE_REWRITE_HTTPS_ARGS = [
-    "^.*$", "https://%{SERVER_NAME}%{REQUEST_URI}", "[L,R=permanent]"]
-"""Apache rewrite rule arguments used for redirections to https vhost"""
-
-
 DVSNI_CHALLENGE_PORT = 443
 """Port to perform DVSNI challenge."""
 
diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py
index 2390330b6..80a43f885 100644
--- a/letsencrypt/client/interfaces.py
+++ b/letsencrypt/client/interfaces.py
@@ -117,18 +117,6 @@ class IConfig(zope.interface.Interface):
     cert_path = zope.interface.Attribute("Let's Encrypt certificate file.")
     chain_path = zope.interface.Attribute("Let's Encrypt chain file.")
 
-    apache_server_root = zope.interface.Attribute(
-        "Apache server root directory.")
-    apache_ctl = zope.interface.Attribute(
-        "Path to the 'apache2ctl' binary, used for 'configtest' and "
-        "retrieving Apache2 version number.")
-    apache_enmod = zope.interface.Attribute(
-        "Path to the Apache 'a2enmod' binary.")
-    apache_init_script = zope.interface.Attribute(
-        "Path to the Apache init script (used for server reload/restart).")
-    apache_mod_ssl_conf = zope.interface.Attribute(
-        "Contains standard Apache SSL directives.")
-
 
 class IInstaller(IPlugin):
     """Generic Let's Encrypt Installer Interface.
diff --git a/letsencrypt/client/plugins/apache/configurator.py b/letsencrypt/client/plugins/apache/configurator.py
index e6104a559..55f7e2875 100644
--- a/letsencrypt/client/plugins/apache/configurator.py
+++ b/letsencrypt/client/plugins/apache/configurator.py
@@ -13,11 +13,11 @@ from letsencrypt.acme import challenges
 
 from letsencrypt.client import achallenges
 from letsencrypt.client import augeas_configurator
-from letsencrypt.client import constants
 from letsencrypt.client import errors
 from letsencrypt.client import interfaces
 from letsencrypt.client import le_util
 
+from letsencrypt.client.plugins.apache import constants
 from letsencrypt.client.plugins.apache import dvsni
 from letsencrypt.client.plugins.apache import obj
 from letsencrypt.client.plugins.apache import parser
@@ -82,6 +82,20 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
     description = "Apache Web Server"
 
+    @classmethod
+    def add_parser_arguments(cls, add):
+        add("server-root", default=constants.DEFAULT_SERVER_ROOT,
+            help="Apache server root directory.")
+        add("mod-ssl-conf", default=constants.DEFAULT_MOD_SSL_CONF,
+            help="Contains standard Apache SSL directives.")
+        add("ctl", default=constants.DEFAULT_CTL,
+            help="Path to the 'apache2ctl' binary, used for 'configtest' and "
+                 "retrieving Apache2 version number.")
+        add("enmod", default=constants.DEFAULT_ENMOD,
+            help="Path to the Apache 'a2enmod' binary.")
+        add("init-script", default=constants.DEFAULT_INIT_SCRIPT,
+            help="Path to the Apache init script (used for server reload/restart).")
+
     def __init__(self, config, version=None):
         """Initialize an Apache Configurator.
 
@@ -599,7 +613,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             # Add directives to server
             self.parser.add_dir(general_v.path, "RewriteEngine", "On")
             self.parser.add_dir(general_v.path, "RewriteRule",
-                                constants.APACHE_REWRITE_HTTPS_ARGS)
+                                constants.REWRITE_HTTPS_ARGS)
             self.save_notes += ("Redirecting host in %s to ssl vhost in %s\n" %
                                 (general_v.filep, ssl_vhost.filep))
             self.save()
@@ -638,10 +652,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         if not rewrite_path:
             # "No existing redirection for virtualhost"
             return False, -1
-        if len(rewrite_path) == len(constants.APACHE_REWRITE_HTTPS_ARGS):
+        if len(rewrite_path) == len(constants.REWRITE_HTTPS_ARGS):
             for idx, match in enumerate(rewrite_path):
                 if (self.aug.get(match) !=
-                        constants.APACHE_REWRITE_HTTPS_ARGS[idx]):
+                        constants.REWRITE_HTTPS_ARGS[idx]):
                     # Not a letsencrypt https rewrite
                     return True, 2
             # Existing letsencrypt https rewrite rule is in place
@@ -693,7 +707,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                          "LogLevel warn\n"
                          "</VirtualHost>\n"
                          % (servername, serveralias,
-                            " ".join(constants.APACHE_REWRITE_HTTPS_ARGS)))
+                            " ".join(constants.REWRITE_HTTPS_ARGS)))
 
         # Write out the file
         # This is the default name
@@ -1160,4 +1174,4 @@ def temp_install(options_ssl):
 
     # Check to make sure options-ssl.conf is installed
     if not os.path.isfile(options_ssl):
-        shutil.copyfile(constants.APACHE_MOD_SSL_CONF, options_ssl)
+        shutil.copyfile(constants.MOD_SSL_CONF, options_ssl)
diff --git a/letsencrypt/client/plugins/apache/constants.py b/letsencrypt/client/plugins/apache/constants.py
new file mode 100644
index 000000000..63b3bd148
--- /dev/null
+++ b/letsencrypt/client/plugins/apache/constants.py
@@ -0,0 +1,20 @@
+"""Apache plugin constants."""
+import pkg_resources
+
+
+# CLI/IConfig defaults
+DEFAULT_SERVER_ROOT = "/etc/apache2"
+DEFAULT_MOD_SSL_CONF = "/etc/letsencrypt/options-ssl.conf"
+DEFAULT_CTL = "apache2ctl"
+DEFAULT_ENMOD = "a2enmod"
+DEFAULT_INIT_SCRIPT = "/etc/init.d/apache2"
+
+
+MOD_SSL_CONF = pkg_resources.resource_filename(
+    "letsencrypt.client.plugins.apache", "options-ssl.conf")
+"""Path to the Apache mod_ssl config file found in the Let's Encrypt
+distribution."""
+
+REWRITE_HTTPS_ARGS = [
+    "^.*$", "https://%{SERVER_NAME}%{REQUEST_URI}", "[L,R=permanent]"]
+"""Apache rewrite rule arguments used for redirections to https vhost"""
diff --git a/letsencrypt/client/plugins/apache/tests/util.py b/letsencrypt/client/plugins/apache/tests/util.py
index d1ba17f5a..feeb9490e 100644
--- a/letsencrypt/client/plugins/apache/tests/util.py
+++ b/letsencrypt/client/plugins/apache/tests/util.py
@@ -7,8 +7,8 @@ import unittest
 
 import mock
 
-from letsencrypt.client import constants
 from letsencrypt.client.plugins.apache import configurator
+from letsencrypt.client.plugins.apache import constants
 from letsencrypt.client.plugins.apache import obj
 
 
@@ -49,7 +49,7 @@ def dir_setup(test_dir="debian_apache_2_4/two_vhost_80"):
 def setup_apache_ssl_options(config_dir):
     """Move the ssl_options into position and return the path."""
     option_path = os.path.join(config_dir, "options-ssl.conf")
-    shutil.copyfile(constants.APACHE_MOD_SSL_CONF, option_path)
+    shutil.copyfile(constants.MOD_SSL_CONF, option_path)
     return option_path
 
 

From 4f0d0936af099321f581262c1b956fd10db76df5 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 08:44:51 +0000
Subject: [PATCH 10/43] IPluginFactory

---
 letsencrypt/client/interfaces.py              | 20 +++++++++++++++++++
 .../client/plugins/apache/configurator.py     |  1 +
 .../plugins/standalone/authenticator.py       |  5 +++++
 3 files changed, 26 insertions(+)

diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py
index 80a43f885..3ce0e990a 100644
--- a/letsencrypt/client/interfaces.py
+++ b/letsencrypt/client/interfaces.py
@@ -5,6 +5,26 @@ import zope.interface
 # pylint: disable=too-few-public-methods
 
 
+class IPluginFactory(zope.interface.Interface):
+
+    def __call__(config):
+        """Create new `IPlugin`.
+
+        :param IConfig config: Configuration.
+
+        """
+
+    def add_parser_arguments(add):
+        """Add plugin arguments to the CLI argument parser.
+
+        :param callable add: Function that proxies calls to
+            `argparse.ArgumentParser.add_argument` prepending options
+            with unique plugin name prefix.
+
+        """
+        # TODO: move to IPlugin?
+
+
 class IPlugin(zope.interface.Interface):
     """Let's Encrypt plugin."""
 
diff --git a/letsencrypt/client/plugins/apache/configurator.py b/letsencrypt/client/plugins/apache/configurator.py
index 55f7e2875..34a57b2b0 100644
--- a/letsencrypt/client/plugins/apache/configurator.py
+++ b/letsencrypt/client/plugins/apache/configurator.py
@@ -79,6 +79,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
     """
     zope.interface.implements(interfaces.IAuthenticator, interfaces.IInstaller)
+    zope.interface.classProvides(interfaces.IPluginFactory)
 
     description = "Apache Web Server"
 
diff --git a/letsencrypt/client/plugins/standalone/authenticator.py b/letsencrypt/client/plugins/standalone/authenticator.py
index e0b06aa30..fa4e62d7f 100644
--- a/letsencrypt/client/plugins/standalone/authenticator.py
+++ b/letsencrypt/client/plugins/standalone/authenticator.py
@@ -30,9 +30,14 @@ class StandaloneAuthenticator(object):
 
     """
     zope.interface.implements(interfaces.IAuthenticator)
+    zope.interface.classProvides(interfaces.IPluginFactory)
 
     description = "Standalone Authenticator"
 
+    @classmethod
+    def add_parser_arguments(cls, add):
+        pass
+
     def __init__(self, unused_config):
         self.child_pid = None
         self.parent_pid = os.getpid()

From cfe95323f6c8a51fedbaf47efc8e7f4b96ba3e89 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 09:02:38 +0000
Subject: [PATCH 11/43] Revert "Unit tests for setting authenticator via cmd
 line"

This reverts commit 0d7f32fa984e2e82918d644dfe6913bfe765055f.
---
 letsencrypt/client/client.py            |  7 ++--
 letsencrypt/client/tests/client_test.py | 50 +++++++------------------
 letsencrypt/scripts/main.py             |  5 ++-
 3 files changed, 21 insertions(+), 41 deletions(-)

diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py
index 91b271784..19b982502 100644
--- a/letsencrypt/client/client.py
+++ b/letsencrypt/client/client.py
@@ -397,10 +397,11 @@ def determine_authenticator(all_auths, config):
         try:
             auth = avail_auths[config.authenticator]
         except KeyError:
-            logging.info(list_available_authenticators(avail_auths))
-            raise errors.LetsEncryptClientError(
-                "The specified authenticator '%s' could not be found" %
+            logging.error(
+                "The specified authenticator '%s' could not be found",
                 config.authenticator)
+            logging.info(list_available_authenticators(avail_auths))
+            return
     elif len(avail_auths) > 1:
         auth = display_ops.choose_authenticator(avail_auths.values(), errs)
     elif len(avail_auths.keys()) == 1:
diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py
index 63170b517..2310dbe87 100644
--- a/letsencrypt/client/tests/client_test.py
+++ b/letsencrypt/client/tests/client_test.py
@@ -1,9 +1,9 @@
 """letsencrypt.client.client.py tests."""
+from collections import namedtuple
 import unittest
 
 import mock
 
-from letsencrypt.client import configuration
 from letsencrypt.client import errors
 
 
@@ -19,8 +19,7 @@ class DetermineAuthenticatorTest(unittest.TestCase):
         self.mock_apache = mock.MagicMock(
             spec=ApacheConfigurator, description="Standalone Authenticator")
 
-        self.mock_config = mock.MagicMock(
-            spec=configuration.NamespaceConfig, authenticator=None)
+        self.mock_config = mock.Mock()
 
         self.all_auths = {
             'apache': self.mock_apache,
@@ -28,30 +27,29 @@ class DetermineAuthenticatorTest(unittest.TestCase):
         }
 
     @classmethod
-    def _call(cls, all_auths, config):
+    def _call(cls, all_auths):
         from letsencrypt.client.client import determine_authenticator
-        return determine_authenticator(all_auths, config)
+        # TODO: add tests for setting the authenticator via the command line
+        mock_config = namedtuple("Config", ['authenticator'])
+        return determine_authenticator(all_auths,
+                                       mock_config(authenticator=None))
 
     @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_config),
-                         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
-        one_avail_auth = {
-            'apache': self.mock_apache
-        }
-        self.assertEqual(self._call(one_avail_auth, self.mock_config),
-                         self.mock_apache)
+        self.assertEqual(
+            self._call(dict(apache=self.all_auths['apache'])),
+            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_config),
-                         self.mock_stand)
+        self.assertEqual(self._call(self.all_auths), self.mock_stand)
 
     def test_no_installations(self):
         self.mock_apache.prepare.side_effect = (
@@ -61,8 +59,7 @@ class DetermineAuthenticatorTest(unittest.TestCase):
 
         self.assertRaises(errors.LetsEncryptClientError,
                           self._call,
-                          self.all_auths,
-                          self.mock_config)
+                          self.all_auths)
 
     @mock.patch("letsencrypt.client.client.logging")
     @mock.patch("letsencrypt.client.client.display_ops.choose_authenticator")
@@ -71,26 +68,7 @@ class DetermineAuthenticatorTest(unittest.TestCase):
             errors.LetsEncryptMisconfigurationError)
         mock_choose.return_value = self.mock_apache
 
-        self.assertTrue(self._call(self.all_auths, self.mock_config) is None)
-
-    def test_choose_valid_auth_from_cmd_line(self):
-        standalone_config = mock.MagicMock(spec=configuration.NamespaceConfig,
-                                           authenticator='standalone')
-        self.assertEqual(self._call(self.all_auths, standalone_config),
-                         self.mock_stand)
-
-        apache_config = mock.MagicMock(spec=configuration.NamespaceConfig,
-                                       authenticator='apache')
-        self.assertEqual(self._call(self.all_auths, apache_config),
-                         self.mock_apache)
-
-    def test_choose_invalid_auth_from_cmd_line(self):
-        invalid_config = mock.MagicMock(spec=configuration.NamespaceConfig,
-                                        authenticator='foobar')
-        self.assertRaises(errors.LetsEncryptClientError,
-                          self._call,
-                          self.all_auths,
-                          invalid_config)
+        self.assertTrue(self._call(self.all_auths) is None)
 
 
 class RollbackTest(unittest.TestCase):
diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py
index 9da8c30b0..ae8eafc47 100644
--- a/letsencrypt/scripts/main.py
+++ b/letsencrypt/scripts/main.py
@@ -178,8 +178,9 @@ def main():  # pylint: disable=too-many-branches, too-many-statements
     try:
         auth = client.determine_authenticator(all_auths, config)
         logging.debug("Selected authenticator: %s", auth)
-    except errors.LetsEncryptClientError as err:
-        logging.critical(str(err))
+    except errors.LetsEncryptClientError:
+        logging.critical("No authentication mechanisms were found on your "
+                         "system.")
         sys.exit(1)
 
     if auth is None:

From b76e8b6c412ab48de3d57b09013d58af152f6030 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 09:02:39 +0000
Subject: [PATCH 12/43] Revert "Update unit tests for determine_authenticator"

This reverts commit 79f5ebe734d18ddbc70dfbd22de4ce76f995a20a.
---
 .gitignore                              |  2 +-
 letsencrypt/client/tests/client_test.py | 14 +++-----------
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/.gitignore b/.gitignore
index 51164db97..2e0578223 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,4 +10,4 @@ m3
 *~
 .vagrant
 *.swp
-\#*#
+\#*#
\ No newline at end of file
diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py
index 2310dbe87..1c1a0d68a 100644
--- a/letsencrypt/client/tests/client_test.py
+++ b/letsencrypt/client/tests/client_test.py
@@ -1,5 +1,4 @@
 """letsencrypt.client.client.py tests."""
-from collections import namedtuple
 import unittest
 
 import mock
@@ -21,18 +20,12 @@ class DetermineAuthenticatorTest(unittest.TestCase):
 
         self.mock_config = mock.Mock()
 
-        self.all_auths = {
-            'apache': self.mock_apache,
-            'standalone': self.mock_stand
-        }
+        self.all_auths = [self.mock_apache, self.mock_stand]
 
     @classmethod
     def _call(cls, all_auths):
         from letsencrypt.client.client import determine_authenticator
-        # TODO: add tests for setting the authenticator via the command line
-        mock_config = namedtuple("Config", ['authenticator'])
-        return determine_authenticator(all_auths,
-                                       mock_config(authenticator=None))
+        return determine_authenticator(all_auths)
 
     @mock.patch("letsencrypt.client.client.display_ops.choose_authenticator")
     def test_accept_two(self, mock_choose):
@@ -42,8 +35,7 @@ class DetermineAuthenticatorTest(unittest.TestCase):
     def test_accept_one(self):
         self.mock_apache.prepare.return_value = self.mock_apache
         self.assertEqual(
-            self._call(dict(apache=self.all_auths['apache'])),
-            self.mock_apache)
+            self._call(self.all_auths[:1]), self.mock_apache)
 
     def test_no_installation_one(self):
         self.mock_apache.prepare.side_effect = (

From f26549dc58016c004d3dbcfc56b2588ea4403957 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Wed, 22 Apr 2015 09:02:41 +0000
Subject: [PATCH 13/43] Revert "Add cmd line arg for the authenticator"

This reverts commit 5d2abc30f0700366196cebb39a5a1a2275fb9d01.
---
 letsencrypt/client/client.py     | 46 +++++++-------------------------
 letsencrypt/client/interfaces.py |  6 -----
 letsencrypt/scripts/main.py      | 15 +++--------
 3 files changed, 13 insertions(+), 54 deletions(-)

diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py
index 19b982502..2fcb45d40 100644
--- a/letsencrypt/client/client.py
+++ b/letsencrypt/client/client.py
@@ -349,29 +349,13 @@ def init_csr(privkey, names, cert_dir):
     return le_util.CSR(csr_filename, csr_der, "der")
 
 
-def list_available_authenticators(avail_auths):
-    """Return a pretty-printed list of authenticators.
-
-    This is used to provide helpful feedback in the case where a user
-    specifies an invalid authenticator on the command line.
-
-    """
-    output_lines = ["Available authenticators:"]
-    for auth_name, auth in avail_auths.iteritems():
-        output_lines.append(" - %s : %s" % (auth_name, auth.description))
-    return '\n'.join(output_lines)
-
-
 # This should be controlled by commandline parameters
-def determine_authenticator(all_auths, config):
+def determine_authenticator(all_auths):
     """Returns a valid IAuthenticator.
 
     :param list all_auths: Where each is a
         :class:`letsencrypt.client.interfaces.IAuthenticator` object
 
-    :param config: Used if an authenticator was specified on the command line.
-    :type config: :class:`letsencrypt.client.interfaces.IConfig`
-
     :returns: Valid Authenticator object or None
 
     :raises letsencrypt.client.errors.LetsEncryptClientError: If no
@@ -379,33 +363,23 @@ def determine_authenticator(all_auths, config):
 
     """
     # Available Authenticator objects
-    avail_auths = {}
+    avail_auths = []
     # Error messages for misconfigured authenticators
     errs = {}
 
-    for auth_name, auth in all_auths.iteritems():
+    for pot_auth in all_auths:
         try:
-            auth.prepare()
+            pot_auth.prepare()
         except errors.LetsEncryptMisconfigurationError as err:
-            errs[auth] = err
+            errs[pot_auth] = err
         except errors.LetsEncryptNoInstallationError:
             continue
-        avail_auths[auth_name] = auth
+        avail_auths.append(pot_auth)
 
-    # If an authenticator was specified on the command line, try to use it
-    if config.authenticator:
-        try:
-            auth = avail_auths[config.authenticator]
-        except KeyError:
-            logging.error(
-                "The specified authenticator '%s' could not be found",
-                config.authenticator)
-            logging.info(list_available_authenticators(avail_auths))
-            return
-    elif len(avail_auths) > 1:
-        auth = display_ops.choose_authenticator(avail_auths.values(), errs)
-    elif len(avail_auths.keys()) == 1:
-        auth = avail_auths[avail_auths.keys()[0]]
+    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.")
 
diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py
index 3d3001377..9c0e5553d 100644
--- a/letsencrypt/client/interfaces.py
+++ b/letsencrypt/client/interfaces.py
@@ -13,10 +13,6 @@ class IAuthenticator(zope.interface.Interface):
 
     """
 
-    description = zope.interface.Attribute(
-        "Short description of this authenticator. "
-        "Used in interactive configuration.")
-
     def prepare():
         """Prepare the authenticator.
 
@@ -93,8 +89,6 @@ class IConfig(zope.interface.Interface):
     server = zope.interface.Attribute(
         "CA hostname (and optionally :port). The server certificate must "
         "be trusted in order to avoid further modifications to the client.")
-    authenticator = zope.interface.Attribute(
-        "Authenticator to use for responding to challenges.")
     rsa_key_size = zope.interface.Attribute("Size of the RSA key.")
 
     config_dir = zope.interface.Attribute("Configuration directory.")
diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py
index ae8eafc47..1b50e2cda 100644
--- a/letsencrypt/scripts/main.py
+++ b/letsencrypt/scripts/main.py
@@ -32,8 +32,6 @@ SETUPTOOLS_AUTHENTICATORS_ENTRY_POINT = "letsencrypt.authenticators"
 
 def init_auths(config):
     """Find (setuptools entry points) and initialize Authenticators."""
-    # TODO: handle collisions in authenticator names. Or is this
-    # already handled for us by pkg_resources?
     auths = {}
     for entrypoint in pkg_resources.iter_entry_points(
             SETUPTOOLS_AUTHENTICATORS_ENTRY_POINT):
@@ -46,7 +44,7 @@ def init_auths(config):
                 "%r object does not provide IAuthenticator, skipping",
                 entrypoint.name)
         else:
-            auths[entrypoint.name] = auth
+            auths[auth] = entrypoint.name
     return auths
 
 
@@ -62,12 +60,6 @@ def create_parser():
     add("-s", "--server", default="letsencrypt-demo.org:443",
         help=config_help("server"))
 
-    # TODO: we should generate the list of choices from the set of
-    # available authenticators, but that is tricky due to the
-    # dependency between init_auths and config. Hardcoding it for now.
-    add("-a", "--authenticator", dest="authenticator",
-        help=config_help("authenticator"))
-
     add("-k", "--authkey", type=read_file,
         help="Path to the authorized key file")
     add("-B", "--rsa-key-size", type=int, default=2048, metavar="N",
@@ -174,10 +166,9 @@ def main():  # pylint: disable=too-many-branches, too-many-statements
         display_eula()
 
     all_auths = init_auths(config)
-    logging.debug('Initialized authenticators: %s', all_auths.keys())
+    logging.debug('Initialized authenticators: %s', all_auths.values())
     try:
-        auth = client.determine_authenticator(all_auths, config)
-        logging.debug("Selected authenticator: %s", auth)
+        auth = client.determine_authenticator(all_auths.keys())
     except errors.LetsEncryptClientError:
         logging.critical("No authentication mechanisms were found on your "
                          "system.")

From 17e8ddcb5cca97d6f3568ebbcc3cd755363b59da Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Fri, 1 May 2015 20:19:26 +0000
Subject: [PATCH 14/43] Assert CLI test (--help) raises SystemExit

---
 letsencrypt/client/tests/cli_test.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/letsencrypt/client/tests/cli_test.py b/letsencrypt/client/tests/cli_test.py
index 8afede3bc..bb50715e5 100644
--- a/letsencrypt/client/tests/cli_test.py
+++ b/letsencrypt/client/tests/cli_test.py
@@ -5,6 +5,7 @@ class CLITest(unittest.TestCase):
 
     def test_it(self):
         from letsencrypt.client import cli
+        self.assertRaises(SystemExit, cli.main, ['--help'])
 
 
 if __name__ == '__main__':

From 19cff0083556288fb64b4dbb38d2f3a7949d43e4 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 2 May 2015 07:01:44 +0000
Subject: [PATCH 15/43] common.Plugin (with .conf(var), PluginEntryPoint,
 PluginRegistry

---
 .../plugins/letsencrypt_example_plugins.py    |  21 ++--
 examples/plugins/setup.py                     |   3 +-
 letsencrypt/client/augeas_configurator.py     |   9 +-
 letsencrypt/client/cli.py                     |  65 +++++------
 letsencrypt/client/interfaces.py              |  51 +++++++--
 .../client/plugins/apache/configurator.py     |  29 +++--
 .../client/plugins/apache/tests/util.py       |   5 +-
 letsencrypt/client/plugins/common.py          |  62 +++++++++++
 letsencrypt/client/plugins/disco.py           | 101 +++++++++++++-----
 .../client/plugins/nginx/configurator.py      |  25 +++--
 .../client/plugins/nginx/tests/util.py        |   5 +-
 .../plugins/standalone/authenticator.py       |  13 +--
 .../standalone/tests/authenticator_test.py    |  24 ++---
 13 files changed, 278 insertions(+), 135 deletions(-)
 create mode 100644 letsencrypt/client/plugins/common.py

diff --git a/examples/plugins/letsencrypt_example_plugins.py b/examples/plugins/letsencrypt_example_plugins.py
index 987a2b33b..11baf35a7 100644
--- a/examples/plugins/letsencrypt_example_plugins.py
+++ b/examples/plugins/letsencrypt_example_plugins.py
@@ -1,18 +1,27 @@
-"""Example Let's Encrypt plugins."""
+"""Example Let's Encrypt plugins.
+
+For full examples, see `letsencrypt.client.plugins`.
+
+"""
 import zope.interface
 
 from letsencrypt.client import interfaces
+from letsencrypt.client.plugins import common
 
 
-class Authenticator(object):
+class Authenticator(common.Plugin):
     zope.interface.implements(interfaces.IAuthenticator)
 
     description = 'Example Authenticator plugin'
 
-    def __init__(self, config):
-        self.config = config
-
     # Implement all methods from IAuthenticator, remembering to add
     # "self" as first argument, e.g. def prepare(self)...
 
-    # For full examples, see letsencrypt.client.plugins
+
+class Installer(common.Plugins):
+    zope.interface.implements(interfaces.IInstaller)
+
+    description = 'Example Installer plugin'
+
+    # Implement all methods from IInstaller, remembering to add
+    # "self" as first argument, e.g. def get_all_names(self)...
diff --git a/examples/plugins/setup.py b/examples/plugins/setup.py
index 599d57020..71bb95333 100644
--- a/examples/plugins/setup.py
+++ b/examples/plugins/setup.py
@@ -10,7 +10,8 @@ setup(
     ],
     entry_points={
         'letsencrypt.plugins': [
-            'example = letsencrypt_example_plugins:Authenticator',
+            'example_authenticator = letsencrypt_example_plugins:Authenticator',
+            'example_installer = letsencrypt_example_plugins:Installer',
         ],
     },
 )
diff --git a/letsencrypt/client/augeas_configurator.py b/letsencrypt/client/augeas_configurator.py
index 8854fef09..713d49291 100644
--- a/letsencrypt/client/augeas_configurator.py
+++ b/letsencrypt/client/augeas_configurator.py
@@ -4,9 +4,10 @@ import logging
 import augeas
 
 from letsencrypt.client import reverter
+from letsencrypt.client.plugins import common
 
 
-class AugeasConfigurator(object):
+class AugeasConfigurator(common.Plugin):
     """Base Augeas Configurator class.
 
     :ivar config: Configuration.
@@ -21,8 +22,8 @@ class AugeasConfigurator(object):
 
     """
 
-    def __init__(self, config):
-        self.config = config
+    def __init__(self, *args, **kwargs):
+        super(AugeasConfigurator, self).__init__(*args, **kwargs)
 
         # Set Augeas flags to not save backup (we do it ourselves)
         # Set Augeas to not load anything by default
@@ -34,7 +35,7 @@ class AugeasConfigurator(object):
         # This needs to occur before VirtualHost objects are setup...
         # because this will change the underlying configuration and potential
         # vhosts
-        self.reverter = reverter.Reverter(config)
+        self.reverter = reverter.Reverter(self.config)
         self.reverter.recovery_routine()
 
     def check_parsing_errors(self, lens):
diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index 3a8faf481..148562e4c 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -171,48 +171,49 @@ def config_changes(args, config):
     client.config_changes(config)
 
 
-def _print_plugins(filtered, plugins, names):
-    if not filtered:
+def _print_plugins(plugins):
+    # TODO: this functions should use IDisplay rather than printing
+
+    if not plugins:
         print "No plugins found"
 
-    for plugin_cls, content in filtered.iteritems():
-        print "* {0}".format(names[plugin_cls])
-        print "Description: {0}".format(plugin_cls.description)
+    for plugin_ep in plugins.itervalues():
+        print "* {0}".format(plugin_ep.name)
+        print "Description: {0}".format(plugin_ep.plugin_cls.description)
         print "Interfaces: {0}".format(", ".join(
             iface.__name__ for iface in zope.interface.implementedBy(
-                plugin_cls)))
-        print "Entry points:"
-        for entry_point in plugins[plugin_cls]:
-            print "- {0.dist}: {0}".format(entry_point)
+                plugin_ep.plugin_cls)))
+        print "Entry point: {0}".format(plugin_ep.entry_point)
+
+        if plugin_ep.initialized:
+            print "Initialized: {0}".format(plugin_ep.init())
 
         # if filtered == prepared:
-        if isinstance(content, tuple) and content[1] is not None:
-            print content[1]  # error
-        print
+        #if isinstance(content, tuple) and content[1] is not None:
+        #    print content[1]  # error
+
+        print  # whitespace between plugins
 
 
 def plugins(args, config):
     """List plugins."""
-    plugins = plugins_disco.find_plugins()
+    plugins = plugins_disco.PluginRegistry.find_all()
     logging.debug("Discovered plugins: %s", plugins)
 
-    names = plugins_disco.name_plugins(plugins)
-
     ifaces = [] if args.ifaces is None else args.ifaces
-    filtered = plugins_disco.filter_plugins(
-        plugins, *((iface,) for iface in ifaces))
+    filtered = plugins.filter(*((iface,) for iface in ifaces))
     logging.debug("Filtered plugins: %s", filtered)
 
     if not args.init and not args.prepare:
-        return _print_plugins(filtered, plugins, names)
+        return _print_plugins(filtered)
 
-    initialized = dict((plugin_cls, plugin_cls(config))
-                       for plugin_cls in filtered)
-    verified = plugins_disco.verify_plugins(initialized, ifaces)
-    logging.debug("Verified plugins: %s", initialized)
+    for plugin_ep in filtered.itervalues():
+        plugin_ep.init(config)
+    #verified = plugins_disco.verify_plugins(initialized, ifaces)
+    #logging.debug("Verified plugins: %s", initialized)
 
     if not args.prepare:
-        return _print_plugins(initialized, plugins, names)
+        return _print_plugins(filtered)
 
     prepared = plugins_disco.prepare_plugins(initialized)
     logging.debug("Prepared plugins: %s", plugins)
@@ -327,12 +328,10 @@ def create_parser():
     paths_parser(parser.add_argument_group("paths"))
 
     # TODO: plugin_parser should be called for every detected plugin
-    plugin_parser(
-        parser.add_argument_group("apache"), prefix="apache",
-        plugin_cls=apache_configurator.ApacheConfigurator)
-    plugin_parser(
-        parser.add_argument_group("nginx"), prefix="nginx",
-        plugin_cls=nginx_configurator.NginxConfigurator)
+    for name, plugin_cls in [
+            ("apache", apache_configurator.ApacheConfigurator),
+            ("nginx", nginx_configurator.NginxConfigurator)]:
+        plugin_cls.inject_parser_options(parser.add_argument_group(name), name)
 
     return parser
 
@@ -360,14 +359,6 @@ def paths_parser(parser):
     return parser
 
 
-def plugin_parser(parser, prefix, plugin_cls):
-    def add(arg_name_no_prefix, *args, **kwargs):
-        parser.add_argument(
-            "--{0}-{1}".format(prefix, arg_name_no_prefix), *args, **kwargs)
-    plugin_cls.add_parser_arguments(add)
-    return parser
-
-
 def main(args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     # note: arg parser internally handles --help (and exits afterwards)
diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py
index 85bfa7256..df8b616a6 100644
--- a/letsencrypt/client/interfaces.py
+++ b/letsencrypt/client/interfaces.py
@@ -6,30 +6,63 @@ import zope.interface
 
 
 class IPluginFactory(zope.interface.Interface):
+    """IPlugin factory.
 
-    def __call__(config):
+    Objects providing this interface will be called without satisfying
+    any entry point "extras" (extra dependencies) you might have defined
+    for your plugin, e.g (excerpt from ``setup.py`` script)::
+
+      setup(
+          ...
+          entry_points={
+              'letsencrypt.plugins': [
+                  'name=example_project.plugin[plugin_deps]',
+              ],
+          },
+          extras_require={
+              'plugin_deps': ['dep1', 'dep2'],
+          }
+      )
+
+    Therefore, make sure such objects are importable and usable without
+    extras. This is necessary, because CLI does the following operations
+    (in order):
+
+      - loads an entry point,
+      - calls `inject_parser_options`,
+      - requires an entry point,
+      - creates plugin instance (`__call__`).
+
+    """
+
+    description = zope.interface.Attribute("Short plugin description")
+
+    def __call__(config, name):
         """Create new `IPlugin`.
 
         :param IConfig config: Configuration.
+        :param str name: Unique plugin name.
 
         """
 
-    def add_parser_arguments(add):
-        """Add plugin arguments to the CLI argument parser.
+    def inject_parser_options(parser, name):
+        """Inject argument parser options (flags).
 
-        :param callable add: Function that proxies calls to
-            `argparse.ArgumentParser.add_argument` prepending options
-            with unique plugin name prefix.
+        1. Be nice and prepend all options and destinations with
+        `~.option_namespace` and `~.dest_namespace`.
+
+        2. Inject options (flags) only. Positional arguments are not
+        allowed, as this would break the CLI.
+
+        :param ArgumentParser parser: (Almost) top-level CLI parser.
+        :param str name: Unique plugin name.
 
         """
-        # TODO: move to IPlugin?
 
 
 class IPlugin(zope.interface.Interface):
     """Let's Encrypt plugin."""
 
-    description = zope.interface.Attribute("Short plugin description")
-
     def prepare():
         """Prepare the plugin.
 
diff --git a/letsencrypt/client/plugins/apache/configurator.py b/letsencrypt/client/plugins/apache/configurator.py
index 26e81e641..d014e6c2e 100644
--- a/letsencrypt/client/plugins/apache/configurator.py
+++ b/letsencrypt/client/plugins/apache/configurator.py
@@ -97,14 +97,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         add("init-script", default=constants.DEFAULT_INIT_SCRIPT,
             help="Path to the Apache init script (used for server reload/restart).")
 
-    def __init__(self, config, version=None):
+    def __init__(self, *args, **kwargs):
         """Initialize an Apache Configurator.
 
         :param tup version: version of Apache as a tuple (2, 4, 7)
             (used mostly for unittesting)
 
         """
-        super(ApacheConfigurator, self).__init__(config)
+        version = kwargs.pop('version', None)
+        super(ApacheConfigurator, self).__init__(*args, **kwargs)
 
         # Verify that all directories and files exist with proper permissions
         if os.geteuid() == 0:
@@ -124,8 +125,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
     def prepare(self):
         """Prepare the authenticator/installer."""
         self.parser = parser.ApacheParser(
-            self.aug, self.config.apache_server_root,
-            self.config.apache_mod_ssl_conf)
+            self.aug, self.conf('server-root'), self.conf('mod-ssl-conf'))
         # Check for errors in parsing files with Augeas
         self.check_parsing_errors("httpd.aug")
 
@@ -143,7 +143,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         #     on initialization
         self._prepare_server_https()
 
-        temp_install(self.config.apache_mod_ssl_conf)
+        temp_install(self.conf('mod-ssl-conf'))
 
     def deploy_cert(self, domain, cert, key, cert_chain=None):
         """Deploys certificate to specified virtual host.
@@ -401,10 +401,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         is appropriately listening on port 443.
 
         """
-        if not mod_loaded("ssl_module", self.config.apache_ctl):
+        if not mod_loaded("ssl_module", self.conf('ctl')):
             logging.info("Loading mod_ssl into Apache Server")
-            enable_mod("ssl", self.config.apache_init_script,
-                       self.config.apache_enmod)
+            enable_mod("ssl", self.conf('init-script'),
+                       self.conf('enmod'))
 
         # Check for Listen 443
         # Note: This could be made to also look for ip:443 combo
@@ -587,9 +587,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             :class:`~letsencrypt.client.plugins.apache.obj.VirtualHost`)
 
         """
-        if not mod_loaded("rewrite_module", self.config.apache_ctl):
-            enable_mod("rewrite", self.config.apache_init_script,
-                       self.config.apache_enmod)
+        if not mod_loaded("rewrite_module", self.conf('ctl')):
+            enable_mod("rewrite", self.conf('init-script'), self.conf('enmod'))
 
         general_v = self._general_vhost(ssl_vhost)
         if general_v is None:
@@ -912,7 +911,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :rtype: bool
 
         """
-        return apache_restart(self.config.apache_init_script)
+        return apache_restart(self.conf('init-script'))
 
     def config_test(self):  # pylint: disable=no-self-use
         """Check the configuration of Apache for errors.
@@ -923,7 +922,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
         try:
             proc = subprocess.Popen(
-                ["sudo", self.config.apache_ctl, "configtest"], # TODO: sudo?
+                ["sudo", self.conf('ctl'), "configtest"], # TODO: sudo?
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE)
             stdout, stderr = proc.communicate()
@@ -970,13 +969,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
         try:
             proc = subprocess.Popen(
-                [self.config.apache_ctl, "-v"],
+                [self.conf('ctl'), "-v"],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE)
             text = proc.communicate()[0]
         except (OSError, ValueError):
             raise errors.LetsEncryptConfiguratorError(
-                "Unable to run %s -v" % self.config.apache_ctl)
+                "Unable to run %s -v" % self.conf('ctl'))
 
         regex = re.compile(r"Apache/([0-9\.]*)", re.IGNORECASE)
         matches = regex.findall(text)
diff --git a/letsencrypt/client/plugins/apache/tests/util.py b/letsencrypt/client/plugins/apache/tests/util.py
index feeb9490e..95c7928dd 100644
--- a/letsencrypt/client/plugins/apache/tests/util.py
+++ b/letsencrypt/client/plugins/apache/tests/util.py
@@ -64,7 +64,7 @@ def get_apache_configurator(
         # This just states that the ssl module is already loaded
         mock_popen().communicate.return_value = ("ssl_module", "")
         config = configurator.ApacheConfigurator(
-            mock.MagicMock(
+            config=mock.MagicMock(
                 apache_server_root=config_path,
                 apache_mod_ssl_conf=ssl_options,
                 le_vhost_ext="-le-ssl.conf",
@@ -73,7 +73,8 @@ def get_apache_configurator(
                 temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"),
                 in_progress_dir=os.path.join(backups, "IN_PROGRESS"),
                 work_dir=work_dir),
-            version)
+            name="apache",
+            version=version)
 
     config.prepare()
 
diff --git a/letsencrypt/client/plugins/common.py b/letsencrypt/client/plugins/common.py
new file mode 100644
index 000000000..ba6efeccc
--- /dev/null
+++ b/letsencrypt/client/plugins/common.py
@@ -0,0 +1,62 @@
+"""Plugin common functions."""
+import zope.interface
+
+from letsencrypt.acme.jose import util as jose_util
+
+from letsencrypt.client import interfaces
+
+
+def option_namespace(name):
+    """ArgumentParser options namespace (prefix of all options)."""
+    return name + '-'
+
+def dest_namespace(name):
+    """ArgumentParser dest namespace (prefix of all destinations)."""
+    return name + '_'
+
+
+class Plugin(object):
+    """Generic plugin."""
+    zope.interface.implements(interfaces.IPlugin)
+    zope.interface.classProvides(interfaces.IPluginFactory)
+
+    def __init__(self, config, name):
+        self.config = config
+        self.name = name
+
+    @property
+    def option_namespace(self):
+        return option_namespace(self.name)
+
+    @property
+    def dest_namespace(self):
+        return dest_namespace(self.name)
+
+    def dest(self, var):
+        """Find a destination for given variable ``var``."""
+        # this should do exactly the same what ArgumentParser(arg),
+        # does to "arg" to compute "dest"
+        return self.dest_namespace + var.replace('-', '_')
+
+    def conf(self, var):
+        """Find a configuration value for variable ``var``."""
+        return getattr(self.config, self.dest(var))
+
+    @classmethod
+    def inject_parser_options(cls, parser, name):
+        # dummy function, doesn't check if dest.startswith(self.dest_namespace)
+        def add(arg_name_no_prefix, *args, **kwargs):
+            return parser.add_argument(
+                "--{0}{1}".format(option_namespace(name), arg_name_no_prefix),
+                *args, **kwargs)
+        cls.add_parser_arguments(add)
+
+    @jose_util.abstractclassmethod
+    def add_parser_arguments(cls, add):
+        """Add plugin arguments to the CLI argument parser.
+
+        :param callable add: Function that proxies calls to
+            `argparse.ArgumentParser.add_argument` prepending options
+            with unique plugin name prefix.
+
+        """
diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index d2526d583..350929641 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -12,52 +12,97 @@ from letsencrypt.client import interfaces
 from letsencrypt.client.display import ops as display_ops
 
 
-def name_plugins(plugins):
-    # TODO: actually make it unambiguous...
-    names = {}
-    for plugin_cls, entry_points in plugins.iteritems():
-        entry_point = next(iter(entry_points))  # entry_points.peek()
-        names[plugin_cls] = entry_point.name
-    return names
+class PluginEntryPoint(object):
+    """Plugin entry point."""
+
+    PREFIX_FREE_DISTRIBUTIONS = ['letsencrypt']
+    """Distributions for which prefix will be omitted."""
+
+    def __init__(self, entry_point):
+        self.name = self.entry_point_to_plugin_name(entry_point)
+        self.plugin_cls = entry_point.load()
+        self.entry_point = entry_point
+        self._initialized = None
+
+    @property
+    def initialized(self):
+        return self._initialized is not None
+
+    @classmethod
+    def entry_point_to_plugin_name(cls, entry_point):
+        if entry_point.dist.key in cls.PREFIX_FREE_DISTRIBUTIONS:
+            return entry_point.name
+        return entry_point.dist.key + ':' + entry_point.name
+
+    def init(self, config=None):
+        """Memoized plugin inititialization."""
+        if not self.initialized:
+            self.entry_point.require()  # fetch extras!
+            self._initialized = self.plugin_cls(config, self.name)
+        return self._initialized
+
+    def __repr__(self):
+        return 'PluginEntryPoint#{0}'.format(self.name)
 
 
-def find_plugins():
-    """Find plugins using setuptools entry points."""
-    plugins = collections.defaultdict(set)
-    for entry_point in pkg_resources.iter_entry_points(
-            constants.SETUPTOOLS_PLUGINS_ENTRY_POINT):
-        plugin_cls = entry_point.load()
-        plugins[plugin_cls].add(entry_point)
-    return plugins
+class PluginRegistry(collections.Mapping):
+    """Plugin registry."""
 
+    def __init__(self, plugins):
+        self.plugins = plugins
 
-def filter_plugins(plugins, *ifaces_groups):
-    """Filter plugins based on interfaces."""
-    return dict(
-        (plugin_cls, entry_points)
-        for plugin_cls, entry_points in plugins.iteritems()
-        if not ifaces_groups or any(
-            all(iface.implementedBy(plugin_cls) for iface in ifaces)
-            for ifaces in ifaces_groups))
+    @classmethod
+    def find_all(cls):
+        """Find plugins using setuptools entry points."""
+        plugins = {}
+        for entry_point in pkg_resources.iter_entry_points(
+                constants.SETUPTOOLS_PLUGINS_ENTRY_POINT):
+            plugin_ep = PluginEntryPoint(entry_point)
+            assert plugin_ep.name not in plugins, (
+                'PREFIX_FREE_DISTRIBTIONS messed up')
+            plugins[plugin_ep.name] = plugin_ep
+        return cls(plugins)
+
+    def filter(self, *ifaces_groups):
+        """Filter plugins based on interfaces."""
+        return type(self)(dict(
+            plugin_ep
+            for plugin_ep in self.plugins.iteritems()
+            if not ifaces_groups or any(
+                all(iface.implementedBy(plugin_ep.plugin_cls)
+                    for iface in ifaces)
+                for ifaces in ifaces_groups)))
+
+    def __repr__(self):
+        return '{0}({1!r})'.format(self.__class__.__name__, self.plugins)
+
+    def __getitem__(self, name):
+        return self.plugins[name]
+
+    def __iter__(self):
+        return iter(self.plugins)
+
+    def __len__(self):
+        return len(self.plugins)
 
 
 def verify_plugins(initialized, ifaces):
     """Verify plugin objects."""
     verified = {}
-    for plugin_cls, plugin in initialized.iteritems():
+    for name, plugin_ep in initialized.iteritems():
         verifies = True
         for iface in ifaces:  # zope.interface.providedBy(plugin)
             try:
-                zope.interface.verify.verifyObject(iface, plugin)
+                zope.interface.verify.verifyObject(iface, plugin_ep.init())
             except zope.interface.exceptions.BrokenImplementation:
-                if iface.implementedBy(plugin_cls):
+                if iface.implementedBy(plugin_ep.plugin_cls):
                     logging.debug(
                         "%s implements %s but object does "
-                        "not verify", plugin_cls, iface.__name__)
+                        "not verify", plugin_ep.plugin_cls, iface.__name__)
                 verifies = False
                 break
         if verifies:
-            verified[plugin_cls] = plugin
+            verified[name] = plugin_ep
     return verified
 
 
diff --git a/letsencrypt/client/plugins/nginx/configurator.py b/letsencrypt/client/plugins/nginx/configurator.py
index 4a0ae9657..edeb4adb0 100644
--- a/letsencrypt/client/plugins/nginx/configurator.py
+++ b/letsencrypt/client/plugins/nginx/configurator.py
@@ -17,12 +17,14 @@ from letsencrypt.client import interfaces
 from letsencrypt.client import le_util
 from letsencrypt.client import reverter
 
+from letsencrypt.client.plugins import common
+
 from letsencrypt.client.plugins.nginx import constants
 from letsencrypt.client.plugins.nginx import dvsni
 from letsencrypt.client.plugins.nginx import parser
 
 
-class NginxConfigurator(object):
+class NginxConfigurator(common.Plugin):
     # pylint: disable=too-many-instance-attributes,too-many-public-methods
     """Nginx configurator.
 
@@ -60,14 +62,15 @@ class NginxConfigurator(object):
             "'nginx' binary, used for 'configtest' and retrieving nginx "
             "version number.")
 
-    def __init__(self, config, version=None):
+    def __init__(self, *args, **kwargs):
         """Initialize an Nginx Configurator.
 
         :param tup version: version of Nginx as a tuple (1, 4, 7)
             (used mostly for unittesting)
 
         """
-        self.config = config
+        version = kwargs.pop("version", None)
+        super(NginxConfigurator, self).__init__(*args, **kwargs)
 
         # Verify that all directories and files exist with proper permissions
         if os.geteuid() == 0:
@@ -85,21 +88,21 @@ class NginxConfigurator(object):
         self._enhance_func = {}  # TODO: Support at least redirects
 
         # Set up reverter
-        self.reverter = reverter.Reverter(config)
+        self.reverter = reverter.Reverter(self.config)
         self.reverter.recovery_routine()
 
     # This is called in determine_authenticator and determine_installer
     def prepare(self):
         """Prepare the authenticator/installer."""
         self.parser = parser.NginxParser(
-            self.config.nginx_server_root,
-            self.config.nginx_mod_ssl_conf)
+            self.conf('server-root'),
+            self.conf('mod-ssl-conf'))
 
         # Set Version
         if self.version is None:
             self.version = self.get_version()
 
-        temp_install(self.config.nginx_mod_ssl_conf)
+        temp_install(self.conf('mod-ssl-conf'))
 
     # Entry point in main.py for installing cert
     def deploy_cert(self, domain, cert, key, cert_chain=None):
@@ -323,7 +326,7 @@ class NginxConfigurator(object):
         :rtype: bool
 
         """
-        return nginx_restart(self.config.nginx_ctl)
+        return nginx_restart(self.conf('ctl'))
 
     def config_test(self):  # pylint: disable=no-self-use
         """Check the configuration of Nginx for errors.
@@ -334,7 +337,7 @@ class NginxConfigurator(object):
         """
         try:
             proc = subprocess.Popen(
-                [self.config.nginx_ctl, "-t"],
+                [self.conf('ctl'), "-t"],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE)
             stdout, stderr = proc.communicate()
@@ -381,13 +384,13 @@ class NginxConfigurator(object):
         """
         try:
             proc = subprocess.Popen(
-                [self.config.nginx_ctl, "-V"],
+                [self.conf('ctl'), "-V"],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE)
             text = proc.communicate()[1]  # nginx prints output to stderr
         except (OSError, ValueError):
             raise errors.LetsEncryptConfiguratorError(
-                "Unable to run %s -V" % self.config.nginx_ctl)
+                "Unable to run %s -V" % self.conf('ctl'))
 
         version_regex = re.compile(r"nginx/([0-9\.]*)", re.IGNORECASE)
         version_matches = version_regex.findall(text)
diff --git a/letsencrypt/client/plugins/nginx/tests/util.py b/letsencrypt/client/plugins/nginx/tests/util.py
index 8acfa8ff6..dc112af16 100644
--- a/letsencrypt/client/plugins/nginx/tests/util.py
+++ b/letsencrypt/client/plugins/nginx/tests/util.py
@@ -65,12 +65,13 @@ def get_nginx_configurator(
     backups = os.path.join(work_dir, "backups")
 
     config = configurator.NginxConfigurator(
-        mock.MagicMock(
+        config=mock.MagicMock(
             nginx_server_root=config_path, nginx_mod_ssl_conf=ssl_options,
             le_vhost_ext="-le-ssl.conf", backup_dir=backups,
             config_dir=config_dir, work_dir=work_dir,
             temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"),
             in_progress_dir=os.path.join(backups, "IN_PROGRESS")),
-        version)
+        name="nginx",
+        version=version)
     config.prepare()
     return config
diff --git a/letsencrypt/client/plugins/standalone/authenticator.py b/letsencrypt/client/plugins/standalone/authenticator.py
index aaefd09da..668b3f716 100644
--- a/letsencrypt/client/plugins/standalone/authenticator.py
+++ b/letsencrypt/client/plugins/standalone/authenticator.py
@@ -17,8 +17,10 @@ from letsencrypt.acme import challenges
 from letsencrypt.client import achallenges
 from letsencrypt.client import interfaces
 
+from letsencrypt.client.plugins import common
 
-class StandaloneAuthenticator(object):
+
+class StandaloneAuthenticator(common.Plugin):
     # pylint: disable=too-many-instance-attributes
     """Standalone authenticator.
 
@@ -29,15 +31,10 @@ class StandaloneAuthenticator(object):
 
     """
     zope.interface.implements(interfaces.IAuthenticator)
-    zope.interface.classProvides(interfaces.IPluginFactory)
-
     description = "Standalone Authenticator"
 
-    @classmethod
-    def add_parser_arguments(cls, add):
-        pass
-
-    def __init__(self, unused_config):
+    def __init__(self, *args, **kwargs):
+        super(StandaloneAuthenticator, self).__init__(*args, **kwargs)
         self.child_pid = None
         self.parent_pid = os.getpid()
         self.subproc_state = None
diff --git a/letsencrypt/client/plugins/standalone/tests/authenticator_test.py b/letsencrypt/client/plugins/standalone/tests/authenticator_test.py
index 23cd43bd5..e13452ccc 100644
--- a/letsencrypt/client/plugins/standalone/tests/authenticator_test.py
+++ b/letsencrypt/client/plugins/standalone/tests/authenticator_test.py
@@ -53,7 +53,7 @@ class ChallPrefTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
 
     def test_chall_pref(self):
         self.assertEqual(self.authenticator.get_chall_pref("example.com"),
@@ -65,7 +65,7 @@ class SNICallbackTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
         test_key = pkg_resources.resource_string(
             "letsencrypt.client.tests", "testdata/rsa256_key.pem")
         key = le_util.Key("foo", test_key)
@@ -108,7 +108,7 @@ class ClientSignalHandlerTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
         self.authenticator.tasks = {"foononce.acme.invalid": "stuff"}
         self.authenticator.child_pid = 12345
 
@@ -137,7 +137,7 @@ class SubprocSignalHandlerTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
         self.authenticator.tasks = {"foononce.acme.invalid": "stuff"}
         self.authenticator.child_pid = 12345
         self.authenticator.parent_pid = 23456
@@ -189,7 +189,7 @@ class AlreadyListeningTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
 
     @mock.patch("letsencrypt.client.plugins.standalone.authenticator.psutil."
                 "net_connections")
@@ -296,7 +296,7 @@ class PerformTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
 
         test_key = pkg_resources.resource_string(
             "letsencrypt.client.tests", "testdata/rsa256_key.pem")
@@ -375,7 +375,7 @@ class StartListenerTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
 
     @mock.patch("letsencrypt.client.plugins.standalone.authenticator."
                 "Crypto.Random.atfork")
@@ -410,7 +410,7 @@ class DoParentProcessTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
 
     @mock.patch("letsencrypt.client.plugins.standalone.authenticator."
                 "signal.signal")
@@ -464,7 +464,7 @@ class DoChildProcessTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
         test_key = pkg_resources.resource_string(
             "letsencrypt.client.tests", "testdata/rsa256_key.pem")
         key = le_util.Key("foo", test_key)
@@ -562,7 +562,7 @@ class CleanupTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import \
             StandaloneAuthenticator
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
         self.achall = achallenges.DVSNI(
             challb=acme_util.chall_to_challb(
                 challenges.DVSNI(r="whee", nonce="foononce"), "pending"),
@@ -595,7 +595,7 @@ class MoreInfoTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import (
             StandaloneAuthenticator)
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
 
     def test_more_info(self):
         """Make sure exceptions aren't raised."""
@@ -607,7 +607,7 @@ class InitTest(unittest.TestCase):
     def setUp(self):
         from letsencrypt.client.plugins.standalone.authenticator import (
             StandaloneAuthenticator)
-        self.authenticator = StandaloneAuthenticator(None)
+        self.authenticator = StandaloneAuthenticator(config=None, name=None)
 
     def test_prepare(self):
         """Make sure exceptions aren't raised.

From c9a7172388f39fbca478d2aaee771ab5d957e300 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 2 May 2015 08:53:06 +0000
Subject: [PATCH 16/43] Somewhat usable CLI+disco

---
 letsencrypt/client/cli.py                     | 91 +++++++++----------
 letsencrypt/client/display/ops.py             | 14 +--
 .../client/plugins/apache/configurator.py     |  7 +-
 letsencrypt/client/plugins/disco.py           | 56 ++++++------
 .../client/plugins/nginx/configurator.py      |  7 +-
 5 files changed, 89 insertions(+), 86 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index 148562e4c..167ed4a1d 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -59,7 +59,8 @@ def _common_run(args, config, acc, authenticator, installer):
         doms = args.domains
 
     if not doms:
-        return None
+        sys.exit("Please specify --domains, or --installer that will "
+                 "help in domain names autodiscovery")
 
     acme = client.Client(config, acc, authenticator, installer)
 
@@ -73,10 +74,10 @@ def _common_run(args, config, acc, authenticator, installer):
             except errors.LetsEncryptClientError:
                 return None
 
-    return acme, doms, authkey
+    return acme, doms
 
 
-def run(args, config):
+def run(args, config, plugins):
     """Obtain a certificate and install."""
     acc = _account_init(args, config)
     if acc is None:
@@ -89,63 +90,61 @@ def run(args, config):
 
     if args.authenticator is not None or args.installer is not None:
         installer = plugins_disco.pick_installer(
-            config, args.installer)
+            config, args.installer, plugins)
         authenticator = plugins_disco.pick_authenticator(
-            config, args.authenticator)
+            config, args.authenticator, plugins)
     else:
+        # TODO: this assume that user doesn't want to pick authenticator
+        #       and installer separately...
         authenticator = installer = plugins_disco.pick_configurator(
-            config, args.configurator)
+            config, args.configurator, plugins)
 
     if installer is None or authenticator is None:
         return "Configurator could not be determined"
 
-    acme, auth, installer, doms, auth_key = _common_run(args, config, acc)
-    cert_file, chain_file = acme.obtain_certificate(doms)
-    acme.deploy_certificate(doms, authkey, cert_file, chain_file)
+    acme, doms = _common_run(args, config, acc, authenticator, installer)
+    cert_path, chain_path = acme.obtain_certificate(doms)
+    acme.deploy_certificate(doms, acc.key, cert_path, chain_path)
     acme.enhance_config(doms, args.redirect)
 
 
-def auth(args, config):
+def auth(args, config, plugins):
     """Obtain a certificate (no install)."""
     acc = _account_init(args, config)
     if acc is None:
         return None
 
-    authenticator = plugins_disco.pick_authenticator(config, args.authenticator)
+    authenticator = plugins_disco.pick_authenticator(config, args.authenticator, plugins)
     if authenticator is None:
         return "Authenticator could not be determined"
 
     if args.installer is not None:
-        installer = plugins_disco.pick_installer(config, args.installer)
+        installer = plugins_disco.pick_installer(config, args.installer, plugins)
     else:
         installer = None
 
-    if args.domains is None:
-        if args.installer is not None:
-            return ("--domains not set and provided --installer does not "
-                    "help in autodiscovery")
-        else:
-           return ("Please specify --domains, or --installer that will "
-                   "help in domain names autodiscovery")
-
-    acme, doms, _ = _common_run(
-        args, config, authenticator=authenticator, installer=None)
+    acme, doms = _common_run(
+        args, config, acc, authenticator=authenticator, installer=None)
     acme.obtain_certificate(doms)
 
 
-def install(args, config):
+def install(args, config, plugins):
     """Install (no auth)."""
-    installer = plugins_disco.pick_installer(config, args.installer)
+    acc = _account_init(args, config)
+    if acc is None:
+        return None
+
+    installer = plugins_disco.pick_installer(config, args.installer, plugins)
     if installer is None:
         return "Installer could not be determined"
-    acme, doms, authkey = _common_run(
-        args, config, authenticator=None, installer=installer)
-    assert args.cert_file is not None and args.chain_file is not None
-    acme.deploy_certificate(doms, authkey, args.cert_file, args.chain_file)
+    acme, doms = _common_run(
+        args, config, acc, authenticator=None, installer=installer)
+    assert args.cert_path is not None and args.chain_path is not None
+    acme.deploy_certificate(doms, acc.key, args.cert_path, args.chain_path)
     acme.enhance_config(doms, args.redirect)
 
 
-def revoke(args, config):
+def revoke(args, config, plugins):
     """Revoke."""
     if args.rev_cert is None and args.rev_key is None:
         return "At least one of --certificate or --key is required"
@@ -156,18 +155,17 @@ def revoke(args, config):
     #client.revoke(config, args.no_confirm, args.rev_cert, args.rev_key)
 
 
-def rollback(args, config):
+def rollback(args, config, plugins):
     """Rollback."""
     client.rollback(args.checkpoints, config)
 
 
-def config_changes(args, config):
+def config_changes(args, config, plugins):
     """View config changes.
 
     View checkpoints and associated configuration changes.
 
     """
-    print args, config
     client.config_changes(config)
 
 
@@ -195,9 +193,8 @@ def _print_plugins(plugins):
         print  # whitespace between plugins
 
 
-def plugins(args, config):
+def plugins_cmd(args, config, plugins):
     """List plugins."""
-    plugins = plugins_disco.PluginRegistry.find_all()
     logging.debug("Discovered plugins: %s", plugins)
 
     ifaces = [] if args.ifaces is None else args.ifaces
@@ -243,7 +240,7 @@ def config_help(name):
     return interfaces.IConfig[name].__doc__
 
 
-def create_parser():
+def create_parser(plugins):
     """Create parser."""
     parser = configargparse.ArgParser(
         description=__doc__,
@@ -278,7 +275,7 @@ def create_parser():
     parser_rollback = add_subparser("rollback", rollback)
     parrser_config_changes = add_subparser("config_changes", config_changes)
 
-    parser_plugins = add_subparser("plugins", plugins)
+    parser_plugins = add_subparser("plugins", plugins_cmd)
     parser_plugins.add_argument("--init", action="store_true")
     parser_plugins.add_argument("--prepare", action="store_true")
     parser_plugins.add_argument(
@@ -288,12 +285,10 @@ def create_parser():
         "--installers", action="append_const", dest="ifaces",
         const=interfaces.IInstaller)
 
-    parser_run.add_argument("--configurator")
-    for subparser in parser_run, parser_auth:
-        subparser.add_argument("-a", "--authenticator")
-    for subparser in parser_run, parser_auth, parser_install:
-        # parser_auth uses --installer for domains autodiscovery
-        subparser.add_argument("-i", "--installer")
+    parser.add_argument("--configurator")
+    parser.add_argument("-a", "--authenticator")
+    parser.add_argument("-i", "--installer")
+
     # 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:
@@ -328,10 +323,9 @@ def create_parser():
     paths_parser(parser.add_argument_group("paths"))
 
     # TODO: plugin_parser should be called for every detected plugin
-    for name, plugin_cls in [
-            ("apache", apache_configurator.ApacheConfigurator),
-            ("nginx", nginx_configurator.NginxConfigurator)]:
-        plugin_cls.inject_parser_options(parser.add_argument_group(name), name)
+    for name, plugin_ep in plugins.iteritems():
+        plugin_ep.plugin_cls.inject_parser_options(
+            parser.add_argument_group(name), name)
 
     return parser
 
@@ -362,7 +356,8 @@ def paths_parser(parser):
 def main(args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     # note: arg parser internally handles --help (and exits afterwards)
-    args = create_parser().parse_args(args)
+    plugins = plugins_disco.PluginRegistry.find_all()
+    args = create_parser(plugins).parse_args(args)
     config = configuration.NamespaceConfig(args)
 
     # Displayer
@@ -391,7 +386,7 @@ def main(args=sys.argv[1:]):
         #    "{0}Root is required to run letsencrypt.  Please use sudo.{0}"
         #    .format(os.linesep))
 
-    return args.func(args, config)
+    return args.func(args, config, plugins)
 
 
 if __name__ == "__main__":
diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py
index 4243d6b4a..6582035bd 100644
--- a/letsencrypt/client/display/ops.py
+++ b/letsencrypt/client/display/ops.py
@@ -1,4 +1,5 @@
 """Contains UI methods for LE user operations."""
+import logging
 import os
 
 import zope.component
@@ -11,13 +12,13 @@ util = zope.component.getUtility  # pylint: disable=invalid-name
 
 
 def choose_plugin(prepared, question):
-    descs = [plugin.description if error is None
-             else "%s (Misconfigured)" % plugin.description
-             for (plugin, error) in 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]
 
     while True:
         code, index = util(interfaces.IDisplay).menu(
-            question, descs, help_label="More Info")
+            question, opts, help_label="More Info")
 
         if code == display_util.OK:
             return prepared[index][0]
@@ -25,11 +26,11 @@ def choose_plugin(prepared, question):
             if prepared[index][1] is not None:
                 msg = "Reported Error: %s" % prepared[index][1]
             else:
-                msg = prepared[index][0].more_info()
+                msg = prepared[index][0].init().more_info()
             util(interfaces.IDisplay).notification(
                 msg, height=display_util.HEIGHT)
         else:
-            return
+            return None
 
 
 def choose_authenticator(auths, errs):
@@ -98,6 +99,7 @@ def choose_names(installer):
 
     """
     if installer is None:
+        logging.debug("No installer, picking names manually")
         return _choose_names_manually()
 
     names = list(installer.get_all_names())
diff --git a/letsencrypt/client/plugins/apache/configurator.py b/letsencrypt/client/plugins/apache/configurator.py
index d014e6c2e..0a5d91132 100644
--- a/letsencrypt/client/plugins/apache/configurator.py
+++ b/letsencrypt/client/plugins/apache/configurator.py
@@ -13,6 +13,7 @@ from letsencrypt.acme import challenges
 
 from letsencrypt.client import achallenges
 from letsencrypt.client import augeas_configurator
+from letsencrypt.client import constants as core_constants
 from letsencrypt.client import errors
 from letsencrypt.client import interfaces
 from letsencrypt.client import le_util
@@ -949,11 +950,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
         uid = os.geteuid()
         le_util.make_or_verify_dir(
-            self.config.config_dir, constants.CONFIG_DIRS_MODE, uid)
+            self.config.config_dir, core_constants.CONFIG_DIRS_MODE, uid)
         le_util.make_or_verify_dir(
-            self.config.work_dir, constants.CONFIG_DIRS_MODE, uid)
+            self.config.work_dir, core_constants.CONFIG_DIRS_MODE, uid)
         le_util.make_or_verify_dir(
-            self.config.backup_dir, constants.CONFIG_DIRS_MODE, uid)
+            self.config.backup_dir, core_constants.CONFIG_DIRS_MODE, uid)
 
     def get_version(self):
         """Return version of Apache Server.
diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index 350929641..f684395a8 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -44,6 +44,10 @@ class PluginEntryPoint(object):
     def __repr__(self):
         return 'PluginEntryPoint#{0}'.format(self.name)
 
+    @property
+    def name_with_description(self):
+        return "{0} ({1})".format(self.name, self.plugin_cls.description)
+
 
 class PluginRegistry(collections.Mapping):
     """Plugin registry."""
@@ -66,8 +70,8 @@ class PluginRegistry(collections.Mapping):
     def filter(self, *ifaces_groups):
         """Filter plugins based on interfaces."""
         return type(self)(dict(
-            plugin_ep
-            for plugin_ep in self.plugins.iteritems()
+            (name, plugin_ep)
+            for name, plugin_ep in self.plugins.iteritems()
             if not ifaces_groups or any(
                 all(iface.implementedBy(plugin_ep.plugin_cls)
                     for iface in ifaces)
@@ -110,59 +114,59 @@ def prepare_plugins(initialized):
     """Prepare plugins."""
     prepared = {}
 
-    for plugin_cls, plugin in initialized.iteritems():
+    for name, plugin_ep in initialized.iteritems():
         error = None
         try:
-            plugin.prepare()
+            plugin_ep.init().prepare()
         except errors.LetsEncryptMisconfigurationError as error:
-            logging.debug("Misconfigured %s: %s", plugin, error)
+            logging.debug("Misconfigured %s: %s", plugin_ep, error)
         except errors.LetsEncryptNoInstallationError as error:
-            logging.debug("No installation (%s): %s", plugin, error)
+            logging.debug("No installation (%s): %s", plugin_ep, error)
             continue
-        prepared[plugin_cls] = (plugin, error)
+        prepared[name] = (plugin_ep, error)
 
     return prepared  # succefully prepared + misconfigured
 
 
-def pick_plugin(config, default, ifaces, question):
-    plugins = find_plugins()
-    names = name_plugins(plugins)
-
+def pick_plugin(config, default, plugins, ifaces, question):
     if default is not None:
-        filtered = [names[default]]
+        filtered = {default: plugins[default]}
     else:
-        filtered = filter_plugins(plugins, ifaces)
+        filtered = plugins.filter(ifaces)
 
-    initialized = dict((plugin_cls, plugin_cls(config))
-                       for plugin_cls in filtered)
-    verified = verify_plugins(initialized, ifaces)
-    prepared = prepare_plugins(initialized)
+    for plugin_ep in plugins.itervalues():
+        plugin_ep.init(config)
+    verified = verify_plugins(filtered, ifaces)
+    prepared = prepare_plugins(filtered)
 
     if len(prepared) > 1:
         logging.debug("Multiple candidate plugins: %s", prepared)
-        return display_ops.choose_plugin(prepared.values(), question)
+        return display_ops.choose_plugin(prepared.values(), question).init()
     elif len(prepared) == 1:
-        logging.debug("Single candidate plugin: %s", prepared)
-        return prepared.values()[0]
+        plugin_ep = prepared.values()[0][0]
+        logging.debug("Single candidate plugin: %s", plugin_ep)
+        return plugin_ep.init()
     else:
         logging.debug("No candidate plugin")
         return None
 
 
-def pick_authenticator(config, default):
+def pick_authenticator(config, default, plugins):
     """Pick authentication plugin."""
     return pick_plugin(
-            config, default, (interfaces.IAuthenticator,),
+            config, default, plugins, (interfaces.IAuthenticator,),
             "How would you like to authenticate with Let's Encrypt CA?")
 
 
-def pick_installer(config, default):
+def pick_installer(config, default, plugins):
     """Pick installer plugin."""
-    return pick_plugin(config, default, (interfaces.IInstaller,),
+    return pick_plugin(config, default, plugins, (interfaces.IInstaller,),
                        "How would you like to install certificates?")
 
-def pick_configurator(config, default):
+
+def pick_configurator(config, default, plugins):
     """Pick configurator plugin."""
     return pick_plugin(
-        config, default, (interfaces.IAuthenticator, interfaces.IInstaller),
+        config, default, plugins,
+        (interfaces.IAuthenticator, interfaces.IInstaller),
         "How would you like to install certificates?")
diff --git a/letsencrypt/client/plugins/nginx/configurator.py b/letsencrypt/client/plugins/nginx/configurator.py
index edeb4adb0..660653efa 100644
--- a/letsencrypt/client/plugins/nginx/configurator.py
+++ b/letsencrypt/client/plugins/nginx/configurator.py
@@ -12,6 +12,7 @@ import zope.interface
 from letsencrypt.acme import challenges
 
 from letsencrypt.client import achallenges
+from letsencrypt.client import constants as core_constants
 from letsencrypt.client import errors
 from letsencrypt.client import interfaces
 from letsencrypt.client import le_util
@@ -364,11 +365,11 @@ class NginxConfigurator(common.Plugin):
         """
         uid = os.geteuid()
         le_util.make_or_verify_dir(
-            self.config.work_dir, constants.CONFIG_DIRS_MODE, uid)
+            self.config.work_dir, core_constants.CONFIG_DIRS_MODE, uid)
         le_util.make_or_verify_dir(
-            self.config.backup_dir, constants.CONFIG_DIRS_MODE, uid)
+            self.config.backup_dir, core_constants.CONFIG_DIRS_MODE, uid)
         le_util.make_or_verify_dir(
-            self.config.config_dir, constants.CONFIG_DIRS_MODE, uid)
+            self.config.config_dir, core_constants.CONFIG_DIRS_MODE, uid)
 
     def get_version(self):
         """Return version of Nginx Server.

From 5672916cb24a44024ea4ceba7a3998c0a71f3c81 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 2 May 2015 11:19:46 +0000
Subject: [PATCH 17/43] Tests and lint for plugins.common

---
 letsencrypt/client/plugins/common.py      | 16 ++++--
 letsencrypt/client/plugins/common_test.py | 61 +++++++++++++++++++++++
 2 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 letsencrypt/client/plugins/common_test.py

diff --git a/letsencrypt/client/plugins/common.py b/letsencrypt/client/plugins/common.py
index ba6efeccc..e9ee4d573 100644
--- a/letsencrypt/client/plugins/common.py
+++ b/letsencrypt/client/plugins/common.py
@@ -8,11 +8,11 @@ from letsencrypt.client import interfaces
 
 def option_namespace(name):
     """ArgumentParser options namespace (prefix of all options)."""
-    return name + '-'
+    return name + "-"
 
 def dest_namespace(name):
     """ArgumentParser dest namespace (prefix of all destinations)."""
-    return name + '_'
+    return name + "_"
 
 
 class Plugin(object):
@@ -26,17 +26,19 @@ class Plugin(object):
 
     @property
     def option_namespace(self):
+        """ArgumentParser options namespace (prefix of all options)."""
         return option_namespace(self.name)
 
     @property
     def dest_namespace(self):
+        """ArgumentParser dest namespace (prefix of all destinations)."""
         return dest_namespace(self.name)
 
     def dest(self, var):
         """Find a destination for given variable ``var``."""
         # this should do exactly the same what ArgumentParser(arg),
         # does to "arg" to compute "dest"
-        return self.dest_namespace + var.replace('-', '_')
+        return self.dest_namespace + var.replace("-", "_")
 
     def conf(self, var):
         """Find a configuration value for variable ``var``."""
@@ -44,12 +46,18 @@ class Plugin(object):
 
     @classmethod
     def inject_parser_options(cls, parser, name):
+        """Inject parser options.
+
+        See `~.IPlugin.inject_parser_options` for docs.
+
+        """
         # dummy function, doesn't check if dest.startswith(self.dest_namespace)
         def add(arg_name_no_prefix, *args, **kwargs):
+            # pylint: disable=missing-docstring
             return parser.add_argument(
                 "--{0}{1}".format(option_namespace(name), arg_name_no_prefix),
                 *args, **kwargs)
-        cls.add_parser_arguments(add)
+        return cls.add_parser_arguments(add)
 
     @jose_util.abstractclassmethod
     def add_parser_arguments(cls, add):
diff --git a/letsencrypt/client/plugins/common_test.py b/letsencrypt/client/plugins/common_test.py
new file mode 100644
index 000000000..bdb5f7f3c
--- /dev/null
+++ b/letsencrypt/client/plugins/common_test.py
@@ -0,0 +1,61 @@
+"""Tests for letsencrypt.client.plugins.common."""
+import unittest
+
+import mock
+
+
+class NamespaceFunctionsTest(unittest.TestCase):
+    """Tests for letsencrypt.client.plugins.common.*_namespace functions."""
+
+    def test_option_namespace(self):
+        from letsencrypt.client.plugins.common import option_namespace
+        self.assertEqual("foo-", option_namespace("foo"))
+
+    def test_dest_namespace(self):
+        from letsencrypt.client.plugins.common import dest_namespace
+        self.assertEqual("foo_", dest_namespace("foo"))
+
+
+class PluginTest(unittest.TestCase):
+    """Test for letsencrypt.client.plugins.common.Plugin."""
+
+    def setUp(self):
+        from letsencrypt.client.plugins.common import Plugin
+
+        class MockPlugin(Plugin):  # pylint: disable=missing-docstring
+            @classmethod
+            def add_parser_arguments(cls, add):
+                add("foo-bar", dest="different_to_foo_bar", x=1, y=None)
+
+        self.plugin_cls = MockPlugin
+        self.config = mock.MagicMock()
+        self.plugin = MockPlugin(config=self.config, name="mock")
+
+    def test_init(self):
+        self.assertEqual("mock", self.plugin.name)
+        self.assertEqual(self.config, self.plugin.config)
+
+    def test_option_namespace(self):
+        self.assertEqual("mock-", self.plugin.option_namespace)
+
+    def test_dest_namespace(self):
+        self.assertEqual("mock_", self.plugin.dest_namespace)
+
+    def test_dest(self):
+        self.assertEqual("mock_foo_bar", self.plugin.dest("foo-bar"))
+        self.assertEqual("mock_foo_bar", self.plugin.dest("foo_bar"))
+
+    def test_conf(self):
+        self.assertEqual(self.config.mock_foo_bar, self.plugin.conf("foo-bar"))
+
+    def test_inject_parser_options(self):
+        parser = mock.MagicMock()
+        self.plugin_cls.inject_parser_options(parser, "mock")
+        # note that inject_parser_options doesn't check if dest has
+        # correct prefix
+        parser.add_argument.assert_called_once_with(
+            "--mock-foo-bar", dest="different_to_foo_bar", x=1, y=None)
+
+
+if __name__ == "__main__":
+    unittest.main()

From fc059b62698602c505ad01b6d710eff0d79d66af Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 2 May 2015 12:18:46 +0000
Subject: [PATCH 18/43] Add docs for plugins.common and disco

---
 docs/api/client/plugins/common.rst | 5 +++++
 docs/api/client/plugins/disco.rst  | 5 +++++
 2 files changed, 10 insertions(+)
 create mode 100644 docs/api/client/plugins/common.rst
 create mode 100644 docs/api/client/plugins/disco.rst

diff --git a/docs/api/client/plugins/common.rst b/docs/api/client/plugins/common.rst
new file mode 100644
index 000000000..9ee3e6b3e
--- /dev/null
+++ b/docs/api/client/plugins/common.rst
@@ -0,0 +1,5 @@
+:mod:`letsencrypt.client.plugins.common`
+----------------------------------------
+
+.. automodule:: letsencrypt.client.plugins.common
+   :members:
diff --git a/docs/api/client/plugins/disco.rst b/docs/api/client/plugins/disco.rst
new file mode 100644
index 000000000..2b877b654
--- /dev/null
+++ b/docs/api/client/plugins/disco.rst
@@ -0,0 +1,5 @@
+:mod:`letsencrypt.client.plugins.disco`
+---------------------------------------
+
+.. automodule:: letsencrypt.client.plugins.disco
+   :members:

From 9c6bb5e63cae09590e8391471e21b1cb06f96431 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 2 May 2015 12:16:05 +0000
Subject: [PATCH 19/43] Tests and lint for plugins disco

---
 letsencrypt/client/cli.py                |  2 +-
 letsencrypt/client/plugins/disco.py      | 33 ++++----
 letsencrypt/client/plugins/disco_test.py | 97 ++++++++++++++++++++++++
 3 files changed, 116 insertions(+), 16 deletions(-)
 create mode 100644 letsencrypt/client/plugins/disco_test.py

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index 167ed4a1d..c0d83fb61 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -356,7 +356,7 @@ def paths_parser(parser):
 def main(args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     # note: arg parser internally handles --help (and exits afterwards)
-    plugins = plugins_disco.PluginRegistry.find_all()
+    plugins = plugins_disco.PluginsRegistry.find_all()
     args = create_parser(plugins).parse_args(args)
     config = configuration.NamespaceConfig(args)
 
diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index f684395a8..37d60cbc8 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -15,7 +15,7 @@ from letsencrypt.client.display import ops as display_ops
 class PluginEntryPoint(object):
     """Plugin entry point."""
 
-    PREFIX_FREE_DISTRIBUTIONS = ['letsencrypt']
+    PREFIX_FREE_DISTRIBUTIONS = ["letsencrypt"]
     """Distributions for which prefix will be omitted."""
 
     def __init__(self, entry_point):
@@ -26,13 +26,15 @@ class PluginEntryPoint(object):
 
     @property
     def initialized(self):
+        """Has the plugin been initialized already?"""
         return self._initialized is not None
 
     @classmethod
     def entry_point_to_plugin_name(cls, entry_point):
+        """Unique plugin name for an ``entry_point``"""
         if entry_point.dist.key in cls.PREFIX_FREE_DISTRIBUTIONS:
             return entry_point.name
-        return entry_point.dist.key + ':' + entry_point.name
+        return entry_point.dist.key + ":" + entry_point.name
 
     def init(self, config=None):
         """Memoized plugin inititialization."""
@@ -42,15 +44,16 @@ class PluginEntryPoint(object):
         return self._initialized
 
     def __repr__(self):
-        return 'PluginEntryPoint#{0}'.format(self.name)
+        return "PluginEntryPoint#{0}".format(self.name)
 
     @property
     def name_with_description(self):
+        """Name with description. Handy for UI."""
         return "{0} ({1})".format(self.name, self.plugin_cls.description)
 
 
-class PluginRegistry(collections.Mapping):
-    """Plugin registry."""
+class PluginsRegistry(collections.Mapping):
+    """Plugins registry."""
 
     def __init__(self, plugins):
         self.plugins = plugins
@@ -63,7 +66,7 @@ class PluginRegistry(collections.Mapping):
                 constants.SETUPTOOLS_PLUGINS_ENTRY_POINT):
             plugin_ep = PluginEntryPoint(entry_point)
             assert plugin_ep.name not in plugins, (
-                'PREFIX_FREE_DISTRIBTIONS messed up')
+                "PREFIX_FREE_DISTRIBTIONS messed up")
             plugins[plugin_ep.name] = plugin_ep
         return cls(plugins)
 
@@ -78,7 +81,7 @@ class PluginRegistry(collections.Mapping):
                 for ifaces in ifaces_groups)))
 
     def __repr__(self):
-        return '{0}({1!r})'.format(self.__class__.__name__, self.plugins)
+        return "{0}({1!r})".format(self.__class__.__name__, self.plugins)
 
     def __getitem__(self, name):
         return self.plugins[name]
@@ -128,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, ifaces, question):
     if default is not None:
         filtered = {default: plugins[default]}
     else:
@@ -137,7 +140,7 @@ def pick_plugin(config, default, plugins, ifaces, question):
     for plugin_ep in plugins.itervalues():
         plugin_ep.init(config)
     verified = verify_plugins(filtered, ifaces)
-    prepared = prepare_plugins(filtered)
+    prepared = prepare_plugins(verified)
 
     if len(prepared) > 1:
         logging.debug("Multiple candidate plugins: %s", prepared)
@@ -153,20 +156,20 @@ def pick_plugin(config, default, plugins, ifaces, question):
 
 def pick_authenticator(config, default, plugins):
     """Pick authentication plugin."""
-    return pick_plugin(
-            config, default, plugins, (interfaces.IAuthenticator,),
-            "How would you like to authenticate with Let's Encrypt CA?")
+    return _pick_plugin(
+        config, default, plugins, (interfaces.IAuthenticator,),
+        "How would you like to authenticate with Let's Encrypt CA?")
 
 
 def pick_installer(config, default, plugins):
     """Pick installer plugin."""
-    return pick_plugin(config, default, plugins, (interfaces.IInstaller,),
-                       "How would you like to install certificates?")
+    return _pick_plugin(config, default, plugins, (interfaces.IInstaller,),
+                        "How would you like to install certificates?")
 
 
 def pick_configurator(config, default, plugins):
     """Pick configurator plugin."""
-    return pick_plugin(
+    return _pick_plugin(
         config, default, plugins,
         (interfaces.IAuthenticator, interfaces.IInstaller),
         "How would you like to install certificates?")
diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py
new file mode 100644
index 000000000..dcfdb66f1
--- /dev/null
+++ b/letsencrypt/client/plugins/disco_test.py
@@ -0,0 +1,97 @@
+"""Tests for letsencrypt.client.plugins.disco."""
+import pkg_resources
+import unittest
+
+import mock
+
+from letsencrypt.client.plugins.standalone import authenticator
+
+
+class PluginEntryPointTest(unittest.TestCase):
+    """Tests for letsencrypt.client.plugins.disco.PluginEntryPoint."""
+
+    def setUp(self):
+        self.ep1 = pkg_resources.EntryPoint(
+            "ep1", "p1.ep1", dist=mock.MagicMock(key="p1"))
+        self.ep1prim = pkg_resources.EntryPoint(
+            "ep1", "p2.ep2", dist=mock.MagicMock(key="p2"))
+        # nested
+        self.ep2 = pkg_resources.EntryPoint(
+            "ep2", "p2.foo.ep2", dist=mock.MagicMock(key="p2"))
+        # project name != top-level package name
+        self.ep3 = pkg_resources.EntryPoint(
+            "ep3", "a.ep3", dist=mock.MagicMock(key="p3"))
+        # something we can load()/require()
+        self.ep_sa = pkg_resources.EntryPoint(
+            "sa", "letsencrypt.client.plugins.standalone.authenticator",
+            attrs=('StandaloneAuthenticator',),
+            dist=mock.MagicMock(key="letsencrypt"))
+
+        from letsencrypt.client.plugins.disco import PluginEntryPoint
+        self.plugin_ep = PluginEntryPoint(self.ep_sa)
+
+    def test__init__(self):
+        self.assertFalse(self.plugin_ep.initialized)
+        self.assertTrue(self.plugin_ep.entry_point is self.ep_sa)
+        self.assertEqual("sa", self.plugin_ep.name)
+
+        self.assertTrue(
+            self.plugin_ep.plugin_cls is authenticator.StandaloneAuthenticator)
+
+    def test_init(self):
+        config = mock.MagicMock()
+        plugin = self.plugin_ep.init(config=config)
+        self.assertTrue(self.plugin_ep.initialized)
+        self.assertTrue(plugin.config is config)
+        # memoize!
+        self.assertTrue(self.plugin_ep.init() is plugin)
+        self.assertTrue(plugin.config is config)
+        # try to give different config
+        self.assertTrue(self.plugin_ep.init(123) is plugin)
+        self.assertTrue(plugin.config is config)
+
+    def test_entry_point_to_plugin_name(self):
+        from letsencrypt.client.plugins.disco import PluginEntryPoint
+
+        names = {
+            self.ep1: "p1:ep1",
+            self.ep1prim: "p2:ep1",
+            self.ep2: "p2:ep2",
+            self.ep3: "p3:ep3",
+            self.ep_sa: "sa",
+        }
+
+        for entry_point, name in names.iteritems():
+            self.assertEqual(
+                name, PluginEntryPoint.entry_point_to_plugin_name(entry_point))
+
+    def test_name_with_description(self):
+        self.assertTrue(
+            self.plugin_ep.name_with_description.startswith("sa ("))
+
+    def test_repr(self):
+        self.assertEqual("PluginEntryPoint#sa", repr(self.plugin_ep))
+
+
+class PluginsRegistryTest(unittest.TestCase):
+    """Tests for letsencrypt.client.plugins.disco.PluginsRegistry."""
+
+    def setUp(self):
+        from letsencrypt.client.plugins.disco import PluginsRegistry
+        # TODO: mock out pkg_resources.iter_entry_points
+        self.plugins = PluginsRegistry.find_all()
+
+    def test_init(self):
+        self.assertTrue(self.plugins["standalone"].plugin_cls
+                        is authenticator.StandaloneAuthenticator)
+
+    def test_filter(self):
+        filtered = self.plugins.filter()
+        self.assertEqual(len(self.plugins), len(filtered))
+
+    def test_repr(self):
+        repr(self.plugins)
+
+
+if __name__ == "__main__":
+    unittest.main()

From bd45d5ceeaab48473db9a1e43f59e8112d6bb2ef Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 2 May 2015 12:38:33 +0000
Subject: [PATCH 20/43] constants.CLI_DEFAULTS / flag_default()

---
 letsencrypt/client/cli.py       | 33 +++++++++++++++++++--------------
 letsencrypt/client/constants.py | 28 +++++++++++++++-------------
 2 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index c0d83fb61..c4ede223f 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -114,7 +114,8 @@ def auth(args, config, plugins):
     if acc is None:
         return None
 
-    authenticator = plugins_disco.pick_authenticator(config, args.authenticator, plugins)
+    authenticator = plugins_disco.pick_authenticator(
+        config, args.authenticator, plugins)
     if authenticator is None:
         return "Authenticator could not be determined"
 
@@ -236,7 +237,12 @@ def read_file(filename):
         raise argparse.ArgumentTypeError(exc.strerror)
 
 
+def flag_default(name):
+    """Default value for CLI flag."""
+    return constants.CLI_DEFAULTS[name]
+
 def config_help(name):
+    """Help message for `.IConfig` attribute."""
     return interfaces.IConfig[name].__doc__
 
 
@@ -246,14 +252,14 @@ def create_parser(plugins):
         description=__doc__,
         formatter_class=argparse.ArgumentDefaultsHelpFormatter,
         args_for_setting_config_path=["-c", "--config"],
-        default_config_files=constants.DEFAULT_CONFIG_FILES)
+        default_config_files=flag_default("config_files"))
     add = parser.add_argument
 
     # --help is automatically provided by argparse
     add("--version", action="version", version="%(prog)s {0}".format(
             letsencrypt.__version__))
     add("-v", "--verbose", dest="verbose_count", action="count",
-        default=constants.DEFAULT_VERBOSE_COUNT)
+        default=flag_default("verbose_count"))
     add("--no-confirm", dest="no_confirm", action="store_true",
         help="Turn off confirmation screens, currently used for --revoke")
     add("-e", "--agree-tos", dest="tos", action="store_true",
@@ -295,14 +301,13 @@ def create_parser(plugins):
     #    subparser.add_argument("domains", nargs="*", metavar="domain")
 
     add("-d", "--domains", metavar="DOMAIN", action="append")
-    add("-s", "--server", default=constants.DEFAULT_SERVER,
+    add("-s", "--server", default=flag_default("server"),
         help=config_help("server"))
     add("-k", "--authkey", type=read_file,
         help="Path to the authorized key file")
     add("-m", "--email", help=config_help("email"))
     add("-B", "--rsa-key-size", type=int, metavar="N",
-        default=constants.DEFAULT_RSA_KEY_SIZE,
-        help=config_help("rsa_key_size"))
+        default=flag_default("rsa_key_size"), help=config_help("rsa_key_size"))
     # TODO: resolve - assumes binary logic while client.py assumes ternary.
     add("-r", "--redirect", action="store_true",
         help="Automatically redirect all HTTP traffic to HTTPS for the newly "
@@ -317,7 +322,7 @@ def create_parser(plugins):
 
     parser_rollback.add_argument(
         "--checkpoints", type=int, metavar="N",
-        default=constants.DEFAULT_ROLLBACK_CHECKPOINTS,
+        default=flag_default("rollback_checkpoints"),
         help="Revert configuration N number of checkpoints.")
 
     paths_parser(parser.add_argument_group("paths"))
@@ -332,22 +337,22 @@ def create_parser(plugins):
 
 def paths_parser(parser):
     add = parser.add_argument
-    add("--config-dir", default=constants.DEFAULT_CONFIG_DIR,
+    add("--config-dir", default=flag_default("config_dir"),
         help=config_help("config_dir"))
-    add("--work-dir", default=constants.DEFAULT_WORK_DIR,
+    add("--work-dir", default=flag_default("work_dir"),
         help=config_help("work_dir"))
-    add("--backup-dir", default=constants.DEFAULT_BACKUP_DIR,
+    add("--backup-dir", default=flag_default("backup_dir"),
         help=config_help("backup_dir"))
-    add("--key-dir", default=constants.DEFAULT_KEY_DIR,
+    add("--key-dir", default=flag_default("key_dir"),
         help=config_help("key_dir"))
-    add("--cert-dir", default=constants.DEFAULT_CERTS_DIR,
+    add("--cert-dir", default=flag_default("certs_dir"),
         help=config_help("cert_dir"))
 
     add("--le-vhost-ext", default="-le-ssl.conf",
         help=config_help("le_vhost_ext"))
-    add("--cert-path", default=constants.DEFAULT_CERT_PATH,
+    add("--cert-path", default=flag_default("cert_path"),
         help=config_help("cert_path"))
-    add("--chain-path", default=constants.DEFAULT_CHAIN_PATH,
+    add("--chain-path", default=flag_default("chain_path"),
         help=config_help("chain_path"))
 
     return parser
diff --git a/letsencrypt/client/constants.py b/letsencrypt/client/constants.py
index 05f127662..d7ed55bb7 100644
--- a/letsencrypt/client/constants.py
+++ b/letsencrypt/client/constants.py
@@ -9,19 +9,21 @@ SETUPTOOLS_PLUGINS_ENTRY_POINT = "letsencrypt.plugins"
 """Setuptools entry point group name for plugins."""
 
 
-# CLI/IConfig defaults
-DEFAULT_CONFIG_FILES = ["/etc/letsencrypt/cli.ini"]
-DEFAULT_VERBOSE_COUNT = -(logging.WARNING / 10)
-DEFAULT_SERVER = "www.letsencrypt-demo.org/acme/new-reg"
-DEFAULT_RSA_KEY_SIZE = 2048
-DEFAULT_ROLLBACK_CHECKPOINTS = 0
-DEFAULT_CONFIG_DIR = "/etc/letsencrypt"
-DEFAULT_WORK_DIR = "/var/lib/letsencrypt"
-DEFAULT_BACKUP_DIR = "/var/lib/letsencrypt/backups"
-DEFAULT_KEY_DIR = "/etc/letsencrypt/keys"
-DEFAULT_CERTS_DIR = "/etc/letsencrypt/certs"
-DEFAULT_CERT_PATH = "/etc/letsencrypt/certs/cert-letsencrypt.pem"
-DEFAULT_CHAIN_PATH = "/etc/letsencrypt/certs/chain-letsencrypt.pem"
+CLI_DEFAULTS = dict(
+    config_files=["/etc/letsencrypt/cli.ini"],
+    verbose_count=-(logging.WARNING / 10),
+    server="www.letsencrypt-demo.org/acme/new-reg",
+    rsa_key_size=2048,
+    rollback_checkpoints=0,
+    config_dir="/etc/letsencrypt",
+    work_dir="/var/lib/letsencrypt",
+    backup_dir="/var/lib/letsencrypt/backups",
+    key_dir="/etc/letsencrypt/keys",
+    certs_dir="/etc/letsencrypt/certs",
+    cert_path="/etc/letsencrypt/certs/cert-letsencrypt.pem",
+    chain_path="/etc/letsencrypt/certs/chain-letsencrypt.pem",
+)
+"""Defaults for CLI flags and `.IConfig` attributes."""
 
 
 EXCLUSIVE_CHALLENGES = frozenset([frozenset([

From 8a5be3ee3ab01d578812872d3c48e0ce687cded1 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 2 May 2015 13:14:03 +0000
Subject: [PATCH 21/43] 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."""

From 2ffc3c37cba611262cd846c30ede64c642722f96 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 2 May 2015 18:00:57 +0000
Subject: [PATCH 22/43] Fix missing comma

---
 letsencrypt/client/plugins/disco.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index b60c836d6..3262a315b 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -175,5 +175,5 @@ def pick_configurator(
                  "certificates?"):
     """Pick configurator plugin."""
     return _pick_plugin(
-        config, default, plugins, question
+        config, default, plugins, question,
         (interfaces.IAuthenticator, interfaces.IInstaller))

From 595230fd8ecc004ff56433b7ec5902e39018ceb6 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sun, 3 May 2015 07:56:49 +0000
Subject: [PATCH 23/43] PluginsEntryPoint.prepare, move pick_* to display_ops

---
 .../plugins/letsencrypt_example_plugins.py    |   8 +-
 letsencrypt/client/cli.py                     |  19 +--
 letsencrypt/client/client.py                  |   6 +-
 letsencrypt/client/display/ops.py             |  54 +++++-
 letsencrypt/client/interfaces.py              |   6 +-
 letsencrypt/client/plugins/common.py          |   3 +-
 letsencrypt/client/plugins/disco.py           | 155 ++++++++----------
 .../plugins/standalone/authenticator.py       |   2 +
 letsencrypt/client/tests/client_test.py       |   4 +-
 9 files changed, 146 insertions(+), 111 deletions(-)

diff --git a/examples/plugins/letsencrypt_example_plugins.py b/examples/plugins/letsencrypt_example_plugins.py
index 11baf35a7..7c6d1311c 100644
--- a/examples/plugins/letsencrypt_example_plugins.py
+++ b/examples/plugins/letsencrypt_example_plugins.py
@@ -10,18 +10,22 @@ from letsencrypt.client.plugins import common
 
 
 class Authenticator(common.Plugin):
+    """Example Authenticator."""
     zope.interface.implements(interfaces.IAuthenticator)
+    zope.interface.classProvides(interfaces.IPluginFactory)
 
-    description = 'Example Authenticator plugin'
+    description = "Example Authenticator plugin"
 
     # Implement all methods from IAuthenticator, remembering to add
     # "self" as first argument, e.g. def prepare(self)...
 
 
 class Installer(common.Plugins):
+    """Example Installer."""
     zope.interface.implements(interfaces.IInstaller)
+    zope.interface.classProvides(interfaces.IPluginFactory)
 
-    description = 'Example Installer plugin'
+    description = "Example Installer plugin"
 
     # Implement all methods from IInstaller, remembering to add
     # "self" as first argument, e.g. def get_all_names(self)...
diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index b10868b3e..e4150df7c 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -89,14 +89,14 @@ def run(args, config, plugins):
                 "pair, but not both, is allowed")
 
     if args.authenticator is not None or args.installer is not None:
-        installer = plugins_disco.pick_installer(
+        installer = display_ops.pick_installer(
             config, args.installer, plugins)
-        authenticator = plugins_disco.pick_authenticator(
+        authenticator = display_ops.pick_authenticator(
             config, args.authenticator, plugins)
     else:
         # TODO: this assume that user doesn't want to pick authenticator
         #       and installer separately...
-        authenticator = installer = plugins_disco.pick_configurator(
+        authenticator = installer = display_ops.pick_configurator(
             config, args.configurator, plugins)
 
     if installer is None or authenticator is None:
@@ -114,13 +114,13 @@ def auth(args, config, plugins):
     if acc is None:
         return None
 
-    authenticator = plugins_disco.pick_authenticator(
+    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 = plugins_disco.pick_installer(config, args.installer, plugins)
+        installer = display_ops.pick_installer(config, args.installer, plugins)
     else:
         installer = None
 
@@ -135,7 +135,7 @@ def install(args, config, plugins):
     if acc is None:
         return None
 
-    installer = plugins_disco.pick_installer(config, args.installer, plugins)
+    installer = display_ops.pick_installer(config, args.installer, plugins)
     if installer is None:
         return "Installer could not be determined"
     acme, doms = _common_run(
@@ -214,11 +214,10 @@ def plugins_cmd(args, config, plugins):
     if not args.prepare:
         return _print_plugins(filtered)
 
-    prepared = plugins_disco.prepare_plugins(initialized)
-    logging.debug("Prepared plugins: %s", plugins)
+    available = plugins_disco.available_plugins(filtered)
+    logging.debug("Prepared plugins: %s", available)
 
-    _print_plugins(prepared, plugins, names)
-    plugins_disco
+    _print_plugins(available)
 
 
 def read_file(filename):
diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py
index a98de272d..3f1c627e8 100644
--- a/letsencrypt/client/client.py
+++ b/letsencrypt/client/client.py
@@ -20,8 +20,6 @@ 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
@@ -349,7 +347,7 @@ def rollback(default_installer, checkpoints, config, plugins):
 
     """
     # Misconfigurations are only a slight problems... allow the user to rollback
-    installer = plugins_disco.pick_installer(
+    installer = display_ops.pick_installer(
         config, default_installer, plugins, question="Which installer "
         "should be used for rollback?")
 
@@ -368,7 +366,7 @@ def revoke(default_installer, config, plugins, no_confirm, cert, authkey):
     :type config: :class:`letsencrypt.client.interfaces.IConfig`
 
     """
-    installer = plugins_disco.pick_installer(
+    installer = display_ops.pick_installer(
         config, default_installer, plugins, question="Which installer "
         "should be used for certificate revocation?")
 
diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py
index d59c1ceb1..7b035ffd0 100644
--- a/letsencrypt/client/display/ops.py
+++ b/letsencrypt/client/display/ops.py
@@ -6,6 +6,7 @@ import zope.component
 
 from letsencrypt.client import interfaces
 from letsencrypt.client.display import util as display_util
+from letsencrypt.client.plugins import disco as plugins_disco
 
 # Define a helper function to avoid verbose code
 util = zope.component.getUtility  # pylint: disable=invalid-name
@@ -17,9 +18,9 @@ def choose_plugin(prepared, question):
     :param list prepared:
 
     """
-    opts = [plugin_ep.name_with_description if error is None
+    opts = [plugin_ep.name_with_description if not plugin_ep.misconfigured
             else "%s (Misconfigured)" % plugin_ep.name_with_description
-            for (plugin_ep, error) in prepared]
+            for plugin_ep in prepared.itervalues()]
 
     while True:
         code, index = util(interfaces.IDisplay).menu(
@@ -29,7 +30,7 @@ def choose_plugin(prepared, question):
             return prepared[index][0]
         elif code == display_util.HELP:
             if prepared[index][1] is not None:
-                msg = "Reported Error: %s" % prepared[index][1]
+                msg = "Reported Error: %s" % prepared[index].prepare()
             else:
                 msg = prepared[index][0].init().more_info()
             util(interfaces.IDisplay).notification(
@@ -37,6 +38,53 @@ def choose_plugin(prepared, question):
         else:
             return None
 
+def _pick_plugin(config, default, plugins, question, ifaces):
+    if default is not None:
+        filtered = {default: plugins[default]}
+    else:
+        filtered = plugins.filter(ifaces)
+
+    for plugin_ep in plugins.itervalues():
+        plugin_ep.init(config)
+    verified = plugins_disco.verify_plugins(filtered, ifaces)
+    prepared = plugins_disco.available_plugins(verified)
+
+    if len(prepared) > 1:
+        logging.debug("Multiple candidate plugins: %s", prepared)
+        return choose_plugin(prepared.values(), question).init()
+    elif len(prepared) == 1:
+        plugin_ep = prepared.values()[0]
+        logging.debug("Single candidate plugin: %s", plugin_ep)
+        return plugin_ep.init()
+    else:
+        logging.debug("No candidate plugin")
+        return None
+
+
+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, question, (interfaces.IAuthenticator,))
+
+
+def pick_installer(config, default, plugins,
+                   question="How would you like to install certificates?"):
+    """Pick installer plugin."""
+    return _pick_plugin(
+        config, default, plugins, question, (interfaces.IInstaller,))
+
+
+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, question,
+        (interfaces.IAuthenticator, interfaces.IInstaller))
+
 
 def choose_account(accounts):
     """Choose an account.
diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py
index df8b616a6..018462b3c 100644
--- a/letsencrypt/client/interfaces.py
+++ b/letsencrypt/client/interfaces.py
@@ -69,9 +69,11 @@ class IPlugin(zope.interface.Interface):
          Finish up any additional initialization.
 
          :raises letsencrypt.client.errors.LetsEncryptMisconfigurationError:
-             when full initialization cannot be completed.
+             when full initialization cannot be completed. Plugin will be
+             displayed on a list of available plugins.
          :raises letsencrypt.client.errors.LetsEncryptNoInstallationError:
-             when the necessary programs/files cannot be located.
+             when the necessary programs/files cannot be located. Plugin
+             will NOT be displayed on a list of available plugins.
 
         """
 
diff --git a/letsencrypt/client/plugins/common.py b/letsencrypt/client/plugins/common.py
index e9ee4d573..60b868c37 100644
--- a/letsencrypt/client/plugins/common.py
+++ b/letsencrypt/client/plugins/common.py
@@ -18,7 +18,8 @@ def dest_namespace(name):
 class Plugin(object):
     """Generic plugin."""
     zope.interface.implements(interfaces.IPlugin)
-    zope.interface.classProvides(interfaces.IPluginFactory)
+    # classProvides is not inherited, subclasses must define it on their own
+    #zope.interface.classProvides(interfaces.IPluginFactory)
 
     def __init__(self, config, name):
         self.config = config
diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index 3262a315b..dec80afa9 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -9,8 +9,6 @@ from letsencrypt.client import constants
 from letsencrypt.client import errors
 from letsencrypt.client import interfaces
 
-from letsencrypt.client.display import ops as display_ops
-
 
 class PluginEntryPoint(object):
     """Plugin entry point."""
@@ -23,11 +21,7 @@ class PluginEntryPoint(object):
         self.plugin_cls = entry_point.load()
         self.entry_point = entry_point
         self._initialized = None
-
-    @property
-    def initialized(self):
-        """Has the plugin been initialized already?"""
-        return self._initialized is not None
+        self._prepared = None
 
     @classmethod
     def entry_point_to_plugin_name(cls, entry_point):
@@ -36,6 +30,11 @@ class PluginEntryPoint(object):
             return entry_point.name
         return entry_point.dist.key + ":" + entry_point.name
 
+    @property
+    def initialized(self):
+        """Has the plugin been initialized already?"""
+        return self._initialized is not None
+
     def init(self, config=None):
         """Memoized plugin inititialization."""
         if not self.initialized:
@@ -43,6 +42,39 @@ class PluginEntryPoint(object):
             self._initialized = self.plugin_cls(config, self.name)
         return self._initialized
 
+    @property
+    def prepared(self):
+        """Has the plugin been prepared already?"""
+        if not self.initialized:
+            logging.debug(".prepared called on uninitialized %s", self)
+        return self._prepared is not None
+
+    def prepare(self):
+        """Memoized plugin preparation."""
+        assert self.initialized
+        if self._prepared is None:
+            try:
+                self._initialized.prepare()
+            except errors.LetsEncryptMisconfigurationError as error:
+                logging.debug("Misconfigured %s: %s", self, error)
+                self._prepared = error
+            except errors.LetsEncryptNoInstallationError as error:
+                logging.debug("No installation (%s): %s", self, error)
+                self._prepared = error
+            else:
+                self._prepared = True
+        return self._prepared
+
+    @property
+    def misconfigured(self):
+        """Is plugin misconfigured?"""
+        return isinstance(self._prepared, errors.LetsEncryptMisconfigurationError)
+
+    @property
+    def available(self):
+        """Is plugin available, i.e. prepared or misconfigured?"""
+        return self._prepared is True or self.misconfigured
+
     def __repr__(self):
         return "PluginEntryPoint#{0}".format(self.name)
 
@@ -51,6 +83,20 @@ class PluginEntryPoint(object):
         """Name with description. Handy for UI."""
         return "{0} ({1})".format(self.name, self.plugin_cls.description)
 
+    def verify(self, ifaces):
+        assert self.initialized
+        for iface in ifaces:  # zope.interface.providedBy(plugin)
+            try:
+                zope.interface.verify.verifyObject(iface, self.init())
+            except zope.interface.exceptions.BrokenImplementation:
+                if iface.implementedBy(self.plugin_cls):
+                    logging.debug(
+                        "%s implements %s but object does "
+                        "not verify", self.plugin_cls, iface.__name__)
+                return False
+        return True
+
+
 
 class PluginsRegistry(collections.Mapping):
     """Plugins registry."""
@@ -66,8 +112,12 @@ class PluginsRegistry(collections.Mapping):
                 constants.SETUPTOOLS_PLUGINS_ENTRY_POINT):
             plugin_ep = PluginEntryPoint(entry_point)
             assert plugin_ep.name not in plugins, (
-                "PREFIX_FREE_DISTRIBTIONS messed up")
-            plugins[plugin_ep.name] = plugin_ep
+                "PREFIX_FREE_DISTRIBUTIONS messed up")
+            if interfaces.IPluginFactory.providedBy(plugin_ep.plugin_cls):
+                plugins[plugin_ep.name] = plugin_ep
+            else:
+                logging.warning("Plugin entry point %s does not provide "
+                                "IPluginFactory, skipping", plugin_ep)
         return cls(plugins)
 
     def filter(self, *ifaces_groups):
@@ -81,7 +131,8 @@ class PluginsRegistry(collections.Mapping):
                 for ifaces in ifaces_groups)))
 
     def __repr__(self):
-        return "{0}({1!r})".format(self.__class__.__name__, self.plugins)
+        return "{0}({1!r})".format(
+            self.__class__.__name__, set(self.plugins.itervalues()))
 
     def __getitem__(self, name):
         return self.plugins[name]
@@ -95,85 +146,15 @@ class PluginsRegistry(collections.Mapping):
 
 def verify_plugins(initialized, ifaces):
     """Verify plugin objects."""
-    verified = {}
-    for name, plugin_ep in initialized.iteritems():
-        verifies = True
-        for iface in ifaces:  # zope.interface.providedBy(plugin)
-            try:
-                zope.interface.verify.verifyObject(iface, plugin_ep.init())
-            except zope.interface.exceptions.BrokenImplementation:
-                if iface.implementedBy(plugin_ep.plugin_cls):
-                    logging.debug(
-                        "%s implements %s but object does "
-                        "not verify", plugin_ep.plugin_cls, iface.__name__)
-                verifies = False
-                break
-        if verifies:
-            verified[name] = plugin_ep
-    return verified
+    return dict((name, plugin_ep) for name, plugin_ep in initialized.iteritems()
+                if plugin_ep.verify(ifaces))
 
 
-def prepare_plugins(initialized):
-    """Prepare plugins."""
+def available_plugins(initialized):
+    """Prepare plugins and filter available."""
     prepared = {}
-
     for name, plugin_ep in initialized.iteritems():
-        error = None
-        try:
-            plugin_ep.init().prepare()
-        except errors.LetsEncryptMisconfigurationError as error:
-            logging.debug("Misconfigured %s: %s", plugin_ep, error)
-        except errors.LetsEncryptNoInstallationError as error:
-            logging.debug("No installation (%s): %s", plugin_ep, error)
-            continue
-        prepared[name] = (plugin_ep, error)
-
+        plugin_ep.prepare()
+        if plugin_ep.available:
+            prepared[name] = plugin_ep
     return prepared  # succefully prepared + misconfigured
-
-
-def _pick_plugin(config, default, plugins, question, ifaces):
-    if default is not None:
-        filtered = {default: plugins[default]}
-    else:
-        filtered = plugins.filter(ifaces)
-
-    for plugin_ep in plugins.itervalues():
-        plugin_ep.init(config)
-    verified = verify_plugins(filtered, ifaces)
-    prepared = prepare_plugins(verified)
-
-    if len(prepared) > 1:
-        logging.debug("Multiple candidate plugins: %s", prepared)
-        return display_ops.choose_plugin(prepared.values(), question).init()
-    elif len(prepared) == 1:
-        plugin_ep = prepared.values()[0][0]
-        logging.debug("Single candidate plugin: %s", plugin_ep)
-        return plugin_ep.init()
-    else:
-        logging.debug("No candidate plugin")
-        return None
-
-
-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, question, (interfaces.IAuthenticator,))
-
-
-def pick_installer(config, default, plugins,
-                   question="How would you like to install certificates?"):
-    """Pick installer plugin."""
-    return _pick_plugin(
-        config, default, plugins, question, (interfaces.IInstaller,))
-
-
-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, question,
-        (interfaces.IAuthenticator, interfaces.IInstaller))
diff --git a/letsencrypt/client/plugins/standalone/authenticator.py b/letsencrypt/client/plugins/standalone/authenticator.py
index 668b3f716..a10ffd32d 100644
--- a/letsencrypt/client/plugins/standalone/authenticator.py
+++ b/letsencrypt/client/plugins/standalone/authenticator.py
@@ -31,6 +31,8 @@ class StandaloneAuthenticator(common.Plugin):
 
     """
     zope.interface.implements(interfaces.IAuthenticator)
+    zope.interface.classProvides(interfaces.IPluginFactory)
+
     description = "Standalone Authenticator"
 
     def __init__(self, *args, **kwargs):
diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py
index b4595f257..aff9b5c84 100644
--- a/letsencrypt/client/tests/client_test.py
+++ b/letsencrypt/client/tests/client_test.py
@@ -66,7 +66,7 @@ class RollbackTest(unittest.TestCase):
         from letsencrypt.client.client import rollback
         rollback(None, checkpoints, {}, mock.MagicMock())
 
-    @mock.patch("letsencrypt.client.client.plugins_disco.pick_installer")
+    @mock.patch("letsencrypt.client.client.display_ops.pick_installer")
     def test_no_problems(self, mock_pick_installer):
         mock_pick_installer.side_effect = self.m_install
 
@@ -75,7 +75,7 @@ class RollbackTest(unittest.TestCase):
         self.assertEqual(self.m_install().rollback_checkpoints.call_count, 1)
         self.assertEqual(self.m_install().restart.call_count, 1)
 
-    @mock.patch("letsencrypt.client.client.plugins_disco.pick_installer")
+    @mock.patch("letsencrypt.client.client.display_ops.pick_installer")
     def test_no_installer(self, mock_pick_installer):
         mock_pick_installer.return_value = None
         self._call(1)

From b600e2d27089c0daca2ed794451b6ecf101b80b7 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sun, 3 May 2015 12:38:53 +0000
Subject: [PATCH 24/43] PLuginsRegistry: verify, prepare, misconfigured,
 available.

---
 letsencrypt/client/cli.py                | 51 +++++-------------
 letsencrypt/client/display/ops.py        | 19 +++----
 letsencrypt/client/plugins/disco.py      | 66 ++++++++++++++++--------
 letsencrypt/client/plugins/disco_test.py |  4 +-
 4 files changed, 71 insertions(+), 69 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index e4150df7c..e77bef92d 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -82,7 +82,7 @@ def run(args, config, plugins):
     acc = _account_init(args, config)
     if acc is None:
         return None
-    
+
     if args.configurator is not None and (args.installer is not None or
                                           args.authenticator is not None):
         return ("Either --configurator or --authenticator/--installer"
@@ -171,53 +171,30 @@ def config_changes(args, config, plugins):
     client.config_changes(config)
 
 
-def _print_plugins(plugins):
-    # TODO: this functions should use IDisplay rather than printing
-
-    if not plugins:
-        print "No plugins found"
-
-    for plugin_ep in plugins.itervalues():
-        print "* {0}".format(plugin_ep.name)
-        print "Description: {0}".format(plugin_ep.plugin_cls.description)
-        print "Interfaces: {0}".format(", ".join(
-            iface.__name__ for iface in zope.interface.implementedBy(
-                plugin_ep.plugin_cls)))
-        print "Entry point: {0}".format(plugin_ep.entry_point)
-
-        if plugin_ep.initialized:
-            print "Initialized: {0}".format(plugin_ep.init())
-
-        # if filtered == prepared:
-        #if isinstance(content, tuple) and content[1] is not None:
-        #    print content[1]  # error
-
-        print  # whitespace between plugins
-
-
-def plugins_cmd(args, config, plugins):
+def plugins_cmd(args, config, plugins):  # TODO: Use IDiplay rathern than print
     """List plugins."""
     logging.debug("Discovered plugins: %s", plugins)
 
     ifaces = [] if args.ifaces is None else args.ifaces
-    filtered = plugins.filter(*((iface,) for iface in ifaces))
-    logging.debug("Filtered plugins: %s", filtered)
+    filtered = plugins.filter_ifaces(*((iface,) for iface in ifaces))
+    logging.debug("Filtered plugins: %r", filtered)
 
     if not args.init and not args.prepare:
-        return _print_plugins(filtered)
+        print str(filtered)
+        return
 
-    for plugin_ep in filtered.itervalues():
-        plugin_ep.init(config)
-    #verified = plugins_disco.verify_plugins(initialized, ifaces)
-    #logging.debug("Verified plugins: %s", initialized)
+    filtered.init(config)
+    verified = filtered.verify(ifaces)
+    logging.debug("Verified plugins: %r", verified)
 
     if not args.prepare:
-        return _print_plugins(filtered)
+        print str(verified)
+        return
 
-    available = plugins_disco.available_plugins(filtered)
+    verified.prepare()
+    available = verified.available()
     logging.debug("Prepared plugins: %s", available)
-
-    _print_plugins(available)
+    print str(available)
 
 
 def read_file(filename):
diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py
index 7b035ffd0..8ca534883 100644
--- a/letsencrypt/client/display/ops.py
+++ b/letsencrypt/client/display/ops.py
@@ -6,7 +6,7 @@ import zope.component
 
 from letsencrypt.client import interfaces
 from letsencrypt.client.display import util as display_util
-from letsencrypt.client.plugins import disco as plugins_disco
+
 
 # Define a helper function to avoid verbose code
 util = zope.component.getUtility  # pylint: disable=invalid-name
@@ -18,8 +18,8 @@ def choose_plugin(prepared, question):
     :param list prepared:
 
     """
-    opts = [plugin_ep.name_with_description if not plugin_ep.misconfigured
-            else "%s (Misconfigured)" % plugin_ep.name_with_description
+    opts = [plugin_ep.name_with_description
+            + (" [Misconfigured]" if plugin_ep.misconfigured else "")
             for plugin_ep in prepared.itervalues()]
 
     while True:
@@ -40,14 +40,15 @@ def choose_plugin(prepared, question):
 
 def _pick_plugin(config, default, plugins, question, ifaces):
     if default is not None:
-        filtered = {default: plugins[default]}
+        # throw more UX-friendly error if default not in plugins
+        filtered = plugins.filter(lambda p_ep: p_ep.name == default)
     else:
-        filtered = plugins.filter(ifaces)
+        filtered = plugins.filter_ifaces(ifaces)
 
-    for plugin_ep in plugins.itervalues():
-        plugin_ep.init(config)
-    verified = plugins_disco.verify_plugins(filtered, ifaces)
-    prepared = plugins_disco.available_plugins(verified)
+    filtered.init(config)
+    verified = filtered.verify(ifaces)
+    filtered.prepare()
+    prepared = filtered.available()
 
     if len(prepared) > 1:
         logging.debug("Multiple candidate plugins: %s", prepared)
diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index dec80afa9..a4557255e 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -96,6 +96,22 @@ class PluginEntryPoint(object):
                 return False
         return True
 
+    def __str__(self):
+        lines = [
+            "* {0}".format(self.name),
+            "Description: {0}".format(self.plugin_cls.description),
+            "Interfaces: {0}".format(", ".join(
+                iface.__name__ for iface in zope.interface.implementedBy(
+                self.plugin_cls))),
+            "Entry point: {0}".format(self.entry_point),
+        ]
+
+        if self.initialized:
+            lines.append("Initialized: {0}".format(self.init()))
+            if self.prepared:
+                lines.append("Prep: {0}".format(self.prepare()))
+
+        return "\n".join(lines)
 
 
 class PluginsRegistry(collections.Mapping):
@@ -120,15 +136,34 @@ class PluginsRegistry(collections.Mapping):
                                 "IPluginFactory, skipping", plugin_ep)
         return cls(plugins)
 
-    def filter(self, *ifaces_groups):
+    def init(self, config):
+        """Initialize all plugins in the registry."""
+        return [plugin_ep.init(config) for plugin_ep
+                in self.plugins.itervalues()]
+
+    def filter(self, pred):
+        """Filter plugins based on predicate."""
+        return type(self)(dict((name, plugin_ep) for name, plugin_ep
+                               in self.plugins.iteritems() if pred(plugin_ep)))
+
+    def filter_ifaces(self, *ifaces_groups):
         """Filter plugins based on interfaces."""
-        return type(self)(dict(
-            (name, plugin_ep)
-            for name, plugin_ep in self.plugins.iteritems()
-            if not ifaces_groups or any(
+        return self.filter(lambda plugin_ep: not ifaces_groups or any(
                 all(iface.implementedBy(plugin_ep.plugin_cls)
                     for iface in ifaces)
-                for ifaces in ifaces_groups)))
+                for ifaces in ifaces_groups))
+
+    def verify(self, ifaces):
+        """Filter plugins based on verification."""
+        return self.filter(lambda p_ep: p_ep.verify(ifaces))
+
+    def prepare(self):
+        return [plugin_ep.prepare() for plugin_ep in self.plugins.itervalues()]
+
+    def available(self):
+        """Filter plugins based on availability."""
+        return self.filter(lambda p_ep: p_ep.available)
+        # succefully prepared + misconfigured
 
     def __repr__(self):
         return "{0}({1!r})".format(
@@ -143,18 +178,7 @@ class PluginsRegistry(collections.Mapping):
     def __len__(self):
         return len(self.plugins)
 
-
-def verify_plugins(initialized, ifaces):
-    """Verify plugin objects."""
-    return dict((name, plugin_ep) for name, plugin_ep in initialized.iteritems()
-                if plugin_ep.verify(ifaces))
-
-
-def available_plugins(initialized):
-    """Prepare plugins and filter available."""
-    prepared = {}
-    for name, plugin_ep in initialized.iteritems():
-        plugin_ep.prepare()
-        if plugin_ep.available:
-            prepared[name] = plugin_ep
-    return prepared  # succefully prepared + misconfigured
+    def __str__(self):
+        if not self.plugins:
+            return "No plugins"
+        return "\n\n".join(map(str, self.plugins.itervalues()))
diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py
index dcfdb66f1..b55faba0c 100644
--- a/letsencrypt/client/plugins/disco_test.py
+++ b/letsencrypt/client/plugins/disco_test.py
@@ -85,8 +85,8 @@ class PluginsRegistryTest(unittest.TestCase):
         self.assertTrue(self.plugins["standalone"].plugin_cls
                         is authenticator.StandaloneAuthenticator)
 
-    def test_filter(self):
-        filtered = self.plugins.filter()
+    def test_id_filter(self):
+        filtered = self.plugins.filter(lambda _: True)
         self.assertEqual(len(self.plugins), len(filtered))
 
     def test_repr(self):

From 1878db34163157f6115f40e76cfa17eb4883587d Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sun, 3 May 2015 13:21:36 +0000
Subject: [PATCH 25/43] More coverage for plugins.disco

---
 letsencrypt/client/plugins/disco.py      |  40 ++++-----
 letsencrypt/client/plugins/disco_test.py | 100 ++++++++++++++++++-----
 2 files changed, 100 insertions(+), 40 deletions(-)

diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index a4557255e..68cb230d0 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -30,6 +30,11 @@ class PluginEntryPoint(object):
             return entry_point.name
         return entry_point.dist.key + ":" + entry_point.name
 
+    @property
+    def name_with_description(self):
+        """Name with description. Handy for UI."""
+        return "{0} ({1})".format(self.name, self.plugin_cls.description)
+
     @property
     def initialized(self):
         """Has the plugin been initialized already?"""
@@ -42,6 +47,20 @@ class PluginEntryPoint(object):
             self._initialized = self.plugin_cls(config, self.name)
         return self._initialized
 
+    def verify(self, ifaces):
+        """Verify that the plugin conforms to the specified interfaces."""
+        assert self.initialized
+        for iface in ifaces:  # zope.interface.providedBy(plugin)
+            try:
+                zope.interface.verify.verifyObject(iface, self.init())
+            except zope.interface.exceptions.BrokenImplementation:
+                if iface.implementedBy(self.plugin_cls):
+                    logging.debug(
+                        "%s implements %s but object does "
+                        "not verify", self.plugin_cls, iface.__name__)
+                return False
+        return True
+
     @property
     def prepared(self):
         """Has the plugin been prepared already?"""
@@ -68,7 +87,8 @@ class PluginEntryPoint(object):
     @property
     def misconfigured(self):
         """Is plugin misconfigured?"""
-        return isinstance(self._prepared, errors.LetsEncryptMisconfigurationError)
+        return isinstance(
+            self._prepared, errors.LetsEncryptMisconfigurationError)
 
     @property
     def available(self):
@@ -78,24 +98,6 @@ class PluginEntryPoint(object):
     def __repr__(self):
         return "PluginEntryPoint#{0}".format(self.name)
 
-    @property
-    def name_with_description(self):
-        """Name with description. Handy for UI."""
-        return "{0} ({1})".format(self.name, self.plugin_cls.description)
-
-    def verify(self, ifaces):
-        assert self.initialized
-        for iface in ifaces:  # zope.interface.providedBy(plugin)
-            try:
-                zope.interface.verify.verifyObject(iface, self.init())
-            except zope.interface.exceptions.BrokenImplementation:
-                if iface.implementedBy(self.plugin_cls):
-                    logging.debug(
-                        "%s implements %s but object does "
-                        "not verify", self.plugin_cls, iface.__name__)
-                return False
-        return True
-
     def __str__(self):
         lines = [
             "* {0}".format(self.name),
diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py
index b55faba0c..54f77c941 100644
--- a/letsencrypt/client/plugins/disco_test.py
+++ b/letsencrypt/client/plugins/disco_test.py
@@ -3,7 +3,9 @@ import pkg_resources
 import unittest
 
 import mock
+import zope.interface
 
+from letsencrypt.client import errors
 from letsencrypt.client.plugins.standalone import authenticator
 
 
@@ -21,7 +23,8 @@ class PluginEntryPointTest(unittest.TestCase):
         # project name != top-level package name
         self.ep3 = pkg_resources.EntryPoint(
             "ep3", "a.ep3", dist=mock.MagicMock(key="p3"))
-        # something we can load()/require()
+
+        # something we can load()/require(), TODO: use mock
         self.ep_sa = pkg_resources.EntryPoint(
             "sa", "letsencrypt.client.plugins.standalone.authenticator",
             attrs=('StandaloneAuthenticator',),
@@ -30,26 +33,6 @@ class PluginEntryPointTest(unittest.TestCase):
         from letsencrypt.client.plugins.disco import PluginEntryPoint
         self.plugin_ep = PluginEntryPoint(self.ep_sa)
 
-    def test__init__(self):
-        self.assertFalse(self.plugin_ep.initialized)
-        self.assertTrue(self.plugin_ep.entry_point is self.ep_sa)
-        self.assertEqual("sa", self.plugin_ep.name)
-
-        self.assertTrue(
-            self.plugin_ep.plugin_cls is authenticator.StandaloneAuthenticator)
-
-    def test_init(self):
-        config = mock.MagicMock()
-        plugin = self.plugin_ep.init(config=config)
-        self.assertTrue(self.plugin_ep.initialized)
-        self.assertTrue(plugin.config is config)
-        # memoize!
-        self.assertTrue(self.plugin_ep.init() is plugin)
-        self.assertTrue(plugin.config is config)
-        # try to give different config
-        self.assertTrue(self.plugin_ep.init(123) is plugin)
-        self.assertTrue(plugin.config is config)
-
     def test_entry_point_to_plugin_name(self):
         from letsencrypt.client.plugins.disco import PluginEntryPoint
 
@@ -69,6 +52,81 @@ class PluginEntryPointTest(unittest.TestCase):
         self.assertTrue(
             self.plugin_ep.name_with_description.startswith("sa ("))
 
+    def test__init__(self):
+        self.assertFalse(self.plugin_ep.initialized)
+        self.assertFalse(self.plugin_ep.prepared)
+        self.assertFalse(self.plugin_ep.misconfigured)
+        self.assertFalse(self.plugin_ep.available)
+        self.assertTrue(self.plugin_ep.entry_point is self.ep_sa)
+        self.assertEqual("sa", self.plugin_ep.name)
+
+        self.assertTrue(
+            self.plugin_ep.plugin_cls is authenticator.StandaloneAuthenticator)
+
+    def test_init(self):
+        config = mock.MagicMock()
+        plugin = self.plugin_ep.init(config=config)
+        self.assertTrue(self.plugin_ep.initialized)
+        self.assertTrue(plugin.config is config)
+        # memoize!
+        self.assertTrue(self.plugin_ep.init() is plugin)
+        self.assertTrue(plugin.config is config)
+        # try to give different config
+        self.assertTrue(self.plugin_ep.init(123) is plugin)
+        self.assertTrue(plugin.config is config)
+
+        self.assertFalse(self.plugin_ep.prepared)
+        self.assertFalse(self.plugin_ep.misconfigured)
+        self.assertFalse(self.plugin_ep.available)
+
+    def test_verify(self):
+        i1 = mock.MagicMock(__name__="i1")
+        i2 = mock.MagicMock(__name__="i2")
+        i3 = mock.MagicMock(__name__="i3")
+        self.plugin_ep._initialized = plugin = mock.MagicMock()
+
+        exceptions = zope.interface.exceptions
+        with mock.patch("letsencrypt.client.plugins.disco.zope.interface") as mock_zope:
+            mock_zope.exceptions = exceptions
+            def verify_object(iface, obj):
+                assert obj is plugin
+                assert iface is i1 or iface is i2 or iface is i3
+                if iface is i3:
+                    raise mock_zope.exceptions.BrokenImplementation(None, None)
+            mock_zope.verify.verifyObject.side_effect = verify_object
+            self.assertTrue(self.plugin_ep.verify((i1,)))
+            self.assertTrue(self.plugin_ep.verify((i1, i2)))
+            self.assertFalse(self.plugin_ep.verify((i3,)))
+            self.assertFalse(self.plugin_ep.verify((i1, i3)))
+
+    def test_prepare(self):
+        config = mock.MagicMock()
+        self.plugin_ep.init(config=config)
+        self.plugin_ep.prepare()
+        self.assertTrue(self.plugin_ep.prepared)
+        self.assertFalse(self.plugin_ep.misconfigured)
+        str(self.plugin_ep)  # output doesn't matter that much, just jest if it runs
+
+    def test_prepare_misconfigured(self):
+        plugin = mock.MagicMock()
+        plugin.prepare.side_effect = errors.LetsEncryptMisconfigurationError
+        self.plugin_ep._initialized = plugin
+        self.assertTrue(isinstance(self.plugin_ep.prepare(),
+                                   errors.LetsEncryptMisconfigurationError))
+        self.assertTrue(self.plugin_ep.prepared)
+        self.assertTrue(self.plugin_ep.misconfigured)
+        self.assertTrue(self.plugin_ep.available)
+
+    def test_prepare_no_installation(self):
+        plugin = mock.MagicMock()
+        plugin.prepare.side_effect = errors.LetsEncryptNoInstallationError
+        self.plugin_ep._initialized = plugin
+        self.assertTrue(isinstance(self.plugin_ep.prepare(),
+                                   errors.LetsEncryptNoInstallationError))
+        self.assertTrue(self.plugin_ep.prepared)
+        self.assertFalse(self.plugin_ep.misconfigured)
+        self.assertFalse(self.plugin_ep.available)
+
     def test_repr(self):
         self.assertEqual("PluginEntryPoint#sa", repr(self.plugin_ep))
 

From 138a9e9f0161e4a771929adc2aa134bb7dab3626 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sun, 3 May 2015 14:27:22 +0000
Subject: [PATCH 26/43] Full coverage and lint for disco/disco_test

---
 letsencrypt/client/cli.py                |   2 +-
 letsencrypt/client/display/ops.py        |   2 +-
 letsencrypt/client/plugins/disco.py      |  41 ++++---
 letsencrypt/client/plugins/disco_test.py | 133 ++++++++++++++++++-----
 4 files changed, 130 insertions(+), 48 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index e77bef92d..d17b2b459 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -176,7 +176,7 @@ def plugins_cmd(args, config, plugins):  # TODO: Use IDiplay rathern than print
     logging.debug("Discovered plugins: %s", plugins)
 
     ifaces = [] if args.ifaces is None else args.ifaces
-    filtered = plugins.filter_ifaces(*((iface,) for iface in ifaces))
+    filtered = plugins.ifaces(*((iface,) for iface in ifaces))
     logging.debug("Filtered plugins: %r", filtered)
 
     if not args.init and not args.prepare:
diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py
index 8ca534883..24b9dc1d5 100644
--- a/letsencrypt/client/display/ops.py
+++ b/letsencrypt/client/display/ops.py
@@ -43,7 +43,7 @@ def _pick_plugin(config, default, plugins, question, ifaces):
         # throw more UX-friendly error if default not in plugins
         filtered = plugins.filter(lambda p_ep: p_ep.name == default)
     else:
-        filtered = plugins.filter_ifaces(ifaces)
+        filtered = plugins.ifaces(ifaces)
 
     filtered.init(config)
     verified = filtered.verify(ifaces)
diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index 68cb230d0..b2ceeb767 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -35,6 +35,13 @@ class PluginEntryPoint(object):
         """Name with description. Handy for UI."""
         return "{0} ({1})".format(self.name, self.plugin_cls.description)
 
+    def ifaces(self, *ifaces_groups):
+        """Does plugin implements specified interface groups?"""
+        return not ifaces_groups or any(
+            all(iface.implementedBy(self.plugin_cls)
+                for iface in ifaces)
+            for ifaces in ifaces_groups)
+
     @property
     def initialized(self):
         """Has the plugin been initialized already?"""
@@ -104,7 +111,7 @@ class PluginEntryPoint(object):
             "Description: {0}".format(self.plugin_cls.description),
             "Interfaces: {0}".format(", ".join(
                 iface.__name__ for iface in zope.interface.implementedBy(
-                self.plugin_cls))),
+                    self.plugin_cls))),
             "Entry point: {0}".format(self.entry_point),
         ]
 
@@ -131,13 +138,23 @@ class PluginsRegistry(collections.Mapping):
             plugin_ep = PluginEntryPoint(entry_point)
             assert plugin_ep.name not in plugins, (
                 "PREFIX_FREE_DISTRIBUTIONS messed up")
+            # providedBy | pylint: disable=no-member
             if interfaces.IPluginFactory.providedBy(plugin_ep.plugin_cls):
                 plugins[plugin_ep.name] = plugin_ep
-            else:
+            else:  # pragma: no cover
                 logging.warning("Plugin entry point %s does not provide "
                                 "IPluginFactory, skipping", plugin_ep)
         return cls(plugins)
 
+    def __getitem__(self, name):
+        return self.plugins[name]
+
+    def __iter__(self):
+        return iter(self.plugins)
+
+    def __len__(self):
+        return len(self.plugins)
+
     def init(self, config):
         """Initialize all plugins in the registry."""
         return [plugin_ep.init(config) for plugin_ep
@@ -148,18 +165,17 @@ class PluginsRegistry(collections.Mapping):
         return type(self)(dict((name, plugin_ep) for name, plugin_ep
                                in self.plugins.iteritems() if pred(plugin_ep)))
 
-    def filter_ifaces(self, *ifaces_groups):
+    def ifaces(self, *ifaces_groups):
         """Filter plugins based on interfaces."""
-        return self.filter(lambda plugin_ep: not ifaces_groups or any(
-                all(iface.implementedBy(plugin_ep.plugin_cls)
-                    for iface in ifaces)
-                for ifaces in ifaces_groups))
+        # pylint: disable=star-args
+        return self.filter(lambda p_ep: p_ep.ifaces(*ifaces_groups))
 
     def verify(self, ifaces):
         """Filter plugins based on verification."""
         return self.filter(lambda p_ep: p_ep.verify(ifaces))
 
     def prepare(self):
+        """Prepare all plugins in the registry."""
         return [plugin_ep.prepare() for plugin_ep in self.plugins.itervalues()]
 
     def available(self):
@@ -171,16 +187,7 @@ class PluginsRegistry(collections.Mapping):
         return "{0}({1!r})".format(
             self.__class__.__name__, set(self.plugins.itervalues()))
 
-    def __getitem__(self, name):
-        return self.plugins[name]
-
-    def __iter__(self):
-        return iter(self.plugins)
-
-    def __len__(self):
-        return len(self.plugins)
-
     def __str__(self):
         if not self.plugins:
             return "No plugins"
-        return "\n\n".join(map(str, self.plugins.itervalues()))
+        return "\n\n".join(str(p_ep) for p_ep in self.plugins.itervalues())
diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py
index 54f77c941..ac80fae71 100644
--- a/letsencrypt/client/plugins/disco_test.py
+++ b/letsencrypt/client/plugins/disco_test.py
@@ -6,8 +6,15 @@ import mock
 import zope.interface
 
 from letsencrypt.client import errors
+from letsencrypt.client import interfaces
+
 from letsencrypt.client.plugins.standalone import authenticator
 
+EP_SA = pkg_resources.EntryPoint(
+    "sa", "letsencrypt.client.plugins.standalone.authenticator",
+    attrs=("StandaloneAuthenticator",),
+    dist=mock.MagicMock(key="letsencrypt"))
+
 
 class PluginEntryPointTest(unittest.TestCase):
     """Tests for letsencrypt.client.plugins.disco.PluginEntryPoint."""
@@ -24,14 +31,8 @@ class PluginEntryPointTest(unittest.TestCase):
         self.ep3 = pkg_resources.EntryPoint(
             "ep3", "a.ep3", dist=mock.MagicMock(key="p3"))
 
-        # something we can load()/require(), TODO: use mock
-        self.ep_sa = pkg_resources.EntryPoint(
-            "sa", "letsencrypt.client.plugins.standalone.authenticator",
-            attrs=('StandaloneAuthenticator',),
-            dist=mock.MagicMock(key="letsencrypt"))
-
         from letsencrypt.client.plugins.disco import PluginEntryPoint
-        self.plugin_ep = PluginEntryPoint(self.ep_sa)
+        self.plugin_ep = PluginEntryPoint(EP_SA)
 
     def test_entry_point_to_plugin_name(self):
         from letsencrypt.client.plugins.disco import PluginEntryPoint
@@ -41,7 +42,7 @@ class PluginEntryPointTest(unittest.TestCase):
             self.ep1prim: "p2:ep1",
             self.ep2: "p2:ep2",
             self.ep3: "p3:ep3",
-            self.ep_sa: "sa",
+            EP_SA: "sa",
         }
 
         for entry_point, name in names.iteritems():
@@ -52,12 +53,18 @@ class PluginEntryPointTest(unittest.TestCase):
         self.assertTrue(
             self.plugin_ep.name_with_description.startswith("sa ("))
 
+    def test_ifaces(self):
+        self.assertTrue(self.plugin_ep.ifaces((interfaces.IAuthenticator,)))
+        self.assertFalse(self.plugin_ep.ifaces((interfaces.IInstaller,)))
+        self.assertFalse(self.plugin_ep.ifaces((
+            interfaces.IInstaller, interfaces.IAuthenticator)))
+
     def test__init__(self):
         self.assertFalse(self.plugin_ep.initialized)
         self.assertFalse(self.plugin_ep.prepared)
         self.assertFalse(self.plugin_ep.misconfigured)
         self.assertFalse(self.plugin_ep.available)
-        self.assertTrue(self.plugin_ep.entry_point is self.ep_sa)
+        self.assertTrue(self.plugin_ep.entry_point is EP_SA)
         self.assertEqual("sa", self.plugin_ep.name)
 
         self.assertTrue(
@@ -80,24 +87,26 @@ class PluginEntryPointTest(unittest.TestCase):
         self.assertFalse(self.plugin_ep.available)
 
     def test_verify(self):
-        i1 = mock.MagicMock(__name__="i1")
-        i2 = mock.MagicMock(__name__="i2")
-        i3 = mock.MagicMock(__name__="i3")
+        iface1 = mock.MagicMock(__name__="iface1")
+        iface2 = mock.MagicMock(__name__="iface2")
+        iface3 = mock.MagicMock(__name__="iface3")
+        # pylint: disable=protected-access
         self.plugin_ep._initialized = plugin = mock.MagicMock()
 
         exceptions = zope.interface.exceptions
-        with mock.patch("letsencrypt.client.plugins.disco.zope.interface") as mock_zope:
+        with mock.patch("letsencrypt.client.plugins."
+                        "disco.zope.interface") as mock_zope:
             mock_zope.exceptions = exceptions
-            def verify_object(iface, obj):
+            def verify_object(iface, obj):  # pylint: disable=missing-docstring
                 assert obj is plugin
-                assert iface is i1 or iface is i2 or iface is i3
-                if iface is i3:
+                assert iface is iface1 or iface is iface2 or iface is iface3
+                if iface is iface3:
                     raise mock_zope.exceptions.BrokenImplementation(None, None)
             mock_zope.verify.verifyObject.side_effect = verify_object
-            self.assertTrue(self.plugin_ep.verify((i1,)))
-            self.assertTrue(self.plugin_ep.verify((i1, i2)))
-            self.assertFalse(self.plugin_ep.verify((i3,)))
-            self.assertFalse(self.plugin_ep.verify((i1, i3)))
+            self.assertTrue(self.plugin_ep.verify((iface1,)))
+            self.assertTrue(self.plugin_ep.verify((iface1, iface2)))
+            self.assertFalse(self.plugin_ep.verify((iface3,)))
+            self.assertFalse(self.plugin_ep.verify((iface1, iface3)))
 
     def test_prepare(self):
         config = mock.MagicMock()
@@ -105,11 +114,14 @@ class PluginEntryPointTest(unittest.TestCase):
         self.plugin_ep.prepare()
         self.assertTrue(self.plugin_ep.prepared)
         self.assertFalse(self.plugin_ep.misconfigured)
-        str(self.plugin_ep)  # output doesn't matter that much, just jest if it runs
+
+        # output doesn't matter that much, just test if it runs
+        str(self.plugin_ep)
 
     def test_prepare_misconfigured(self):
         plugin = mock.MagicMock()
         plugin.prepare.side_effect = errors.LetsEncryptMisconfigurationError
+        # pylint: disable=protected-access
         self.plugin_ep._initialized = plugin
         self.assertTrue(isinstance(self.plugin_ep.prepare(),
                                    errors.LetsEncryptMisconfigurationError))
@@ -120,6 +132,7 @@ class PluginEntryPointTest(unittest.TestCase):
     def test_prepare_no_installation(self):
         plugin = mock.MagicMock()
         plugin.prepare.side_effect = errors.LetsEncryptNoInstallationError
+        # pylint: disable=protected-access
         self.plugin_ep._initialized = plugin
         self.assertTrue(isinstance(self.plugin_ep.prepare(),
                                    errors.LetsEncryptNoInstallationError))
@@ -136,19 +149,81 @@ class PluginsRegistryTest(unittest.TestCase):
 
     def setUp(self):
         from letsencrypt.client.plugins.disco import PluginsRegistry
-        # TODO: mock out pkg_resources.iter_entry_points
-        self.plugins = PluginsRegistry.find_all()
+        self.plugin_ep = mock.MagicMock(name="mock")
+        self.plugins = {"mock": self.plugin_ep}
+        self.reg = PluginsRegistry(self.plugins)
+
+    def test_find_all(self):
+        from letsencrypt.client.plugins.disco import PluginsRegistry
+        with mock.patch("letsencrypt.client.plugins.disco"
+                        ".pkg_resources") as mock_pkg:
+            mock_pkg.iter_entry_points.return_value = iter([EP_SA])
+            plugins = PluginsRegistry.find_all()
+        self.assertTrue(plugins["sa"].plugin_cls
+                        is authenticator.StandaloneAuthenticator)
+        self.assertTrue(plugins["sa"].entry_point is EP_SA)
+
+    def test_getitem(self):
+        self.assertEqual(self.plugin_ep, self.reg["mock"])
+
+    def test_iter(self):
+        self.assertEqual(["mock"], list(self.reg))
+
+    def test_len(self):
+        self.assertEqual(1, len(self.reg))
+        self.plugins.clear()
+        self.assertEqual(0, len(self.reg))
 
     def test_init(self):
-        self.assertTrue(self.plugins["standalone"].plugin_cls
-                        is authenticator.StandaloneAuthenticator)
+        self.plugin_ep.init.return_value = "baz"
+        self.assertEqual(["baz"], self.reg.init("bar"))
+        self.plugin_ep.init.assert_called_once_with("bar")
 
-    def test_id_filter(self):
-        filtered = self.plugins.filter(lambda _: True)
-        self.assertEqual(len(self.plugins), len(filtered))
+    def test_filter(self):
+        self.plugins.update({
+            "foo": "bar",
+            "bar": "foo",
+            "baz": "boo",
+        })
+        self.assertEqual(
+            {"foo": "bar", "baz": "boo"},
+            self.reg.filter(lambda p_ep: str(p_ep).startswith("b")))
+
+    def test_ifaces(self):
+        self.plugin_ep.ifaces.return_value = True
+        self.assertEqual(self.plugins, self.reg.ifaces().plugins)
+        self.plugin_ep.ifaces.return_value = False
+        self.assertEqual({}, self.reg.ifaces().plugins)
+
+    def test_verify(self):
+        self.plugin_ep.verify.return_value = True
+        self.assertEqual(
+            self.plugins, self.reg.verify(mock.MagicMock()).plugins)
+        self.plugin_ep.verify.return_value = False
+        self.assertEqual({}, self.reg.verify(mock.MagicMock()).plugins)
+
+    def test_prepare(self):
+        self.plugin_ep.prepare.return_value = "baz"
+        self.assertEqual(["baz"], self.reg.prepare())
+        self.plugin_ep.prepare.assert_called_once_with()
+
+    def test_available(self):
+        self.plugin_ep.available = True
+        self.assertEqual(self.plugins, self.reg.available().plugins)
+        self.plugin_ep.available = False
+        self.assertEqual({}, self.reg.available().plugins)
 
     def test_repr(self):
-        repr(self.plugins)
+        self.plugin_ep.__repr__ = lambda _: "PluginEntryPoint#mock"
+        self.assertEqual("PluginsRegistry(set([PluginEntryPoint#mock]))",
+                         repr(self.reg))
+
+    def test_str(self):
+        self.plugin_ep.__str__ = lambda _: "Mock"
+        self.plugins["foo"] = "Mock"
+        self.assertEqual("Mock\n\nMock", str(self.reg))
+        self.plugins.clear()
+        self.assertEqual("No plugins", str(self.reg))
 
 
 if __name__ == "__main__":

From b4f99df7987a999a59ccafef63dd7d02a4287305 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sun, 3 May 2015 22:15:30 +0000
Subject: [PATCH 27/43] Full coverage and lint for display.ops

---
 letsencrypt/client/display/ops.py            |  41 +++++--
 letsencrypt/client/tests/display/ops_test.py | 119 +++++++++++++++++++
 2 files changed, 149 insertions(+), 11 deletions(-)

diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py
index 24b9dc1d5..5a77e0ffb 100644
--- a/letsencrypt/client/display/ops.py
+++ b/letsencrypt/client/display/ops.py
@@ -15,30 +15,49 @@ util = zope.component.getUtility  # pylint: disable=invalid-name
 def choose_plugin(prepared, question):
     """Allow the user to choose ther plugin.
 
-    :param list prepared:
+    :param list prepared: List of `~.PluginEntryPoint`.
+    :param str question: Question to be presented to the user.
+
+    :returns: Plugin entry point chosen by the user.
+    :rtype: `~.PluginEntryPoint`
 
     """
     opts = [plugin_ep.name_with_description
             + (" [Misconfigured]" if plugin_ep.misconfigured else "")
-            for plugin_ep in prepared.itervalues()]
+            for plugin_ep in prepared]
 
     while True:
         code, index = util(interfaces.IDisplay).menu(
             question, opts, help_label="More Info")
 
         if code == display_util.OK:
-            return prepared[index][0]
+            return prepared[index]
         elif code == display_util.HELP:
-            if prepared[index][1] is not None:
+            if prepared[index].misconfigured:
                 msg = "Reported Error: %s" % prepared[index].prepare()
             else:
-                msg = prepared[index][0].init().more_info()
+                msg = prepared[index].init().more_info()
             util(interfaces.IDisplay).notification(
                 msg, height=display_util.HEIGHT)
         else:
             return None
 
-def _pick_plugin(config, default, plugins, question, ifaces):
+
+def pick_plugin(config, default, plugins, question, ifaces):
+    """Pick plugin.
+
+    :param letsencrypt.client.interfaces.IConfig: Configuration
+    :param str default: Plugin name supplied by user or ``None``.
+    :param letsencrypt.client.plugins.disco.PluginsRegistry plugins:
+        All plugins registered as entry points.
+    :param str question: Question to be presented to the user in case
+        multiple candidates are found.
+    :param list ifaces: Interfaces that plugins must provide.
+
+    :returns: Initialized plugin.
+    :rtype: IPlugin
+
+    """
     if default is not None:
         # throw more UX-friendly error if default not in plugins
         filtered = plugins.filter(lambda p_ep: p_ep.name == default)
@@ -47,8 +66,8 @@ def _pick_plugin(config, default, plugins, question, ifaces):
 
     filtered.init(config)
     verified = filtered.verify(ifaces)
-    filtered.prepare()
-    prepared = filtered.available()
+    verified.prepare()
+    prepared = verified.available()
 
     if len(prepared) > 1:
         logging.debug("Multiple candidate plugins: %s", prepared)
@@ -66,14 +85,14 @@ def pick_authenticator(
         config, default, plugins, question="How would you "
         "like to authenticate with Let's Encrypt CA?"):
     """Pick authentication plugin."""
-    return _pick_plugin(
+    return pick_plugin(
         config, default, plugins, question, (interfaces.IAuthenticator,))
 
 
 def pick_installer(config, default, plugins,
                    question="How would you like to install certificates?"):
     """Pick installer plugin."""
-    return _pick_plugin(
+    return pick_plugin(
         config, default, plugins, question, (interfaces.IInstaller,))
 
 
@@ -82,7 +101,7 @@ def pick_configurator(
         question="How would you like to authenticate and install "
                  "certificates?"):
     """Pick configurator plugin."""
-    return _pick_plugin(
+    return pick_plugin(
         config, default, plugins, question,
         (interfaces.IAuthenticator, interfaces.IInstaller))
 
diff --git a/letsencrypt/client/tests/display/ops_test.py b/letsencrypt/client/tests/display/ops_test.py
index 2da411b6b..151358f8a 100644
--- a/letsencrypt/client/tests/display/ops_test.py
+++ b/letsencrypt/client/tests/display/ops_test.py
@@ -8,10 +8,129 @@ import mock
 import zope.component
 
 from letsencrypt.client import account
+from letsencrypt.client import interfaces
 from letsencrypt.client import le_util
+
 from letsencrypt.client.display import util as display_util
 
 
+class ChoosePluginTest(unittest.TestCase):
+    """Tests for letsencrypt.client.display.ops.choose_plugin."""
+
+    def setUp(self):
+        zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
+        self.mock_apache = mock.Mock(
+            name_with_description="a", misconfigured=True)
+        self.mock_stand = mock.Mock(
+            name_with_description="s", misconfigured=False)
+        self.mock_stand.init().more_info.return_value = "standalone"
+        self.plugins = [
+            self.mock_apache,
+            self.mock_stand,
+        ]
+
+    def _call(self):
+        from letsencrypt.client.display.ops import choose_plugin
+        return choose_plugin(self.plugins, "Question?")
+
+    @mock.patch("letsencrypt.client.display.ops.util")
+    def test_successful_choice(self, mock_util):
+        mock_util().menu.return_value = (display_util.OK, 0)
+        self.assertEqual(self.mock_apache, self._call())
+
+    @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),
+        ]
+
+        self.assertEqual(self.mock_stand, self._call())
+        self.assertEqual(mock_util().notification.call_count, 2)
+
+    @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() is None)
+
+
+class PickPluginTest(unittest.TestCase):
+    """Tests for letsencrypt.client.display.ops.pick_plugin."""
+
+    def setUp(self):
+        self.config = mock.Mock()
+        self.default = None
+        self.reg = mock.MagicMock()
+        self.question = "Question?"
+        self.ifaces = []
+
+    def _call(self):
+        from letsencrypt.client.display.ops import pick_plugin
+        return pick_plugin(self.config, self.default, self.reg,
+                           self.question, self.ifaces)
+
+    def test_default_provided(self):
+        self.default = "foo"
+        self._call()
+        self.reg.filter.assert_called_once()
+
+    def test_no_default(self):
+        self._call()
+        self.reg.filter.assert_called_once()
+
+    def test_no_candidate(self):
+        self.assertTrue(self._call() is None)
+
+    def test_single(self):
+        plugin_ep = mock.MagicMock()
+        plugin_ep.init.return_value = "foo"
+        self.reg.ifaces().verify().available.return_value = {"bar": plugin_ep}
+        self.assertEqual("foo", self._call())
+
+    def test_multiple(self):
+        plugin_ep = mock.MagicMock()
+        plugin_ep.init.return_value = "foo"
+        self.reg.ifaces().verify().available.return_value = {
+            "bar": plugin_ep,
+            "baz": plugin_ep,
+        }
+        with mock.patch("letsencrypt.client.display"
+                        ".ops.choose_plugin") as mock_choose:
+            mock_choose.return_value = plugin_ep
+            self.assertEqual("foo", self._call())
+        mock_choose.assert_called_once_with(
+            [plugin_ep, plugin_ep], self.question)
+
+
+class ConveniencePickPluginTest(unittest.TestCase):
+    """Tests for letsencrypt.client.display.ops.pick_*."""
+
+    def _test(self, fun, ifaces):
+        config = mock.Mock()
+        default = mock.Mock()
+        plugins = mock.Mock()
+
+        with mock.patch("letsencrypt.client.display.ops.pick_plugin") as mock_p:
+            mock_p.return_value = "foo"
+            self.assertEqual("foo", fun(config, default, plugins, "Question?"))
+        mock_p.assert_called_once_with(
+            config, default, plugins, "Question?", ifaces)
+
+    def test_authenticator(self):
+        from letsencrypt.client.display.ops import pick_authenticator
+        self._test(pick_authenticator, (interfaces.IAuthenticator,))
+
+    def test_installer(self):
+        from letsencrypt.client.display.ops import pick_installer
+        self._test(pick_installer, (interfaces.IInstaller,))
+
+    def test_configurator(self):
+        from letsencrypt.client.display.ops import pick_configurator
+        self._test(pick_configurator, (
+            interfaces.IAuthenticator, interfaces.IInstaller))
+
+
 class ChooseAccountTest(unittest.TestCase):
     """Test choose_account."""
     def setUp(self):

From 9e7918fc75c665c86b3e4983b267d35bd1753b6c Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 4 May 2015 08:10:52 +0000
Subject: [PATCH 28/43] Test CLI plugins command

---
 letsencrypt/client/tests/cli_test.py | 30 ++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/client/tests/cli_test.py b/letsencrypt/client/tests/cli_test.py
index bb50715e5..0b90953b1 100644
--- a/letsencrypt/client/tests/cli_test.py
+++ b/letsencrypt/client/tests/cli_test.py
@@ -1,11 +1,37 @@
+"""Tests for letsencrypt.client.cli."""
+import itertools
+import sys
 import unittest
 
+import mock
+import zope.component
+
+from letsencrypt.client.display import util as display_util
+
 
 class CLITest(unittest.TestCase):
+    """Tests for different commands."""
 
-    def test_it(self):
+    def _call(self, args):
         from letsencrypt.client import cli
-        self.assertRaises(SystemExit, cli.main, ['--help'])
+        args = ['--text'] + args
+        with mock.patch("letsencrypt.client.cli.sys.stdout") as stdout:
+            with mock.patch("letsencrypt.client.cli.sys.stderr") as stderr:
+                ret = cli.main(args)
+        return ret, stdout, stderr
+
+    def test_no_flags(self):
+        self.assertRaises(SystemExit, self._call, [])
+
+    def test_help(self):
+        self.assertRaises(SystemExit, self._call, ['--help'])
+
+    def test_plugins(self):
+        flags = ['--init', '--prepare', '--authenticators', '--installers']
+        for args in itertools.chain(*(itertools.combinations(flags, r)
+                     for r in xrange(len(flags)))):
+            print args
+            self._call(['plugins',] + list(args))
 
 
 if __name__ == '__main__':

From 8ae6a60fbaa13f99cb39a03dc3e86db0419c86af Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 4 May 2015 08:26:08 +0000
Subject: [PATCH 29/43] pep8

---
 letsencrypt/client/cli.py                     | 31 ++++++++-----------
 letsencrypt/client/client.py                  |  2 --
 letsencrypt/client/constants.py               |  1 -
 .../client/plugins/apache/configurator.py     |  3 +-
 letsencrypt/client/tests/cli_test.py          | 12 +++----
 letsencrypt/client/tests/client_test.py       |  1 -
 6 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index d17b2b459..2035a00db 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -1,10 +1,8 @@
 """Let's Encrypt CLI."""
 # TODO: Sanity check all input.  Be sure to avoid shell code etc...
 import argparse
-import collections
 import logging
 import os
-import pkg_resources
 import sys
 
 import configargparse
@@ -28,9 +26,6 @@ from letsencrypt.client.display import ops as display_ops
 
 from letsencrypt.client.plugins import disco as plugins_disco
 
-from letsencrypt.client.plugins.apache import configurator as apache_configurator
-from letsencrypt.client.plugins.nginx import configurator as nginx_configurator
-
 
 def _account_init(args, config):
     le_util.make_or_verify_dir(
@@ -72,7 +67,7 @@ def _common_run(args, config, acc, authenticator, installer):
             try:
                 acme.register()
             except errors.LetsEncryptClientError:
-                return None
+                sys.exit("Unable to register an account with ACME server")
 
     return acme, doms
 
@@ -125,7 +120,7 @@ def auth(args, config, plugins):
         installer = None
 
     acme, doms = _common_run(
-        args, config, acc, authenticator=authenticator, installer=None)
+        args, config, acc, authenticator=authenticator, installer=installer)
     acme.obtain_certificate(doms)
 
 
@@ -145,7 +140,7 @@ def install(args, config, plugins):
     acme.enhance_config(doms, args.redirect)
 
 
-def revoke(args, config, plugins):
+def revoke(args, unused_config, unused_plugins):
     """Revoke."""
     if args.rev_cert is None and args.rev_key is None:
         return "At least one of --certificate or --key is required"
@@ -162,13 +157,13 @@ def rollback(args, config, plugins):
     client.rollback(args.installer, args.checkpoints, config, plugins)
 
 
-def config_changes(args, config, plugins):
+def config_changes(unused_args, config, unused_plugins):
     """View config changes.
 
     View checkpoints and associated configuration changes.
 
     """
-    client.config_changes(config)
+    client.view_config_changes(config)
 
 
 def plugins_cmd(args, config, plugins):  # TODO: Use IDiplay rathern than print
@@ -234,7 +229,7 @@ def create_parser(plugins):
 
     # --help is automatically provided by argparse
     add("--version", action="version", version="%(prog)s {0}".format(
-            letsencrypt.__version__))
+        letsencrypt.__version__))
     add("-v", "--verbose", dest="verbose_count", action="count",
         default=flag_default("verbose_count"))
     add("--no-confirm", dest="no_confirm", action="store_true",
@@ -245,18 +240,18 @@ def create_parser(plugins):
         help="Use the text output instead of the curses UI.")
 
     subparsers = parser.add_subparsers(metavar="SUBCOMMAND")
-    def add_subparser(name, func):
+    def add_subparser(name, func):  # pylint: disable=missing-docstring
         subparser = subparsers.add_parser(
             name, help=func.__doc__.splitlines()[0], description=func.__doc__)
         subparser.set_defaults(func=func)
         return subparser
 
-    parser_run = add_subparser("run", run)
-    parser_auth = add_subparser("auth", auth)
-    parser_install = add_subparser("install", install)
+    add_subparser("run", run)
+    add_subparser("auth", auth)
+    add_subparser("install", install)
     parser_revoke = add_subparser("revoke", revoke)
     parser_rollback = add_subparser("rollback", rollback)
-    parrser_config_changes = add_subparser("config_changes", config_changes)
+    add_subparser("config_changes", config_changes)
 
     parser_plugins = add_subparser("plugins", plugins_cmd)
     parser_plugins.add_argument("--init", action="store_true")
@@ -302,7 +297,7 @@ def create_parser(plugins):
         default=flag_default("rollback_checkpoints"),
         help="Revert configuration N number of checkpoints.")
 
-    paths_parser(parser.add_argument_group("paths"))
+    _paths_parser(parser.add_argument_group("paths"))
 
     # TODO: plugin_parser should be called for every detected plugin
     for name, plugin_ep in plugins.iteritems():
@@ -312,7 +307,7 @@ def create_parser(plugins):
     return parser
 
 
-def paths_parser(parser):
+def _paths_parser(parser):
     add = parser.add_argument
     add("--config-dir", default=flag_default("config_dir"),
         help=config_help("config_dir"))
diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py
index 3f1c627e8..4fb02a74f 100644
--- a/letsencrypt/client/client.py
+++ b/letsencrypt/client/client.py
@@ -20,8 +20,6 @@ from letsencrypt.client import network2
 from letsencrypt.client import reverter
 from letsencrypt.client import revoker
 
-from letsencrypt.client.plugins.apache import configurator
-
 from letsencrypt.client.display import ops as display_ops
 from letsencrypt.client.display import enhancements
 
diff --git a/letsencrypt/client/constants.py b/letsencrypt/client/constants.py
index d7ed55bb7..3f8cf4f05 100644
--- a/letsencrypt/client/constants.py
+++ b/letsencrypt/client/constants.py
@@ -1,6 +1,5 @@
 """Let's Encrypt constants."""
 import logging
-import pkg_resources
 
 from letsencrypt.acme import challenges
 
diff --git a/letsencrypt/client/plugins/apache/configurator.py b/letsencrypt/client/plugins/apache/configurator.py
index 5c56e0d21..fbbc0d579 100644
--- a/letsencrypt/client/plugins/apache/configurator.py
+++ b/letsencrypt/client/plugins/apache/configurator.py
@@ -96,7 +96,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         add("enmod", default=constants.DEFAULT_ENMOD,
             help="Path to the Apache 'a2enmod' binary.")
         add("init-script", default=constants.DEFAULT_INIT_SCRIPT,
-            help="Path to the Apache init script (used for server reload/restart).")
+            help="Path to the Apache init script (used for server "
+            "reload/restart).")
 
     def __init__(self, *args, **kwargs):
         """Initialize an Apache Configurator.
diff --git a/letsencrypt/client/tests/cli_test.py b/letsencrypt/client/tests/cli_test.py
index 0b90953b1..271b9c5aa 100644
--- a/letsencrypt/client/tests/cli_test.py
+++ b/letsencrypt/client/tests/cli_test.py
@@ -1,18 +1,15 @@
 """Tests for letsencrypt.client.cli."""
 import itertools
-import sys
 import unittest
 
 import mock
-import zope.component
-
-from letsencrypt.client.display import util as display_util
 
 
 class CLITest(unittest.TestCase):
     """Tests for different commands."""
 
-    def _call(self, args):
+    @classmethod
+    def _call(cls, args):
         from letsencrypt.client import cli
         args = ['--text'] + args
         with mock.patch("letsencrypt.client.cli.sys.stdout") as stdout:
@@ -28,8 +25,9 @@ class CLITest(unittest.TestCase):
 
     def test_plugins(self):
         flags = ['--init', '--prepare', '--authenticators', '--installers']
-        for args in itertools.chain(*(itertools.combinations(flags, r)
-                     for r in xrange(len(flags)))):
+        for args in itertools.chain(
+                *(itertools.combinations(flags, r)
+                  for r in xrange(len(flags)))):
             print args
             self._call(['plugins',] + list(args))
 
diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py
index aff9b5c84..33530a083 100644
--- a/letsencrypt/client/tests/client_test.py
+++ b/letsencrypt/client/tests/client_test.py
@@ -8,7 +8,6 @@ import mock
 
 from letsencrypt.client import account
 from letsencrypt.client import configuration
-from letsencrypt.client import errors
 from letsencrypt.client import le_util
 
 

From a2df2455672d87c0d19a3274443c4fa9ee30b71b Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 4 May 2015 11:55:17 +0000
Subject: [PATCH 30/43] Temporary fox for ConfigArgParse#17

---
 Vagrantfile           | 2 +-
 docs/contributing.rst | 4 ++--
 docs/using.rst        | 2 +-
 requirements.txt      | 3 +++
 tox.ini               | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)
 create mode 100644 requirements.txt

diff --git a/Vagrantfile b/Vagrantfile
index b4a06ea05..fa9195dd1 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -10,7 +10,7 @@ cd /vagrant
 sudo ./bootstrap/ubuntu.sh
 if [ ! -d "venv" ]; then
   virtualenv --no-site-packages -p python2 venv
-  ./venv/bin/python setup.py dev
+  ./venv/bin/pip install -r requirements.txt -e .[dev]
 fi
 SETUP_SCRIPT
 
diff --git a/docs/contributing.rst b/docs/contributing.rst
index 0ed022724..bfe5216ae 100644
--- a/docs/contributing.rst
+++ b/docs/contributing.rst
@@ -15,7 +15,7 @@ Now you can install the development packages:
 
 .. code-block:: shell
 
-   ./venv/bin/python setup.py dev
+   ./venv/bin/pip install -r requirements.txt -e .[dev]
 
 The code base, including your pull requests, **must** have 100% test
 statement coverage **and** be compliant with the :ref:`coding style
@@ -48,7 +48,7 @@ synced to ``/vagrant``, so you can get started with:
 
   vagrant ssh
   cd /vagrant
-  ./venv/bin/python setup.py install
+  ./venv/bin/pip install -r requirements.txt
   sudo ./venv/bin/letsencrypt
 
 Support for other Linux distributions coming soon.
diff --git a/docs/using.rst b/docs/using.rst
index 3a7940993..f10966602 100644
--- a/docs/using.rst
+++ b/docs/using.rst
@@ -53,7 +53,7 @@ Installation
 .. code-block:: shell
 
    virtualenv --no-site-packages -p python2 venv
-   ./venv/bin/python setup.py install
+   ./venv/bin/pip install -r requirements.txt
    sudo ./venv/bin/letsencrypt
 
 
diff --git a/requirements.txt b/requirements.txt
new file mode 100644
index 000000000..d7174f103
--- /dev/null
+++ b/requirements.txt
@@ -0,0 +1,3 @@
+# https://github.com/bw2/ConfigArgParse/issues/17
+-e git+https://github.com/kuba/ConfigArgParse.git#egg=ConfigArgParse
+-e .
diff --git a/tox.ini b/tox.ini
index 47b509203..cd6f3c7b0 100644
--- a/tox.ini
+++ b/tox.ini
@@ -7,7 +7,7 @@ envlist = py26,py27,cover,lint
 
 [testenv]
 commands =
-    pip install -e .[testing]
+    pip install -r requirements.txt -e .[testing]
     python setup.py test -q # -q does not suppress errors
 
 setenv =

From f38911eb10e026aca174fe70f956d0c0f361c076 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 4 May 2015 12:04:27 +0000
Subject: [PATCH 31/43] Actually install ConfigArgParse@python2.6 branch

---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index d7174f103..0f0223dab 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,3 +1,3 @@
 # https://github.com/bw2/ConfigArgParse/issues/17
--e git+https://github.com/kuba/ConfigArgParse.git#egg=ConfigArgParse
+-e git+https://github.com/kuba/ConfigArgParse.git@python2.6#egg=ConfigArgParse
 -e .

From 73ac2e36cce1b3a5db36aca8659b55e664f5c9d2 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 4 May 2015 12:46:24 +0000
Subject: [PATCH 32/43] Fix plugins command

---
 letsencrypt/client/cli.py            | 6 ++++--
 letsencrypt/client/tests/cli_test.py | 1 -
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index 2035a00db..7f54afebe 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -168,10 +168,10 @@ def config_changes(unused_args, config, unused_plugins):
 
 def plugins_cmd(args, config, plugins):  # TODO: Use IDiplay rathern than print
     """List plugins."""
-    logging.debug("Discovered plugins: %s", plugins)
+    logging.debug("Expected interfaces: %s", args.ifaces)
 
     ifaces = [] if args.ifaces is None else args.ifaces
-    filtered = plugins.ifaces(*((iface,) for iface in ifaces))
+    filtered = plugins.ifaces(ifaces)
     logging.debug("Filtered plugins: %r", filtered)
 
     if not args.init and not args.prepare:
@@ -352,6 +352,8 @@ def main(args=sys.argv[1:]):
     if args.use_curses:
         logger.addHandler(log.DialogHandler())
 
+    logging.debug("Discovered plugins: %r", plugins)
+
     if not os.geteuid() == 0:
         logging.warning(
             "Root (sudo) is required to run most of letsencrypt functionality.")
diff --git a/letsencrypt/client/tests/cli_test.py b/letsencrypt/client/tests/cli_test.py
index 271b9c5aa..bd25a9792 100644
--- a/letsencrypt/client/tests/cli_test.py
+++ b/letsencrypt/client/tests/cli_test.py
@@ -28,7 +28,6 @@ class CLITest(unittest.TestCase):
         for args in itertools.chain(
                 *(itertools.combinations(flags, r)
                   for r in xrange(len(flags)))):
-            print args
             self._call(['plugins',] + list(args))
 
 

From c185480ae9ab0dea35ca295b388a2728be9c97d5 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 4 May 2015 14:02:03 +0000
Subject: [PATCH 33/43] setup.cfg aliases don't work with pip

---
 Vagrantfile           | 2 +-
 docs/contributing.rst | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Vagrantfile b/Vagrantfile
index fa9195dd1..1d3b48f06 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -10,7 +10,7 @@ cd /vagrant
 sudo ./bootstrap/ubuntu.sh
 if [ ! -d "venv" ]; then
   virtualenv --no-site-packages -p python2 venv
-  ./venv/bin/pip install -r requirements.txt -e .[dev]
+  ./venv/bin/pip install -r requirements.txt -e .[dev,docs,testing]
 fi
 SETUP_SCRIPT
 
diff --git a/docs/contributing.rst b/docs/contributing.rst
index bfe5216ae..d5088705b 100644
--- a/docs/contributing.rst
+++ b/docs/contributing.rst
@@ -15,7 +15,7 @@ Now you can install the development packages:
 
 .. code-block:: shell
 
-   ./venv/bin/pip install -r requirements.txt -e .[dev]
+   ./venv/bin/pip install -r requirements.txt -e .[dev,docs,testing]
 
 The code base, including your pull requests, **must** have 100% test
 statement coverage **and** be compliant with the :ref:`coding style

From aa6984e3103bb979fbe58e9c13519aeba4a4de34 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 4 May 2015 14:29:32 +0000
Subject: [PATCH 34/43] Attempt at cleaning {cert,chain}_path mess

---
 letsencrypt/client/cli.py                     |  2 +-
 letsencrypt/client/client.py                  | 25 +++++++--------
 letsencrypt/client/interfaces.py              |  4 +--
 .../client/plugins/apache/configurator.py     | 31 ++++++++++---------
 4 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index 7f54afebe..bf9a9ed19 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -135,7 +135,7 @@ def install(args, config, plugins):
         return "Installer could not be determined"
     acme, doms = _common_run(
         args, config, acc, authenticator=None, installer=installer)
-    assert args.cert_path is not None and args.chain_path is not None
+    assert args.cert_path is not None
     acme.deploy_certificate(doms, acc.key, args.cert_path, args.chain_path)
     acme.enhance_config(doms, args.redirect)
 
diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py
index 4fb02a74f..12a652a7f 100644
--- a/letsencrypt/client/client.py
+++ b/letsencrypt/client/client.py
@@ -108,7 +108,7 @@ class Client(object):
             this CSR can be different than self.authkey
         :type csr: :class:`CSR`
 
-        :returns: cert_file, chain_file (paths to respective files)
+        :returns: cert_path, chain_path (paths to respective files)
         :rtype: `tuple` of `str`
 
         """
@@ -136,13 +136,13 @@ class Client(object):
             authzr)
 
         # Save Certificate
-        cert_file, chain_file = self.save_certificate(
+        cert_path, chain_path = self.save_certificate(
             certr, self.config.cert_path, self.config.chain_path)
 
         revoker.Revoker.store_cert_key(
-            cert_file, self.account.key.file, self.config)
+            cert_path, self.account.key.file, self.config)
 
-        return cert_file, chain_file
+        return cert_path, chain_path
 
     def save_certificate(self, certr, cert_path, chain_path):
         # pylint: disable=no-self-use
@@ -154,7 +154,7 @@ class Client(object):
         :param str cert_path: Path to attempt to save the cert file
         :param str chain_path: Path to attempt to save the chain file
 
-        :returns: cert_file, chain_file (absolute paths to the actual files)
+        :returns: cert_path, chain_path (absolute paths to the actual files)
         :rtype: `tuple` of `str`
 
         :raises IOError: If unable to find room to write the cert files
@@ -191,7 +191,7 @@ class Client(object):
 
         return os.path.abspath(act_cert_path), cert_chain_abspath
 
-    def deploy_certificate(self, domains, privkey, cert_file, chain_file=None):
+    def deploy_certificate(self, domains, privkey, cert_path, chain_path=None):
         """Install certificate
 
         :param list domains: list of domains to install the certificate
@@ -199,8 +199,8 @@ class Client(object):
         :param privkey: private key for certificate
         :type privkey: :class:`letsencrypt.client.le_util.Key`
 
-        :param str cert_file: certificate file path
-        :param str chain_file: chain file path
+        :param str cert_path: certificate file path
+        :param str chain_path: chain file path
 
         """
         if self.installer is None:
@@ -208,13 +208,12 @@ class Client(object):
                             "the certificate")
             raise errors.LetsEncryptClientError("No installer available")
 
-        chain = None if chain_file is None else os.path.abspath(chain_file)
+        chain_path = None if chain_path is None else os.path.abspath(chain_path)
 
         for dom in domains:
-            self.installer.deploy_cert(dom,
-                                       os.path.abspath(cert_file),
-                                       os.path.abspath(privkey.file),
-                                       chain)
+            self.installer.deploy_cert(
+                dom, os.path.abspath(cert_path),
+                os.path.abspath(privkey.file), chain_path)
 
         self.installer.save("Deployed Let's Encrypt Certificate")
         # sites may have been enabled / final cleanup
diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py
index 018462b3c..b005eb02d 100644
--- a/letsencrypt/client/interfaces.py
+++ b/letsencrypt/client/interfaces.py
@@ -175,8 +175,8 @@ class IConfig(zope.interface.Interface):
 
     le_vhost_ext = zope.interface.Attribute(
         "SSL vhost configuration extension.")
-    cert_path = zope.interface.Attribute("Let's Encrypt certificate file.")
-    chain_path = zope.interface.Attribute("Let's Encrypt chain file.")
+    cert_path = zope.interface.Attribute("Let's Encrypt certificate file path.")
+    chain_path = zope.interface.Attribute("Let's Encrypt chain file path.")
 
 
 class IInstaller(IPlugin):
diff --git a/letsencrypt/client/plugins/apache/configurator.py b/letsencrypt/client/plugins/apache/configurator.py
index fbbc0d579..3bc545475 100644
--- a/letsencrypt/client/plugins/apache/configurator.py
+++ b/letsencrypt/client/plugins/apache/configurator.py
@@ -147,7 +147,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         temp_install(self.conf('mod-ssl-conf'))
 
-    def deploy_cert(self, domain, cert, key, cert_chain=None):
+    def deploy_cert(self, domain, cert_path, key, chain_path=None):
         """Deploys certificate to specified virtual host.
 
         Currently tries to find the last directives to deploy the cert in
@@ -163,25 +163,26 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                   This shouldn't happen within letsencrypt though
 
         :param str domain: domain to deploy certificate
-        :param str cert: certificate filename
+        :param str cert_path: certificate filename
         :param str key: private key filename
-        :param str cert_chain: certificate chain filename
+        :param str chain_path: certificate chain filename
 
         """
         vhost = self.choose_vhost(domain)
+        # TODO(jdkasten): vhost might be None
         path = {}
 
-        path["cert_file"] = self.parser.find_dir(parser.case_i(
+        path["cert_path"] = self.parser.find_dir(parser.case_i(
             "SSLCertificateFile"), None, vhost.path)
         path["cert_key"] = self.parser.find_dir(parser.case_i(
             "SSLCertificateKeyFile"), None, vhost.path)
 
         # Only include if a certificate chain is specified
-        if cert_chain is not None:
-            path["cert_chain"] = self.parser.find_dir(
+        if chain_path is not None:
+            path["chain_path"] = self.parser.find_dir(
                 parser.case_i("SSLCertificateChainFile"), None, vhost.path)
 
-        if len(path["cert_file"]) == 0 or len(path["cert_key"]) == 0:
+        if not path["cert_path"] or not path["cert_key"]:
             # Throw some can't find all of the directives error"
             logging.warn(
                 "Cannot find a cert or key directive in %s", vhost.path)
@@ -191,22 +192,22 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         logging.info("Deploying Certificate to VirtualHost %s", vhost.filep)
 
-        self.aug.set(path["cert_file"][0], cert)
+        self.aug.set(path["cert_path"][0], cert_path)
         self.aug.set(path["cert_key"][0], key)
-        if cert_chain is not None:
-            if len(path["cert_chain"]) == 0:
+        if chain_path is not None:
+            if not path["chain_path"]:
                 self.parser.add_dir(
-                    vhost.path, "SSLCertificateChainFile", cert_chain)
+                    vhost.path, "SSLCertificateChainFile", chain_path)
             else:
-                self.aug.set(path["cert_chain"][0], cert_chain)
+                self.aug.set(path["chain_path"][0], chain_path)
 
         self.save_notes += ("Changed vhost at %s with addresses of %s\n" %
                             (vhost.filep,
                              ", ".join(str(addr) for addr in vhost.addrs)))
-        self.save_notes += "\tSSLCertificateFile %s\n" % cert
+        self.save_notes += "\tSSLCertificateFile %s\n" % cert_path
         self.save_notes += "\tSSLCertificateKeyFile %s\n" % key
-        if cert_chain:
-            self.save_notes += "\tSSLCertificateChainFile %s\n" % cert_chain
+        if chain_path is not None:
+            self.save_notes += "\tSSLCertificateChainFile %s\n" % chain_path
 
         # Make sure vhost is enabled
         if not vhost.enabled:

From 5863323eb05c34ef9ddfe2cf0a88c2943da3ede6 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Mon, 4 May 2015 14:30:02 +0000
Subject: [PATCH 35/43] PluginEntryPoint.description, show in CLI

---
 letsencrypt/client/cli.py                | 3 ++-
 letsencrypt/client/plugins/disco.py      | 7 ++++++-
 letsencrypt/client/plugins/disco_test.py | 3 +++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index bf9a9ed19..f874f4d7d 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -302,7 +302,8 @@ def create_parser(plugins):
     # TODO: plugin_parser should be called for every detected plugin
     for name, plugin_ep in plugins.iteritems():
         plugin_ep.plugin_cls.inject_parser_options(
-            parser.add_argument_group(name), name)
+            parser.add_argument_group(
+                name, description=plugin_ep.description), name)
 
     return parser
 
diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index b2ceeb767..f4a9faecf 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -30,10 +30,15 @@ class PluginEntryPoint(object):
             return entry_point.name
         return entry_point.dist.key + ":" + entry_point.name
 
+    @property
+    def description(self):
+        """Description of the plugin."""
+        return self.plugin_cls.description
+
     @property
     def name_with_description(self):
         """Name with description. Handy for UI."""
-        return "{0} ({1})".format(self.name, self.plugin_cls.description)
+        return "{0} ({1})".format(self.name, self.description)
 
     def ifaces(self, *ifaces_groups):
         """Does plugin implements specified interface groups?"""
diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py
index ac80fae71..945f72d6b 100644
--- a/letsencrypt/client/plugins/disco_test.py
+++ b/letsencrypt/client/plugins/disco_test.py
@@ -49,6 +49,9 @@ class PluginEntryPointTest(unittest.TestCase):
             self.assertEqual(
                 name, PluginEntryPoint.entry_point_to_plugin_name(entry_point))
 
+    def test_description(self):
+        self.assertEqual("Standalone Authenticator", self.plugin_ep.description)
+
     def test_name_with_description(self):
         self.assertTrue(
             self.plugin_ep.name_with_description.startswith("sa ("))

From fce08ea30c0dc60cb49761a2b0108c6481f09daa Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Fri, 8 May 2015 21:32:13 +0000
Subject: [PATCH 36/43] Use CLI_DEFAULTS in plugins

---
 letsencrypt/client/plugins/apache/configurator.py | 10 +++++-----
 letsencrypt/client/plugins/apache/constants.py    | 14 ++++++++------
 letsencrypt/client/plugins/nginx/configurator.py  |  6 +++---
 letsencrypt/client/plugins/nginx/constants.py     |  9 ++++++---
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/letsencrypt/client/plugins/apache/configurator.py b/letsencrypt/client/plugins/apache/configurator.py
index 3bc545475..82d6f323c 100644
--- a/letsencrypt/client/plugins/apache/configurator.py
+++ b/letsencrypt/client/plugins/apache/configurator.py
@@ -86,16 +86,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
     @classmethod
     def add_parser_arguments(cls, add):
-        add("server-root", default=constants.DEFAULT_SERVER_ROOT,
+        add("server-root", default=constants.CLI_DEFAULTS["server_root"],
             help="Apache server root directory.")
-        add("mod-ssl-conf", default=constants.DEFAULT_MOD_SSL_CONF,
+        add("mod-ssl-conf", default=constants.CLI_DEFAULTS["mod_ssl_conf"],
             help="Contains standard Apache SSL directives.")
-        add("ctl", default=constants.DEFAULT_CTL,
+        add("ctl", default=constants.CLI_DEFAULTS["ctl"],
             help="Path to the 'apache2ctl' binary, used for 'configtest' and "
                  "retrieving Apache2 version number.")
-        add("enmod", default=constants.DEFAULT_ENMOD,
+        add("enmod", default=constants.CLI_DEFAULTS["enmod"],
             help="Path to the Apache 'a2enmod' binary.")
-        add("init-script", default=constants.DEFAULT_INIT_SCRIPT,
+        add("init-script", default=constants.CLI_DEFAULTS["init_script"],
             help="Path to the Apache init script (used for server "
             "reload/restart).")
 
diff --git a/letsencrypt/client/plugins/apache/constants.py b/letsencrypt/client/plugins/apache/constants.py
index 63b3bd148..d9f2a0b9d 100644
--- a/letsencrypt/client/plugins/apache/constants.py
+++ b/letsencrypt/client/plugins/apache/constants.py
@@ -2,12 +2,14 @@
 import pkg_resources
 
 
-# CLI/IConfig defaults
-DEFAULT_SERVER_ROOT = "/etc/apache2"
-DEFAULT_MOD_SSL_CONF = "/etc/letsencrypt/options-ssl.conf"
-DEFAULT_CTL = "apache2ctl"
-DEFAULT_ENMOD = "a2enmod"
-DEFAULT_INIT_SCRIPT = "/etc/init.d/apache2"
+CLI_DEFAULTS = dict(
+    server_root="/etc/apache2",
+    mod_ssl_conf="/etc/letsencrypt/options-ssl.conf",
+    ctl="apache2ctl",
+    enmod="a2enmod",
+    init_script="/etc/init.d/apache2",
+)
+"""CLI defaults."""
 
 
 MOD_SSL_CONF = pkg_resources.resource_filename(
diff --git a/letsencrypt/client/plugins/nginx/configurator.py b/letsencrypt/client/plugins/nginx/configurator.py
index 660653efa..49d5a6dd0 100644
--- a/letsencrypt/client/plugins/nginx/configurator.py
+++ b/letsencrypt/client/plugins/nginx/configurator.py
@@ -55,11 +55,11 @@ class NginxConfigurator(common.Plugin):
 
     @classmethod
     def add_parser_arguments(cls, add):
-        add("server-root", default=constants.DEFAULT_SERVER_ROOT,
+        add("server-root", default=constants.CLI_DEFAULTS["server_root"],
             help="Nginx server root directory.")
-        add("mod-ssl-conf", default=constants.DEFAULT_MOD_SSL_CONF,
+        add("mod-ssl-conf", default=constants.CLI_DEFAULTS["mod_ssl_conf"],
             help="Contains standard nginx SSL directives.")
-        add("ctl", default=constants.DEFAULT_CTL, help="Path to the "
+        add("ctl", default=constants.CLI_DEFAULTS["ctl"], help="Path to the "
             "'nginx' binary, used for 'configtest' and retrieving nginx "
             "version number.")
 
diff --git a/letsencrypt/client/plugins/nginx/constants.py b/letsencrypt/client/plugins/nginx/constants.py
index 51fff39e1..17d05f438 100644
--- a/letsencrypt/client/plugins/nginx/constants.py
+++ b/letsencrypt/client/plugins/nginx/constants.py
@@ -2,9 +2,12 @@
 import pkg_resources
 
 
-DEFAULT_SERVER_ROOT = "/etc/nginx"
-DEFAULT_MOD_SSL_CONF = "/etc/letsencrypt/options-ssl-nginx.conf"
-DEFAULT_CTL = "nginx"
+CLI_DEFAULTS = dict(
+    server_root="/etc/nginx",
+    mod_ssl_conf="/etc/letsencrypt/options-ssl-nginx.conf",
+    ctl="nginx",
+)
+"""CLI defaults."""
 
 
 MOD_SSL_CONF = pkg_resources.resource_filename(

From e197f156a72e33c63e7068e012fe3bb0ce397e81 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 9 May 2015 05:47:09 +0000
Subject: [PATCH 37/43] choose_plugin can return None

---
 letsencrypt/client/display/ops.py            |  6 +++++-
 letsencrypt/client/tests/display/ops_test.py | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py
index 5a77e0ffb..dc6992c8c 100644
--- a/letsencrypt/client/display/ops.py
+++ b/letsencrypt/client/display/ops.py
@@ -71,7 +71,11 @@ def pick_plugin(config, default, plugins, question, ifaces):
 
     if len(prepared) > 1:
         logging.debug("Multiple candidate plugins: %s", prepared)
-        return choose_plugin(prepared.values(), question).init()
+        plugin_ep = choose_plugin(prepared.values(), question)
+        if plugin_ep is None:
+            return None
+        else:
+            return plugin_ep.init()
     elif len(prepared) == 1:
         plugin_ep = prepared.values()[0]
         logging.debug("Single candidate plugin: %s", plugin_ep)
diff --git a/letsencrypt/client/tests/display/ops_test.py b/letsencrypt/client/tests/display/ops_test.py
index 151358f8a..7c5c1f74f 100644
--- a/letsencrypt/client/tests/display/ops_test.py
+++ b/letsencrypt/client/tests/display/ops_test.py
@@ -102,6 +102,17 @@ class PickPluginTest(unittest.TestCase):
         mock_choose.assert_called_once_with(
             [plugin_ep, plugin_ep], self.question)
 
+    def test_choose_plugin_none(self):
+        self.reg.ifaces().verify().available.return_value = {
+            "bar": None,
+            "baz": None,
+        }
+
+        with mock.patch("letsencrypt.client.display"
+                        ".ops.choose_plugin") as mock_choose:
+            mock_choose.return_value = None
+            self.assertTrue(self._call() is None)
+
 
 class ConveniencePickPluginTest(unittest.TestCase):
     """Tests for letsencrypt.client.display.ops.pick_*."""

From 75a7b7605b82945a8701f34df414da98411afd52 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 9 May 2015 07:25:36 +0000
Subject: [PATCH 38/43] PluginsRegistry.find_init

---
 letsencrypt/client/plugins/disco.py      | 26 ++++++++++++++++++++++++
 letsencrypt/client/plugins/disco_test.py |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index f4a9faecf..fd636d59a 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -16,6 +16,9 @@ class PluginEntryPoint(object):
     PREFIX_FREE_DISTRIBUTIONS = ["letsencrypt"]
     """Distributions for which prefix will be omitted."""
 
+    # this object is mutable, don't allow it to be hashed!
+    __hash__ = None
+
     def __init__(self, entry_point):
         self.name = self.entry_point_to_plugin_name(entry_point)
         self.plugin_cls = entry_point.load()
@@ -188,6 +191,29 @@ class PluginsRegistry(collections.Mapping):
         return self.filter(lambda p_ep: p_ep.available)
         # succefully prepared + misconfigured
 
+    def find_init(self, plugin):
+        """Find an initialized plugin.
+
+        This is particularly useful for finding a name for the plugin
+        (although `.IPluginFactory.__call__` takes ``name`` as one of
+        the arguments, ``IPlugin.name`` is not part of the interface)::
+
+          # plugin is an instance providing IPlugin, initialized
+          # somewhere else in the code
+          plugin_registry.find_init(plugin).name
+
+        Returns ``None`` if ``plugin`` is not found in the registry.
+
+        """
+        # use list instead of set beacse PluginEntryPoint is not hashable
+        candidates = [plugin_ep for plugin_ep in self.plugins.itervalues()
+                      if plugin_ep.initialized and plugin_ep.init() is plugin]
+        assert len(candidates) <= 1
+        if candidates:
+            return candidates[0]
+        else:
+            return None
+
     def __repr__(self):
         return "{0}({1!r})".format(
             self.__class__.__name__, set(self.plugins.itervalues()))
diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py
index 945f72d6b..ca3bc42d4 100644
--- a/letsencrypt/client/plugins/disco_test.py
+++ b/letsencrypt/client/plugins/disco_test.py
@@ -216,6 +216,12 @@ class PluginsRegistryTest(unittest.TestCase):
         self.plugin_ep.available = False
         self.assertEqual({}, self.reg.available().plugins)
 
+    def test_find_init(self):
+        self.assertTrue(self.reg.find_init(mock.Mock()) is None)
+        self.plugin_ep.initalized = True
+        self.assertTrue(
+            self.reg.find_init(self.plugin_ep.init()) is self.plugin_ep)
+
     def test_repr(self):
         self.plugin_ep.__repr__ = lambda _: "PluginEntryPoint#mock"
         self.assertEqual("PluginsRegistry(set([PluginEntryPoint#mock]))",

From e415a63d1fa6d969f9deaadd087284eac5510af6 Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 9 May 2015 07:29:51 +0000
Subject: [PATCH 39/43] PluginsRegistry.plugins -> PluginsRegistry._plugins

---
 letsencrypt/client/plugins/disco.py      | 22 +++++++++++-----------
 letsencrypt/client/plugins/disco_test.py | 15 +++++++++------
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index fd636d59a..ce6e23172 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -135,7 +135,7 @@ class PluginsRegistry(collections.Mapping):
     """Plugins registry."""
 
     def __init__(self, plugins):
-        self.plugins = plugins
+        self._plugins = plugins
 
     @classmethod
     def find_all(cls):
@@ -155,23 +155,23 @@ class PluginsRegistry(collections.Mapping):
         return cls(plugins)
 
     def __getitem__(self, name):
-        return self.plugins[name]
+        return self._plugins[name]
 
     def __iter__(self):
-        return iter(self.plugins)
+        return iter(self._plugins)
 
     def __len__(self):
-        return len(self.plugins)
+        return len(self._plugins)
 
     def init(self, config):
         """Initialize all plugins in the registry."""
         return [plugin_ep.init(config) for plugin_ep
-                in self.plugins.itervalues()]
+                in self._plugins.itervalues()]
 
     def filter(self, pred):
         """Filter plugins based on predicate."""
         return type(self)(dict((name, plugin_ep) for name, plugin_ep
-                               in self.plugins.iteritems() if pred(plugin_ep)))
+                               in self._plugins.iteritems() if pred(plugin_ep)))
 
     def ifaces(self, *ifaces_groups):
         """Filter plugins based on interfaces."""
@@ -184,7 +184,7 @@ class PluginsRegistry(collections.Mapping):
 
     def prepare(self):
         """Prepare all plugins in the registry."""
-        return [plugin_ep.prepare() for plugin_ep in self.plugins.itervalues()]
+        return [plugin_ep.prepare() for plugin_ep in self._plugins.itervalues()]
 
     def available(self):
         """Filter plugins based on availability."""
@@ -206,7 +206,7 @@ class PluginsRegistry(collections.Mapping):
 
         """
         # use list instead of set beacse PluginEntryPoint is not hashable
-        candidates = [plugin_ep for plugin_ep in self.plugins.itervalues()
+        candidates = [plugin_ep for plugin_ep in self._plugins.itervalues()
                       if plugin_ep.initialized and plugin_ep.init() is plugin]
         assert len(candidates) <= 1
         if candidates:
@@ -216,9 +216,9 @@ class PluginsRegistry(collections.Mapping):
 
     def __repr__(self):
         return "{0}({1!r})".format(
-            self.__class__.__name__, set(self.plugins.itervalues()))
+            self.__class__.__name__, set(self._plugins.itervalues()))
 
     def __str__(self):
-        if not self.plugins:
+        if not self._plugins:
             return "No plugins"
-        return "\n\n".join(str(p_ep) for p_ep in self.plugins.itervalues())
+        return "\n\n".join(str(p_ep) for p_ep in self._plugins.itervalues())
diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py
index ca3bc42d4..f5ea9e6ee 100644
--- a/letsencrypt/client/plugins/disco_test.py
+++ b/letsencrypt/client/plugins/disco_test.py
@@ -194,16 +194,18 @@ class PluginsRegistryTest(unittest.TestCase):
 
     def test_ifaces(self):
         self.plugin_ep.ifaces.return_value = True
-        self.assertEqual(self.plugins, self.reg.ifaces().plugins)
+        # pylint: disable=protected-access
+        self.assertEqual(self.plugins, self.reg.ifaces()._plugins)
         self.plugin_ep.ifaces.return_value = False
-        self.assertEqual({}, self.reg.ifaces().plugins)
+        self.assertEqual({}, self.reg.ifaces()._plugins)
 
     def test_verify(self):
         self.plugin_ep.verify.return_value = True
+        # pylint: disable=protected-access
         self.assertEqual(
-            self.plugins, self.reg.verify(mock.MagicMock()).plugins)
+            self.plugins, self.reg.verify(mock.MagicMock())._plugins)
         self.plugin_ep.verify.return_value = False
-        self.assertEqual({}, self.reg.verify(mock.MagicMock()).plugins)
+        self.assertEqual({}, self.reg.verify(mock.MagicMock())._plugins)
 
     def test_prepare(self):
         self.plugin_ep.prepare.return_value = "baz"
@@ -212,9 +214,10 @@ class PluginsRegistryTest(unittest.TestCase):
 
     def test_available(self):
         self.plugin_ep.available = True
-        self.assertEqual(self.plugins, self.reg.available().plugins)
+        # pylint: disable=protected-access
+        self.assertEqual(self.plugins, self.reg.available()._plugins)
         self.plugin_ep.available = False
-        self.assertEqual({}, self.reg.available().plugins)
+        self.assertEqual({}, self.reg.available()._plugins)
 
     def test_find_init(self):
         self.assertTrue(self.reg.find_init(mock.Mock()) is None)

From 21411266087a6db13501241e0c34f0a5240a395f Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 9 May 2015 07:36:52 +0000
Subject: [PATCH 40/43] clean up disco logging

---
 letsencrypt/client/plugins/disco.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index ce6e23172..50e0bce50 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -80,7 +80,7 @@ class PluginEntryPoint(object):
     def prepared(self):
         """Has the plugin been prepared already?"""
         if not self.initialized:
-            logging.debug(".prepared called on uninitialized %s", self)
+            logging.debug(".prepared called on uninitialized %r", self)
         return self._prepared is not None
 
     def prepare(self):
@@ -90,10 +90,10 @@ class PluginEntryPoint(object):
             try:
                 self._initialized.prepare()
             except errors.LetsEncryptMisconfigurationError as error:
-                logging.debug("Misconfigured %s: %s", self, error)
+                logging.debug("Misconfigured %r: %s", self, error)
                 self._prepared = error
             except errors.LetsEncryptNoInstallationError as error:
-                logging.debug("No installation (%s): %s", self, error)
+                logging.debug("No installation (%r): %s", self, error)
                 self._prepared = error
             else:
                 self._prepared = True
@@ -150,8 +150,8 @@ class PluginsRegistry(collections.Mapping):
             if interfaces.IPluginFactory.providedBy(plugin_ep.plugin_cls):
                 plugins[plugin_ep.name] = plugin_ep
             else:  # pragma: no cover
-                logging.warning("Plugin entry point %s does not provide "
-                                "IPluginFactory, skipping", plugin_ep)
+                logging.warning(
+                    "%r does not provide IPluginFactory, skipping", plugin_ep)
         return cls(plugins)
 
     def __getitem__(self, name):

From 42e7ec4bd2432397927f8d9661455f6681dfc5ac Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sat, 9 May 2015 19:50:58 +0000
Subject: [PATCH 41/43] CLI: use_curses -> text_mode

Previously, the --help output seemed to be broken:

-t, --text            Use the text output instead of the curses UI.
                      (default: True)
---
 letsencrypt/client/cli.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/client/cli.py b/letsencrypt/client/cli.py
index 762e9a850..a83c0f767 100644
--- a/letsencrypt/client/cli.py
+++ b/letsencrypt/client/cli.py
@@ -236,7 +236,7 @@ def create_parser(plugins):
         help="Turn off confirmation screens, currently used for --revoke")
     add("-e", "--agree-tos", dest="tos", action="store_true",
         help="Skip the end user license agreement screen.")
-    add("-t", "--text", dest="use_curses", action="store_false",
+    add("-t", "--text", dest="text_mode", action="store_true",
         help="Use the text output instead of the curses UI.")
 
     subparsers = parser.add_subparsers(metavar="SUBCOMMAND")
@@ -339,10 +339,10 @@ def main(args=sys.argv[1:]):
     config = configuration.NamespaceConfig(args)
 
     # Displayer
-    if args.use_curses:
-        displayer = display_util.NcursesDisplay()
-    else:
+    if args.text_mode:
         displayer = display_util.FileDisplay(sys.stdout)
+    else:
+        displayer = display_util.NcursesDisplay()
     zope.component.provideUtility(displayer)
 
     # Logging
@@ -350,7 +350,7 @@ def main(args=sys.argv[1:]):
     logger = logging.getLogger()
     logger.setLevel(level)
     logging.debug("Logging level set at %d", level)
-    if args.use_curses:
+    if not args.text_mode:
         logger.addHandler(log.DialogHandler())
 
     logging.debug("Discovered plugins: %r", plugins)

From 771ddf0aaf007bb9fb967ec46cdca1cf39d1cc9f Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sun, 10 May 2015 14:53:59 +0000
Subject: [PATCH 42/43] Update docs for the new CLI

---
 README.rst     | 10 +++++++---
 docs/using.rst |  1 -
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/README.rst b/README.rst
index fac36dbd7..4f170f11b 100644
--- a/README.rst
+++ b/README.rst
@@ -17,11 +17,15 @@ It's all automated:
 * If domain control has been proven, a certificate will get issued and the tool
   will automatically install it.
 
-All you need to do is:
+All you need to do is::
 
-::
+  user@www:~$ sudo letsencrypt -d www.example.org auth
 
-   user@www:~$ sudo letsencrypt -d www.example.org
+and if you have a compatbile web server (Apache), Let's Encrypt can
+not only get a new certificate, but also deploy it and configure your
+server automatically!::
+
+  user@www:~$ sudo letsencrypt -d www.example.org run
 
 
 **Encrypt ALL the things!**
diff --git a/docs/using.rst b/docs/using.rst
index f10966602..daa2425ea 100644
--- a/docs/using.rst
+++ b/docs/using.rst
@@ -54,7 +54,6 @@ Installation
 
    virtualenv --no-site-packages -p python2 venv
    ./venv/bin/pip install -r requirements.txt
-   sudo ./venv/bin/letsencrypt
 
 
 Usage

From a5e927c657a127a4079c17470a5b3f211c8460fb Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Sun, 10 May 2015 14:56:44 +0000
Subject: [PATCH 43/43] name_with_description -> description_with_name

---
 letsencrypt/client/display/ops.py            | 2 +-
 letsencrypt/client/plugins/disco.py          | 6 +++---
 letsencrypt/client/plugins/disco_test.py     | 7 ++++---
 letsencrypt/client/tests/display/ops_test.py | 4 ++--
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py
index dc6992c8c..706e8bd7c 100644
--- a/letsencrypt/client/display/ops.py
+++ b/letsencrypt/client/display/ops.py
@@ -22,7 +22,7 @@ def choose_plugin(prepared, question):
     :rtype: `~.PluginEntryPoint`
 
     """
-    opts = [plugin_ep.name_with_description
+    opts = [plugin_ep.description_with_name
             + (" [Misconfigured]" if plugin_ep.misconfigured else "")
             for plugin_ep in prepared]
 
diff --git a/letsencrypt/client/plugins/disco.py b/letsencrypt/client/plugins/disco.py
index 50e0bce50..6ab110a20 100644
--- a/letsencrypt/client/plugins/disco.py
+++ b/letsencrypt/client/plugins/disco.py
@@ -39,9 +39,9 @@ class PluginEntryPoint(object):
         return self.plugin_cls.description
 
     @property
-    def name_with_description(self):
-        """Name with description. Handy for UI."""
-        return "{0} ({1})".format(self.name, self.description)
+    def description_with_name(self):
+        """Description with name. Handy for UI."""
+        return "{0} ({1})".format(self.description, self.name)
 
     def ifaces(self, *ifaces_groups):
         """Does plugin implements specified interface groups?"""
diff --git a/letsencrypt/client/plugins/disco_test.py b/letsencrypt/client/plugins/disco_test.py
index f5ea9e6ee..88aa1275c 100644
--- a/letsencrypt/client/plugins/disco_test.py
+++ b/letsencrypt/client/plugins/disco_test.py
@@ -52,9 +52,10 @@ class PluginEntryPointTest(unittest.TestCase):
     def test_description(self):
         self.assertEqual("Standalone Authenticator", self.plugin_ep.description)
 
-    def test_name_with_description(self):
-        self.assertTrue(
-            self.plugin_ep.name_with_description.startswith("sa ("))
+    def test_description_with_name(self):
+        self.plugin_ep.plugin_cls = mock.MagicMock(description="Desc")
+        self.assertEqual(
+            "Desc (sa)", self.plugin_ep.description_with_name)
 
     def test_ifaces(self):
         self.assertTrue(self.plugin_ep.ifaces((interfaces.IAuthenticator,)))
diff --git a/letsencrypt/client/tests/display/ops_test.py b/letsencrypt/client/tests/display/ops_test.py
index 7c5c1f74f..4716a5b11 100644
--- a/letsencrypt/client/tests/display/ops_test.py
+++ b/letsencrypt/client/tests/display/ops_test.py
@@ -20,9 +20,9 @@ class ChoosePluginTest(unittest.TestCase):
     def setUp(self):
         zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
         self.mock_apache = mock.Mock(
-            name_with_description="a", misconfigured=True)
+            description_with_name="a", misconfigured=True)
         self.mock_stand = mock.Mock(
-            name_with_description="s", misconfigured=False)
+            description_with_name="s", misconfigured=False)
         self.mock_stand.init().more_info.return_value = "standalone"
         self.plugins = [
             self.mock_apache,