Merge remote-tracking branch 'origin/master' into noninteractive-location

.travis.yml

@@ -73,6 +73,8 @@ addons:
     - le2.wtf
     - le3.wtf
     - nginx.wtf
+    - boulder-mysql
+    - boulder-rabbitmq
   mariadb: "10.0"
   apt:
     sources:

letsencrypt-auto-source/letsencrypt-auto

@@ -348,28 +348,45 @@ BootstrapFreeBsd() {
 }
 
 BootstrapMac() {
-  if ! hash brew 2>/dev/null; then
-      echo "Homebrew not installed.\nDownloading..."
-      ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
+  if hash brew 2>/dev/null; then
+    echo "Using Homebrew to install dependencies..."
+    pkgman=brew
+    pkgcmd="brew install"
+  elif hash port 2>/dev/null; then
+    echo "Using MacPorts to install dependencies..."
+    pkgman=port
+    pkgcmd="$SUDO port install"
+  else
+    echo "No Homebrew/MacPorts; installing Homebrew..."
+    ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
+    pkgman=brew
+    pkgcmd="brew install"
   fi
 
-  if [ -z "$(brew list --versions augeas)" ]; then
-      echo "augeas not installed.\nInstalling augeas from Homebrew..."
-      brew install augeas
+  $pkgcmd augeas
+  $pkgcmd dialog
+  if [ "$(which python)" = "/System/Library/Frameworks/Python.framework/Versions/2.7/bin/python" ]; then
+    # We want to avoid using the system Python because it requires root to use pip.
+    # python.org, MacPorts or HomeBrew Python installations should all be OK.
+    echo "Installing python..."
+    $pkgcmd python
   fi
 
-  if [ -z "$(brew list --versions dialog)" ]; then
-      echo "dialog not installed.\nInstalling dialog from Homebrew..."
-      brew install dialog
+  # Workaround for _dlopen not finding augeas on OS X
+  if [ "$pkgman" = "port" ] && ! [ -e "/usr/local/lib/libaugeas.dylib" ] && [ -e "/opt/local/lib/libaugeas.dylib" ]; then
+    echo "Applying augeas workaround"
+    $SUDO ln -s /opt/local/lib/libaugeas.dylib /usr/local/lib
   fi
 
-  if [ -z "$(brew list --versions python)" ]; then
-      echo "python not installed.\nInstalling python from Homebrew..."
-      brew install python
+  if ! hash pip 2>/dev/null; then
+      echo "pip not installed"
+      echo "Installing pip..."
+      curl --silent --show-error --retry 5 https://bootstrap.pypa.io/get-pip.py | python
   fi
 
   if ! hash virtualenv 2>/dev/null; then
-      echo "virtualenv not installed.\nInstalling with pip..."
+      echo "virtualenv not installed."
+      echo "Installing with pip..."
       pip install virtualenv
   fi
 }

letsencrypt-auto-source/pieces/bootstrappers/mac.sh

@@ -1,26 +1,43 @@
 BootstrapMac() {
-  if ! hash brew 2>/dev/null; then
-      echo "Homebrew not installed.\nDownloading..."
-      ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
+  if hash brew 2>/dev/null; then
+    echo "Using Homebrew to install dependencies..."
+    pkgman=brew
+    pkgcmd="brew install"
+  elif hash port 2>/dev/null; then
+    echo "Using MacPorts to install dependencies..."
+    pkgman=port
+    pkgcmd="$SUDO port install"
+  else
+    echo "No Homebrew/MacPorts; installing Homebrew..."
+    ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
+    pkgman=brew
+    pkgcmd="brew install"
   fi
 
-  if [ -z "$(brew list --versions augeas)" ]; then
-      echo "augeas not installed.\nInstalling augeas from Homebrew..."
-      brew install augeas
+  $pkgcmd augeas
+  $pkgcmd dialog
+  if [ "$(which python)" = "/System/Library/Frameworks/Python.framework/Versions/2.7/bin/python" ]; then
+    # We want to avoid using the system Python because it requires root to use pip.
+    # python.org, MacPorts or HomeBrew Python installations should all be OK.
+    echo "Installing python..."
+    $pkgcmd python
   fi
 
-  if [ -z "$(brew list --versions dialog)" ]; then
-      echo "dialog not installed.\nInstalling dialog from Homebrew..."
-      brew install dialog
+  # Workaround for _dlopen not finding augeas on OS X
+  if [ "$pkgman" = "port" ] && ! [ -e "/usr/local/lib/libaugeas.dylib" ] && [ -e "/opt/local/lib/libaugeas.dylib" ]; then
+    echo "Applying augeas workaround"
+    $SUDO ln -s /opt/local/lib/libaugeas.dylib /usr/local/lib
   fi
 
-  if [ -z "$(brew list --versions python)" ]; then
-      echo "python not installed.\nInstalling python from Homebrew..."
-      brew install python
+  if ! hash pip 2>/dev/null; then
+      echo "pip not installed"
+      echo "Installing pip..."
+      curl --silent --show-error --retry 5 https://bootstrap.pypa.io/get-pip.py | python
   fi
 
   if ! hash virtualenv 2>/dev/null; then
-      echo "virtualenv not installed.\nInstalling with pip..."
+      echo "virtualenv not installed."
+      echo "Installing with pip..."
       pip install virtualenv
   fi
 }

letsencrypt-auto-source/pieces/pipstrap.py

@@ -41,7 +41,7 @@ except ImportError:
             cmd = kwargs.get("args")
             if cmd is None:
                 cmd = popenargs[0]
-            raise CalledProcessError(retcode, cmd, output=output)
+            raise CalledProcessError(retcode, cmd)
         return output
 from sys import exit, version_info
 from tempfile import mkdtemp
@@ -55,7 +55,7 @@ except ImportError:
     from urllib.parse import urlparse  # 3.4
 
 
-__version__ = 1, 1, 0
+__version__ = 1, 1, 1
 
 
 # wheel has a conditional dependency on argparse:

letsencrypt/account.py

@@ -98,7 +98,7 @@ def report_new_account(acc, config):
         recovery_msg = ("If you lose your account credentials, you can "
                         "recover through e-mails sent to {0}.".format(
                             ", ".join(acc.regr.body.emails)))
-        reporter.add_message(recovery_msg, reporter.HIGH_PRIORITY)
+        reporter.add_message(recovery_msg, reporter.MEDIUM_PRIORITY)
 
 
 class AccountMemoryStorage(interfaces.AccountStorage):

letsencrypt/cli.py

@@ -2,7 +2,6 @@
 from __future__ import print_function
 import argparse
 import glob
-import json
 import logging
 import logging.handlers
 import os
@@ -87,6 +86,48 @@ More detailed help:
 """
 
 
+# These argparse parameters should be removed when detecting defaults.
+ARGPARSE_PARAMS_TO_REMOVE = ("const", "nargs", "type",)
+
+
+# These sets are used when to help detect options set by the user.
+EXIT_ACTIONS = set(("help", "version",))
+
+
+ZERO_ARG_ACTIONS = set(("store_const", "store_true",
+                        "store_false", "append_const", "count",))
+
+
+# Maps a config option to a set of config options that may have modified it.
+# This dictionary is used recursively, so if A modifies B and B modifies C,
+# it is determined that C was modified by the user if A was modified.
+VAR_MODIFIERS = {"account": set(("server",)),
+                 "server": set(("dry_run", "staging",)),
+                 "webroot_map": set(("webroot_path",))}
+
+
+def report_config_interaction(modified, modifiers):
+    """Registers config option interaction to be checked by set_by_cli.
+
+    This function can be called by during the __init__ or
+    add_parser_arguments methods of plugins to register interactions
+    between config options.
+
+    :param modified: config options that can be modified by modifiers
+    :type modified: iterable or str
+    :param modifiers: config options that modify modified
+    :type modifiers: iterable or str
+
+    """
+    if isinstance(modified, str):
+        modified = (modified,)
+    if isinstance(modifiers, str):
+        modifiers = (modifiers,)
+
+    for var in modified:
+        VAR_MODIFIERS.setdefault(var, set()).update(modifiers)
+
+
 def usage_strings(plugins):
     """Make usage strings late so that plugins can be initialised late"""
     if "nginx" in plugins:
@@ -100,6 +141,22 @@ def usage_strings(plugins):
     return USAGE % (apache_doc, nginx_doc), SHORT_USAGE
 
 
+class _Default(object):
+    """A class to use as a default to detect if a value is set by a user"""
+
+    def __bool__(self):
+        return False
+
+    def __eq__(self, other):
+        return isinstance(other, _Default)
+
+    def __hash__(self):
+        return id(_Default)
+
+    def __nonzero__(self):
+        return self.__bool__()
+
+
 def set_by_cli(var):
     """
     Return True if a particular config variable has been set by the user
@@ -116,30 +173,18 @@ def set_by_cli(var):
         detector = set_by_cli.detector = prepare_and_parse_args(
             plugins, reconstructed_args, detect_defaults=True)
         # propagate plugin requests: eg --standalone modifies config.authenticator
-        auth, inst = plugin_selection.cli_plugin_requests(detector)
-        detector.authenticator = auth if auth else ""
-        detector.installer = inst if inst else ""
+        detector.authenticator, detector.installer = (
+            plugin_selection.cli_plugin_requests(detector))
         logger.debug("Default Detector is %r", detector)
 
-    try:
-        # Is detector.var something that isn't false?
-        change_detected = getattr(detector, var)
-    except AttributeError:
-        logger.warning("Missing default analysis for %r", var)
-        return False
+    if not isinstance(getattr(detector, var), _Default):
+        return True
 
