From 14e03f9af0e8b4cc3c22d7b80c79e2fb04fe7b81 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 16:34:15 +0200 Subject: [PATCH 1/5] Parse possible multiple domain definitions to a list --- letsencrypt/cli.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 999555741..141309174 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -668,8 +668,32 @@ class HelpfulArgumentParser(object): parsed_args = self.parser.parse_args(self.args) parsed_args.func = self.VERBS[self.verb] + parsed_args.domains = self._parse_domains(parsed_args.domains) return parsed_args + def _parse_domains(self, domains): + """Helper function for parse_args() that parses domains from a + (possibly) comma separated list and returns list of unique domains. + + :param domains: List of domain flags + :type domains: `list` of `string` + + :returns: List of unique domains + :rtype: `list` of `string` + + """ + + uniqd = None + + if domains: + dlist = [] + for domain in domains: + dlist.extend([d.strip() for d in domain.split(",")]) + # Make sure we don't have duplicates + uniqd = [d for i,d in enumerate(dlist) if d not in dlist[:i]] + + return uniqd + def determine_verb(self): """Determines the verb/subcommand provided by the user. From 3b8d6ec58b0b355a9c83439626cba5a60d416a19 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 16:42:58 +0200 Subject: [PATCH 2/5] Modified -d help text to reflect the change --- letsencrypt/cli.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 141309174..bd51e4a0f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -837,8 +837,9 @@ def prepare_and_parse_args(plugins, args): # subparser.add_argument("domains", nargs="*", metavar="domain") helpful.add(None, "-d", "--domain", dest="domains", metavar="DOMAIN", action="append", - help="Domain names to apply. Use multiple -d flags if you want " - "to specify multiple domains") + 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.") helpful.add( None, "--duplicate", dest="duplicate", action="store_true", help="Allow getting a certificate that duplicates an existing one") From 56f21e1d35f82750b1cc8115a28182020c673acc Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 16:53:40 +0200 Subject: [PATCH 3/5] Refactor --domain flag back to -- domains --- letsencrypt-nginx/tests/boulder-integration.sh | 2 +- letsencrypt/cli.py | 12 ++++++------ letsencrypt/tests/cli_test.py | 4 ++-- tests/boulder-integration.sh | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/letsencrypt-nginx/tests/boulder-integration.sh b/letsencrypt-nginx/tests/boulder-integration.sh index 0e3e7e77a..3cbe9f6b9 100755 --- a/letsencrypt-nginx/tests/boulder-integration.sh +++ b/letsencrypt-nginx/tests/boulder-integration.sh @@ -18,7 +18,7 @@ letsencrypt_test_nginx () { "$@" } -letsencrypt_test_nginx --domain nginx.wtf run +letsencrypt_test_nginx --domains nginx.wtf run echo | openssl s_client -connect localhost:5001 \ | openssl x509 -out $root/nginx.pem diff -q $root/nginx.pem $root/conf/live/nginx.wtf/cert.pem diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index bd51e4a0f..f01bf0b96 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -106,7 +106,7 @@ def _find_domains(args, installer): domains = args.domains if not domains: - raise errors.Error("Please specify --domain, or --installer that " + raise errors.Error("Please specify --domains, or --installer that " "will help in domain names autodiscovery") return domains @@ -465,9 +465,9 @@ def obtaincert(args, config, plugins): """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 --domain is + # TODO: --csr could have a priority, when --domains is # supplied, check if CSR matches given domains? - return "--domain and --csr are mutually exclusive" + return "--domains and --csr are mutually exclusive" try: # installers are used in auth mode to determine domain names @@ -831,11 +831,11 @@ def prepare_and_parse_args(plugins, args): None, "-t", "--text", dest="text_mode", action="store_true", help="Use the text output instead of the curses UI.") helpful.add(None, "-m", "--email", help=config_help("email")) - # positional arg shadows --domain, instead of appending, and - # --domain is useful, because it can be stored in config + # positional arg shadows --domains, instead of appending, and + # --domains is useful, because it can be stored in config #for subparser in parser_run, parser_auth, parser_install: # subparser.add_argument("domains", nargs="*", metavar="domain") - helpful.add(None, "-d", "--domain", dest="domains", + helpful.add(None, "-d", "--domains", dest="domains", metavar="DOMAIN", action="append", help="Domain names to apply. For multiple domains you can use " "multiple -d flags or enter a comma separated list of domains" diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 31f528cbf..83f78e9ab 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -117,7 +117,7 @@ class CLITest(unittest.TestCase): @mock.patch('letsencrypt.cli.display_ops') def test_installer_selection(self, mock_display_ops): - self._call(['install', '--domain', 'foo.bar', '--cert-path', 'cert', + self._call(['install', '--domains', 'foo.bar', '--cert-path', 'cert', '--key-path', 'key', '--chain-path', 'chain']) self.assertEqual(mock_display_ops.pick_installer.call_count, 1) @@ -170,7 +170,7 @@ class CLITest(unittest.TestCase): def test_certonly_bad_args(self): ret, _, _, _ = self._call(['-d', 'foo.bar', 'certonly', '--csr', CSR]) - self.assertEqual(ret, '--domain and --csr are mutually exclusive') + self.assertEqual(ret, '--domains and --csr are mutually exclusive') ret, _, _, _ = self._call(['-a', 'bad_auth', 'certonly']) self.assertEqual(ret, 'The requested bad_auth plugin does not appear to be installed') diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 97babb591..53996cd20 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -27,8 +27,8 @@ common() { "$@" } -common --domain le1.wtf --standalone-supported-challenges tls-sni-01 auth -common --domain le2.wtf --standalone-supported-challenges http-01 run +common --domains le1.wtf --standalone-supported-challenges tls-sni-01 auth +common --domains le2.wtf --standalone-supported-challenges http-01 run common -a manual -d le.wtf auth export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \ @@ -40,7 +40,7 @@ common auth --csr "$CSR_PATH" \ openssl x509 -in "${root}/csr/0000_cert.pem" -text openssl x509 -in "${root}/csr/0000_chain.pem" -text -common --domain le3.wtf install \ +common --domains le3.wtf install \ --cert-path "${root}/csr/cert.pem" \ --key-path "${root}/csr/key.pem" From 37e94e631d428131131243460bce417531e2a62c Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 17:25:38 +0200 Subject: [PATCH 4/5] Added tests --- letsencrypt/tests/cli_test.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 83f78e9ab..57807555d 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -193,6 +193,27 @@ class CLITest(unittest.TestCase): self._call, ['-d', '*.wildcard.tld']) + def test_parse_domains(self): + from letsencrypt import cli + plugins = disco.PluginsRegistry.find_all() + + short_args = ['-d', 'example.com'] + namespace = cli.prepare_and_parse_args(plugins, short_args) + self.assertEqual(namespace.domains, ['example.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', + 'third.org']) + + long_args = ['--domains', 'example.com'] + namespace = cli.prepare_and_parse_args(plugins, long_args) + self.assertEqual(namespace.domains, ['example.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']) + @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.zope.component.getUtility') def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter): From 0ccbbdb120de6ce414da6a4951b009620208dbb0 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 17:46:44 +0200 Subject: [PATCH 5/5] Style fix --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f01bf0b96..7848c4afc 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -690,7 +690,7 @@ class HelpfulArgumentParser(object): for domain in domains: dlist.extend([d.strip() for d in domain.split(",")]) # Make sure we don't have duplicates - uniqd = [d for i,d in enumerate(dlist) if d not in dlist[:i]] + uniqd = [d for i, d in enumerate(dlist) if d not in dlist[:i]] return uniqd