From 397f9614869468f25331c3e62ea2537ed1aeb804 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 17 May 2023 06:56:57 -0600 Subject: [PATCH] Fix organizations not being deleted by start_cleanup_deleted_organizations (#1950) Organizations that have been deleted outside OnCall were not being cleaned up by this task as expected. - Use PluginAuthToken instead of GCOM token == None to determine if the oncall organization should be matched in GCOM - Fix how delete was being checked for the instance, the previous method does not work. --- engine/apps/grafana_plugin/helpers/client.py | 5 +++-- engine/apps/user_management/sync.py | 9 ++++++--- engine/apps/user_management/tests/test_sync.py | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/engine/apps/grafana_plugin/helpers/client.py b/engine/apps/grafana_plugin/helpers/client.py index 592ccaed..628244da 100644 --- a/engine/apps/grafana_plugin/helpers/client.py +++ b/engine/apps/grafana_plugin/helpers/client.py @@ -247,8 +247,9 @@ class GcomAPIClient(APIClient): return self.api_get(query) def is_stack_deleted(self, stack_id: str) -> bool: - instance_info = self.get_instance_info(stack_id) - return instance_info and instance_info.get("status") == self.STACK_STATUS_DELETED + url = f"instances?includeDeleted=true&id={stack_id}" + instance_infos, _ = self.api_get(url) + return instance_infos["items"] and instance_infos["items"][0].get("status") == self.STACK_STATUS_DELETED def post_active_users(self, body): return self.api_post("app-active-users", body) diff --git a/engine/apps/user_management/sync.py b/engine/apps/user_management/sync.py index 18e694cd..8fe16747 100644 --- a/engine/apps/user_management/sync.py +++ b/engine/apps/user_management/sync.py @@ -1,6 +1,7 @@ import logging from celery.utils.log import get_task_logger +from django.apps import apps from django.conf import settings from django.utils import timezone @@ -126,12 +127,14 @@ def check_grafana_incident_is_enabled(client): def delete_organization_if_needed(organization): # Organization has a manually set API token, it will not be found within GCOM # and would need to be deleted manually. - if organization.gcom_token is None: - logger.info(f"Organization {organization.pk} has no gcom_token. Probably it's needed to delete org manually.") + PluginAuthToken = apps.get_model("auth_token", "PluginAuthToken") + manually_provisioned_token = PluginAuthToken.objects.filter(organization_id=organization.pk).first() + if manually_provisioned_token: + logger.info(f"Organization {organization.pk} has PluginAuthToken. Probably it's needed to delete org manually.") return False # Use common token as organization.gcom_token could be already revoked - client = GcomAPIClient(settings.GRAFANA_COM_API_TOKEN) + client = GcomAPIClient(settings.GRAFANA_COM_ADMIN_API_TOKEN) is_stack_deleted = client.is_stack_deleted(organization.stack_id) if not is_stack_deleted: return False diff --git a/engine/apps/user_management/tests/test_sync.py b/engine/apps/user_management/tests/test_sync.py index 1157cc74..4a1bf05f 100644 --- a/engine/apps/user_management/tests/test_sync.py +++ b/engine/apps/user_management/tests/test_sync.py @@ -335,7 +335,7 @@ def test_duplicate_user_ids(make_organization, make_user_for_organization): def test_cleanup_organization_deleted(make_organization): organization = make_organization(gcom_token="TEST_GCOM_TOKEN") - with patch.object(GcomAPIClient, "get_instance_info", return_value={"status": "deleted"}): + with patch.object(GcomAPIClient, "api_get", return_value=({"items": [{"status": "deleted"}]}, None)): cleanup_organization(organization.id) organization.refresh_from_db()