From 06e273413b609883cbd4587f002460c089a77fa4 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 1 Dec 2015 16:33:35 -0800 Subject: [PATCH] Fix nits and address review comments --- letsencrypt/cli.py | 27 ++++++++++++--------------- letsencrypt/plugins/webroot.py | 8 ++++---- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index bd95cd372..85478132e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -10,7 +10,6 @@ import logging import logging.handlers import os import pkg_resources -import string import sys import time import traceback @@ -103,7 +102,7 @@ def usage_strings(plugins): def _find_domains(args, installer): - if args.domains is None: + if not args.domains: domains = display_ops.choose_names(installer) else: domains = args.domains @@ -477,7 +476,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo def obtain_cert(args, config, plugins): """Authenticate & obtain cert, but do not install it.""" - if args.domains is not None and args.csr is not None: + if args.domains and args.csr is not None: # TODO: --csr could have a priority, when --domains is # supplied, check if CSR matches given domains? return "--domains and --csr are mutually exclusive" @@ -840,7 +839,7 @@ def prepare_and_parse_args(plugins, args): #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, + metavar="DOMAIN", action=DomainFlagProcessor, 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.") @@ -1073,17 +1072,18 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring Keep a record of --webroot-path / -w flags during processing, so that we know which apply to which -d flags """ - if not config.webroot_path: + if config.webroot_path is None: # first -w flag encountered config.webroot_path = [] # if any --domain flags preceded the first --webroot-path flag, # apply that webroot path to those; subsequent entries in # config.webroot_map are filled in by cli.DomainFlagProcessor if config.domains: - config.webroot_map = dict([(d, webroot) for d in config.domains]) self.domain_before_webroot = True - else: - config.webroot_map = {} + for d in config.domains: + config.webroot_map.setdefault(d, webroot) elif self.domain_before_webroot: + # FIXME if you set domains in a config 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") config.webroot_path.append(webroot) @@ -1095,15 +1095,12 @@ class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring Process a new -d flag, helping the webroot plugin construct a map of {domain : webrootpath} if -w / --webroot-path is in use """ - if not config.domains: - config.domains = [] - - for d in map(string.strip, domain_arg.split(",")): # pylint: disable=bad-builtin - if d not in config.domains: - config.domains.append(d) + for domain in (d.strip() for d in domain_arg.split(",")): + if domain not in config.domains: + config.domains.append(domain) # Each domain has a webroot_path of the most recent -w flag if config.webroot_path: - config.webroot_map[d] = config.webroot_path[-1] + config.webroot_map[domain] = config.webroot_path[-1] def setup_log_file_handler(args, logfile, fmt): diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 6709d0c87..705f08113 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -15,7 +15,6 @@ from letsencrypt.plugins import common logger = logging.getLogger(__name__) - class Authenticator(common.Plugin): """Webroot Authenticator.""" zope.interface.implements(interfaces.IAuthenticator) @@ -72,9 +71,10 @@ to serve all files under specified web root ({0}).""" return [self._perform_single(achall) for achall in achalls] def _path_for_achall(self, achall): - path = self.full_roots[achall.domain] - if not path: - raise errors.PluginError("Cannot find path {0} for domain: {1}" + try: + path = self.full_roots[achall.domain] + except IndexError: + raise errors.PluginError("Cannot find webroot path for domain: {1}" .format(path, achall.domain)) return os.path.join(path, achall.chall.encode("token"))