diff --git a/.github/actions/build-docker/action.yml b/.github/actions/build-docker/action.yml index e34e16549c45..ffa28057b904 100644 --- a/.github/actions/build-docker/action.yml +++ b/.github/actions/build-docker/action.yml @@ -66,12 +66,13 @@ runs: - name: Create .env and version.json files shell: bash - env: - DOCKER_TARGET: ${{ inputs.target }} - DOCKER_VERSION: ${{ steps.meta.outputs.version }} - DOCKER_COMMIT: ${{ steps.context.outputs.git_sha }} - DOCKER_BUILD: ${{ steps.context.outputs.git_build_url }} - run: make setup + run: | + echo "DOCKER_TARGET=${{ inputs.target }}" >> $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 + + make setup - name: Build Image id: build @@ -80,19 +81,16 @@ runs: targets: web files: | docker-compose.yml + docker-bake.hcl + .env ${{ steps.meta.outputs.bake-file-tags }} ${{ steps.meta.outputs.bake-file-labels }} ${{ steps.meta.outputs.bake-file-annotations }} push: ${{ inputs.push }} - set: | - *.cache-from=type=gha - *.cache-to=type=gha,mode=max,ignore-error=true - name: Get image digest id: build_meta shell: bash - env: - BUILDX_BAKE_METADATA_FILE: metadata.json run: | echo '${{ steps.build.outputs.metadata }}' > metadata.json echo "digest=$(cat metadata.json | jq -r '.web."containerimage.digest"')" >> $GITHUB_OUTPUT diff --git a/.github/actions/run-docker/action.yml b/.github/actions/run-docker/action.yml index 5bddffd82136..e8d3c519d2ad 100644 --- a/.github/actions/run-docker/action.yml +++ b/.github/actions/run-docker/action.yml @@ -28,9 +28,17 @@ runs: run: | echo "id=$(id -u)" >> $GITHUB_OUTPUT + - name: Set up Docker Buildx + id: buildx + uses: docker/setup-buildx-action@v1 + with: + version: latest + buildkitd-flags: --debug + - name: Run Docker Container shell: bash env: + DOCKER_BUILDER: ${{ steps.buildx.outputs.name}} DOCKER_VERSION: ${{ inputs.version }} DOCKER_DIGEST: ${{ inputs.digest }} COMPOSE_FILE: ${{ inputs.compose_file }} diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index 7ceb4d75f0ac..bfc8d3744b5e 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -41,7 +41,9 @@ jobs: - name: Needs Locale Compilation services: '' - run: make test_needs_locales_compilation + run: | + make compile_locales + make test_needs_locales_compilation - name: Static Assets services: '' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 349463678f42..98a6689664ac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -125,6 +125,27 @@ jobs: echo 'a * is born' echo 'wow an array []' + - name: Verify Build Metadata + uses: ./.github/actions/run-docker + env: + DOCKER_COMMIT: should_not_be_this + with: + digest: ${{ needs.build.outputs.digest }} + version: ${{ needs.build.outputs.version }} + run: | + expected_version="${{ needs.build.outputs.version }}" + expected_commit="${{ github.sha }}" + + if [ "$DOCKER_COMMIT" != "$expected_commit" ]; then + echo "DOCKER_COMMIT: '$DOCKER_COMMIT' is not equal to '$expected_commit'" + exit 1 + fi + + if [ "$DOCKER_VERSION" != "$expected_version" ]; then + echo "DOCKER_VERSION: '$DOCKER_VERSION' is not equal to '$expected_version'" + exit 1 + fi + docs_build: runs-on: ubuntu-latest needs: build diff --git a/Makefile-os b/Makefile-os index 5da6b3ddaf5b..daaa759a35c8 100644 --- a/Makefile-os +++ b/Makefile-os @@ -6,18 +6,21 @@ DOCKER_BUILDER ?= container DOCKER_PROGRESS ?= auto -DOCKER_PUSH ?= -BUILDX_BAKE_METADATA_FILE ?= export DOCKER_COMMIT ?= export DOCKER_BUILD ?= export DOCKER_VERSION ?= -BUILDX_BAKE_COMMAND := docker buildx bake web override DOCKER_MYSQLD_VOLUME = addons-server_data_mysqld override BACKUPS_DIR = $(shell pwd)/backups override EXPORT_DIR = $(BACKUPS_DIR)/$(shell date +%Y%m%d%H%M%S) RESTORE_DIR ?= $(BACKUPS_DIR)/$(shell ls -1 backups | sort -r | head -n 1) +DOCKER_COMPOSE_ARGS := \ + -d \ + --wait \ + --remove-orphans \ + --quiet-pull \ + # Paths should be cleaned before mounting .:/data/olympia # These are files which should be sourced from the container # or should be fresh on every run of the project @@ -83,35 +86,39 @@ create_docker_builder: ## Create a custom builder for buildkit to efficiently bu --name $(DOCKER_BUILDER) \ --driver=docker-container -BUILDX_BAKE_COMMAND += \ ---progress=$(DOCKER_PROGRESS) \ ---builder=$(DOCKER_BUILDER) \ - -ifneq ($(DOCKER_PUSH),) - BUILDX_BAKE_COMMAND += --push -else - BUILDX_BAKE_COMMAND += --load -endif - -ifneq ($(BUILDX_BAKE_METADATA_FILE),) - BUILDX_BAKE_COMMAND += --metadata-file=$(BUILDX_BAKE_METADATA_FILE) -endif - .PHONY: docker_compose_config docker_compose_config: ## Show the docker compose configuration @docker compose config web --format json -.PHONY: docker_build_args -docker_build_args: ## Show the docker build configuration - @echo $(BUILDX_BAKE_COMMAND) - -.PHONY: docker_build_config -docker_build_config: - @$(BUILDX_BAKE_COMMAND) --print - -.PHONY: build_docker_image -build_docker_image: create_docker_builder ## Build the docker image - $(BUILDX_BAKE_COMMAND) +.PHONY: docker_build_web ## Build the docker images using buildx bake +docker_build_web: create_docker_builder + docker buildx bake \ + --file docker-bake.hcl \ + --file .env \ + --builder $(DOCKER_BUILDER) \ + --progress $(DOCKER_PROGRESS) \ + $(ARGS); \ + +.PHONY: docker_pull_web ## Pull the latest docker image using current tag +docker_pull_web: + docker compose pull web --policy always + +.PHONY: docker_pull_or_build ## Pull or build the docker image based on the image version +docker_pull_or_build: +# If the image is tagged with version "local" then we should build the image before running +# docker compose up. The image will be available to docker compose, skipping a pull attempt. +# This is useful for local development where the image is built and tagged with "local". +# Also for CI/CID pipelines on forks where we cannot pull the image and must build locally. +# If the image is tagged with a version other than "local" then we should skip the build +# and let docker compose pull the image instead. This is useful for CI/CD pipelines where +# the image is already built and pushed to a registry. + @IMAGE=$$(docker compose config web --format json | jq -r '.services.web.image'); \ + echo "image: $$IMAGE"; \ + if echo "$$IMAGE" | grep -q ":local"; then \ + $(MAKE) docker_build_web; \ + else \ + $(MAKE) docker_pull_web; \ + fi .PHONY: docker_mysqld_volume_create docker_mysqld_volume_create: ## Create the mysqld volume @@ -146,26 +153,10 @@ clean_docker: docker_compose_down docker_mysqld_volume_remove docker_clean_image .PHONY: docker_compose_up docker_compose_up: docker_mysqld_volume_create ## Start the docker containers - docker compose up $(DOCKER_SERVICES) -d --wait --remove-orphans --quiet-pull $(ARGS) - docker compose rm -f olympia - -.PHONY: docker_extract_deps -docker_extract_deps: ## Extract dependencies from the docker image to a local volume mount -# Run a fresh container from the base image to install deps. Since /deps is -# shared via a volume in docker-compose.yml, this installs deps for both web -# and worker containers, and does so without requiring the containers to be up. -# We just create dummy empty package.json and package-lock.json in deps/ so -# that docker compose doesn't create dummy ones itself, as they would be owned -# by root. They don't matter: the ones at the root directory are mounted -# instead. - touch deps/package.json - touch deps/package-lock.json - # mounting ./deps:/deps effectively removes dependencies from the /deps directory in the container - # running `update_deps` will install the dependencies in the /deps directory before running - docker compose run --rm --quiet-pull web make update_deps + docker compose up $(DOCKER_SERVICES) $(DOCKER_COMPOSE_ARGS) $(ARGS) .PHONY: up -up: setup docker_mysqld_volume_create docker_compose_up docker_clean_images docker_clean_volumes ## Create and start docker compose +up: setup docker_pull_or_build docker_compose_up docker_clean_images docker_clean_volumes ## Create and start docker compose .PHONY: down down: docker_compose_down docker_clean_images docker_clean_volumes ## Stop the docker containers and clean up non-peristent dangling resources diff --git a/docker-bake.hcl b/docker-bake.hcl new file mode 100644 index 000000000000..d00d71d50721 --- /dev/null +++ b/docker-bake.hcl @@ -0,0 +1,32 @@ +group "default" { + targets = ["web"] +} + +variable DOCKER_BUILD {} +variable DOCKER_COMMIT {} +variable DOCKER_VERSION {} +variable DOCKER_TARGET {} +variable DOCKER_TAG {} + +target "web" { + context = "." + dockerfile = "Dockerfile" + target = "${DOCKER_TARGET}" + tags = ["${DOCKER_TAG}"] + platforms = ["linux/amd64"] + args = { + DOCKER_COMMIT = "${DOCKER_COMMIT}" + DOCKER_VERSION = "${DOCKER_VERSION}" + DOCKER_BUILD = "${DOCKER_BUILD}" + } + pull = true + cache-to = [ + "type=gha,mode=max,scope=mozilla/addons-server/web" + ] + cache-from = ["type=gha,scope=mozilla/addons-server/web"] + + output = [ + "type=docker", + ] + +} diff --git a/docker-compose.yml b/docker-compose.yml index 4ed9c6443542..20b65355db2b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -25,8 +25,10 @@ x-env-mapping: &env x-olympia: &olympia <<: *env image: ${DOCKER_TAG:-} - # Ignore any linting saying we have an invalid value. - pull_policy: ${DOCKER_PULL_POLICY:-} + # We don't want docker compose to manage the image for us. + # We sometimes build the image locally and sometimes pull from a registry + # but docker compose should always assume the image is available. + pull_policy: never # We drop down to a different user through entrypoint.sh, but starting as # root allows us to fix the ownership of files generated at image build # time through the ./docker/entrypoint.sh script. @@ -37,17 +39,6 @@ x-olympia: &olympia services: olympia: <<: *olympia - build: - args: - DOCKER_COMMIT: ${DOCKER_COMMIT:-} - DOCKER_VERSION: ${DOCKER_VERSION:-} - DOCKER_BUILD: ${DOCKER_BUILD:-} - context: . - dockerfile: Dockerfile - target: ${DOCKER_TARGET:-} - x-bake: - pull: true - platforms: linux/amd64 volumes: - data_deps:/deps - ./package.json:/deps/package.json diff --git a/scripts/setup.py b/scripts/setup.py index a5bb8c4cc679..8508dfd8ba7a 100755 --- a/scripts/setup.py +++ b/scripts/setup.py @@ -7,7 +7,7 @@ def set_env_file(values): with open('.env', 'w') as f: print('Environment:') for key, value in values.items(): - f.write(f'{key}={value}\n') + f.write(f'{key}="{value}"\n') print(f'{key}={value}') @@ -18,7 +18,7 @@ def get_env_file(): with open('.env', 'r') as f: for line in f: key, value = line.strip().split('=', 1) - env[key] = value + env[key] = value.strip('"') return env @@ -76,16 +76,14 @@ def get_docker_tag(): docker_tag, docker_version, docker_digest = get_docker_tag() -# set pull_policy of web/worker containers based on the specified tag -# for digest or non `local` versions, we should avoid building and pull aggressively -docker_pull_policy = 'always' if docker_digest or docker_version != 'local' else 'build' +docker_target = get_value('DOCKER_TARGET', 'development') +compose_file = get_value('COMPOSE_FILE', ('docker-compose.yml')) set_env_file( { - 'COMPOSE_FILE': get_value('COMPOSE_FILE', ('docker-compose.yml')), + 'COMPOSE_FILE': compose_file, 'DOCKER_TAG': docker_tag, - 'DOCKER_TARGET': get_value('DOCKER_TARGET', 'development'), - 'DOCKER_PULL_POLICY': docker_pull_policy, + 'DOCKER_TARGET': docker_target, 'HOST_UID': get_value('HOST_UID', os.getuid()), } ) diff --git a/tests/make/make.spec.js b/tests/make/make.spec.js index a33afc307572..859cae69208c 100644 --- a/tests/make/make.spec.js +++ b/tests/make/make.spec.js @@ -46,6 +46,62 @@ test('map docker compose config', () => { ); }); +describe('docker-bake.hcl', () => { + function getBakeConfig(env = {}) { + fs.writeFileSync(envPath, ''); + runSetup(env); + const { stdout: output } = spawnSync( + 'make', + ['docker_build_web', 'ARGS=--print'], + { + encoding: 'utf-8', + env: { ...process.env, ...env }, + }, + ); + + return output; + } + it('renders empty values for undefined variables', () => { + const output = getBakeConfig(); + expect(output).toContain('"DOCKER_BUILD": ""'); + expect(output).toContain('"DOCKER_COMMIT": ""'); + expect(output).toContain('"DOCKER_VERSION": ""'); + expect(output).toContain('"target": "development"'); + expect(output).toContain('mozilla/addons-server:local'); + }); + + it('renders custom DOCKER_BUILD', () => { + const build = 'build'; + const output = getBakeConfig({ DOCKER_BUILD: build }); + expect(output).toContain(`"DOCKER_BUILD": "${build}"`); + }); + + it('renders custom DOCKER_COMMIT', () => { + const commit = 'commit'; + const output = getBakeConfig({ DOCKER_COMMIT: commit }); + expect(output).toContain(`"DOCKER_COMMIT": "${commit}"`); + }); + + it('renders custom DOCKER_VERSION', () => { + const version = 'version'; + const output = getBakeConfig({ DOCKER_VERSION: version }); + expect(output).toContain(`"DOCKER_VERSION": "${version}"`); + expect(output).toContain(`mozilla/addons-server:${version}`); + }); + + it('renders custom DOCKER_DIGEST', () => { + const digest = 'sha256:digest'; + const output = getBakeConfig({ DOCKER_DIGEST: digest }); + expect(output).toContain(`mozilla/addons-server@${digest}`); + }); + + it('renders custom target', () => { + const target = 'target'; + const output = getBakeConfig({ DOCKER_TARGET: target }); + expect(output).toContain(`"target": "${target}"`); + }); +}); + function standardPermutations(name, defaultValue) { return [ { @@ -144,45 +200,10 @@ describe.each(testCases)('.env file', ({ name, file, env, expected }) => { }); }); -describe.each([ - { - version: 'local', - digest: undefined, - expected: 'build', - }, - { - version: 'local', - digest: 'sha256:123', - expected: 'always', - }, - { - version: 'latest', - digest: undefined, - expected: 'always', - }, -])('DOCKER_PULL_POLICY', ({ version, digest, expected }) => { - it(`is set to ${expected} when version is ${version} and digest is ${digest}`, () => { - fs.writeFileSync(envPath, ''); - runSetup({ - DOCKER_VERSION: version, - DOCKER_DIGEST: digest, - }); - - const actual = readEnvFile('DOCKER_PULL_POLICY'); - expect(actual).toStrictEqual(expected); - }); -}); - const testedKeys = new Set(testCases.map(({ name }) => name)); // Keys testsed outside the scope of testCases -const skippedKeys = [ - 'DOCKER_PULL_POLICY', - 'DOCKER_COMMIT', - 'DOCKER_VERSION', - 'DOCKER_BUILD', - 'PWD', -]; +const skippedKeys = ['DOCKER_COMMIT', 'DOCKER_VERSION', 'DOCKER_BUILD', 'PWD']; test('All dynamic properties in any docker compose file are referenced in the test', () => { const composeFiles = globSync('docker-compose*.yml', { cwd: rootPath });