Browse Source

ref: hashed frontend assets attempt 2 (#47334)

this time with the updated configmap path (mounted as directory instead
of as a file)

corresponding ops change: https://github.com/getsentry/ops/pull/6593

tested in staging environment
anthony sottile 1 year ago
parent
commit
01c9043304

+ 1 - 0
requirements-dev-frozen.txt

@@ -186,6 +186,7 @@ tqdm==4.64.1
 trio==0.21.0
 trio-websocket==0.9.2
 types-beautifulsoup4==4.11.6
+types-cachetools==5.3.0.5
 types-freezegun==1.1.10
 types-jsonschema==4.16.1
 types-psycopg2==2.9.21

+ 1 - 0
requirements-dev.txt

@@ -32,6 +32,7 @@ lxml-stubs
 msgpack-types>=0.2.0
 mypy>=0.982
 types-beautifulsoup4
+types-cachetools
 types-freezegun
 types-jsonschema
 types-psycopg2

+ 40 - 4
src/sentry/utils/assets.py

@@ -1,5 +1,23 @@
+from __future__ import annotations
+
+import os.path
+
+from cachetools.func import ttl_cache
 from django.conf import settings
 
+from sentry.utils import json
+
+
+@ttl_cache(ttl=60)
+def _frontend_versions() -> dict[str, str]:
+    try:
+        with open(
+            os.path.join(settings.CONF_DIR, "settings", "frontend", "frontend-versions.json")
+        ) as f:
+            return json.load(f)  # type: ignore[no-any-return]  # getsentry path
+    except OSError:
+        return {}  # common case for self-hosted
+
 
 def get_frontend_app_asset_url(module: str, key: str) -> str:
     """
@@ -8,12 +26,30 @@ def get_frontend_app_asset_url(module: str, key: str) -> str:
     server before using their locally cached asset.
 
     Example:
-      {% frontend_app_asset_url 'sentry' 'sentry.css' %}
-      =>  "/_static/dist/sentry/sentry.css"
+      {% frontend_app_asset_url 'sentry' 'entrypoints/sentry.css' %}
+      =>  "/_static/dist/sentry/entrypoints/sentry.css"
     """
-    args = (settings.STATIC_FRONTEND_APP_URL.rstrip("/"), module, key.lstrip("/"))
+    if not key.startswith("entrypoints/"):
+        raise AssertionError(f"unexpected key: {key}")
+
+    entrypoints, key = key.split("/", 1)
+    versions = _frontend_versions()
+    if versions:
+        entrypoints = "entrypoints-hashed"
+        key = versions[key]
+
+    return "/".join(
+        (
+            settings.STATIC_FRONTEND_APP_URL.rstrip("/"),
+            module,
+            entrypoints,
+            key,
+        )
+    )
+
 
-    return "{}/{}/{}".format(*args)
+def get_frontend_dist_prefix() -> str:
+    return f"{settings.STATIC_FRONTEND_APP_URL.rstrip('/')}/sentry/"
 
 
 def get_asset_url(module: str, path: str) -> str:

+ 2 - 2
src/sentry/web/client_config.py

@@ -14,7 +14,7 @@ from sentry.services.hybrid_cloud.organization import organization_service
 from sentry.services.hybrid_cloud.project_key import ProjectKeyRole, project_key_service
 from sentry.services.hybrid_cloud.user import UserSerializeType, user_service
 from sentry.utils import auth
-from sentry.utils.assets import get_frontend_app_asset_url
+from sentry.utils.assets import get_frontend_dist_prefix
 from sentry.utils.email import is_smtp_enabled
 from sentry.utils.http import is_using_customer_domain
 from sentry.utils.settings import is_self_hosted
@@ -181,7 +181,7 @@ def get_client_config(request=None):
         "urlPrefix": options.get("system.url-prefix"),
         "version": version_info,
         "features": enabled_features,
-        "distPrefix": get_frontend_app_asset_url("sentry", ""),
+        "distPrefix": get_frontend_dist_prefix(),
         "needsUpgrade": needs_upgrade,
         "dsn": public_dsn,
         "statuspage": _get_statuspage(),

+ 70 - 0
tests/sentry/utils/test_assets.py

@@ -0,0 +1,70 @@
+from __future__ import annotations
+
+from unittest import mock
+
+import pytest
+from django.conf import settings
+
+from sentry.utils import assets
+
+
+@pytest.fixture(autouse=True)
+def reset_cache():
+    assets._frontend_versions.cache_clear()
+    yield
+
+
+@pytest.fixture
+def self_hosted(tmp_path):
+    with mock.patch.object(settings, "STATIC_FRONTEND_APP_URL", "/_static/dist/"):
+        conf_dir = tmp_path.joinpath("conf")
+        conf_dir.mkdir()
+        with mock.patch.object(settings, "CONF_DIR", conf_dir):
+            yield
+
+
+@pytest.fixture
+def getsentry_no_configmap(tmp_path):
+    # shouldn't actually happen -- but make sure it still works!
+    with mock.patch.object(
+        settings, "STATIC_FRONTEND_APP_URL", "https://static.example.com/_static/dist/"
+    ):
+        conf_dir = tmp_path.joinpath("conf")
+        conf_dir.mkdir()
+        with mock.patch.object(settings, "CONF_DIR", conf_dir):
+            yield
+
+
+@pytest.fixture
+def getsentry(tmp_path):
+    with mock.patch.object(
+        settings, "STATIC_FRONTEND_APP_URL", "https://static.example.com/_static/dist/"
+    ):
+        conf_dir = tmp_path.joinpath("conf")
+        conf_dir.mkdir()
+        conf_dir.joinpath("settings/frontend").mkdir(parents=True)
+        conf_dir.joinpath("settings/frontend/frontend-versions.json").write_text(
+            '{"app.js": "app-deadbeef.js", "app.css": "app-cafecafe.css"}'
+        )
+        with mock.patch.object(settings, "CONF_DIR", conf_dir):
+            yield
+
+
+@pytest.mark.usefixtures("self_hosted")
+def test_frontend_app_asset_url_self_hosted():
+    ret = assets.get_frontend_app_asset_url("sentry", "entrypoints/app.js")
+    assert ret == "/_static/dist/sentry/entrypoints/app.js"
+
+
+@pytest.mark.usefixtures("getsentry_no_configmap")
+def test_frontend_app_asset_url_getsentry_no_configmap():
+    ret = assets.get_frontend_app_asset_url("sentry", "entrypoints/app.js")
+    assert ret == "https://static.example.com/_static/dist/sentry/entrypoints/app.js"
+
+
+@pytest.mark.usefixtures("getsentry")
+def test_frontend_app_asset_url_getsentry():
+    ret = assets.get_frontend_app_asset_url("sentry", "entrypoints/app.js")
+    assert (
+        ret == "https://static.example.com/_static/dist/sentry/entrypoints-hashed/app-deadbeef.js"
+    )

+ 2 - 2
tests/sentry/web/frontend/generic/test_static_media.py

@@ -58,12 +58,12 @@ class StaticMediaTest(TestCase):
         response = self.client.get("/_static/dist/sentry/invalid.js")
         assert response.status_code == 404, response
 
-        dist_path = os.path.join("src", "sentry", "static", "sentry", "dist")
+        dist_path = os.path.join("src", "sentry", "static", "sentry", "dist", "entrypoints")
         os.makedirs(dist_path, exist_ok=True)
 
         try:
             with open(os.path.join(dist_path, "test.js"), "a"):
-                url = get_frontend_app_asset_url("sentry", "test.js")
+                url = get_frontend_app_asset_url("sentry", "entrypoints/test.js")
 
                 response = self.client.get(url)
                 close_streaming_response(response)