CI tests for make check to prevent breakage to our local dev:

- Removed volume definitions for `storage` in `docker-compose.ci.yml`.
- Added `storage` volume mapping in `docker-compose.yml` for the `web` and `nginx` services.
- Updated `Makefile-docker` to simplify the Django check command.
- Introduced a new GitHub Actions workflow `_test_check.yml` for testing local development setup.
- Cleaned up the existing CI workflow by removing redundant steps and integrating the new test workflow.
- Enhanced tests in `make.spec.js` to validate Docker Compose configurations.

This commit improves the organization of Docker configurations and enhances CI testing capabilities.

.github/workflows/_test.yml

@@ -65,12 +65,6 @@ jobs:
             services: web
             compose_file: docker-compose.yml:docker-compose.ci.yml
             run: make lint-codestyle
-          -
-            name: Manage Check
-            services: web nginx
-            compose_file: docker-compose.yml:docker-compose.ci.yml
-            run: make check
-            data_backup_skip: true
     steps:
       - uses: actions/checkout@v4
       - name: Test (${{ matrix.name }})
@@ -81,4 +75,3 @@ jobs:
           services: ${{ matrix.services }}
           compose_file: ${{ matrix.compose_file }}
           run: ${{ matrix.run }}
-          data_backup_skip: ${{ matrix.data_backup_skip || 'true' }}

.github/workflows/_test_check.yml

@@ -0,0 +1,144 @@
+name: Test make up and check the local dev setup
+
+run-name: |
+  ref: ${{ github.ref_name }} |
+  version: ${{ inputs.version }} |
+  digest: ${{ inputs.digest }} |
+
+on:
+  workflow_call:
+    inputs:
+      version:
+        description: The version of the image to run
+        type: string
+        required: true
+      digest:
+        description: The build digest of the image to run. Overrides version.
+        type: string
+        required: false
+  workflow_dispatch:
+    inputs:
+      version:
+        description: The version of the image to run
+        type: string
+        required: true
+      digest:
+        description: The build digest of the image to run. Overrides version.
+        type: string
+        required: false
+
+concurrency:
+  group: test_check-${{ github.workflow }}-${{ github.event_name}}-${{ github.ref}}-${{ toJson(inputs) }}
+  cancel-in-progress: true
+
+jobs:
+  context:
+    runs-on: ubuntu-latest
+    outputs:
+      is_fork: ${{ steps.context.outputs.is_fork }}
+    steps:
+      - uses: actions/checkout@v4
+      - id: context
+        uses: ./.github/actions/context
+
+  test_check:
+    runs-on: ubuntu-latest
+    name: |
+      version: '${{ matrix.version }}' |
+      compose_file: '${{ matrix.compose_file }}'
+    strategy:
+      fail-fast: false
+      matrix:
+        version:
+          - local
+          - ${{ inputs.version }}
+        compose_file:
+          - docker-compose.yml
+          - docker-compose.yml:docker-compose.ci.yml
+    steps:
+      - uses: actions/checkout@v4
+      - shell: bash
+        continue-on-error: true
+        run: |
+          cat <<EOF
+            Values passed to the action:
+            version: ${{ matrix.version }}
+            compose_file: ${{ matrix.compose_file }}
+          EOF
+      - uses: ./.github/actions/run-docker
+        # Set environment variables that are expected to be ignored
+        env:
+          DOCKER_COMMIT: 'not-expected'
+          DOCKER_VERSION: 'not-expected'
+        with:
+          version: ${{ matrix.version }}
+          compose_file: ${{ matrix.compose_file }}
+          run: make check
+
+  test_make_docker_configuration:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v2
+      - name: Install dependencies
+        shell: bash
+        run: npm ci
+      - name: Check make/docker configuration
+        shell: bash
+        run: |
+          docker compose version
+          make test_setup
+      - name: Test setup
+        uses: ./.github/actions/run-docker
+        with:
+          digest: ${{ inputs.digest }}
+          version: ${{ inputs.version }}
+          run: |
+            pytest tests/make/
+
+  test_run_docker_action:
+    runs-on: ubuntu-latest
+    needs: context
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Create failure
+        id: failure
+        continue-on-error: true
+        uses: ./.github/actions/run-docker
+        with:
+          digest: ${{ inputs.digest }}
+          version: ${{ inputs.version }}
+          logs: true
+          run: |
+            exit 1
+
+      - name: Verify failure
+        if: always()
+        run: |
+          if [[ "${{ steps.failure.outcome }}" != "failure" ]]; then
+            echo "Expected failure"
+            exit 1
+          fi
+
+      - name: Check (special characters in command)
+        uses: ./.github/actions/run-docker
+        with:
+          digest: ${{ inputs.digest }}
+          version: ${{ inputs.version }}
+          run: |
+            echo 'this is a question?'
+            echo 'a * is born'
+            echo 'wow an array []'
+
+  test_migrations:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ./.github/actions/run-docker
+        with:
+          version: ${{ inputs.version }}
+          data_backup_skip: false
+          run: echo true

