Fix the host/olympia stuff

.github/actions/run-docker/action.yml

@@ -35,20 +35,14 @@ inputs:
 runs:
   using: 'composite'
   steps:
-    - id: id
-      shell: bash
-      run: |
-        echo "id=$(id -u)" >> $GITHUB_OUTPUT
-
     - name: Run Docker Container
       id: run
       continue-on-error: true
       shell: bash
       env:
         DOCKER_VERSION: ${{ inputs.version }}
         DOCKER_DIGEST: ${{ inputs.digest }}
-        HOST_UID: ${{ steps.id.outputs.id }}
-        HOST_MOUNT: ${{ inputs.mount }}
+        OLYMPIA_MOUNT_INPUT: ${{ inputs.mount }}
         DATA_BACKUP_SKIP: ${{ inputs.initialize == 'true' && '' || 'true' }}
         INSTALL_CI_DEPS: ${{ inputs.install_ci_deps }}
         DOCKER_TARGET: ${{ inputs.target }}

docker-compose.yml

@@ -20,8 +20,8 @@ x-env-mapping: &env
     - HISTIGNORE=ls:exit:"cd .."
     - HISTCONTROL=erasedups
     - CIRCLECI
-    - HOST_UID
-    - HOST_MOUNT
+    - OLYMPIA_UID
+    - OLYMPIA_MOUNT
     - DEBUG
     - DATA_BACKUP_SKIP
     - INSTALL_CI_DEPS
@@ -30,7 +30,7 @@ x-env-mapping: &env
 # production: mount the files from the container, effectively disabling the mount
 # development: mount the host files, mapping host files into the container
 x-olympia-mount: &olympia-mount
-  data_olympia_${HOST_MOUNT}:/data/olympia
+  data_olympia_${OLYMPIA_MOUNT}:/data/olympia
 
 x-deps-mount: &deps-mount
   data_deps:/deps

docker/entrypoint.sh

@@ -18,9 +18,9 @@ OLYMPIA_USER="olympia"
 function get_olympia_uid() { echo "$(id -u "$OLYMPIA_USER")"; }
 function get_olympia_gid() { echo "$(id -g "$OLYMPIA_USER")"; }
 
-if [[ "${HOST_MOUNT}" == "development" ]]; then
-  usermod -u ${HOST_UID} ${OLYMPIA_USER}
-  echo "${OLYMPIA_USER} UID: ${OLYMPIA_UID} -> ${HOST_UID}"
+if [[ "${OLYMPIA_MOUNT}" == "development" ]]; then
+  usermod -u ${OLYMPIA_UID} ${OLYMPIA_USER}
+  echo "Updating ${OLYMPIA_USER} UID to ${OLYMPIA_UID}"
 fi
 
 cat <<EOF | su -s /bin/bash $OLYMPIA_USER

docs/topics/development/building_and_running_services.md

@@ -17,7 +17,7 @@ The Dockerfile for the **addons-server** project uses a multi-stage build to opt
    - **Mounting Dependencies**: The volume `./deps:/deps` mounts the dependencies directory, enabling better caching across builds and providing visibility for debugging directly on the host.
 
 4. **Environment Variables**:
-   - **Development Setup**: The `HOST_UID` and `HOST_MOUNT` environment variables are set to the host user ID and mount host, ensuring that the container runs with the correct permissions and mounts the correct host directory.
+   - **Development Setup**: The `OLYMPIA_UID` and `OLYMPIA_MOUNT_INPUT` environment variables are set to the host user ID and mount host, ensuring that the container runs with the correct permissions and mounts the correct host directory.
    - **CI Setup**: In CI environments, the Olympia mount is removed by default. This makes the container a closed system, mimicking production behavior closely.
 
 ### Best Practices for the Dockerfile

scripts/setup.py

@@ -100,16 +100,16 @@ def main():
     docker_target = get_value(
         'DOCKER_TARGET', ('development' if docker_version == 'local' else 'production')
     )
