Add django check to verify docker image (#22210)

* Extract logic to load verison.json to utils

* Reimplement version.json check as django check

* Better defaults for version commit/build

* Extract django check to make command

.github/workflows/verify-docker-image.yml

@@ -23,29 +23,4 @@ jobs:
           options:
           run: |
             make update_deps
-            echo 'from olympia.lib.settings_base import *' > settings_local.py
-            DJANGO_SETTINGS_MODULE='settings_local' python3 ./manage.py check
-
-      - id: version
-        shell: bash
-        run: |
-          # Create the version JSON we expect
-          make version \
-            DOCKER_VERSION="${{ steps.build.outputs.version }}" \
-            DOCKER_COMMIT="${{ github.sha }}" \
-            VERSION_BUILD_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
-
-          # Extract the version JSON from the image
-          touch version-container.json
-          docker run \
-            -v $(pwd)/version-container.json:/data/olympia/version-container.json \
-            --rm -u 0 ${{ steps.build.outputs.tags }} \
-            cp version.json version-container.json
-
-          # expect them to be the same
-          if [[ "$(cat version.json)" != "$(cat version-container.json)" ]]; then
-            echo "version.json is incorrect"
-            exit 1
-          else
-            echo "version.json is correct"
-          fi
+            make check

Makefile-docker

@@ -57,6 +57,15 @@ jquery-ui/ui/widgets/sortable.js
 help_redirect:
 	@$(MAKE) help --no-print-directory
 
+.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
+
+.PHONY: check
+check: check_django
+
 .PHONY: initialize_db
 initialize_db: ## create a new database
 	rm -rf ./user-media/* ./tmp/*

Makefile-os

@@ -8,12 +8,11 @@ export DOCKER_BUILDER=container
 DOCKER_TAG := addons-server-test
 DOCKER_PLATFORM := linux/amd64
 DOCKER_PROGRESS := auto
-DOCKER_COMMIT := $(shell git rev-parse HEAD)
+export DOCKER_COMMIT := $(shell git rev-parse HEAD || echo "commit")
 DOCKER_CACHE_DIR := docker-cache
 
 export DOCKER_VERSION ?= local
-export DOCKER_COMMIT ?= $(shell git rev-parse --short HEAD)
-export VERSION_BUILD_URL ?= local
+export VERSION_BUILD_URL ?= build
 
 .PHONY: help_redirect
 help_redirect:

src/olympia/core/apps.py

@@ -7,6 +7,8 @@ from django.conf import settings
 from django.core.checks import Error, Tags, register
 from django.utils.translation import gettext_lazy as _
 
+from olympia.core.utils import get_version_json
+
 
 log = logging.getLogger('z.startup')
 
@@ -32,6 +34,34 @@ def uwsgi_check(app_configs, **kwargs):
     return errors
 
 
+@register(CustomTags.custom_setup)
+def version_check(app_configs, **kwargs):
+    """Check the version.json file exists and has the correct keys."""
+    required_keys = ['version', 'build', 'commit', 'source']
+
+    version = get_version_json()
+
+    if not version:
+        return [
+            Error(
+                'version.json is missing',
+                id='setup.E002',
+            )
+        ]
+
+    missing_keys = [key for key in required_keys if key not in version]
+
+    if missing_keys:
+        return [
+            Error(
+                f'{", ".join(missing_keys)} is missing from version.json',
+                id='setup.E002',
+            )
+        ]
+
+    return []
+
+
 class CoreConfig(AppConfig):
     name = 'olympia.core'
     verbose_name = _('Core')

src/olympia/core/sentry.py

@@ -1,8 +1,9 @@
-import json
 import os
 import re
 import urllib.parse
 
+from olympia.core.utils import get_version_json
+
 
 # List of fields to scrub in our custom sentry_before_send() callback.
 # /!\ Each value needs to be in lowercase !
@@ -16,18 +17,8 @@ SENTRY_SENSITIVE_FIELDS = (
 
 
 def get_sentry_release():
-    root = os.path.join(os.path.dirname(os.path.dirname(__file__)), '..', '..')
-    version_json = os.path.join(root, 'version.json')
-    version = 'unknown'
-
-    if os.path.exists(version_json):
-        try:
-            with open(version_json) as fobj:
-                contents = fobj.read()
-                data = json.loads(contents)
-                version = data.get('version') or data.get('commit')
-        except (OSError, KeyError):
-            version = None
+    version_json = get_version_json() or {}
+    version = version_json.get('version') or version_json.get('commit')
 
     # sentry is loaded before django so we have to read the env directly
     ensure_version = os.environ.get('REQUIRE_SENTRY_VERSION', False)

src/olympia/core/tests/test_apps.py

@@ -15,3 +15,38 @@ class SystemCheckIntegrationTest(SimpleTestCase):
                 SystemCheckError, 'uwsgi --version returned a non-zero value'
             ):
                 call_command('check')
+
+    def test_version_check_missing_file(self):
+        call_command('check')
+
+        with mock.patch('olympia.core.apps.get_version_json') as get_version_json:
+            get_version_json.return_value = None
+            with self.assertRaisesMessage(SystemCheckError, 'version.json is missing'):
+                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
+
+                with self.assertRaisesMessage(
+                    SystemCheckError, f'{key} is missing from version.json'
+                ):
+                    call_command('check')
+
+    def test_version_missing_multiple_keys(self):
+        call_command('check')
+
+        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')

src/olympia/core/tests/test_utils.py

@@ -0,0 +1,24 @@
+import json
+from unittest import mock
+
+from olympia.core.utils import get_version_json
+
+
+def test_get_version_json():
+    version = {'version': '1.0.0'}
+    with mock.patch('builtins.open', mock.mock_open(read_data=json.dumps(version))):
+        assert get_version_json() == version
+
+
+def test_get_version_json_missing():
+    with mock.patch('os.path.exists', return_value=False):
+        assert get_version_json() is None
+
+
+def test_get_version_json_error():
+    with mock.patch('builtins.open', mock.mock_open(read_data='')):
+        with mock.patch('builtins.print', mock.Mock()) as mock_print:
+            assert get_version_json() is None
+            mock_print.assert_called_once()
+            args, _ = mock_print.call_args
+            assert args[0].startswith('Error reading')

src/olympia/core/utils.py

@@ -0,0 +1,19 @@
+import json
+import os
+
+
+def get_version_json():
+    root = os.path.join(os.path.dirname(os.path.dirname(__file__)), '..', '..')
+    version_json = os.path.join(root, 'version.json')
+    version = None
+
+    if os.path.exists(version_json):
+        try:
+            with open(version_json) as fobj:
+                contents = fobj.read()
+                version = json.loads(contents)
+        except (OSError, json.JSONDecodeError) as exc:
+            print(f'Error reading {version_json}: {exc}')
+            pass
+
+    return version