Merge branch 'master' into real-py26-support

letsencrypt-apache/letsencrypt_apache/display_ops.py

@@ -85,12 +85,12 @@ def _vhost_menu(domain, vhosts):
             "or Address of {0}.{1}Which virtual host would you "
             "like to choose?".format(domain, os.linesep),
             choices, help_label="More Info", ok_label="Select")
-    except errors.MissingCommandlineFlag, e:
+    except errors.MissingCommandlineFlag as e:
         msg = ("Failed to run Apache plugin non-interactively{1}{0}{1}"
                "(The best solution is to add ServerName or ServerAlias "
                "entries to the VirtualHost directives of your apache "
                "configuration files.)".format(e, os.linesep))
-        raise errors.MissingCommandlineFlag, msg
+        raise errors.MissingCommandlineFlag(msg)
 
     return code, tag
 

letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py

@@ -37,7 +37,7 @@ class SelectVhostTest(unittest.TestCase):
         mock_util().menu.side_effect = errors.MissingCommandlineFlag("no vhost default")
         try:
             self._call(self.vhosts)
-        except errors.MissingCommandlineFlag, e:
+        except errors.MissingCommandlineFlag as e:
             self.assertTrue("VirtualHost directives" in e.message)
 
     @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility")

letsencrypt/cli.py

@@ -378,13 +378,21 @@ def _report_new_cert(cert_path, fullchain_path):
            .format(and_chain, path, expiry))
     reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY)
 
-def _suggest_donate():
+
-    "Suggest a donation to support Let's Encrypt"
+def _suggest_donation_if_appropriate(config):
+    """Potentially suggest a donation to support Let's Encrypt."""
+    if not config.staging:  # --dry-run implies --staging
+        reporter_util = zope.component.getUtility(interfaces.IReporter)
+        msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n"
+               "Donating to ISRG / Let's Encrypt:   https://letsencrypt.org/donate\n"
+               "Donating to EFF:                    https://eff.org/donate-le\n\n")
+        reporter_util.add_message(msg, reporter_util.LOW_PRIORITY)
+
+
+def _report_successful_dry_run():
     reporter_util = zope.component.getUtility(interfaces.IReporter)
-    msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n"
+    reporter_util.add_message("The dry run was successful.",
-           "Donating to ISRG / Let's Encrypt:   https://letsencrypt.org/donate\n"
+                              reporter_util.HIGH_PRIORITY, on_crash=False)
-           "Donating to EFF:                    https://eff.org/donate-le\n\n")
-    reporter_util.add_message(msg, reporter_util.LOW_PRIORITY)
 
 
 def _auth_from_domains(le_client, config, domains):
@@ -397,6 +405,11 @@ def _auth_from_domains(le_client, config, domains):
     # (which results in treating the request as a new certificate request).
 
     action, lineage = _treat_as_renewal(config, domains)
+    if config.dry_run and action == "reinstall":
+        logger.info(
+            "Cert not due for renewal, but simulating renewal for dry run")
+        action = "renew"
+
     if action == "reinstall":
         # The lineage already exists; allow the caller to try installing
         # it without getting a new certificate at all.
@@ -408,25 +421,31 @@ def _auth_from_domains(le_client, config, domains):
         # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574
         new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains)
         # TODO: Check whether it worked! <- or make sure errors are thrown (jdk)
-        lineage.save_successor(
+        if config.dry_run:
-            lineage.latest_common_version(), OpenSSL.crypto.dump_certificate(
+            logger.info("Dry run: skipping updating lineage at %s",
-                OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped),
+                        os.path.dirname(lineage.cert))
-            new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain))
+        else:
+            lineage.save_successor(
+                lineage.latest_common_version(), OpenSSL.crypto.dump_certificate(
+                    OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped),
+                new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain))
 
-        lineage.update_all_links_to(lineage.latest_common_version())
+            lineage.update_all_links_to(lineage.latest_common_version())
         # TODO: Check return value of save_successor
         # TODO: Also update lineage renewal config with any relevant
         #       configuration values from this attempt? <- Absolutely (jdkasten)
     elif action == "newcert":
         # TREAT AS NEW REQUEST
         lineage = le_client.obtain_and_enroll_certificate(domains)
