diff --git a/.github/actions/build-docker/action.yml b/.github/actions/build-docker/action.yml index b5b77025cf5f..17c2c47eeb77 100644 --- a/.github/actions/build-docker/action.yml +++ b/.github/actions/build-docker/action.yml @@ -37,6 +37,7 @@ 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 @@ -51,33 +52,28 @@ 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 \ - ARGS="--file ${{ env.TAGS_FILE }} --file ${{ env.ANNOTATIONS_FILE }}" \ - DOCKER_PUSH=${{ inputs.push }} + DOCKER_COMMIT="${{ steps.context.outputs.git_sha }}" \ + DOCKER_BUILD="${{ steps.context.outputs.git_build_url }}" - name: Get image digest id: build_meta shell: bash run: | - metadata=$(cat $DOCKER_METADATA_FILE) + metadata=$(cat ${{ steps.context.outputs.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 1619f498eb95..dea9fdd64908 100644 --- a/.github/actions/run-docker/action.yml +++ b/.github/actions/run-docker/action.yml @@ -23,10 +23,6 @@ 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 @@ -44,7 +40,6 @@ 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 eebd0f900289..88ec6f90ecb6 100644 --- a/.github/workflows/_test_check.yml +++ b/.github/workflows/_test_check.yml @@ -46,7 +46,6 @@ jobs: name: | version: '${{ matrix.version }}' | target: '${{ matrix.target }}' | - mount: '${{ matrix.mount }}' | deps: '${{ matrix.deps }}' strategy: fail-fast: false @@ -57,9 +56,6 @@ jobs: target: - development - production - mount: - - development - - production deps: - development - production @@ -72,7 +68,6 @@ 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 @@ -84,7 +79,6 @@ jobs: with: version: ${{ matrix.version }} target: ${{ matrix.target }} - mount: ${{ matrix.mount }} deps: ${{ matrix.deps }} run: make check - name: Cached Build Check @@ -93,7 +87,6 @@ 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 38457e047a11..cd69c91de696 100644 --- a/.github/workflows/_test_main.yml +++ b/.github/workflows/_test_main.yml @@ -88,7 +88,6 @@ 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 d782354e3346..743afda17c0c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,7 +87,6 @@ jobs: with: digest: ${{ needs.build.outputs.digest }} version: ${{ needs.build.outputs.version }} - mount: development deps: development target: development run: | @@ -140,7 +139,6 @@ 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 d2a01bd4f2c1..1303a8a5131d 100644 --- a/.gitignore +++ b/.gitignore @@ -55,5 +55,3 @@ 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 21dcad79a82c..9dfdaf7feb16 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,13 +10,24 @@ 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 +make -f Makefile-docker update_assets EOF FROM base AS production @@ -194,4 +199,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 /deps +COPY --from=pip_production --chown=olympia:olympia ${DEPS_DIR} ${DEPS_DIR} diff --git a/Makefile-docker b/Makefile-docker index 87c2ef8c6bc2..2039d0d36a4e 100644 --- a/Makefile-docker +++ b/Makefile-docker @@ -8,16 +8,6 @@ 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 @@ -39,29 +29,12 @@ 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_nginx check_files check_olympia_user check_debian_packages check_pip_packages check_django +check: check_debian_packages check_pip_packages check_django .PHONY: data_dump data_dump: @@ -72,14 +45,8 @@ data_load: ./manage.py data_load $(ARGS) .PHONY: update_assets -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 - +update_assets: ## Update the static assets + $(HOME)/scripts/update_assets.py .PHONY: update_deps update_deps: ## Update the dependencies @@ -94,7 +61,7 @@ setup-ui-tests: lint: ## lint the code ruff check . ruff format --check . - NODE_PATH=$(NODE_MODULES) npm exec $(NPM_ARGS) -- prettier --check '**' + npm exec $(NPM_ARGS) -- prettier --check '**' curlylint src/ lint-codestyle: lint @@ -199,15 +166,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). - NODE_PATH=$(NODE_MODULES) npm exec $(NPM_ARGS) -- jest tests/js + npm exec $(NPM_ARGS) -- jest tests/js .PHONY: watch_js_tests watch_js_tests: ## Run+watch the JavaScript test suite (requires compiled/compressed assets). - NODE_PATH=$(NODE_MODULES) npm exec $(NPM_ARGS) -- jest --watch + npm exec $(NPM_ARGS) -- jest --watch .PHONY: format format: ## Autoformat our codebase. - NODE_PATH=$(NODE_MODULES) npm exec $(NPM_ARGS) -- prettier --write '**' + npm exec $(NPM_ARGS) -- prettier --write '**' ruff check --fix-only . ruff format . @@ -218,7 +185,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 - ./locale/compile-mo.sh ./locale/ + $(HOME)/scripts/compile_locales.py .PHONY: help_submake help_submake: diff --git a/Makefile-os b/Makefile-os index e3079503fcd3..e4dc9c6f1bdd 100644 --- a/Makefile-os +++ b/Makefile-os @@ -39,6 +39,14 @@ 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 \ @@ -153,17 +161,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_update_deps -docker_update_deps: docker_mysqld_volume_create ## Update the dependencies in the container based on the docker tag and target +.PHONY: docker_sync_host +docker_sync_host: 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 \ - make update_deps + ./scripts/sync_host_files.py .PHONY: up_pre -up_pre: setup docker_pull_or_build docker_update_deps ## Pre-up the environment, setup files, volumes and host state +up_pre: setup docker_pull_or_build docker_sync_host ## 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 deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/docker-compose.yml b/docker-compose.yml index 4e847cacc989..7e4caaaf81fe 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -45,10 +45,7 @@ services: command: ["sleep", "infinity"] volumes: # used by: web, worker, nginx - - ${HOST_MOUNT_SOURCE:?}:/data/olympia - - ${HOST_MOUNT_SOURCE:?}deps:/deps - - data_site_static:/data/olympia/site-static - - ${HOST_MOUNT_SOURCE:?}storage:/data/olympia/storage + - ./:/data/olympia worker: <<: *olympia command: [ @@ -63,9 +60,7 @@ services: "celery -A olympia.amo.celery:app worker -E -c 2 --loglevel=INFO", ] volumes: - - ${HOST_MOUNT_SOURCE:?}:/data/olympia - - ${HOST_MOUNT_SOURCE:?}deps:/deps - - ${HOST_MOUNT_SOURCE:?}storage:/data/olympia/storage + - ./:/data/olympia extra_hosts: - "olympia.test:127.0.0.1" restart: on-failure:5 @@ -93,19 +88,12 @@ 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 - - ${HOST_MOUNT_SOURCE:?}:/srv - - data_site_static:/srv/site-static - - ${HOST_MOUNT_SOURCE:?}storage:/srv/storage + - ./:/srv ports: - "80:80" networks: @@ -204,21 +192,6 @@ 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 e3f59fcdc489..06aca156b3c0 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -30,12 +30,6 @@ 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]) -> None: + def _clean_storage( + self, root: str, dir_dict: dict[str, str | dict], clean: bool = False + ) -> 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) + self._clean_storage(curr_path, value, clean=clean) else: - shutil.rmtree(curr_path, ignore_errors=True) + if clean: + shutil.rmtree(curr_path, ignore_errors=True) os.makedirs(curr_path, exist_ok=True) - def clean_storage(self): - self.logger.info('Cleaning storage...') - self._clean_storage(settings.STORAGE_ROOT, storage_structure) + def make_storage(self, clean: bool = False): + self.logger.info('Making storage...') + self._clean_storage(settings.STORAGE_ROOT, storage_structure, clean=clean) diff --git a/src/olympia/amo/management/commands/data_load.py b/src/olympia/amo/management/commands/data_load.py index 4973409f63c6..70969226f6c5 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.clean_storage() + self.make_storage(clean=True) call_command( 'mediarestore', diff --git a/src/olympia/amo/management/commands/data_seed.py b/src/olympia/amo/management/commands/data_seed.py index 33078c1f5e05..00a673a51e8c 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.clean_storage() + self.make_storage(clean=True) # 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 413b478ea388..620bde565c1a 100644 --- a/src/olympia/amo/management/commands/initialize.py +++ b/src/olympia/amo/management/commands/initialize.py @@ -73,6 +73,9 @@ 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 5bbaed663be5..ff7368dea6a7 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.clean_storage() + self.base_data_command.make_storage(clean=True) 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_clean_storage', - 'olympia.amo.management.commands.data_seed.BaseDataCommand.clean_storage', + 'mock_make_storage', + 'olympia.amo.management.commands.data_seed.BaseDataCommand.make_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_clean_storage'].assert_called_once() + self.mocks['mock_make_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 879b30868248..9d878f84781e 100644 --- a/src/olympia/core/apps.py +++ b/src/olympia/core/apps.py @@ -13,6 +13,8 @@ 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 @@ -93,16 +95,10 @@ def static_check(app_configs, **kwargs): try: call_command('compress_assets', dry_run=True, stdout=output) - file_paths = output.getvalue().strip().split('\n') + stripped_output = output.getvalue().strip() - if not file_paths: - errors.append( - Error( - 'No compressed asset files were found.', - id='setup.E003', - ) - ) - else: + if stripped_output: + file_paths = stripped_output.split('\n') for file_path in file_paths: if not os.path.exists(file_path): error = f'Compressed asset file does not exist: {file_path}' @@ -112,6 +108,14 @@ 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( @@ -151,6 +155,61 @@ 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 bb4c8b6d1ffe..8c6823aeca15 100644 --- a/src/olympia/core/tests/test_apps.py +++ b/src/olympia/core/tests/test_apps.py @@ -1,10 +1,14 @@ +import os +import tempfile from unittest import mock -from django.core.management import call_command +from django.core.management import CommandError, 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 @@ -20,11 +24,25 @@ 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 = ( @@ -85,3 +103,115 @@ 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 4bd6faee9c2d..ac9618ca3106 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -73,7 +73,6 @@ 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 @@ -98,15 +97,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='/deps/node_modules/less/bin/lessc') +LESS_BIN = env('LESS_BIN', default=path('node_modules/less/bin/lessc')) # Path to cleancss (our CSS minifier). CLEANCSS_BIN = env( - 'CLEANCSS_BIN', default='/deps/node_modules/clean-css-cli/bin/cleancss' + 'CLEANCSS_BIN', default=path('node_modules/clean-css-cli/bin/cleancss') ) # Path to our JS minifier. -JS_MINIFIER_BIN = env('JS_MINIFIER_BIN', default='/deps/node_modules/terser/bin/terser') +JS_MINIFIER_BIN = env('JS_MINIFIER_BIN', default=path('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') @@ -116,7 +115,7 @@ def path(*folders): # Path to our addons-linter binary ADDONS_LINTER_BIN = env( - 'ADDONS_LINTER_BIN', default='/deps/node_modules/addons-linter/bin/addons-linter' + 'ADDONS_LINTER_BIN', default=path('node_modules/addons-linter/bin/addons-linter') ) # --enable-background-service-worker linter flag value ADDONS_LINTER_ENABLE_SERVICE_WORKER = False @@ -1327,14 +1326,15 @@ def read_only_mode(env): 'django_node_assets.finders.NodeModulesFinder', ) -NODE_MODULES_ROOT = os.path.join('/', 'deps', 'node_modules') -NODE_PACKAGE_JSON = os.path.join('/', 'deps', 'package.json') +NODE_MODULES_ROOT = path('node_modules') +NODE_PACKAGE_JSON = path('package.json') NODE_PACKAGE_MANAGER_INSTALL_OPTIONS = ['--dry-run'] -STATIC_BUILD_PATH = os.path.join('/', 'data', 'olympia', 'static-build') +STATIC_BUILD_PATH = path('static-build') +STATIC_FILES_PATH = path('static') STATICFILES_DIRS = ( - path('static'), + STATIC_FILES_PATH, STATIC_BUILD_PATH, ) diff --git a/storage/.gitignore b/storage/.gitignore deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/make/make.spec.js b/tests/make/make.spec.js index 8c3abc779c5d..fb9739dec556 100644 --- a/tests/make/make.spec.js +++ b/tests/make/make.spec.js @@ -34,7 +34,10 @@ function getConfig(env = {}) { ); try { if (rawError) throw new Error(rawError); - return JSON.parse(rawConfig); + return { + config: JSON.parse(rawConfig), + env: rawEnv, + }; } catch (error) { throw new Error( JSON.stringify({ error, rawConfig, rawError, rawEnv }, null, 2), @@ -42,36 +45,45 @@ 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([ - ['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; + describe.each( + permutations({ + DOCKER_TARGET: ['development', 'production'], + DOCKER_VERSION: ['local', 'latest'], + }), + )('\n%s\n', (config) => { + const { DOCKER_TARGET, DOCKER_VERSION } = config; const inputValues = { DOCKER_TARGET, - OLYMPIA_MOUNT, - DOCKER_TAG: 'mozilla/addons-server:tag', + DOCKER_VERSION, DEBUG: 'debug', DATA_BACKUP_SKIP: 'skip', }; it('.services.(web|worker) should have the correct configuration', () => { const { - services: { web, worker }, + config: { + services: { web, worker }, + }, + env: { DOCKER_TAG }, } = getConfig(inputValues); for (let service of [web, worker]) { - expect(service.image).toStrictEqual(inputValues.DOCKER_TAG); + expect(service.image).toStrictEqual(DOCKER_TAG); expect(service.pull_policy).toStrictEqual('never'); expect(service.user).toStrictEqual('root'); expect(service.platform).toStrictEqual('linux/amd64'); @@ -101,18 +113,13 @@ describe('docker-compose.yml', () => { expect(service.volumes).toEqual( expect.arrayContaining([ expect.objectContaining({ - source: isProdMountTarget ? 'data_olympia_' : expect.any(String), + source: expect.any(String), target: '/data/olympia', }), - expect.objectContaining({ - source: isProdMountTarget - ? 'data_olympia_storage' - : expect.any(String), - target: '/data/olympia/storage', - }), ]), ); - const { OLYMPIA_MOUNT, ...environmentOutput } = inputValues; + const { DOCKER_VERSION, DOCKER_TARGET, ...environmentOutput } = + inputValues; expect(service.environment).toEqual( expect.objectContaining({ ...environmentOutput, @@ -121,24 +128,13 @@ 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 { - services: { nginx }, + config: { + services: { nginx }, + }, } = getConfig(inputValues); // nginx is mapped from http://olympia.test to port 80 in /etc/hosts on the host expect(nginx.ports).toStrictEqual([ @@ -156,29 +152,20 @@ describe('docker-compose.yml', () => { source: 'data_nginx', target: '/etc/nginx/conf.d', }), - // mapping for local host directory to /data/olympia + // mapping for /data/olympia/ directory to /srv expect.objectContaining({ - source: isProdMountTarget ? 'data_olympia_' : expect.any(String), + source: 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 { services } = getConfig(inputValues); + const { + config: { 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 @@ -223,7 +210,9 @@ describe('docker-compose.yml', () => { }); it('.services.*.volumes does not contain anonymous or unnamed volumes', () => { - const { services } = getConfig(inputValues); + const { + config: { services }, + } = getConfig(inputValues); for (let [name, config] of Object.entries(services)) { for (let volume of config.volumes ?? []) { if (!volume.bind && !volume.source) { @@ -242,7 +231,9 @@ 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 { - services: { web, worker }, + config: { + services: { web, worker }, + }, } = getConfig({ ...inputValues, ...Object.fromEntries(EXCLUDED_KEYS.map((key) => [key, 'filtered'])), @@ -259,16 +250,12 @@ 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'; @@ -285,7 +272,9 @@ describe('docker-compose.yml', () => { } else if (ignoreKeys.includes(key)) { it('variable should be ignored', () => { const { - services: { web, worker }, + config: { + services: { web, worker }, + }, } = getConfig({ ...defaultEnv, [key]: customValue }); for (let service of [web, worker]) { expect(service.environment).not.toEqual( @@ -298,7 +287,9 @@ describe('docker-compose.yml', () => { } else { it('variable should be overriden based on the input', () => { const { - services: { web, worker }, + config: { + 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 2a4d6344b24a..6ed420817136 100644 --- a/tests/make/test_install_deps.py +++ b/tests/make/test_install_deps.py @@ -1,30 +1,13 @@ import unittest from unittest import mock -from scripts.install_deps import copy_package_json, main +from scripts.install_deps import 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', 'copy_package_json'] + mocks = ['shutil.rmtree', 'os.listdir', 'subprocess.run'] self.mocks = {} for mock_name in mocks: patch = mock.patch( @@ -53,6 +36,8 @@ 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, } @@ -62,8 +47,7 @@ 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('/deps/lib'), - mock.call('/deps/node_modules'), + mock.call('/data/olympia/deps/lib'), ] else: assert not self.mocks['os.listdir'].called @@ -109,11 +93,6 @@ 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']) @@ -165,10 +144,8 @@ def test_correct_args_passed_to_subprocesses(self): '--no-save', '--no-audit', '--no-fund', - '--prefix', - '/deps/', '--cache', - '/deps/cache/npm', + '/data/olympia/deps/cache/npm', '--loglevel', 'verbose', '--include', diff --git a/tests/make/test_setup.py b/tests/make/test_setup.py index 6656f64d66e3..ed08f7aa8b4b 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_tag, main +from scripts.setup import get_docker_image_meta, main from tests import override_env @@ -10,8 +10,6 @@ 'DOCKER_TAG', 'DOCKER_TARGET', 'HOST_UID', - 'HOST_MOUNT', - 'HOST_MOUNT_SOURCE', 'OLYMPIA_DEPS', 'DEBUG', ] @@ -35,83 +33,103 @@ def setUp(self): @override_env() class TestGetDockerTag(BaseTestClass): def test_default_value_is_local(self): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() 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, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'image@sha256:123') + self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') @@ -164,44 +182,6 @@ 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): @@ -229,3 +209,15 @@ 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'] + ]