Skip to content

Use relative path resolution and manually include static assets in vite's module graph. #23209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Makefile-os
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ override DOCKER_MYSQLD_VOLUME = addons-server_data_mysqld

export FXA_CLIENT_ID ?=
export FXA_CLIENT_SECRET ?=

export STATIC_URL_PREFIX ?= /static-server/
export MEDIA_URL_PREFIX ?= /user-media/
INITIALIZE_ARGS ?=
INIT_CLEAN ?=
INIT_LOAD ?=
Expand Down
7 changes: 5 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ x-env-mapping: &env
- MEMCACHE_LOCATION=memcached:11211
- MYSQL_DATABASE=olympia
- MYSQL_ROOT_PASSWORD=docker
- OLYMPIA_SITE_URL=http://olympia.test
- SITE_URL=http://olympia.test
- PYTHONDONTWRITEBYTECODE=1
- PYTHONUNBUFFERED=1
- PYTHONBREAKPOINT=ipdb.set_trace
Expand All @@ -25,6 +25,8 @@ x-env-mapping: &env
- SKIP_DATA_SEED
- FXA_CLIENT_ID
- FXA_CLIENT_SECRET
- STATIC_URL_PREFIX
- MEDIA_URL_PREFIX

x-olympia: &olympia
<<: *env
Expand Down Expand Up @@ -94,8 +96,9 @@ services:

nginx:
image: nginx
<<: *env
volumes:
- data_nginx:/etc/nginx/conf.d
- data_nginx:/etc/nginx/templates
- ./:/srv
ports:
- "80:80"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ server {
alias /srv/storage/;
}

