Include DOCKER_* build info statically in image

- Modified `Dockerfile` to expose build arguments and create a static build info file, ensuring that build variables are hard-coded and not overridden at runtime.
- Refactored `get_version_json` function in `utils.py` to read build information from the newly created static file, ensuring required keys are validated.
- Updated tests in `test_apps.py` and `test_utils.py` to check for required version keys and validate the build info retrieval process.

Dockerfile

@@ -46,7 +46,28 @@ RUN localedef -i en_US -f UTF-8 en_US.UTF-8
 ENV LANG=en_US.UTF-8
 ENV LC_ALL=en_US.UTF-8
 
+# Build args that determine the tag and stage of the build
+# These are passed to docker via the bake.hcl file
+ARG DOCKER_COMMIT
+ARG DOCKER_VERSION
+ARG DOCKER_BUILD
+ARG DOCKER_TARGET
+
+# Expose these variables via hard coded file to prevent them from being
+# overridden by the environment when running the container. These
+# values represent the build and should not be changable afterwards.
+ENV BUILD_INFO=/build-info
 RUN <<EOF
+# Create the build file hard coding build variables to the image
+cat <<INNEREOF > ${BUILD_INFO}
+commit="${DOCKER_COMMIT}"
+version="${DOCKER_VERSION}"
+build="${DOCKER_BUILD}"
+target="${DOCKER_TARGET}"
+INNEREOF
+# Set permissions to make the file readable by all but only writable by root
+chmod 644 ${BUILD_INFO}
+
 # Create directory for dependencies
 mkdir /deps
 chown -R olympia:olympia /deps

Makefile-os

@@ -7,6 +7,10 @@
 DOCKER_PROGRESS ?= auto
 DOCKER_METADATA_FILE ?= buildx-bake-metadata.json
 DOCKER_PUSH ?=
+# Values that are not saved to .env
+# but should be set in the docker image
+# default to static values to prevent
+# invalidating docker build cache
 export DOCKER_COMMIT ?=
 export DOCKER_BUILD ?=
 export DOCKER_VERSION ?=

docker-bake.hcl

