From 8e8a89bdfb6b5eeea6d5f0c7be4175862088f056 Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Mon, 2 Nov 2020 16:17:11 -0800 Subject: [PATCH 1/6] Refactor notification PUT to expect add/remove sets instead of full certificate set --- lemur/notifications/messaging.py | 3 ++- lemur/notifications/schemas.py | 2 ++ lemur/notifications/service.py | 14 +++++++++++-- lemur/notifications/views.py | 3 +++ .../app/angular/notifications/services.js | 20 ++++++++++++++++++- 5 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lemur/notifications/messaging.py b/lemur/notifications/messaging.py index 3928689e..35cb1806 100644 --- a/lemur/notifications/messaging.py +++ b/lemur/notifications/messaging.py @@ -105,6 +105,7 @@ def send_plugin_notification(event_type, data, recipients, notification): "message": f"Sending expiration notification for to recipients {recipients}", "notification_type": "expiration", "certificate_targets": recipients, + "plugin": notification.plugin.slug, } status = FAILURE_METRIC_STATUS try: @@ -120,7 +121,7 @@ def send_plugin_notification(event_type, data, recipients, notification): "notification", "counter", 1, - metric_tags={"status": status, "event_type": event_type}, + metric_tags={"status": status, "event_type": event_type, "plugin": notification.plugin.slug}, ) if status == SUCCESS_METRIC_STATUS: diff --git a/lemur/notifications/schemas.py b/lemur/notifications/schemas.py index a3ff4c99..6ef5c506 100644 --- a/lemur/notifications/schemas.py +++ b/lemur/notifications/schemas.py @@ -21,6 +21,8 @@ class NotificationInputSchema(LemurInputSchema): active = fields.Boolean() plugin = fields.Nested(PluginInputSchema, required=True) certificates = fields.Nested(AssociatedCertificateSchema, many=True, missing=[]) + added_certificates = fields.Nested(AssociatedCertificateSchema, many=True, missing=[]) + removed_certificates = fields.Nested(AssociatedCertificateSchema, many=True, missing=[]) class NotificationOutputSchema(LemurOutputSchema): diff --git a/lemur/notifications/service.py b/lemur/notifications/service.py index 34edccc0..84dbef6b 100644 --- a/lemur/notifications/service.py +++ b/lemur/notifications/service.py @@ -104,7 +104,7 @@ def create(label, plugin_name, options, description, certificates): return database.create(notification) -def update(notification_id, label, plugin_name, options, description, active, certificates): +def update(notification_id, label, plugin_name, options, description, active, certificates, added_certificates, removed_certificates): """ Updates an existing notification. @@ -115,6 +115,8 @@ def update(notification_id, label, plugin_name, options, description, active, ce :param description: :param active: :param certificates: + :param added_certificates: + :param removed_certificates: :rtype : Notification :return: """ @@ -125,7 +127,15 @@ def update(notification_id, label, plugin_name, options, description, active, ce notification.options = options notification.description = description notification.active = active - notification.certificates = certificates + current_app.logger.info(f"Initial: {notification.certificates}") + current_app.logger.info(f"Adding: {added_certificates}") + current_app.logger.info(f"Removing: {removed_certificates}") + if certificates: + notification.certificates = certificates + else: + notification.certificates = notification.certificates + added_certificates + notification.certificates = [c for c in notification.certificates if c not in removed_certificates] + current_app.logger.info(f"Final: {notification.certificates}") return database.update(notification) diff --git a/lemur/notifications/views.py b/lemur/notifications/views.py index f6eef655..3f19f5df 100644 --- a/lemur/notifications/views.py +++ b/lemur/notifications/views.py @@ -337,6 +337,7 @@ class Notifications(AuthenticatedResource): :reqheader Authorization: OAuth token to authenticate :statuscode 200: no error """ + print(f"Updating with data: {data}") return service.update( notification_id, data["label"], @@ -345,6 +346,8 @@ class Notifications(AuthenticatedResource): data["description"], data["active"], data["certificates"], + data["added_certificates"], + data["removed_certificates"], ) def delete(self, notification_id): diff --git a/lemur/static/app/angular/notifications/services.js b/lemur/static/app/angular/notifications/services.js index 9e8c9b33..e1a645db 100644 --- a/lemur/static/app/angular/notifications/services.js +++ b/lemur/static/app/angular/notifications/services.js @@ -8,10 +8,27 @@ angular.module('lemur') if (this.certificates === undefined) { this.certificates = []; } + if (this.addedCertificates === undefined) { + this.addedCertificates = []; + } this.certificates.push(certificate); + this.addedCertificates.push(certificate); + if (this.removedCertificates !== undefined) { + const index = this.removedCertificates.indexOf(certificate); + if (index > -1) { + this.removedCertificates.splice(index, 1); + } + } }, removeCertificate: function (index) { - this.certificates.splice(index, 1); + if (this.removedCertificates === undefined) { + this.removedCertificates = []; + } + const removedCert = this.certificates.splice(index, 1); + this.removedCertificates.push(removedCert); + if (this.addedCertificates !== undefined && this.addedCertificates.indexOf(removedCert) > -1) { + this.addedCertificates.splice(this.addedCertificates.indexOf(removedCert), 1); + } } }); }); @@ -52,6 +69,7 @@ angular.module('lemur') }; NotificationService.update = function (notification) { + this.certificates = []; return notification.put(); }; From 8659504a8ba6e1d57f916b1c27d90348a77eb75e Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Mon, 2 Nov 2020 16:19:30 -0800 Subject: [PATCH 2/6] Remove debug logs --- lemur/notifications/service.py | 4 ---- lemur/notifications/views.py | 1 - 2 files changed, 5 deletions(-) diff --git a/lemur/notifications/service.py b/lemur/notifications/service.py index 84dbef6b..5ec6f045 100644 --- a/lemur/notifications/service.py +++ b/lemur/notifications/service.py @@ -127,15 +127,11 @@ def update(notification_id, label, plugin_name, options, description, active, ce notification.options = options notification.description = description notification.active = active - current_app.logger.info(f"Initial: {notification.certificates}") - current_app.logger.info(f"Adding: {added_certificates}") - current_app.logger.info(f"Removing: {removed_certificates}") if certificates: notification.certificates = certificates else: notification.certificates = notification.certificates + added_certificates notification.certificates = [c for c in notification.certificates if c not in removed_certificates] - current_app.logger.info(f"Final: {notification.certificates}") return database.update(notification) diff --git a/lemur/notifications/views.py b/lemur/notifications/views.py index 3f19f5df..c120982c 100644 --- a/lemur/notifications/views.py +++ b/lemur/notifications/views.py @@ -337,7 +337,6 @@ class Notifications(AuthenticatedResource): :reqheader Authorization: OAuth token to authenticate :statuscode 200: no error """ - print(f"Updating with data: {data}") return service.update( notification_id, data["label"], From 1918b911b3e798d0d48cf9902c0eb92b742c7b46 Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Thu, 18 Feb 2021 14:28:15 -0800 Subject: [PATCH 3/6] Make code more parallel --- lemur/static/app/angular/notifications/services.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lemur/static/app/angular/notifications/services.js b/lemur/static/app/angular/notifications/services.js index e1a645db..2259d6b3 100644 --- a/lemur/static/app/angular/notifications/services.js +++ b/lemur/static/app/angular/notifications/services.js @@ -14,9 +14,9 @@ angular.module('lemur') this.certificates.push(certificate); this.addedCertificates.push(certificate); if (this.removedCertificates !== undefined) { - const index = this.removedCertificates.indexOf(certificate); - if (index > -1) { - this.removedCertificates.splice(index, 1); + const removedIndex = this.removedCertificates.indexOf(certificate); + if (removedIndex > -1) { + this.removedCertificates.splice(removedIndex, 1); } } }, @@ -26,8 +26,11 @@ angular.module('lemur') } const removedCert = this.certificates.splice(index, 1); this.removedCertificates.push(removedCert); - if (this.addedCertificates !== undefined && this.addedCertificates.indexOf(removedCert) > -1) { - this.addedCertificates.splice(this.addedCertificates.indexOf(removedCert), 1); + if (this.addedCertificates !== undefined) { + const addedIndex = this.addedCertificates.indexOf(removedCert); + if (addedIndex > -1) { + this.addedCertificates.splice(addedIndex, 1); + } } } }); From 85b053ed9877d68907e693b823d5fd70b65833d1 Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Thu, 18 Feb 2021 14:35:51 -0800 Subject: [PATCH 4/6] Ignore submitted certificates --- lemur/notifications/schemas.py | 1 - lemur/notifications/service.py | 10 +++---- lemur/notifications/views.py | 48 +++++++++++++++++++++++++--------- 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/lemur/notifications/schemas.py b/lemur/notifications/schemas.py index 6ef5c506..d69da14d 100644 --- a/lemur/notifications/schemas.py +++ b/lemur/notifications/schemas.py @@ -20,7 +20,6 @@ class NotificationInputSchema(LemurInputSchema): description = fields.String() active = fields.Boolean() plugin = fields.Nested(PluginInputSchema, required=True) - certificates = fields.Nested(AssociatedCertificateSchema, many=True, missing=[]) added_certificates = fields.Nested(AssociatedCertificateSchema, many=True, missing=[]) removed_certificates = fields.Nested(AssociatedCertificateSchema, many=True, missing=[]) diff --git a/lemur/notifications/service.py b/lemur/notifications/service.py index 2e4566eb..372c1843 100644 --- a/lemur/notifications/service.py +++ b/lemur/notifications/service.py @@ -104,7 +104,7 @@ def create(label, plugin_name, options, description, certificates): return database.create(notification) -def update(notification_id, label, plugin_name, options, description, active, certificates, added_certificates, removed_certificates): +def update(notification_id, label, plugin_name, options, description, active, added_certificates, removed_certificates): """ Updates an existing notification. @@ -114,7 +114,6 @@ def update(notification_id, label, plugin_name, options, description, active, ce :param options: :param description: :param active: - :param certificates: :param added_certificates: :param removed_certificates: :rtype: Notification @@ -127,11 +126,8 @@ def update(notification_id, label, plugin_name, options, description, active, ce notification.options = options notification.description = description notification.active = active - if certificates: - notification.certificates = certificates - else: - notification.certificates = notification.certificates + added_certificates - notification.certificates = [c for c in notification.certificates if c not in removed_certificates] + notification.certificates = notification.certificates + added_certificates + notification.certificates = [c for c in notification.certificates if c not in removed_certificates] return database.update(notification) diff --git a/lemur/notifications/views.py b/lemur/notifications/views.py index fc7be4e7..b1200091 100644 --- a/lemur/notifications/views.py +++ b/lemur/notifications/views.py @@ -117,7 +117,7 @@ class NotificationsList(AuthenticatedResource): """ .. http:post:: /notifications - Creates a new account + Creates a new notification **Example request**: @@ -214,9 +214,12 @@ class NotificationsList(AuthenticatedResource): "id": 2 } - :arg accountNumber: aws account number - :arg label: human readable account label - :arg comments: some description about the account + :label label: notification name + :label slug: notification plugin slug + :label plugin_options: notification plugin options + :label description: notification description + :label active: whether or not the notification is active/enabled + :label certificates: certificates to attach to notification :reqheader Authorization: OAuth token to authenticate :statuscode 200: no error """ @@ -306,17 +309,29 @@ class Notifications(AuthenticatedResource): """ .. http:put:: /notifications/1 - Updates an account + Updates a notification **Example request**: .. sourcecode:: http - POST /notifications/1 HTTP/1.1 + PUT /notifications/1 HTTP/1.1 Host: example.com Accept: application/json, text/javascript Content-Type: application/json;charset=UTF-8 + { + "label": "labelChanged", + "plugin": { + "slug": "email-notification", + "plugin_options": "???" + }, + "description": "Sample notification", + "active": "true", + "added_certificates": "???", + "removed_certificates": "???" + } + **Example response**: @@ -328,14 +343,24 @@ class Notifications(AuthenticatedResource): { "id": 1, - "accountNumber": 11111111111, "label": "labelChanged", - "comments": "this is a thing" + "plugin": { + "slug": "email-notification", + "plugin_options": "???" + }, + "description": "Sample notification", + "active": "true", + "added_certificates": "???", + "removed_certificates": "???" } - :arg accountNumber: aws account number - :arg label: human readable account label - :arg comments: some description about the account + :label label: notification name + :label slug: notification plugin slug + :label plugin_options: notification plugin options + :label description: notification description + :label active: whether or not the notification is active/enabled + :label added_certificates: certificates to add + :label removed_certificates: certificates to remove :reqheader Authorization: OAuth token to authenticate :statuscode 200: no error """ @@ -346,7 +371,6 @@ class Notifications(AuthenticatedResource): data["plugin"]["plugin_options"], data["description"], data["active"], - data["certificates"], data["added_certificates"], data["removed_certificates"], ) From b3d0b7ce1b5d192ab61930d2c7aa157f2a612a38 Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Fri, 19 Feb 2021 18:25:05 -0800 Subject: [PATCH 5/6] Fix issue with repeatedly adding and removing --- lemur/notifications/schemas.py | 1 + lemur/notifications/views.py | 2 +- .../app/angular/notifications/services.js | 27 ++++++++++++------- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/lemur/notifications/schemas.py b/lemur/notifications/schemas.py index d69da14d..6ef5c506 100644 --- a/lemur/notifications/schemas.py +++ b/lemur/notifications/schemas.py @@ -20,6 +20,7 @@ class NotificationInputSchema(LemurInputSchema): description = fields.String() active = fields.Boolean() plugin = fields.Nested(PluginInputSchema, required=True) + certificates = fields.Nested(AssociatedCertificateSchema, many=True, missing=[]) added_certificates = fields.Nested(AssociatedCertificateSchema, many=True, missing=[]) removed_certificates = fields.Nested(AssociatedCertificateSchema, many=True, missing=[]) diff --git a/lemur/notifications/views.py b/lemur/notifications/views.py index b1200091..8ac8e06f 100644 --- a/lemur/notifications/views.py +++ b/lemur/notifications/views.py @@ -242,7 +242,7 @@ class Notifications(AuthenticatedResource): """ .. http:get:: /notifications/1 - Get a specific account + Get a specific notification **Example request**: diff --git a/lemur/static/app/angular/notifications/services.js b/lemur/static/app/angular/notifications/services.js index 2259d6b3..6bb36e65 100644 --- a/lemur/static/app/angular/notifications/services.js +++ b/lemur/static/app/angular/notifications/services.js @@ -11,26 +11,31 @@ angular.module('lemur') if (this.addedCertificates === undefined) { this.addedCertificates = []; } + if (_.some(this.addedCertificates, function (cert) { + return cert.id === certificate.id; + })) { + return; + } this.certificates.push(certificate); this.addedCertificates.push(certificate); if (this.removedCertificates !== undefined) { - const removedIndex = this.removedCertificates.indexOf(certificate); - if (removedIndex > -1) { - this.removedCertificates.splice(removedIndex, 1); - } + const indexInRemovedList = _.findIndex(this.removedCertificates, function (cert) { + return cert.id === certificate.id; + }); + this.removedCertificates.splice(indexInRemovedList, 1); } }, removeCertificate: function (index) { if (this.removedCertificates === undefined) { this.removedCertificates = []; } - const removedCert = this.certificates.splice(index, 1); + const removedCert = this.certificates.splice(index, 1)[0]; this.removedCertificates.push(removedCert); if (this.addedCertificates !== undefined) { - const addedIndex = this.addedCertificates.indexOf(removedCert); - if (addedIndex > -1) { - this.addedCertificates.splice(addedIndex, 1); - } + const indexInAddedList = _.findIndex(this.addedCertificates, function (cert) { + return cert.id === removedCert.id; + }); + this.addedCertificates.splice(indexInAddedList, 1); } } }); @@ -72,7 +77,9 @@ angular.module('lemur') }; NotificationService.update = function (notification) { - this.certificates = []; + // this.certificates = []; + // this.removedCertificates = []; + // this.addedCertificates = []; return notification.put(); }; From 811ac1a970e8d09932ee6e3ccb3800bc36664506 Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Fri, 19 Feb 2021 18:27:29 -0800 Subject: [PATCH 6/6] Remove stray comments --- lemur/static/app/angular/notifications/services.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lemur/static/app/angular/notifications/services.js b/lemur/static/app/angular/notifications/services.js index 6bb36e65..9c484277 100644 --- a/lemur/static/app/angular/notifications/services.js +++ b/lemur/static/app/angular/notifications/services.js @@ -77,9 +77,6 @@ angular.module('lemur') }; NotificationService.update = function (notification) { - // this.certificates = []; - // this.removedCertificates = []; - // this.addedCertificates = []; return notification.put(); };