-        if not lineage:
+        if lineage is False:
             raise errors.Error("Certificate could not be obtained")
 
-    _report_new_cert(lineage.cert, lineage.fullchain)
+    if not config.dry_run:
+        _report_new_cert(lineage.cert, lineage.fullchain)
 
     return lineage, action
 
+
 def _avoid_invalidating_lineage(config, lineage, original_server):
     "Do not renew a valid cert with one from a staging server!"
     def _is_staging(srv):
@@ -543,7 +562,7 @@ def choose_configurator_plugins(args, config, plugins, verb):
                    '{1} and "--help plugins" for more information.)'.format(
                    req_auth, os.linesep, cli_command))
 
-            raise errors.MissingCommandlineFlag, msg
+            raise errors.MissingCommandlineFlag(msg)
     else:
         need_inst = need_auth = False
     if verb == "certonly":
@@ -591,7 +610,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     """Obtain a certificate and install."""
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "run")
-    except errors.PluginSelectionError, e:
+    except errors.PluginSelectionError as e:
         return e.message
 
     domains = _find_domains(config, installer)
@@ -612,7 +631,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     else:
         display_ops.success_renewal(domains, action)
 
-    _suggest_donate()
+    _suggest_donation_if_appropriate(config)
 
 
 def obtain_cert(args, config, plugins):
@@ -626,7 +645,7 @@ def obtain_cert(args, config, plugins):
     try:
         # installers are used in auth mode to determine domain names
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "certonly")
-    except errors.PluginSelectionError, e:
+    except errors.PluginSelectionError as e:
         return e.message
 
     # TODO: Handle errors from _init_le_client?
@@ -636,14 +655,20 @@ def obtain_cert(args, config, plugins):
     if args.csr is not None:
         certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR(
             file=args.csr[0], data=args.csr[1], form="der"))
-        cert_path, _, cert_fullchain = le_client.save_certificate(
+        if args.dry_run:
-            certr, chain, args.cert_path, args.chain_path, args.fullchain_path)
+            logger.info(
-        _report_new_cert(cert_path, cert_fullchain)
+                "Dry run: skipping saving certificate to %s", args.cert_path)
+        else:
+            cert_path, _, cert_fullchain = le_client.save_certificate(
+                certr, chain, args.cert_path, args.chain_path, args.fullchain_path)
+            _report_new_cert(cert_path, cert_fullchain)
     else:
         domains = _find_domains(config, installer)
         _auth_from_domains(le_client, config, domains)
 
-    _suggest_donate()
+    if args.dry_run:
+        _report_successful_dry_run()
+    _suggest_donation_if_appropriate(config)
 
 
 def install(args, config, plugins):
@@ -655,7 +680,7 @@ def install(args, config, plugins):
     try:
         installer, _ = choose_configurator_plugins(args, config,
                                                    plugins, "install")
-    except errors.PluginSelectionError, e:
+    except errors.PluginSelectionError as e:
         return e.message
 
     domains = _find_domains(config, installer)
@@ -839,14 +864,23 @@ class HelpfulArgumentParser(object):
                 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 or parsed_args.dry_run:
-        if parsed_args.staging:
+            if (parsed_args.server not in
-            if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
+                    (flag_default("server"), constants.STAGING_URI)):
-                raise errors.Error("--server value conflicts with --staging")
+                conflicts = ["--staging"] if parsed_args.staging else []
+                conflicts += ["--dry-run"] if parsed_args.dry_run else []
+                raise errors.Error("--server value conflicts with {0}".format(
+                    " and ".join(conflicts)))
+
             parsed_args.server = constants.STAGING_URI
 
-        return parsed_args
+            if parsed_args.dry_run:
+                if self.verb != "certonly":
+                    raise errors.Error("--dry-run currently only works with the "
+                                       "'certonly' subcommand")
+                parsed_args.break_my_certs = parsed_args.staging = True
 
