Merge pull request #2309 from letsencrypt/webroot-map-and-flags

Set webroot-map from diverse flags, user interaction, and config files correctly

letsencrypt/cli.py

@@ -112,11 +112,15 @@ def usage_strings(plugins):
     return USAGE % (apache_doc, nginx_doc), SHORT_USAGE
 
 
-def _find_domains(args, installer):
+def _find_domains(config, installer):
-    if not args.domains:
+    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:
+            _process_domain(config, d)
     else:
-        domains = args.domains
+        domains = config.domains
 
     if not domains:
         raise errors.Error("Please specify --domains, or --installer that "
@@ -590,7 +594,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     except errors.PluginSelectionError, e:
         return e.message
 
-    domains = _find_domains(args, installer)
+    domains = _find_domains(config, installer)
 
     # TODO: Handle errors from _init_le_client?
     le_client = _init_le_client(args, config, authenticator, installer)
@@ -612,7 +616,8 @@ 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."""
+    """Implements "certonly": authenticate & obtain cert, but do not install it."""
+
     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?
@@ -635,7 +640,7 @@ def obtain_cert(args, config, plugins):
             certr, chain, args.cert_path, args.chain_path, args.fullchain_path)
         _report_new_cert(cert_path, cert_fullchain)
     else:
-        domains = _find_domains(args, installer)
+        domains = _find_domains(config, installer)
         _auth_from_domains(le_client, config, domains)
 
     _suggest_donate()
@@ -653,7 +658,7 @@ def install(args, config, plugins):
     except errors.PluginSelectionError, e:
         return e.message
 
-    domains = _find_domains(args, installer)
+    domains = _find_domains(config, installer)
     le_client = _init_le_client(
         args, config, authenticator=None, installer=installer)
     assert args.cert_path is not None  # required=True in the subparser
@@ -828,6 +833,12 @@ class HelpfulArgumentParser(object):
 
         # Do any post-parsing homework here
 
+        # 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)
+
         # argparse seemingly isn't flexible enough to give us this behaviour easily...
         if parsed_args.staging:
             if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
@@ -1250,11 +1261,12 @@ def _plugins_parsing(helpful, plugins):
                      "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`")
-    parse_dict = lambda s: dict(json.loads(s))
     # --webroot-map still has some awkward properties, so it is undocumented
-    helpful.add("webroot", "--webroot-map", default={}, type=parse_dict,
+    helpful.add("webroot", "--webroot-map", default={}, action=WebrootMapProcessor,
-                help=argparse.SUPPRESS)
+        help="JSON dictionary mapping domains to webroot paths; this implies -d "
-
+             "for each entry. You may need to escape this from your shell. "
+             """Eg: --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")
 
 class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
     def __init__(self, *args, **kwargs):
@@ -1283,18 +1295,36 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
         config.webroot_path.append(webroot)
 
 
+_undot = lambda domain: domain[:-1] if domain.endswith('.') else domain
+
+def _process_domain(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
+    """
+    webroot_path = webroot_path if webroot_path else config.webroot_path
+
+    for domain in (d.strip() for d in domain_arg.split(",")):
+        if domain not in config.domains:
+            domain = _undot(domain)
+            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:
+                config.webroot_map.setdefault(domain, webroot_path[-1])
+
+
+class WebrootMapProcessor(argparse.Action): # pylint: disable=missing-docstring
+    def __call__(self, parser, config, webroot_map_arg, option_string=None):
+        webroot_map = json.loads(webroot_map_arg)
+        for domains, webroot_path in webroot_map.iteritems():
+            _process_domain(config, domains, [webroot_path])
+
+
 class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring
     def __call__(self, parser, config, domain_arg, option_string=None):
-        """
+        """Just wrap _process_domain in argparseese."""
-        Process a new -d flag, helping the webroot plugin construct a map of
+        _process_domain(config, domain_arg)
-        {domain : webrootpath} if -w / --webroot-path is in use
-        """
-        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[domain] = config.webroot_path[-1]
 
 
 def setup_log_file_handler(args, logfile, fmt):

letsencrypt/le_util.py

@@ -298,18 +298,19 @@ def check_domain_sanity(domain):
     # Check if there's a wildcard domain
     if domain.startswith("*."):
         raise errors.ConfigurationError(
-            "Wildcard domains are not supported")
+            "Wildcard domains are not supported: {0}".format(domain))
     # Punycode
     if "xn--" in domain:
         raise errors.ConfigurationError(
-            "Punycode domains are not presently supported")
+            "Punycode domains are not presently supported: {0}".format(domain))
 
     # Unicode
     try:
         domain.encode('ascii')
     except UnicodeDecodeError:
         raise errors.ConfigurationError(
-            "Internationalized domain names are not presently supported")
+            "Internationalized domain names are not presently supported: {0}"
+            .format(domain))
 
     # FQDN checks from
     # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/
@@ -317,4 +318,4 @@ def check_domain_sanity(domain):
     #  first and last char is not "-"
     fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\\.)+[A-Za-z]{2,63}$")
     if not fqdn.match(domain):
-        raise errors.ConfigurationError("Requested domain is not a FQDN")
+        raise errors.ConfigurationError("Requested domain {0} is not a FQDN".format(domain))

letsencrypt/tests/cli_test.py

@@ -356,6 +356,10 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         namespace = cli.prepare_and_parse_args(plugins, short_args)
         self.assertEqual(namespace.domains, ['example.com'])
 
+        short_args = ['-d', 'trailing.period.com.']
+        namespace = cli.prepare_and_parse_args(plugins, short_args)
+        self.assertEqual(namespace.domains, ['trailing.period.com'])
+
         short_args = ['-d', 'example.com,another.net,third.org,example.com']
         namespace = cli.prepare_and_parse_args(plugins, short_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net',
@@ -365,6 +369,10 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         namespace = cli.prepare_and_parse_args(plugins, long_args)
         self.assertEqual(namespace.domains, ['example.com'])
 
+        long_args = ['--domains', 'trailing.period.com.']
+        namespace = cli.prepare_and_parse_args(plugins, long_args)
+        self.assertEqual(namespace.domains, ['trailing.period.com'])
+
         long_args = ['--domains', 'example.com,another.net,example.com']
         namespace = cli.prepare_and_parse_args(plugins, long_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net'])
@@ -382,11 +390,26 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         short_args = ['--staging', '--server', 'example.com']
         self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args)
 
+    def _webroot_map_test(self, map_arg, path_arg, domains_arg, # pylint: disable=too-many-arguments
+                          expected_map, expectect_domains, extra_args=None):
+        plugins = disco.PluginsRegistry.find_all()
+        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 = cli.prepare_and_parse_args(plugins, webroot_map_args)
+        domains = cli._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):
         plugins = disco.PluginsRegistry.find_all()
         webroot_args = ['--webroot', '-w', '/var/www/example',
             '-d', 'example.com,www.example.com', '-w', '/var/www/superfluous',
-            '-d', 'superfluo.us', '-d', 'www.superfluo.us']
+            '-d', 'superfluo.us', '-d', 'www.superfluo.us.']
         namespace = cli.prepare_and_parse_args(plugins, webroot_args)
         self.assertEqual(namespace.webroot_map, {
             'example.com': '/var/www/example',
@@ -397,9 +420,29 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         webroot_args = ['-d', 'stray.example.com'] + webroot_args
         self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, webroot_args)
 
-        webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}']
+        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.cli.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 = cli.prepare_and_parse_args(plugins, webroot_map_args)
-        self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"})
+        self.assertEqual(namespace.webroot_map,
+                         {"eg.com": "/tmp", "www.eg.com": "/tmp", "eg.is": "/tmp2"})
 
     @mock.patch('letsencrypt.cli._suggest_donate')
     @mock.patch('letsencrypt.crypto_util.notAfter')

letsencrypt/tests/testdata/webrootconftest.ini

@@ -0,0 +1,3 @@
+webroot
+webroot-path = /tmp
+domains = eg.com, eg2.com