location ^~ /static/bundle/ {
location ^~ ${STATIC_URL_PREFIX}bundle/ {
proxy_pass http://static;

proxy_set_header Upgrade $http_upgrade;
Expand All @@ -25,7 +25,7 @@ server {
add_header X-Served-By "vite" always;
}

location ~ ^/static/(.*)$ {
location ~ ^${STATIC_URL_PREFIX}(.*)$ {
add_header X-Served-By "nginx" always;

# "root /srv" as we mount all files in this directory
Expand All @@ -36,7 +36,7 @@ server {
try_files /site-static/$1 /static/$1 @olympia;
}

location /user-media/ {
location ${MEDIA_URL_PREFIX} {
alias /srv/storage/shared_storage/uploads/;
add_header X-Served-By "nginx" always;
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
},
"type": "module",
"scripts": {
"dev": "vite -d --strictPort --clearScreen",
"dev": "vite",
"build": "vite build",
"preview": "vite preview",
"test": "vitest run",
Expand Down
8 changes: 5 additions & 3 deletions settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,15 @@ def insert_debug_toolbar_middleware(middlewares):
# replicas to zero.
ES_DEFAULT_NUM_REPLICAS = 0

SITE_URL = os.environ.get('OLYMPIA_SITE_URL') or 'http://localhost:8000'
SITE_URL = env('SITE_URL')

DOMAIN = SERVICES_DOMAIN = urlparse(SITE_URL).netloc
ADDONS_FRONTEND_PROXY_PORT = '7000'
SERVICES_URL = SITE_URL
INTERNAL_SITE_URL = 'http://nginx'
EXTERNAL_SITE_URL = SITE_URL
STATIC_URL = '%s/static/' % EXTERNAL_SITE_URL
MEDIA_URL = '%s/user-media/' % EXTERNAL_SITE_URL
STATIC_URL = EXTERNAL_SITE_URL + STATIC_URL_PREFIX
MEDIA_URL = EXTERNAL_SITE_URL + MEDIA_URL_PREFIX

ALLOWED_HOSTS = ALLOWED_HOSTS + [SERVICES_DOMAIN, 'nginx', '127.0.0.1']

Expand Down Expand Up @@ -159,6 +160,7 @@ def insert_debug_toolbar_middleware(middlewares):

ENABLE_ADMIN_MLBF_UPLOAD = True


# Use dev mode if we are on a non production imqage and debug is enabled.
if get_version_json().get('target') != 'production' and DEBUG:
DJANGO_VITE = {
Expand Down
11 changes: 6 additions & 5 deletions src/olympia/core/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,17 @@ def nginx_check(app_configs, **kwargs):
return []

configs = [
(settings.MEDIA_ROOT, 'http://nginx/user-media'),
(settings.STATIC_FILES_PATH, 'http://nginx/static'),
(settings.STATIC_ROOT, 'http://nginx/static'),
(settings.MEDIA_ROOT, settings.MEDIA_URL_PREFIX),
(settings.STATIC_FILES_PATH, settings.STATIC_URL_PREFIX),
(settings.STATIC_ROOT, settings.STATIC_URL_PREFIX),
]

files_to_remove = []

for dir, base_url in configs:
for dir, prefix in configs:
base_url = f'{settings.INTERNAL_SITE_URL}{prefix}'
file_path = os.path.join(dir, 'test.txt')
file_url = f'{base_url}/test.txt'
file_url = f'{base_url}test.txt'

if not os.path.exists(dir):
errors.append(Error(f'{dir} does not exist', id='setup.E007'))
Expand Down
17 changes: 8 additions & 9 deletions src/olympia/core/tests/test_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import tempfile
from unittest import mock

from django.conf import settings
from django.core.management import call_command
from django.core.management.base import SystemCheckError
from django.test import TestCase
Expand Down Expand Up @@ -199,23 +200,21 @@ def test_nginx_raises_missing_directory(self):
):
call_command('check')

def _test_nginx_response(
self, base_url, status_code=200, response_text='', served_by='nginx'
):
def _test_nginx_response(self, status_code=200, body='', served_by='nginx'):
self.mock_get_version_json.return_value['target'] = 'development'
url = f'{base_url}/test.txt'
url = f'{settings.INTERNAL_SITE_URL}{settings.MEDIA_URL_PREFIX}test.txt'

responses.add(
responses.GET,
url,
status=status_code,
body=response_text,
body=body,
headers={'X-Served-By': served_by},
)

expected_config = (
(status_code, 200),
(response_text, self.media_root),
(body, self.media_root),
(served_by, 'nginx'),
)

Expand All @@ -228,12 +227,12 @@ def _test_nginx_response(

def test_nginx_raises_non_200_status_code(self):
"""Test that files return a 200 status code."""
self._test_nginx_response('http://nginx/user-media', status_code=404)
self._test_nginx_response(status_code=404)

def test_nginx_raises_unexpected_content(self):
"""Test that files return the expected content."""
self._test_nginx_response('http://nginx/user-media', response_text='foo')
self._test_nginx_response(body='foo')

def test_nginx_raises_unexpected_served_by(self):
"""Test that files are served by nginx and not redirected elsewhere."""
self._test_nginx_response('http://nginx/user-media', served_by='wow')
self._test_nginx_response(served_by='wow')
26 changes: 16 additions & 10 deletions src/olympia/lib/settings_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,20 @@ def get_db_config(environ_var, atomic_requests=True):
PROD_STATIC_URL = 'https://addons.mozilla.org/static-server/'
PROD_MEDIA_URL = 'https://addons.mozilla.org/user-media/'

# Static
STATIC_ROOT = path('site-static')
# Allow overriding static/media url path prefix
STATIC_URL_PREFIX = env('STATIC_URL_PREFIX', default='/static-server/')
MEDIA_URL_PREFIX = env('MEDIA_URL_PREFIX', default='/user-media/')
# URL that handles the media served from MEDIA_ROOT. Make sure to use a
# trailing slash if there is a path component (optional in other cases).
# Examples: "http://media.lawrence.com", "http://example.com/media/"
MEDIA_URL = MEDIA_URL_PREFIX
# URL that handles the static files served from STATIC_ROOT. Make sure to use a
# trailing slash if there is a path component (optional in other cases).
# Examples: "http://media.lawrence.com", "http://example.com/static/"
STATIC_URL = STATIC_URL_PREFIX

# Filter IP addresses of allowed clients that can post email through the API.
ALLOWED_CLIENTS_EMAIL_API = env.list('ALLOWED_CLIENTS_EMAIL_API', default=[])
# Auth token required to authorize inbound email.
Expand All @@ -252,11 +266,6 @@ def get_db_config(environ_var, atomic_requests=True):
# Domain emails should be sent to.
INBOUND_EMAIL_DOMAIN = env('INBOUND_EMAIL_DOMAIN', default=DOMAIN)

# URL that handles the media served from MEDIA_ROOT. Make sure to use a
# trailing slash if there is a path component (optional in other cases).
# Examples: "http://media.lawrence.com", "http://example.com/media/"
MEDIA_URL = '/user-media/'

# Tarballs in DUMPED_APPS_PATH deleted 30 days after they have been written.
DUMPED_APPS_DAYS_DELETE = 3600 * 24 * 30

Expand Down Expand Up @@ -288,7 +297,7 @@ def get_db_config(environ_var, atomic_requests=True):
'statistics',
'services',
'sitemap.xml',
'static',
STATIC_URL_PREFIX.strip('/'),
'update',
'user-media',
'__heartbeat__',
Expand All @@ -309,7 +318,7 @@ def get_db_config(environ_var, atomic_requests=True):
'services',
'sitemap.xml',
'downloads',
'static',
STATIC_URL_PREFIX.strip('/'),
'update',
'user-media',
'__heartbeat__',
Expand Down Expand Up @@ -1176,9 +1185,6 @@ def read_only_mode(env):
'auth_mechanism': 'PLAIN',
}

# Static
STATIC_ROOT = path('site-static')
STATIC_URL = '/static/'

STATICFILES_FINDERS = (
'django.contrib.staticfiles.finders.FileSystemFinder',
Expand Down
11 changes: 8 additions & 3 deletions src/olympia/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ def serve_javascript_catalog(request, locale, **kwargs):
# Remove leading and trailing slashes so the regex matches.
media_url = settings.MEDIA_URL.lstrip('/').rstrip('/')

static_url_prefix = settings.STATIC_URL_PREFIX.lstrip('/').rstrip('/')

urlpatterns.extend(
[
re_path(
Expand All @@ -141,19 +143,22 @@ def serve_javascript_catalog(request, locale, **kwargs):
),
# Serve javascript catalog locales bundle directly from django
re_path(
r'^static/js/i18n/(?P<locale>\w{2,3}(?:-\w{2,6})?)\.js$',
(
r'^%s/js/i18n/(?P<locale>\w{2,3}(?:-\w{2,6})?)\.js$'
% static_url_prefix
),
serve_javascript_catalog,
name='javascript-catalog',
),
# Serve swagger UI JS directly from django
re_path(
r'^static/js/swagger/(?P<version>[^/]+)\.js$',
r'^%s/js/swagger/(?P<version>[^/]+)\.js$' % static_url_prefix,
serve_swagger_ui_js,
),
# fallback for static files that are not available directly over nginx.
# Mostly vendor files from python or npm dependencies that are not available
# in the static files directory.
re_path(r'^static/(?P<path>.*)$', serve_static_files),
re_path(r'^%s/(?P<path>.*)$' % static_url_prefix, serve_static_files),
]
)

Expand Down
2 changes: 1 addition & 1 deletion tests/make/make.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('docker-compose.yml', () => {
// mapping for nginx conf.d adding addon-server routing
expect.objectContaining({
source: 'data_nginx',
target: '/etc/nginx/conf.d',
target: '/etc/nginx/templates',
}),
// mapping for /data/olympia/ directory to /srv
expect.objectContaining({
Expand Down
31 changes: 21 additions & 10 deletions vite.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,29 @@ const jqueryGlobals = {
jQuery: 'jquery',
};

const env = (name) => {
const value = process.env[name];
if (!value) {
throw new Error(`${name} is not defined`);
}
return value;
};
const env = (name, defaultValue) =>
process.env[name] || defaultValue || new Error(`${name} is not defined`);

export default defineConfig(({ command }) => {
const isLocal = env('ENV') === 'local';
const isDev = command === 'serve';

const baseConfig = {
// Ensure all static assets are treated as
// 'in-scope' assets that can be tracked by vite
// this ensures any imported static assets are correctly
// mapped across file transformations.
assetsInclude: `${INPUT_DIR}/*`,
strict: true,
root: resolve(INPUT_DIR),
debug: env('DEBUG', false),
// In dev mode, prefix 'bundle' to static file URLs
// so that nginx knows to forward the request to the vite
// dev server instead of serving from static files or olympia
base: '/static/',
// Use a relative path during the build
// this ensures that import paths can be transformed
// independently of where the importing file ends up in the bundle
base: './',
resolve: {
alias: {
// Alias 'highcharts' to our local vendored copy
Expand All @@ -62,6 +66,11 @@ export default defineConfig(({ command }) => {
}),
],
build: {
// Disable inline assets. For performance reasons we want to serve
// all static assets from dedicated URLs which in production will
// be cached in our CDN. The incremental build size is worse than
// the increase in number of requests.
assetsInlineLimit: 0,
// This value should be kept in sync with settings_base.py
// which determines where to read static file paths
// for production URL resolution
Expand Down Expand Up @@ -106,7 +115,6 @@ export default defineConfig(({ command }) => {
preprocessorOptions: {
less: {
math: 'always',
// relativeUrls: true,
javascriptEnabled: true,
},
},
Expand All @@ -126,12 +134,15 @@ export default defineConfig(({ command }) => {
if (isDev) {
// In dev mode, add the bundle path to direct
// static requests to the vite dev server via nginx
baseConfig.base += 'bundle/';
baseConfig.base = `${env('STATIC_URL_PREFIX')}bundle/`;
// Configure the dev server in dev mode
baseConfig.server = {
host: true,
port: 5173,
allowedHosts: true,
origin: env('SITE_URL'),
strictPort: true,
clearScreen: true,
};
}

Expand Down
Loading