From 334c7bb5126431155a4162acbff3a562867d0099 Mon Sep 17 00:00:00 2001 From: Ihor Panasiuk Date: Wed, 18 Dec 2024 16:24:56 +0200 Subject: [PATCH 1/4] Add cleaning up ability for providers during deletion --- keep/providers/base/base_provider.py | 9 +++++++++ keep/providers/providers_service.py | 28 +++++++++++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/keep/providers/base/base_provider.py b/keep/providers/base/base_provider.py index 44e2ebe20..3a8e55e29 100644 --- a/keep/providers/base/base_provider.py +++ b/keep/providers/base/base_provider.py @@ -558,6 +558,15 @@ def setup_webhook( """ raise NotImplementedError("setup_webhook() method not implemented") + def clean_up(self): + """ + Clean up the provider. + + Raises:s + NotImplementedError: for providers who does not implement this method. + """ + raise NotImplementedError("clean_up() method not implemented") + @staticmethod def get_alert_schema() -> dict: """ diff --git a/keep/providers/providers_service.py b/keep/providers/providers_service.py index b58a2c0a2..a99a8cbdf 100644 --- a/keep/providers/providers_service.py +++ b/keep/providers/providers_service.py @@ -227,34 +227,48 @@ def update_provider( def delete_provider( tenant_id: str, provider_id: str, session: Session, allow_provisioned=False ): - provider = session.exec( + provider_model: Provider = session.exec( select(Provider).where( (Provider.tenant_id == tenant_id) & (Provider.id == provider_id) ) ).one_or_none() - if not provider: + if not provider_model: raise HTTPException(404, detail="Provider not found") - if provider.provisioned and not allow_provisioned: + if provider_model.provisioned and not allow_provisioned: raise HTTPException(403, detail="Cannot delete a provisioned provider") context_manager = ContextManager(tenant_id=tenant_id) secret_manager = SecretManagerFactory.get_secret_manager(context_manager) + config = secret_manager.read_secret(provider_model.configuration_key, is_json=True) + provider = ProvidersFactory.get_provider( + context_manager, provider_model.id, provider_model.type, config + ) try: - secret_manager.delete_secret(provider.configuration_key) + secret_manager.delete_secret(provider_model.configuration_key) except Exception: logger.exception("Failed to delete the provider secret") - if provider.consumer: + if provider_model.consumer: try: event_subscriber = EventSubscriber.get_instance() - event_subscriber.remove_consumer(provider) + event_subscriber.remove_consumer(provider_model) except Exception: logger.exception("Failed to unregister provider as a consumer") - session.delete(provider) + try: + provider.clean_up() + except NotImplementedError: + logger.info( + "Being deleted provider of type %s does not have a clean_up method", + provider_model.type + ) + except Exception: + logger.exception("Failed to clean up provider") + + session.delete(provider_model) session.commit() @staticmethod From a2bbdb51449450b56b4dd0d248a72c9ca83d1f51 Mon Sep 17 00:00:00 2001 From: Ihor Panasiuk Date: Wed, 18 Dec 2024 16:26:04 +0200 Subject: [PATCH 2/4] Implement cleaning up for PagerDuty provider --- keep/contextmanager/contextmanager.py | 8 +++++ .../pagerduty_provider/pagerduty_provider.py | 35 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/keep/contextmanager/contextmanager.py b/keep/contextmanager/contextmanager.py index 2dad2563b..9ec99ed9b 100644 --- a/keep/contextmanager/contextmanager.py +++ b/keep/contextmanager/contextmanager.py @@ -4,6 +4,7 @@ import click from pympler.asizeof import asizeof +from keep.api.core.config import config from keep.api.core.db import get_last_workflow_execution_by_workflow_id, get_session from keep.api.logging import WorkflowLoggerAdapter from keep.api.models.alert import AlertDto @@ -59,6 +60,13 @@ def __init__(self, tenant_id, workflow_id=None, workflow_execution_id=None): self._api_key = None self.__loggers = {} + @property + def api_url(self): + """ + The URL of the Keep API + """ + return config("KEEP_API_URL") + @property def api_key(self): # avoid circular import diff --git a/keep/providers/pagerduty_provider/pagerduty_provider.py b/keep/providers/pagerduty_provider/pagerduty_provider.py index d58ddff9a..c2a69b438 100644 --- a/keep/providers/pagerduty_provider/pagerduty_provider.py +++ b/keep/providers/pagerduty_provider/pagerduty_provider.py @@ -475,6 +475,41 @@ def _trigger_incident( ) # This will give us a better error message in Keep workflows raise Exception(r.text) from e + + def clean_up(self): + """ + Clean up the provider. + It will remove the webhook from PagerDuty if it exists. + """ + self.logger.info("Cleaning up %s provider with id %s", self.PROVIDER_DISPLAY_NAME, self.provider_id) + keep_webhook_incidents_api_url = ( + f"{self.context_manager.api_url}/incidents/event/{self.provider_type}?provider_id={self.provider_id}" + ) + headers = self.__get_headers() + request = requests.get(self.SUBSCRIPTION_API_URL, headers=headers) + if not request.ok: + raise Exception("Could not get existing webhooks") + existing_webhooks = request.json().get("webhook_subscriptions", []) + webhook_exists = next( + iter( + [ + webhook + for webhook in existing_webhooks + if keep_webhook_incidents_api_url == webhook.get("delivery_method", {}).get("url", "") + ] + ), + False, + ) + if webhook_exists: + self.logger.info("Webhook exists, removing it") + webhook_id = webhook_exists.get("id") + request = requests.delete( + f"{self.SUBSCRIPTION_API_URL}/{webhook_id}", headers=headers + ) + if not request.ok: + raise Exception("Could not remove existing webhook") + self.logger.info("Webhook removed", extra={"webhook_id": webhook_id}) + def dispose(self): """ From 3f422e3b583ca79a6a555172960f74257489273a Mon Sep 17 00:00:00 2001 From: Ihor Panasiuk Date: Wed, 18 Dec 2024 18:06:54 +0200 Subject: [PATCH 3/4] Log original exception when cleaning up fails --- keep/providers/providers_service.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/keep/providers/providers_service.py b/keep/providers/providers_service.py index a99a8cbdf..3cc2d6e38 100644 --- a/keep/providers/providers_service.py +++ b/keep/providers/providers_service.py @@ -265,8 +265,9 @@ def delete_provider( "Being deleted provider of type %s does not have a clean_up method", provider_model.type ) - except Exception: - logger.exception("Failed to clean up provider") + except Exception as exc: + logger.exception(msg="Failed to clean up provider", + extra={ "error_msg": str(exc) }) session.delete(provider_model) session.commit() From a18a75ab2bcb2d233e20e457d8d2d5133eb57b57 Mon Sep 17 00:00:00 2001 From: Ihor Panasiuk Date: Wed, 18 Dec 2024 18:16:57 +0200 Subject: [PATCH 4/4] fix --- keep/providers/providers_service.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/keep/providers/providers_service.py b/keep/providers/providers_service.py index 3cc2d6e38..bba824121 100644 --- a/keep/providers/providers_service.py +++ b/keep/providers/providers_service.py @@ -265,9 +265,8 @@ def delete_provider( "Being deleted provider of type %s does not have a clean_up method", provider_model.type ) - except Exception as exc: - logger.exception(msg="Failed to clean up provider", - extra={ "error_msg": str(exc) }) + except Exception: + logger.exception(msg="Failed to clean up provider") session.delete(provider_model) session.commit()