From 68b8e937a9410021e7fd82bc6425ca50bd354901 Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Fri, 3 May 2024 15:49:12 +0200 Subject: [PATCH] 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 | 27 +---------------- Makefile-docker | 9 ++++++ Makefile-os | 5 ++-- src/olympia/core/apps.py | 30 +++++++++++++++++++ src/olympia/core/sentry.py | 17 +++-------- src/olympia/core/tests/test_apps.py | 35 +++++++++++++++++++++++ src/olympia/core/tests/test_utils.py | 24 ++++++++++++++++ src/olympia/core/utils.py | 19 ++++++++++++ 8 files changed, 124 insertions(+), 42 deletions(-) create mode 100644 src/olympia/core/tests/test_utils.py create mode 100644 src/olympia/core/utils.py diff --git a/.github/workflows/verify-docker-image.yml b/.github/workflows/verify-docker-image.yml index f902bf7a8a..05002b55e2 100644 --- a/.github/workflows/verify-docker-image.yml +++ b/.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 diff --git a/Makefile-docker b/Makefile-docker index fea76b1822..b4fbc7656f 100644 --- a/Makefile-docker +++ b/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/* diff --git a/Makefile-os b/Makefile-os index 4dfc65c799..5b402ecd0c 100644 --- a/Makefile-os +++ b/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: diff --git a/src/olympia/core/apps.py b/src/olympia/core/apps.py index e0e675d7b0..872c975df8 100644 --- a/src/olympia/core/apps.py +++ b/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') diff --git a/src/olympia/core/sentry.py b/src/olympia/core/sentry.py index fa5c3ab8f4..3f99f90d72 100644 --- a/src/olympia/core/sentry.py +++ b/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) diff --git a/src/olympia/core/tests/test_apps.py b/src/olympia/core/tests/test_apps.py index 7fefd167d2..9930929a4f 100644 --- a/src/olympia/core/tests/test_apps.py +++ b/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') diff --git a/src/olympia/core/tests/test_utils.py b/src/olympia/core/tests/test_utils.py new file mode 100644 index 0000000000..77c51fe934 --- /dev/null +++ b/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') diff --git a/src/olympia/core/utils.py b/src/olympia/core/utils.py new file mode 100644 index 0000000000..3d7b65b782 --- /dev/null +++ b/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