From 7cb8c1264f9eb7002243bda72e90db4467fd8876 Mon Sep 17 00:00:00 2001 From: r5d Date: Thu, 31 Aug 2017 14:01:15 -0400 Subject: [PATCH] certbot: Update `renew` command output in quiet mode. (#5062) * certbot: Update `renew` command output in quiet mode. * certbot/renewal.py (_renew_describe_results): Update function. * certbot/tests/main_test.py (_test_renewal_common): Update method. Add optional arg `stdout`; Modify `mock_get_utilitiy`, `stdout`. (test_quiet_renew): Update method. --- certbot/renewal.py | 22 +++++++++++++--------- certbot/tests/main_test.py | 18 ++++++++++++++---- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/certbot/renewal.py b/certbot/renewal.py index b6c343694..2c41d2f9e 100644 --- a/certbot/renewal.py +++ b/certbot/renewal.py @@ -320,6 +320,12 @@ def _renew_describe_results(config, renew_successes, renew_failures, out = [] notify = out.append + disp = zope.component.getUtility(interfaces.IDisplay) + + def notify_error(err): + """Notify and log errors.""" + notify(err) + logger.error(err) if config.dry_run: notify("** DRY RUN: simulating 'certbot renew' close to cert expiry") @@ -338,14 +344,14 @@ def _renew_describe_results(config, renew_successes, renew_failures, "have been renewed:") notify(report(renew_successes, "success")) elif renew_failures and not renew_successes: - notify("All renewal attempts failed. The following certs could not be " - "renewed:") - notify(report(renew_failures, "failure")) + notify_error("All renewal attempts failed. The following certs could " + "not be renewed:") + notify_error(report(renew_failures, "failure")) elif renew_failures and renew_successes: notify("The following certs were successfully renewed:") - notify(report(renew_successes, "success")) - notify("\nThe following certs could not be renewed:") - notify(report(renew_failures, "failure")) + notify(report(renew_successes, "success") + "\n") + notify_error("The following certs could not be renewed:") + notify_error(report(renew_failures, "failure")) if parse_failures: notify("\nAdditionally, the following renewal configuration files " @@ -356,9 +362,7 @@ def _renew_describe_results(config, renew_successes, renew_failures, notify("** DRY RUN: simulating 'certbot renew' close to cert expiry") notify("** (The test certificates above have not been saved.)") - if config.quiet and not (renew_failures or parse_failures): - return - print("\n".join(out)) + disp.notification("\n".join(out), wrap=False) def handle_renewal_request(config): diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 51f0697f4..f0b055d4c 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -696,7 +696,8 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met self._certonly_new_request_common, mock_client) def _test_renewal_common(self, due_for_renewal, extra_args, log_out=None, - args=None, should_renew=True, error_expected=False): + args=None, should_renew=True, error_expected=False, + quiet_mode=False): # pylint: disable=too-many-locals,too-many-arguments cert_path = test_util.vector_path('cert_512.pem') chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' @@ -708,15 +709,23 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met mock_certr = mock.MagicMock() mock_key = mock.MagicMock(pem='pem_key') mock_client = mock.MagicMock() - stdout = None + stdout = six.StringIO() mock_client.obtain_certificate.return_value = (mock_certr, 'chain', mock_key, 'csr') + + def write_msg(message, *args, **kwargs): + """Write message to stdout.""" + _, _ = args, kwargs + stdout.write(message) + try: with mock.patch('certbot.cert_manager.find_duplicative_certs') as mock_fdc: mock_fdc.return_value = (mock_lineage, None) with mock.patch('certbot.main._init_le_client') as mock_init: mock_init.return_value = mock_client with test_util.patch_get_utility() as mock_get_utility: + if not quiet_mode: + mock_get_utility().notification.side_effect = write_msg with mock.patch('certbot.main.renewal.OpenSSL') as mock_ssl: mock_latest = mock.MagicMock() mock_latest.get_issuer.return_value = "Fake fake" @@ -727,7 +736,7 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met if extra_args: args += extra_args try: - ret, stdout, _, _ = self._call(args) + ret, stdout, _, _ = self._call(args, stdout) if ret: print("Returned", ret) raise AssertionError(ret) @@ -797,7 +806,8 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met self.assertTrue("renew" in out) args = ["renew", "--dry-run", "-q"] - _, _, stdout = self._test_renewal_common(True, [], args=args, should_renew=True) + _, _, stdout = self._test_renewal_common(True, [], args=args, + should_renew=True, quiet_mode=True) out = stdout.getvalue() self.assertEqual("", out)