From 512b1acfddbdc2558717e53791303651a63c4f8d Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Tue, 8 Dec 2020 18:29:48 -0800 Subject: [PATCH] PR feedback: use days threshold instead of interval set, etc. --- docs/administration.rst | 4 ++-- lemur/common/celery.py | 4 ++-- lemur/notifications/messaging.py | 6 +++--- lemur/tests/test_messaging.py | 7 +++++-- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/docs/administration.rst b/docs/administration.rst index fb576b9c..bd0b5f96 100644 --- a/docs/administration.rst +++ b/docs/administration.rst @@ -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. diff --git a/lemur/common/celery.py b/lemur/common/celery.py index 574d6ec5..578592dc 100644 --- a/lemur/common/celery.py +++ b/lemur/common/celery.py @@ -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) diff --git a/lemur/notifications/messaging.py b/lemur/notifications/messaging.py index 9a55ab3a..1d7bda4c 100644 --- a/lemur/notifications/messaging.py +++ b/lemur/notifications/messaging.py @@ -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 diff --git a/lemur/tests/test_messaging.py b/lemur/tests/test_messaging.py index c1c9b24a..d897f931 100644 --- a/lemur/tests/test_messaging.py +++ b/lemur/tests/test_messaging.py @@ -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(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)