-    if change_detected:
-        return True
-    # Special case: we actually want account to be set to "" if the server
-    # the account was on has changed
-    elif var == "account" and (detector.server or detector.dry_run or detector.staging):
-        return True
-    # Special case: vars like --no-redirect that get set True -> False
-    # default to None; False means they were set
-    elif var in detector.store_false_vars and change_detected is not None:
-        return True
-    else:
-        return False
+    for modifier in VAR_MODIFIERS.get(var, []):
+        if set_by_cli(modifier):
+            return True
+
+    return False
 # static housekeeping var
 set_by_cli.detector = None
 
@@ -188,21 +233,23 @@ def config_help(name, hidden=False):
         return interfaces.IConfig[name].__doc__
 
 
-class SilentParser(object):  # pylint: disable=too-few-public-methods
-    """Silent wrapper around argparse.
+class HelpfulArgumentGroup(object):
+    """Emulates an argparse group for use with HelpfulArgumentParser.
 
-    A mini parser wrapper that doesn't print help for its
-    arguments. This is needed for the use of callbacks to define
-    arguments within plugins.
+    This class is used in the add_group method of HelpfulArgumentParser.
+    Command line arguments can be added to the group, but help
+    suppression and default detection is applied by
+    HelpfulArgumentParser when necessary.
 
     """
-    def __init__(self, parser):
-        self.parser = parser
+    def __init__(self, helpful_arg_parser, topic):
+        self._parser = helpful_arg_parser
+        self._topic = topic
 
     def add_argument(self, *args, **kwargs):
-        """Wrap, but silence help"""
-        kwargs["help"] = argparse.SUPPRESS
-        self.parser.add_argument(*args, **kwargs)
+        """Add a new command line argument to the argument group."""
+        self._parser.add(self._topic, *args, **kwargs)
+
 
 class HelpfulArgumentParser(object):
     """Argparse Wrapper.
@@ -235,15 +282,8 @@ class HelpfulArgumentParser(object):
 
         # This is the only way to turn off overly verbose config flag documentation
         self.parser._add_config_file_help = False  # pylint: disable=protected-access
-        self.silent_parser = SilentParser(self.parser)
 
-        # This setting attempts to force all default values to things that are
-        # pythonically false; it is used to detect when values have been
-        # explicitly set by the user, including when they are set to their
-        # normal default value
         self.detect_defaults = detect_defaults
-        if detect_defaults:
-            self.store_false_vars = {}  # vars that use "store_false"
 
         self.args = args
         self.determine_verb()
@@ -269,24 +309,20 @@ class HelpfulArgumentParser(object):
         parsed_args.func = self.VERBS[self.verb]
         parsed_args.verb = self.verb
 
+        if self.detect_defaults:
+            return parsed_args
+
         # Do any post-parsing homework here
 
         if self.verb == "renew":
             parsed_args.noninteractive_mode = True
 
-        # we get domains from -d, but also from the webroot map...
-        if parsed_args.webroot_map:
-            for domain in parsed_args.webroot_map.keys():
-                if domain not in parsed_args.domains:
-                    parsed_args.domains.append(domain)
-
         if parsed_args.staging or parsed_args.dry_run:
             if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
                 conflicts = ["--staging"] if parsed_args.staging else []
                 conflicts += ["--dry-run"] if parsed_args.dry_run else []
-                if not self.detect_defaults:
-                    raise errors.Error("--server value conflicts with {0}".format(
-                        " and ".join(conflicts)))
+                raise errors.Error("--server value conflicts with {0}".format(
+                    " and ".join(conflicts)))
 
             parsed_args.server = constants.STAGING_URI
 
@@ -298,7 +334,7 @@ class HelpfulArgumentParser(object):
                 if glob.glob(os.path.join(parsed_args.config_dir, constants.ACCOUNTS_DIR, "*")):
                     # The user has a prod account, but might not have a staging
                     # one; we don't want to start trying to perform interactive registration
-                    parsed_args.agree_tos = True
+                    parsed_args.tos = True
                     parsed_args.register_unsafely_without_email = True
 
         if parsed_args.csr:
@@ -307,18 +343,12 @@ class HelpfulArgumentParser(object):
                                    "cannot be used with --csr")
             self.handle_csr(parsed_args)
 
-        if self.detect_defaults:  # plumbing
-            parsed_args.store_false_vars = self.store_false_vars
-
         hooks.validate_hooks(parsed_args)
 
         return parsed_args
 
     def handle_csr(self, parsed_args):
-        """
-        Process a --csr flag. This needs to happen early enough that the
-        webroot plugin can know about the calls to process_domain
-        """
+        """Process a --csr flag."""
         if parsed_args.verb != "certonly":
             raise errors.Error("Currently, a CSR file may only be specified "
                                "when obtaining a new or replacement "
@@ -339,14 +369,11 @@ class HelpfulArgumentParser(object):
                 logger.debug("DER CSR parse error %s", e1)
                 logger.debug("PEM CSR parse error %s", traceback.format_exc())
                 raise errors.Error("Failed to parse CSR file: {0}".format(parsed_args.csr[0]))
-        for d in domains:
-            process_domain(parsed_args, d)
 
-        for d in domains:
-            sanitised = le_util.enforce_domain_sanity(d)
-            if d.lower() != sanitised:
-                raise errors.ConfigurationError(
-                    "CSR domain {0} needs to be sanitised to {1}.".format(d, sanitised))
+        # This is not necessary for webroot to work, however,
+        # obtain_certificate_from_csr requires parsed_args.domains to be set
+        for domain in domains:
+            add_domains(parsed_args, domain)
 
         if not domains:
             # TODO: add CN to domains instead:
@@ -418,7 +445,7 @@ class HelpfulArgumentParser(object):
         """
 
         if self.detect_defaults:
-            kwargs = self.modify_arg_for_default_detection(self, *args, **kwargs)
+            kwargs = self.modify_kwargs_for_default_detection(**kwargs)
 
         if self.visible_topics[topic]:
             if topic in self.groups:
@@ -430,39 +457,28 @@ class HelpfulArgumentParser(object):
             kwargs["help"] = argparse.SUPPRESS
             self.parser.add_argument(*args, **kwargs)
 
+    def modify_kwargs_for_default_detection(self, **kwargs):
+        """Modify an arg so we can check if it was set by the user.
 
-    def modify_arg_for_default_detection(self, *args, **kwargs):
-        """
-        Adding an arg, but ensure that it has a default that evaluates to false,
-        so that set_by_cli can tell if it was set.  Only called if detect_defaults==True.
+        Changes the parameters given to argparse when adding an argument
+        so we can properly detect if the value was set by the user.
 
-        :param list *args: the names of this argument flag
-        :param dict **kwargs: various argparse settings for this argument
+        :param dict kwargs: various argparse settings for this argument
 
         :returns: a modified versions of kwargs
+        :rtype: dict
+
         """
-        # argument either doesn't have a default, or the default doesn't
-        # isn't Pythonically false
-        if kwargs.get("default", True):
-            arg_type = kwargs.get("type", None)
-            if arg_type == int or kwargs.get("action", "") == "count":
-                kwargs["default"] = 0
-            elif arg_type == read_file or "-c" in args:
-                kwargs["default"] = ""
-                kwargs["type"] = str
-            else:
-                kwargs["default"] = ""
-            # This doesn't matter at present (none of the store_false args
-            # are renewal-relevant), but implement it for future sanity:
-            # detect the setting of args whose presence causes True -> False
-        if kwargs.get("action", "") == "store_false":
-            kwargs["default"] = None
-            for var in args:
-                self.store_false_vars[var] = True
+        action = kwargs.get("action", None)
+        if action not in EXIT_ACTIONS:
+            kwargs["action"] = ("store_true" if action in ZERO_ARG_ACTIONS else
+                                "store")
+            kwargs["default"] = _Default()
+            for param in ARGPARSE_PARAMS_TO_REMOVE:
+                kwargs.pop(param, None)
 
         return kwargs
 
-
     def add_deprecated_argument(self, argument_name, num_args):
         """Adds a deprecated argument with the name argument_name.
 
@@ -478,22 +494,22 @@ class HelpfulArgumentParser(object):
             self.parser.add_argument, argument_name, num_args)
 
     def add_group(self, topic, **kwargs):
-        """
+        """Create a new argument group.
 
-        This has to be called once for every topic; but we leave those calls
-        next to the argument definitions for clarity. Return something
-        arguments can be added to if necessary, either the parser or an argument
-        group.
+        This method must be called once for every topic, however, calls
+        to this function are left next to the argument definitions for
+        clarity.
+
+        :param str topic: Name of the new argument group.
+
+        :returns: The new argument group.
+        :rtype: `HelpfulArgumentGroup`
 
         """
         if self.visible_topics[topic]:
-            #print("Adding visible group " + topic)
-            group = self.parser.add_argument_group(topic, **kwargs)
-            self.groups[topic] = group
-            return group
-        else:
-            #print("Invisible group " + topic)
-            return self.silent_parser
+            self.groups[topic] = self.parser.add_argument_group(topic, **kwargs)
+
+        return HelpfulArgumentGroup(self, topic)
 
     def add_plugin_args(self, plugins):
         """
@@ -504,7 +520,6 @@ class HelpfulArgumentParser(object):
         """
         for name, plugin_ep in six.iteritems(plugins):
             parser_or_group = self.add_group(name, description=plugin_ep.description)
-            #print(parser_or_group)
             plugin_ep.plugin_cls.inject_parser_options(parser_or_group, name)
 
     def determine_help_topics(self, chosen_topic):
@@ -585,7 +600,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
     #for subparser in parser_run, parser_auth, parser_install:
     #    subparser.add_argument("domains", nargs="*", metavar="domain")
     helpful.add(None, "-d", "--domains", "--domain", dest="domains",
-                metavar="DOMAIN", action=DomainFlagProcessor, default=[],
+                metavar="DOMAIN", action=_DomainsAction, default=[],
                 help="Domain names to apply. For multiple domains you can use "
                 "multiple -d flags or enter a comma separated list of domains "
                 "as a parameter.")
@@ -613,6 +628,13 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
              "regardless of whether it is near expiry. (Often "
              "--keep-until-expiring is more appropriate). Also implies "
              "--expand.")
