try and improve should_autorenew comments (#10391)

i wrote this in response to erica's thread at
https://opensource.eff.org/eff-open-source/pl/xtuemgdti78xfx1hn9jwbrfrjy

this hopefully helps some, but i think our logic here is unfortunately
fairly complicated which is reflected in the code comments. feel free to
suggest alternate wording or even just close this if you think our
comments currently in main are good enough
This commit is contained in:
Brad Warren
2025-08-04 13:48:12 -07:00
committed by GitHub
parent d80b1d395a
commit 581977c92d
+10 -11
View File
@@ -389,14 +389,15 @@ def should_autorenew(config: configuration.NamespaceConfig,
acme_clients: Dict[str, acme_client.ClientV2]) -> bool: acme_clients: Dict[str, acme_client.ClientV2]) -> bool:
"""Should we now try to autorenew the most recent cert version? """Should we now try to autorenew the most recent cert version?
If ACME Renewal Info (ARI) is available in the directory, check that first, If automatic renewal is disabled for the lineage, this function
and renew if ARI indicates it is time, or if we are within the default immediately returns False.
renweal window.
If the certificate has an OCSP URL, renew if it is revoked. Otherwise, if any of ACME Renewal Info (ARI), OCSP, or the
renew_before_expiry config option indicates we should renew, we
If neither of the above is true, but the "renew_before_expiry" config return True. If none of those values say we should renew and at
indicates it is time, renew. Otherwise, don't. least one of ARI or renew_before_expiry has an available value, we
return False. We otherwise decide whether to renew based on the
certificate's remaining lifetime.
Note that this examines the numerically most recent cert version, Note that this examines the numerically most recent cert version,
not the currently deployed version. not the currently deployed version.
@@ -425,9 +426,7 @@ def should_autorenew(config: configuration.NamespaceConfig,
logger.debug("Should renew, certificate is revoked.") logger.debug("Should renew, certificate is revoked.")
return True return True
# The "renew_before_expiry" config field can make us renew earlier than the # If the renew_before_expiry config field is set, check if it says we should renew
# default. If ARI response was None and no "renew_before_expiry" is set,
# check against the default.
config_interval = lineage.configuration.get("renew_before_expiry") config_interval = lineage.configuration.get("renew_before_expiry")
if config_interval is not None: if config_interval is not None:
notAfter = crypto_util.notAfter(cert) notAfter = crypto_util.notAfter(cert)
@@ -436,7 +435,7 @@ def should_autorenew(config: configuration.NamespaceConfig,
"expiry %s.", config_interval, "expiry %s.", config_interval,
notAfter.strftime("%Y-%m-%d %H:%M:%S %Z")) notAfter.strftime("%Y-%m-%d %H:%M:%S %Z"))
return True return True
# Only use the default if we don't have an ARI response # Only use the default if we don't have an ARI or renew_before_expiry value
elif renewal_time is None: elif renewal_time is None:
default_renewal_time = _default_renewal_time(cert_pem) default_renewal_time = _default_renewal_time(cert_pem)
if now > default_renewal_time: if now > default_renewal_time: