Remove version.json file from host environment (#22571)

* Remove version.json file from host environment

* tmp: fix

* TMP: fix tests

* TMP: remove unecessary test

* TMP: fix test for get_version_json

* TMP: fixy

.circleci/config.yml

@@ -27,7 +27,7 @@ commands:
           command: |
             echo 'export DOCKER_VERSION=<< parameters.image_tag >>' >> $BASH_ENV
             echo 'export DOCKER_COMMIT=$CIRCLE_SHA1' >> $BASH_ENV
-            echo 'export VERSION_BUILD_URL=$CIRCLE_BUILD_URL' >> $BASH_ENV
+            echo 'export DOCKER_BUILD=$CIRCLE_BUILD_URL' >> $BASH_ENV
             echo 'export BUILDX_BAKE_METADATA_FILE=metadata.json' >> $BASH_ENV
             echo 'export DOCKER_PROGRESS=plain' >> $BASH_ENV
       - when:

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

@@ -70,7 +70,7 @@ runs:
         DOCKER_TARGET: ${{ inputs.target }}
         DOCKER_VERSION: ${{ steps.meta.outputs.version }}
         DOCKER_COMMIT: ${{ steps.context.outputs.git_sha }}
-        VERSION_BUILD_URL: ${{ steps.context.outputs.git_build_url }}
+        DOCKER_BUILD: ${{ steps.context.outputs.git_build_url }}
       run: make setup
 
     - name: Build Image

.gitignore

@@ -65,8 +65,5 @@ private/
 # Ignore buildx bake output files
 buildx-bake-metadata.json
 
-# Ignore the version.json as this should be created during the build
-version.json
-
 # Ignore db backup files
 backups

Dockerfile

@@ -155,6 +155,12 @@ EOF
 
 FROM base as sources
 
+ARG DOCKER_BUILD DOCKER_COMMIT DOCKER_VERSION
+
+ENV DOCKER_BUILD=${DOCKER_BUILD}
+ENV DOCKER_COMMIT=${DOCKER_COMMIT}
+ENV DOCKER_VERSION=${DOCKER_VERSION}
+
 # Add our custom mime types (required for for ts/json/md files)
 COPY docker/etc/mime.types /etc/mime.types
 # Copy the rest of the source files from the host
@@ -167,10 +173,6 @@ COPY --from=assets --chown=olympia:olympia ${HOME}/site-static ${HOME}/site-stat
 # Add build.py build UUID
 RUN ${HOME}/scripts/generate_build.py > build.py
 
-# version.json is overwritten by CircleCI (see circle.yml).
-# The pipeline v2 standard requires the existence of /app/version.json
-# inside the docker image, thus it's copied there.
-COPY version.json /app/version.json
 
 # Set shell back to sh until we can prove we can use bash at runtime
 SHELL ["/bin/sh", "-c"]

Makefile-docker

@@ -57,7 +57,6 @@ REQUIRED_FILES := \
 	Makefile \
 	Makefile-os \
 	Makefile-docker \
-	version.json \
 	/deps/package.json \
 	/deps/package-lock.json \
 	/addons-server-docker-container \

Makefile-os

@@ -8,8 +8,9 @@ DOCKER_BUILDER ?= container
 DOCKER_PROGRESS ?= auto
 DOCKER_PUSH ?=
 BUILDX_BAKE_METADATA_FILE ?=
-DOCKER_COMMIT ?= $(shell git rev-parse HEAD || echo "commit")
-VERSION_BUILD_URL ?= build
+export DOCKER_COMMIT ?=
+export DOCKER_BUILD ?=
+export DOCKER_VERSION ?=
 BUILDX_BAKE_COMMAND := docker buildx bake web
 override DOCKER_MYSQLD_VOLUME = addons-server_data_mysqld
 

docker-compose.yml

@@ -29,6 +29,10 @@ services:
     # Ignore any linting saying we have an invalid value.
     pull_policy: ${DOCKER_PULL_POLICY:-}
     build:
+      args:
+        DOCKER_COMMIT: ${DOCKER_COMMIT:-}
+        DOCKER_VERSION: ${DOCKER_VERSION:-}
+        DOCKER_BUILD: ${DOCKER_BUILD:-}
       context: .
       dockerfile: Dockerfile
       target: ${DOCKER_TARGET:-}

scripts/setup.py

@@ -1,8 +1,6 @@
 #!/usr/bin/env python3
 
-import json
 import os
-import subprocess
 
 
 def set_env_file(values):
@@ -37,15 +35,6 @@ def get_value(key, default_value):
     return default_value
 
 
-def git_ref():
-    try:
-        git_ref = subprocess.check_output(['git', 'rev-parse', 'HEAD']).decode().strip()
-    except subprocess.CalledProcessError:
-        git_ref = 'commit'
-
-    return get_value('DOCKER_COMMIT', git_ref)
-
-
 def get_docker_tag():
     image_name = 'mozilla/addons-server'
     version = os.environ.get('DOCKER_VERSION')
@@ -100,20 +89,3 @@ set_env_file(
         'HOST_UID': get_value('HOST_UID', os.getuid()),
     }
 )
