From 5dce1e15abb99aa5b08260b9f1b1dc1a63cd7a57 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 22 Nov 2015 01:39:57 -0800 Subject: [PATCH 01/17] If we're going to have --webroot-map, make integrate it fully [tests broken] * -d is implied for things included in it * if --webroot-path and -w are both used, the later does not override explicit entries in the former --- letsencrypt/cli.py | 28 ++++++++++++++++------------ letsencrypt/tests/cli_test.py | 3 +++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 9c4c4a5f5..19cfc76f9 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -101,7 +101,13 @@ def usage_strings(plugins): def _find_domains(args, installer): - if args.domains is None: + # we get domains from -d, but also from the webroot map... + if args.webroot_map: + for domain in args.webroot_map.keys(): + if domain not in args.domains: + args.domains.append(domain) + + if not args.domains: domains = display_ops.choose_names(installer) else: domains = args.domains @@ -474,7 +480,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.""" + """Implements "certonly": authenticate & obtain cert, but do not install it.""" if args.domains is not None and args.csr is not None: # TODO: --csr could have a priority, when --domains is @@ -839,7 +845,7 @@ def prepare_and_parse_args(plugins, args): # --domains is useful, because it can be stored in config #for subparser in parser_run, parser_auth, parser_install: # subparser.add_argument("domains", nargs="*", metavar="domain") - helpful.add(None, "-d", "--domains", dest="domains", + helpful.add(None, "-d", "--domains", dest="domains", default=[], metavar="DOMAIN", action=DomainFlagProcessor, help="Domain names to apply. For multiple domains you can use " "multiple -d flags or enter a comma separated list of domains " @@ -1038,7 +1044,7 @@ def _plugins_parsing(helpful, plugins): help="public_html / webroot path") parse_dict = lambda s: dict(json.loads(s)) helpful.add("webroot", "--webroot-map", default={}, type=parse_dict, - help="Mapping from domains to webroot paths") + help="JSON dictionary mapping domains to webroot paths; this implies -d for each entry.") class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring @@ -1047,15 +1053,15 @@ 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]) - else: - config.webroot_map = {} + for d in config.domains: + config.webroot_map.setdefault(d, webroot) + config.webroot_path.append(webroot) @@ -1065,15 +1071,13 @@ 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) # Each domain has a webroot_path of the most recent -w flag + # unless it was explicitly included in webroot_map if config.webroot_path: - config.webroot_map[d] = config.webroot_path[-1] + config.webroot_map.setdefault(d, config.webroot_path[-1]) def setup_log_file_handler(args, logfile, fmt): diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 853109636..60f3c245a 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -341,6 +341,7 @@ 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', 'another.net']) + def test_parse_webroot(self): plugins = disco.PluginsRegistry.find_all() webroot_args = ['--webroot', '-d', 'stray.example.com', '-w', @@ -356,7 +357,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}'] namespace = cli.prepare_and_parse_args(plugins, webroot_map_args) + domains = cli._find_domains(namespace, mock.MagicMock()) self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"}) + self.assertEqual(domains, ["eg.com"]) @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.zope.component.getUtility') From c816910c0d811377bd5a3823609d720ba7b30554 Mon Sep 17 00:00:00 2001 From: Filip Ochnik Date: Wed, 20 Jan 2016 14:25:22 +0700 Subject: [PATCH 02/17] Support trailing period in domain names --- letsencrypt/cli.py | 1 + letsencrypt/tests/cli_test.py | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 89606089f..58d7609fd 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1236,6 +1236,7 @@ class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring {domain : webrootpath} if -w / --webroot-path is in use """ for domain in (d.strip() for d in domain_arg.split(",")): + domain = domain[:-1] if domain.endswith('.') else domain if domain not in config.domains: config.domains.append(domain) # Each domain has a webroot_path of the most recent -w flag diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 16ef5c093..8424c7a51 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -330,6 +330,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', @@ -339,6 +343,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']) @@ -360,7 +368,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods 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', From 94f24a5982990fa111b46ba69e13633f4ae798ee Mon Sep 17 00:00:00 2001 From: Filip Ochnik Date: Thu, 21 Jan 2016 13:01:10 +0700 Subject: [PATCH 03/17] Support trailing periods in webroot-map --- letsencrypt/cli.py | 13 ++++++++++--- letsencrypt/tests/cli_test.py | 6 ++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 58d7609fd..63c7604aa 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1196,10 +1196,9 @@ 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, - help=argparse.SUPPRESS) + helpful.add("webroot", "--webroot-map", default={}, + action=WebrootMapProcessor, help=argparse.SUPPRESS) class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring @@ -1229,6 +1228,14 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring config.webroot_path.append(webroot) +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 domain, webroot in webroot_map.iteritems(): + domain = domain[:-1] if domain.endswith('.') else domain + config.webroot_map[domain] = webroot + + class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring def __call__(self, parser, config, domain_arg, option_string=None): """ diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 8424c7a51..19bdb058f 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -379,9 +379,11 @@ 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"}'] + webroot_map_args = ['--webroot-map', + '{"eg.com": "/tmp", "www.eg.com.": "/tmp"}'] 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, + {u"eg.com": u"/tmp", u"www.eg.com": u"/tmp"}) @mock.patch('letsencrypt.cli._suggest_donate') @mock.patch('letsencrypt.crypto_util.notAfter') From 63851bfa52ce76a7a23b2fb379dd87a74e2d9bd0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 14:54:11 -0800 Subject: [PATCH 04/17] Treat webroot_map -> domain importation as a general property of configs --- letsencrypt/cli.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index c15c9e6a6..e703c83f9 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -113,12 +113,6 @@ def usage_strings(plugins): def _find_domains(args, installer): - # we get domains from -d, but also from the webroot map... - if args.webroot_map: - for domain in args.webroot_map.keys(): - if domain not in args.domains: - args.domains.append(domain) - if not args.domains: domains = display_ops.choose_names(installer) else: @@ -835,6 +829,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): From aac52e755ae6f7a952d7d05c3c8f06a2c6a9e013 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 15:54:28 -0800 Subject: [PATCH 05/17] Whatever domains we picked should make it to the renewal conf --- letsencrypt/cli.py | 15 ++++++++------- letsencrypt/tests/cli_test.py | 12 +++++++++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e703c83f9..fb0f6a17d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -112,11 +112,12 @@ def usage_strings(plugins): return USAGE % (apache_doc, nginx_doc), SHORT_USAGE -def _find_domains(args, installer): - if not args.domains: - domains = display_ops.choose_names(installer) +def _find_domains(config, installer): + if not config.domains: + # set args.domains so that it's written to the renewal conf file + domains = config.domains = display_ops.choose_names(installer) else: - domains = args.domains + domains = config.domains if not domains: raise errors.Error("Please specify --domains, or --installer that " @@ -590,7 +591,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) @@ -636,7 +637,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() @@ -654,7 +655,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 diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index ad74c5c0a..2a9532f00 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -400,9 +400,19 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}'] namespace = cli.prepare_and_parse_args(plugins, webroot_map_args) domains = cli._find_domains(namespace, mock.MagicMock()) - self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"}) + expected_map = {u"eg.com": u"/tmp"} + self.assertEqual(namespace.webroot_map, expected_map) self.assertEqual(domains, ["eg.com"]) + # test merging webroot maps from the cli and a webroot map + webroot_map_args.extend(["-w", "/tmp2", "-d", "eg2.com,eg.com"]) + namespace = cli.prepare_and_parse_args(plugins, webroot_map_args) + domains = cli._find_domains(namespace, mock.MagicMock()) + # for eg.com, --webroot-map should take precedence over -w / -d + expected_map[u"eg2.com"] = u"/tmp2" + self.assertEqual(namespace.webroot_map, expected_map) + self.assertEqual(set(domains), set(["eg.com", "eg2.com"])) + @mock.patch('letsencrypt.cli._suggest_donate') @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.zope.component.getUtility') From 10c8c1f533e49affd0dd79417bfddf3108b671bb Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 16:46:06 -0800 Subject: [PATCH 06/17] Include interactively specified domains in webroot_map --- letsencrypt/cli.py | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index fb0f6a17d..669bed7c7 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -115,7 +115,10 @@ def usage_strings(plugins): def _find_domains(config, installer): if not config.domains: # set args.domains so that it's written to the renewal conf file - domains = config.domains = display_ops.choose_names(installer) + domains = display_ops.choose_names(installer) + # record in config.domains, and set webroot_map entries if applicable + for d in domains: + _process_domain(config, d) else: domains = config.domains @@ -1291,19 +1294,24 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring config.webroot_path.append(webroot) +def _process_domain(config, domain_arg): + """ + Process a new -d flag, helping the webroot plugin construct a map of + {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 + # unless it was explicitly included in webroot_map + if config.webroot_path: + config.webroot_map.setdefault(domain, config.webroot_path[-1]) + + class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring def __call__(self, parser, config, domain_arg, option_string=None): - """ - Process a new -d flag, helping the webroot plugin construct a map of - {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 - # unless it was explicitly included in webroot_map - if config.webroot_path: - config.webroot_map.setdefault(domain, config.webroot_path[-1]) + """Just wrap _process_domain in argparseese.""" + _process_domain(config, domain_arg) def setup_log_file_handler(args, logfile, fmt): From c9c81ef01575af8e186fc1f638d4d35bc6d06d99 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 16:46:52 -0800 Subject: [PATCH 07/17] Test interactive domain -> webroot-map inclusion Also factorise test cases --- letsencrypt/tests/cli_test.py | 40 ++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 2a9532f00..acb6e97c2 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -1,5 +1,6 @@ """Tests for letsencrypt.cli.""" import argparse +import copy import itertools import os import shutil @@ -382,6 +383,21 @@ 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, + expected_map, expectect_domains): + plugins = disco.PluginsRegistry.find_all() + webroot_map_args = [] + 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()) + 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', @@ -397,21 +413,21 @@ 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"}'] - namespace = cli.prepare_and_parse_args(plugins, webroot_map_args) - domains = cli._find_domains(namespace, mock.MagicMock()) - expected_map = {u"eg.com": u"/tmp"} - self.assertEqual(namespace.webroot_map, expected_map) - self.assertEqual(domains, ["eg.com"]) + simple_map = '{"eg.com" : "/tmp"}' + expected_map = {u"eg.com" : u"/tmp"} + self._webroot_map_test(simple_map, None, None, expected_map, ["eg.com"]) # test merging webroot maps from the cli and a webroot map - webroot_map_args.extend(["-w", "/tmp2", "-d", "eg2.com,eg.com"]) - namespace = cli.prepare_and_parse_args(plugins, webroot_map_args) - domains = cli._find_domains(namespace, mock.MagicMock()) - # for eg.com, --webroot-map should take precedence over -w / -d expected_map[u"eg2.com"] = u"/tmp2" - self.assertEqual(namespace.webroot_map, expected_map) - self.assertEqual(set(domains), set(["eg.com", "eg2.com"])) + 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[u"eg2.com"] = u"/tmp" + self._webroot_map_test(None, "/tmp", None, expected_map, domains) + @mock.patch('letsencrypt.cli._suggest_donate') @mock.patch('letsencrypt.crypto_util.notAfter') From bf7f9d2cc15019485c5b2bedda761b62538d1b8f Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 16:57:31 -0800 Subject: [PATCH 08/17] Debugging, but also helpful errors... --- letsencrypt/le_util.py | 2 +- letsencrypt/tests/cli_test.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 64295a80f..3eb4be0a7 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -317,4 +317,4 @@ def check_domain_sanity(domain): # first and last char is not "-" fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? Date: Thu, 28 Jan 2016 17:14:55 -0800 Subject: [PATCH 09/17] Test setting webroot-path in a config file --- letsencrypt/tests/cli_test.py | 7 +++++-- letsencrypt/tests/testdata/webrootconftest.ini | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 letsencrypt/tests/testdata/webrootconftest.ini diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index ee135825d..ac600a357 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -383,9 +383,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args) def _webroot_map_test(self, map_arg, path_arg, domains_arg, - expected_map, expectect_domains): + expected_map, expectect_domains, extra_args=[]): plugins = disco.PluginsRegistry.find_all() - webroot_map_args = [] + webroot_map_args = [] + extra_args if map_arg: webroot_map_args.extend(["--webroot-map", map_arg]) if path_arg: @@ -427,6 +427,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods expected_map[u"eg2.com"] = u"/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) + @mock.patch('letsencrypt.cli._suggest_donate') @mock.patch('letsencrypt.crypto_util.notAfter') diff --git a/letsencrypt/tests/testdata/webrootconftest.ini b/letsencrypt/tests/testdata/webrootconftest.ini new file mode 100644 index 000000000..de3bd98a6 --- /dev/null +++ b/letsencrypt/tests/testdata/webrootconftest.ini @@ -0,0 +1,3 @@ +webroot +webroot-path = /tmp +domains = eg.com, eg2.com From c552bce609090c0d942d3f4bd07a3b2dfafb0f14 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 17:22:43 -0800 Subject: [PATCH 10/17] lintmonster --- letsencrypt/tests/cli_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index ac600a357..6c8facd81 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -382,10 +382,10 @@ 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, - expected_map, expectect_domains, extra_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 + webroot_map_args = extra_args if extra_args else [] if map_arg: webroot_map_args.extend(["--webroot-map", map_arg]) if path_arg: @@ -393,7 +393,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods 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()) + 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)) @@ -413,7 +413,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, webroot_args) simple_map = '{"eg.com" : "/tmp"}' - expected_map = {u"eg.com" : u"/tmp"} + expected_map = {u"eg.com": u"/tmp"} self._webroot_map_test(simple_map, None, None, expected_map, ["eg.com"]) # test merging webroot maps from the cli and a webroot map From e056b35f28338f9b098ad6f9a820d8d7d422d0cf Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 17:51:42 -0800 Subject: [PATCH 11/17] Fix some merge bugs --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 09a9646e0..5df6ffb6d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1308,14 +1308,14 @@ def _process_domain(config, domain_arg, webroot_path=None): # 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, config.webroot_path[-1]) + 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) + _process_domain(config, domains, [webroot_path]) class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring From 4114ca1c23dd33fe25924578583b96c043d83e8a Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 17:54:35 -0800 Subject: [PATCH 12/17] Since we have a hook now, let's be done with all this unicode nonsense --- letsencrypt/cli.py | 3 +-- letsencrypt/tests/cli_test.py | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5df6ffb6d..6537ee8b8 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1315,7 +1315,7 @@ 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]) + _process_domain(config, str(domains), [str(webroot_path)]) class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring @@ -1324,7 +1324,6 @@ class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring _process_domain(config, domain_arg) - def setup_log_file_handler(args, logfile, fmt): """Setup file debug logging.""" log_file_path = os.path.join(args.logs_dir, logfile) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index ac8932112..f316fe2e8 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -421,18 +421,18 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, webroot_args) simple_map = '{"eg.com" : "/tmp"}' - expected_map = {u"eg.com": u"/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[u"eg2.com"] = u"/tmp2" + 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[u"eg2.com"] = u"/tmp" + expected_map["eg2.com"] = "/tmp" self._webroot_map_test(None, "/tmp", None, expected_map, domains) extra_args = ['-c', test_util.vector_path('webrootconftest.ini')] @@ -442,7 +442,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods '{"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", u"www.eg.com": u"/tmp", u"eg.is": "/tmp2"}) + {"eg.com": "/tmp", "www.eg.com": "/tmp", "eg.is": "/tmp2"}) @mock.patch('letsencrypt.cli._suggest_donate') @mock.patch('letsencrypt.crypto_util.notAfter') From 35ce4236e09dd3b31a9f374677be73727f955744 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 18:01:41 -0800 Subject: [PATCH 13/17] Better docs for --webroot-map --- letsencrypt/cli.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 6537ee8b8..0b9a1e5d3 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1264,7 +1264,9 @@ def _plugins_parsing(helpful, plugins): # --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.") + "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): From 9bc4efe50c81b66308d2a85809887a578be156f7 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 18:07:47 -0800 Subject: [PATCH 14/17] Better comment --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 0b9a1e5d3..a6e9eb3ed 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -114,9 +114,9 @@ def usage_strings(plugins): def _find_domains(config, installer): if not config.domains: - # set args.domains so that it's written to the renewal conf file domains = display_ops.choose_names(installer) - # record in config.domains, and set webroot_map entries if applicable + # 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: From 9cce97ee6614add4ee7725f5abe5db2b2631abd6 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 28 Jan 2016 18:19:28 -0800 Subject: [PATCH 15/17] delint --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index a6e9eb3ed..84723da3c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1294,7 +1294,7 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring "them must precede all domain flags") config.webroot_path.append(webroot) -_undot = lambda domain : domain[:-1] if domain.endswith('.') else domain +_undot = lambda domain: domain[:-1] if domain.endswith('.') else domain def _process_domain(config, domain_arg, webroot_path=None): """ From d281162f170f746bd72b97747c073a37444b7a50 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 29 Jan 2016 13:41:24 -0800 Subject: [PATCH 16/17] Domain errors should include the domain in question --- letsencrypt/le_util.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 3eb4be0a7..d97d43dc6 100644 --- a/letsencrypt/le_util.py +++ b/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/ From 60e72188e440282d79702e09461fe3c4649bb993 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 29 Jan 2016 14:01:59 -0800 Subject: [PATCH 17/17] Don't stringify unicode after all - Paths can be unicode; that's fine. - For now, unicode domains are caught and errored appropriately in le_util.check_domain_sanity(); one day they may be allowed --- letsencrypt/cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 84723da3c..dc095a42e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1294,6 +1294,7 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring "them must precede all domain flags") config.webroot_path.append(webroot) + _undot = lambda domain: domain[:-1] if domain.endswith('.') else domain def _process_domain(config, domain_arg, webroot_path=None): @@ -1317,7 +1318,7 @@ 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, str(domains), [str(webroot_path)]) + _process_domain(config, domains, [webroot_path]) class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring