Browse Source

feat(remote-config): Update API to use new blueprint format (#71705)

Adds support for multiple key, value user-config definitions.

Related: https://github.com/getsentry/sentry/issues/70942
Colton Allen 9 months ago
parent
commit
3d002f2d1c

+ 3 - 3
src/sentry/api/urls.py

@@ -163,7 +163,7 @@ from sentry.monitors.endpoints.project_monitor_stats import ProjectMonitorStatsE
 from sentry.monitors.endpoints.project_processing_errors_details import (
     ProjectProcessingErrorsDetailsEndpoint,
 )
-from sentry.remote_config.endpoints import ProjectKeyConfigurationEndpoint
+from sentry.remote_config.endpoints import ProjectConfigurationEndpoint
 from sentry.replays.endpoints.organization_replay_count import OrganizationReplayCountEndpoint
 from sentry.replays.endpoints.organization_replay_details import OrganizationReplayDetailsEndpoint
 from sentry.replays.endpoints.organization_replay_events_meta import (
@@ -2364,8 +2364,8 @@ PROJECT_URLS: list[URLPattern | URLResolver] = [
         ProjectKeyStatsEndpoint.as_view(),
     ),
     re_path(
-        r"^(?P<organization_id_or_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/keys/(?P<key_id>[^\/]+)/configuration/$",
-        ProjectKeyConfigurationEndpoint.as_view(),
+        r"^(?P<organization_id_or_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/configuration/$",
+        ProjectConfigurationEndpoint.as_view(),
         name="sentry-api-0-project-key-configuration",
     ),
     re_path(

+ 23 - 28
src/sentry/remote_config/endpoints.py

@@ -8,25 +8,34 @@ from sentry.api.api_owners import ApiOwner
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.project import ProjectEndpoint, ProjectEventPermission
-from sentry.api.exceptions import ResourceDoesNotExist
 from sentry.models.project import Project
-from sentry.models.projectkey import ProjectKey
 from sentry.remote_config.storage import make_storage
 
 
-class ConfigurationValidator(Serializer):
-    id = serializers.UUIDField(read_only=True)
+class OptionsValidator(Serializer):
     sample_rate = serializers.FloatField(max_value=1.0, min_value=0, required=True)
     traces_sample_rate = serializers.FloatField(max_value=1.0, min_value=0, required=True)
-    user_config = serializers.JSONField(required=True, allow_null=True)
+
+
+class FeatureValidator(Serializer):
+    key = serializers.CharField(required=True)
+    value = serializers.JSONField(required=True, allow_null=True)
+
+
+class ConfigurationValidator(Serializer):
+    id = serializers.UUIDField(read_only=True)
+    features: serializers.ListSerializer = serializers.ListSerializer(
+        child=FeatureValidator(), required=True
+    )
+    options = OptionsValidator(required=True)
 
 
 class ConfigurationContainerValidator(Serializer):
-    data = ConfigurationValidator()  # type: ignore[assignment]
+    data = ConfigurationValidator(required=True)  # type: ignore[assignment]
 
 
 @region_silo_endpoint
-class ProjectKeyConfigurationEndpoint(ProjectEndpoint):
+class ProjectConfigurationEndpoint(ProjectEndpoint):
     owner = ApiOwner.REMOTE_CONFIG
     permission_classes = (ProjectEventPermission,)
     publish_status = {
@@ -35,31 +44,20 @@ class ProjectKeyConfigurationEndpoint(ProjectEndpoint):
         "DELETE": ApiPublishStatus.EXPERIMENTAL,
     }
 
-    def convert_args(self, request: Request, key_id: str, *args, **kwargs):
-        args, kwargs = super().convert_args(request, *args, **kwargs)
-        project = kwargs["project"]
-
-        try:
-            key = ProjectKey.objects.for_request(request).get(project=project, public_key=key_id)
-        except ProjectKey.DoesNotExist:
-            raise ResourceDoesNotExist
-
-        kwargs["key"] = key
-        return args, kwargs
-
-    def get(self, request: Request, project: Project, key: ProjectKey) -> Response:
+    def get(self, request: Request, project: Project) -> Response:
         """Get remote configuration from project options."""
         if not features.has(
             "organizations:remote-config", project.organization, actor=request.user
         ):
             return Response(status=404)
 
-        remote_config = make_storage(key).get()
+        remote_config = make_storage(project).get()
         if remote_config is None:
             return Response("Not found.", status=404)
+
         return Response({"data": remote_config}, status=200)
 
-    def post(self, request: Request, project: Project, key: ProjectKey) -> Response:
+    def post(self, request: Request, project: Project) -> Response:
         """Set remote configuration in project options."""
         if not features.has(
             "organizations:remote-config", project.organization, actor=request.user
@@ -72,18 +70,15 @@ class ProjectKeyConfigurationEndpoint(ProjectEndpoint):
 
         result = validator.validated_data["data"]
 
-        # Propagate config to Relay.
-        make_storage(key).set(result)
-
-        result["id"] = key.public_key
+        make_storage(project).set(result)
         return Response({"data": result}, status=201)
 
-    def delete(self, request: Request, project: Project, key: ProjectKey) -> Response:
+    def delete(self, request: Request, project: Project) -> Response:
         """Delete remote configuration from project options."""
         if not features.has(
             "organizations:remote-config", project.organization, actor=request.user
         ):
             return Response(status=404)
 
-        make_storage(key).pop()
+        make_storage(project).pop()
         return Response("", status=204)

+ 18 - 24
src/sentry/remote_config/storage.py

@@ -3,7 +3,7 @@ from typing import TypedDict
 
 from sentry import options
 from sentry.models.files.utils import get_storage
-from sentry.models.projectkey import ProjectKey
+from sentry.models.project import Project
 from sentry.utils import json
 
 JSONValue = str | int | float | bool | None | list["JSONValue"] | dict[str, "JSONValue"]
@@ -12,23 +12,26 @@ JSONValue = str | int | float | bool | None | list["JSONValue"] | dict[str, "JSO
 class Options(TypedDict):
     sample_rate: float
     traces_sample_rate: float
-    user_config: JSONValue
+
+
+class Feature(TypedDict):
+    key: str
+    value: JSONValue
 
 
 class StorageFormat(TypedDict):
+    features: list[Feature]
     options: Options
     version: int
 
 
 class APIFormat(TypedDict):
-    id: str
-    sample_rate: float
-    traces_sample_rate: float
-    user_config: JSONValue
+    features: list[Feature]
+    options: Options
 
 
 class StorageBackend:
-    def __init__(self, key: ProjectKey) -> None:
+    def __init__(self, key: Project) -> None:
         self.driver = BlobDriver(key)
         self.key = key
 
@@ -45,35 +48,26 @@ class StorageBackend:
         self.driver.pop()
 
     def _deserialize(self, result: StorageFormat) -> APIFormat:
-        assert self.key.public_key is not None
         return {
-            "id": self.key.public_key,
-            "sample_rate": result["options"]["sample_rate"],
-            "traces_sample_rate": result["options"]["traces_sample_rate"],
-            "user_config": result["options"]["user_config"],
+            "features": result["features"],
+            "options": result["options"],
         }
 
     def _serialize(self, result: APIFormat) -> StorageFormat:
         return {
-            "options": {
-                "sample_rate": result["sample_rate"],
-                "traces_sample_rate": result["traces_sample_rate"],
-                "user_config": result["user_config"],
-            },
+            "features": result["features"],
+            "options": result["options"],
             "version": 1,
         }
 
 
 class BlobDriver:
-    def __init__(self, project_key: ProjectKey) -> None:
-        self.project_key = project_key
+    def __init__(self, project: Project) -> None:
+        self.project = project
 
     @property
     def key(self):
-        assert self.project_key.public_key is not None
-        return (
-            f"configurations/{self.project_key.project_id}/{self.project_key.public_key}/production"
-        )
+        return f"configurations/{self.project.id}/production"
 
     @property
     def storage(self):
@@ -111,5 +105,5 @@ class BlobDriver:
             return None
 
 
-def make_storage(key: ProjectKey):
+def make_storage(key: Project):
     return StorageBackend(key)

+ 36 - 54
tests/sentry/remote_config/endpoints/test_configuration.py

@@ -14,22 +14,17 @@ class ConfiguratioAPITestCase(APITestCase):
     def setUp(self):
         super().setUp()
         self.login_as(self.user)
-        self.url = reverse(
-            self.endpoint,
-            args=(self.organization.slug, self.project.slug, self.projectkey.public_key),
-        )
+        self.url = reverse(self.endpoint, args=(self.organization.slug, self.project.slug))
 
     @property
     def storage(self):
-        return StorageBackend(self.projectkey)
+        return StorageBackend(self.project)
 
     def test_get_configuration(self):
         self.storage.set(
             {
-                "id": self.projectkey.public_key,
-                "sample_rate": 0.5,
-                "traces_sample_rate": 0,
-                "user_config": {"abc": "def"},
+                "features": [{"key": "abc", "value": "def"}],
+                "options": {"sample_rate": 0.5, "traces_sample_rate": 0},
             },
         )
 
@@ -39,26 +34,23 @@ class ConfiguratioAPITestCase(APITestCase):
         assert response.status_code == 200
         assert response.json() == {
             "data": {
-                "id": self.projectkey.public_key,
-                "sample_rate": 0.5,
-                "traces_sample_rate": 0,
-                "user_config": {"abc": "def"},
+                "features": [{"key": "abc", "value": "def"}],
+                "options": {"sample_rate": 0.5, "traces_sample_rate": 0},
             }
         }
 
     def test_get_configuration_not_enabled(self):
         self.storage.set(
             {
-                "id": self.projectkey.public_key,
-                "sample_rate": 0.5,
-                "traces_sample_rate": 0,
-                "user_config": {"abc": "def"},
+                "features": [{"key": "abc", "value": "def"}],
+                "options": {"sample_rate": 0.5, "traces_sample_rate": 0},
             },
         )
         response = self.client.get(self.url)
         assert response.status_code == 404
 
     def test_get_configuration_not_found(self):
+        self.storage.pop()  # Pop anything that might be in the cache.
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.get(self.url)
             assert response.status_code == 404
@@ -69,11 +61,8 @@ class ConfiguratioAPITestCase(APITestCase):
                 self.url,
                 data={
                     "data": {
-                        "sample_rate": 1.0,
-                        "traces_sample_rate": 0.2,
-                        "user_config": {
-                            "hello": "world",
-                        },
+                        "features": [{"key": "hello", "value": "world"}],
+                        "options": {"sample_rate": 1.0, "traces_sample_rate": 0.2},
                     }
                 },
                 format="json",
@@ -82,12 +71,8 @@ class ConfiguratioAPITestCase(APITestCase):
         assert response.status_code == 201, response.content
         assert response.json() == {
             "data": {
-                "id": self.projectkey.public_key,
-                "sample_rate": 1.0,
-                "traces_sample_rate": 0.2,
-                "user_config": {
-                    "hello": "world",
-                },
+                "features": [{"key": "hello", "value": "world"}],
+                "options": {"sample_rate": 1.0, "traces_sample_rate": 0.2},
             }
         }
 
@@ -99,58 +84,60 @@ class ConfiguratioAPITestCase(APITestCase):
         assert response.status_code == 404
 
     def test_post_configuration_different_types(self):
-        data: dict[str, Any] = {"data": {"sample_rate": 1.0, "traces_sample_rate": 0.2}}
+        data: dict[str, Any] = {
+            "data": {"options": {"sample_rate": 1.0, "traces_sample_rate": 0.2}}
+        }
 
         # Null type
-        data["data"]["user_config"] = None
+        data["data"]["features"] = [{"key": "abc", "value": None}]
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.post(self.url, data=data, format="json")
         assert response.status_code == 201, response.content
-        assert response.json()["data"]["user_config"] is None
+        assert response.json()["data"]["features"][0]["value"] is None
 
         # Bool types
-        data["data"]["user_config"] = False
+        data["data"]["features"] = [{"key": "abc", "value": False}]
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.post(self.url, data=data, format="json")
         assert response.status_code == 201, response.content
-        assert response.json()["data"]["user_config"] is False
+        assert response.json()["data"]["features"][0]["value"] is False
 
         # String types
-        data["data"]["user_config"] = "string"
+        data["data"]["features"] = [{"key": "abc", "value": "string"}]
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.post(self.url, data=data, format="json")
         assert response.status_code == 201, response.content
-        assert response.json()["data"]["user_config"] == "string"
+        assert response.json()["data"]["features"][0]["value"] == "string"
 
         # Integer types
-        data["data"]["user_config"] = 1
+        data["data"]["features"] = [{"key": "abc", "value": 1}]
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.post(self.url, data=data, format="json")
         assert response.status_code == 201, response.content
-        assert response.json()["data"]["user_config"] == 1
+        assert response.json()["data"]["features"][0]["value"] == 1
 
         # Float types
-        data["data"]["user_config"] = 1.0
+        data["data"]["features"] = [{"key": "abc", "value": 1.0}]
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.post(self.url, data=data, format="json")
         assert response.status_code == 201, response.content
-        assert response.json()["data"]["user_config"] == 1.0
+        assert response.json()["data"]["features"][0]["value"] == 1.0
 
         # Array types
-        data["data"]["user_config"] = ["a", "b"]
+        data["data"]["features"] = [{"key": "abc", "value": ["a", "b"]}]
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.post(self.url, data=data, format="json")
         assert response.status_code == 201, response.content
-        assert response.json()["data"]["user_config"] == ["a", "b"]
+        assert response.json()["data"]["features"][0]["value"] == ["a", "b"]
 
         # Object types
-        data["data"]["user_config"] = {"hello": "world"}
+        data["data"]["features"] = [{"key": "abc", "value": {"hello": "world"}}]
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.post(self.url, data=data, format="json")
         assert response.status_code == 201, response.content
-        assert response.json()["data"]["user_config"] == {"hello": "world"}
+        assert response.json()["data"]["features"][0]["value"] == {"hello": "world"}
 
-    def test_post_configuration_validation_error(self):
+    def test_post_configuration_required_fields(self):
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.post(
                 self.url,
@@ -160,18 +147,15 @@ class ConfiguratioAPITestCase(APITestCase):
         assert response.status_code == 400, response.content
 
         result = response.json()
-        assert len(result["data"]) == 3
-        assert result["data"]["sample_rate"] is not None
-        assert result["data"]["traces_sample_rate"] is not None
-        assert result["data"]["user_config"] is not None
+        assert len(result["data"]) == 2
+        assert result["data"]["options"] is not None
+        assert result["data"]["features"] is not None
 
     def test_delete_configuration(self):
         self.storage.set(
             {
-                "id": self.projectkey.public_key,
-                "sample_rate": 1.0,
-                "traces_sample_rate": 1.0,
-                "user_config": None,
+                "features": [],
+                "options": {"sample_rate": 1.0, "traces_sample_rate": 1.0},
             }
         )
         assert self.storage.get() is not None
@@ -182,9 +166,7 @@ class ConfiguratioAPITestCase(APITestCase):
         assert self.storage.get() is None
 
     def test_delete_configuration_not_found(self):
-        # Eagerly delete option if one exists.
         self.storage.pop()
-
         with self.feature(REMOTE_CONFIG_FEATURES):
             response = self.client.delete(self.url)
         assert response.status_code == 204