feat: Use unversioned frontend asset paths

.github/workflows/acceptance.yml

@@ -64,15 +64,11 @@ jobs:
           name: jest-html
           path: .artifacts/visual-snapshots/jest
 
-      - name: Get CSS filename
-        id: css-manifest
-        run: echo "::set-output name=sentrycss::$(jq -r '.["sentry.css"]' src/sentry/static/sentry/dist/manifest.json)"
-
       - name: Create Images from HTML
         uses: getsentry/action-html-to-image@main
         with:
           base-path: .artifacts/visual-snapshots/jest
-          css-path: src/sentry/static/sentry/dist/${{ steps.css-manifest.outputs.sentrycss }}
+          css-path: src/sentry/static/sentry/dist/entrypoints/sentry.css
 
       - name: Save snapshots
         if: always()

conftest.py

@@ -1,10 +1,6 @@
 import os
 import sys
 
-dist_path = os.path.abspath(
-    os.path.join(os.path.dirname(__file__), "src", "sentry", "static", "sentry", "dist")
-)
-manifest_path = os.path.join(dist_path, "manifest.json")
 pytest_plugins = ["sentry.utils.pytest"]
 
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), "src"))
@@ -16,24 +12,3 @@ def pytest_configure(config):
     # XXX(dcramer): Kombu throws a warning due to transaction.commit_manually
     # being used
     warnings.filterwarnings("error", "", Warning, r"^(?!(|kombu|raven|sentry))")
-
-    # Create an empty webpack manifest file - otherwise tests will crash if it does not exist
-    os.makedirs(dist_path, exist_ok=True)
-
-    # Only create manifest if it doesn't exist
-    # (e.g. acceptance tests will have an actual manifest from webpack)
-    if os.path.exists(manifest_path):
-        return
-
-    with open(manifest_path, "w+") as fp:
-        fp.write("{}")
-
-
-def pytest_unconfigure():
-    if not os.path.exists(manifest_path):
-        return
-
-    # Clean up manifest file if contents are empty
-    with open(manifest_path) as f:
-        if f.read() == "{}":
-            os.remove(manifest_path)

package.json

@@ -129,7 +129,6 @@
     "u2f-api": "1.0.10",
     "webpack": "5.41.1",
     "webpack-cli": "4.7.2",
-    "webpack-manifest-plugin": "^3.1.1",
     "webpack-remove-empty-scripts": "^0.7.1",
     "wink-jaro-distance": "^2.0.0",
     "zxcvbn": "^4.4.2"

requirements-base.txt

@@ -9,7 +9,6 @@ croniter==0.3.37
 dataclasses==0.8; python_version <= '3.6'
 datadog==0.29.3
 django-crispy-forms==1.7.2
-django-manifest-loader==1.0.0
 django-picklefield==1.0.0
 Django==1.11.29
 djangorestframework==3.11.2

src/sentry/conf/server.py

@@ -327,7 +327,6 @@ def env(key, default="", type=None):
     "django.contrib.sites",
     "crispy_forms",
     "rest_framework",
-    "manifest_loader",
     "sentry",
     "sentry.analytics",
     "sentry.incidents.apps.Config",
@@ -367,16 +366,13 @@ def env(key, default="", type=None):
 STATIC_URL = "/_static/{version}/"
 # webpack assets live at a different URL that is unversioned
 # as we configure webpack to include file content based hash in the filename
-STATIC_MANIFEST_URL = "/_static/dist/"
+STATIC_UNVERSIONED_URL = "/_static/dist/"
 
-# The webpack output directory, used by django-manifest-loader
+# The webpack output directory
 STATICFILES_DIRS = [
     os.path.join(STATIC_ROOT, "sentry", "dist"),
 ]
 