-
-build = get_value('VERSION_BUILD_URL', 'build')
-
-with open('version.json', 'w') as f:
-    data = {
-        'commit': git_ref(),
-        'version': docker_version,
-        'digest': docker_digest,
-        'build': build,
-        'source': 'https://github.com/mozilla/addons-server',
-    }
-    print('Version:')
-    print(json.dumps(data, indent=2))
-    json.dump(
-        data,
-        f,
-    )

src/olympia/amo/tests/test_settings.py

@@ -11,29 +11,29 @@ from sentry_sdk.hub import Hub
 from olympia.core.sentry import get_sentry_release
 
 
-@mock.patch.dict('os.environ', {'REQUIRE_SENTRY_VERSION': 'True'})
-def test_sentry_release_nothing_provided_strict():
-    version = {}
-    with mock.patch('builtins.open', mock.mock_open(read_data=json.dumps(version))):
-        pytest.raises(ValueError, get_sentry_release)
+@mock.patch.dict(os.environ, {'REQUIRE_SENTRY_VERSION': 'True'})
+@mock.patch('olympia.core.sentry.get_version_json', return_value={})
+def test_sentry_release_nothing_provided_strict(mock_get_version_json):
+    pytest.raises(ValueError, get_sentry_release)
 
 
-def test_sentry_release_nothing_provided_loose():
-    version = {}
-    with mock.patch('builtins.open', mock.mock_open(read_data=json.dumps(version))):
-        assert get_sentry_release() is None
+@mock.patch('olympia.core.sentry.get_version_json', return_value={})
+def test_sentry_release_nothing_provided_loose(mock_get_version_json):
+    assert get_sentry_release() is None
 
 
-def test_sentry_release_version_provided():
+@mock.patch('olympia.core.sentry.get_version_json')
+def test_sentry_release_version_provided(mock_get_version_json):
     version = {'version': '2024.01.01'}
-    with mock.patch('builtins.open', mock.mock_open(read_data=json.dumps(version))):
-        assert get_sentry_release() == version['version']
+    mock_get_version_json.return_value = version
+    assert get_sentry_release() == version['version']
 
 
-def test_sentry_release_commit_provided():
+@mock.patch('olympia.core.sentry.get_version_json')
+def test_sentry_release_commit_provided(mock_get_version_json):
     version = {'commit': 'abc'}
-    with mock.patch('builtins.open', mock.mock_open(read_data=json.dumps(version))):
-        assert get_sentry_release() == version['commit']
+    mock_get_version_json.return_value = version
+    assert get_sentry_release() == version['commit']
 
 
 def test_sentry_data_scrubbing():

src/olympia/amo/views.py

@@ -1,9 +1,7 @@
 import json
 import os
 import re
-import sys
 
-import django
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
 from django.contrib.sitemaps.views import x_robots_tag
@@ -24,6 +22,7 @@ from olympia import amo
 from olympia.amo.utils import HttpResponseXSendFile, use_fake_fxa
 from olympia.api.exceptions import base_500_data
 from olympia.api.serializers import SiteStatusSerializer
+from olympia.core.utils import get_version_json
 from olympia.users.models import UserProfile
 
 from . import monitors
@@ -167,22 +166,7 @@ def csrf_failure(request, reason=''):
 
 @non_atomic_requests
 def version(request):
-    path = os.path.join(settings.ROOT, 'version.json')
-    with open(path) as f:
-        contents = json.loads(f.read())
-
-    py_info = sys.version_info
-    contents['python'] = '{major}.{minor}'.format(
-        major=py_info.major, minor=py_info.minor
-    )
-    contents['django'] = '{major}.{minor}'.format(
-        major=django.VERSION[0], minor=django.VERSION[1]
-    )
-
-    path = os.path.join(settings.ROOT, 'package.json')
-    with open(path) as f:
-        data = json.loads(f.read())
-        contents['addons-linter'] = data['dependencies']['addons-linter']
+    contents = get_version_json()
 
     res = HttpResponse(json.dumps(contents), content_type='application/json')
     res.headers['Access-Control-Allow-Origin'] = '*'

src/olympia/core/apps.py

@@ -36,19 +36,11 @@ def uwsgi_check(app_configs, **kwargs):
 
 @register(CustomTags.custom_setup)
 def version_check(app_configs, **kwargs):
