diff --git a/.github/actions/build-docker/action.yml b/.github/actions/build-docker/action.yml index 17c2c47eeb77..b5b77025cf5f 100644 --- a/.github/actions/build-docker/action.yml +++ b/.github/actions/build-docker/action.yml @@ -37,7 +37,6 @@ runs: echo "git_build_url=$git_repo_url/actions/runs/${{ github.run_id }}" >> $GITHUB_OUTPUT echo "git_sha=${{ github.sha }}" >> $GITHUB_OUTPUT - echo "metadata_file=buildx-bake-metadata.json" >> $GITHUB_OUTPUT cat $GITHUB_OUTPUT @@ -52,28 +51,33 @@ runs: # and to prevent multiple tags from being associated with a build type=raw,value=${{ inputs.version }} + - name: Create .env and version.json files + shell: bash + run: | + # We only build the production image in CI + echo "DOCKER_TARGET=production" >> $GITHUB_ENV + echo "DOCKER_VERSION=${{ steps.meta.outputs.version }}" >> $GITHUB_ENV + echo "DOCKER_COMMIT=${{ steps.context.outputs.git_sha }}" >> $GITHUB_ENV + echo "DOCKER_BUILD=${{ steps.context.outputs.git_build_url }}" >> $GITHUB_ENV + echo "TAGS_FILE=${{ steps.meta.outputs.bake-file-tags }}" >> $GITHUB_ENV + echo "ANNOTATIONS_FILE=${{ steps.meta.outputs.bake-file-annotations }}" >> $GITHUB_ENV + echo "DOCKER_METADATA_FILE=buildx-bake-metadata.json" >> $GITHUB_ENV + + make setup + - name: Build Image id: build shell: bash - env: - DOCKER_TAGS_FILE: ${{ steps.meta.outputs.bake-file-tags }} - DOCKER_ANNOTATIONS_FILE: ${{ steps.meta.outputs.bake-file-annotations }} - DOCKER_METADATA_FILE: ${{ steps.context.outputs.metadata_file }} - DOCKER_PUSH: ${{ inputs.push }} run: | - make setup \ - DOCKER_TARGET="production" \ - DOCKER_VERSION="${{ steps.meta.outputs.version }}" - make docker_build_web \ - DOCKER_COMMIT="${{ steps.context.outputs.git_sha }}" \ - DOCKER_BUILD="${{ steps.context.outputs.git_build_url }}" + ARGS="--file ${{ env.TAGS_FILE }} --file ${{ env.ANNOTATIONS_FILE }}" \ + DOCKER_PUSH=${{ inputs.push }} - name: Get image digest id: build_meta shell: bash run: | - metadata=$(cat ${{ steps.context.outputs.metadata_file }}) + metadata=$(cat $DOCKER_METADATA_FILE) echo "digest=$(echo $metadata | jq -r '.web."containerimage.digest"')" >> $GITHUB_OUTPUT echo "tag=$(echo $metadata | jq -r '.web."image.name"')" >> $GITHUB_OUTPUT diff --git a/.github/actions/run-docker/action.yml b/.github/actions/run-docker/action.yml index dea9fdd64908..1619f498eb95 100644 --- a/.github/actions/run-docker/action.yml +++ b/.github/actions/run-docker/action.yml @@ -23,6 +23,10 @@ inputs: description: 'Docker target to run (development|production)' required: false default: 'production' + mount: + description: 'Mount host files at runtime? (development|production)' + required: false + default: 'production' deps: description: 'Which dependencies to install at runtime? (development|production)' required: false @@ -40,6 +44,7 @@ runs: DOCKER_DIGEST="${{ inputs.digest }}" \ DOCKER_TARGET="${{ inputs.target }}" \ OLYMPIA_UID="$(id -u)" \ + OLYMPIA_MOUNT="${{ inputs.mount }}" \ OLYMPIA_DEPS="${{ inputs.deps }}" \ DATA_BACKUP_SKIP="${{ inputs.data_backup_skip }}" \ DOCKER_WAIT="true" diff --git a/.github/workflows/_test_check.yml b/.github/workflows/_test_check.yml index 88ec6f90ecb6..eebd0f900289 100644 --- a/.github/workflows/_test_check.yml +++ b/.github/workflows/_test_check.yml @@ -46,6 +46,7 @@ jobs: name: | version: '${{ matrix.version }}' | target: '${{ matrix.target }}' | + mount: '${{ matrix.mount }}' | deps: '${{ matrix.deps }}' strategy: fail-fast: false @@ -56,6 +57,9 @@ jobs: target: - development - production + mount: + - development + - production deps: - development - production @@ -68,6 +72,7 @@ jobs: Values passed to the action: version: ${{ matrix.version }} target: ${{ matrix.target }} + mount: ${{ matrix.mount }} deps: ${{ matrix.deps }} EOF - name: ${{ matrix.version == 'local' && 'Uncached Build' || 'Pull' }} Check @@ -79,6 +84,7 @@ jobs: with: version: ${{ matrix.version }} target: ${{ matrix.target }} + mount: ${{ matrix.mount }} deps: ${{ matrix.deps }} run: make check - name: Cached Build Check @@ -87,6 +93,7 @@ jobs: with: version: ${{ matrix.version }} target: ${{ matrix.target }} + mount: ${{ matrix.mount }} deps: ${{ matrix.deps }} run: echo true diff --git a/.github/workflows/_test_main.yml b/.github/workflows/_test_main.yml index cd69c91de696..38457e047a11 100644 --- a/.github/workflows/_test_main.yml +++ b/.github/workflows/_test_main.yml @@ -88,6 +88,7 @@ jobs: services: '' digest: ${{ inputs.digest }} version: ${{ inputs.version }} + mount: development deps: development run: | split="--splits ${{ needs.test_config.outputs.splits }}" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 743afda17c0c..d782354e3346 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,6 +87,7 @@ jobs: with: digest: ${{ needs.build.outputs.digest }} version: ${{ needs.build.outputs.version }} + mount: development deps: development target: development run: | @@ -139,6 +140,7 @@ jobs: with: digest: ${{ needs.build.outputs.digest }} version: ${{ needs.build.outputs.version }} + mount: development deps: development run: make extract_locales diff --git a/.gitignore b/.gitignore index 1303a8a5131d..d2a01bd4f2c1 100644 --- a/.gitignore +++ b/.gitignore @@ -55,3 +55,5 @@ tmp/* # do not ignore the following files !docker-compose.private.yml !private/README.md +!storage/.gitignore +!deps/.gitkeep diff --git a/Dockerfile b/Dockerfile index 9dfdaf7feb16..21dcad79a82c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,24 +10,13 @@ ENV BUILD_INFO=/build-info.json SHELL ["/bin/bash", "-xue", "-c"] ENV OLYMPIA_UID=9500 -# give olympia access to the HOME directory -ENV HOME=/data/olympia -ENV DEPS_DIR=${HOME}/deps -ENV NPM_DEPS_DIR=${HOME}/node_modules - RUN < settings_local.py +DJANGO_SETTINGS_MODULE="settings_local" make -f Makefile-docker update_assets EOF FROM base AS production @@ -199,4 +194,4 @@ COPY --from=info ${BUILD_INFO} ${BUILD_INFO} # Copy compiled locales from builder COPY --from=locales --chown=olympia:olympia ${HOME}/locale ${HOME}/locale # Copy dependencies from `pip_production` -COPY --from=pip_production --chown=olympia:olympia ${DEPS_DIR} ${DEPS_DIR} +COPY --from=pip_production --chown=olympia:olympia /deps /deps diff --git a/Makefile-docker b/Makefile-docker index 2039d0d36a4e..87c2ef8c6bc2 100644 --- a/Makefile-docker +++ b/Makefile-docker @@ -8,6 +8,16 @@ export PYTHON_COMMAND=python3 export PIP_COMMAND=$(PYTHON_COMMAND) -m pip APP=src/olympia/ +NODE_MODULES := $(NPM_CONFIG_PREFIX)node_modules/ + +REQUIRED_FILES := \ + Makefile \ + Makefile-os \ + Makefile-docker \ + /deps/package.json \ + /deps/package-lock.json \ + /addons-server-docker-container \ + # Build list of dependencies to install DEPS = pip prod # If we're running a development image, then we should install the development dependencies @@ -29,12 +39,29 @@ check_pip_packages: ## check the existence of multiple python packages ./scripts/check_pip_packages.sh $$dep.txt; \ done +.PHONY: check_files +check_files: ## check the existence of multiple files + @for file in $(REQUIRED_FILES); do test -f "$$file" || (echo "$$file is missing." && exit 1); done + @echo "All required files are present." + +.PHONY: check_olympia_user +check_olympia_user: ## check if the olympia user exists and is current user + @if [ "$$(id -u olympia)" != "$$(id -u)" ]; then echo "The current user is not the olympia user."; exit 1; fi + @echo "The current user is the olympia user." + .PHONY: check_django check_django: ## check if the django app is configured properly ./manage.py check +.PHONY: check_nginx +check_nginx: ## check if the nginx config for local development is configured properly + mkdir -p /data/olympia/storage/shared_storage/uploads + echo "OK" > /data/olympia/storage/shared_storage/uploads/.check + @if [ "$$(curl -sf http://nginx/user-media/.check)" != "OK" ]; then echo "Requesting http://nginx/user-media/.check failed"; exit 1; fi + @echo "Nginx user-media configuration looks correct." + .PHONY: check -check: check_debian_packages check_pip_packages check_django +check: check_nginx check_files check_olympia_user check_debian_packages check_pip_packages check_django .PHONY: data_dump data_dump: @@ -45,8 +72,14 @@ data_load: ./manage.py data_load $(ARGS) .PHONY: update_assets -update_assets: ## Update the static assets - $(HOME)/scripts/update_assets.py +update_assets: + # Copy files required in compress_assets to the static folder + # If changing this here, make sure to adapt tests in amo/test_commands.py + $(PYTHON_COMMAND) manage.py compress_assets + $(PYTHON_COMMAND) manage.py generate_jsi18n_files + # Collect static files: This MUST be run last or files will be missing + $(PYTHON_COMMAND) manage.py collectstatic --noinput + .PHONY: update_deps update_deps: ## Update the dependencies @@ -61,7 +94,7 @@ setup-ui-tests: lint: ## lint the code ruff check . ruff format --check . - npm exec $(NPM_ARGS) -- prettier --check '**' + NODE_PATH=$(NODE_MODULES) npm exec $(NPM_ARGS) -- prettier --check '**' curlylint src/ lint-codestyle: lint @@ -166,15 +199,15 @@ test_failed: ## rerun the failed tests from the previous run .PHONY: run_js_tests run_js_tests: ## Run the JavaScript test suite (requires compiled/compressed assets). - npm exec $(NPM_ARGS) -- jest tests/js + NODE_PATH=$(NODE_MODULES) npm exec $(NPM_ARGS) -- jest tests/js .PHONY: watch_js_tests watch_js_tests: ## Run+watch the JavaScript test suite (requires compiled/compressed assets). - npm exec $(NPM_ARGS) -- jest --watch + NODE_PATH=$(NODE_MODULES) npm exec $(NPM_ARGS) -- jest --watch .PHONY: format format: ## Autoformat our codebase. - npm exec $(NPM_ARGS) -- prettier --write '**' + NODE_PATH=$(NODE_MODULES) npm exec $(NPM_ARGS) -- prettier --write '**' ruff check --fix-only . ruff format . @@ -185,7 +218,7 @@ extract_locales: ## extracts and merges translation strings .PHONE: compile_locales compile_locales: ## compiles translation strings $(PIP_COMMAND) install --progress-bar=off --no-deps -r requirements/locale.txt - $(HOME)/scripts/compile_locales.py + ./locale/compile-mo.sh ./locale/ .PHONY: help_submake help_submake: diff --git a/Makefile-os b/Makefile-os index e4dc9c6f1bdd..e3079503fcd3 100644 --- a/Makefile-os +++ b/Makefile-os @@ -39,14 +39,6 @@ ifeq ($(DOCKER_PUSH), true) DOCKER_BAKE_ARGS += --push endif -ifneq ($(DOCKER_ANNOTATIONS_FILE),) - DOCKER_BAKE_ARGS += --file $(DOCKER_ANNOTATIONS_FILE) -endif - -ifneq ($(DOCKER_TAGS_FILE),) - DOCKER_BAKE_ARGS += --file $(DOCKER_TAGS_FILE) -endif - DOCKER_COMPOSE_ARGS := \ -d \ --remove-orphans \ @@ -161,17 +153,17 @@ docker_clean_build_cache: ## Remove buildx build cache .PHONY: clean_docker clean_docker: docker_compose_down docker_mysqld_volume_remove docker_clean_images docker_clean_volumes docker_clean_build_cache ## Remove all docker resources taking space on the host machine -.PHONY: docker_sync_host -docker_sync_host: docker_mysqld_volume_create ## Update the dependencies in the container based on the docker tag and target +.PHONY: docker_update_deps +docker_update_deps: docker_mysqld_volume_create ## Update the dependencies in the container based on the docker tag and target docker compose run \ --rm \ --no-deps \ $(DOCKER_RUN_ARGS) \ web \ - ./scripts/sync_host_files.py + make update_deps .PHONY: up_pre -up_pre: setup docker_pull_or_build docker_sync_host ## Pre-up the environment, setup files, volumes and host state +up_pre: setup docker_pull_or_build docker_update_deps ## Pre-up the environment, setup files, volumes and host state .PHONY: up_start up_start: docker_mysqld_volume_create ## Start the docker containers diff --git a/deps/.gitkeep b/deps/.gitkeep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/docker-compose.yml b/docker-compose.yml index 7e4caaaf81fe..4e847cacc989 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -45,7 +45,10 @@ services: command: ["sleep", "infinity"] volumes: # used by: web, worker, nginx - - ./:/data/olympia + - ${HOST_MOUNT_SOURCE:?}:/data/olympia + - ${HOST_MOUNT_SOURCE:?}deps:/deps + - data_site_static:/data/olympia/site-static + - ${HOST_MOUNT_SOURCE:?}storage:/data/olympia/storage worker: <<: *olympia command: [ @@ -60,7 +63,9 @@ services: "celery -A olympia.amo.celery:app worker -E -c 2 --loglevel=INFO", ] volumes: - - ./:/data/olympia + - ${HOST_MOUNT_SOURCE:?}:/data/olympia + - ${HOST_MOUNT_SOURCE:?}deps:/deps + - ${HOST_MOUNT_SOURCE:?}storage:/data/olympia/storage extra_hosts: - "olympia.test:127.0.0.1" restart: on-failure:5 @@ -88,12 +93,19 @@ services: interval: 30s retries: 3 start_interval: 1s + volumes: + # Don't mount generated files. They only exist in the container + # and would otherwiser be deleted by mounting the cwd volume above + - data_static_build:/data/olympia/static-build + - data_site_static:/data/olympia/site-static nginx: image: nginx volumes: - data_nginx:/etc/nginx/conf.d - - ./:/srv + - ${HOST_MOUNT_SOURCE:?}:/srv + - data_site_static:/srv/site-static + - ${HOST_MOUNT_SOURCE:?}storage:/srv/storage ports: - "80:80" networks: @@ -192,6 +204,21 @@ networks: enable_ipv6: false volumes: + # Volumes for static files that should not be + # mounted from the host. + data_static_build: + data_site_static: + # Volumes for the production olympia mounts + # allowing to conditionally mount directories + # from the host or from the image to + # 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_deps: + 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 06aca156b3c0..e3f59fcdc489 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -30,6 +30,12 @@ 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 < None: os.makedirs(path, exist_ok=True) - def _clean_storage( - self, root: str, dir_dict: dict[str, str | dict], clean: bool = False - ) -> None: + def _clean_storage(self, root: str, dir_dict: dict[str, str | dict]) -> None: for key, value in dir_dict.items(): curr_path = os.path.join(root, key) if isinstance(value, dict): - self._clean_storage(curr_path, value, clean=clean) + self._clean_storage(curr_path, value) else: - if clean: - shutil.rmtree(curr_path, ignore_errors=True) + shutil.rmtree(curr_path, ignore_errors=True) os.makedirs(curr_path, exist_ok=True) - def make_storage(self, clean: bool = False): - self.logger.info('Making storage...') - self._clean_storage(settings.STORAGE_ROOT, storage_structure, clean=clean) + def clean_storage(self): + self.logger.info('Cleaning storage...') + self._clean_storage(settings.STORAGE_ROOT, storage_structure) diff --git a/src/olympia/amo/management/commands/data_load.py b/src/olympia/amo/management/commands/data_load.py index 70969226f6c5..4973409f63c6 100644 --- a/src/olympia/amo/management/commands/data_load.py +++ b/src/olympia/amo/management/commands/data_load.py @@ -38,7 +38,7 @@ def handle(self, *args, **options): raise CommandError(f'Storage backup not found: {storage_path}') cache.clear() - self.make_storage(clean=True) + self.clean_storage() call_command( 'mediarestore', diff --git a/src/olympia/amo/management/commands/data_seed.py b/src/olympia/amo/management/commands/data_seed.py index 00a673a51e8c..33078c1f5e05 100644 --- a/src/olympia/amo/management/commands/data_seed.py +++ b/src/olympia/amo/management/commands/data_seed.py @@ -22,7 +22,7 @@ def handle(self, *args, **options): # Delete any local storage files # This should happen after we reset the database to ensure any records # relying on storage are deleted. - self.make_storage(clean=True) + self.clean_storage() # Migrate the database call_command('migrate', '--noinput') diff --git a/src/olympia/amo/management/commands/initialize.py b/src/olympia/amo/management/commands/initialize.py index 620bde565c1a..413b478ea388 100644 --- a/src/olympia/amo/management/commands/initialize.py +++ b/src/olympia/amo/management/commands/initialize.py @@ -73,9 +73,6 @@ def handle(self, *args, **options): # so our database tables should be accessible call_command('monitors', services=['database']) - # Ensure that the storage directories exist. - self.make_storage(clean=False) - # Ensure any additional required dependencies are available before proceeding. call_command( 'monitors', diff --git a/src/olympia/amo/tests/test_commands.py b/src/olympia/amo/tests/test_commands.py index ff7368dea6a7..5bbaed663be5 100644 --- a/src/olympia/amo/tests/test_commands.py +++ b/src/olympia/amo/tests/test_commands.py @@ -489,7 +489,7 @@ def test_make_dir_non_existing_path(self, mock_makedirs, mock_exists): @mock.patch('olympia.amo.management.shutil.rmtree') @mock.patch('olympia.amo.management.os.makedirs') def test_clean_storage(self, mock_makedirs, mock_rmtree): - self.base_data_command.make_storage(clean=True) + self.base_data_command.clean_storage() def walk_keys(root, dir_dict): for key, value in dir_dict.items(): @@ -650,8 +650,8 @@ def setUp(self): 'olympia.amo.management.commands.data_seed.BaseDataCommand.clean_dir', ), ( - 'mock_make_storage', - 'olympia.amo.management.commands.data_seed.BaseDataCommand.make_storage', + 'mock_clean_storage', + 'olympia.amo.management.commands.data_seed.BaseDataCommand.clean_storage', ), ) @@ -668,7 +668,7 @@ def test_default(self): self.mocks['mock_clean_dir'].assert_called_once_with( self.base_data_command.data_backup_init ) - self.mocks['mock_make_storage'].assert_called_once() + self.mocks['mock_clean_storage'].assert_called_once() self._assert_commands_called_in_order( self.mocks['mock_call_command'], diff --git a/src/olympia/core/apps.py b/src/olympia/core/apps.py index 9d878f84781e..879b30868248 100644 --- a/src/olympia/core/apps.py +++ b/src/olympia/core/apps.py @@ -13,8 +13,6 @@ from django.db import connection from django.utils.translation import gettext_lazy as _ -import requests - from olympia.core.utils import REQUIRED_VERSION_KEYS, get_version_json @@ -95,10 +93,16 @@ def static_check(app_configs, **kwargs): try: call_command('compress_assets', dry_run=True, stdout=output) - stripped_output = output.getvalue().strip() + file_paths = output.getvalue().strip().split('\n') - if stripped_output: - file_paths = stripped_output.split('\n') + if not file_paths: + errors.append( + Error( + 'No compressed asset files were found.', + id='setup.E003', + ) + ) + else: for file_path in file_paths: if not os.path.exists(file_path): error = f'Compressed asset file does not exist: {file_path}' @@ -108,14 +112,6 @@ def static_check(app_configs, **kwargs): id='setup.E003', ) ) - else: - errors.append( - Error( - 'No compressed asset files were found.', - id='setup.E003', - ) - ) - except CommandError as e: errors.append( Error( @@ -155,61 +151,6 @@ def db_charset_check(app_configs, **kwargs): return errors -@register(CustomTags.custom_setup) -def nginx_check(app_configs, **kwargs): - errors = [] - version = get_version_json() - - if version.get('target') == 'production': - return [] - - configs = [ - (settings.MEDIA_ROOT, 'http://nginx/user-media'), - (settings.STATIC_FILES_PATH, 'http://nginx/static'), - (settings.STATIC_ROOT, 'http://nginx/static'), - ] - - files_to_remove = [] - - for dir, base_url in configs: - file_path = os.path.join(dir, 'test.txt') - file_url = f'{base_url}/test.txt' - - if not os.path.exists(dir): - errors.append(Error(f'{dir} does not exist', id='setup.E007')) - - try: - with open(file_path, 'w') as f: - f.write(dir) - - files_to_remove.append(file_path) - response = requests.get(file_url) - - expected_config = ( - (response.status_code, 200), - (response.text, dir), - (response.headers.get('X-Served-By'), 'nginx'), - ) - - if any(item[0] != item[1] for item in expected_config): - message = f'Failed to access {file_url}. {expected_config}' - errors.append(Error(message, id='setup.E008')) - - except Exception as e: - errors.append( - Error( - f'Unknown error accessing {file_path} via {file_url}: {e}', - id='setup.E010', - ) - ) - - # Always remove the files we created. - for file_path in files_to_remove: - os.remove(file_path) - - return errors - - class CoreConfig(AppConfig): name = 'olympia.core' verbose_name = _('Core') diff --git a/src/olympia/core/tests/test_apps.py b/src/olympia/core/tests/test_apps.py index 8c6823aeca15..bb4c8b6d1ffe 100644 --- a/src/olympia/core/tests/test_apps.py +++ b/src/olympia/core/tests/test_apps.py @@ -1,14 +1,10 @@ -import os -import tempfile from unittest import mock -from django.core.management import CommandError, call_command +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 -import responses - from olympia.core.utils import REQUIRED_VERSION_KEYS @@ -24,25 +20,11 @@ def setUp(self): } patch = mock.patch( 'olympia.core.apps.get_version_json', + return_value=self.default_version_json, ) self.mock_get_version_json = patch.start() - self.mock_get_version_json.return_value = self.default_version_json self.addCleanup(patch.stop) - mock_dir = tempfile.mkdtemp(prefix='static-root') - self.fake_css_file = os.path.join(mock_dir, 'foo.css') - with open(self.fake_css_file, 'w') as f: - f.write('body { background: red; }') - - patch_command = mock.patch('olympia.core.apps.call_command') - self.mock_call_command = patch_command.start() - self.mock_call_command.side_effect = ( - lambda command, dry_run, stdout: stdout.write(f'{self.fake_css_file}\n') - ) - self.addCleanup(patch_command.stop) - - self.media_root = tempfile.mkdtemp(prefix='media-root') - @mock.patch('olympia.core.apps.connection.cursor') def test_db_charset_check(self, mock_cursor): mock_cursor.return_value.__enter__.return_value.fetchone.return_value = ( @@ -103,115 +85,3 @@ def test_illegal_override_uid_check(self, mock_getpwnam): with override_settings(HOST_UID=1000): call_command('check') - - def test_static_check_no_assets_found(self): - """ - Test static_check fails if compress_assets reports no files. - """ - self.mock_get_version_json.return_value['target'] = 'production' - # Simulate "compress_assets" returning no file paths. - self.mock_call_command.side_effect = ( - lambda command, dry_run, stdout: stdout.write('') - ) - with self.assertRaisesMessage( - SystemCheckError, 'No compressed asset files were found.' - ): - call_command('check') - - @mock.patch('os.path.exists') - def test_static_check_missing_assets(self, mock_exists): - """ - Test static_check fails if at least one specified compressed - asset file does not exist. - """ - self.mock_get_version_json.return_value['target'] = 'production' - # Simulate "compress_assets" returning a couple of files. - self.mock_call_command.side_effect = ( - lambda command, dry_run, stdout: stdout.write( - f'{self.fake_css_file}\nfoo.js\n' - ) - ) - # Pretend neither file exists on disk. - mock_exists.return_value = False - - with self.assertRaisesMessage( - SystemCheckError, - # Only the first missing file triggers the AssertionError message check - 'Compressed asset file does not exist: foo.js', - ): - call_command('check') - - def test_static_check_command_error(self): - """ - Test static_check fails if there's an error during compress_assets. - """ - self.mock_get_version_json.return_value['target'] = 'production' - self.mock_call_command.side_effect = CommandError('Oops') - with self.assertRaisesMessage( - SystemCheckError, 'Error running compress_assets command: Oops' - ): - call_command('check') - - def test_static_check_command_success(self): - """ - Test static_check succeeds if compress_assets runs without errors. - """ - self.mock_get_version_json.return_value['target'] = 'production' - self.mock_call_command.side_effect = ( - lambda command, dry_run, stdout: stdout.write(f'{self.fake_css_file}\n') - ) - call_command('check') - - def test_nginx_skips_check_on_production_target(self): - fake_media_root = '/fake/not/real' - with override_settings(MEDIA_ROOT=fake_media_root): - call_command('check') - - def test_nginx_raises_missing_directory(self): - self.mock_get_version_json.return_value['target'] = 'development' - fake_media_root = '/fake/not/real' - with override_settings(MEDIA_ROOT=fake_media_root): - with self.assertRaisesMessage( - SystemCheckError, - f'{fake_media_root} does not exist', - ): - call_command('check') - - def _test_nginx_response( - self, base_url, status_code=200, response_text='', served_by='nginx' - ): - self.mock_get_version_json.return_value['target'] = 'development' - url = f'{base_url}/test.txt' - - responses.add( - responses.GET, - url, - status=status_code, - body=response_text, - headers={'X-Served-By': served_by}, - ) - - expected_config = ( - (status_code, 200), - (response_text, self.media_root), - (served_by, 'nginx'), - ) - - with override_settings(MEDIA_ROOT=self.media_root): - with self.assertRaisesMessage( - SystemCheckError, - f'Failed to access {url}. {expected_config}', - ): - call_command('check') - - def test_nginx_raises_non_200_status_code(self): - """Test that files return a 200 status code.""" - self._test_nginx_response('http://nginx/user-media', status_code=404) - - def test_nginx_raises_unexpected_content(self): - """Test that files return the expected content.""" - self._test_nginx_response('http://nginx/user-media', response_text='foo') - - def test_nginx_raises_unexpected_served_by(self): - """Test that files are served by nginx and not redirected elsewhere.""" - self._test_nginx_response('http://nginx/user-media', served_by='wow') diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py index ac9618ca3106..4bd6faee9c2d 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -73,6 +73,7 @@ def path(*folders): # Host info that is hard coded for production images. HOST_UID = None +HOST_MOUNT = None # Used to determine if django should serve static files. # For local deployments we want nginx to proxy static file requests to the @@ -97,15 +98,15 @@ def path(*folders): # LESS CSS OPTIONS (Debug only). LESS_PREPROCESS = True # Compile LESS with Node, rather than client-side JS? LESS_LIVE_REFRESH = False # Refresh the CSS on save? -LESS_BIN = env('LESS_BIN', default=path('node_modules/less/bin/lessc')) +LESS_BIN = env('LESS_BIN', default='/deps/node_modules/less/bin/lessc') # Path to cleancss (our CSS minifier). CLEANCSS_BIN = env( - 'CLEANCSS_BIN', default=path('node_modules/clean-css-cli/bin/cleancss') + 'CLEANCSS_BIN', default='/deps/node_modules/clean-css-cli/bin/cleancss' ) # Path to our JS minifier. -JS_MINIFIER_BIN = env('JS_MINIFIER_BIN', default=path('node_modules/terser/bin/terser')) +JS_MINIFIER_BIN = env('JS_MINIFIER_BIN', default='/deps/node_modules/terser/bin/terser') # rsvg-convert is used to save our svg static theme previews to png RSVG_CONVERT_BIN = env('RSVG_CONVERT_BIN', default='rsvg-convert') @@ -115,7 +116,7 @@ def path(*folders): # Path to our addons-linter binary ADDONS_LINTER_BIN = env( - 'ADDONS_LINTER_BIN', default=path('node_modules/addons-linter/bin/addons-linter') + 'ADDONS_LINTER_BIN', default='/deps/node_modules/addons-linter/bin/addons-linter' ) # --enable-background-service-worker linter flag value ADDONS_LINTER_ENABLE_SERVICE_WORKER = False @@ -1326,15 +1327,14 @@ def read_only_mode(env): 'django_node_assets.finders.NodeModulesFinder', ) -NODE_MODULES_ROOT = path('node_modules') -NODE_PACKAGE_JSON = path('package.json') +NODE_MODULES_ROOT = os.path.join('/', 'deps', 'node_modules') +NODE_PACKAGE_JSON = os.path.join('/', 'deps', 'package.json') NODE_PACKAGE_MANAGER_INSTALL_OPTIONS = ['--dry-run'] -STATIC_BUILD_PATH = path('static-build') -STATIC_FILES_PATH = path('static') +STATIC_BUILD_PATH = os.path.join('/', 'data', 'olympia', 'static-build') STATICFILES_DIRS = ( - STATIC_FILES_PATH, + path('static'), STATIC_BUILD_PATH, ) diff --git a/storage/.gitignore b/storage/.gitignore new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/make/make.spec.js b/tests/make/make.spec.js index fb9739dec556..8c3abc779c5d 100644 --- a/tests/make/make.spec.js +++ b/tests/make/make.spec.js @@ -34,10 +34,7 @@ function getConfig(env = {}) { ); try { if (rawError) throw new Error(rawError); - return { - config: JSON.parse(rawConfig), - env: rawEnv, - }; + return JSON.parse(rawConfig); } catch (error) { throw new Error( JSON.stringify({ error, rawConfig, rawError, rawEnv }, null, 2), @@ -45,45 +42,36 @@ function getConfig(env = {}) { } } -function permutations(configObj) { - return Object.entries(configObj).reduce((acc, [key, values]) => { - if (!acc.length) return values.map((value) => ({ [key]: value })); - return acc.flatMap((obj) => - values.map((value) => ({ ...obj, [key]: value })), - ); - }, []); -} - describe('docker-compose.yml', () => { afterAll(() => { clearEnv(); }); - describe.each( - permutations({ - DOCKER_TARGET: ['development', 'production'], - DOCKER_VERSION: ['local', 'latest'], - }), - )('\n%s\n', (config) => { - const { DOCKER_TARGET, DOCKER_VERSION } = config; + describe.each([ + ['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, - DOCKER_VERSION, + OLYMPIA_MOUNT, + DOCKER_TAG: 'mozilla/addons-server:tag', DEBUG: 'debug', DATA_BACKUP_SKIP: 'skip', }; it('.services.(web|worker) should have the correct configuration', () => { const { - config: { - services: { web, worker }, - }, - env: { DOCKER_TAG }, + services: { web, worker }, } = getConfig(inputValues); for (let service of [web, worker]) { - expect(service.image).toStrictEqual(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'); @@ -113,13 +101,18 @@ describe('docker-compose.yml', () => { expect(service.volumes).toEqual( expect.arrayContaining([ expect.objectContaining({ - source: expect.any(String), + source: isProdMountTarget ? 'data_olympia_' : expect.any(String), target: '/data/olympia', }), + expect.objectContaining({ + source: isProdMountTarget + ? 'data_olympia_storage' + : expect.any(String), + target: '/data/olympia/storage', + }), ]), ); - const { DOCKER_VERSION, DOCKER_TARGET, ...environmentOutput } = - inputValues; + const { OLYMPIA_MOUNT, ...environmentOutput } = inputValues; expect(service.environment).toEqual( expect.objectContaining({ ...environmentOutput, @@ -128,13 +121,24 @@ describe('docker-compose.yml', () => { // We excpect not to pass the input values to the container expect(service.environment).not.toHaveProperty('OLYMPIA_UID'); } + + expect(web.volumes).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + source: 'data_static_build', + target: '/data/olympia/static-build', + }), + expect.objectContaining({ + source: 'data_site_static', + target: '/data/olympia/site-static', + }), + ]), + ); }); it('.services.nginx should have the correct configuration', () => { const { - config: { - services: { nginx }, - }, + services: { nginx }, } = getConfig(inputValues); // nginx is mapped from http://olympia.test to port 80 in /etc/hosts on the host expect(nginx.ports).toStrictEqual([ @@ -152,20 +156,29 @@ describe('docker-compose.yml', () => { source: 'data_nginx', target: '/etc/nginx/conf.d', }), - // mapping for /data/olympia/ directory to /srv + // 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({ + 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 { - config: { services }, - } = getConfig(inputValues); + 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 @@ -210,9 +223,7 @@ describe('docker-compose.yml', () => { }); it('.services.*.volumes does not contain anonymous or unnamed volumes', () => { - const { - config: { services }, - } = getConfig(inputValues); + const { services } = getConfig(inputValues); for (let [name, config] of Object.entries(services)) { for (let volume of config.volumes ?? []) { if (!volume.bind && !volume.source) { @@ -231,9 +242,7 @@ describe('docker-compose.yml', () => { // and should not be able to deviate from the state at build time. it('.services.(web|worker).environment excludes build info variables', () => { const { - config: { - services: { web, worker }, - }, + services: { web, worker }, } = getConfig({ ...inputValues, ...Object.fromEntries(EXCLUDED_KEYS.map((key) => [key, 'filtered'])), @@ -250,12 +259,16 @@ describe('docker-compose.yml', () => { const failKeys = [ // Invalid docker tag leads to docker not parsing the image 'DOCKER_TAG', + // 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 defaultEnv = runSetup(); const customValue = 'custom'; @@ -272,9 +285,7 @@ describe('docker-compose.yml', () => { } else if (ignoreKeys.includes(key)) { it('variable should be ignored', () => { const { - config: { - services: { web, worker }, - }, + services: { web, worker }, } = getConfig({ ...defaultEnv, [key]: customValue }); for (let service of [web, worker]) { expect(service.environment).not.toEqual( @@ -287,9 +298,7 @@ describe('docker-compose.yml', () => { } else { it('variable should be overriden based on the input', () => { const { - config: { - services: { web, worker }, - }, + services: { web, worker }, } = getConfig({ ...defaultEnv, [key]: customValue }); for (let service of [web, worker]) { expect(service.environment).toEqual( diff --git a/tests/make/test_install_deps.py b/tests/make/test_install_deps.py index 6ed420817136..2a4d6344b24a 100644 --- a/tests/make/test_install_deps.py +++ b/tests/make/test_install_deps.py @@ -1,13 +1,30 @@ import unittest from unittest import mock -from scripts.install_deps import main +from scripts.install_deps import copy_package_json, main from tests import override_env +@mock.patch('scripts.install_deps.shutil.copy') +class TestCopyPackageJson(unittest.TestCase): + def test_copy_package_json(self, mock_shutil): + copy_package_json() + assert mock_shutil.call_args_list == [ + mock.call('/data/olympia/package.json', '/deps'), + mock.call('/data/olympia/package-lock.json', '/deps'), + ] + + def test_copy_package_json_no_files(self, mock_shutil): + mock_shutil.side_effect = IOError + copy_package_json() + assert mock_shutil.call_args_list == [ + mock.call('/data/olympia/package.json', '/deps'), + ] + + class TestInstallDeps(unittest.TestCase): def setUp(self): - mocks = ['shutil.rmtree', 'os.listdir', 'subprocess.run'] + mocks = ['shutil.rmtree', 'os.listdir', 'subprocess.run', 'copy_package_json'] self.mocks = {} for mock_name in mocks: patch = mock.patch( @@ -36,8 +53,6 @@ def _test_remove_existing_deps(self, args, expect_remove=False): with override_env( **{ 'PIP_COMMAND': 'pip-test', - 'DEPS_DIR': '/data/olympia/deps', - 'NPM_DEPS_DIR': '/data/olympia/deps/npm', 'NPM_ARGS': 'npm-test', **args, } @@ -47,7 +62,8 @@ def _test_remove_existing_deps(self, args, expect_remove=False): if expect_remove: assert self.mocks['os.listdir'].called assert self.mocks['shutil.rmtree'].call_args_list == [ - mock.call('/data/olympia/deps/lib'), + mock.call('/deps/lib'), + mock.call('/deps/node_modules'), ] else: assert not self.mocks['os.listdir'].called @@ -93,6 +109,11 @@ def test_remove_deps_for_prod_deps_in_prod(self): } self._test_remove_existing_deps(args, expect_remove=True) + def test_copy_package_json_called(self): + """Test that copy_package_json is called""" + main(['prod']) + assert self.mocks['copy_package_json'].called + @override_env(PIP_COMMAND='pip-test', NPM_ARGS='npm-test') def test_pip_command_set_on_environment(self): main(['prod']) @@ -144,8 +165,10 @@ def test_correct_args_passed_to_subprocesses(self): '--no-save', '--no-audit', '--no-fund', + '--prefix', + '/deps/', '--cache', - '/data/olympia/deps/cache/npm', + '/deps/cache/npm', '--loglevel', 'verbose', '--include', diff --git a/tests/make/test_setup.py b/tests/make/test_setup.py index ed08f7aa8b4b..6656f64d66e3 100644 --- a/tests/make/test_setup.py +++ b/tests/make/test_setup.py @@ -2,7 +2,7 @@ import unittest from unittest import mock -from scripts.setup import get_docker_image_meta, main +from scripts.setup import get_docker_tag, main from tests import override_env @@ -10,6 +10,8 @@ 'DOCKER_TAG', 'DOCKER_TARGET', 'HOST_UID', + 'HOST_MOUNT', + 'HOST_MOUNT_SOURCE', 'OLYMPIA_DEPS', 'DEBUG', ] @@ -33,103 +35,83 @@ def setUp(self): @override_env() class TestGetDockerTag(BaseTestClass): def test_default_value_is_local(self): - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'mozilla/addons-server:local') - self.assertEqual(target, 'development') self.assertEqual(version, 'local') self.assertEqual(digest, None) - with override_env(DOCKER_TARGET='production'): - _, target, _, _ = get_docker_image_meta() - self.assertEqual(target, 'production') - @override_env(DOCKER_VERSION='test') def test_version_overrides_default(self): - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'mozilla/addons-server:test') - self.assertEqual(target, 'production') self.assertEqual(version, 'test') self.assertEqual(digest, None) @override_env(DOCKER_DIGEST='sha256:123') def test_digest_overrides_version_and_default(self): - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'mozilla/addons-server@sha256:123') - self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') with override_env(DOCKER_VERSION='test', DOCKER_DIGEST='sha256:123'): - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'mozilla/addons-server@sha256:123') - self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') @override_env(DOCKER_TAG='image:latest') def test_tag_overrides_default_version(self): - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'image:latest') - self.assertEqual(target, 'production') self.assertEqual(version, 'latest') self.assertEqual(digest, None) with override_env(DOCKER_TAG='image:latest', DOCKER_VERSION='test'): - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'image:test') - self.assertEqual(target, 'production') self.assertEqual(version, 'test') self.assertEqual(digest, None) @override_env(DOCKER_TAG='image@sha256:123') def test_tag_overrides_default_digest(self): - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'image@sha256:123') - self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') with mock.patch.dict(os.environ, {'DOCKER_DIGEST': 'test'}): - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'image@test') - self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'test') def test_version_from_env_file(self): self.mock_get_env_file.return_value = {'DOCKER_TAG': 'image:latest'} - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'image:latest') - self.assertEqual(target, 'production') self.assertEqual(version, 'latest') self.assertEqual(digest, None) def test_digest_from_env_file(self): self.mock_get_env_file.return_value = {'DOCKER_TAG': 'image@sha256:123'} - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'image@sha256:123') - self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') @override_env(DOCKER_VERSION='') def test_default_when_version_is_empty(self): - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'mozilla/addons-server:local') - self.assertEqual(target, 'development') self.assertEqual(version, 'local') self.assertEqual(digest, None) - with override_env(DOCKER_VERSION='', DOCKER_TARGET='production'): - _, target, _, _ = get_docker_image_meta() - self.assertEqual(target, 'production') - @override_env(DOCKER_DIGEST='', DOCKER_TAG='image@sha256:123') def test_default_when_digest_is_empty(self): self.mock_get_env_file.return_value = {'DOCKER_TAG': 'image@sha256:123'} - tag, target, version, digest = get_docker_image_meta() + tag, version, digest = get_docker_tag() self.assertEqual(tag, 'image@sha256:123') - self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') @@ -182,6 +164,44 @@ def test_debug_override(self): 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_' + ) + + @override_env() class TestOlympiaDeps(BaseTestClass): def test_default_olympia_deps(self): @@ -209,15 +229,3 @@ def test_override_env_olympia_deps_development_on_target_development(self): def test_olympia_deps_override(self): main() self.assert_set_env_file_called_with(OLYMPIA_DEPS='test') - - -@override_env() -@mock.patch('scripts.setup.os.makedirs') -def test_make_dirs(mock_makedirs): - from scripts.setup import root - - main() - assert mock_makedirs.call_args_list == [ - mock.call(os.path.join(root, dir), exist_ok=True) - for dir in ['deps', 'site-static', 'static-build', 'storage'] - ]