chore(data-forwarding): Limit access with feature flag (#102873)

Adds a feature flag to limit the access to the new endpoints.

Also checking the existing flag for access to the create/update
endpoints, but not for get/delete (so that users who downgrade can
delete their configs if they wish)

Author: leeandher

src/sentry/features/temporary.py

@@ -122,6 +122,10 @@ def register_temporary_features(manager: FeatureManager) -> None:
     manager.add("organizations:data-secrecy", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
     # Data Secrecy v2 (with Break the Glass feature)
     manager.add("organizations:data-secrecy-v2", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
+    # Enables access to the global data forwarding features
+    # Note: organizations:data-forwarding is a permanent, plan-based flag that enables access to the actual feature.
+    #       This flag will only be used to access the new UI/UX and endpoints before release.
+    manager.add("organizations:data-forwarding-revamp-access", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
     # Enables synthesis of device.class in ingest
     manager.add("organizations:device-class-synthesis", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=False)
     # Enable device.class as a selectable column

src/sentry/integrations/api/endpoints/data_forwarding_details.py

@@ -10,6 +10,7 @@
 from rest_framework.response import Response
 from rest_framework.views import APIView
 
+from sentry import features
 from sentry.api.api_owners import ApiOwner
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
@@ -78,6 +79,14 @@ def convert_args(
     ):
         args, kwargs = super().convert_args(request, organization_id_or_slug, *args, **kwargs)
 
+        if not features.has("organizations:data-forwarding-revamp-access", kwargs["organization"]):
+            raise PermissionDenied
+
+        if request.method == "PUT" and not features.has(
+            "organizations:data-forwarding", kwargs["organization"]
+        ):
+            raise PermissionDenied
+
         try:
             data_forwarder = DataForwarder.objects.get(
                 id=data_forwarder_id,

src/sentry/integrations/api/endpoints/data_forwarding_index.py

@@ -2,9 +2,11 @@
 from django.views.decorators.cache import never_cache
 from drf_spectacular.utils import extend_schema
 from rest_framework import status
+from rest_framework.exceptions import PermissionDenied
 from rest_framework.request import Request
 from rest_framework.response import Response
 
+from sentry import features
 from sentry.api.api_owners import ApiOwner
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
@@ -40,6 +42,12 @@ class DataForwardingIndexEndpoint(OrganizationEndpoint):
     }
     permission_classes = (OrganizationDataForwardingDetailsPermission,)
 
+    def convert_args(self, request: Request, *args, **kwargs):
+        args, kwargs = super().convert_args(request, *args, **kwargs)
+        if not features.has("organizations:data-forwarding-revamp-access", kwargs["organization"]):
+            raise PermissionDenied
+        return args, kwargs
+
     @extend_schema(
         operation_id="Retrieve Data Forwarding Configurations for an Organization",
         parameters=[GlobalParams.ORG_ID_OR_SLUG],
@@ -73,6 +81,9 @@ def get(self, request: Request, organization) -> Response:
     @set_referrer_policy("strict-origin-when-cross-origin")
     @method_decorator(never_cache)
     def post(self, request: Request, organization) -> Response:
+        if not features.has("organizations:data-forwarding", organization):
+            return self.respond(status=status.HTTP_403_FORBIDDEN)
+
         data = request.data
         data["organization_id"] = organization.id
 

tests/sentry/integrations/api/endpoints/test_data_forwarding.py

@@ -1,3 +1,5 @@
+from django.urls import reverse
+
 from sentry.integrations.models.data_forwarder import DataForwarder
 from sentry.integrations.types import DataForwarderProviderSlug
 from sentry.testutils.cases import APITestCase
@@ -12,9 +14,42 @@ def setUp(self) -> None:
         super().setUp()
         self.login_as(user=self.user)
 
+    def get_response(self, *args, **kwargs):
+        """
+        Override get_response to always add the required feature flag.
+        """
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": True,
+                "organizations:data-forwarding": True,
+            }
+        ):
+            return super().get_response(*args, **kwargs)
+
 
 @region_silo_test
 class DataForwardingIndexGetTest(DataForwardingIndexEndpointTest):
+
+    def test_without_revamp_feature_flag_access(self) -> None:
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": False,
+                "organizations:data-forwarding": True,
+            }
+        ):
+            response = self.client.get(reverse(self.endpoint, args=(self.organization.slug,)))
+            assert response.status_code == 403
+
+    def test_without_data_forwarding_feature_flag_access(self) -> None:
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": True,
+                "organizations:data-forwarding": False,
+            }
+        ):
+            response = self.client.get(reverse(self.endpoint, args=(self.organization.slug,)))
+            assert response.status_code == 200
+
     def test_get_single_data_forwarder(self) -> None:
         data_forwarder = self.create_data_forwarder(
             provider=DataForwarderProviderSlug.SEGMENT,
@@ -123,6 +158,26 @@ def test_get_with_disabled_data_forwarder(self) -> None:
 class DataForwardingIndexPostTest(DataForwardingIndexEndpointTest):
     method = "POST"
 
+    def test_without_revamp_feature_flag_access(self) -> None:
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": False,
+                "organizations:data-forwarding": True,
+            }
+        ):
+            response = self.client.post(reverse(self.endpoint, args=(self.organization.slug,)))
+            assert response.status_code == 403
+
+    def test_without_data_forwarding_feature_flag_access(self) -> None:
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": True,
+                "organizations:data-forwarding": False,
+            }
+        ):
+            response = self.client.post(reverse(self.endpoint, args=(self.organization.slug,)))
+            assert response.status_code == 403
+
     def test_create_segment_data_forwarder(self) -> None:
         payload = {
             "provider": DataForwarderProviderSlug.SEGMENT,

tests/sentry/integrations/api/endpoints/test_data_forwarding_details.py

@@ -1,3 +1,5 @@
+from django.urls import reverse
+
 from sentry.integrations.models.data_forwarder import DataForwarder
 from sentry.integrations.models.data_forwarder_project import DataForwarderProject
 from sentry.integrations.types import DataForwarderProviderSlug
@@ -13,11 +15,57 @@ def setUp(self) -> None:
         super().setUp()
         self.login_as(user=self.user)
 
+    def get_response(self, *args, **kwargs):
+        """
+        Override get_response to always add the required feature flag.
+        """
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": True,
+                "organizations:data-forwarding": True,
+            }
+        ):
+            return super().get_response(*args, **kwargs)
+
 
 @region_silo_test
 class DataForwardingDetailsPutTest(DataForwardingDetailsEndpointTest):
     method = "PUT"
 