-# django-manifest-loader settings
-MANIFEST_LOADER = {"cache": True}
-
 # various middleware will use this to identify resources which should not access
 # cookies
 ANONYMOUS_STATIC_PREFIXES = (

src/sentry/middleware/user.py

@@ -7,6 +7,7 @@
 
 class UserActiveMiddleware(MiddlewareMixin):
     disallowed_paths = (
+        "sentry.web.frontend.generic.unversioned_static_media",
         "sentry.web.frontend.generic.static_media",
         "sentry.web.frontend.organization_avatar",
         "sentry.web.frontend.project_avatar",

src/sentry/templates/sentry/bases/react.html

@@ -1,2 +1 @@
 {% extends "sentry/base-react.html" %}
-{% load sentry_assets %}

src/sentry/templates/sentry/bases/react_pipeline.html

@@ -14,6 +14,6 @@
 {% endblock %}
 
 {% block scripts_main_entrypoint %}
-    {% manifest_asset_url "sentry" "pipeline.js" as asset_url %}
+    {% unversioned_asset_url "sentry" "entrypoints/pipeline.js" as asset_url %}
     {% script src=asset_url data-entry="true" %}{% endscript %}
 {% endblock %}

src/sentry/templates/sentry/layout.html

@@ -25,7 +25,7 @@
 
   <link rel="mask-icon" sizes="any" href="{% absolute_asset_url "sentry" "images/logos/logo-sentry.svg" %}" color="#FB4226">
 
-  <link href="{% manifest_asset_url "sentry" "sentry.css" %}" rel="stylesheet"/>
+  <link href="{% unversioned_asset_url "sentry" "entrypoints/sentry.css" %}" rel="stylesheet"/>
 
   {% block css %}{% endblock %}
 
@@ -53,11 +53,11 @@
   {% endscript %}
 
   {% block scripts %}
-  {% manifest_asset_url "sentry" "runtime.js" as asset_url %}
+  {% unversioned_asset_url "sentry" "entrypoints/runtime.js" as asset_url %}
   {% script src=asset_url %}{% endscript %}
 
   {% block scripts_main_entrypoint %}
-    {% manifest_asset_url "sentry" "app.js" as asset_url %}
+    {% unversioned_asset_url "sentry" "entrypoints/app.js" as asset_url %}
     {% script src=asset_url data-entry="true" %}{% endscript %}
   {% endblock %}
 

src/sentry/templatetags/sentry_assets.py

@@ -5,13 +5,13 @@
 from django.template.base import token_kwargs
 
 from sentry import options
-from sentry.utils.assets import get_asset_url, get_manifest_url
+from sentry.utils.assets import get_asset_url, get_unversioned_asset_url
 from sentry.utils.http import absolute_uri
 
 register = template.Library()
 
 register.simple_tag(get_asset_url, name="asset_url")
-register.simple_tag(get_manifest_url, name="manifest_asset_url")
+register.simple_tag(get_unversioned_asset_url, name="unversioned_asset_url")
 
 
 @register.simple_tag

src/sentry/testutils/cases.py

@@ -106,36 +106,6 @@ def setup_dummy_auth_provider(self):
     def tasks(self):
         return TaskRunner()
 
-    @classmethod
-    @contextmanager
-    def static_asset_manifest(cls, manifest_data):
-        dist_path = "src/sentry/static/sentry/dist"
-        manifest_path = f"{dist_path}/manifest.json"
-
-        with open(manifest_path, "w") as manifest_fp:
-            json.dump(manifest_data, manifest_fp)
-
-        files = []
-        for file_path in manifest_data.values():
-            full_path = f"{dist_path}/{file_path}"
-            # make directories in case they don't exist
-            # (e.g. dist path should exist, but subdirs won't)
-            os.makedirs(os.path.dirname(full_path), exist_ok=True)
-            open(full_path, "a").close()
-            files.append(full_path)
-
-        try:
-            yield {"manifest": manifest_data, "files": files}
-        finally:
-            with open(manifest_path, "w") as manifest_fp:
-                # Instead of unlinking, preserve an empty manifest file so that other tests that
-                # may or may not load static assets, do not fail
-                manifest_fp.write("{}")
-
-            # Remove any files created from the test manifest
-            for filepath in files:
-                os.unlink(filepath)
-
     @classmethod
     @contextmanager
     def capture_on_commit_callbacks(cls, using=DEFAULT_DB_ALIAS, execute=False):

src/sentry/utils/assets.py

@@ -1,37 +1,26 @@
 from django.conf import settings
-from manifest_loader.utils import _get_manifest, _load_from_manifest
 
 
-def get_manifest_obj():
+def get_unversioned_asset_url(module, key):
     """
-    Returns the webpack asset manifest as a dict of <file key, hashed file name>
-
-    The `manifest_loader` library caches this (if `cache` settings is set)
-    """
-    return _get_manifest()
-
-
-def get_manifest_url(module, key):
-    """
-    Returns an asset URL that is produced by webpack. Uses webpack's manifest to map
-    `key` to the asset produced by webpack. Required if using file contents based hashing for filenames.
+    Returns an asset URL that is unversioned. These assets should have a
+    `Cache-Control: max-age=0, must-revalidate` so that clients must validate with the origin
+    server before using their locally cached asset.
 
     Example:
-      {% manifest_asset_url 'sentry' 'sentry.css' %}
-      =>  "/_static/dist/sentry/sentry.filehash123.css"
+      {% unversioned_asset_url 'sentry' 'sentry.css' %}
+      =>  "/_static/dist/sentry/sentry.css"
     """
-    manifest_obj = get_manifest_obj()
-    manifest_value = _load_from_manifest(manifest_obj, key=key)
 
-    return "{}/{}/{}".format(settings.STATIC_MANIFEST_URL.rstrip("/"), module, manifest_value)
+    return "{}/{}/{}".format(settings.STATIC_UNVERSIONED_URL.rstrip("/"), module, key.lstrip("/"))
 
 
 def get_asset_url(module, path):
     """
     Returns a versioned asset URL (located within Sentry's static files).
 
     Example:
-      {% asset_url 'sentry' 'images/sentry.png' %}
-      =>  "/_static/74d127b78dc7daf2c51f/sentry/sentry.png"
+    {% asset_url 'sentry' 'images/sentry.png' %}
+    =>  "/_static/74d127b78dc7daf2c51f/sentry/sentry.png"
     """
     return "{}/{}/{}".format(settings.STATIC_URL.rstrip("/"), module, path.lstrip("/"))

src/sentry/web/client_config.py

@@ -12,7 +12,7 @@
 from sentry.auth.superuser import is_active_superuser
 from sentry.models import ProjectKey
 from sentry.utils import auth
-from sentry.utils.assets import get_manifest_url
+from sentry.utils.assets import get_unversioned_asset_url
 from sentry.utils.email import is_smtp_enabled
 from sentry.utils.support import get_support_mail
 
@@ -146,7 +146,7 @@ def get_client_config(request=None):
         "urlPrefix": options.get("system.url-prefix"),
         "version": version_info,
         "features": enabled_features,
-        "distPrefix": get_manifest_url("sentry", ""),
+        "distPrefix": get_unversioned_asset_url("sentry", ""),
         "needsUpgrade": needs_upgrade,
         "dsn": public_dsn,
         "dsn_requests": _get_dsn_requests(),

src/sentry/web/frontend/generic.py

@@ -8,6 +8,18 @@
 from django.views import static
 
 FOREVER_CACHE = "max-age=315360000"
+
+# See
+# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#requiring_revalidation
+# This means that clients *CAN* cache the resource, but they must revalidate
+# before using it This means we will have a small HTTP request overhead to
+# verify that the local resource is not outdated
+#
+# Note that the above docs state that "no-cache" is the same as "max-age=0,
+# must-revalidate", but some CDNs will not treat them as the same
+NO_CACHE = "max-age=0, must-revalidate"
+
+# no-store means that the response should not be stored in *ANY* cache
 NEVER_CACHE = "max-age=0, no-cache, no-store, must-revalidate"
 
 
@@ -33,19 +45,21 @@ def resolve(path):
     return os.path.split(absolute_path)
 
 
-def static_media_with_manifest(request, **kwargs):
+def unversioned_static_media(request, **kwargs):
     """
-    Serve static files that are generated with webpack.
-
-    Only these assets should have a long TTL as its filename has a hash based on file contents
+    Serve static files that should not have any versioned paths/filenames.
+    These assets will have cache headers to say that it can be cached by a
+    client, but it *must* be validated against the origin server before the
+    cached asset can be used.
     """
+
     path = kwargs.get("path", "")
 
     kwargs["path"] = f"dist/{path}"
     response = static_media(request, **kwargs)
 
     if not settings.DEBUG:
-        response["Cache-Control"] = FOREVER_CACHE
+        response["Cache-Control"] = NO_CACHE
 
     return response
 

src/sentry/web/urls.py

@@ -96,8 +96,8 @@
     # a filecontent-based hash in its filenames so that it can be cached long term
     url(
         r"^_static/dist/(?P<module>[^/]+)/(?P<path>.*)$",
-        generic.static_media_with_manifest,
-        name="sentry-webpack-media",
+        generic.unversioned_static_media,
+        name="sentry-unversioned-media",
     ),
     # The static version is either a 10 digit timestamp, a sha1, or md5 hash
     url(

tests/sentry/web/frontend/generic/test_static_media.py

@@ -3,8 +3,8 @@
 from django.test.utils import override_settings
 
 from sentry.testutils import TestCase
-from sentry.utils.assets import get_manifest_url
-from sentry.web.frontend.generic import FOREVER_CACHE, NEVER_CACHE
+from sentry.utils.assets import get_unversioned_asset_url
+from sentry.web.frontend.generic import FOREVER_CACHE, NEVER_CACHE, NO_CACHE
 
 
 class StaticMediaTest(TestCase):
@@ -44,36 +44,40 @@ def test_versioned(self):
             assert response["Access-Control-Allow-Origin"] == "*"
 
     @override_settings(DEBUG=False)
-    def test_from_manifest(self):
+    def test_unversioned(self):
         """
-        manifest here refers to the webpack manifest for frontend assets
+        static assets that do not have versioned filenames/paths
         """
 
-        app_manifest = {
-            "app.js": "app.f00f00.js",
-        }
+        # non-existant dist file
+        response = self.client.get("/_static/dist/sentry/invalid.js")
+        assert response.status_code == 404, response
 
-        with self.static_asset_manifest(app_manifest):
-            # `get_manifest_url()` should return the mapped filename
-            url = get_manifest_url("sentry", "app.js")
+        dist_path = os.path.join("src", "sentry", "static", "sentry", "dist")
+        os.makedirs(dist_path, exist_ok=True)
 
-            response = self.client.get(url)
-            assert response.status_code == 200, response
-            assert response["Cache-Control"] == FOREVER_CACHE
-            assert response["Vary"] == "Accept-Encoding"
-            assert response["Access-Control-Allow-Origin"] == "*"
-            assert "Content-Encoding" not in response
+        try:
+            with open(os.path.join(dist_path, "test.js"), "a"):
+                url = get_unversioned_asset_url("sentry", "test.js")
 
-            # non-existant dist file
-            response = self.client.get("/_static/dist/sentry/invalid.js")
-            assert response.status_code == 404, response
+                response = self.client.get(url)
+                assert response.status_code == 200, response
+                assert response["Cache-Control"] == NO_CACHE
+                assert response["Vary"] == "Accept-Encoding"
+                assert response["Access-Control-Allow-Origin"] == "*"
+                assert "Content-Encoding" not in response
 
             with override_settings(DEBUG=True):
                 response = self.client.get(url)
                 assert response.status_code == 200, response
                 assert response["Cache-Control"] == NEVER_CACHE
                 assert response["Vary"] == "Accept-Encoding"
                 assert response["Access-Control-Allow-Origin"] == "*"
+        finally:
+            try:
+                os.unlink(os.path.join(dist_path, "test.js"))
+            except Exception:
+                pass
 
     @override_settings(DEBUG=False)
     def test_no_cors(self):