Add test for static file loading

Makefile-docker

@@ -33,15 +33,8 @@ check_pip_packages: ## check the existence of multiple python packages
 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_debian_packages check_pip_packages check_django
+check: check_debian_packages check_pip_packages check_django
 
 .PHONY: data_dump
 data_dump:

docker-compose.yml

@@ -46,9 +46,6 @@ services:
     volumes:
       # 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
   worker:
     <<: *olympia
     command: [
@@ -64,7 +61,6 @@ services:
     ]
     volumes:
       - ${HOST_MOUNT_SOURCE:?}:/data/olympia
-      - ${HOST_MOUNT_SOURCE:?}storage:/data/olympia/storage
     extra_hosts:
      - "olympia.test:127.0.0.1"
     restart: on-failure:5
@@ -92,19 +88,12 @@ services:
       interval: 30s
       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
 
   nginx:
     image: nginx
     volumes:
       - data_nginx:/etc/nginx/conf.d
       - ${HOST_MOUNT_SOURCE:?}:/srv
-      - data_site_static:/srv/static
-      - ${HOST_MOUNT_SOURCE:?}storage:/srv/storage
     ports:
       - "80:80"
     networks:
@@ -203,10 +192,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>
@@ -216,8 +201,6 @@ volumes:
   # it will map to the current directory ./<name>
   # (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

@@ -11,13 +11,14 @@ server {
     }
 
     location /static/ {
-        root /srv;
+        alias /srv/site-static/;
         try_files $uri @olympia;
-        add_header X-Served-By "nginx-direct" always;
+        add_header X-Served-By "nginx" always;
     }
 
     location /user-media/ {
         alias /srv/storage/shared_storage/uploads/;
+        add_header X-Served-By "nginx" always;
     }
 
     location ~ ^/api/ {

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

@@ -68,6 +68,7 @@ def handle(self, *args, **options):
                     call_command(
                         'reindex', '--wipe', '--force', '--noinput', '--skip-if-exists'
                     )
+                    self.make_storage(clean=False)
 
             # By now, we excpect the database to exist, and to be migrated
             # so our database tables should be accessible

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():

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
 
 
@@ -151,6 +153,97 @@ 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 = [
+        {
+            'dir': settings.MEDIA_ROOT,
+            'base_url': 'http://nginx/user-media/',
+            'source': 'nginx',
+        },
+        {
+            'dir': settings.STATIC_ROOT,
+            'base_url': 'http://nginx/static/',
+            'source': 'nginx',
+        },
+        {
+            'dir': settings.STATIC_FILES_PATH,
+            'base_url': 'http://nginx/static/',
+            'source': 'olympia',
+        },
+    ]
+
+    for config in configs:
+        dir = config['dir']
+        base_url = config['base_url']
+        source = config['source']
+
+        # Make a test file in the path and check it is accessible via the static url
+        test_file_name = 'test.txt'
+        test_file_content = 'test'
+        test_file_path = os.path.join(dir, test_file_name)
+
+        if not os.path.exists(dir):
+            errors.append(
+                Error(
+                    f'{dir} does not exist',
+                    id='setup.E007',
+                )
+            )
+
+        try:
+            with open(test_file_path, 'w') as f:
+                f.write(test_file_content)
+
+            test_file_url = f'{base_url}{test_file_name}'
+            response = requests.get(test_file_url)
+
+            if response.status_code != 200:
+                errors.append(
+                    Error(
+                        f'Failed to access {test_file_url} '
+                        f'with status code {response.status_code}',
+                        id='setup.E008',
+                    )
+                )
+
+            if response.text != test_file_content:
+                errors.append(
+                    Error(
+                        f'Unexpected content in {test_file_url}, "{response.text}"',
+                        id='setup.E009',
+                    )
+                )
+
+            if response.headers.get('X-Served-By') != source:
+                errors.append(
+                    Error(
+                        f'Expected file to be served by {source} '
+                        f'recieved {response.headers["X-Served-By"]}',
+                        id='setup.E010',
+                    )
+                )
+
+        except Exception as e:
+            errors.append(
+                Error(
+                    f'Unknown error accessing {config}: {e}',
+                    id='setup.E010',
+                )
+            )
+        finally:
+            if os.path.exists(test_file_path):
+                os.remove(test_file_path)
+
+    return errors
+
+
 class CoreConfig(AppConfig):
     name = 'olympia.core'
     verbose_name = _('Core')

src/olympia/lib/settings_base.py

@@ -73,7 +73,6 @@ def path(*folders):
 
 # Host info that is hard coded for production images.
 HOST_UID = None
-HOST_MOUNT = None
 
 # Used to determine if django should serve static files.
 # For local deployments we want nginx to proxy static file requests to the
@@ -1319,6 +1318,7 @@ def read_only_mode(env):
 }
 
 # Static
+# mounted from the host.
 STATIC_ROOT = path('site-static')
 STATIC_URL = '/static/'
 
@@ -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,
 )
 

tests/make/make.spec.js

@@ -161,7 +161,7 @@ describe('docker-compose.yml', () => {
             source: 'data_nginx',
             target: '/etc/nginx/conf.d',
           }),
-          // mapping for local host directory to /data/olympia
+          // mapping for /data/olympia/ directory to /srv
           expect.objectContaining({
             source: isProdMountTarget ? 'data_olympia_' : expect.any(String),
             target: '/srv',
@@ -259,16 +259,13 @@ describe('docker-compose.yml', () => {
   const failKeys = [
     // Invalid docker tag leads to docker not parsing the image
     'DOCKER_TAG',
-    // Value is read directly as the volume source for /data/olympia and must be valid
-    'HOST_MOUNT_SOURCE',
   ];
   const ignoreKeys = [
     // Ignored because these values are explicitly mapped to the host_* values
     'OLYMPIA_UID',
     'OLYMPIA_MOUNT',
     // Ignored because the HOST_UID is always set to the host user's UID
     'HOST_UID',
-    'HOST_MOUNT',
   ];
   const defaultEnv = runSetup();
   const customValue = 'custom';