-    """Check the version.json file exists and has the correct keys."""
+    """Check the (virtual) 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:

src/olympia/core/tests/test_apps.py

@@ -16,14 +16,6 @@ class SystemCheckIntegrationTest(SimpleTestCase):
             ):
                 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')
 

src/olympia/core/tests/test_utils.py

@@ -1,24 +1,88 @@
 import json
+import sys
 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
+default_version = {
+    'commit': 'commit',
+    'version': 'local',
+    'build': 'build',
+    'source': 'https://github.com/mozilla/addons-server',
+}
 
 
-def test_get_version_json_missing():
+@mock.patch.dict('os.environ', clear=True)
+def test_get_version_json_defaults():
+    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']
+
+
+def test_get_version_json_commit():
+    with mock.patch.dict('os.environ', {'DOCKER_COMMIT': 'new_commit'}):
+        result = get_version_json()
+
+    assert result['commit'] == 'new_commit'
+
+
+def test_get_version_json_version():
+    with mock.patch.dict('os.environ', {'DOCKER_VERSION': 'new_version'}):
+        result = get_version_json()
+
+    assert result['version'] == 'new_version'
+
+
+def test_get_version_json_build():
+    with mock.patch.dict('os.environ', {'DOCKER_BUILD': 'new_build'}):
+        result = get_version_json()
+
+    assert result['build'] == 'new_build'
+
+
+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['python'] == '3.9'
+
+
+def test_get_version_json_django():
+    with mock.patch('django.VERSION', (3, 2)):
+        result = get_version_json()
+
+    assert result['django'] == '3.2'
+
+
+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'
+
+
+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()
+
+    assert result['addons-linter'] == ''
+
+
+def test_get_version_json_addons_linter_missing_file():
     with mock.patch('os.path.exists', return_value=False):
-        assert get_version_json() is None
+        result = get_version_json()
 
-
-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')
+    assert result['addons-linter'] == ''

src/olympia/core/utils.py

@@ -1,19 +1,36 @@
 import json
 import os
+import sys
+
+import django
 
 
 def get_version_json():
+    contents = {}
+
     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
+    contents['commit'] = os.environ.get('DOCKER_COMMIT', 'commit')
+    contents['version'] = os.environ.get('DOCKER_VERSION', 'local')
+    contents['source'] = 'https://github.com/mozilla/addons-server'
+    contents['build'] = os.environ.get('DOCKER_BUILD', 'build')
 
-    return version
+    py_info = sys.version_info
+    contents['python'] = '{major}.{minor}'.format(
+        major=py_info.major, minor=py_info.minor
+    )
+    contents['django'] = '{major}.{minor}'.format(
+        major=django.VERSION[0], minor=django.VERSION[1]
+    )
+
+    pkg_json_path = os.path.join(root, 'package.json')
+
+    if os.path.exists(pkg_json_path):
+        with open(pkg_json_path) as f:
+            data = json.loads(f.read())
+            dependencies = data.get('dependencies', {})
+            contents['addons-linter'] = dependencies.get('addons-linter', '')
+    else:
+        contents['addons-linter'] = ''
+
+    return contents

tests/make/make.spec.js

@@ -18,23 +18,6 @@ function readEnvFile(name) {
   return parse(fs.readFileSync(envPath, { encoding: 'utf-8' }))[name];
 }
 
-test('version.json', () => {
-  runSetup({
-    DOCKER_VERSION: 'version',
-    DOCKER_COMMIT: '123',
-    VERSION_BUILD_URL: 'https://',
-  });
-
-  const version = require(path.join(rootPath, 'version.json'));
-
-  expect(version.version).toStrictEqual('version');
-  expect(version.commit).toStrictEqual('123');
-  expect(version.build).toStrictEqual('https://');
-  expect(version.source).toStrictEqual(
-    'https://github.com/mozilla/addons-server',
-  );
-});
-
 test('map docker compose config', () => {
   const values = {
     DOCKER_VERSION: 'version',
@@ -46,7 +29,7 @@ test('map docker compose config', () => {
 
   const { stdout: rawConfig } = spawnSync(
     'docker',
-    ['compose', 'config', 'web', '--format', 'json'],
+    ['compose', 'config', '--format', 'json'],
     { encoding: 'utf-8' },
   );
 
@@ -193,7 +176,12 @@ describe.each([
 const testedKeys = new Set(testCases.map(({ name }) => name));
 
 // Keys testsed outside the scope of testCases
-const skippedKeys = ['DOCKER_PULL_POLICY'];
+const skippedKeys = [
+  'DOCKER_PULL_POLICY',
+  'DOCKER_COMMIT',
+  'DOCKER_VERSION',
+  'DOCKER_BUILD',
+];
 
 test('All dynamic properties in any docker compose file are referenced in the test', () => {
   const composeFiles = globSync('docker-compose*.yml', { cwd: rootPath });