+    def test_without_revamp_feature_flag_access(self) -> None:
+        data_forwarder = self.create_data_forwarder(
+            provider=DataForwarderProviderSlug.SEGMENT,
+            config={"write_key": "old_key"},
+            is_enabled=True,
+        )
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": False,
+                "organizations:data-forwarding": True,
+            }
+        ):
+            response = self.client.put(
+                reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id))
+            )
+            assert response.status_code == 403
+
+    def test_without_data_forwarding_feature_flag_access(self) -> None:
+        data_forwarder = self.create_data_forwarder(
+            provider=DataForwarderProviderSlug.SEGMENT,
+            config={"write_key": "old_key"},
+            is_enabled=True,
+        )
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": True,
+                "organizations:data-forwarding": False,
+            }
+        ):
+            response = self.client.put(
+                reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id))
+            )
+            assert response.status_code == 403
+
     def test_update_data_forwarder(self) -> None:
         data_forwarder = self.create_data_forwarder(
             provider=DataForwarderProviderSlug.SEGMENT,
@@ -37,9 +85,10 @@ def test_update_data_forwarder(self) -> None:
             "project_ids": [self.project.id],
         }
 
-        response = self.get_success_response(
-            self.organization.slug, data_forwarder.id, status_code=200, **payload
-        )
+        with self.feature({"organizations:data-forwarding-revamp-access": True}):
+            response = self.get_success_response(
+                self.organization.slug, data_forwarder.id, status_code=200, **payload
+            )
 
         assert response.data["config"] == {"write_key": "new_key"}
         assert not response.data["isEnabled"]
@@ -458,6 +507,40 @@ def test_update_single_project_creates_new_config(self) -> None:
 class DataForwardingDetailsDeleteTest(DataForwardingDetailsEndpointTest):
     method = "DELETE"
 
+    def test_without_revamp_feature_flag_access(self) -> None:
+        data_forwarder = self.create_data_forwarder(
+            provider=DataForwarderProviderSlug.SEGMENT,
+            config={"write_key": "old_key"},
+            is_enabled=True,
+        )
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": False,
+                "organizations:data-forwarding": True,
+            }
+        ):
+            response = self.client.delete(
+                reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id))
+            )
+            assert response.status_code == 403
+
+    def test_without_data_forwarding_feature_flag_access(self) -> None:
+        data_forwarder = self.create_data_forwarder(
+            provider=DataForwarderProviderSlug.SEGMENT,
+            config={"write_key": "old_key"},
+            is_enabled=True,
+        )
+        with self.feature(
+            {
+                "organizations:data-forwarding-revamp-access": True,
+                "organizations:data-forwarding": False,
+            }
+        ):
+            response = self.client.delete(
+                reverse(self.endpoint, args=(self.organization.slug, data_forwarder.id))
+            )
+            assert response.status_code == 204
+
     def test_delete_data_forwarder(self) -> None:
         data_forwarder = self.create_data_forwarder(
             provider=DataForwarderProviderSlug.SEGMENT,