+        return parsed_args
 
     def determine_verb(self):
         """Determines the verb/subcommand provided by the user.
@@ -1218,6 +1252,10 @@ def _paths_parser(helpful):
     add("testing", "--test-cert", "--staging", action='store_true', dest='staging',
         help='Use the staging server to obtain test (invalid) certs; equivalent'
              ' to --server ' + constants.STAGING_URI)
+    add("testing", "--dry-run", action="store_true", dest="dry_run",
+        help="Perform a test run of the client, obtaining test (invalid) certs"
+             " but not saving them to disk. This can currently only be used"
+             " with the 'certonly' subcommand.")
 
 
 def _plugins_parsing(helpful, plugins):

letsencrypt/client.py

@@ -146,7 +146,7 @@ def perform_registration(acme, config):
     """
     try:
         return acme.register(messages.NewRegistration.from_data(email=config.email))
-    except messages.Error, e:
+    except messages.Error as e:
         err = repr(e)
         if "MX record" in err or "Validation of contact mailto" in err:
             config.namespace.email = display_ops.get_email(more=True, invalid=True)
@@ -276,8 +276,8 @@ class Client(object):
         :param plugins: A PluginsFactory object.
 
         :returns: A new :class:`letsencrypt.storage.RenewableCert` instance
-            referred to the enrolled cert lineage, or False if the cert could
+            referred to the enrolled cert lineage, False if the cert could not
-            not be obtained.
+            be obtained, or None if doing a successful dry run.
 
         """
         certr, chain, key, _ = self.obtain_certificate(domains)
@@ -298,12 +298,16 @@ class Client(object):
                 "Non-standard path(s), might not work with crontab installed "
                 "by your operating system package manager")
 
-        lineage = storage.RenewableCert.new_lineage(
+        if cli_config.dry_run:
-            domains[0], OpenSSL.crypto.dump_certificate(
+            logger.info("Dry run: Skipping creating new lineage for %s",
-                OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped),
+                        domains[0])
-            key.pem, crypto_util.dump_pyopenssl_chain(chain),
+            return None
-            params, config, cli_config)
+        else:
-        return lineage
+            return storage.RenewableCert.new_lineage(
+                domains[0], OpenSSL.crypto.dump_certificate(
+                    OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped),
+                key.pem, crypto_util.dump_pyopenssl_chain(chain),
+                params, config, cli_config)
 
     def save_certificate(self, certr, chain_cert,
                          cert_path, chain_path, fullchain_path):

letsencrypt/display/ops.py

@@ -78,11 +78,12 @@ def pick_plugin(config, default, plugins, question, ifaces):
             # it's really bad to auto-select the single available plugin in
             # non-interactive mode, because an update could later add a second
             # available plugin
-            raise errors.MissingCommandlineFlag, ("Missing command line flags. For non-interactive "
+            raise errors.MissingCommandlineFlag(
-                "execution, you will need to specify a plugin on the command line.  Run with "
+                "Missing command line flags. For non-interactive execution, "
-                "'--help plugins' to see a list of options, and see "
+                "you will need to specify a plugin on the command line.  Run "
-                " https://eff.org/letsencrypt-plugins for more detail on what the plugins "
+                "with '--help plugins' to see a list of options, and see "
-                "do and how to use them.")
+                "https://eff.org/letsencrypt-plugins for more detail on what "
+                "the plugins do and how to use them.")
 
         filtered = plugins.visible().ifaces(ifaces)
 
@@ -158,7 +159,7 @@ def get_email(more=False, invalid=False):
     except errors.MissingCommandlineFlag:
         msg = ("You should register before running non-interactively, or provide --agree-tos"
                " and --email <email_address> flags")
-        raise errors.MissingCommandlineFlag, msg
+        raise errors.MissingCommandlineFlag(msg)
 
     if code == display_util.OK:
         if le_util.safe_email(email):

letsencrypt/display/util.py

@@ -428,7 +428,7 @@ class NoninteractiveDisplay(object):
             msg += "\n" + extra
         if cli_flag:
             msg += "\n\n(You can set this with the {0} flag)".format(cli_flag)
-        raise errors.MissingCommandlineFlag, msg
+        raise errors.MissingCommandlineFlag(msg)
 
     def notification(self, message, height=10, pause=False):
         # pylint: disable=unused-argument

letsencrypt/tests/cli_test.py

@@ -1,5 +1,6 @@
 """Tests for letsencrypt.cli."""
 import argparse
+import functools
 import itertools
 import os
 import shutil
@@ -49,18 +50,16 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def _call(self, args):
         "Run the cli with output streams and actual client mocked out"
-        with mock.patch('letsencrypt.cli._suggest_donate'):
+        with mock.patch('letsencrypt.cli.client') as client:
-            with mock.patch('letsencrypt.cli.client') as client:
+            ret, stdout, stderr = self._call_no_clientmock(args)
-                ret, stdout, stderr = self._call_no_clientmock(args)
+            return ret, stdout, stderr, client
-                return ret, stdout, stderr, client
 
     def _call_no_clientmock(self, args):
         "Run the client with output streams mocked out"
         args = self.standard_args + args
-        with mock.patch('letsencrypt.cli._suggest_donate'):
+        with mock.patch('letsencrypt.cli.sys.stdout') as stdout:
-            with mock.patch('letsencrypt.cli.sys.stdout') as stdout:
+            with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
-                with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
+                ret = cli.main(args[:])  # NOTE: parser can alter its args!
-                    ret = cli.main(args[:])  # NOTE: parser can alter its args!
         return ret, stdout, stderr
 
     def _call_stdout(self, args):
@@ -69,10 +68,9 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         caller.
         """
         args = self.standard_args + args
