feat(difs): Redirect to Objectstore for Objectstore-backed DIF downloads

pyproject.toml

@@ -57,7 +57,7 @@ dependencies = [
   "mmh3>=4.0.0",
   "msgspec>=0.19.0",
   "msgpack>=1.2.1",
-  "objectstore-client>=0.1.13",
+  "objectstore-client>=0.1.14",
   "openai>=1.3.5",
   "orjson>=3.10.10",
   "packaging>=24.1",

src/sentry/api/endpoints/debug_files.py

@@ -6,12 +6,14 @@
 import uuid
 from collections.abc import Iterable, Mapping, Sequence, Set
 from typing import TYPE_CHECKING, NotRequired, TypedDict, TypeGuard, cast
+from urllib.parse import urlsplit
 
 import jsonschema
 import orjson
 from django.db import IntegrityError, router
 from django.db.models import Case, Exists, F, IntegerField, Q, QuerySet, Value, When
-from django.http import Http404, HttpResponse, StreamingHttpResponse
+from django.http import Http404, HttpResponse, HttpResponseRedirect, StreamingHttpResponse
+from django.urls import reverse
 from drf_spectacular.utils import OpenApiParameter, extend_schema
 from objectstore_client import RequestError
 from rest_framework import status
@@ -35,11 +37,12 @@
 from sentry.api.paginator import OffsetPaginator
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.debug_file import DebugFileSerializerResponse
-from sentry.api.utils import to_valid_int_id
+from sentry.api.utils import generate_locality_url, to_valid_int_id
 from sentry.apidocs.constants import RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND, RESPONSE_UNAUTHORIZED
 from sentry.apidocs.examples.dsym_examples import DebugFileExamples
 from sentry.apidocs.parameters import CursorQueryParam, GlobalParams
 from sentry.apidocs.utils import inline_sentry_response_serializer
+from sentry.auth import system
 from sentry.auth.access import Access
 from sentry.auth.superuser import is_active_superuser
 from sentry.auth.system import is_system_auth
@@ -60,6 +63,7 @@
 from sentry.models.project import Project
 from sentry.models.release import Release, get_artifact_counts
 from sentry.models.releasefile import ReleaseFile
+from sentry.objectstore import maybe_rewrite_url_for_symbolicator
 from sentry.roles import organization_roles
 from sentry.tasks.assemble import (
     AssembleTask,
@@ -275,6 +279,9 @@ def download(self, debug_file_id, project: Project):
         if debug_file is None:
             raise Http404
 
+        if debug_file.storage_path is not None:
+            return self._redirect_to_objectstore(self.request, debug_file, project)
+
         try:
             fp = debug_file.get_file()
 
@@ -295,6 +302,32 @@ def stream_debug_file():
         except (OSError, RequestError, Project.DoesNotExist, HTTPError):
             raise Http404
 
+    def _redirect_to_objectstore(
+        self, request: Request, debug_file: ProjectDebugFile, project: Project
+    ) -> HttpResponseRedirect:
+        presigned_url = debug_file.get_objectstore_presigned_url()
+
+        if system.is_internal_ip(request):
+            # Redirect to a URL pointing to the internal Objectstore ip/hostname.
+            # In dev/test, we potentially need to rewrite this URL to point to the hostname in the docker network
+            # instead, so we need to additionally wrap this with `maybe_rewrite_url_for_symbolicator`.
+            # TODO(lcian): Find a more robust way to do this. Here we assume that the caller is Symbolicator,
+            # which is currently the case in practice, but in theory it could be any other service.
+            url = maybe_rewrite_url_for_symbolicator(presigned_url)
+        else:
+            parts = urlsplit(presigned_url)
+            proxy_path = reverse(
+                "sentry-api-0-organization-objectstore",
+                kwargs={
+                    "organization_id_or_slug": project.organization_id,
+                    "path": parts.path.lstrip("/"),
+                },
+            )
+            base = generate_locality_url().rstrip("/")
+            url = f"{base}{proxy_path}?{parts.query}"
+
+        return HttpResponseRedirect(url)
+
     @extend_schema(
         operation_id="listProjectDebugFiles",
         summary="List a Project's Debug Information Files",

src/sentry/models/debugfile.py

@@ -15,7 +15,7 @@
 import uuid
 import zipfile
 from collections.abc import Container, Iterable, Mapping
-from datetime import datetime
+from datetime import datetime, timedelta
 from typing import IO, TYPE_CHECKING, Any, BinaryIO, ClassVar
 
 from django.db import models
@@ -278,6 +278,16 @@ def get_file(self) -> IO[bytes]:
             return self.file.getfile()
         raise ValueError("ProjectDebugFile has neither file nor storage_path")
 
+    def get_objectstore_presigned_url(self) -> str:
+        """
+        Returns a pre-signed URL authorizing a GET/HEAD request of this debug file directly from
+        Objectstore. Only valid when the file is stored in Objectstore.
+        """
+        assert self.storage_path is not None, "debug file is not stored in Objectstore"
+        return self._get_objectstore_session().presigned_object_url(
+            "GET", self.storage_path, duration=timedelta(minutes=5)
+        )
+
     def save_to(self, path: str) -> None:
         if self.storage_path is not None:
             path = os.path.abspath(path)

src/sentry/objectstore/__init__.py

@@ -145,15 +145,16 @@ def get_preprod_session(org: int, project: int) -> Session:
 _IS_SYMBOLICATOR_CONTAINER: bool | None = None
 
 
-def get_symbolicator_url(session: Session, key: str) -> str:
+def maybe_rewrite_url_for_symbolicator(url: str) -> str:
     """
-    Gets the URL that Symbolicator shall use to access the object at the given key in Objectstore.
+    Rewrites a full Objectstore URL so that Symbolicator can reach it.
 
-    In prod, this is simply the `object_url` returned by `objectstore_client`, as both Sentry and Symbolicator
-    will talk to Objectstore using the same hostname.
+    In prod, the URL is returned unchanged, as both Sentry and Symbolicator talk to Objectstore
+    using the same hostname.
 
-    While in development or testing, we might need to replace the hostname, depending on how Symbolicator is running.
-    This function runs a `docker ps` to automatically return the correct URL in the following 2 cases:
+    While in development or testing, we might need to replace the hostname, depending on how
+    Symbolicator is running. This function runs a `docker ps` to automatically return the correct
+    URL in the following 2 cases:
         - Symbolicator running in Docker (possibly via `devservices`) -- this mirrors `sentry`'s CI.
           If this is detected, we replace Objectstore's hostname with the one reachable in the Docker network.
 
@@ -164,7 +165,6 @@ def get_symbolicator_url(session: Session, key: str) -> str:
     """
     global _IS_SYMBOLICATOR_CONTAINER  # Cached to avoid running `docker ps` multiple times
 
-    url = session.object_url(key)
     if not (settings.IS_DEV or in_test_environment()):
         return url
 
@@ -186,3 +186,12 @@ def get_symbolicator_url(session: Session, key: str) -> str:
         replacement += f":{parsed.port}"
     updated = parsed._replace(netloc=replacement)
     return urlunparse(updated)
+
+
+def get_symbolicator_url(session: Session, key: str) -> str:
+    """
+    Gets the URL that Symbolicator shall use to access the object at the given key in Objectstore.
+
+    The URL is only rewritten in dev/test mode. See `maybe_rewrite_url_for_symbolicator` for details.
+    """
+    return maybe_rewrite_url_for_symbolicator(session.object_url(key))

tests/sentry/api/endpoints/test_debug_files.py

@@ -1,7 +1,9 @@
 import zipfile
 from io import BytesIO
+from unittest.mock import patch
 from uuid import uuid4
 
+import requests
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.urls import reverse
 
@@ -12,6 +14,7 @@
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.helpers.response import close_streaming_response
 from sentry.testutils.objectstore import debug_files_test_both_backends
+from sentry.testutils.skips import requires_objectstore
 
 # This is obviously a freely generated UUID and not the checksum UUID.
 # This is permissible if users want to send different UUIDs
@@ -51,6 +54,27 @@ def _upload_proguard(self, url, uuid):
             format="multipart",
         )
 
+    def _assert_successful_download(self, response, content: bytes, filename=None) -> None:
+        if response.status_code == 302:
+            location = response["Location"]
+            # In dev/test the host may be rewritten to the Docker-internal `objectstore` hostname
+            # (so Symbolicator can reach it); rewrite it back so this host-side test can follow it.
+            location = location.replace("://objectstore:", "://127.0.0.1:")
+            response = requests.get(location)
+            assert response.status_code == 200, response.text
+            assert response.content == content
+            if filename is not None:
+                assert (
+                    response.headers["Content-Disposition"] == f'attachment; filename="{filename}"'
+                )
+        else:
+            assert response.status_code == 200, response.content
+            if filename is not None:
+                assert response.get("Content-Disposition") == f'attachment; filename="{filename}"'
+                assert response.get("Content-Length") == str(len(content))
+            assert response.get("Content-Type") == "application/octet-stream"
+            assert content == close_streaming_response(response)
+
 
 @debug_files_test_both_backends
 class DebugFilesTest(DebugFilesTestCases):
@@ -137,8 +161,7 @@ def test_access_control(self) -> None:
 
         # `self.user` has access to these files
         response = self.client.get(f"{self.url}?id={download_id}")
-        assert response.status_code == 200, response.content
-        assert PROGUARD_SOURCE == close_streaming_response(response)
+        self._assert_successful_download(response, PROGUARD_SOURCE)
 
         # with another user on a different org
         other_user = self.create_user()
@@ -177,21 +200,13 @@ def test_dsyms_requests(self) -> None:
         # Download as a user with sufficient role
         self.organization.update_option("sentry:debug_files_role", "admin")
         response = self.client.get(self.url + "?id=" + download_id)
-        assert response.status_code == 200, response.content
-        assert (
-            response.get("Content-Disposition")
-            == 'attachment; filename="' + PROGUARD_UUID + '.txt"'
-        )
-        assert response.get("Content-Length") == str(len(PROGUARD_SOURCE))
-        assert response.get("Content-Type") == "application/octet-stream"
-        assert PROGUARD_SOURCE == close_streaming_response(response)
+        self._assert_successful_download(response, PROGUARD_SOURCE, filename=f"{PROGUARD_UUID}.txt")
 
         # Download as a superuser
         superuser = self.create_user(is_superuser=True)
         self.login_as(user=superuser, superuser=True)
         response = self.client.get(self.url + "?id=" + download_id)
-        assert response.get("Content-Type") == "application/octet-stream"
-        close_streaming_response(response)
+        self._assert_successful_download(response, PROGUARD_SOURCE)
 
         # Download as a user without sufficient role
         self.organization.update_option("sentry:debug_files_role", "owner")
@@ -284,9 +299,7 @@ def test_dsyms_as_team_admin(self) -> None:
         self.login_as(team_admin)
         # Team admin with project access can download
         response = self.client.get(self.url + "?id=" + download_id)
-        assert response.status_code == 200, response.content
-        assert response.get("Content-Type") == "application/octet-stream"
-        close_streaming_response(response)
+        self._assert_successful_download(response, PROGUARD_SOURCE)
 
         # Team admin with project access can delete
         response = self.client.delete(self.url + "?id=" + download_id)
@@ -315,9 +328,7 @@ def test_project_debug_files_role_overrides_organization(self) -> None:
         # Set project debug_files_role to "member" - member should now be able to download
         self.project.update_option("sentry:debug_files_role", "member")
         response = self.client.get(f"{self.url}?id={download_id}")
-        assert response.status_code == 200, response.content
-        assert response.get("Content-Type") == "application/octet-stream"
-        close_streaming_response(response)
+        self._assert_successful_download(response, PROGUARD_SOURCE)
 
         # Remove project option - should fall back to organization setting (owner)
         self.project.delete_option("sentry:debug_files_role")
@@ -327,9 +338,42 @@ def test_project_debug_files_role_overrides_organization(self) -> None:
         # Set organization to "member" - member should be able to download
         self.organization.update_option("sentry:debug_files_role", "member")
         response = self.client.get(f"{self.url}?id={download_id}")
-        assert response.status_code == 200, response.content
-        assert response.get("Content-Type") == "application/octet-stream"
-        close_streaming_response(response)
+        self._assert_successful_download(response, PROGUARD_SOURCE)
+
+
+@requires_objectstore
+class DebugFileObjectstoreRedirectTest(DebugFilesTestCases):
+    """Explicit coverage of both redirect branches for Objectstore-backed debug files."""
+
+    def _upload_and_get_download_id(self) -> str:
+        with self.feature({"organizations:objectstore-debugfiles-write": True}):
+            response = self._upload_proguard(self.url, PROGUARD_UUID)
+        assert response.status_code == 201, response.content
+        return self.client.get(self.url).data[0]["id"]
+
+    def test_internal_request_redirects_to_objectstore(self) -> None:
+        download_id = self._upload_and_get_download_id()
+
+        with patch("sentry.auth.system.is_internal_ip", return_value=True):
+            response = self.client.get(f"{self.url}?id={download_id}")
+
+        assert response.status_code == 302
+        # Internal callers are redirected straight to Objectstore, not through the cell proxy.
+        assert "/organizations/" not in response["Location"]
+        self._assert_successful_download(response, PROGUARD_SOURCE)
+
+    def test_external_request_redirects_to_cell_proxy(self) -> None:
+        download_id = self._upload_and_get_download_id()
+
+        with patch("sentry.auth.system.is_internal_ip", return_value=False):
+            response = self.client.get(f"{self.url}?id={download_id}")
+
+        assert response.status_code == 302
+        location = response["Location"]
+        assert (
+            f"/organizations/{self.organization.id}/objectstore/v1/objects/debug_files/" in location
+        )
+        assert "os_sig=" in location
 
 
 @debug_files_test_both_backends