From af041f6b71f037fe504db6d27e3050bfc804cd79 Mon Sep 17 00:00:00 2001 From: James Stott <158563996+jamesstottmoj@users.noreply.github.com> Date: Mon, 11 Nov 2024 14:48:05 +0000 Subject: [PATCH] Fixes app not being deleted from control panel as github repo doesn't exist (#1383) * Ensure that app can still be deleted from control panel despite github repository not existing any more * Added tests * Renamed tests * Fixed test that wouldn't run via github * Update tests/api/cluster/test_app.py Co-authored-by: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> * Ran black --------- Co-authored-by: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> --- controlpanel/api/cluster.py | 27 +++++++++++++++++++-------- tests/api/cluster/test_app.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/controlpanel/api/cluster.py b/controlpanel/api/cluster.py index e7da77f73..5b4df3864 100644 --- a/controlpanel/api/cluster.py +++ b/controlpanel/api/cluster.py @@ -23,7 +23,7 @@ iam_arn, s3_arn, ) -from controlpanel.api.github import GithubAPI, extract_repo_info_from_url +from controlpanel.api.github import GithubAPI, RepositoryNotFound, extract_repo_info_from_url from controlpanel.api.kubernetes import KubernetesClient log = structlog.getLogger(__name__) @@ -519,7 +519,13 @@ def revoke_bucket_access(self, bucket_arn): def delete(self): self.aws_role_service.delete_role(self.iam_role_name) if self.github_api_token: - for env_name in self.get_deployment_envs(): + try: + deployment_envs = self.get_deployment_envs() + except requests.exceptions.HTTPError: + # if repo doesn't exist, assume dev and prod exist in Auth0 + deployment_envs = ["dev", "prod"] + + for env_name in deployment_envs: self.remove_auth_settings(env_name) def list_role_names(self): @@ -670,12 +676,17 @@ def create_auth_settings( return client, group def remove_auth_settings(self, env_name): - secrets_require_remove = [App.AUTH0_CLIENT_ID, App.AUTH0_CLIENT_SECRET] - for secret_name in secrets_require_remove: - self.delete_secret(env_name, secret_name) - envs_require_remove = [App.AUTH0_DOMAIN] - for app_env_name in envs_require_remove: - self.delete_env_var(env_name, app_env_name) + try: + secrets_require_remove = [App.AUTH0_CLIENT_ID, App.AUTH0_CLIENT_SECRET] + for secret_name in secrets_require_remove: + self.delete_secret(env_name, secret_name) + + envs_require_remove = [App.AUTH0_DOMAIN] + for app_env_name in envs_require_remove: + self.delete_env_var(env_name, app_env_name) + except RepositoryNotFound: + log.info("Repository not found. Skipping deletion of secrets and env vars") + self._get_auth0_instance().clear_up_app(self.app.get_auth_client(env_name)) self.app.clear_auth_settings(env_name) diff --git a/tests/api/cluster/test_app.py b/tests/api/cluster/test_app.py index 638153535..5f61543e2 100644 --- a/tests/api/cluster/test_app.py +++ b/tests/api/cluster/test_app.py @@ -4,10 +4,12 @@ # Third-party import pytest +import requests # First-party/Local from controlpanel.api import cluster, models from controlpanel.api.cluster import BASE_ASSUME_ROLE_POLICY +from controlpanel.api.github import RepositoryNotFound @pytest.fixture @@ -206,6 +208,38 @@ def test_get_github_key_display_name(key, expected): assert cluster.App(None).get_github_key_display_name(key) == expected +@patch("controlpanel.api.cluster.App.remove_auth_settings") +@patch("controlpanel.api.cluster.App.get_deployment_envs") +def test_delete_http_error(get_deployment_envs_mock, remove_auth_settings_mock, app): + app_cluster = cluster.App(app, github_api_token="testing") + get_deployment_envs_mock.side_effect = requests.exceptions.HTTPError() + + app_cluster.delete() + remove_auth_settings_mock.assert_has_calls( + [ + call("dev"), + call("prod"), + ] + ) + + +@patch("controlpanel.api.cluster.App._get_auth0_instance") +@patch("controlpanel.api.cluster.App.delete_env_var") +@patch("controlpanel.api.cluster.App.delete_secret") +def test_remove_auth_settings_repo_error( + delete_secret_mock, delete_env_var_mock, get_auth0_mock, app +): + auth0_instance = MagicMock() + app_cluster = cluster.App(app) + delete_secret_mock.side_effect = RepositoryNotFound("test") + get_auth0_mock.return_value = auth0_instance + + app_cluster.remove_auth_settings(env_name="dev") + delete_secret_mock.assert_called_once() + delete_env_var_mock.assert_not_called() + auth0_instance.clear_up_app.assert_called_once() + + # TODO can this be removed? mock_ingress = MagicMock(name="Ingress") mock_ingress.spec.rules = [MagicMock(name="Rule", host="test-app.example.com")]