diff --git a/.github/actions/run-docker/action.yml b/.github/actions/run-docker/action.yml index c0ccd77c1de2..5a826110115a 100644 --- a/.github/actions/run-docker/action.yml +++ b/.github/actions/run-docker/action.yml @@ -12,10 +12,6 @@ inputs: run: description: 'Run command in container' required: true - compose_file: - description: 'The docker-compose file to use' - required: false - default: 'docker-compose.yml:docker-compose.ci.yml' logs: description: 'Show logs' required: false @@ -23,31 +19,26 @@ inputs: description: 'Skip data backup' required: false default: 'true' + mount: + description: 'Mount olympia files from host' + required: false + default: 'production' runs: using: 'composite' steps: - - id: id - shell: bash - run: | - echo "id=$(id -u)" >> $GITHUB_OUTPUT - - name: Run Docker Container shell: bash - env: - DOCKER_VERSION: ${{ inputs.version }} - DOCKER_DIGEST: ${{ inputs.digest }} - COMPOSE_FILE: ${{ inputs.compose_file }} - HOST_UID: ${{ steps.id.outputs.id }} - DATA_BACKUP_SKIP: ${{ inputs.data_backup_skip }} - # In CI, we should use the docker-compose wait flag to ensure - # healthchecks are passing before running any commands on the containers. - # This comes at a performance cost, but ensures containers are ready - # to accept commands before CI continues to execute. - DOCKER_WAIT: true run: | # Start the specified services - make up + make up \ + DOCKER_VERSION="${{ inputs.version }}" \ + DOCKER_DIGEST="${{ inputs.digest }}" \ + OLYMPIA_UID="$(id -u)" \ + OLYMPIA_MOUNT="${{ inputs.mount }}" \ + DATA_BACKUP_SKIP="${{ inputs.data_backup_skip }}" \ + DOCKER_WAIT="true" + # Exec the run command in the container # quoted 'EOF' to prevent variable expansion diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index 6aa6ee00b36e..80da00809c98 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -41,29 +41,24 @@ jobs: - name: Needs Locale Compilation services: '' - compose_file: docker-compose.yml:docker-compose.ci.yml run: | make compile_locales make test_needs_locales_compilation - name: Static Assets services: '' - compose_file: docker-compose.yml:docker-compose.ci.yml run: make test_static_assets - name: Internal Routes services: '' - compose_file: docker-compose.yml:docker-compose.ci.yml run: make test_internal_routes_allowed - name: Elastic Search services: '' - compose_file: docker-compose.yml:docker-compose.ci.yml run: make test_es_tests - name: Codestyle services: web - compose_file: docker-compose.yml:docker-compose.ci.yml run: make lint-codestyle steps: - uses: actions/checkout@v4 @@ -73,5 +68,4 @@ jobs: version: ${{ inputs.version }} digest: ${{ inputs.digest }} services: ${{ matrix.services }} - compose_file: ${{ matrix.compose_file }} run: ${{ matrix.run }} diff --git a/.github/workflows/_test_check.yml b/.github/workflows/_test_check.yml index 19c2d4bcc3a1..50e634405ffd 100644 --- a/.github/workflows/_test_check.yml +++ b/.github/workflows/_test_check.yml @@ -45,16 +45,16 @@ jobs: runs-on: ubuntu-latest name: | version: '${{ matrix.version }}' | - compose_file: '${{ matrix.compose_file }}' + mount: '${{ matrix.mount }}' strategy: fail-fast: false matrix: version: - local - ${{ inputs.version }} - compose_file: - - docker-compose.yml - - docker-compose.yml:docker-compose.ci.yml + mount: + - development + - production steps: - uses: actions/checkout@v4 - shell: bash @@ -63,7 +63,7 @@ jobs: cat < + # in the running docker container. + # If OLYMPIA_MOUNT_SOURCE matches (data_olympia_) + # then we use the production volume mounts. Otherwise + # it will map to the current directory ./ + # (data_olympia_):/ + data_olympia_: + data_olympia_storage: # Volume for rabbitmq/redis to avoid anonymous volumes data_rabbitmq: data_redis: diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index a48da2312246..e3f59fcdc489 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -18,13 +18,26 @@ OLYMPIA_USER="olympia" function get_olympia_uid() { echo "$(id -u "$OLYMPIA_USER")"; } function get_olympia_gid() { echo "$(id -g "$OLYMPIA_USER")"; } -if [[ -n "${HOST_UID:-}" ]]; then +OLD_HOST_UID=$(get_olympia_uid) + +# If the olympia user's uid is different in the container than from the build, +# we need to update the olympia user's uid to match the new one. +if [[ "${HOST_UID}" != "${OLD_HOST_UID}" ]]; then usermod -u ${HOST_UID} ${OLYMPIA_USER} - echo "${OLYMPIA_USER} UID: ${OLYMPIA_UID} -> ${HOST_UID}" + echo "${OLYMPIA_USER} UID: ${OLD_HOST_UID} -> ${HOST_UID}" +fi + +NEW_HOST_UID=$(get_olympia_uid) +OLYMPIA_ID_STRING="${NEW_HOST_UID}:$(get_olympia_gid)" + +# If we are on production mode, update the ownership of /data/olympia and /deps to match the new id +if [[ "${HOST_MOUNT}" == "production" ]]; then + echo "Updating ownership of /data/olympia and /deps to ${OLYMPIA_ID_STRING}" + chown -R ${OLYMPIA_ID_STRING} /data/olympia /deps fi cat < { }); describe.each([ - 'docker-compose.yml', - 'docker-compose.yml:docker-compose.ci.yml', - ])('COMPOSE_FILE=%s', (COMPOSE_FILE) => { - const isProd = COMPOSE_FILE.includes('docker-compose.ci.yml'); + ['development', 'development'], + ['development', 'production'], + ['production', 'development'], + ['production', 'production'], + ])('DOCKER_TARGET=%s, OLYMPIA_MOUNT=%s', (DOCKER_TARGET, OLYMPIA_MOUNT) => { + const isProdTarget = DOCKER_TARGET === 'production'; + const isProdMount = OLYMPIA_MOUNT === 'production'; + const isProdMountTarget = isProdMount && isProdTarget; + + const inputValues = { + DOCKER_TARGET, + OLYMPIA_MOUNT, + DOCKER_TAG: 'mozilla/addons-server:tag', + DEBUG: 'debug', + DATA_BACKUP_SKIP: 'skip', + }; + it('.services.(web|worker) should have the correct configuration', () => { - const values = { - COMPOSE_FILE, - DOCKER_TAG: 'mozilla/addons-server:tag', - // set docker target to production to ensure we are allowed - // to override the olympia mount - DOCKER_TARGET: 'production', - DEBUG: 'debug', - OLYMPIA_UID: '1', - DATA_BACKUP_SKIP: 'skip', - }; const { services: { web, worker }, - } = getConfig(values); + } = getConfig(inputValues); for (let service of [web, worker]) { - expect(service.image).toStrictEqual(values.DOCKER_TAG); + expect(service.image).toStrictEqual(inputValues.DOCKER_TAG); expect(service.pull_policy).toStrictEqual('never'); expect(service.user).toStrictEqual('root'); expect(service.platform).toStrictEqual('linux/amd64'); @@ -96,28 +101,25 @@ describe('docker-compose.yml', () => { expect(service.volumes).toEqual( expect.arrayContaining([ expect.objectContaining({ - ...(isProd ? {} : { source: expect.any(String) }), + source: isProdMountTarget ? 'data_olympia_' : expect.any(String), target: '/data/olympia', }), - expect.objectContaining( - isProd - ? { - source: 'storage', - target: '/data/olympia/storage', - } - : {}, - ), + expect.objectContaining({ + source: isProdMountTarget + ? 'data_olympia_storage' + : expect.any(String), + target: '/data/olympia/storage', + }), ]), ); + const { OLYMPIA_MOUNT, ...environmentOutput } = inputValues; expect(service.environment).toEqual( expect.objectContaining({ - COMPOSE_FILE: values.COMPOSE_FILE, - DEBUG: values.DEBUG, - // Should be set to the UID of the host user - HOST_UID: expect.any(String), - DATA_BACKUP_SKIP: values.DATA_BACKUP_SKIP, + ...environmentOutput, }), ); + // We excpect not to pass the input values to the container + expect(service.environment).not.toHaveProperty('OLYMPIA_UID'); } expect(web.volumes).toEqual( @@ -137,9 +139,7 @@ describe('docker-compose.yml', () => { it('.services.nginx should have the correct configuration', () => { const { services: { nginx }, - } = getConfig({ - COMPOSE_FILE, - }); + } = getConfig(inputValues); // nginx is mapped from http://olympia.test to port 80 in /etc/hosts on the host expect(nginx.ports).toStrictEqual([ expect.objectContaining({ @@ -158,25 +158,27 @@ describe('docker-compose.yml', () => { }), // mapping for local host directory to /data/olympia expect.objectContaining({ - source: expect.any(String), + source: isProdMountTarget ? 'data_olympia_' : expect.any(String), target: '/srv', }), + expect.objectContaining({ + source: 'data_site_static', + target: '/srv/site-static', + }), // mapping for local host directory to /data/olympia/storage - expect.objectContaining( - isProd - ? { - source: 'storage', - target: '/srv/storage', - } - : {}, - ), + expect.objectContaining({ + source: isProdMountTarget + ? 'data_olympia_storage' + : expect.any(String), + target: '/srv/storage', + }), ]), ); }); it('.services.*.volumes duplicate volumes should be defined on services.olympia_volumes.volumes', () => { const key = 'olympia_volumes'; - const { services } = getConfig({ COMPOSE_FILE }); + const { services } = getConfig(inputValues); // all volumes defined on any service other than olympia const volumesMap = new Map(); // volumes defined on the olympia service, any dupes in other services should be here also @@ -221,68 +223,53 @@ describe('docker-compose.yml', () => { }); it('.services.*.volumes does not contain anonymous or unnamed volumes', () => { - const { services } = getConfig({ COMPOSE_FILE }); + const { services } = getConfig(inputValues); for (let [name, config] of Object.entries(services)) { for (let volume of config.volumes ?? []) { - if (volume.bind) { - console.warn( - `'.services.${name}.volumes' contains anonymous bind mount: ` + - `'${volume.source}:${volume.target}'. Please use a named volume mount instead.` + - 'In the future, this will raise an error!', - ); - } else if (!volume.source) { - console.warn( + if (!volume.bind && !volume.source) { + throw new Error( `'.services.${name}.volumes' contains unnamed volume mount: ` + - `'${volume.target}'. Please use a named volume mount instead.` + - 'In the future, this will raise an error!', + `'${volume.target}'. Please use a named volume mount instead.`, ); } } } }); - describe.each(['development', 'production'])( - 'When DOCKER_TARGET=%s', - (DOCKER_TARGET) => { - const FILTERED_KEYS = [ - 'DOCKER_COMMIT', - 'DOCKER_VERSION', - 'DOCKER_BUILD', - ]; - // This test ensures that we do NOT include environment variables that are used - // at build time in the container. Cointainer environment variables are dynamic - // and should not be able to deviate from the state at build time. - it('.services.(web|worker).environment excludes build info variables', () => { - const { - services: { web, worker }, - } = getConfig({ - COMPOSE_FILE, - DOCKER_TARGET, - ...Object.fromEntries( - FILTERED_KEYS.map((key) => [key, 'filtered']), - ), - }); - for (let service of [web, worker]) { - for (let key of FILTERED_KEYS) { - expect(service.environment).not.toHaveProperty(key); - } - expect(service.environment.DOCKER_TARGET).toStrictEqual( - DOCKER_TARGET, - ); - } - }); - }, - ); + const EXCLUDED_KEYS = ['DOCKER_COMMIT', 'DOCKER_VERSION', 'DOCKER_BUILD']; + // This test ensures that we do NOT include environment variables that are used + // at build time in the container. Cointainer environment variables are dynamic + // and should not be able to deviate from the state at build time. + it('.services.(web|worker).environment excludes build info variables', () => { + const { + services: { web, worker }, + } = getConfig({ + ...inputValues, + ...Object.fromEntries(EXCLUDED_KEYS.map((key) => [key, 'filtered'])), + }); + for (let service of [web, worker]) { + for (let key of EXCLUDED_KEYS) { + expect(service.environment).not.toHaveProperty(key); + } + } + }); }); // these keys require special handling to prevent runtime errors in make setup const failKeys = [ // Invalid docker tag leads to docker not parsing the image 'DOCKER_TAG', - // Invalid compose file leads to inability to create a compose config - 'COMPOSE_FILE', + // Value is read directly as the volume source for /data/olympia and must be valid + 'HOST_MOUNT_SOURCE', + ]; + const ignoreKeys = [ + // Ignored because these values are explicitly mapped to the host_* values + 'OLYMPIA_UID', + 'OLYMPIA_MOUNT', + // Ignored because the HOST_UID is always set to the host user's UID + 'HOST_UID', + 'HOST_MOUNT', ]; - const ignoreKeys = []; const defaultEnv = runSetup(); const customValue = 'custom'; diff --git a/tests/make/test_setup.py b/tests/make/test_setup.py index 086bc5e8540e..a72037b331c2 100644 --- a/tests/make/test_setup.py +++ b/tests/make/test_setup.py @@ -9,7 +9,14 @@ def override_env(**kwargs): return mock.patch.dict(os.environ, kwargs, clear=True) -keys = ['COMPOSE_FILE', 'DOCKER_TAG', 'DOCKER_TARGET', 'HOST_UID', 'DEBUG'] +keys = [ + 'DOCKER_TAG', + 'DOCKER_TARGET', + 'HOST_UID', + 'HOST_MOUNT', + 'HOST_MOUNT_SOURCE', + 'DEBUG', +] class BaseTestClass(unittest.TestCase): @@ -130,25 +137,6 @@ def test_default_env_file(self): self.assert_set_env_file_called_with(DOCKER_TARGET='production') -@override_env() -class TestComposeFile(BaseTestClass): - def test_default_compose_file(self): - main() - self.assert_set_env_file_called_with(COMPOSE_FILE='docker-compose.yml') - - @override_env(DOCKER_TARGET='production') - def test_default_target_production(self): - main() - self.assert_set_env_file_called_with( - COMPOSE_FILE='docker-compose.yml:docker-compose.ci.yml' - ) - - @override_env(COMPOSE_FILE='test') - def test_compose_file_override(self): - main() - self.assert_set_env_file_called_with(COMPOSE_FILE='test') - - @override_env() class TestDebug(BaseTestClass): def test_default_debug(self): @@ -176,3 +164,41 @@ def test_override_env_debug_true_on_target_development(self): def test_debug_override(self): main() self.assert_set_env_file_called_with(DEBUG='test') + + +@override_env() +class TestHostMount(BaseTestClass): + def test_default_host_mount(self): + main() + self.assert_set_env_file_called_with( + HOST_MOUNT='development', HOST_MOUNT_SOURCE='./' + ) + + @override_env(DOCKER_TARGET='production') + def test_host_mount_set_by_docker_target(self): + main() + self.assert_set_env_file_called_with( + HOST_MOUNT='production', HOST_MOUNT_SOURCE='data_olympia_' + ) + + @override_env(DOCKER_TARGET='production', OLYMPIA_MOUNT='test') + def test_invalid_host_mount_set_by_env_ignored(self): + main() + self.assert_set_env_file_called_with( + HOST_MOUNT='production', HOST_MOUNT_SOURCE='data_olympia_' + ) + + @override_env(DOCKER_TARGET='development', OLYMPIA_MOUNT='production') + def test_host_mount_overriden_by_development_target(self): + main() + self.assert_set_env_file_called_with( + HOST_MOUNT='development', HOST_MOUNT_SOURCE='./' + ) + + @override_env(DOCKER_TARGET='production') + def test_host_mount_from_file_ignored(self): + self.mock_get_env_file.return_value = {'HOST_MOUNT': 'development'} + main() + self.assert_set_env_file_called_with( + HOST_MOUNT='production', HOST_MOUNT_SOURCE='data_olympia_' + )