Skip to content

Commit

Permalink
Include DOCKER_* build info statically in image:
Browse files Browse the repository at this point in the history
- Updated Dockerfile to create a build info file containing static build variables (commit, version, build, target) that are now read-only at runtime.
- Modified get_version_json function to read build information from the newly created build info file instead of relying on environment variables.
- Introduced REQUIRED_VERSION_KEYS to ensure all necessary keys are present in version.json during checks.
- Enhanced tests to validate the presence of required version keys and ensure proper functionality of the get_version_json method.
- Updated docker-bake.hcl to maintain consistency in build arguments.
  • Loading branch information
KevinMind committed Dec 13, 2024
1 parent 40a2dc6 commit cff36e2
Show file tree
Hide file tree
Showing 8 changed files with 200 additions and 128 deletions.
34 changes: 28 additions & 6 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

FROM python:3.11-slim-bookworm AS olympia

ENV BUILD_INFO=/build-info

# Set shell to bash with logs and errors for build
SHELL ["/bin/bash", "-xue", "-c"]

Expand All @@ -18,6 +20,29 @@ ENV HOME=/data/olympia
WORKDIR ${HOME}
RUN chown -R olympia:olympia ${HOME}

FROM olympia AS info

# Build args that represent static build information
# These are passed to docker via the bake.hcl file and
# should not be overridden in the container environment.
ARG DOCKER_COMMIT
ARG DOCKER_VERSION
ARG DOCKER_BUILD
ARG DOCKER_TARGET

# Create the build file hard coding build variables to the image
RUN <<EOF
cat <<INNEREOF > ${BUILD_INFO}
commit="${DOCKER_COMMIT}"
version="${DOCKER_VERSION}"
build="${DOCKER_BUILD}"
target="${DOCKER_TARGET}"
source="https://github.com/mozilla/addons-server"
INNEREOF
# Set permissions to make the file readable by all but only writable by root
chmod 644 ${BUILD_INFO}
EOF

FROM olympia AS base
# Add keys and repos for node and mysql
# TODO: replace this with a bind mount on the RUN command
Expand Down Expand Up @@ -95,8 +120,8 @@ rm -rf /deps/build/*
${PIP_COMMAND} install --progress-bar=off --no-deps --exists-action=w -r requirements/pip.txt
EOF

# Expose the DOCKER_TARGET variable to all subsequent stages
# This value is used to determine if we are building for production or development
# TODO: we should remove dependency on the environment variable
# and instead read from the /build-info file
ARG DOCKER_TARGET
ENV DOCKER_TARGET=${DOCKER_TARGET}

Expand Down Expand Up @@ -174,11 +199,7 @@ 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
Expand All @@ -187,6 +208,7 @@ COPY --chown=olympia:olympia . ${HOME}
# Copy assets from assets
COPY --from=assets --chown=olympia:olympia ${HOME}/site-static ${HOME}/site-static
COPY --from=assets --chown=olympia:olympia ${HOME}/static-build ${HOME}/static-build
COPY --from=info ${BUILD_INFO} ${BUILD_INFO}

# Set shell back to sh until we can prove we can use bash at runtime
SHELL ["/bin/sh", "-c"]
Expand Down
4 changes: 4 additions & 0 deletions Makefile-os
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?=
Expand Down
9 changes: 5 additions & 4 deletions docker-bake.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ 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}"
DOCKER_SOURCE = "https://github.com/mozilla/addons-server"
}
pull = true

Expand Down
13 changes: 8 additions & 5 deletions src/olympia/core/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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 [
Expand All @@ -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:
Expand Down
76 changes: 26 additions & 50 deletions src/olympia/core/tests/test_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,36 +3,26 @@
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):
Expand All @@ -56,29 +46,15 @@ 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'{broken_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')
129 changes: 75 additions & 54 deletions src/olympia/core/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -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'] == ''
Loading

0 comments on commit cff36e2

Please sign in to comment.