@@ -15,10 +15,10 @@ target "web" {
   tags = ["${DOCKER_TAG}"]
   platforms = ["linux/amd64"]
   args = {
-	DOCKER_COMMIT = "${DOCKER_COMMIT}"
-	DOCKER_VERSION = "${DOCKER_VERSION}"
-	DOCKER_BUILD = "${DOCKER_BUILD}"
-  DOCKER_TARGET = "${DOCKER_TARGET}"
+    DOCKER_COMMIT = "${DOCKER_COMMIT}"
+    DOCKER_VERSION = "${DOCKER_VERSION}"
+    DOCKER_BUILD = "${DOCKER_BUILD}"
+    DOCKER_TARGET = "${DOCKER_TARGET}"
   }
   pull = true
 

src/olympia/core/apps.py

@@ -12,7 +12,10 @@
 from django.db import connection
 from django.utils.translation import gettext_lazy as _
 
-from olympia.core.utils import get_version_json
+from olympia.core.utils import (
+    REQUIRED_VERSION_KEYS,
+    get_version_json,
+)
 
 
 log = logging.getLogger('z.startup')
@@ -64,11 +67,9 @@ def host_check(app_configs, **kwargs):
 @register(CustomTags.custom_setup)
 def version_check(app_configs, **kwargs):
     """Check the (virtual) version.json file exists and has the correct keys."""
-    required_keys = ['version', 'build', 'commit', 'source']
-
     version = get_version_json()
 
-    missing_keys = [key for key in required_keys if key not in version]
+    missing_keys = [key for key in REQUIRED_VERSION_KEYS if key not in version]
 
     if missing_keys:
         return [
@@ -85,8 +86,10 @@ def version_check(app_configs, **kwargs):
 def static_check(app_configs, **kwargs):
     errors = []
     output = StringIO()
+    version = get_version_json()
 
-    if settings.DEV_MODE:
+    # We only run this check in production images.
+    if version.get('target') != 'production':
         return []
 
     try:

src/olympia/core/tests/test_apps.py

@@ -1,38 +1,30 @@
+import os
 from unittest import mock
 
 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
 
+from olympia.core.utils import REQUIRED_VERSION_KEYS
 
-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')
+class SystemCheckIntegrationTest(TestCase):
+    def setUp(self):
+        self.default_version_json = {
+            'tag': 'mozilla/addons-server:1.0',
+            'target': 'production',
+            'commit': 'abc',
+            'version': '1.0',
+            'build': 'http://example.com/build',
+            'source': 'https://github.com/mozilla/addons-server',
+        }
+        patch = mock.patch(
+            'olympia.core.apps.get_version_json',
+            return_value=self.default_version_json,
+        )
+        self.mock_get_version_json = patch.start()
+        self.addCleanup(patch.stop)
 
     @mock.patch('olympia.core.apps.connection.cursor')
     def test_db_charset_check(self, mock_cursor):
@@ -56,29 +48,35 @@ def test_uwsgi_check(self):
             ):
                 call_command('check')
 
-    def test_version_missing_key(self):
-        call_command('check')
-
-        with mock.patch('olympia.core.apps.get_version_json') as get_version_json:
-            keys = ['version', 'build', 'commit', 'source']
-            version_mock = {key: 'test' for key in keys}
-
-            for key in keys:
-                version = version_mock.copy()
-                version.pop(key)
-                get_version_json.return_value = version
-
+    def test_missing_version_keys_check(self):
+        """
+        We expect all required version keys to be set during the docker build.
+        """
+        for broken_key in REQUIRED_VERSION_KEYS:
+            with self.subTest(broken_key=broken_key):
+                del self.mock_get_version_json.return_value[broken_key]
                 with self.assertRaisesMessage(
-                    SystemCheckError, f'{key} is missing from version.json'
+                    SystemCheckError,
+                    f'Expected key: {broken_key} to exist',
                 ):
                     call_command('check')
 
-    def test_version_missing_multiple_keys(self):
-        call_command('check')
+    def test_dockerignore_file_exists_check(self):
+        """
+        In production, or when the host mount is set to production, we expect
+        not to find docker ignored files like Makefile-os in the file system.
+        """
+        original_exists = os.path.exists
 
-        with mock.patch('olympia.core.apps.get_version_json') as get_version_json:
-            get_version_json.return_value = {'version': 'test', 'build': 'test'}
-            with self.assertRaisesMessage(
-                SystemCheckError, 'commit, source is missing from version.json'
-            ):
-                call_command('check')
+        def mock_exists(path):
+            return path == '/data/olympia/Makefile-os' or original_exists(path)
+
+        for host_mount in (None, 'production'):
+            with self.subTest(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')

src/olympia/core/tests/test_utils.py

@@ -1,88 +1,109 @@
 import json
+import os
+import shutil
 import sys
-from unittest import mock
+import tempfile
+from unittest import TestCase, mock
 
 from olympia.core.utils import get_version_json
 
 
 default_version = {
-    'commit': 'commit',
+    'commit': '',
     'version': 'local',
-    'build': 'build',
+    'build': '',
+    'target': 'development',
     'source': 'https://github.com/mozilla/addons-server',
 }
 
 
-@mock.patch.dict('os.environ', clear=True)
-def test_get_version_json_defaults():
-    result = get_version_json()
+class TestGetVersionJson(TestCase):
+    def setUp(self):
+        self.tmp_dir = tempfile.mkdtemp()
+        self.build_info_path = os.path.join(self.tmp_dir, 'build-info')
+        self.pkg_json_path = os.path.join(self.tmp_dir, 'package.json')
 
-    assert result['commit'] == default_version['commit']
-    assert result['version'] == default_version['version']
-    assert result['build'] == default_version['build']
-    assert result['source'] == default_version['source']
+        self.with_build_info()
+        self.with_pkg_json({})
 
+    def tearDown(self):
+        shutil.rmtree(self.tmp_dir)
 
-def test_get_version_json_commit():
-    with mock.patch.dict('os.environ', {'DOCKER_COMMIT': 'new_commit'}):
-        result = get_version_json()
+    def with_build_info(self, **kwargs):
+        data = '\n'.join(
+            f'{key}="{value}"' for key, value in {**default_version, **kwargs}.items()
+        )
+        with open(self.build_info_path, 'w') as f:
+            f.write(data)
+        f.close()
 
-    assert result['commit'] == 'new_commit'
+    def with_pkg_json(self, data):
+        with open(self.pkg_json_path, 'w') as f:
+            f.write(json.dumps(data))
+        f.close()
 
+    def test_get_version_json_defaults(self):
+        result = get_version_json(build_info_path=self.build_info_path)
 
-def test_get_version_json_version():
-    with mock.patch.dict('os.environ', {'DOCKER_VERSION': 'new_version'}):
-        result = get_version_json()
+        assert result['commit'] == default_version['commit']
+        assert result['version'] == default_version['version']
+        assert result['build'] == default_version['build']
+        assert result['source'] == default_version['source']
 
-    assert result['version'] == 'new_version'
+    def test_get_version_json_commit(self):
+        self.with_build_info(commit='new_commit')
+        result = get_version_json(build_info_path=self.build_info_path)
 
+        assert result['commit'] == 'new_commit'
 
-def test_get_version_json_build():
-    with mock.patch.dict('os.environ', {'DOCKER_BUILD': 'new_build'}):
-        result = get_version_json()
+    def test_get_version_json_version(self):
+        self.with_build_info(version='new_version')
+        result = get_version_json(build_info_path=self.build_info_path)
 
-    assert result['build'] == 'new_build'
+        assert result['version'] == 'new_version'
 
+    def test_get_version_json_build(self):
+        self.with_build_info(build='new_build')
+        result = get_version_json(build_info_path=self.build_info_path)
 
-def test_get_version_json_python():
-    with mock.patch.object(sys, 'version_info') as v_info:
-        v_info.major = 3
-        v_info.minor = 9
-        result = get_version_json()
+        assert result['build'] == 'new_build'
 
-    assert result['python'] == '3.9'
+    def test_get_version_json_python(self):
+        with mock.patch.object(sys, 'version_info') as v_info:
+            v_info.major = 3
+            v_info.minor = 9
+            result = get_version_json(build_info_path=self.build_info_path)
 
+        assert result['python'] == '3.9'
 
-def test_get_version_json_django():
-    with mock.patch('django.VERSION', (3, 2)):
-        result = get_version_json()
+    def test_get_version_json_django(self):
+        with mock.patch('django.VERSION', (3, 2)):
+            result = get_version_json(build_info_path=self.build_info_path)
 
-    assert result['django'] == '3.2'
+        assert result['django'] == '3.2'
 
+    def test_get_version_json_addons_linter(self):
+        self.with_pkg_json({'dependencies': {'addons-linter': '1.2.3'}})
+        result = get_version_json(
+            build_info_path=self.build_info_path,
+            pkg_json_path=self.pkg_json_path,
+        )
 
-def test_get_version_json_addons_linter():
-    with mock.patch('os.path.exists', return_value=True):
-        with mock.patch(
-            'builtins.open',
-            mock.mock_open(read_data='{"dependencies": {"addons-linter": "1.2.3"}}'),
-        ):
-            result = get_version_json()
+        assert result['addons-linter'] == '1.2.3'
 
-    assert result['addons-linter'] == '1.2.3'
+    def test_get_version_json_addons_linter_missing_package(self):
+        self.with_pkg_json({'dependencies': {}})
+        result = get_version_json(
+            build_info_path=self.build_info_path,
+            pkg_json_path=self.pkg_json_path,
+        )
 
+        assert result['addons-linter'] == ''
 
-def test_get_version_json_addons_linter_missing_package():
-    with mock.patch('os.path.exists', return_value=True):
-        with mock.patch(
-            'builtins.open', mock.mock_open(read_data=json.dumps({'dependencies': {}}))
-        ):
-            result = get_version_json()
+    def test_get_version_json_addons_linter_missing_file(self):
+        result = get_version_json(
+            build_info_path=self.build_info_path,
+            pkg_json_path=self.pkg_json_path,
+        )
 
-    assert result['addons-linter'] == ''
-
-
-def test_get_version_json_addons_linter_missing_file():
-    with mock.patch('os.path.exists', return_value=False):
-        result = get_version_json()
-
-    assert result['addons-linter'] == ''
+        assert result['addons-linter'] == ''