From da98d8c1cf492fb0c0e1ad91490eac63f8d6df99 Mon Sep 17 00:00:00 2001 From: Radu Woinaroski <5281987+RaduW@users.noreply.github.com> Date: Mon, 27 Feb 2023 15:08:32 +0100 Subject: [PATCH 1/5] Add admin relay project config view --- .../api/endpoints/admin_project_configs.py | 42 +++++++ src/sentry/api/urls.py | 6 + .../endpoints/test_admin_project_configs.py | 106 ++++++++++++++++++ 3 files changed, 154 insertions(+) create mode 100644 src/sentry/api/endpoints/admin_project_configs.py create mode 100644 tests/sentry/api/endpoints/test_admin_project_configs.py diff --git a/src/sentry/api/endpoints/admin_project_configs.py b/src/sentry/api/endpoints/admin_project_configs.py new file mode 100644 index 00000000000000..c0092275d18311 --- /dev/null +++ b/src/sentry/api/endpoints/admin_project_configs.py @@ -0,0 +1,42 @@ +from django.http import Http404 +from rest_framework.request import Request +from rest_framework.response import Response + +from sentry.api.base import Endpoint, pending_silo_endpoint +from sentry.api.permissions import SuperuserPermission +from sentry.models import Project +from sentry.relay import projectconfig_cache + + +@pending_silo_endpoint +class AdminRelayProjectConfigsEndpoint(Endpoint): + permission_classes = (SuperuserPermission,) + + def get(self, request: Request) -> Response: + project_id = request.GET.get("project-id") + + project_keys = [] + if project_id is not None: + try: + project = Project.objects.get_from_cache(id=project_id) + for project_key in project.key_set.all(): + project_keys.append(project_key.public_key) + + except Exception: + raise Http404 + + project_key = request.GET.get("project-key") + if project_key is not None: + project_keys.append(project_key) + + configs = {} + for key in project_keys: + cached_config = projectconfig_cache.get(key) + if cached_config is not None: + configs[key] = cached_config + else: + configs[key] = "EMPTY" + + # TODO if we don't think we'll add anything to the endpoint + # we may as well return just the configs + return Response({"configs": configs}, status=200) diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index 73b870bc2b0bb4..0d54ba8eaceeb5 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -69,6 +69,7 @@ from .endpoints.accept_organization_invite import AcceptOrganizationInvite from .endpoints.accept_project_transfer import AcceptProjectTransferEndpoint +from .endpoints.admin_project_configs import AdminRelayProjectConfigsEndpoint from .endpoints.api_application_details import ApiApplicationDetailsEndpoint from .endpoints.api_applications import ApiApplicationsEndpoint from .endpoints.api_authorizations import ApiAuthorizationsEndpoint @@ -2485,6 +2486,11 @@ url(r"^packages/$", InternalPackagesEndpoint.as_view()), url(r"^environment/$", InternalEnvironmentEndpoint.as_view()), url(r"^mail/$", InternalMailEndpoint.as_view()), + url( + r"^project-config/$", + AdminRelayProjectConfigsEndpoint.as_view(), + name="sentry-api-0-internal-project-config", + ), ] ), ), diff --git a/tests/sentry/api/endpoints/test_admin_project_configs.py b/tests/sentry/api/endpoints/test_admin_project_configs.py new file mode 100644 index 00000000000000..456fe8f89d10b0 --- /dev/null +++ b/tests/sentry/api/endpoints/test_admin_project_configs.py @@ -0,0 +1,106 @@ +from django.urls import reverse +from rest_framework import status + +from sentry.relay import projectconfig_cache +from sentry.testutils import APITestCase +from sentry.testutils.silo import no_silo_test + + +@no_silo_test +class AcceptTransferProjectTest(APITestCase): + def setUp(self): + super().setUp() + self.owner = self.create_user( + email="example@example.com", is_superuser=False, is_staff=True, is_active=True + ) + self.org = self.create_organization(owner=self.owner) + self.first_team = self.create_team(organization=self.org) + self.proj1 = self.create_project( + name="proj1", organization=self.org, teams=[self.first_team] + ) + self.proj2 = self.create_project( + name="proj2", organization=self.org, teams=[self.first_team] + ) + self.superuser = self.create_user( + "superuser@example.com", is_superuser=True, is_staff=True, is_active=True + ) + self.path = "sentry-api-0-internal-project-config" + + self.p1_pk = self.create_project_key(self.proj1) + self.p2_pk = self.create_project_key(self.proj2) + + projectconfig_cache.set_many( + { + self.p1_pk.public_key: "proj1 config", + } + ) + + def get_url(self, proj_id=None, key=None): + ret_val = reverse(self.path) + ret_val += "?" + if proj_id: + ret_val += f"project-id={proj_id}" + if proj_id and key: + ret_val += "&" + if key: + ret_val += f"project-key={key}" + + return ret_val + + def test_normal_users_do_not_have_access(self): + """ + Request denied for non super-users + """ + self.login_as(self.user) + url = self.get_url(proj_id=self.proj1.id) + response = self.client.get(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_retrieving_project_configs(self): + """ + Asking for a project will return all project configs from all public + keys in redis + """ + self.login_as(self.superuser, superuser=True) + url = self.get_url(proj_id=self.proj1.id) + response = self.client.get(url) + assert response.status_code == status.HTTP_200_OK + + expected = {"configs": {self.p1_pk.public_key: "proj1 config"}} + actual = response.json() + assert actual == expected + + def test_retrieving_public_key_configs(self): + """ + Asking for a particular public key will return only the project config + for that public key + """ + self.login_as(self.superuser, superuser=True) + url = self.get_url(key=self.p1_pk.public_key) + response = self.client.get(url) + assert response.status_code == status.HTTP_200_OK + + expected = {"configs": {self.p1_pk.public_key: "proj1 config"}} + actual = response.json() + assert actual == expected + + def test_uncached_project(self): + """ + Asking for a project that was not cached in redis will return + an empty marker + """ + expected = {"configs": {self.p2_pk.public_key: "EMPTY"}} + + self.login_as(self.superuser, superuser=True) + url = self.get_url(proj_id=self.proj2.id) + response = self.client.get(url) + assert response.status_code == status.HTTP_200_OK + + actual = response.json() + assert actual == expected + url = self.get_url(key=self.p2_pk.public_key) + response = self.client.get(url) + assert response.status_code == status.HTTP_200_OK + + actual = response.json() + assert actual == expected From 98cde5b8967a641bbbcc70e188c55bd554423501 Mon Sep 17 00:00:00 2001 From: Radu Woinaroski <5281987+RaduW@users.noreply.github.com> Date: Mon, 27 Feb 2023 17:30:18 +0100 Subject: [PATCH 2/5] Mark api as private --- src/sentry/api/endpoints/admin_project_configs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sentry/api/endpoints/admin_project_configs.py b/src/sentry/api/endpoints/admin_project_configs.py index c0092275d18311..cb547991865513 100644 --- a/src/sentry/api/endpoints/admin_project_configs.py +++ b/src/sentry/api/endpoints/admin_project_configs.py @@ -10,6 +10,7 @@ @pending_silo_endpoint class AdminRelayProjectConfigsEndpoint(Endpoint): + private = True permission_classes = (SuperuserPermission,) def get(self, request: Request) -> Response: From e5bb38d58536eb65ef37fd114b44db8ef30b42d9 Mon Sep 17 00:00:00 2001 From: Radu Woinaroski <5281987+RaduW@users.noreply.github.com> Date: Mon, 27 Feb 2023 17:36:26 +0100 Subject: [PATCH 3/5] code review feedback, Fixed naming --- src/sentry/api/endpoints/admin_project_configs.py | 4 ++-- tests/sentry/api/endpoints/test_admin_project_configs.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/sentry/api/endpoints/admin_project_configs.py b/src/sentry/api/endpoints/admin_project_configs.py index cb547991865513..1d82eda53d171d 100644 --- a/src/sentry/api/endpoints/admin_project_configs.py +++ b/src/sentry/api/endpoints/admin_project_configs.py @@ -14,7 +14,7 @@ class AdminRelayProjectConfigsEndpoint(Endpoint): permission_classes = (SuperuserPermission,) def get(self, request: Request) -> Response: - project_id = request.GET.get("project-id") + project_id = request.GET.get("projectId") project_keys = [] if project_id is not None: @@ -26,7 +26,7 @@ def get(self, request: Request) -> Response: except Exception: raise Http404 - project_key = request.GET.get("project-key") + project_key = request.GET.get("projectKey") if project_key is not None: project_keys.append(project_key) diff --git a/tests/sentry/api/endpoints/test_admin_project_configs.py b/tests/sentry/api/endpoints/test_admin_project_configs.py index 456fe8f89d10b0..1dd7d34d6c144d 100644 --- a/tests/sentry/api/endpoints/test_admin_project_configs.py +++ b/tests/sentry/api/endpoints/test_admin_project_configs.py @@ -7,7 +7,7 @@ @no_silo_test -class AcceptTransferProjectTest(APITestCase): +class AdminRelayProjectConfigsEndpointTest(APITestCase): def setUp(self): super().setUp() self.owner = self.create_user( @@ -39,11 +39,11 @@ def get_url(self, proj_id=None, key=None): ret_val = reverse(self.path) ret_val += "?" if proj_id: - ret_val += f"project-id={proj_id}" + ret_val += f"projectId={proj_id}" if proj_id and key: ret_val += "&" if key: - ret_val += f"project-key={key}" + ret_val += f"projectKey={key}" return ret_val From 6d844d43dff9d0305cb02271d88df0b6db820bd4 Mon Sep 17 00:00:00 2001 From: Radu Woinaroski <5281987+RaduW@users.noreply.github.com> Date: Tue, 28 Feb 2023 09:58:27 +0100 Subject: [PATCH 4/5] added unit tests for missing project and project_key --- .../endpoints/test_admin_project_configs.py | 36 +++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/tests/sentry/api/endpoints/test_admin_project_configs.py b/tests/sentry/api/endpoints/test_admin_project_configs.py index 1dd7d34d6c144d..63a33da683d59e 100644 --- a/tests/sentry/api/endpoints/test_admin_project_configs.py +++ b/tests/sentry/api/endpoints/test_admin_project_configs.py @@ -52,8 +52,10 @@ def test_normal_users_do_not_have_access(self): Request denied for non super-users """ self.login_as(self.user) + url = self.get_url(proj_id=self.proj1.id) response = self.client.get(url) + assert response.status_code == status.HTTP_403_FORBIDDEN def test_retrieving_project_configs(self): @@ -62,10 +64,11 @@ def test_retrieving_project_configs(self): keys in redis """ self.login_as(self.superuser, superuser=True) + url = self.get_url(proj_id=self.proj1.id) response = self.client.get(url) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_200_OK expected = {"configs": {self.p1_pk.public_key: "proj1 config"}} actual = response.json() assert actual == expected @@ -76,10 +79,11 @@ def test_retrieving_public_key_configs(self): for that public key """ self.login_as(self.superuser, superuser=True) + url = self.get_url(key=self.p1_pk.public_key) response = self.client.get(url) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_200_OK expected = {"configs": {self.p1_pk.public_key: "proj1 config"}} actual = response.json() assert actual == expected @@ -92,15 +96,41 @@ def test_uncached_project(self): expected = {"configs": {self.p2_pk.public_key: "EMPTY"}} self.login_as(self.superuser, superuser=True) + url = self.get_url(proj_id=self.proj2.id) response = self.client.get(url) assert response.status_code == status.HTTP_200_OK - actual = response.json() assert actual == expected + url = self.get_url(key=self.p2_pk.public_key) response = self.client.get(url) assert response.status_code == status.HTTP_200_OK + actual = response.json() + assert actual == expected + def test_inexistent_project(self): + """ + Asking for an inexitent project will return 404 + """ + inexistent_project_id = 2 ^ 32 + self.login_as(self.superuser, superuser=True) + + url = self.get_url(proj_id=inexistent_project_id) + response = self.client.get(url) + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_inexistent_key(self): + """ + Asking for an inexistent project key will return an empty result + """ + inexsitent_key = 123 + self.login_as(self.superuser, superuser=True) + + url = self.get_url(key=inexsitent_key) + response = self.client.get(url) + + assert response.status_code == status.HTTP_200_OK + expected = {"configs": {str(inexsitent_key): "EMPTY"}} actual = response.json() assert actual == expected From 0107de81c3746b451f173032ad1ed74d4a6bb577 Mon Sep 17 00:00:00 2001 From: Radu Woinaroski <5281987+RaduW@users.noreply.github.com> Date: Tue, 28 Feb 2023 10:55:37 +0100 Subject: [PATCH 5/5] fixes from code review --- .../api/endpoints/admin_project_configs.py | 2 +- .../endpoints/test_admin_project_configs.py | 25 +++++++++++-------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/sentry/api/endpoints/admin_project_configs.py b/src/sentry/api/endpoints/admin_project_configs.py index 1d82eda53d171d..b2f57cda22dfaa 100644 --- a/src/sentry/api/endpoints/admin_project_configs.py +++ b/src/sentry/api/endpoints/admin_project_configs.py @@ -36,7 +36,7 @@ def get(self, request: Request) -> Response: if cached_config is not None: configs[key] = cached_config else: - configs[key] = "EMPTY" + configs[key] = None # TODO if we don't think we'll add anything to the endpoint # we may as well return just the configs diff --git a/tests/sentry/api/endpoints/test_admin_project_configs.py b/tests/sentry/api/endpoints/test_admin_project_configs.py index 63a33da683d59e..8a45fee22550dd 100644 --- a/tests/sentry/api/endpoints/test_admin_project_configs.py +++ b/tests/sentry/api/endpoints/test_admin_project_configs.py @@ -1,3 +1,5 @@ +from urllib import parse + from django.urls import reverse from rest_framework import status @@ -36,22 +38,23 @@ def setUp(self): ) def get_url(self, proj_id=None, key=None): - ret_val = reverse(self.path) - ret_val += "?" - if proj_id: - ret_val += f"projectId={proj_id}" - if proj_id and key: - ret_val += "&" - if key: - ret_val += f"projectKey={key}" + query = {} + if proj_id is not None: + query["projectId"] = proj_id + if key is not None: + query["projectKey"] = key + query_string = parse.urlencode(query) + + ret_val = reverse(self.path) + ret_val += f"?{query_string}" return ret_val def test_normal_users_do_not_have_access(self): """ Request denied for non super-users """ - self.login_as(self.user) + self.login_as(self.owner) url = self.get_url(proj_id=self.proj1.id) response = self.client.get(url) @@ -93,7 +96,7 @@ def test_uncached_project(self): Asking for a project that was not cached in redis will return an empty marker """ - expected = {"configs": {self.p2_pk.public_key: "EMPTY"}} + expected = {"configs": {self.p2_pk.public_key: None}} self.login_as(self.superuser, superuser=True) @@ -131,6 +134,6 @@ def test_inexistent_key(self): response = self.client.get(url) assert response.status_code == status.HTTP_200_OK - expected = {"configs": {str(inexsitent_key): "EMPTY"}} + expected = {"configs": {str(inexsitent_key): None}} actual = response.json() assert actual == expected