-        with mock.patch('letsencrypt.cli._suggest_donate'):
+        with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
-            with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
+            with mock.patch('letsencrypt.cli.client') as client:
-                with mock.patch('letsencrypt.cli.client') as client:
+                ret = cli.main(args[:])  # NOTE: parser can alter its args!
-                    ret = cli.main(args[:])  # NOTE: parser can alter its args!
         return ret, None, stderr, client
 
     def test_no_flags(self):
@@ -138,7 +136,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         try:
             with mock.patch('letsencrypt.cli.sys.stderr'):
                 cli.main(self.standard_args + args[:])  # NOTE: parser can alter its args!
-        except errors.MissingCommandlineFlag, exc:
+        except errors.MissingCommandlineFlag as exc:
             self.assertTrue(message in str(exc))
         self.assertTrue(exc is not None)
 
@@ -349,50 +347,91 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                           self._call,
                           ['-d', '*.wildcard.tld'])
 
-    def test_parse_domains(self):
+    def _get_argument_parser(self):
         plugins = disco.PluginsRegistry.find_all()
+        return functools.partial(cli.prepare_and_parse_args, plugins)
+
+    def test_parse_domains(self):
+        parse = self._get_argument_parser()
 
         short_args = ['-d', 'example.com']
-        namespace = cli.prepare_and_parse_args(plugins, short_args)
+        namespace = parse(short_args)
         self.assertEqual(namespace.domains, ['example.com'])
 
         short_args = ['-d', 'trailing.period.com.']
-        namespace = cli.prepare_and_parse_args(plugins, short_args)
+        namespace = parse(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)
+        namespace = parse(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)
+        namespace = parse(long_args)
         self.assertEqual(namespace.domains, ['example.com'])
 
         long_args = ['--domains', 'trailing.period.com.']
