Static checck

Author: KevinMind

docker/nginx/addons.conf

@@ -31,9 +31,11 @@ server {
         # "root /srv" as we mount all files in this directory
         root /srv;
 
+        # NEED TO MAKE SURE WE ONLY SERVE /site-static if DOCKER_TARGET == 'production'
+
         # 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;
+        # else forward to @olympia
+        try_files /site-static/$1 @olympia;
     }
 
     location /user-media/ {

settings.py

@@ -22,9 +22,6 @@
 
 INTERNAL_ROUTES_ALLOWED = True
 
-# Always
-SERVE_STATIC_FILES = True
-
 # These apps are great during development.
 INSTALLED_APPS += ('olympia.landfill',)
 

src/olympia/core/apps.py

@@ -4,6 +4,7 @@
 import subprocess
 import warnings
 from io import StringIO
+from pathlib import Path
 from pwd import getpwnam
 
 from django.apps import AppConfig
@@ -88,10 +89,9 @@ def version_check(app_configs, **kwargs):
 def static_check(app_configs, **kwargs):
     errors = []
     output = StringIO()
-    version = get_version_json()
 
     # We only run this check in production images.
-    if version.get('target') != 'production':
+    if settings.TARGET != 'production':
         return []
 
     try:
@@ -188,54 +188,88 @@ def db_charset_check(app_configs, **kwargs):
 @register(CustomTags.custom_setup)
 def nginx_check(app_configs, **kwargs):
     errors = []
-    version = get_version_json()
 
-    if version.get('target') == 'production':
+    # We only run this check in local environments
+    # Becuase that is the only environment where the local
+    # nginx server is routing requests.
+    if settings.ENV != 'local':
         return []
 
-    configs = [
-        (settings.MEDIA_ROOT, 'http://nginx/user-media'),
-        (settings.STATIC_FILES_PATH, 'http://nginx/static'),
-        (settings.STATIC_ROOT, 'http://nginx/static'),
+    is_production = settings.TARGET == 'production'
+
+    nginx_routing_configs = [
+        {
+            'document_root': settings.MEDIA_ROOT,
+            'url_prefix': 'user-media',
+            # MEDIA_ROOT is available over nginx in all local environments.
+            'served_by': 'nginx',
+        },
+        {
+            'document_root': settings.STATIC_ROOT,
+            'url_prefix': 'static',
+            # STATIC_ROOT is only available over nginx in production images
+            # where we expect collectstatic to have copied files to it.
+            'served_by': 'nginx' if is_production else None,
+        },
+        {
+            'document_root': settings.STATIC_FILES_PATH,
+            'url_prefix': 'static',
+            # STATIC_FILES_PATH is available via olympia static server
+            # in non production images.
+            'served_by': None if is_production else 'olympia',
+        },
     ]
 
-    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)
+    def process_request(config, path):
+        url = f'http://nginx/{config["url_prefix"]}/{path}'
+        response = requests.get(url)
 
-            expected_config = (
-                (response.status_code, 200),
-                (response.text, dir),
-                (response.headers.get('X-Served-By'), 'nginx'),
-            )
+        expected_served_by = config['served_by']
+        document_root = config['document_root']
 
-            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'))
+        expected_status_code = 404 if expected_served_by is None else 200
 
-        except Exception as e:
+        if response.status_code != expected_status_code:
             errors.append(
                 Error(
-                    f'Unknown error accessing {file_path} via {file_url}: {e}',
-                    id='setup.E010',
+                    (
+                        f'Expected {url} serving file from {document_root} '
+                        f'to respond with code {expected_status_code}, '
+                        f'received {response.status_code}'
+                    ),
+                    id='setup.E007',
                 )
             )
+        elif expected_served_by is not None:
+            actual_served_by = response.headers.get('X-Served-By')
+            if actual_served_by != expected_served_by:
+                errors.append(
+                    Error(
+                        (
+                            f'Expected {url} serving file from {document_root} '
+                            f'to be served by {expected_served_by}, '
+                            f'received {actual_served_by}'
+                        ),
+                        id='setup.E007',
+                    )
+                )
+
+    for config in nginx_routing_configs:
+        document_root = config['document_root']
+        file_name = 'test.txt'
+        file_path = Path(document_root) / file_name
 
-    # Always remove the files we created.
-    for file_path in files_to_remove:
-        os.remove(file_path)
+        try:
+            # First expect the request to fail while the path does not exist
+            process_request(config, file_name)
+            # Then create the file and expect the request to succeed
+            with open(file_path, 'w') as f:
+                f.write('test')
+            process_request(config, file_name)
+        except FileNotFoundError:
+            errors.append(Error(f'{document_root} does not exist', id='setup.E007'))
+        finally:
+            file_path.unlink(missing_ok=True)
 
     return errors