PR feedback: use days threshold instead of interval set, etc.

This commit is contained in:
Jasmine Schladen 2020-12-08 18:29:48 -08:00
parent 15d43d6c27
commit 512b1acfdd
4 changed files with 12 additions and 9 deletions

View File

@ -362,8 +362,8 @@ disabled by default; to enable it, you must set the option ``--notify`` (when us
**Security certificate expiration summary**
If you enable the Celery or cron task to send this notification type, Lemur will send a summary of all
certificates with upcoming expiration date that matches one of the intervals configured in the
``LEMUR_EXPIRATION_SUMMARY_EMAIL_INTERVALS`` configuration parameter (with a fallback of 14 days).
certificates with upcoming expiration date that occurs within the number of days specified by the
``LEMUR_EXPIRATION_SUMMARY_EMAIL_THRESHOLD_DAYS`` configuration parameter (with a fallback of 14 days).
Note that certificates will be included in this summary even if they do not have any associated notifications.
This notification type also supports the same ``--exclude`` and ``EXCLUDE_CN_FROM_NOTIFICATION`` options as expiration emails.

View File

@ -860,7 +860,7 @@ def notify_authority_expirations():
@celery.task(soft_time_limit=3600)
def send_security_expiration_summary():
"""
This celery task sends a summary about expiring certificates to the security team. TODO document
This celery task sends a summary about expiring certificates to the security team.
:return:
"""
function = f"{__name__}.{sys._getframe().f_code.co_name}"
@ -881,7 +881,7 @@ def send_security_expiration_summary():
current_app.logger.debug(log_data)
try:
cli_notification.send_security_expiration_summary()
cli_notification.security_expiration_summary(current_app.config.get("EXCLUDE_CN_FROM_NOTIFICATION", []))
except SoftTimeLimitExceeded:
log_data["message"] = "Send summary for expiring certs Time limit exceeded."
current_app.logger.error(log_data)

View File

@ -70,8 +70,8 @@ def get_certificates_for_security_summary_email(exclude=None):
:return:
"""
now = arrow.utcnow()
expiration_summary_intervals = current_app.config.get("LEMUR_EXPIRATION_SUMMARY_EMAIL_INTERVALS", [14])
max_not_after = now + timedelta(days=max(expiration_summary_intervals) + 1)
threshold_days = current_app.config.get("LEMUR_EXPIRATION_SUMMARY_EMAIL_THRESHOLD_DAYS", 14)
max_not_after = now + timedelta(days=threshold_days + 1)
q = (
database.db.session.query(Certificate)
@ -91,7 +91,7 @@ def get_certificates_for_security_summary_email(exclude=None):
certs = []
for c in windowed_query(q, Certificate.id, 10000):
days_remaining = (c.not_after - now).days
if days_remaining in expiration_summary_intervals:
if days_remaining <= threshold_days:
certs.append(c)
return certs

View File

@ -119,8 +119,11 @@ def test_send_expiration_summary_notification(certificate, notification, notific
# we don't actually test the email contents, but adding an assortment of certs here is useful for step debugging
# to confirm the produced email body looks like we expect
for i in range(1, 6):
create_cert_that_expires_in_days(14)
create_cert_that_expires_in_days(12)
create_cert_that_expires_in_days(9)
create_cert_that_expires_in_days(7)
create_cert_that_expires_in_days(7)
create_cert_that_expires_in_days(2)
create_cert_that_expires_in_days(30)
create_cert_that_expires_in_days(15)