-        namespace = cli.prepare_and_parse_args(plugins, long_args)
+        namespace = parse(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)
+        namespace = parse(long_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net'])
 
-    def test_parse_server(self):
+    def test_server_flag(self):
-        plugins = disco.PluginsRegistry.find_all()
+        parse = self._get_argument_parser()
-        short_args = ['--server', 'example.com']
+        namespace = parse('--server example.com'.split())
-        namespace = cli.prepare_and_parse_args(plugins, short_args)
         self.assertEqual(namespace.server, 'example.com')
 
+    def _check_server_conflict_message(self, parser_args, conflicting_args):
+        parse = self._get_argument_parser()
+        try:
+            parse(parser_args)
+            self.fail(  # pragma: no cover
+                "The following flags didn't conflict with "
+                '--server: {0}'.format(', '.join(conflicting_args)))
+        except errors.Error as error:
+            self.assertTrue('--server' in error.message)
+            for arg in conflicting_args:
+                self.assertTrue(arg in error.message)
+
+    def test_staging_flag(self):
+        parse = self._get_argument_parser()
         short_args = ['--staging']
-        namespace = cli.prepare_and_parse_args(plugins, short_args)
+        namespace = parse(short_args)
+        self.assertTrue(namespace.staging)
         self.assertEqual(namespace.server, constants.STAGING_URI)
 
-        short_args = ['--staging', '--server', 'example.com']
+        short_args += '--server example.com'.split()
-        self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args)
+        self._check_server_conflict_message(short_args, '--staging')
+
+    def _assert_dry_run_flag_worked(self, namespace):
+        self.assertTrue(namespace.dry_run)
+        self.assertTrue(namespace.break_my_certs)
+        self.assertTrue(namespace.staging)
+        self.assertEqual(namespace.server, constants.STAGING_URI)
+
+    def test_dry_run_flag(self):
+        parse = self._get_argument_parser()
+        short_args = ['--dry-run']
+        self.assertRaises(errors.Error, parse, short_args)
+
+        self._assert_dry_run_flag_worked(parse(short_args + ['auth']))
+        short_args += ['certonly']
+        self._assert_dry_run_flag_worked(parse(short_args))
+
+        short_args += '--server example.com'.split()
+        conflicts = ['--dry-run']
+        self._check_server_conflict_message(short_args, '--dry-run')
+
+        short_args += ['--staging']
+        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):
-        plugins = disco.PluginsRegistry.find_all()
+        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])
@@ -400,17 +439,17 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             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)
+        namespace = parse(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()
+        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.']
+            '-d', 'superfluo.us', '-d', 'www.superfluo.us']
-        namespace = cli.prepare_and_parse_args(plugins, webroot_args)
+        namespace = parse(webroot_args)
         self.assertEqual(namespace.webroot_map, {
             'example.com': '/var/www/example',
             'www.example.com': '/var/www/example',
@@ -418,7 +457,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             'superfluo.us': '/var/www/superfluous'})
 
         webroot_args = ['-d', 'stray.example.com'] + webroot_args
-        self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, webroot_args)
+        self.assertRaises(errors.Error, parse, webroot_args)
 
         simple_map = '{"eg.com" : "/tmp"}'
         expected_map = {"eg.com": "/tmp"}
@@ -440,14 +479,35 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
         webroot_map_args = ['--webroot-map',
                             '{"eg.com.,www.eg.com": "/tmp", "eg.is.": "/tmp2"}']
-        namespace = cli.prepare_and_parse_args(plugins, webroot_map_args)
+        namespace = parse(webroot_map_args)
         self.assertEqual(namespace.webroot_map,
                          {"eg.com": "/tmp", "www.eg.com": "/tmp", "eg.is": "/tmp2"})
 
-    @mock.patch('letsencrypt.cli._suggest_donate')
+    def _certonly_new_request_common(self, mock_client, args=None):
+        with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal:
+            mock_renewal.return_value = ("newcert", None)
+            with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
+                mock_init.return_value = mock_client
+                if args is None:
+                    args = []
+                args += '-d foo.bar -a standalone certonly'.split()
+                self._call(args)
+
+    @mock.patch('letsencrypt.cli.zope.component.getUtility')
+    def test_certonly_dry_run_new_request_success(self, mock_get_utility):
+        mock_client = mock.MagicMock()
+        mock_client.obtain_and_enroll_certificate.return_value = None
+        self._certonly_new_request_common(mock_client, ['--dry-run'])
+        self.assertEqual(
+            mock_client.obtain_and_enroll_certificate.call_count, 1)
+        self.assertTrue(
+            'dry run' in mock_get_utility().add_message.call_args[0][0])
+        # Asserts we don't suggest donating after a successful dry run
+        self.assertEqual(mock_get_utility().add_message.call_count, 1)
+
     @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, _suggest):
+    def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter):
         cert_path = '/etc/letsencrypt/live/foo.bar'
         date = '1970-01-01'
         mock_notAfter().date.return_value = date
@@ -458,10 +518,11 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self._certonly_new_request_common(mock_client)
         self.assertEqual(
             mock_client.obtain_and_enroll_certificate.call_count, 1)
+        cert_msg = mock_get_utility().add_message.call_args_list[0][0][0]
+        self.assertTrue(cert_path in cert_msg)
+        self.assertTrue(date in cert_msg)
         self.assertTrue(
-            cert_path in mock_get_utility().add_message.call_args[0][0])
+            'donate' in mock_get_utility().add_message.call_args[0][0])
-        self.assertTrue(
-            date in mock_get_utility().add_message.call_args[0][0])
 
     def test_certonly_new_request_failure(self):
         mock_client = mock.MagicMock()
@@ -469,69 +530,101 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertRaises(errors.Error,
                           self._certonly_new_request_common, mock_client)
 
-    def _certonly_new_request_common(self, mock_client):
+    def _test_certonly_renewal_common(self, renewal_verb, extra_args=None):
-        with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal:
-            mock_renewal.return_value = ("newcert", None)
-            with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
-                mock_init.return_value = mock_client
-                self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly'])
-
-    @mock.patch('letsencrypt.cli._suggest_donate')
-    @mock.patch('letsencrypt.cli.zope.component.getUtility')
-    @mock.patch('letsencrypt.cli._treat_as_renewal')
-    @mock.patch('letsencrypt.cli._init_le_client')
-    def test_certonly_renewal(self, mock_init, mock_renewal, mock_get_utility, _suggest):
         cert_path = 'letsencrypt/tests/testdata/cert.pem'
         chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem'
-
         mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path)
         mock_certr = mock.MagicMock()
         mock_key = mock.MagicMock(pem='pem_key')
-        mock_renewal.return_value = ("renew", mock_lineage)
+        with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal:
-        mock_client = mock.MagicMock()
+            mock_renewal.return_value = (renewal_verb, mock_lineage)
-        mock_client.obtain_certificate.return_value = (mock_certr, 'chain',
+            mock_client = mock.MagicMock()
-                                                       mock_key, 'csr')
+            mock_client.obtain_certificate.return_value = (mock_certr, 'chain',
-        mock_init.return_value = mock_client
+                                                           mock_key, 'csr')
-        with mock.patch('letsencrypt.cli.OpenSSL'):
+            with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
-            with mock.patch('letsencrypt.cli.crypto_util'):
+                mock_init.return_value = mock_client
-                self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly'])
+                get_utility_path = 'letsencrypt.cli.zope.component.getUtility'
+                with mock.patch(get_utility_path) as mock_get_utility:
+                    with mock.patch('letsencrypt.cli.OpenSSL'):
+                        with mock.patch('letsencrypt.cli.crypto_util'):
+                            args = ['-d', 'foo.bar', '-a',
+                                    'standalone', 'certonly']
+                            if extra_args:
+                                args += extra_args
+                            self._call(args)
+
         mock_client.obtain_certificate.assert_called_once_with(['foo.bar'])
-        self.assertEqual(mock_lineage.save_successor.call_count, 1)
-        mock_lineage.update_all_links_to.assert_called_once_with(
-            mock_lineage.latest_common_version())
-        self.assertTrue(
-            chain_path in mock_get_utility().add_message.call_args[0][0])
 
-    @mock.patch('letsencrypt.cli._suggest_donate')
+        return mock_lineage, mock_get_utility
-    @mock.patch('letsencrypt.crypto_util.notAfter')
+
-    @mock.patch('letsencrypt.cli.display_ops.pick_installer')
+    def test_certonly_renewal(self):
+        lineage, get_utility = self._test_certonly_renewal_common('renew')
+        self.assertEqual(lineage.save_successor.call_count, 1)
+        lineage.update_all_links_to.assert_called_once_with(
+            lineage.latest_common_version())
+        cert_msg = get_utility().add_message.call_args_list[0][0][0]
+        self.assertTrue('fullchain.pem' in cert_msg)
+        self.assertTrue('donate' in get_utility().add_message.call_args[0][0])
+
+    def test_certonly_dry_run_reinstall_is_renewal(self):
+        _, get_utility = self._test_certonly_renewal_common('reinstall',
+                                                            ['--dry-run'])
+        self.assertEqual(get_utility().add_message.call_count, 1)
+        self.assertTrue('dry run' in get_utility().add_message.call_args[0][0])
+
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
+    @mock.patch('letsencrypt.cli._treat_as_renewal')
     @mock.patch('letsencrypt.cli._init_le_client')