.github/workflows/ci.yml

@@ -75,97 +75,6 @@ jobs:
           target: development
           push: true
 
-  test_make_docker_configuration:
-    runs-on: ubuntu-latest
-
-    steps:
-      - uses: actions/checkout@v4
-      - uses: actions/setup-node@v2
-      - name: Install dependencies
-        shell: bash
-        run: npm ci
-      - name: Check make/docker configuration
-        shell: bash
-        run: |
-          docker compose version
-          make test_setup
-
-  test_run_docker_action:
-    runs-on: ubuntu-latest
-    needs: [build, context]
-
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Create failure
-        id: failure
-        continue-on-error: true
-        uses: ./.github/actions/run-docker
-        with:
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            exit 1
-
-      - name: Verify failure
-        if: always()
-        run: |
-          if [[ "${{ steps.failure.outcome }}" != "failure" ]]; then
-            echo "Expected failure"
-            exit 1
-          fi
-
-      - name: Check (special characters in command)
-        uses: ./.github/actions/run-docker
-        with:
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            echo 'this is a question?'
-            echo 'a * is born'
-            echo 'wow an array []'
-
-      - name: Verify Build Metadata
-        uses: ./.github/actions/run-docker
-        if: needs.context.outputs.is_fork == 'false'
-        with:
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            expected_version="${{ needs.build.outputs.version }}"
-            expected_commit="${{ github.sha }}"
-
-            if [ "$DOCKER_COMMIT" != "$expected_commit" ]; then
-              echo "DOCKER_COMMIT: '$DOCKER_COMMIT' is not equal to '$expected_commit'"
-              exit 1
-            fi
-
-            if [ "$DOCKER_VERSION" != "$expected_version" ]; then
-              echo "DOCKER_VERSION: '$DOCKER_VERSION' is not equal to '$expected_version'"
-              exit 1
-            fi
-
-      - name: Check ignored files
-        uses: ./.github/actions/run-docker
-        with:
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            # Verify that .dockerignore is working and the
-            # Makefile-os is not in the production container
-            if [ -f Makefile-os ]; then
-              echo "Makefile-os exists"
-              exit 1
-            fi
-
-      - name: Test setup
-        uses: ./.github/actions/run-docker
-        with:
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            pytest tests/make/
-
   docs_build:
     runs-on: ubuntu-latest
     needs: build
@@ -272,6 +181,13 @@ jobs:
       version: ${{ needs.build.outputs.version }}
       digest: ${{ needs.build.outputs.digest }}
 
+  test_check:
+    needs: [context, build]
+    uses: ./.github/workflows/_test_check.yml
+    with:
+      version: ${{ needs.build.outputs.version }}
+      digest: ${{ needs.build.outputs.digest }}
+
   push_dockerhub:
     name: Push Production Docker Image (Dockerhub)
     runs-on: ubuntu-latest