-    # If the user has specified using the development image,
-    # we must use the development host mount, as neither the
-    # development image nor the production volume include the
-    # necessary files to run the application and it will fail.
-    # Otherwise, we use the host mount specified in the .env file
-    # or the environment variable.
-    host_mount = (
+    # On development images, we ignore the user provided OLYMPIA_MOUNT_INPUT
+    # and hard code the volume to development. This is because neither
+    # the image nor the volume would provide the files needed by the container.
+    # That is also why the value saved to .env is different from the input value.
+    # This way the docker-compose.yml and the container only read the computed
+    # OLYMPIA_MOUNT value and not the OLYMPIA_MOUNT_INPUT.
+    data_olympia_mount = (
         docker_target
         if docker_target == 'development'
-        else get_value('HOST_MOUNT', docker_target)
+        else get_value('OLYMPIA_MOUNT_INPUT', docker_target)
     )
 
     # DEBUG is special, as we should allow the user to override it
@@ -118,15 +118,15 @@ def main():
     debug = get_value(
         'DEBUG', str(False if docker_target == 'production' else True), from_file=False
     )
-    # HOST_UID should always be set to the current user's UID
+    # OLYMPIA_UID should always be set to the current user's UID
     host_uid = os.getuid()
 
     set_env_file(
         {
             'DOCKER_TAG': docker_tag,
             'DOCKER_TARGET': docker_target,
-            'HOST_UID': host_uid,
-            'HOST_MOUNT': host_mount,
+            'OLYMPIA_UID': host_uid,
+            'OLYMPIA_MOUNT': data_olympia_mount,
             'DEBUG': debug,
         }
     )

settings.py

@@ -18,8 +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')
-HOST_MOUNT = os.environ.get('HOST_MOUNT')
+OLYMPIA_UID = os.environ.get('OLYMPIA_UID')
+OLYMPIA_MOUNT = os.environ.get('OLYMPIA_MOUNT')
 
 WSGI_APPLICATION = 'olympia.wsgi.application'
 

src/olympia/core/apps.py

@@ -67,15 +67,8 @@ def host_check(app_configs, **kwargs):
     """Check that the host settings are valid."""
     errors = []
 
-    # If we are on a production image or the host mount is set to production,
-    # then we expect the host files like Makefile-os to be excluded by dockerignore.
-    print('HOST_MOUNT', settings.HOST_MOUNT)
-    print(
-        'os.path.exists(/data/olympia/Makefile-os)',
-        os.path.exists('/data/olympia/Makefile-os'),
-    )
     if (
-        settings.HOST_MOUNT is None or settings.HOST_MOUNT == 'production'
+        settings.OLYMPIA_MOUNT is None or settings.OLYMPIA_MOUNT == 'production'
     ) and os.path.exists('/data/olympia/Makefile-os'):
         errors.append(
             Error(
@@ -84,9 +77,9 @@ def host_check(app_configs, **kwargs):
             )
         )
 
-    # If we are on a production image, or the HOST_UID is not defined,
+    # If we are on a production image, or the OLYMPIA_UID is not defined,
     # then we expect to retain the original uid of 9500.
-    if settings.HOST_UID is None:
+    if settings.OLYMPIA_UID is None:
         if os.getuid() != 9500:
             return [
                 Error(

src/olympia/core/tests/test_apps.py

@@ -73,19 +73,19 @@ def mock_exists(path):
 
         for host_mount in (None, 'production'):
             with self.subTest(host_mount=host_mount):
-                with override_settings(HOST_MOUNT=host_mount):
+                with override_settings(OLYMPIA_MOUNT=host_mount):
                     with mock.patch('os.path.exists', side_effect=mock_exists):
                         with self.assertRaisesMessage(
                             SystemCheckError,
                             'Makefile-os should be excluded by dockerignore',
                         ):
                             call_command('check')
 
-    @override_settings(HOST_UID=None)
+    @override_settings(OLYMPIA_UID=None)
     @mock.patch('olympia.core.apps.os.getuid')
     def test_illegal_override_uid_check(self, mock_getuid):
         """
-        In production, or when HOST_UID is not set, we expect to not override
+        In production, or when OLYMPIA_UID is not set, we expect to not override
         the default uid of 9500 for the olympia user.
         """
         mock_getuid.return_value = 1000

src/olympia/lib/settings_base.py

@@ -67,8 +67,8 @@ def path(*folders):
 DEV_MODE = False
 
 # Host info that is hard coded for production images.
-HOST_UID = None
-HOST_MOUNT = None
+OLYMPIA_UID = None
+OLYMPIA_MOUNT = None
 
 # Used to determine if django should serve static files.
 # For local deployments we want nginx to proxy static file requests to the

tests/make/make.spec.js

@@ -65,12 +65,15 @@ describe('docker-compose.yml', () => {
   });
 
   it.each(['production', 'development'])(
-    '.services.nginx should have the correct configuration for HOST_MOUNT=%s',
-    (HOST_MOUNT) => {
+    '.services.nginx should have the correct configuration for OLYMPIA_MOUNT_INPUT=%s',
+    (OLYMPIA_MOUNT_INPUT) => {
       const {
         services: { nginx },
       } = getConfig({
-        HOST_MOUNT,
+        OLYMPIA_MOUNT_INPUT,
+        // set docker target to production to ensure we are allowed
+        // to override the olympia mount
+        DOCKER_TARGET: 'production',
       });
       // nginx is mapped from http://olympia.test to port 80 in /etc/hosts on the host
       expect(nginx.ports).toStrictEqual([
@@ -96,7 +99,7 @@ describe('docker-compose.yml', () => {
           }),
           // mapping for local host directory to /data/olympia
           expect.objectContaining({
-            source: `data_olympia_${HOST_MOUNT}`,
+            source: `data_olympia_${OLYMPIA_MOUNT_INPUT}`,
             target: '/data/olympia',
           }),
         ]),
@@ -148,36 +151,44 @@ describe('docker-compose.yml', () => {
   // these keys require special handling to prevent runtime errors in make setup
   const ignoreKeys = [
     'DOCKER_TAG',
-    'COMPOSE_FILE',
-    'HOST_MOUNT',
+    'OLYMPIA_MOUNT_INPUT',
+    'OLYMPIA_MOUNT',
     'DOCKER_TARGET',
   ];
-
-  it.each(ignoreKeys)('does not allow invalid values for %s', (key) => {
-    expect(() => getConfig({ [key]: 'invalid' })).toThrow();
-  });
-
-  it('.services.(web|worker) should allow overriding .env variables', () => {
-    const customEnv = Object.keys(runSetup()).reduce((acc, key) => {
-      if (ignoreKeys.includes(key)) {
-        return acc;
-      }
-      acc[key] = 'custom';
-      return acc;
-    }, {});
-
-    const {
-      services: { web, worker },
-    } = getConfig(customEnv);
-    for (let service of [web, worker]) {
-      expect(service.environment).toEqual(expect.objectContaining(customEnv));
+  const defaultEnv = runSetup();
+  const customValue = 'custom';
+
+  describe.each(
+    Array.from(new Set([...Object.keys(defaultEnv), ...ignoreKeys])),
+  )(`environment variable %s=${customValue}`, (key) => {
+    const ignored = ignoreKeys.includes(key);
+
+    if (ignored) {
+      it('should fail if set to an arbitrary custom value', () => {
+        expect(() =>
+          getConfig({ DOCKER_TARGET: 'production', [key]: customValue }),
+        ).toThrow();
+      });
+    } else {
+      it('should be overriden based on the input', () => {
+        const {
+          services: { web, worker },
+        } = getConfig({ ...defaultEnv, [key]: customValue });
+        for (let service of [web, worker]) {
+          expect(service.environment).toEqual(
+            expect.objectContaining({
+              [key]: customValue,
+            }),
+          );
+        }
+      });
     }
   });
 
   it('.services.web maps environment variables to placeholders', () => {
     const values = {
       DOCKER_VERSION: 'version',
-      HOST_UID: '9500',
+      OLYMPIA_UID: '9500',
     };
     const {
       services: { web },
@@ -187,7 +198,7 @@ describe('docker-compose.yml', () => {
       `mozilla/addons-server:${values.DOCKER_VERSION}`,
     );
     expect(web.platform).toStrictEqual('linux/amd64');
-    expect(web.environment.HOST_UID).toStrictEqual(values.HOST_UID);
+    expect(web.environment.OLYMPIA_UID).toStrictEqual(values.OLYMPIA_UID);
   });
 
   it('.volumes.data_mysqld.name should map to the correct volume', () => {
@@ -196,10 +207,10 @@ describe('docker-compose.yml', () => {
   });
 
   describe.each(['production', 'development'])(
-    '.services.*.volumes for HOST_MOUNT=%s',
-    (HOST_MOUNT) => {
+    '.services.*.volumes for OLYMPIA_MOUNT_INPUT=%s',
+    (OLYMPIA_MOUNT_INPUT) => {
       it('duplicate volumes should be defined on services.olympia.volumes', () => {
-        const { services } = getConfig({ HOST_MOUNT });
+        const { services } = getConfig({ OLYMPIA_MOUNT_INPUT });
         // volumes defined on the olympia service, any dupes in other services should be here also
         const olympiaVolumes = new Set(
           Object.values(services.olympia.volumes).map((v) => v.source),

tests/make/test_setup.py

@@ -292,36 +292,36 @@ def test_debug_override(self):
 class TestHostMount(BaseTestClass):
     def test_default_host_mount(self):
         main()
-        self.assert_set_env_file_called_with(HOST_MOUNT='development')
+        self.assert_set_env_file_called_with(OLYMPIA_MOUNT_INPUT='development')
 
     @override_env(DOCKER_TARGET='production')
     def test_host_mount_set_by_docker_target(self):
         main()
-        self.assert_set_env_file_called_with(HOST_MOUNT='production')
+        self.assert_set_env_file_called_with(OLYMPIA_MOUNT_INPUT='production')
 
-    @override_env(DOCKER_TARGET='production', HOST_MOUNT='test')
+    @override_env(DOCKER_TARGET='production', OLYMPIA_MOUNT_INPUT='test')
     def test_host_mount_set_by_env(self):
         main()
-        self.assert_set_env_file_called_with(HOST_MOUNT='test')
+        self.assert_set_env_file_called_with(OLYMPIA_MOUNT_INPUT='test')
 
-    @override_env(DOCKER_TARGET='production', HOST_MOUNT='test')
+    @override_env(DOCKER_TARGET='production', OLYMPIA_MOUNT_INPUT='test')
     def test_host_mount_set_by_docker_target_and_env(self):
         main()
-        self.assert_set_env_file_called_with(HOST_MOUNT='test')
+        self.assert_set_env_file_called_with(OLYMPIA_MOUNT_INPUT='test')
 
-    @override_env(DOCKER_TARGET='development', HOST_MOUNT='test')
+    @override_env(DOCKER_TARGET='development', OLYMPIA_MOUNT_INPUT='test')
     def test_host_mount_overriden_by_development_target(self):
         main()
-        self.assert_set_env_file_called_with(HOST_MOUNT='development')
+        self.assert_set_env_file_called_with(OLYMPIA_MOUNT_INPUT='development')
 
     @override_env(DOCKER_TARGET='development')
     def test_host_mount_overriden_by_production_target_from_file(self):
-        self.mock_get_env_file.return_value = {'HOST_MOUNT': 'test'}
+        self.mock_get_env_file.return_value = {'OLYMPIA_MOUNT_INPUT': 'test'}
         main()
-        self.assert_set_env_file_called_with(HOST_MOUNT='development')
+        self.assert_set_env_file_called_with(OLYMPIA_MOUNT_INPUT='development')
 
     @override_env(DOCKER_TARGET='production')
     def test_host_mount_from_file(self):
-        self.mock_get_env_file.return_value = {'HOST_MOUNT': 'test'}
+        self.mock_get_env_file.return_value = {'OLYMPIA_MOUNT_INPUT': 'test'}
         main()
-        self.assert_set_env_file_called_with(HOST_MOUNT='test')
+        self.assert_set_env_file_called_with(OLYMPIA_MOUNT_INPUT='test')