diff --git a/Makefile-docker b/Makefile-docker index 91ff05f2d683..f49394883600 100644 --- a/Makefile-docker +++ b/Makefile-docker @@ -10,14 +10,6 @@ APP=src/olympia/ NODE_MODULES := $(NPM_CONFIG_PREFIX)node_modules/ -REQUIRED_FILES := \ - Makefile \ - Makefile-os \ - Makefile-docker \ - /deps/package.json \ - /deps/package-lock.json \ - /addons-server-docker-container \ - # Build list of dependencies to install DEPS = pip prod # If we're running a development image, then we should install the development dependencies @@ -39,29 +31,12 @@ check_pip_packages: ## check the existence of multiple python packages ./scripts/check_pip_packages.sh $$dep.txt; \ done -.PHONY: check_files -check_files: ## check the existence of multiple files - @for file in $(REQUIRED_FILES); do test -f "$$file" || (echo "$$file is missing." && exit 1); done - @echo "All required files are present." - -.PHONY: check_olympia_user -check_olympia_user: ## check if the olympia user exists and is current user - @if [ "$$(id -u olympia)" != "$$(id -u)" ]; then echo "The current user is not the olympia user."; exit 1; fi - @echo "The current user is the olympia user." - .PHONY: check_django check_django: ## check if the django app is configured properly ./manage.py check -.PHONY: check_nginx -check_nginx: ## check if the nginx config for local development is configured properly - mkdir -p /data/olympia/storage/shared_storage/uploads - echo "OK" > /data/olympia/storage/shared_storage/uploads/.check - @if [ "$$(curl -sf http://nginx/user-media/.check)" != "OK" ]; then echo "Requesting http://nginx/user-media/.check failed"; exit 1; fi - @echo "Nginx user-media configuration looks correct." - .PHONY: check -check: check_nginx check_files check_olympia_user check_debian_packages check_pip_packages check_django +check: check_debian_packages check_pip_packages check_django .PHONY: data_dump data_dump: diff --git a/docker/nginx/addons.conf b/docker/nginx/addons.conf index 97eb1678f1d0..89f00d702fcc 100644 --- a/docker/nginx/addons.conf +++ b/docker/nginx/addons.conf @@ -10,17 +10,20 @@ server { alias /srv/storage/; } - location /static/ { - alias /srv/static/; + location ~ ^/static/(.*)$ { + add_header X-Served-By "nginx" always; - # Fallback to the uwsgi server if the file is not found in the static files directory. - # This will happen for vendor files from pytnon or npm dependencies that won't be available - # in the static files directory. - error_page 404 = @olympia; + # "root /srv" as we mount all files in this directory + root /srv; + + # If it doesn't exist under /srv/site-static/$1, + # fall back to /srv/static/$1, else forward to @olympia + try_files /site-static/$1 /static/$1 @olympia; } location /user-media/ { alias /srv/storage/shared_storage/uploads/; + add_header X-Served-By "nginx" always; } location ~ ^/api/ { @@ -50,6 +53,8 @@ server { uwsgi_param X-Real-IP $remote_addr; uwsgi_param X-Forwarded-For $proxy_add_x_forwarded_for; uwsgi_param X-Forwarded-Protocol ssl; + + add_header X-Served-By "olympia" always; } location @frontendamo { diff --git a/src/olympia/amo/management/__init__.py b/src/olympia/amo/management/__init__.py index 1ba869ecaab2..d1a4328e6fe7 100644 --- a/src/olympia/amo/management/__init__.py +++ b/src/olympia/amo/management/__init__.py @@ -211,15 +211,18 @@ def make_dir(self, name: str, force: bool = False) -> None: os.makedirs(path, exist_ok=True) - def _clean_storage(self, root: str, dir_dict: dict[str, str | dict]) -> None: + def _clean_storage( + self, root: str, dir_dict: dict[str, str | dict], clean: bool = False + ) -> None: for key, value in dir_dict.items(): curr_path = os.path.join(root, key) if isinstance(value, dict): - self._clean_storage(curr_path, value) + self._clean_storage(curr_path, value, clean=clean) else: - shutil.rmtree(curr_path, ignore_errors=True) + if clean: + shutil.rmtree(curr_path, ignore_errors=True) os.makedirs(curr_path, exist_ok=True) - def clean_storage(self): - self.logger.info('Cleaning storage...') - self._clean_storage(settings.STORAGE_ROOT, storage_structure) + def make_storage(self, clean: bool = False): + self.logger.info('Making storage...') + self._clean_storage(settings.STORAGE_ROOT, storage_structure, clean=clean) diff --git a/src/olympia/amo/management/commands/data_load.py b/src/olympia/amo/management/commands/data_load.py index 4973409f63c6..70969226f6c5 100644 --- a/src/olympia/amo/management/commands/data_load.py +++ b/src/olympia/amo/management/commands/data_load.py @@ -38,7 +38,7 @@ def handle(self, *args, **options): raise CommandError(f'Storage backup not found: {storage_path}') cache.clear() - self.clean_storage() + self.make_storage(clean=True) call_command( 'mediarestore', diff --git a/src/olympia/amo/management/commands/data_seed.py b/src/olympia/amo/management/commands/data_seed.py index 33078c1f5e05..00a673a51e8c 100644 --- a/src/olympia/amo/management/commands/data_seed.py +++ b/src/olympia/amo/management/commands/data_seed.py @@ -22,7 +22,7 @@ def handle(self, *args, **options): # Delete any local storage files # This should happen after we reset the database to ensure any records # relying on storage are deleted. - self.clean_storage() + self.make_storage(clean=True) # Migrate the database call_command('migrate', '--noinput') diff --git a/src/olympia/amo/management/commands/initialize.py b/src/olympia/amo/management/commands/initialize.py index 413b478ea388..620bde565c1a 100644 --- a/src/olympia/amo/management/commands/initialize.py +++ b/src/olympia/amo/management/commands/initialize.py @@ -73,6 +73,9 @@ def handle(self, *args, **options): # so our database tables should be accessible call_command('monitors', services=['database']) + # Ensure that the storage directories exist. + self.make_storage(clean=False) + # Ensure any additional required dependencies are available before proceeding. call_command( 'monitors', diff --git a/src/olympia/amo/tests/test_commands.py b/src/olympia/amo/tests/test_commands.py index 5bbaed663be5..ff7368dea6a7 100644 --- a/src/olympia/amo/tests/test_commands.py +++ b/src/olympia/amo/tests/test_commands.py @@ -489,7 +489,7 @@ def test_make_dir_non_existing_path(self, mock_makedirs, mock_exists): @mock.patch('olympia.amo.management.shutil.rmtree') @mock.patch('olympia.amo.management.os.makedirs') def test_clean_storage(self, mock_makedirs, mock_rmtree): - self.base_data_command.clean_storage() + self.base_data_command.make_storage(clean=True) def walk_keys(root, dir_dict): for key, value in dir_dict.items(): @@ -650,8 +650,8 @@ def setUp(self): 'olympia.amo.management.commands.data_seed.BaseDataCommand.clean_dir', ), ( - 'mock_clean_storage', - 'olympia.amo.management.commands.data_seed.BaseDataCommand.clean_storage', + 'mock_make_storage', + 'olympia.amo.management.commands.data_seed.BaseDataCommand.make_storage', ), ) @@ -668,7 +668,7 @@ def test_default(self): self.mocks['mock_clean_dir'].assert_called_once_with( self.base_data_command.data_backup_init ) - self.mocks['mock_clean_storage'].assert_called_once() + self.mocks['mock_make_storage'].assert_called_once() self._assert_commands_called_in_order( self.mocks['mock_call_command'], diff --git a/src/olympia/core/apps.py b/src/olympia/core/apps.py index 879b30868248..0a066020137e 100644 --- a/src/olympia/core/apps.py +++ b/src/olympia/core/apps.py @@ -13,6 +13,8 @@ from django.db import connection from django.utils.translation import gettext_lazy as _ +import requests + from olympia.core.utils import REQUIRED_VERSION_KEYS, get_version_json @@ -93,16 +95,10 @@ def static_check(app_configs, **kwargs): try: call_command('compress_assets', dry_run=True, stdout=output) - file_paths = output.getvalue().strip().split('\n') + stripped_output = output.getvalue().strip() - if not file_paths: - errors.append( - Error( - 'No compressed asset files were found.', - id='setup.E003', - ) - ) - else: + if stripped_output: + file_paths = stripped_output.split('\n') for file_path in file_paths: if not os.path.exists(file_path): error = f'Compressed asset file does not exist: {file_path}' @@ -112,6 +108,14 @@ def static_check(app_configs, **kwargs): id='setup.E003', ) ) + else: + errors.append( + Error( + 'No compressed asset files were found.', + id='setup.E003', + ) + ) + except CommandError as e: errors.append( Error( @@ -151,6 +155,55 @@ def db_charset_check(app_configs, **kwargs): return errors +@register(CustomTags.custom_setup) +def nginx_check(app_configs, **kwargs): + errors = [] + version = get_version_json() + + if version.get('target') == 'production': + return [] + + configs = [ + (settings.MEDIA_ROOT, 'http://nginx/user-media'), + (settings.STATIC_ROOT, 'http://nginx/static'), + (settings.STATIC_FILES_PATH, 'http://nginx/static'), + ] + + for dir, base_url in configs: + file_path = os.path.join(dir, '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')) + + try: + open(file_path, 'w').close() + response = requests.get(file_url) + + expected_config = ( + (response.status_code, 200), + (response.text, ''), + (response.headers.get('X-Served-By'), 'nginx'), + ) + + if any(item[0] != item[1] for item in expected_config): + message = f'Failed to access {file_url}. {expected_config}' + errors.append(Error(message, id='setup.E008')) + + except Exception as e: + errors.append( + Error( + f'Unknown error accessing {file_path} via {file_url}: {e}', + id='setup.E010', + ) + ) + finally: + if os.path.exists(file_path): + os.remove(file_path) + + return errors + + class CoreConfig(AppConfig): name = 'olympia.core' verbose_name = _('Core') diff --git a/src/olympia/core/tests/test_apps.py b/src/olympia/core/tests/test_apps.py index bb4c8b6d1ffe..fe0641cf6fd3 100644 --- a/src/olympia/core/tests/test_apps.py +++ b/src/olympia/core/tests/test_apps.py @@ -1,10 +1,14 @@ +import os +import tempfile from unittest import mock -from django.core.management import call_command +from django.core.management import CommandError, call_command from django.core.management.base import SystemCheckError from django.test import TestCase from django.test.utils import override_settings +import responses + from olympia.core.utils import REQUIRED_VERSION_KEYS @@ -20,11 +24,25 @@ def setUp(self): } patch = mock.patch( 'olympia.core.apps.get_version_json', - return_value=self.default_version_json, ) self.mock_get_version_json = patch.start() + self.mock_get_version_json.return_value = self.default_version_json self.addCleanup(patch.stop) + mock_dir = tempfile.mkdtemp(prefix='static-root') + self.fake_css_file = os.path.join(mock_dir, 'foo.css') + with open(self.fake_css_file, 'w') as f: + f.write('body { background: red; }') + + patch_command = mock.patch('olympia.core.apps.call_command') + self.mock_call_command = patch_command.start() + self.mock_call_command.side_effect = ( + lambda command, dry_run, stdout: stdout.write(f'{self.fake_css_file}\n') + ) + self.addCleanup(patch_command.stop) + + self.media_root = tempfile.mkdtemp(prefix='media-root') + @mock.patch('olympia.core.apps.connection.cursor') def test_db_charset_check(self, mock_cursor): mock_cursor.return_value.__enter__.return_value.fetchone.return_value = ( @@ -85,3 +103,115 @@ def test_illegal_override_uid_check(self, mock_getpwnam): with override_settings(HOST_UID=1000): call_command('check') + + def test_static_check_no_assets_found(self): + """ + Test static_check fails if compress_assets reports no files. + """ + self.mock_get_version_json.return_value['target'] = 'production' + # Simulate "compress_assets" returning no file paths. + self.mock_call_command.side_effect = ( + lambda command, dry_run, stdout: stdout.write('') + ) + with self.assertRaisesMessage( + SystemCheckError, 'No compressed asset files were found.' + ): + call_command('check') + + @mock.patch('os.path.exists') + def test_static_check_missing_assets(self, mock_exists): + """ + Test static_check fails if at least one specified compressed + asset file does not exist. + """ + self.mock_get_version_json.return_value['target'] = 'production' + # Simulate "compress_assets" returning a couple of files. + self.mock_call_command.side_effect = ( + lambda command, dry_run, stdout: stdout.write( + f'{self.fake_css_file}\nfoo.js\n' + ) + ) + # Pretend neither file exists on disk. + mock_exists.return_value = False + + with self.assertRaisesMessage( + SystemCheckError, + # Only the first missing file triggers the AssertionError message check + 'Compressed asset file does not exist: foo.js', + ): + call_command('check') + + def test_static_check_command_error(self): + """ + Test static_check fails if there's an error during compress_assets. + """ + self.mock_get_version_json.return_value['target'] = 'production' + self.mock_call_command.side_effect = CommandError('Oops') + with self.assertRaisesMessage( + SystemCheckError, 'Error running compress_assets command: Oops' + ): + call_command('check') + + def test_static_check_command_success(self): + """ + Test static_check succeeds if compress_assets runs without errors. + """ + self.mock_get_version_json.return_value['target'] = 'production' + self.mock_call_command.side_effect = ( + lambda command, dry_run, stdout: stdout.write(f'{self.fake_css_file}\n') + ) + call_command('check') + + def test_nginx_skips_check_on_production_target(self): + fake_media_root = '/fake/not/real' + with override_settings(MEDIA_ROOT=fake_media_root): + call_command('check') + + def test_nginx_raises_missing_directory(self): + self.mock_get_version_json.return_value['target'] = 'development' + fake_media_root = '/fake/not/real' + with override_settings(MEDIA_ROOT=fake_media_root): + with self.assertRaisesMessage( + SystemCheckError, + f'{fake_media_root} does not exist', + ): + call_command('check') + + def _test_nginx_response( + self, base_url, status_code=200, response_text='', served_by='nginx' + ): + self.mock_get_version_json.return_value['target'] = 'development' + url = f'{base_url}/test.txt' + + responses.add( + responses.GET, + url, + status=status_code, + body=response_text, + headers={'X-Served-By': served_by}, + ) + + expected_config = ( + (status_code, 200), + (response_text, ''), + (served_by, 'nginx'), + ) + + with override_settings(MEDIA_ROOT=self.media_root): + with self.assertRaisesMessage( + SystemCheckError, + f'Failed to access {url}. {expected_config}', + ): + call_command('check') + + 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) + + 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') + + 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') diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py index a06c2c1ce663..fd52e15cf4ba 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -1333,9 +1333,10 @@ def read_only_mode(env): NODE_PACKAGE_MANAGER_INSTALL_OPTIONS = ['--dry-run'] STATIC_BUILD_PATH = path('static-build') +STATIC_FILES_PATH = path('static') STATICFILES_DIRS = ( - path('static'), + STATIC_FILES_PATH, STATIC_BUILD_PATH, )