+    helpful.add(
+        "automation", "--allow-subset-of-names", action="store_true",
+        help="When performing domain validation, do not consider it a failure "
+             "if authorizations can not be obtained for a strict subset of "
+             "the requested domains. This may be useful for allowing renewals for "
+             "multiple domains to succeed even if some domains no longer point "
+             "at this system. This option cannot be used with --csr.")
     helpful.add(
         "automation", "--agree-tos", dest="tos", action="store_true",
         help="Agree to the Let's Encrypt Subscriber Agreement")
@@ -630,6 +652,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         "automation", "--no-self-upgrade", action="store_true",
         help="(letsencrypt-auto only) prevent the letsencrypt-auto script from"
              " upgrading itself to newer released versions")
+    helpful.add(
+        "automation", "-q", "--quiet", dest="quiet", action="store_true",
+        help="Silence all output except errors. Useful for automation via cron."
+             "Implies --non-interactive.")
 
     helpful.add_group(
         "testing", description="The following flags are meant for "
@@ -690,12 +716,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         "security", "--strict-permissions", action="store_true",
         help="Require that all configuration files are owned by the current "
              "user; only needed if your config is somewhere unsafe like /tmp/")
-    helpful.add(
-        "automation", "--allow-subset-of-names",
-        action="store_true",
-        help="When performing domain validation, do not consider it a failure "
-             "if authorizations can not be obtained for a strict subset of "
-             "the requested domains. This option cannot be used with --csr.")
 
     helpful.add_group(
         "renew", description="The 'renew' subcommand will attempt to renew all"
@@ -861,83 +881,35 @@ def _plugins_parsing(helpful, plugins):
 
     helpful.add_plugin_args(plugins)
 
-    # These would normally be a flag within the webroot plugin, but because
-    # they are parsed in conjunction with --domains, they live here for
-    # legibility. helpful.add_plugin_ags must be called first to add the
-    # "webroot" topic
-    helpful.add("webroot", "-w", "--webroot-path", default=[], action=WebrootPathProcessor,
-                help="public_html / webroot path. This can be specified multiple times to "
-                     "handle different domains; each domain will have the webroot path that"
-                     " preceded it.  For instance: `-w /var/www/example -d example.com -d "
-                     "www.example.com -w /var/www/thing -d thing.net -d m.thing.net`")
-    # --webroot-map still has some awkward properties, so it is undocumented
-    helpful.add("webroot", "--webroot-map", default={}, action=WebrootMapProcessor,
-                help="JSON dictionary mapping domains to webroot paths; this "
-                     "implies -d for each entry. You may need to escape this "
-                     "from your shell. E.g.: --webroot-map "
-                     """'{"eg1.is,m.eg1.is":"/www/eg1/", "eg2.is":"/www/eg2"}' """
-                     "This option is merged with, but takes precedence over, "
-                     "-w / -d entries. At present, if you put webroot-map in "
-                     "a config file, it needs to be on a single line, like: "
-                     'webroot-map = {"example.com":"/var/www"}.')
+
+class _DomainsAction(argparse.Action):
+    """Action class for parsing domains."""
+
+    def __call__(self, parser, namespace, domain, option_string=None):
+        """Just wrap add_domains in argparseese."""
+        add_domains(namespace, domain)
 
 
-class WebrootPathProcessor(argparse.Action):  # pylint: disable=missing-docstring
-    def __init__(self, *args, **kwargs):
-        self.domain_before_webroot = False
-        argparse.Action.__init__(self, *args, **kwargs)
+def add_domains(args_or_config, domains):
+    """Registers new domains to be used during the current client run.
 
-    def __call__(self, parser, args, webroot, option_string=None):
-        """
-        Keep a record of --webroot-path / -w flags during processing, so that
-        we know which apply to which -d flags
-        """
-        if not args.webroot_path:      # first -w flag encountered
-            # if any --domain flags preceded the first --webroot-path flag,
-            # apply that webroot path to those; subsequent entries in
-            # args.webroot_map are filled in by cli.DomainFlagProcessor
-            if args.domains:
-                self.domain_before_webroot = True
-                for d in args.domains:
-                    args.webroot_map.setdefault(d, webroot)
-        elif self.domain_before_webroot:
-            # FIXME if you set domains in a args file, you should get a different error
-            # here, pointing you to --webroot-map
-            raise errors.Error("If you specify multiple webroot paths, one of "
-                               "them must precede all domain flags")
-        args.webroot_path.append(webroot)
+    Domains are not added to the list of requested domains if they have
+    already been registered.
 
+    :param args_or_config: parsed command line arguments
+    :type args_or_config: argparse.Namespace or
+        configuration.NamespaceConfig
+    :param str domain: one or more comma separated domains
 
-def process_domain(args_or_config, domain_arg, webroot_path=None):
-    """
-    Process a new -d flag, helping the webroot plugin construct a map of
-    {domain : webrootpath} if -w / --webroot-path is in use
-
-    :param args_or_config: may be an argparse args object, or a NamespaceConfig object
-    :param str domain_arg: a string representing 1+ domains, eg: "eg.is, example.com"
-    :param str webroot_path: (optional) the webroot_path for these domains
+    :returns: domains after they have been normalized and validated
+    :rtype: `list` of `str`
 
     """
-    webroot_path = webroot_path if webroot_path else args_or_config.webroot_path
-
-    for domain in (d.strip() for d in domain_arg.split(",")):
-        domain = le_util.enforce_domain_sanity(domain)
+    validated_domains = []
+    for domain in domains.split(","):
+        domain = le_util.enforce_domain_sanity(domain.strip())
+        validated_domains.append(domain)
         if domain not in args_or_config.domains:
             args_or_config.domains.append(domain)
-            # Each domain has a webroot_path of the most recent -w flag
-            # unless it was explicitly included in webroot_map
-            if webroot_path:
-                args_or_config.webroot_map.setdefault(domain, webroot_path[-1])
 
-
-class WebrootMapProcessor(argparse.Action):  # pylint: disable=missing-docstring
-    def __call__(self, parser, args, webroot_map_arg, option_string=None):
-        webroot_map = json.loads(webroot_map_arg)
-        for domains, webroot_path in six.iteritems(webroot_map):
-            process_domain(args, domains, [webroot_path])
-
-
-class DomainFlagProcessor(argparse.Action):  # pylint: disable=missing-docstring
-    def __call__(self, parser, args, domain_arg, option_string=None):
-        """Just wrap process_domain in argparseese."""
-        process_domain(args, domain_arg)
+    return validated_domains

letsencrypt/client.py

@@ -105,7 +105,8 @@ def register(config, account_storage, tos_cb=None):
                    "--register-unsafely-without-email was not present.")
             logger.warn(msg)
             raise errors.Error(msg)
-        logger.warn("Registering without email!")
+        if not config.dry_run:
+            logger.warn("Registering without email!")
 
     # Each new registration shall use a fresh new key
     key = jose.JWKRSA(key=jose.ComparableRSAKey(

letsencrypt/hooks.py

@@ -62,7 +62,7 @@ def renew_hook(config, domains, lineage_path):
             os.environ["RENEWED_LINEAGE"] = lineage_path
             _run_hook(config.renew_hook)
         else:
-            print("Dry run: skipping renewal hook command: {0}".format(config.renew_hook))
+            logger.warning("Dry run: skipping renewal hook command: %s", config.renew_hook)
 
 def _run_hook(shell_cmd):
     """Run a hook command.

letsencrypt/main.py

@@ -34,7 +34,6 @@ from letsencrypt.display import util as display_util, ops as display_ops
 from letsencrypt.plugins import disco as plugins_disco
 from letsencrypt.plugins import selection as plug_sel
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -256,14 +255,10 @@ def _find_duplicative_certs(config, domains):
 
 
 def _find_domains(config, installer):
-    if not config.domains:
-        domains = display_ops.choose_names(installer)
-        # record in config.domains (so that it can be serialised in renewal config files),
-        # and set webroot_map entries if applicable
-        for d in domains:
-            cli.process_domain(config, d)
-    else:
+    if config.domains:
         domains = config.domains
+    else:
+        domains = display_ops.choose_names(installer)
 
     if not domains:
         raise errors.Error("Please specify --domains, or --installer that "
@@ -518,19 +513,22 @@ def obtain_cert(config, plugins, lineage=None):
         action = "newcert"
 
     # POSTPRODUCTION: Cleanup, deployment & reporting
+    notify = zope.component.getUtility(interfaces.IDisplay).notification
     if config.dry_run:
         _report_successful_dry_run(config)
     elif config.verb == "renew":
         if installer is None:
-            print("new certificate deployed without reload, fullchain is",
-                  lineage.fullchain)
+            notify("new certificate deployed without reload, fullchain is {0}".format(
+                   lineage.fullchain), pause=False)
         else:
             # In case of a renewal, reload server to pick up new certificate.
             # In principle we could have a configuration option to inhibit this
             # from happening.
             installer.restart()
-            print("new certificate deployed with reload of",
-                  config.installer, "server; fullchain is", lineage.fullchain)
+            notify("new certificate deployed with reload of {0} server; fullchain is {1}".format(
+                   config.installer, lineage.fullchain), pause=False)
+    elif action == "reinstall" and config.verb == "certonly":
+        notify("Certificate not yet due for renewal; no action taken.")
     _suggest_donation_if_appropriate(config, action)
 
 
@@ -672,7 +670,10 @@ def main(cli_args=sys.argv[1:]):
     sys.excepthook = functools.partial(_handle_exception, config=config)
 
     # Displayer
-    if config.noninteractive_mode:
+    if config.quiet:
+        config.noninteractive_mode = True
+        displayer = display_util.NoninteractiveDisplay(open(os.devnull, "w"))
+    elif config.noninteractive_mode:
         displayer = display_util.NoninteractiveDisplay(sys.stdout)
     elif config.text_mode:
         displayer = display_util.FileDisplay(sys.stdout)
@@ -681,7 +682,7 @@ def main(cli_args=sys.argv[1:]):
     zope.component.provideUtility(displayer)
 
     # Reporter
-    report = reporter.Reporter()
+    report = reporter.Reporter(config)
     zope.component.provideUtility(report)
     atexit.register(report.atexit_print_messages)
 

letsencrypt/plugins/common.py

@@ -45,15 +45,14 @@ class Plugin(object):
     def add_parser_arguments(cls, add):
         """Add plugin arguments to the CLI argument parser.
 
+        NOTE: If some of your flags interact with others, you can
+        use cli.report_config_interaction to register this to ensure
+        values are correctly saved/overridable during renewal.
+
         :param callable add: Function that proxies calls to
             `argparse.ArgumentParser.add_argument` prepending options
             with unique plugin name prefix.
 
-        NOTE: if you add argpase arguments such that users setting them can
-        create a config entry that python's bool() would consider false (ie,
-        the use might set the variable to "", [], 0, etc), please ensure that
-        cli.set_by_cli() works for your variable.
-
         """
 
     @classmethod

letsencrypt/plugins/webroot.py

@@ -1,16 +1,21 @@
 """Webroot plugin."""
+import argparse
+import collections
 import errno
+import json
 import logging
 import os
-from collections import defaultdict
 
-import zope.interface
 import six
+import zope.component
+import zope.interface
 
 from acme import challenges
 
+from letsencrypt import cli
 from letsencrypt import errors
 from letsencrypt import interfaces
+from letsencrypt.display import util as display_util
 from letsencrypt.plugins import common
 
 
@@ -35,9 +40,21 @@ to serve all files under specified web root ({0})."""
 
     @classmethod
     def add_parser_arguments(cls, add):
-        # --webroot-path and --webroot-map are added in cli.py because they
-        # are parsed in conjunction with --domains
-        pass
+        add("path", "-w", default=[], action=_WebrootPathAction,
+            help="public_html / webroot path. This can be specified multiple "
+                 "times to handle different domains; each domain will have "
+                 "the webroot path that preceded it.  For instance: `-w "
+                 "/var/www/example -d example.com -d www.example.com -w "
+                 "/var/www/thing -d thing.net -d m.thing.net`")
+        add("map", default={}, action=_WebrootMapAction,
+            help="JSON dictionary mapping domains to webroot paths; this "
+                 "implies -d for each entry. You may need to escape this from "
+                 "your shell. E.g.: --webroot-map "
+                 '\'{"eg1.is,m.eg1.is":"/www/eg1/", "eg2.is":"/www/eg2"}\' '
+                 "This option is merged with, but takes precedence over, -w / "
+                 "-d entries. At present, if you put webroot-map in a config "
+                 "file, it needs to be on a single line, like: webroot-map = "
+                 '{"example.com":"/var/www"}.')
 
     def get_chall_pref(self, domain):  # pragma: no cover
         # pylint: disable=missing-docstring,no-self-use,unused-argument
@@ -46,19 +63,102 @@ to serve all files under specified web root ({0})."""
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
         self.full_roots = {}
-        self.performed = defaultdict(set)
+        self.performed = collections.defaultdict(set)
 
     def prepare(self):  # pylint: disable=missing-docstring
-        path_map = self.conf("map")
+        pass
 
+    def perform(self, achalls):  # pylint: disable=missing-docstring
+        self._set_webroots(achalls)
+
+        self._create_challenge_dirs()
+
+        return [self._perform_single(achall) for achall in achalls]
+
+    def _set_webroots(self, achalls):
+        if self.conf("path"):
+            webroot_path = self.conf("path")[-1]
+            logger.info("Using the webroot path %s for all unmatched domains.",
+                        webroot_path)
+            for achall in achalls:
+                self.conf("map").setdefault(achall.domain, webroot_path)
+        else:
+            known_webroots = list(set(six.itervalues(self.conf("map"))))
+            for achall in achalls:
+                if achall.domain not in self.conf("map"):
+                    new_webroot = self._prompt_for_webroot(achall.domain,
+                                                           known_webroots)
+                    # Put the most recently input
+                    # webroot first for easy selection
+                    try:
+                        known_webroots.remove(new_webroot)
+                    except ValueError:
+                        pass
+                    known_webroots.insert(0, new_webroot)
+                    self.conf("map")[achall.domain] = new_webroot
+
+    def _prompt_for_webroot(self, domain, known_webroots):
+        webroot = None
+
+        while webroot is None:
+            webroot = self._prompt_with_webroot_list(domain, known_webroots)
+
+            if webroot is None:
+                webroot = self._prompt_for_new_webroot(domain)
+
+        return webroot
+
+    def _prompt_with_webroot_list(self, domain, known_webroots):
+        display = zope.component.getUtility(interfaces.IDisplay)
+
+        while True:
+            code, index = display.menu(
+                "Select the webroot for {0}:".format(domain),
+                ["Enter a new webroot"] + known_webroots,
+                help_label="Help", cli_flag="--" + self.option_name("path"))
+            if code == display_util.CANCEL:
+                raise errors.PluginError(
+                    "Every requested domain must have a "
+                    "webroot when using the webroot plugin.")
+            elif code == display_util.HELP:
+                display.notification(
+                    "To use the webroot plugin, you need to have an "
+                    "HTTP server running on this system serving files "
+                    "for the requested domain. Additionally, this "
+                    "server should be serving all files contained in a "
+                    "public_html or webroot directory. The webroot "
+                    "plugin works by temporarily saving necessary "
+                    "resources in the HTTP server's webroot directory "
+                    "to pass domain validation challenges.")
+            else:  # code == display_util.OK
+                return None if index == 0 else known_webroots[index - 1]
+
+    def _prompt_for_new_webroot(self, domain):
+        display = zope.component.getUtility(interfaces.IDisplay)
+
+        while True:
+            code, webroot = display.directory_select(
+                "Input the webroot for {0}:".format(domain))
+            if code == display_util.HELP:
+                # Help can currently only be selected
+                # when using the ncurses interface
+                display.notification(display_util.DSELECT_HELP)
+            elif code == display_util.CANCEL:
+                return None
+            else:  # code == display_util.OK
+                try:
+                    return _validate_webroot(webroot)
+                except errors.PluginError as error:
+                    display.notification(str(error), pause=False)
+
+    def _create_challenge_dirs(self):
+        path_map = self.conf("map")
         if not path_map:
             raise errors.PluginError(
                 "Missing parts of webroot configuration; please set either "
                 "--webroot-path and --domains, or --webroot-map. Run with "
                 " --help webroot for examples.")
         for name, path in path_map.items():
-            if not os.path.isdir(path):
-                raise errors.PluginError(path + " does not exist or is not a directory")
             self.full_roots[name] = os.path.join(path, challenges.HTTP01.URI_ROOT_PATH)
 
             logger.debug("Creating root challenges validation dir at %s",
@@ -96,28 +196,13 @@ to serve all files under specified web root ({0})."""
             finally:
                 os.umask(old_umask)
 
-    def perform(self, achalls):  # pylint: disable=missing-docstring
-        assert self.full_roots, "Webroot plugin appears to be missing webroot map"
-        return [self._perform_single(achall) for achall in achalls]
-
-    def _get_root_path(self, achall):
-        try:
-            path = self.full_roots[achall.domain]
-        except KeyError:
-            raise errors.PluginError("Missing --webroot-path for domain: {0}"
-                                     .format(achall.domain))
-        if not os.path.exists(path):
-            raise errors.PluginError("Mysteriously missing path {0} for domain: {1}"
-                                     .format(path, achall.domain))
-        return path
-
     def _get_validation_path(self, root_path, achall):
         return os.path.join(root_path, achall.chall.encode("token"))
 
     def _perform_single(self, achall):
         response, validation = achall.response_and_validation()
 
-        root_path = self._get_root_path(achall)
+        root_path = self.full_roots[achall.domain]
         validation_path = self._get_validation_path(root_path, achall)
         logger.debug("Attempting to save validation to %s", validation_path)
 
@@ -136,11 +221,12 @@ to serve all files under specified web root ({0})."""
 
     def cleanup(self, achalls):  # pylint: disable=missing-docstring
         for achall in achalls:
-            root_path = self._get_root_path(achall)
-            validation_path = self._get_validation_path(root_path, achall)
-            logger.debug("Removing %s", validation_path)
-            os.remove(validation_path)
-            self.performed[root_path].remove(achall)
+            root_path = self.full_roots.get(achall.domain, None)
+            if root_path is not None:
+                validation_path = self._get_validation_path(root_path, achall)
+                logger.debug("Removing %s", validation_path)
+                os.remove(validation_path)
+                self.performed[root_path].remove(achall)
 
         for root_path, achalls in six.iteritems(self.performed):
             if not achalls:
@@ -157,3 +243,53 @@ to serve all files under specified web root ({0})."""
                                      root_path)
                     else:
                         raise
+
+
+class _WebrootMapAction(argparse.Action):
+    """Action class for parsing webroot_map."""
+
+    def __call__(self, parser, namespace, webroot_map, option_string=None):
+        for domains, webroot_path in six.iteritems(json.loads(webroot_map)):
+            webroot_path = _validate_webroot(webroot_path)
+            namespace.webroot_map.update(
+                (d, webroot_path) for d in cli.add_domains(namespace, domains))
+
+
+class _WebrootPathAction(argparse.Action):
+    """Action class for parsing webroot_path."""
+
+    def __init__(self, *args, **kwargs):
+        super(_WebrootPathAction, self).__init__(*args, **kwargs)
+        self._domain_before_webroot = False
+
+    def __call__(self, parser, namespace, webroot_path, option_string=None):
+        if self._domain_before_webroot:
+            raise errors.PluginError(
+                "If you specify multiple webroot paths, "
+                "one of them must precede all domain flags")
+
+        if namespace.webroot_path:
+            # Apply previous webroot to all matched
+            # domains before setting the new webroot path
+            prev_webroot = namespace.webroot_path[-1]
+            for domain in namespace.domains:
+                namespace.webroot_map.setdefault(domain, prev_webroot)
+        elif namespace.domains:
+            self._domain_before_webroot = True
+
+        namespace.webroot_path.append(_validate_webroot(webroot_path))
+
+
+def _validate_webroot(webroot_path):
+    """Validates and returns the absolute path of webroot_path.
+
+    :param str webroot_path: path to the webroot directory
+
+    :returns: absolute path of webroot_path
+    :rtype: str
+
+    """
+    if not os.path.isdir(webroot_path):
+        raise errors.PluginError(webroot_path + " does not exist or is not a directory")
+
+    return os.path.abspath(webroot_path)

letsencrypt/plugins/webroot_test.py

@@ -2,6 +2,7 @@
 
 from __future__ import print_function
 
+import argparse
 import errno
 import os
 import shutil
@@ -10,12 +11,14 @@ import tempfile
 import unittest
 
 import mock
+import six
 
 from acme import challenges
 from acme import jose
 
 from letsencrypt import achallenges
 from letsencrypt import errors
+from letsencrypt.display import util as display_util
 
 from letsencrypt.tests import acme_util
 from letsencrypt.tests import test_util
@@ -53,23 +56,74 @@ class AuthenticatorTest(unittest.TestCase):
     def test_add_parser_arguments(self):
         add = mock.MagicMock()
         self.auth.add_parser_arguments(add)
-        self.assertEqual(0, add.call_count)  # args moved to cli.py!
+        self.assertEqual(2, add.call_count)
 
-    def test_prepare_bad_root(self):
-        self.config.webroot_path = os.path.join(self.path, "null")
-        self.config.webroot_map["thing.com"] = self.config.webroot_path
-        self.assertRaises(errors.PluginError, self.auth.prepare)
-
-    def test_prepare_missing_root(self):
-        self.config.webroot_path = None
-        self.config.webroot_map = {}
-        self.assertRaises(errors.PluginError, self.auth.prepare)
-
-    def test_prepare_full_root_exists(self):
-        # prepare() has already been called once in setUp()
+    def test_prepare(self):
         self.auth.prepare()  # shouldn't raise any exceptions
 
-    def test_prepare_reraises_other_errors(self):
+    @mock.patch("letsencrypt.plugins.webroot.zope.component.getUtility")
+    def test_webroot_from_list(self, mock_get_utility):
+        self.config.webroot_path = []
+        self.config.webroot_map = {"otherthing.com": self.path}
+        mock_display = mock_get_utility()
+        mock_display.menu.return_value = (display_util.OK, 1,)
+
+        self.auth.perform([self.achall])
+        self.assertTrue(mock_display.menu.called)
+        for call in mock_display.menu.call_args_list:
+            self.assertTrue(self.achall.domain in call[0][0])
+            self.assertTrue(all(
+                webroot in call[0][1]
+                for webroot in six.itervalues(self.config.webroot_map)))
+        self.assertEqual(self.config.webroot_map[self.achall.domain],
+                         self.path)
+
+    @mock.patch("letsencrypt.plugins.webroot.zope.component.getUtility")
+    def test_webroot_from_list_help_and_cancel(self, mock_get_utility):
+        self.config.webroot_path = []
+        self.config.webroot_map = {"otherthing.com": self.path}
+
+        mock_display = mock_get_utility()
+        mock_display.menu.side_effect = ((display_util.HELP, -1),
+                                         (display_util.CANCEL, -1),)
+        self.assertRaises(errors.PluginError, self.auth.perform, [self.achall])
+        self.assertTrue(mock_display.notification.called)
+        self.assertTrue(mock_display.menu.called)
+        for call in mock_display.menu.call_args_list:
+            self.assertTrue(self.achall.domain in call[0][0])
+            self.assertTrue(all(
+                webroot in call[0][1]
+                for webroot in six.itervalues(self.config.webroot_map)))
+
+    @mock.patch("letsencrypt.plugins.webroot.zope.component.getUtility")
+    def test_new_webroot(self, mock_get_utility):
+        self.config.webroot_path = []
+        self.config.webroot_map = {}
+
+        imaginary_dir = os.path.join(os.sep, "imaginary", "dir")
+
+        mock_display = mock_get_utility()
+        mock_display.menu.return_value = (display_util.OK, 0,)
+        mock_display.directory_select.side_effect = (
+            (display_util.HELP, -1,), (display_util.CANCEL, -1,),
+            (display_util.OK, imaginary_dir,), (display_util.OK, self.path,),)
+        self.auth.perform([self.achall])
+
+        self.assertTrue(mock_display.notification.called)
+        for call in mock_display.notification.call_args_list:
+            self.assertTrue(imaginary_dir in call[0][0] or
+                            display_util.DSELECT_HELP == call[0][0])
+
+        self.assertTrue(mock_display.directory_select.called)
+        for call in mock_display.directory_select.call_args_list:
+            self.assertTrue(self.achall.domain in call[0][0])
+
+    def test_perform_missing_root(self):
+        self.config.webroot_path = None
+        self.config.webroot_map = {}
+        self.assertRaises(errors.PluginError, self.auth.perform, [])
+
+    def test_perform_reraises_other_errors(self):
         self.auth.full_path = os.path.join(self.path, "null")
         permission_canary = os.path.join(self.path, "rnd")
         with open(permission_canary, "w") as f:
@@ -80,20 +134,20 @@ class AuthenticatorTest(unittest.TestCase):
             print("Warning, running tests as root skips permissions tests...")
         except IOError:
             # ok, permissions work, test away...
-            self.assertRaises(errors.PluginError, self.auth.prepare)
+            self.assertRaises(errors.PluginError, self.auth.perform, [])
         os.chmod(self.path, 0o700)
 
     @mock.patch("letsencrypt.plugins.webroot.os.chown")
     def test_failed_chown_eacces(self, mock_chown):
         mock_chown.side_effect = OSError(errno.EACCES, "msg")
-        self.auth.prepare()  # exception caught and logged
+        self.auth.perform([self.achall])  # exception caught and logged
 
     @mock.patch("letsencrypt.plugins.webroot.os.chown")
     def test_failed_chown_not_eacces(self, mock_chown):
         mock_chown.side_effect = OSError()
-        self.assertRaises(errors.PluginError, self.auth.prepare)
+        self.assertRaises(errors.PluginError, self.auth.perform, [])
 
-    def test_prepare_permissions(self):
+    def test_perform_permissions(self):
         self.auth.prepare()
 
         # Remove exec bit from permission check, so that it
@@ -116,18 +170,6 @@ class AuthenticatorTest(unittest.TestCase):
         self.assertEqual(os.stat(self.validation_path).st_gid, parent_gid)
         self.assertEqual(os.stat(self.validation_path).st_uid, parent_uid)
 
-    def test_perform_missing_path(self):
-        self.auth.prepare()
-
-        missing_achall = achallenges.KeyAuthorizationAnnotatedChallenge(
-            challb=acme_util.HTTP01_P, domain="thing2.com", account_key=KEY)
-        self.assertRaises(
-            errors.PluginError, self.auth.perform, [missing_achall])
-
-        self.auth.full_roots[self.achall.domain] = 'null'
-        self.assertRaises(
-            errors.PluginError, self.auth.perform, [self.achall])
-
     def test_perform_cleanup(self):
         self.auth.prepare()
         responses = self.auth.perform([self.achall])
@@ -183,5 +225,52 @@ class AuthenticatorTest(unittest.TestCase):
         self.assertFalse(os.path.exists(self.validation_path))
         self.assertTrue(os.path.exists(self.root_challenge_path))
 
+
+class WebrootActionTest(unittest.TestCase):
+    """Tests for webroot argparse actions."""
+
+    achall = achallenges.KeyAuthorizationAnnotatedChallenge(
+        challb=acme_util.HTTP01_P, domain="thing.com", account_key=KEY)
+
+    def setUp(self):
+        from letsencrypt.plugins.webroot import Authenticator
+        self.path = tempfile.mkdtemp()
+        self.parser = argparse.ArgumentParser()
+        self.parser.add_argument("-d", "--domains",
+                                 action="append", default=[])
+        Authenticator.inject_parser_options(self.parser, "webroot")
+
+    def test_webroot_map_action(self):
+        args = self.parser.parse_args(
+            ["--webroot-map", '{{"thing.com":"{0}"}}'.format(self.path)])
+        self.assertEqual(args.webroot_map["thing.com"], self.path)
+
+    def test_domain_before_webroot(self):
+        args = self.parser.parse_args(
+            "-d {0} -w {1}".format(self.achall.domain, self.path).split())
+        config = self._get_config_after_perform(args)
+        self.assertEqual(config.webroot_map[self.achall.domain], self.path)
+
+    def test_domain_before_webroot_error(self):
+        self.assertRaises(errors.PluginError, self.parser.parse_args,
+                          "-d foo -w bar -w baz".split())
+        self.assertRaises(errors.PluginError, self.parser.parse_args,
+                          "-d foo -w bar -d baz -w qux".split())
+
+    def test_multiwebroot(self):
+        args = self.parser.parse_args("-w {0} -d {1} -w {2} -d bar".format(
+            self.path, self.achall.domain, tempfile.mkdtemp()).split())
+        self.assertEqual(args.webroot_map[self.achall.domain], self.path)
+        config = self._get_config_after_perform(args)
+        self.assertEqual(
+            config.webroot_map[self.achall.domain], self.path)
+
+    def _get_config_after_perform(self, config):
+        from letsencrypt.plugins.webroot import Authenticator
+        auth = Authenticator(config, "webroot")
+        auth.perform([self.achall])
+        return auth.config
+
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

letsencrypt/renewal.py

@@ -17,6 +17,8 @@ from letsencrypt import constants
 
 from letsencrypt import crypto_util
 from letsencrypt import errors
+from letsencrypt import interfaces
+from letsencrypt import le_util
 from letsencrypt import hooks
 from letsencrypt import storage
 from letsencrypt.plugins import disco as plugins_disco
@@ -84,8 +86,8 @@ def _reconstitute(config, full_path):
         return None
 
     try:
-        for d in renewal_candidate.names():
-            cli.process_domain(config, d)
+        config.domains = [le_util.enforce_domain_sanity(d)
+                          for d in renewal_candidate.names()]
     except errors.ConfigurationError as error:
         logger.warning("Renewal configuration file %s references a cert "
                        "that contains an invalid domain name. The problem "
@@ -102,16 +104,14 @@ def _restore_webroot_config(config, renewalparams):
     form.
     """
     if "webroot_map" in renewalparams:
-        # if the user does anything that would create a new webroot map on the
-        # CLI, don't use the old one
-        if not (cli.set_by_cli("webroot_map") or cli.set_by_cli("webroot_path")):
-            setattr(config.namespace, "webroot_map", renewalparams["webroot_map"])
+        if not cli.set_by_cli("webroot_map"):
+            config.namespace.webroot_map = renewalparams["webroot_map"]
     elif "webroot_path" in renewalparams:
         logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path")
         wp = renewalparams["webroot_path"]
         if isinstance(wp, str):  # prior to 0.1.0, webroot_path was a string
             wp = [wp]
-        setattr(config.namespace, "webroot_path", wp)
+        config.namespace.webroot_path = wp
 
 
 def _restore_plugin_configs(config, renewalparams):
@@ -230,7 +230,8 @@ def _avoid_invalidating_lineage(config, lineage, original_server):
 
 def renew_cert(config, domains, le_client, lineage):
     "Renew a certificate lineage."
-    original_server = lineage.configuration["renewalparams"]["server"]
+    renewal_params = lineage.configuration["renewalparams"]
+    original_server = renewal_params.get("server", cli.flag_default("server"))
     _avoid_invalidating_lineage(config, lineage, original_server)
     new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains)
     if config.dry_run:
@@ -242,48 +243,59 @@ def renew_cert(config, domains, le_client, lineage):
             OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped)
         new_chain = crypto_util.dump_pyopenssl_chain(new_chain)
         renewal_conf = configuration.RenewerConfiguration(config.namespace)
+        # TODO: Check return value of save_successor
         lineage.save_successor(prior_version, new_cert, new_key.pem, new_chain, renewal_conf)
         lineage.update_all_links_to(lineage.latest_common_version())
 
     hooks.renew_hook(config, domains, lineage.live_dir)
-    # TODO: Check return value of save_successor
 
 
+def report(msgs, category):
+    "Format a results report for a category of renewal outcomes"
+    lines = ("%s (%s)" % (m, category) for m in msgs)
+    return "  " + "\n  ".join(lines)
+
 def _renew_describe_results(config, renew_successes, renew_failures,
                             renew_skipped, parse_failures):
-    def _status(msgs, category):
-        return "  " + "\n  ".join("%s (%s)" % (m, category) for m in msgs)
+
+    out = []
+    notify = out.append
+
     if config.dry_run:
-        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
-        print("**          (The test certificates below have not been saved.)")
-    print()
+        notify("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
+        notify("**          (The test certificates below have not been saved.)")
+    notify("")
     if renew_skipped:
-        print("The following certs are not due for renewal yet:")
-        print(_status(renew_skipped, "skipped"))
+        notify("The following certs are not due for renewal yet:")
+        notify(report(renew_skipped, "skipped"))
     if not renew_successes and not renew_failures:
-        print("No renewals were attempted.")
+        notify("No renewals were attempted.")
     elif renew_successes and not renew_failures:
-        print("Congratulations, all renewals succeeded. The following certs "
-              "have been renewed:")
-        print(_status(renew_successes, "success"))
+        notify("Congratulations, all renewals succeeded. The following certs "
+               "have been renewed:")
+        notify(report(renew_successes, "success"))
     elif renew_failures and not renew_successes:
-        print("All renewal attempts failed. The following certs could not be "
-              "renewed:")
-        print(_status(renew_failures, "failure"))
+        notify("All renewal attempts failed. The following certs could not be "
+               "renewed:")
+        notify(report(renew_failures, "failure"))
     elif renew_failures and renew_successes:
-        print("The following certs were successfully renewed:")
-        print(_status(renew_successes, "success"))
-        print("\nThe following certs could not be renewed:")
-        print(_status(renew_failures, "failure"))
+        notify("The following certs were successfully renewed:")
+        notify(report(renew_successes, "success"))
+        notify("\nThe following certs could not be renewed:")
+        notify(report(renew_failures, "failure"))
 
     if parse_failures:
-        print("\nAdditionally, the following renewal configuration files "
-              "were invalid: ")
-        print(_status(parse_failures, "parsefail"))
+        notify("\nAdditionally, the following renewal configuration files "
+               "were invalid: ")
+        notify(parse_failures, "parsefail")
 
     if config.dry_run:
-        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
-        print("**          (The test certificates above have not been saved.)")
+        notify("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
+        notify("**          (The test certificates above have not been saved.)")
+
+    if config.quiet and not (renew_failures or parse_failures):
+        return
+    print("\n".join(out))
 
 
 def renew_all_lineages(config):
@@ -303,7 +315,8 @@ def renew_all_lineages(config):
     renew_skipped = []
     parse_failures = []
     for renewal_file in renewal_conf_files(renewer_config):
-        print("Processing " + renewal_file)
+        disp = zope.component.getUtility(interfaces.IDisplay)
+        disp.notification("Processing " + renewal_file, pause=False)
         lineage_config = copy.deepcopy(config)
 
         # Note that this modifies config (to add back the configuration

letsencrypt/reporter.py

@@ -35,8 +35,9 @@ class Reporter(object):
 
     _msg_type = collections.namedtuple('ReporterMsg', 'priority text on_crash')
 
-    def __init__(self):
+    def __init__(self, config):
         self.messages = queue.PriorityQueue()
+        self.config = config
 
     def add_message(self, msg, priority, on_crash=True):
         """Adds msg to the list of messages to be printed.
@@ -76,9 +77,10 @@ class Reporter(object):
         if not self.messages.empty():
             no_exception = sys.exc_info()[0] is None
             bold_on = sys.stdout.isatty()
-            if bold_on:
-                print(le_util.ANSI_SGR_BOLD)
-            print('IMPORTANT NOTES:')
+            if not self.config.quiet:
+                if bold_on:
+                    print(le_util.ANSI_SGR_BOLD)
+                print('IMPORTANT NOTES:')
             first_wrapper = textwrap.TextWrapper(
                 initial_indent=' - ', subsequent_indent=(' ' * 3))
             next_wrapper = textwrap.TextWrapper(
@@ -86,14 +88,20 @@ class Reporter(object):
                 subsequent_indent=first_wrapper.subsequent_indent)
         while not self.messages.empty():
             msg = self.messages.get()
+            if self.config.quiet:
+                # In --quiet mode, we only print high priority messages that
+                # are flagged for crash cases
+                if not (msg.priority == self.HIGH_PRIORITY and msg.on_crash):
+                    continue
             if no_exception or msg.on_crash:
                 if bold_on and msg.priority > self.HIGH_PRIORITY:
-                    sys.stdout.write(le_util.ANSI_SGR_RESET)
-                    bold_on = False
+                    if not self.config.quiet:
+                        sys.stdout.write(le_util.ANSI_SGR_RESET)
+                        bold_on = False
                 lines = msg.text.splitlines()
                 print(first_wrapper.fill(lines[0]))
                 if len(lines) > 1:
                     print("\n".join(
                         next_wrapper.fill(line) for line in lines[1:]))
-        if bold_on:
+        if bold_on and not self.config.quiet:
             sys.stdout.write(le_util.ANSI_SGR_RESET)

letsencrypt/tests/cli_test.py

@@ -12,6 +12,7 @@ import unittest
 
 import mock
 import six
+from six.moves import reload_module  # pylint: disable=import-error
 
 from acme import jose
 
@@ -56,30 +57,21 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         # pylint: disable=protected-access
         cli._parser = cli.set_by_cli.detector = None
 
-    def _call(self, args):
+    def _call(self, args, stdout=None):
         "Run the cli with output streams and actual client mocked out"
         with mock.patch('letsencrypt.main.client') as client:
-            ret, stdout, stderr = self._call_no_clientmock(args)
+            ret, stdout, stderr = self._call_no_clientmock(args, stdout)
             return ret, stdout, stderr, client
 
-    def _call_no_clientmock(self, args):
+    def _call_no_clientmock(self, args, stdout=None):
         "Run the client with output streams mocked out"
         args = self.standard_args + args
-        with mock.patch('letsencrypt.main.sys.stdout') as stdout:
+
+        toy_stdout = stdout if stdout else six.StringIO()
+        with mock.patch('letsencrypt.main.sys.stdout', new=toy_stdout):
             with mock.patch('letsencrypt.main.sys.stderr') as stderr:
                 ret = main.main(args[:])  # NOTE: parser can alter its args!
-        return ret, stdout, stderr
-
-    def _call_stdout(self, args):
-        """
-        Variant of _call that preserves stdout so that it can be mocked by the
-        caller.
-        """
-        args = self.standard_args + args
-        with mock.patch('letsencrypt.main.sys.stderr') as stderr:
-            with mock.patch('letsencrypt.main.client') as client:
-                ret = main.main(args[:])  # NOTE: parser can alter its args!
-        return ret, None, stderr, client
+        return ret, toy_stdout, stderr
 
     def test_no_flags(self):
         with mock.patch('letsencrypt.main.run') as mock_run:
@@ -90,10 +82,9 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         "Run a command, and return the ouput string for scrutiny"
 
         output = six.StringIO()
-        with mock.patch('letsencrypt.main.sys.stdout', new=output):
-            self.assertRaises(SystemExit, self._call_stdout, args)
-            out = output.getvalue()
-            return out
+        self.assertRaises(SystemExit, self._call, args, output)
+        out = output.getvalue()
+        return out
 
     def test_help(self):
         self.assertRaises(SystemExit, self._call, ['--help'])
@@ -234,13 +225,6 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             self.assertTrue("The nginx plugin is not working" in ret)
             self.assertTrue("MisconfigurationError" in ret)
 
-        args = ["certonly", "--webroot"]
-        try:
-            self._call(args)
-            assert False, "Exception should have been raised"
-        except errors.PluginSelectionError as e:
-            self.assertTrue("please set either --webroot-path" in e.message)
-
         self._cli_missing_flag(["--standalone"], "With the standalone plugin, you probably")
 
         with mock.patch("letsencrypt.main._init_le_client") as mock_init:
@@ -283,7 +267,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         plugins.visible.assert_called_once_with()
         plugins.visible().ifaces.assert_called_once_with(ifaces)
         filtered = plugins.visible().ifaces()
-        stdout.write.called_once_with(str(filtered))
+        self.assertEqual(stdout.getvalue().strip(), str(filtered))
 
     @mock.patch('letsencrypt.main.plugins_disco')
     @mock.patch('letsencrypt.main.cli.HelpfulArgumentParser.determine_help_topics')
@@ -298,7 +282,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertEqual(filtered.init.call_count, 1)
         filtered.verify.assert_called_once_with(ifaces)
         verified = filtered.verify()
-        stdout.write.called_once_with(str(verified))
+        self.assertEqual(stdout.getvalue().strip(), str(verified))
 
     @mock.patch('letsencrypt.main.plugins_disco')
     @mock.patch('letsencrypt.main.cli.HelpfulArgumentParser.determine_help_topics')
@@ -315,7 +299,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         verified.prepare.assert_called_once_with()
         verified.available.assert_called_once_with()
         available = verified.available()
-        stdout.write.called_once_with(str(available))
+        self.assertEqual(stdout.getvalue().strip(), str(available))
 
     def test_certonly_abspath(self):
         cert = 'cert'
@@ -373,7 +357,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         try:
             self._call(['--csr', CSR])
         except errors.Error as e:
-            assert "Please try the certonly" in e.message
+            assert "Please try the certonly" in repr(e)
             return
         assert False, "Expected supplying --csr to fail with default verb"
 
@@ -436,20 +420,40 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         short_args += '--server example.com'.split()
         self._check_server_conflict_message(short_args, '--staging')
 
-    def _assert_dry_run_flag_worked(self, namespace):
+    def _assert_dry_run_flag_worked(self, namespace, existing_account):
         self.assertTrue(namespace.dry_run)
         self.assertTrue(namespace.break_my_certs)
         self.assertTrue(namespace.staging)
         self.assertEqual(namespace.server, constants.STAGING_URI)
 
+        if existing_account:
+            self.assertTrue(namespace.tos)
+            self.assertTrue(namespace.register_unsafely_without_email)
+        else:
+            self.assertFalse(namespace.tos)
+            self.assertFalse(namespace.register_unsafely_without_email)
+
     def test_dry_run_flag(self):
         parse = self._get_argument_parser()
-        short_args = ['--dry-run']
+        config_dir = tempfile.mkdtemp()
+        short_args = '--dry-run --config-dir {0}'.format(config_dir).split()
         self.assertRaises(errors.Error, parse, short_args)
 
-        self._assert_dry_run_flag_worked(parse(short_args + ['auth']))
+        self._assert_dry_run_flag_worked(
+            parse(short_args + ['auth']), False)
+        self._assert_dry_run_flag_worked(
+            parse(short_args + ['certonly']), False)
+        self._assert_dry_run_flag_worked(
+            parse(short_args + ['renew']), False)
+
+        account_dir = os.path.join(config_dir, constants.ACCOUNTS_DIR)
+        os.mkdir(account_dir)
+        os.mkdir(os.path.join(account_dir, 'fake_account_dir'))
+
+        self._assert_dry_run_flag_worked(parse(short_args + ['auth']), True)
+        self._assert_dry_run_flag_worked(parse(short_args + ['renew']), True)
         short_args += ['certonly']
-        self._assert_dry_run_flag_worked(parse(short_args))
+        self._assert_dry_run_flag_worked(parse(short_args), True)
 
         short_args += '--server example.com'.split()
         conflicts = ['--dry-run']
@@ -459,60 +463,6 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         conflicts += ['--staging']
         self._check_server_conflict_message(short_args, conflicts)
 
-    def _webroot_map_test(self, map_arg, path_arg, domains_arg, # pylint: disable=too-many-arguments
-                          expected_map, expectect_domains, extra_args=None):
-        parse = self._get_argument_parser()
-        webroot_map_args = extra_args if extra_args else []
-        if map_arg:
-            webroot_map_args.extend(["--webroot-map", map_arg])
-        if path_arg:
-            webroot_map_args.extend(["-w", path_arg])
-        if domains_arg:
-            webroot_map_args.extend(["-d", domains_arg])
-        namespace = parse(webroot_map_args)
-        domains = main._find_domains(namespace, mock.MagicMock()) # pylint: disable=protected-access
-        self.assertEqual(namespace.webroot_map, expected_map)
-        self.assertEqual(set(domains), set(expectect_domains))
-
-    def test_parse_webroot(self):
-        parse = self._get_argument_parser()
-        webroot_args = ['--webroot', '-w', '/var/www/example',
-            '-d', 'example.com,www.example.com', '-w', '/var/www/superfluous',
-            '-d', 'superfluo.us', '-d', 'www.superfluo.us']
-        namespace = parse(webroot_args)
-        self.assertEqual(namespace.webroot_map, {
-            'example.com': '/var/www/example',
-            'www.example.com': '/var/www/example',
-            'www.superfluo.us': '/var/www/superfluous',
-            'superfluo.us': '/var/www/superfluous'})
-
-        webroot_args = ['-d', 'stray.example.com'] + webroot_args
-        self.assertRaises(errors.Error, parse, webroot_args)
-
-        simple_map = '{"eg.com" : "/tmp"}'
-        expected_map = {"eg.com": "/tmp"}
-        self._webroot_map_test(simple_map, None, None, expected_map, ["eg.com"])
-
-        # test merging webroot maps from the cli and a webroot map
-        expected_map["eg2.com"] = "/tmp2"
-        domains = ["eg.com", "eg2.com"]
-        self._webroot_map_test(simple_map, "/tmp2", "eg2.com,eg.com", expected_map, domains)
-
-        # test inclusion of interactively specified domains in the webroot map
-        with mock.patch('letsencrypt.display.ops.choose_names') as mock_choose:
-            mock_choose.return_value = domains
-            expected_map["eg2.com"] = "/tmp"
-            self._webroot_map_test(None, "/tmp", None, expected_map, domains)
-
-        extra_args = ['-c', test_util.vector_path('webrootconftest.ini')]
-        self._webroot_map_test(None, None, None, expected_map, domains, extra_args)
-
-        webroot_map_args = ['--webroot-map',
-                            '{"eg.com.,www.eg.com": "/tmp", "eg.is.": "/tmp2"}']
-        namespace = parse(webroot_map_args)
-        self.assertEqual(namespace.webroot_map,
-                         {"eg.com": "/tmp", "www.eg.com": "/tmp", "eg.is": "/tmp2"})
-
     def _certonly_new_request_common(self, mock_client, args=None):
         with mock.patch('letsencrypt.main._treat_as_renewal') as mock_renewal:
             mock_renewal.return_value = ("newcert", None)
@@ -570,6 +520,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         mock_certr = mock.MagicMock()
         mock_key = mock.MagicMock(pem='pem_key')
         mock_client = mock.MagicMock()
+        stdout = None
         mock_client.obtain_certificate.return_value = (mock_certr, 'chain',
                                                        mock_key, 'csr')
         try:
@@ -589,7 +540,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                                 if extra_args:
                                     args += extra_args
                                 try:
-                                    ret, _, _, _ = self._call(args)
+                                    ret, stdout, _, _ = self._call(args)
                                     if ret:
                                         print("Returned", ret)
                                         raise AssertionError(ret)
@@ -612,10 +563,10 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                 with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf:
                     self.assertTrue(log_out in lf.read())
 
-        return mock_lineage, mock_get_utility
+        return mock_lineage, mock_get_utility, stdout
 
     def test_certonly_renewal(self):
-        lineage, get_utility = self._test_renewal_common(True, [])
+        lineage, get_utility, _ = self._test_renewal_common(True, [])
         self.assertEqual(lineage.save_successor.call_count, 1)
         lineage.update_all_links_to.assert_called_once_with(
             lineage.latest_common_version())
@@ -625,17 +576,18 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def test_certonly_renewal_triggers(self):
         # --dry-run should force renewal
-        _, get_utility = self._test_renewal_common(False, ['--dry-run', '--keep'],
-                                                   log_out="simulating renewal")
+        _, get_utility, _ = self._test_renewal_common(False, ['--dry-run', '--keep'],
+                                                      log_out="simulating renewal")
         self.assertEqual(get_utility().add_message.call_count, 1)
         self.assertTrue('dry run' in get_utility().add_message.call_args[0][0])
 
-        _, _ = self._test_renewal_common(False, ['--renew-by-default', '-tvv', '--debug'],
-                                        log_out="Auto-renewal forced")
+        self._test_renewal_common(False, ['--renew-by-default', '-tvv', '--debug'],
+                                  log_out="Auto-renewal forced")
         self.assertEqual(get_utility().add_message.call_count, 1)
 
-        _, _ = self._test_renewal_common(False, ['-tvv', '--debug', '--keep'],
-                                        log_out="not yet due", should_renew=False)
+        self._test_renewal_common(False, ['-tvv', '--debug', '--keep'],
+                                  log_out="not yet due", should_renew=False)
+
 
     def _dump_log(self):
         with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf:
@@ -660,6 +612,19 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         args = ["renew", "--dry-run", "-tvv"]
         self._test_renewal_common(True, [], args=args, should_renew=True)
 
+    def test_quiet_renew(self):
+        self._make_test_renewal_conf('sample-renewal.conf')
+        args = ["renew", "--dry-run"]
+        _, _, stdout = self._test_renewal_common(True, [], args=args, should_renew=True)
+        out = stdout.getvalue()
+        self.assertTrue("renew" in out)
+
+        args = ["renew", "--dry-run", "-q"]
+        _, _, stdout = self._test_renewal_common(True, [], args=args, should_renew=True)
+        out = stdout.getvalue()
+        self.assertEqual("", out)
+
+
     @mock.patch("letsencrypt.cli.set_by_cli")
     def test_ancient_webroot_renewal_conf(self, mock_set_by_cli):
         mock_set_by_cli.return_value = False
@@ -1023,5 +988,79 @@ class DuplicativeCertsTest(storage_test.BaseRenewableCertTest):
         self.assertEqual(result, (None, None))
 
 
+class DefaultTest(unittest.TestCase):
+    """Tests for letsencrypt.cli._Default."""
+
+    def setUp(self):
+        # pylint: disable=protected-access
+        self.default1 = cli._Default()
+        self.default2 = cli._Default()
+
+    def test_boolean(self):
+        self.assertFalse(self.default1)
+        self.assertFalse(self.default2)
+
+    def test_equality(self):
+        self.assertEqual(self.default1, self.default2)
+
+    def test_hash(self):
+        self.assertEqual(hash(self.default1), hash(self.default2))
+
+
+class SetByCliTest(unittest.TestCase):
+    """Tests for letsencrypt.set_by_cli and related functions."""
+
+    def setUp(self):
+        reload_module(cli)
+
+    def test_webroot_map(self):
+        args = '-w /var/www/html -d example.com'.split()
+        verb = 'renew'
+        self.assertTrue(_call_set_by_cli('webroot_map', args, verb))
+
+    def test_report_config_interaction_str(self):
+        cli.report_config_interaction('manual_public_ip_logging_ok',
+                                      'manual_test_mode')
+        cli.report_config_interaction('manual_test_mode', 'manual')
+
+        self._test_report_config_interaction_common()
+
+    def test_report_config_interaction_iterable(self):
+        cli.report_config_interaction(('manual_public_ip_logging_ok',),
+                                      ('manual_test_mode',))
+        cli.report_config_interaction(('manual_test_mode',), ('manual',))
+
+        self._test_report_config_interaction_common()
+
+    def _test_report_config_interaction_common(self):
+        """Tests implied interaction between manual flags.
+
+        --manual implies --manual-test-mode which implies
+        --manual-public-ip-logging-ok. These interactions don't actually
+        exist in the client, but are used here for testing purposes.
+
+        """
+
+        args = ['--manual']
+        verb = 'renew'
+        for v in ('manual', 'manual_test_mode', 'manual_public_ip_logging_ok'):
+            self.assertTrue(_call_set_by_cli(v, args, verb))
+
+        cli.set_by_cli.detector = None
+
+        args = ['--manual-test-mode']
+        for v in ('manual_test_mode', 'manual_public_ip_logging_ok'):
+            self.assertTrue(_call_set_by_cli(v, args, verb))
+
+        self.assertFalse(_call_set_by_cli('manual', args, verb))
+
+
+def _call_set_by_cli(var, args, verb):
+    with mock.patch('letsencrypt.cli.helpful_parser') as mock_parser:
+        mock_parser.args = args
+        mock_parser.verb = verb
+        return cli.set_by_cli(var)
+
+
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

letsencrypt/tests/client_test.py

@@ -80,6 +80,7 @@ class RegisterTest(unittest.TestCase):
             with mock.patch("letsencrypt.account.report_new_account"):
                 self.config.email = None
                 self.config.register_unsafely_without_email = True
+                self.config.dry_run = False
                 self._call()
                 mock_logger.warn.assert_called_once_with(mock.ANY)
 
@@ -135,8 +136,7 @@ class ClientTest(unittest.TestCase):
 
     # FIXME move parts of this to test_cli.py...
     @mock.patch("letsencrypt.client.logger")
-    @mock.patch("letsencrypt.cli.process_domain")
-    def test_obtain_certificate_from_csr(self, mock_process_domain, mock_logger):
+    def test_obtain_certificate_from_csr(self, mock_logger):
         self._mock_obtain_certificate()
         from letsencrypt import cli
         test_csr = le_util.CSR(form="der", file=None, data=CSR_SAN)
@@ -150,9 +150,6 @@ class ClientTest(unittest.TestCase):
             mock_parser = mock.MagicMock(cli.HelpfulArgumentParser)
             cli.HelpfulArgumentParser.handle_csr(mock_parser, mock_parsed_args)
 
-            # make sure cli processing occurred
-            cli_processed = (call[0][1] for call in mock_process_domain.call_args_list)
-            self.assertEqual(set(cli_processed), set(("example.com", "www.example.com")))
             # Now provoke an inconsistent domains error...
             mock_parsed_args.domains.append("hippopotamus.io")
             self.assertRaises(errors.ConfigurationError,

letsencrypt/tests/hook_test.py

@@ -3,7 +3,6 @@
 
 import os
 import unittest
-import sys
 
 import mock
 
@@ -48,11 +47,13 @@ class HookTest(unittest.TestCase):
         self.assertEqual(hooks._prog("funky"), None)
 
     def _test_a_hook(self, config, hook_function, calls_expected):
-        with mock.patch('letsencrypt.hooks.logger'):
+        with mock.patch('letsencrypt.hooks.logger') as mock_logger:
+            mock_logger.warning = mock.MagicMock()
             with mock.patch('letsencrypt.hooks._run_hook') as mock_run_hook:
                 hook_function(config)
                 hook_function(config)
                 self.assertEqual(mock_run_hook.call_count, calls_expected)
+            return mock_logger.warning
 
     def test_pre_hook(self):
         config = mock.MagicMock(pre_hook="true")
@@ -78,13 +79,8 @@ class HookTest(unittest.TestCase):
             self.assertEqual(os.environ["RENEWED_LINEAGE"], "thing")
 
             config = mock.MagicMock(renew_hook="true", dry_run=True)
-            if sys.version_info < (2, 7):
-                # the print() function is not mockable in py26
-                self._test_a_hook(config, rhook, 0)
-            else:
-                with mock.patch("letsencrypt.hooks.print") as mock_print:
-                    self._test_a_hook(config, rhook, 0)
-                    self.assertEqual(mock_print.call_count, 2)
+            mock_warn = self._test_a_hook(config, rhook, 0)
+            self.assertEqual(mock_warn.call_count, 2)
 
     @mock.patch('letsencrypt.hooks.Popen')
     def test_run_hook(self, mock_popen):

letsencrypt/tests/reporter_test.py

@@ -1,4 +1,5 @@
 """Tests for letsencrypt.reporter."""
+import mock
 import sys
 import unittest
 
@@ -10,7 +11,7 @@ class ReporterTest(unittest.TestCase):
 
     def setUp(self):
         from letsencrypt import reporter
-        self.reporter = reporter.Reporter()
+        self.reporter = reporter.Reporter(mock.MagicMock(quiet=False))
 
         self.old_stdout = sys.stdout
         sys.stdout = six.StringIO()