Better checks that static file routing works as expected

- Removed obsolete file checks and user validation from Makefile-docker.
- Updated Nginx configuration to improve static file handling and added headers for better traceability.
- Refactored storage management commands in the Python codebase:
  - Renamed `clean_storage` to `make_storage` for clarity and added a `clean` parameter.
  - Updated command implementations to use the new `make_storage` method.
- Introduced a new system check for Nginx configurations to ensure proper file accessibility and response validation.

.gitignore

@@ -55,5 +55,3 @@ tmp/*
 # do not ignore the following files
 !docker-compose.private.yml
 !private/README.md
-!storage/.gitignore
-!deps/.gitkeep

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:

docker-compose.yml

@@ -47,8 +47,8 @@ services:
       # used by: web, worker, nginx
       - ${HOST_MOUNT_SOURCE:?}:/data/olympia
       - ${HOST_MOUNT_SOURCE:?}deps:/deps
-      - data_site_static:/data/olympia/site-static
-      - ${HOST_MOUNT_SOURCE:?}storage:/data/olympia/storage
+      - ./site-static:/data/olympia/site-static
+      - ./storage:/data/olympia/storage
   worker:
     <<: *olympia
     command: [
@@ -65,7 +65,7 @@ services:
     volumes:
       - ${HOST_MOUNT_SOURCE:?}:/data/olympia
       - ${HOST_MOUNT_SOURCE:?}deps:/deps
-      - ${HOST_MOUNT_SOURCE:?}storage:/data/olympia/storage
+      - ./storage:/data/olympia/storage
     extra_hosts:
      - "olympia.test:127.0.0.1"
     restart: on-failure:5
@@ -94,18 +94,16 @@ services:
       retries: 3
       start_interval: 1s
     volumes:
-      # Don't mount generated files. They only exist in the container
-      # and would otherwiser be deleted by mounting the cwd volume above
-      - data_static_build:/data/olympia/static-build
-      - data_site_static:/data/olympia/site-static
+      - ./static-build:/data/olympia/static-build
+      - ./site-static:/data/olympia/site-static
 
   nginx:
     image: nginx
     volumes:
       - data_nginx:/etc/nginx/conf.d
       - ${HOST_MOUNT_SOURCE:?}:/srv
-      - data_site_static:/srv/site-static
-      - ${HOST_MOUNT_SOURCE:?}storage:/srv/storage
+      - ./site-static:/srv/site-static
+      - ./storage:/srv/storage
     ports:
       - "80:80"
     networks:
@@ -204,10 +202,6 @@ networks:
     enable_ipv6: false
 
 volumes:
-  # Volumes for static files that should not be
-  # mounted from the host.
-  data_static_build:
-  data_site_static:
   # Volumes for the production olympia mounts
   # allowing to conditionally mount directories
   # from the host or from the image to <path>
@@ -218,7 +212,6 @@ volumes:
   # (data_olympia_)<name>:/<path>
   data_olympia_:
   data_olympia_deps:
-  data_olympia_storage:
   # Volume for rabbitmq/redis to avoid anonymous volumes
   data_rabbitmq:
   data_redis:

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 {

scripts/setup.py

@@ -3,8 +3,13 @@
 import os
 
 
+root = os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))
+
+env_path = os.path.join(root, '.env')
+
+
 def set_env_file(values):
-    with open('.env', 'w') as f:
+    with open(env_path, 'w') as f:
         print('Environment:')
         for key, value in values.items():
             f.write(f'{key}="{value}"\n')
@@ -14,8 +19,8 @@ def set_env_file(values):
 def get_env_file():
     env = {}
 
-    if os.path.exists('.env'):
-        with open('.env', 'r') as f:
+    if os.path.exists(env_path):
+        with open(env_path, 'r') as f:
             for line in f:
                 key, value = line.strip().split('=', 1)
                 env[key] = value.strip('"')
@@ -143,6 +148,10 @@ def main():
         }
     )
 
+    # Create the directories that are expected to exist in the container.
+    for dir in ['deps', 'site-static', 'static-build', 'storage']:
+        os.makedirs(os.path.join(root, dir), exist_ok=True)
+
 
 if __name__ == '__main__':
     main()

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)

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',

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')
 

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',

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'],

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,61 @@ 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_FILES_PATH, 'http://nginx/static'),
+        (settings.STATIC_ROOT, 'http://nginx/static'),
+    ]
+
+    files_to_remove = []
+
+    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:
+            with open(file_path, 'w') as f:
+                f.write(dir)
+
+            files_to_remove.append(file_path)
+            response = requests.get(file_url)
+
+            expected_config = (
+                (response.status_code, 200),
+                (response.text, dir),
+                (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',
+                )
+            )
+
+    # Always remove the files we created.
+    for file_path in files_to_remove:
+        os.remove(file_path)
+
+    return errors
+
+
 class CoreConfig(AppConfig):
     name = 'olympia.core'
     verbose_name = _('Core')