-    @mock.patch('letsencrypt.cli.record_chosen_plugins')
+    def test_certonly_reinstall(self, mock_init, mock_renewal, mock_get_utility):
-    def test_certonly_csr(self, _rec, mock_init, mock_get_utility,
+        mock_renewal.return_value = ('reinstall', mock.MagicMock())
-                          mock_pick_installer, mock_notAfter, _suggest):
+        mock_init.return_value = mock_client = mock.MagicMock()
-        cert_path = '/etc/letsencrypt/live/blahcert.pem'
+        self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly'])
-        date = '1970-01-01'
+        self.assertFalse(mock_client.obtain_certificate.called)
-        mock_notAfter().date.return_value = date
+        self.assertFalse(mock_client.obtain_and_enroll_certificate.called)
+        self.assertTrue(
+            'donate' in mock_get_utility().add_message.call_args[0][0])
 
+    def _test_certonly_csr_common(self, extra_args=None):
+        certr = 'certr'
+        chain = 'chain'
         mock_client = mock.MagicMock()
-        mock_client.obtain_certificate_from_csr.return_value = ('certr',
+        mock_client.obtain_certificate_from_csr.return_value = (certr, chain)
-                                                                'chain')
+        cert_path = '/etc/letsencrypt/live/example.com/cert.pem'
         mock_client.save_certificate.return_value = cert_path, None, None
-        mock_init.return_value = mock_client
+        with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
+            mock_init.return_value = mock_client
+            get_utility_path = 'letsencrypt.cli.zope.component.getUtility'
+            with mock.patch(get_utility_path) as mock_get_utility:
+                chain_path = '/etc/letsencrypt/live/example.com/chain.pem'
+                full_path = '/etc/letsencrypt/live/example.com/fullchain.pem'
+                args = ('-a standalone certonly --csr {0} --cert-path {1} '
+                        '--chain-path {2} --fullchain-path {3}').format(
+                            CSR, cert_path, chain_path, full_path).split()
+                if extra_args:
+                    args += extra_args
+                with mock.patch('letsencrypt.cli.crypto_util'):
+                    self._call(args)
 
-        installer = 'installer'
+        if '--dry-run' in args:
-        self._call(
+            self.assertFalse(mock_client.save_certificate.called)
-            ['-a', 'standalone', '-i', installer, 'certonly', '--csr', CSR,
+        else:
-             '--cert-path', cert_path, '--fullchain-path', '/',
+            mock_client.save_certificate.assert_called_once_with(
-             '--chain-path', '/'])
+                certr, chain, cert_path, chain_path, full_path)
-        self.assertEqual(mock_pick_installer.call_args[0][1], installer)
+
-        mock_client.save_certificate.assert_called_once_with(
+        return mock_get_utility
-            'certr', 'chain', cert_path, '/', '/')
+
+    def test_certonly_csr(self):
+        mock_get_utility = self._test_certonly_csr_common()
+        cert_msg = mock_get_utility().add_message.call_args_list[0][0][0]
+        self.assertTrue('cert.pem' in cert_msg)
         self.assertTrue(
-            cert_path in mock_get_utility().add_message.call_args[0][0])
+            'donate' in mock_get_utility().add_message.call_args[0][0])
+
+    def test_certonly_csr_dry_run(self):
+        mock_get_utility = self._test_certonly_csr_common(['--dry-run'])
+        self.assertEqual(mock_get_utility().add_message.call_count, 1)
         self.assertTrue(
-            date in mock_get_utility().add_message.call_args[0][0])
+            'dry run' in mock_get_utility().add_message.call_args[0][0])
 
     @mock.patch('letsencrypt.cli.client.acme_client')
     def test_revoke_with_key(self, mock_acme_client):

tests/letstest/multitester.py

@@ -141,7 +141,7 @@ def make_instance(instance_name,
     # give instance a name
     try:
         new_instance.create_tags(Tags=[{'Key': 'Name', 'Value': instance_name}])
-    except botocore.exceptions.ClientError, e:
+    except botocore.exceptions.ClientError as e:
         if "InvalidInstanceID.NotFound" in str(e):
             # This seems to be ephemeral... retry
             time.sleep(1)