From 18d38e525625a2a0d8dcce4c7a8e19bc10478e57 Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Tue, 23 Feb 2021 15:09:57 +1100 Subject: [PATCH 01/13] cli: reduce default logging verbosity --- certbot/certbot/_internal/constants.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot/certbot/_internal/constants.py b/certbot/certbot/_internal/constants.py index f79d6b9db..61895edb1 100644 --- a/certbot/certbot/_internal/constants.py +++ b/certbot/certbot/_internal/constants.py @@ -22,7 +22,7 @@ CLI_DEFAULTS = dict( ], # Main parser - verbose_count=-int(logging.INFO / 10), + verbose_count=-int(logging.WARNING / 10), text_mode=False, max_log_backups=1000, preconfigured_renewal=False, @@ -139,7 +139,7 @@ REVOCATION_REASONS = { """Defaults for CLI flags and `.IConfig` attributes.""" -QUIET_LOGGING_LEVEL = logging.WARNING +QUIET_LOGGING_LEVEL = logging.ERROR """Logging level to use in quiet mode.""" RENEWER_DEFAULTS = dict( From 1d7856eceb5940ef152b41755e88d78387993ee1 Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Tue, 23 Feb 2021 15:10:28 +1100 Subject: [PATCH 02/13] fix lock_test: -vv is needed to see logger.debug --- tests/lock_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lock_test.py b/tests/lock_test.py index f310b5753..a60e7b0da 100644 --- a/tests/lock_test.py +++ b/tests/lock_test.py @@ -133,7 +133,7 @@ def set_up_command(config_dir, logs_dir, work_dir, nginx_dir): return ( 'certbot --cert-path {0} --key-path {1} --config-dir {2} ' '--logs-dir {3} --work-dir {4} --nginx-server-root {5} --debug ' - '--force-renewal --nginx --verbose '.format( + '--force-renewal --nginx -vv '.format( test_util.vector_path('cert.pem'), test_util.vector_path('rsa512_key.pem'), config_dir, logs_dir, work_dir, nginx_dir).split()) From 54370c3822f9dffe403d8c4138aabf67a97b3755 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Thu, 1 Apr 2021 12:02:23 -0700 Subject: [PATCH 03/13] Change comment in log.py to match the change to default verbosity --- certbot/certbot/_internal/log.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/certbot/_internal/log.py b/certbot/certbot/_internal/log.py index fb0a996f7..beba10f57 100644 --- a/certbot/certbot/_internal/log.py +++ b/certbot/certbot/_internal/log.py @@ -13,7 +13,7 @@ and properly flushed before program exit. The `logging` module is useful for recording messages about about what Certbot is doing under the hood, but do not necessarily need to be shown -to the user on the terminal. The default verbosity is INFO. +to the user on the terminal. The default verbosity is WARNING. The preferred method to display important information to the user is to use `certbot.display.util` and `certbot.display.ops`. From 473684e86677a73fd53b2f5b35ebf774de9fa64b Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Wed, 31 Mar 2021 15:59:22 -0700 Subject: [PATCH 04/13] Audit and adjust logging levels in apache module --- .../certbot_apache/_internal/configurator.py | 12 ++++++------ .../certbot_apache/_internal/display_ops.py | 2 +- .../certbot_apache/_internal/override_debian.py | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index c24a646db..2f8665a3e 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -1838,13 +1838,13 @@ class ApacheConfigurator(common.Installer): if options: msg_enhancement += ": " + options msg = msg_tmpl.format(domain, msg_enhancement) - logger.warning(msg) + logger.error(msg) raise errors.PluginError(msg) try: for vhost in vhosts: func(vhost, options) except errors.PluginError: - logger.warning("Failed %s for %s", enhancement, domain) + logger.error("Failed %s for %s", enhancement, domain) raise def _autohsts_increase(self, vhost, id_str, nextstep): @@ -2408,7 +2408,7 @@ class ApacheConfigurator(common.Installer): try: util.run_script(self.option("restart_cmd")) except errors.SubprocessError as err: - logger.info("Unable to restart apache using %s", + logger.warning("Unable to restart apache using %s", self.option("restart_cmd")) alt_restart = self.option("restart_cmd_alt") if alt_restart: @@ -2566,7 +2566,7 @@ class ApacheConfigurator(common.Installer): msg_tmpl = ("Certbot was not able to find SSL VirtualHost for a " "domain {0} for enabling AutoHSTS enhancement.") msg = msg_tmpl.format(d) - logger.warning(msg) + logger.error(msg) raise errors.PluginError(msg) for vh in vhosts: try: @@ -2652,7 +2652,7 @@ class ApacheConfigurator(common.Installer): except errors.PluginError: msg = ("Could not find VirtualHost with ID {0}, disabling " "AutoHSTS for this VirtualHost").format(id_str) - logger.warning(msg) + logger.error(msg) # Remove the orphaned AutoHSTS entry from pluginstorage self._autohsts.pop(id_str) continue @@ -2692,7 +2692,7 @@ class ApacheConfigurator(common.Installer): except errors.PluginError: msg = ("VirtualHost with id {} was not found, unable to " "make HSTS max-age permanent.").format(id_str) - logger.warning(msg) + logger.error(msg) self._autohsts.pop(id_str) continue if self._autohsts_vhost_in_lineage(vhost, lineage): diff --git a/certbot-apache/certbot_apache/_internal/display_ops.py b/certbot-apache/certbot_apache/_internal/display_ops.py index dabf20606..875225eb9 100644 --- a/certbot-apache/certbot_apache/_internal/display_ops.py +++ b/certbot-apache/certbot_apache/_internal/display_ops.py @@ -119,7 +119,7 @@ def _vhost_menu(domain, vhosts): "guidance in non-interactive mode. Certbot may need " "vhosts to be explicitly labelled with ServerName or " "ServerAlias directives.".format(domain)) - logger.warning(msg) + logger.error(msg) raise errors.MissingCommandlineFlag(msg) return code, tag diff --git a/certbot-apache/certbot_apache/_internal/override_debian.py b/certbot-apache/certbot_apache/_internal/override_debian.py index 9f938046b..156d979e2 100644 --- a/certbot-apache/certbot_apache/_internal/override_debian.py +++ b/certbot-apache/certbot_apache/_internal/override_debian.py @@ -68,7 +68,7 @@ class DebianConfigurator(configurator.ApacheConfigurator): # Already in shape vhost.enabled = True return None - logger.warning( + logger.error( "Could not symlink %s to %s, got error: %s", enabled_path, vhost.filep, err.strerror) errstring = ("Encountered error while trying to enable a " + From f1b89119196b61b5d71ab9e5e70d319251a64850 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Wed, 31 Mar 2021 16:15:12 -0700 Subject: [PATCH 05/13] Audit and adjust logging levels in nginx module --- certbot-nginx/certbot_nginx/_internal/configurator.py | 2 +- certbot-nginx/certbot_nginx/_internal/http_01.py | 4 ++-- certbot-nginx/certbot_nginx/_internal/parser.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index aab489315..b1eeb3975 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -766,7 +766,7 @@ class NginxConfigurator(common.Installer): raise errors.PluginError( "Unsupported enhancement: {0}".format(enhancement)) except errors.PluginError: - logger.warning("Failed %s for %s", enhancement, domain) + logger.error("Failed %s for %s", enhancement, domain) raise def _has_certbot_redirect(self, vhost, domain): diff --git a/certbot-nginx/certbot_nginx/_internal/http_01.py b/certbot-nginx/certbot_nginx/_internal/http_01.py index abcdfbd53..b1c7acd24 100644 --- a/certbot-nginx/certbot_nginx/_internal/http_01.py +++ b/certbot-nginx/certbot_nginx/_internal/http_01.py @@ -128,12 +128,12 @@ class NginxHttp01(common.ChallengePerformer): ipv6_addr = ipv6_addr + " ipv6only=on" addresses = [obj.Addr.fromstring(default_addr), obj.Addr.fromstring(ipv6_addr)] - logger.info(("Using default addresses %s and %s for authentication."), + logger.debug(("Using default addresses %s and %s for authentication."), default_addr, ipv6_addr) else: addresses = [obj.Addr.fromstring(default_addr)] - logger.info("Using default address %s for authentication.", + logger.debug("Using default address %s for authentication.", default_addr) return addresses diff --git a/certbot-nginx/certbot_nginx/_internal/parser.py b/certbot-nginx/certbot_nginx/_internal/parser.py index 28833b1f7..a9a48407c 100644 --- a/certbot-nginx/certbot_nginx/_internal/parser.py +++ b/certbot-nginx/certbot_nginx/_internal/parser.py @@ -217,7 +217,7 @@ class NginxParser: "character. Only UTF-8 encoding is " "supported.", item) except pyparsing.ParseException as err: - logger.debug("Could not parse file: %s due to %s", item, err) + logger.warning("Could not parse file: %s due to %s", item, err) return trees def _find_config_root(self): @@ -430,7 +430,7 @@ def _parse_ssl_options(ssl_options): logger.warning("Could not read file: %s due to invalid character. " "Only UTF-8 encoding is supported.", ssl_options) except pyparsing.ParseBaseException as err: - logger.debug("Could not parse file: %s due to %s", ssl_options, err) + logger.warning("Could not parse file: %s due to %s", ssl_options, err) return [] def _do_for_subarray(entry, condition, func, path=None): From 2851f54713226b363a907c374c2aad05057d0f4b Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Wed, 31 Mar 2021 18:06:19 -0700 Subject: [PATCH 06/13] Audit, adjust logging levels, and improve logging calls in certbot module --- certbot/certbot/_internal/auth_handler.py | 4 ++-- certbot/certbot/_internal/client.py | 12 ++++++------ certbot/certbot/_internal/hooks.py | 2 +- certbot/certbot/_internal/main.py | 6 +++--- certbot/certbot/_internal/plugins/webroot.py | 2 +- certbot/certbot/_internal/renewal.py | 16 ++++++++-------- certbot/certbot/_internal/storage.py | 6 +++--- certbot/certbot/_internal/updater.py | 2 +- certbot/certbot/crypto_util.py | 10 ++++++---- certbot/certbot/ocsp.py | 10 +++++----- certbot/certbot/plugins/dns_common.py | 2 +- certbot/certbot/reverter.py | 2 +- certbot/certbot/util.py | 5 +++-- 13 files changed, 41 insertions(+), 38 deletions(-) diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py index c2f323a36..769346316 100644 --- a/certbot/certbot/_internal/auth_handler.py +++ b/certbot/certbot/_internal/auth_handler.py @@ -70,7 +70,6 @@ class AuthHandler: resps = self.auth.perform(achalls) # If debug is on, wait for user input before starting the verification process. - logger.info('Waiting for verification...') config = zope.component.getUtility(interfaces.IConfig) if config.debug_challenges: notify = zope.component.getUtility(interfaces.IDisplay).notification @@ -88,6 +87,7 @@ class AuthHandler: self.acme.answer_challenge(achall.challb, resp) # Wait for authorizations to be checked. + logger.info('Waiting for verification...') self._poll_authorizations(authzrs, max_retries, best_effort) # Keep validated authorizations only. If there is none, no certificate can be issued. @@ -148,7 +148,7 @@ class AuthHandler: authzrs_failed = [authzr for authzr, _ in authzrs_to_check.values() if authzr.body.status == messages.STATUS_INVALID] for authzr_failed in authzrs_failed: - logger.warning('Challenge failed for domain %s', + logger.info('Challenge failed for domain %s', authzr_failed.body.identifier.value) # Accumulating all failed authzrs to build a consolidated report # on them at the end of the polling. diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py index 796f05599..b67d15ba6 100644 --- a/certbot/certbot/_internal/client.py +++ b/certbot/certbot/_internal/client.py @@ -154,7 +154,7 @@ def register(config, account_storage, tos_cb=None): if not config.register_unsafely_without_email: msg = ("No email was provided and " "--register-unsafely-without-email was not present.") - logger.warning(msg) + logger.error(msg) raise errors.Error(msg) if not config.dry_run: logger.debug("Registering without email!") @@ -275,7 +275,7 @@ class Client: if self.auth_handler is None: msg = ("Unable to obtain certificate because authenticator is " "not set.") - logger.warning(msg) + logger.error(msg) raise errors.Error(msg) if self.account.regr is None: raise errors.Error("Please register with the ACME server first.") @@ -526,7 +526,7 @@ class Client: """ if self.installer is None: - logger.warning("No installer specified, client is unable to deploy" + logger.error("No installer specified, client is unable to deploy" "the certificate") raise errors.Error("No installer available") @@ -564,7 +564,7 @@ class Client: """ if self.installer is None: - logger.warning("No installer is specified, there isn't any " + logger.error("No installer is specified, there isn't any " "configuration to enhance.") raise errors.Error("No installer available") @@ -585,7 +585,7 @@ class Client: self.apply_enhancement(domains, enhancement_name, option) enhanced = True elif config_value: - logger.warning( + logger.error( "Option %s is not supported by the selected installer. " "Skipping enhancement.", config_name) @@ -645,7 +645,7 @@ class Client: :param str success_msg: message to show on successful rollback """ - logger.critical("Rolling back to previous server configuration...") + logger.info("Rolling back to previous server configuration...") reporter = zope.component.getUtility(interfaces.IReporter) try: self.installer.rollback_checkpoints() diff --git a/certbot/certbot/_internal/hooks.py b/certbot/certbot/_internal/hooks.py index b9f1f1531..c450950cf 100644 --- a/certbot/certbot/_internal/hooks.py +++ b/certbot/certbot/_internal/hooks.py @@ -210,7 +210,7 @@ def _run_deploy_hook(command, domains, lineage_path, dry_run): """ if dry_run: - logger.warning("Dry run: skipping deploy hook command: %s", + logger.info("Dry run: skipping deploy hook command: %s", command) return diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index ac4d18449..5f690b419 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -946,7 +946,7 @@ def enhance(config, plugins): if not enhancements.are_requested(config) and not oldstyle_enh: msg = ("Please specify one or more enhancement types to configure. To list " "the available enhancement types, run:\n\n%s --help enhance\n") - logger.warning(msg, sys.argv[0]) + logger.error(msg, sys.argv[0]) raise errors.MisconfigurationError("No enhancements requested, exiting.") try: @@ -1235,7 +1235,7 @@ def renew_cert(config, plugins, lineage): # installers are used in auth mode to determine domain names installer, auth = plug_sel.choose_configurator_plugins(config, plugins, "certonly") except errors.PluginSelectionError as e: - logger.info("Could not choose appropriate plugin: %s", e) + logger.error("Could not choose appropriate plugin: %s", e) raise le_client = _init_le_client(config, auth, installer) @@ -1278,7 +1278,7 @@ def certonly(config, plugins): # installers are used in auth mode to determine domain names installer, auth = plug_sel.choose_configurator_plugins(config, plugins, "certonly") except errors.PluginSelectionError as e: - logger.info("Could not choose appropriate plugin: %s", e) + logger.error("Could not choose appropriate plugin: %s", e) raise le_client = _init_le_client(config, auth, installer) diff --git a/certbot/certbot/_internal/plugins/webroot.py b/certbot/certbot/_internal/plugins/webroot.py index c5b436b65..25752c387 100644 --- a/certbot/certbot/_internal/plugins/webroot.py +++ b/certbot/certbot/_internal/plugins/webroot.py @@ -183,7 +183,7 @@ to serve all files under specified web root ({0}).""" filesystem.copy_ownership_and_apply_mode( path, prefix, 0o755, copy_user=True, copy_group=True) except (OSError, AttributeError) as exception: - logger.info("Unable to change owner and uid of webroot directory") + logger.warning("Unable to change owner and uid of webroot directory") logger.debug("Error was: %s", exception) except OSError as exception: raise errors.PluginError( diff --git a/certbot/certbot/_internal/renewal.py b/certbot/certbot/_internal/renewal.py index f29709ef4..48ba39b81 100644 --- a/certbot/certbot/_internal/renewal.py +++ b/certbot/certbot/_internal/renewal.py @@ -68,18 +68,18 @@ def _reconstitute(config, full_path): """ try: renewal_candidate = storage.RenewableCert(full_path, config) - except (errors.CertStorageError, IOError): - logger.warning("", exc_info=True) - logger.warning("Renewal configuration file %s is broken. Skipping.", full_path) + except (errors.CertStorageError, IOError) as error: + logger.error("Renewal configuration file %s is broken.", full_path) + logger.error("The error was: %s\nSkipping.", str(error)) logger.debug("Traceback was:\n%s", traceback.format_exc()) return None if "renewalparams" not in renewal_candidate.configuration: - logger.warning("Renewal configuration file %s lacks " + logger.error("Renewal configuration file %s lacks " "renewalparams. Skipping.", full_path) return None renewalparams = renewal_candidate.configuration["renewalparams"] if "authenticator" not in renewalparams: - logger.warning("Renewal configuration file %s does not specify " + logger.error("Renewal configuration file %s does not specify " "an authenticator. Skipping.", full_path) return None # Now restore specific values along with their data types, if @@ -89,7 +89,7 @@ def _reconstitute(config, full_path): restore_required_config_elements(config, renewalparams) _restore_plugin_configs(config, renewalparams) except (ValueError, errors.Error) as error: - logger.warning( + logger.error( "An error occurred while parsing %s. The error was %s. " "Skipping the file.", full_path, str(error)) logger.debug("Traceback was:\n%s", traceback.format_exc()) @@ -99,7 +99,7 @@ def _reconstitute(config, full_path): config.domains = [util.enforce_domain_sanity(d) for d in renewal_candidate.names()] except errors.ConfigurationError as error: - logger.warning("Renewal configuration file %s references a certificate " + logger.error("Renewal configuration file %s references a certificate " "that contains an invalid domain name. The problem " "was: %s. Skipping.", full_path, error) return None @@ -447,7 +447,7 @@ def handle_renewal_request(config): try: renewal_candidate = _reconstitute(lineage_config, renewal_file) except Exception as e: # pylint: disable=broad-except - logger.warning("Renewal configuration file %s (cert: %s) " + logger.error("Renewal configuration file %s (cert: %s) " "produced an unexpected error: %s. Skipping.", renewal_file, lineagename, e) logger.debug("Traceback was:\n%s", traceback.format_exc()) diff --git a/certbot/certbot/_internal/storage.py b/certbot/certbot/_internal/storage.py index 11dae33e9..c9f6f61fa 100644 --- a/certbot/certbot/_internal/storage.py +++ b/certbot/certbot/_internal/storage.py @@ -327,7 +327,7 @@ def delete_files(config, certname): renewal_config = configobj.ConfigObj(renewal_filename) except configobj.ConfigObjError: # config is corrupted - logger.warning("Could not parse %s. You may wish to manually " + logger.error("Could not parse %s. You may wish to manually " "delete the contents of %s and %s.", renewal_filename, full_default_live_dir, full_default_archive_dir) raise errors.CertStorageError( @@ -336,7 +336,7 @@ def delete_files(config, certname): # we couldn't read it, but let's at least delete it # if this was going to fail, it already would have. os.remove(renewal_filename) - logger.debug("Removed %s", renewal_filename) + logger.info("Removed %s", renewal_filename) # cert files and (hopefully) live directory # it's not guaranteed that the files are in our default storage @@ -911,7 +911,7 @@ class RenewableCert(interfaces.RenewableCert): return ocsp.RevocationChecker().ocsp_revoked_by_paths(cert_path, chain_path) except Exception as e: # pylint: disable=broad-except - logger.warning( + logger.error( "An error occurred determining the OCSP status of %s.", cert_path) logger.debug(str(e)) diff --git a/certbot/certbot/_internal/updater.py b/certbot/certbot/_internal/updater.py index 23ba06da3..f38fadfbf 100644 --- a/certbot/certbot/_internal/updater.py +++ b/certbot/certbot/_internal/updater.py @@ -29,7 +29,7 @@ def run_generic_updaters(config, lineage, plugins): try: installer = plug_sel.get_unprepared_installer(config, plugins) except errors.Error as e: - logger.warning("Could not choose appropriate plugin for updaters: %s", e) + logger.error("Could not choose appropriate plugin for updaters: %s", e) return if installer: _run_updaters(lineage, installer, config) diff --git a/certbot/certbot/crypto_util.py b/certbot/certbot/crypto_util.py index 445618ea0..82f1305b8 100644 --- a/certbot/certbot/crypto_util.py +++ b/certbot/certbot/crypto_util.py @@ -64,7 +64,8 @@ def init_save_key( bits=key_size, elliptic_curve=elliptic_curve or "secp256r1", key_type=key_type, ) except ValueError as err: - logger.error("", exc_info=True) + logger.debug("", exc_info=True) + logger.error("Encountered error while making key: %s", str(err)) raise err config = zope.component.getUtility(interfaces.IConfig) @@ -388,8 +389,9 @@ def _load_cert_or_req(cert_or_req_str, load_func, typ=crypto.FILETYPE_PEM): try: return load_func(typ, cert_or_req_str) - except crypto.Error: - logger.error("", exc_info=True) + except crypto.Error as err: + logger.debug("", exc_info=True) + logger.error("Encountered error while loading certificate or csr: %s", str(err)) raise @@ -590,7 +592,7 @@ def find_chain_with_issuer(fullchains, issuer_cn, warn_on_no_match=False): # Nothing matched, return whatever was first in the list. if warn_on_no_match: - logger.info("Certbot has been configured to prefer certificate chains with " + logger.warning("Certbot has been configured to prefer certificate chains with " "issuer '%s', but no chain from the CA matched this issuer. Using " "the default certificate chain instead.", issuer_cn) return fullchains[0] diff --git a/certbot/certbot/ocsp.py b/certbot/certbot/ocsp.py index 0a842e108..6d209bfbb 100644 --- a/certbot/certbot/ocsp.py +++ b/certbot/certbot/ocsp.py @@ -193,7 +193,7 @@ def _check_ocsp_cryptography(cert_path: str, chain_path: str, url: str, timeout: # Check OCSP response validity if response_ocsp.response_status != ocsp.OCSPResponseStatus.SUCCESSFUL: - logger.error("Invalid OCSP response status for %s: %s", + logger.warning("Invalid OCSP response status for %s: %s", cert_path, response_ocsp.response_status) return False @@ -201,13 +201,13 @@ def _check_ocsp_cryptography(cert_path: str, chain_path: str, url: str, timeout: try: _check_ocsp_response(response_ocsp, request, issuer, cert_path) except UnsupportedAlgorithm as e: - logger.error(str(e)) + logger.warning(str(e)) except errors.Error as e: - logger.error(str(e)) + logger.warning(str(e)) except InvalidSignature: - logger.error('Invalid signature on OCSP response for %s', cert_path) + logger.warning('Invalid signature on OCSP response for %s', cert_path) except AssertionError as error: - logger.error('Invalid OCSP response for %s: %s.', cert_path, str(error)) + logger.warning('Invalid OCSP response for %s: %s.', cert_path, str(error)) else: # Check OCSP certificate status logger.debug("OCSP certificate status for %s is: %s", diff --git a/certbot/certbot/plugins/dns_common.py b/certbot/certbot/plugins/dns_common.py index 52328ad85..334b9665d 100644 --- a/certbot/certbot/plugins/dns_common.py +++ b/certbot/certbot/plugins/dns_common.py @@ -63,7 +63,7 @@ class DNSAuthenticator(common.Plugin): # DNS updates take time to propagate and checking to see if the update has occurred is not # reliable (the machine this code is running on might be able to see an update before # the ACME server). So: we sleep for a short amount of time we believe to be long enough. - logger.info("Waiting %d seconds for DNS changes to propagate", + display_util.notify("Waiting %d seconds for DNS changes to propagate", self.conf('propagation-seconds')) sleep(self.conf('propagation-seconds')) diff --git a/certbot/certbot/reverter.py b/certbot/certbot/reverter.py index ebc69fcbe..5e2ff498e 100644 --- a/certbot/certbot/reverter.py +++ b/certbot/certbot/reverter.py @@ -521,7 +521,7 @@ class Reverter: filesystem.replace(self.config.in_progress_dir, final_dir) return except OSError: - logger.warning("Extreme, unexpected race condition, retrying (%s)", timestamp) + logger.warning("Unexpected race condition, retrying (%s)", timestamp) # After 10 attempts... something is probably wrong here... logger.error( diff --git a/certbot/certbot/util.py b/certbot/certbot/util.py index 5f4a08dc7..023852255 100644 --- a/certbot/certbot/util.py +++ b/certbot/certbot/util.py @@ -16,6 +16,7 @@ from typing import Dict from typing import Text from typing import Tuple from typing import Union +import warnings import configargparse @@ -434,14 +435,14 @@ def safe_email(email): """Scrub email address before using it.""" if EMAIL_REGEX.match(email) is not None: return not email.startswith(".") and ".." not in email - logger.warning("Invalid email address: %s.", email) + logger.error("Invalid email address: %s.", email) return False class DeprecatedArgumentAction(argparse.Action): """Action to log a warning when an argument is used.""" def __call__(self, unused1, unused2, unused3, option_string=None): - logger.warning("Use of %s is deprecated.", option_string) + warnings.warn("Use of %s is deprecated." % option_string, DeprecationWarning) def add_deprecated_argument(add_argument, argument_name, nargs): From 0ec54b01cf5bc25ac47e102fed35462525303f45 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Fri, 2 Apr 2021 11:44:43 -0700 Subject: [PATCH 07/13] Fix tests to mock correct methods and classes --- certbot-apache/tests/autohsts_test.py | 4 ++-- certbot-apache/tests/configurator_test.py | 2 +- certbot-dns-cloudflare/tests/dns_cloudflare_test.py | 6 ++++-- .../tests/dns_digitalocean_test.py | 3 ++- certbot-dns-google/tests/dns_google_test.py | 6 ++++-- certbot-dns-rfc2136/tests/dns_rfc2136_test.py | 6 ++++-- certbot/certbot/plugins/dns_common.py | 2 +- certbot/certbot/plugins/dns_test_common_lexicon.py | 3 ++- certbot/tests/client_test.py | 13 ++++++++----- certbot/tests/crypto_util_test.py | 6 +++--- certbot/tests/hook_test.py | 4 ++-- certbot/tests/plugins/dns_common_test.py | 3 ++- certbot/tests/storage_test.py | 2 +- certbot/tests/util_test.py | 8 ++++---- 14 files changed, 40 insertions(+), 28 deletions(-) diff --git a/certbot-apache/tests/autohsts_test.py b/certbot-apache/tests/autohsts_test.py index db1c46b52..f09e02fc7 100644 --- a/certbot-apache/tests/autohsts_test.py +++ b/certbot-apache/tests/autohsts_test.py @@ -146,7 +146,7 @@ class AutoHSTSTest(util.ApacheTest): @mock.patch("certbot_apache._internal.display_ops.select_vhost") def test_autohsts_no_ssl_vhost(self, mock_select): mock_select.return_value = self.vh_truth[0] - with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log: + with mock.patch("certbot_apache._internal.configurator.logger.error") as mock_log: self.assertRaises(errors.PluginError, self.config.enable_autohsts, mock.MagicMock(), "invalid.example.com") @@ -179,7 +179,7 @@ class AutoHSTSTest(util.ApacheTest): self.config._autohsts_fetch_state() self.config._autohsts["orphan_id"] = {"laststep": 999, "timestamp": 0} self.config._autohsts_save_state() - with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log: + with mock.patch("certbot_apache._internal.configurator.logger.error") as mock_log: self.config.deploy_autohsts(mock.MagicMock()) self.assertTrue(mock_log.called) self.assertTrue( diff --git a/certbot-apache/tests/configurator_test.py b/certbot-apache/tests/configurator_test.py index 302bf0023..ccf26a8b2 100644 --- a/certbot-apache/tests/configurator_test.py +++ b/certbot-apache/tests/configurator_test.py @@ -892,7 +892,7 @@ class MultipleVhostsTest(util.ApacheTest): self.config.enhance, "certbot.demo", "unknown_enhancement") def test_enhance_no_ssl_vhost(self): - with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log: + with mock.patch("certbot_apache._internal.configurator.logger.error") as mock_log: self.assertRaises(errors.PluginError, self.config.enhance, "certbot.demo", "redirect") # Check that correct logger.warning was printed diff --git a/certbot-dns-cloudflare/tests/dns_cloudflare_test.py b/certbot-dns-cloudflare/tests/dns_cloudflare_test.py index e9adf4ed9..6aaebde3b 100644 --- a/certbot-dns-cloudflare/tests/dns_cloudflare_test.py +++ b/certbot-dns-cloudflare/tests/dns_cloudflare_test.py @@ -41,7 +41,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic # _get_cloudflare_client | pylint: disable=protected-access self.auth._get_cloudflare_client = mock.MagicMock(return_value=self.mock_client) - def test_perform(self): + @test_util.patch_get_utility() + def test_perform(self, unused_mock_get_utility): self.auth.perform([self.achall]) expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY, mock.ANY)] @@ -55,7 +56,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic expected = [mock.call.del_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY)] self.assertEqual(expected, self.mock_client.mock_calls) - def test_api_token(self): + @test_util.patch_get_utility() + def test_api_token(self, unused_mock_get_utility): dns_test_common.write({"cloudflare_api_token": API_TOKEN}, self.config.cloudflare_credentials) self.auth.perform([self.achall]) diff --git a/certbot-dns-digitalocean/tests/dns_digitalocean_test.py b/certbot-dns-digitalocean/tests/dns_digitalocean_test.py index 84bb35b1f..0dfa2fd59 100644 --- a/certbot-dns-digitalocean/tests/dns_digitalocean_test.py +++ b/certbot-dns-digitalocean/tests/dns_digitalocean_test.py @@ -37,7 +37,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic # _get_digitalocean_client | pylint: disable=protected-access self.auth._get_digitalocean_client = mock.MagicMock(return_value=self.mock_client) - def test_perform(self): + @test_util.patch_get_utility() + def test_perform(self, unused_mock_get_utility): self.auth.perform([self.achall]) expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY, 30)] diff --git a/certbot-dns-google/tests/dns_google_test.py b/certbot-dns-google/tests/dns_google_test.py index 7de5f1d67..4f71db551 100644 --- a/certbot-dns-google/tests/dns_google_test.py +++ b/certbot-dns-google/tests/dns_google_test.py @@ -43,7 +43,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic # _get_google_client | pylint: disable=protected-access self.auth._get_google_client = mock.MagicMock(return_value=self.mock_client) - def test_perform(self): + @test_util.patch_get_utility() + def test_perform(self, unused_mock_get_utility): self.auth.perform([self.achall]) expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY, mock.ANY)] @@ -58,7 +59,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic self.assertEqual(expected, self.mock_client.mock_calls) @mock.patch('httplib2.Http.request', side_effect=ServerNotFoundError) - def test_without_auth(self, unused_mock): + @test_util.patch_get_utility() + def test_without_auth(self, unused_mock_get_utility, unused_mock): self.config.google_credentials = None self.assertRaises(PluginError, self.auth.perform, [self.achall]) diff --git a/certbot-dns-rfc2136/tests/dns_rfc2136_test.py b/certbot-dns-rfc2136/tests/dns_rfc2136_test.py index dc4a73a04..bb1799a45 100644 --- a/certbot-dns-rfc2136/tests/dns_rfc2136_test.py +++ b/certbot-dns-rfc2136/tests/dns_rfc2136_test.py @@ -42,7 +42,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic # _get_rfc2136_client | pylint: disable=protected-access self.auth._get_rfc2136_client = mock.MagicMock(return_value=self.mock_client) - def test_perform(self): + @test_util.patch_get_utility() + def test_perform(self, unused_mock_get_utility): self.auth.perform([self.achall]) expected = [mock.call.add_txt_record('_acme-challenge.'+DOMAIN, mock.ANY, mock.ANY)] @@ -65,7 +66,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic self.auth.perform, [self.achall]) - def test_valid_algorithm_passes(self): + @test_util.patch_get_utility() + def test_valid_algorithm_passes(self, unused_mock_get_utility): config = VALID_CONFIG.copy() config["rfc2136_algorithm"] = "HMAC-sha512" dns_test_common.write(config, self.config.rfc2136_credentials) diff --git a/certbot/certbot/plugins/dns_common.py b/certbot/certbot/plugins/dns_common.py index 334b9665d..3b51700f7 100644 --- a/certbot/certbot/plugins/dns_common.py +++ b/certbot/certbot/plugins/dns_common.py @@ -63,7 +63,7 @@ class DNSAuthenticator(common.Plugin): # DNS updates take time to propagate and checking to see if the update has occurred is not # reliable (the machine this code is running on might be able to see an update before # the ACME server). So: we sleep for a short amount of time we believe to be long enough. - display_util.notify("Waiting %d seconds for DNS changes to propagate", + display_util.notify("Waiting %d seconds for DNS changes to propagate" % self.conf('propagation-seconds')) sleep(self.conf('propagation-seconds')) diff --git a/certbot/certbot/plugins/dns_test_common_lexicon.py b/certbot/certbot/plugins/dns_test_common_lexicon.py index 48261d395..f9c09e821 100644 --- a/certbot/certbot/plugins/dns_test_common_lexicon.py +++ b/certbot/certbot/plugins/dns_test_common_lexicon.py @@ -67,7 +67,8 @@ class _LexiconAwareTestCase(Protocol): class BaseLexiconAuthenticatorTest(dns_test_common.BaseAuthenticatorTest): - def test_perform(self: _AuthenticatorCallableLexiconTestCase): + @test_util.patch_get_utility() + def test_perform(self: _AuthenticatorCallableLexiconTestCase, unused_mock_get_utility): self.auth.perform([self.achall]) expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY)] diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index f058cb658..34fbe8bdf 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -100,7 +100,8 @@ class RegisterTest(test_util.ConfigTestCase): self._call() self.assertTrue(mock_prepare.called) - def test_it(self): + @test_util.patch_get_utility() + def test_it(self, unused_mock_get_utility): with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_client: mock_client().external_account_required.side_effect = self._false_mock with mock.patch("certbot._internal.eff.handle_subscription"): @@ -160,7 +161,8 @@ class RegisterTest(test_util.ConfigTestCase): # check Certbot created an account with no email. Contact should return empty self.assertFalse(mock_client().new_account_and_tos.call_args[0][0].contact) - def test_with_eab_arguments(self): + @test_util.patch_get_utility() + def test_with_eab_arguments(self, unused_mock_get_utility): with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_client: mock_client().client.directory.__getitem__ = mock.Mock( side_effect=self._new_acct_dir_mock @@ -175,7 +177,8 @@ class RegisterTest(test_util.ConfigTestCase): self.assertTrue(mock_eab_from_data.called) - def test_without_eab_arguments(self): + @test_util.patch_get_utility() + def test_without_eab_arguments(self, unused_mock_get_utility): with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_client: mock_client().external_account_required.side_effect = self._false_mock with mock.patch("certbot._internal.eff.handle_subscription"): @@ -316,7 +319,7 @@ class ClientTest(ClientTestCommon): errors.Error, self.client.obtain_certificate_from_csr, test_csr) - mock_logger.warning.assert_called_once_with(mock.ANY) + mock_logger.error.assert_called_once_with(mock.ANY) @mock.patch("certbot._internal.client.crypto_util") def test_obtain_certificate(self, mock_crypto_util): @@ -602,7 +605,7 @@ class EnhanceConfigTest(ClientTestCommon): self.config.hsts = True with mock.patch("certbot._internal.client.logger") as mock_logger: self.client.enhance_config([self.domain], None) - self.assertEqual(mock_logger.warning.call_count, 1) + self.assertEqual(mock_logger.error.call_count, 1) self.client.installer.enhance.assert_not_called() @mock.patch("certbot._internal.client.logger") diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index 3b9c973f7..9829c5f91 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -493,13 +493,13 @@ class FindChainWithIssuerTest(unittest.TestCase): self.assertEqual(matched, fullchains[0]) mock_info.assert_not_called() - @mock.patch('certbot.crypto_util.logger.info') - def test_warning_on_no_match(self, mock_info): + @mock.patch('certbot.crypto_util.logger.warning') + def test_warning_on_no_match(self, mock_warning): fullchains = self._all_fullchains() matched = self._call(fullchains, "non-existent issuer", warn_on_no_match=True) self.assertEqual(matched, fullchains[0]) - mock_info.assert_called_once_with("Certbot has been configured to prefer " + mock_warning.assert_called_once_with("Certbot has been configured to prefer " "certificate chains with issuer '%s', but no chain from the CA matched " "this issuer. Using the default certificate chain instead.", "non-existent issuer") diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py index b0522bd52..4133405c9 100644 --- a/certbot/tests/hook_test.py +++ b/certbot/tests/hook_test.py @@ -345,7 +345,7 @@ class DeployHookTest(RenewalHookTest): mock_execute = self._call_with_mock_execute( self.config, ["example.org"], "/foo/bar") self.assertFalse(mock_execute.called) - self.assertTrue(mock_logger.warning.called) + self.assertTrue(mock_logger.info.called) @mock.patch("certbot._internal.hooks.logger") def test_no_hook(self, mock_logger): @@ -393,7 +393,7 @@ class RenewHookTest(RenewalHookTest): mock_execute = self._call_with_mock_execute( self.config, ["example.org"], "/foo/bar") self.assertFalse(mock_execute.called) - self.assertEqual(mock_logger.warning.call_count, 2) + self.assertEqual(mock_logger.info.call_count, 2) def test_no_hooks(self): self.config.renew_hook = None diff --git a/certbot/tests/plugins/dns_common_test.py b/certbot/tests/plugins/dns_common_test.py index 31761e986..f887a79dc 100644 --- a/certbot/tests/plugins/dns_common_test.py +++ b/certbot/tests/plugins/dns_common_test.py @@ -42,7 +42,8 @@ class DNSAuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthen self.auth = DNSAuthenticatorTest._FakeDNSAuthenticator(self.config, "fake") - def test_perform(self): + @test_util.patch_get_utility() + def test_perform(self, unused_mock_get_utility): self.auth.perform([self.achall]) self.auth._perform.assert_called_once_with(dns_test_common.DOMAIN, mock.ANY, mock.ANY) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index 1e3a1ffd8..5557db76a 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -716,7 +716,7 @@ class RenewableCertTests(BaseRenewableCertTest): # Test with error mock_checker.side_effect = ValueError - with mock.patch("certbot._internal.storage.logger.warning") as logger: + with mock.patch("certbot._internal.storage.logger.error") as logger: self.assertFalse(self.test_rc.ocsp_revoked(version)) self.assertEqual(mock_checker.call_args[0][0], expected_cert_path) self.assertEqual(mock_checker.call_args[0][1], expected_chain_path) diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 18947c342..abde665a5 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -346,19 +346,19 @@ class AddDeprecatedArgumentTest(unittest.TestCase): def test_warning_no_arg(self): self._call("--old-option", 0) - with mock.patch("certbot.util.logger.warning") as mock_warn: + with mock.patch("warnings.warn") as mock_warn: self.parser.parse_args(["--old-option"]) self.assertEqual(mock_warn.call_count, 1) self.assertTrue("is deprecated" in mock_warn.call_args[0][0]) - self.assertEqual("--old-option", mock_warn.call_args[0][1]) + self.assertTrue("--old-option" in mock_warn.call_args[0][0]) def test_warning_with_arg(self): self._call("--old-option", 1) - with mock.patch("certbot.util.logger.warning") as mock_warn: + with mock.patch("warnings.warn") as mock_warn: self.parser.parse_args(["--old-option", "42"]) self.assertEqual(mock_warn.call_count, 1) self.assertTrue("is deprecated" in mock_warn.call_args[0][0]) - self.assertEqual("--old-option", mock_warn.call_args[0][1]) + self.assertTrue("--old-option" in mock_warn.call_args[0][0]) def test_help(self): self._call("--old-option", 2) From 6c1447bb990cdbf751c00fca63fb632fe8d8a434 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Thu, 22 Apr 2021 15:13:05 -0700 Subject: [PATCH 08/13] Change ocsp check error to warning since it's non-fatal --- certbot/certbot/_internal/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/certbot/_internal/storage.py b/certbot/certbot/_internal/storage.py index c9f6f61fa..c6384d491 100644 --- a/certbot/certbot/_internal/storage.py +++ b/certbot/certbot/_internal/storage.py @@ -911,7 +911,7 @@ class RenewableCert(interfaces.RenewableCert): return ocsp.RevocationChecker().ocsp_revoked_by_paths(cert_path, chain_path) except Exception as e: # pylint: disable=broad-except - logger.error( + logger.warning( "An error occurred determining the OCSP status of %s.", cert_path) logger.debug(str(e)) From dd539c514284bcb72fbe12dbd6716329e93b8495 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Thu, 22 Apr 2021 15:24:13 -0700 Subject: [PATCH 09/13] Update storage_test in parallel with last change --- certbot/tests/storage_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index fcf65abf5..d7ef24b43 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -716,7 +716,7 @@ class RenewableCertTests(BaseRenewableCertTest): # Test with error mock_checker.side_effect = ValueError - with mock.patch("certbot._internal.storage.logger.error") as logger: + with mock.patch("certbot._internal.storage.logger.warning") as logger: self.assertFalse(self.test_rc.ocsp_revoked(version)) self.assertEqual(mock_checker.call_args[0][0], expected_cert_path) self.assertEqual(mock_checker.call_args[0][1], expected_chain_path) From 442dd25cb3ebe06afa5fe0652a32a3a81a56832a Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Tue, 11 May 2021 15:10:21 -0700 Subject: [PATCH 10/13] Decrease logging level to info for idempotent operation where enhancement is already set --- certbot/certbot/_internal/client.py | 4 ++-- certbot/tests/client_test.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py index d8e94610c..a5b92999d 100644 --- a/certbot/certbot/_internal/client.py +++ b/certbot/certbot/_internal/client.py @@ -621,10 +621,10 @@ class Client: self.installer.enhance(dom, enhancement, options) except errors.PluginEnhancementAlreadyPresent: if enhancement == "ensure-http-header": - logger.warning("Enhancement %s was already set.", + logger.info("Enhancement %s was already set.", options) else: - logger.warning("Enhancement %s was already set.", + logger.info("Enhancement %s was already set.", enhancement) except errors.PluginError: logger.warning("Unable to set enhancement %s for %s", diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index a49f1c9ac..b789770f4 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -611,16 +611,16 @@ class EnhanceConfigTest(ClientTestCommon): def test_already_exists_header(self, mock_log): self.config.hsts = True self._test_with_already_existing() - self.assertIs(mock_log.warning.called, True) - self.assertEqual(mock_log.warning.call_args[0][1], + self.assertIs(mock_log.info.called, True) + self.assertEqual(mock_log.info.call_args[0][1], 'Strict-Transport-Security') @mock.patch("certbot._internal.client.logger") def test_already_exists_redirect(self, mock_log): self.config.redirect = True self._test_with_already_existing() - self.assertIs(mock_log.warning.called, True) - self.assertEqual(mock_log.warning.call_args[0][1], + self.assertIs(mock_log.info.called, True) + self.assertEqual(mock_log.info.call_args[0][1], 'redirect') @mock.patch("certbot._internal.client.logger") From 17fa7f3d37c6d48a02945d49ffaae81827031c6b Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Tue, 11 May 2021 15:14:05 -0700 Subject: [PATCH 11/13] Display cert not yet due for renewal message when renewing and no other action will be taken, and change cert to certificate --- certbot/certbot/_internal/renewal.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/certbot/certbot/_internal/renewal.py b/certbot/certbot/_internal/renewal.py index 48ba39b81..6b4ab1163 100644 --- a/certbot/certbot/_internal/renewal.py +++ b/certbot/certbot/_internal/renewal.py @@ -295,12 +295,12 @@ def should_renew(config, lineage): logger.debug("Auto-renewal forced with --force-renewal...") return True if lineage.should_autorenew(): - logger.info("Cert is due for renewal, auto-renewing...") + logger.info("Certificate is due for renewal, auto-renewing...") return True if config.dry_run: - logger.info("Cert not due for renewal, but simulating renewal for dry run") + logger.info("Certificate not due for renewal, but simulating renewal for dry run") return True - logger.info("Cert not yet due for renewal") + display_util.notify("Certificate not yet due for renewal") return False From 803a4b0988c259d36b96b879a994a6334b1fe69d Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Tue, 11 May 2021 15:34:15 -0700 Subject: [PATCH 12/13] also write to logger so it goes in the log file --- certbot/certbot/_internal/renewal.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot/certbot/_internal/renewal.py b/certbot/certbot/_internal/renewal.py index 6b4ab1163..cfdc5ada1 100644 --- a/certbot/certbot/_internal/renewal.py +++ b/certbot/certbot/_internal/renewal.py @@ -301,6 +301,7 @@ def should_renew(config, lineage): logger.info("Certificate not due for renewal, but simulating renewal for dry run") return True display_util.notify("Certificate not yet due for renewal") + logger.debug("Certificate not yet due for renewal") return False From c75beb0116a151f107f6d395f98f84ddcf709142 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Thu, 13 May 2021 15:44:24 -0700 Subject: [PATCH 13/13] Don't double write to log file; fix main test --- certbot/certbot/_internal/renewal.py | 1 - certbot/tests/main_test.py | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/certbot/certbot/_internal/renewal.py b/certbot/certbot/_internal/renewal.py index cfdc5ada1..6b4ab1163 100644 --- a/certbot/certbot/_internal/renewal.py +++ b/certbot/certbot/_internal/renewal.py @@ -301,7 +301,6 @@ def should_renew(config, lineage): logger.info("Certificate not due for renewal, but simulating renewal for dry run") return True display_util.notify("Certificate not yet due for renewal") - logger.debug("Certificate not yet due for renewal") return False diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 5689c0281..6cf38e2ce 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1146,8 +1146,9 @@ class MainTest(test_util.ConfigTestCase): log_out="Auto-renewal forced") self.assertEqual(get_utility().add_message.call_count, 1) - self._test_renewal_common(False, ['-tvv', '--debug', '--keep'], - log_out="not yet due", should_renew=False) + _, get_utility, _ = self._test_renewal_common(False, ['-tvv', '--debug', '--keep'], + should_renew=False) + self.assertIn('not yet due', get_utility().notification.call_args[0][0]) def _dump_log(self): print("Logs:")