.gitignore

@@ -53,6 +53,6 @@ tmp/*
 # End of .gitignore. Please keep this in sync with the top section of .dockerignore
 
 # do not ignore the following files
-!docker-compose.development.yml
+!docker-compose.ci.yml
 !docker-compose.private.yml
 !private/README.md

Makefile-docker

@@ -47,9 +47,7 @@ check_olympia_user: ## check if the olympia user exists and is current user
 
 .PHONY: check_django
 check_django: ## check if the django app is configured properly
-	echo 'from olympia.lib.settings_base import *' > settings_local.py
-	DJANGO_SETTINGS_MODULE='settings_local' python3 ./manage.py check
-	rm settings_local.py
+	./manage.py check
 
 .PHONY: check_nginx
 check_nginx:  ## check if the nginx config for local development is configured properly

settings.py

@@ -18,6 +18,8 @@
 # So if the value is anything other than "production" we are in development mode.
 DEV_MODE = DOCKER_TARGET != 'production'
 
+HOST_UID = os.environ.get('HOST_UID')
+
 WSGI_APPLICATION = 'olympia.wsgi.application'
 
 INTERNAL_ROUTES_ALLOWED = True

src/olympia/core/apps.py

@@ -39,6 +39,28 @@ def uwsgi_check(app_configs, **kwargs):
     return errors
 
 
+@register(CustomTags.custom_setup)
+def host_check(app_configs, **kwargs):
+    """Check that the host settings are valid."""
+    errors = []
+
+    # In production, we expect settings.HOST_UID to be None and so
+    # set the expected uid to 9500, otherwise we expect the uid
+    # passed to the environment to be the expected uid.
+    expected_uid = 9500 if settings.HOST_UID is None else int(settings.HOST_UID)
+    actual_uid = os.getuid()
+
+    if actual_uid != expected_uid:
+        return [
+            Error(
+                f'Expected user uid to be {expected_uid}, received {actual_uid}',
+                id='setup.E002',
+            )
+        ]
+
+    return errors
+
+
 @register(CustomTags.custom_setup)
 def version_check(app_configs, **kwargs):
     """Check the (virtual) version.json file exists and has the correct keys."""

src/olympia/core/tests/test_apps.py

@@ -3,9 +3,37 @@
 from django.core.management import call_command
 from django.core.management.base import SystemCheckError
 from django.test import TestCase
+from django.test.utils import override_settings
 
 
 class SystemCheckIntegrationTest(TestCase):
+    @mock.patch('olympia.core.apps.os.getuid')
+    def test_illegal_override_uid_check(self, mock_getuid):
+        """
+        If the HOST_UID is not set or if it is not set to the
+        olympia user actual uid, then file ownership is probably
+        incorrect and we should fail the check.
+        """
+        dummy_uid = '1000'
+        olympia_uid = '9500'
+        for host_uid in [None, olympia_uid]:
+            with override_settings(HOST_UID=host_uid):
+                with self.subTest(host_uid=host_uid):
+                    mock_getuid.return_value = int(dummy_uid)
+                    with self.assertRaisesMessage(
+                        SystemCheckError,
+                        f'Expected user uid to be {olympia_uid}, received {dummy_uid}',
+                    ):
+                        call_command('check')
+
+        with override_settings(HOST_UID=dummy_uid):
+            mock_getuid.return_value = int(olympia_uid)
+            with self.assertRaisesMessage(
+                SystemCheckError,
+                f'Expected user uid to be {dummy_uid}, received {olympia_uid}',
+            ):
+                call_command('check')
+
     @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 = (

src/olympia/lib/settings_base.py

@@ -71,6 +71,9 @@ def path(*folders):
 
 DEV_MODE = False
 
+# Host info that is hard coded for production images.
+HOST_UID = None
+
 # Used to determine if django should serve static files.
 # For local deployments we want nginx to proxy static file requests to the
 # uwsgi server and not try to serve them locally.