[#1779] Fixed Docker parallel build order. #1780
Conversation
|
""" WalkthroughThe changes update Docker Compose configuration and related JSON fixture files by adding an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DockerCompose
participant CLI_Service
participant Nginx/PHP/Solr_Service
User->>DockerCompose: docker compose build
DockerCompose->>CLI_Service: Build cli image first
DockerCompose->>Nginx/PHP/Solr_Service: Build referencing cli service context
Nginx/PHP/Solr_Service->>CLI_Service: Use cli service context for build
DockerCompose-->>User: Build completes successfully
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (5)
🧰 Additional context used🧠 Learnings (3).vortex/tests/bats/fixtures/docker-compose.env.json (1).vortex/tests/bats/fixtures/docker-compose.env_mod.json (1).vortex/tests/bats/fixtures/docker-compose.noenv.json (1)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
🔇 Additional comments (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1780 +/- ##
========================================
Coverage 72.96% 72.96%
========================================
Files 83 83
Lines 4590 4590
Branches 35 35
========================================
Hits 3349 3349
Misses 1241 1241 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
.github/workflows/vortex-test-common.yml (2)
112-113: See comment on lines 22-24 – same concern and recommended fix.
172-173: See comment on lines 22-24 – same concern and recommended fix..github/workflows/build-test-deploy.yml (2)
168-169: Same issue as lines 58-60 – apply the chosen mitigation consistently.
367-368: Same issue as lines 58-60 – apply the chosen mitigation consistently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.circleci/config.yml(1 hunks).github/workflows/build-test-deploy.yml(3 hunks).github/workflows/vortex-test-common.yml(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Composer dependencies in Vortex Drupal projects are automatically updated with compatibility checks via RenovateBot; Node.js dependencies are updated in theme directories, and Docker images are updated for containers.
.github/workflows/vortex-test-common.yml (2)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Automated deployment in Vortex Drupal projects can be configured with GitHub Actions, CircleCI, Lagoon, Acquia, or container registry-based workflows.
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Composer dependencies in Vortex Drupal projects are automatically updated with compatibility checks via RenovateBot; Node.js dependencies are updated in theme directories, and Docker images are updated for containers.
.circleci/config.yml (1)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Automated deployment in Vortex Drupal projects can be configured with GitHub Actions, CircleCI, Lagoon, Acquia, or container registry-based workflows.
.github/workflows/build-test-deploy.yml (1)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Automated deployment in Vortex Drupal projects can be configured with GitHub Actions, CircleCI, Lagoon, Acquia, or container registry-based workflows.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-docs
| container: | ||
| image: drevops/ci-runner:25.5.0 | ||
| image: drevops/ci-runner:canary | ||
| env: |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Canary tag → non-deterministic builds – pin or parameterise the runner image
Switching to the floating canary tag means every workflow run may pull a different image, breaking reproducibility and making roll-backs harder when the canary image changes.
Prefer a fixed semver tag or, at minimum, surface the version via a workflow variable so you can bump it deliberately:
- image: drevops/ci-runner:canary
+ image: drevops/ci-runner:${{ vars.CI_RUNNER_VERSION || '25.5.0' }}(Apply the same pattern to all jobs in this file.)
Also double-check that the canary image still contains the tooling versions your scripts expect.
🏁 Script executed:
#!/bin/bash
echo "Occurrences of drevops/ci-runner references:"
rg 'drevops/ci-runner:' -n .
echo
echo "Occurrences of CI_RUNNER_VERSION usage:"
rg 'CI_RUNNER_VERSION' -n .
echo
echo "vars: blocks in .github/workflows files:"
rg '^\s*vars:' -n .github/workflowsLength of output: 389
🏁 Script executed:
#!/bin/bash
echo "Listing .github/workflows directory structure:"
find .github/workflows -maxdepth 1 -type f -printf "%f\n"
echo
echo "Searching for 'ci-runner' in all workflow files:"
rg 'ci-runner' -n .github/workflows
echo
echo "Searching for 'drevops/ci-runner' in all workflow files:"
rg 'drevops/ci-runner' -n .github/workflowsLength of output: 1716
Pin or parameterise the drevops/ci-runner image version
Switching to a floating canary tag introduces non-deterministic builds and makes roll-backs harder. Replace all instances of :canary with a fixed semver or a workflow variable:
• .github/workflows/vortex-test-common.yml – lines 23, 112, 172
• .github/workflows/build-test-deploy.yml – lines 59, 168, 367
- image: drevops/ci-runner:canary
+ image: drevops/ci-runner:${{ vars.CI_RUNNER_VERSION || '25.5.0' }}- Define
CI_RUNNER_VERSIONunder a top-levelvars:block in each workflow. - Verify your chosen runner image still contains the tooling versions your scripts expect.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| container: | |
| image: drevops/ci-runner:25.5.0 | |
| image: drevops/ci-runner:canary | |
| env: | |
| container: | |
| image: drevops/ci-runner:${{ vars.CI_RUNNER_VERSION || '25.5.0' }} | |
| env: |
🤖 Prompt for AI Agents
In .github/workflows/vortex-test-common.yml around lines 22 to 24, replace the
floating image tag "canary" for drevops/ci-runner with a fixed semantic version
or a workflow variable. Define a top-level vars block in the workflow file and
add a variable like CI_RUNNER_VERSION with the chosen fixed version. Then update
all references to the image tag ":canary" to use this variable instead. Verify
the selected image version includes all required tooling for your scripts.
| container: | ||
| image: drevops/ci-runner:25.5.0 | ||
| image: drevops/ci-runner:canary | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
canary image may introduce breaking changes unnoticed
All CI logic downstream (Composer, PHPStan, etc.) assumes specific toolchain versions. A moving canary image can silently upgrade those tools and break builds.
Pin to a digest or expose the version via an input/variable so upgrades are explicit and reviewable.
Also note cache keys below (e.g. v25.5.0-db11-…, line 121) still reference the old version – consider aligning them or making them version-agnostic.
I’ll verify the workflow file for the moving canary tag and the cache key versions.
🏁 Script executed:
#!/bin/bash
# Locate the canary image reference
rg -n "ci-runner:canary" -n .github/workflows/build-test-deploy.yml
# Inspect cache key lines around where versions appear (e.g. line ~121)
rg -n "key:" -n .github/workflows/build-test-deploy.yml | sed -n '115,130p'Length of output: 266
🏁 Script executed:
#!/bin/bash
# Show cache steps and keys with surrounding context
rg -n -C 5 'actions/cache@' .github/workflows/build-test-deploy.yml
# Find any explicit version strings (e.g., v25.5.0) in the workflow
rg -n 'v[0-9]\+\.[0-9]\+\.[0-9]\+' .github/workflows/build-test-deploy.ymlLength of output: 144
🏁 Script executed:
#!/bin/bash
# Extract lines 115–140 to inspect any cache key/version references
sed -n '115,140p' .github/workflows/build-test-deploy.ymlLength of output: 1533
Pin CI runner image and align cache version keys
The floating canary tag can silently pull breaking changes (PHP, Composer, PHPStan, etc.). Please:
- Pin the CI runner image to a digest or introduce a version variable/input for explicit, reviewable upgrades
• Locations:
–.github/workflows/build-test-deploy.yml:58–60, 168, 367
• Example:- image: drevops/ci-runner:canary + image: drevops/ci-runner@sha256:<DIGEST>
- Update the DB cache keys to match your pinned runner version (or make them version-agnostic)
• Currently at.github/workflows/build-test-deploy.yml:115–120:• Synckey: v25.5.0-db11-${{ hashFiles('db_cache_branch') }}-… restore-keys: | v25.5.0-db11-${{ hashFiles('db_cache_branch') }}-
v25.5.0to your runner version or remove the hard-coded prefix so cache invalidation remains intentional.
🤖 Prompt for AI Agents
In .github/workflows/build-test-deploy.yml around lines 58 to 60, the CI runner
image uses a floating 'canary' tag which can cause unintentional breaking
changes. Fix this by pinning the image to a specific digest or by introducing a
version variable/input for the image tag to allow explicit and reviewable
upgrades. Additionally, update the database cache keys at lines 115 to 120 to
either match the pinned runner version or remove the hard-coded version prefix
to ensure cache invalidation is intentional and consistent with the runner
version.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.docker/php.dockerfile (1)
11-14: Same unused-ARG / standalone-build breakage as insolr.dockerfileApply the same fix pattern to keep both hadolint happy and non-Compose builds working:
-ARG CLI_IMAGE -# hadolint ignore=DL3006 -FROM cli_image AS cli +ARG CLI_IMAGE=cli_image +# hadolint ignore=DL3006 +FROM ${CLI_IMAGE} AS cli.docker/nginx-drupal.dockerfile (1)
10-13: Align with the proposed fix – avoid unusable literal & unused ARGMirror the adjustment suggested for the other Dockerfiles:
-ARG CLI_IMAGE -# hadolint ignore=DL3006 -FROM cli_image AS cli +ARG CLI_IMAGE=cli_image +# hadolint ignore=DL3006 +FROM ${CLI_IMAGE} AS cli
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (5)
.vortex/installer/tests/Fixtures/install/_baseline/.docker/nginx-drupal.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.docker/php.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.docker/solr.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/version_replacement/baseline/php.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/version_replacement/expected/php.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (3)
.docker/nginx-drupal.dockerfile(1 hunks).docker/php.dockerfile(1 hunks).docker/solr.dockerfile(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Composer dependencies in Vortex Drupal projects are automatically updated with compatibility checks via RenovateBot; Node.js dependencies are updated in theme directories, and Docker images are updated for containers.
🪛 Checkov (3.2.334)
.docker/php.dockerfile
[LOW] 13-13: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
.docker/solr.dockerfile
[LOW] 10-10: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
.docker/nginx-drupal.dockerfile
[LOW] 12-12: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
| ARG CLI_IMAGE | ||
| # hadolint ignore=DL3006 | ||
| FROM ${CLI_IMAGE:-cli} AS cli | ||
| FROM cli_image AS cli | ||
|
|
There was a problem hiding this comment.
ARG CLI_IMAGE is now unused & FROM cli_image will fail outside Compose
ARG CLI_IMAGEis declared but never referenced – hadolint will raise DL3008 (ARG declared but not used).- The literal
cli_imageonly resolves when Docker Compose injects theadditional_contexts.
A plaindocker build .docker/run (e.g. in CI or during local debugging) will error with:
failed to resolve reference cli_image: pull access denied
Retain the new context alias while keeping the old override mechanism by defaulting the arg:
-ARG CLI_IMAGE
-# hadolint ignore=DL3006
-FROM cli_image AS cli
+ARG CLI_IMAGE=cli_image # BuildKit context alias remains the default
+# hadolint ignore=DL3006
+FROM ${CLI_IMAGE} AS cli # ← still overridable: `docker build --build-arg CLI_IMAGE=myregistry/cli:1.2`This eliminates the unused-ARG warning and restores backwards compatibility without impacting the Compose-based build order feature.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ARG CLI_IMAGE | |
| # hadolint ignore=DL3006 | |
| FROM ${CLI_IMAGE:-cli} AS cli | |
| FROM cli_image AS cli | |
| ARG CLI_IMAGE=cli_image # BuildKit context alias remains the default | |
| # hadolint ignore=DL3006 | |
| FROM ${CLI_IMAGE} AS cli # ← still overridable: `docker build --build-arg CLI_IMAGE=myregistry/cli:1.2` |
🧰 Tools
🪛 Checkov (3.2.334)
[LOW] 10-10: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
🤖 Prompt for AI Agents
In .docker/solr.dockerfile around lines 8 to 11, the ARG CLI_IMAGE is declared
but not used, causing a hadolint warning, and the FROM cli_image line fails
outside Docker Compose because cli_image is not defined. Fix this by using the
ARG CLI_IMAGE in the FROM statement with a default value, like FROM
${CLI_IMAGE:-cli_image} AS cli, so the ARG is used and the build works both
inside and outside Compose.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
.docker/solr.dockerfile (1)
9-9: Duplicate of earlier feedback – context alias still breaks plain buildsThis issue was already pointed out in a previous review (see history). The recommended fix (keep
ARG CLI_IMAGE=cli_imageand reference${CLI_IMAGE}) still applies..vortex/tests/bats/fixtures/docker-compose.env_mod.json (1)
227-230: Same compatibility check foradditional_contextsapplies hereReplicating the note on required Compose/BuildKit versions to avoid broken builds in environments that haven’t been upgraded yet.
Also applies to: 293-298, 356-361
.vortex/tests/bats/fixtures/docker-compose.env.json (2)
293-295: Same compatibility note applies hereSee comment above – the PHP service will also fail to build on older Compose.
356-358: Same compatibility note applies hereSee first comment.
docker-compose.yml (2)
144-146: Duplicate of the nginx noteThe same version guard applies.
189-191: Duplicate of the nginx noteSame version guard applies.
.vortex/tests/bats/fixtures/docker-compose.env_local.json (3)
227-229: Compatibility reminder duplicatedSee primary note in
.vortex/tests/bats/fixtures/docker-compose.env.json.
293-295: Compatibility reminder duplicatedSee primary note.
356-358: Compatibility reminder duplicatedSee primary note.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (10)
.vortex/installer/tests/Fixtures/install/_baseline/.docker/nginx-drupal.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.docker/php.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.docker/solr.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/.docker/nginx-drupal.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_clamav/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_solr/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_valkey/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_none/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (8)
.docker/nginx-drupal.dockerfile(1 hunks).docker/php.dockerfile(1 hunks).docker/solr.dockerfile(1 hunks).vortex/tests/bats/fixtures/docker-compose.env.json(3 hunks).vortex/tests/bats/fixtures/docker-compose.env_local.json(3 hunks).vortex/tests/bats/fixtures/docker-compose.env_mod.json(3 hunks).vortex/tests/bats/fixtures/docker-compose.noenv.json(3 hunks)docker-compose.yml(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Composer dependencies in Vortex Drupal projects are automatically updated with compatibility checks via RenovateBot; Node.js dependencies are updated in theme directories, and Docker images are updated for containers.
.vortex/tests/bats/fixtures/docker-compose.noenv.json (1)
Learnt from: AlexSkrypnyk
PR: drevops/vortex#0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in `.vortex/installer/tests/Fixtures/install` directory as they are test fixtures.
.vortex/tests/bats/fixtures/docker-compose.env_mod.json (1)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Composer dependencies in Vortex Drupal projects are automatically updated with compatibility checks via RenovateBot; Node.js dependencies are updated in theme directories, and Docker images are updated for containers.
.vortex/tests/bats/fixtures/docker-compose.env.json (2)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Composer dependencies in Vortex Drupal projects are automatically updated with compatibility checks via RenovateBot; Node.js dependencies are updated in theme directories, and Docker images are updated for containers.
Learnt from: AlexSkrypnyk
PR: drevops/vortex#0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in `.vortex/installer/tests/Fixtures/install` directory as they are test fixtures.
🪛 Checkov (3.2.334)
.docker/nginx-drupal.dockerfile
[LOW] 11-11: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
.docker/solr.dockerfile
[LOW] 9-9: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
.docker/php.dockerfile
[LOW] 12-12: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-docs
🔇 Additional comments (3)
.vortex/tests/bats/fixtures/docker-compose.noenv.json (1)
225-230: Confirm Compose / BuildKit version supportsadditional_contexts
additional_contextsrequires Docker Compose ≥ v2.22 & BuildKit. Make sure:
- Local dev containers & CI runners use a recent Compose release.
DOCKER_BUILDKIT=1is enabled in all environments.If any consumer is still on Compose v1 or older v2, these services will fail to build.
Also applies to: 292-297, 355-360
.vortex/tests/bats/fixtures/docker-compose.env.json (1)
227-229:additional_contextsis correctly wired, but double-check CI/Compose plugin versionThe new BuildKit feature requires Docker Compose v2.22+ (or CLI ≥ 24).
If CI or local developers are still on legacydocker-composev1 they will hit an
unknown-field error and the entire build will bail out early.Nothing to fix in the fixture itself, just ensure the minimum version gets enforced
(e.g. with an engine check in CI or a note in the docs/README).docker-compose.yml (1)
119-123: BuildKit feature ok, but guard against older Compose in production pipelines
additional_contextsis valid YAML and properly quoted; however, production
clusters still running Compose v1 (or Swarm stacks) will choke on the field.
Consider:+ # Requires docker compose v2.22+ / Engine 24+ + # Remove this guard once all environments are upgraded.or add a version check in your CI bootstrap.
AlexSkrypnyk
force-pushed
the
feature/fix-docker-services-order
branch
from
87b80c2 to
2cbc55b
Compare
AlexSkrypnyk
force-pushed
the
feature/fix-docker-services-order
branch
from
2cbc55b to
928053f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
docker-compose.yml (1)
140-146: Anchor hygieneIf
*cli-imageis no longer referenced after dropping theCLI_IMAGEarg, remove the anchor to avoid dead YAML artifacts.
♻️ Duplicate comments (3)
.docker/solr.dockerfile (1)
8-10:${CLI_IMAGE:-cli_image}may not parse in many Docker enginesBash-style default expansion isn’t universally supported in
FROM. Older/stock Docker (incl. most CI runners) will treat it literally and fail (invalid reference format).
Use anARGdefault instead:-ARG CLI_IMAGE -# hadolint ignore=DL3006 -FROM ${CLI_IMAGE:-cli_image} AS cli +ARG CLI_IMAGE=cli_image # default matches the BuildKit context alias +# hadolint ignore=DL3006 +FROM ${CLI_IMAGE} AS cli # still overridable via --build-argRestores standalone
docker buildcompatibility and silences hadolint DL3008..docker/php.dockerfile (1)
11-13: Portable default handling forCLI_IMAGESame concern as in the Solr Dockerfile:
${CLI_IMAGE:-cli_image}isn’t reliably expanded inFROM. Prefer anARGwith a default value:-ARG CLI_IMAGE -# hadolint ignore=DL3006 -FROM ${CLI_IMAGE:-cli_image} AS cli +ARG CLI_IMAGE=cli_image +# hadolint ignore=DL3006 +FROM ${CLI_IMAGE} AS cli.docker/nginx-drupal.dockerfile (1)
10-12: Fallback expression can break standalone buildsAdopt the same safe pattern used above to keep builds working on engines lacking
:-expansion:-ARG CLI_IMAGE -# hadolint ignore=DL3006 -FROM ${CLI_IMAGE:-cli_image} AS cli +ARG CLI_IMAGE=cli_image +# hadolint ignore=DL3006 +FROM ${CLI_IMAGE} AS cli
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (5)
.vortex/installer/tests/Fixtures/install/_baseline/.docker/nginx-drupal.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.docker/php.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.docker/solr.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/version_replacement/baseline/php.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/version_replacement/expected/php.dockerfileis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (4)
.docker/nginx-drupal.dockerfile(1 hunks).docker/php.dockerfile(1 hunks).docker/solr.dockerfile(1 hunks)docker-compose.yml(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T01:07:14.235Z
Learning: Composer dependencies in Vortex Drupal projects are automatically updated with compatibility checks via RenovateBot; Node.js dependencies are updated in theme directories, and Docker images are updated for containers.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-docs
🔇 Additional comments (1)
docker-compose.yml (1)
115-123: Ensure CI & docs require Compose V2 / BuildKit ≥ 0.11
additional_contextsis only recognised by recent Compose/BuildKit. Older runners silently ignore it, makingcli_imageunresolved. Verify your pipeline uses a modern Docker (or wrap with a version check).
| build: | ||
| context: . | ||
| dockerfile: .docker/solr.dockerfile | ||
| args: | ||
| CLI_IMAGE: *cli-image | ||
| additional_contexts: | ||
| cli_image: "service:cli" | ||
| environment: |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
DRY up repeated additional_contexts blocks
Consider a shared YAML anchor to avoid three nearly identical stanzas:
x-cli-context: &cli_context
additional_contexts:
cli_image: "service:cli"
nginx:
build:
<<: *cli_context
# …
php:
build:
<<: *cli_context
# …
solr:
build:
<<: *cli_context
# …Reduces duplication and future edit surfaces.
🤖 Prompt for AI Agents
In docker-compose.yml around lines 186 to 191, the additional_contexts block
with cli_image: "service:cli" is repeated in multiple build sections. To fix
this, define a YAML anchor (e.g., x-cli-context) at the top level with the
additional_contexts block, then reference it in each build section using the
merge key (<<: *cli_context) to avoid duplication and simplify future edits.
3c1d46f to
c88fb56
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
docker-compose.yml (3)
121-122: DRY up repeatedadditional_contextsblocks (see previous review).
144-146: DRY up repeatedadditional_contextsblocks (see previous review).
189-191: DRY up repeatedadditional_contextsblocks (see previous review).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
.vortex/installer/tests/Fixtures/install/_baseline/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_solr/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_none/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (1)
docker-compose.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
🔇 Additional comments (1)
docker-compose.yml (1)
115-123: Verified: Nocli_imagebuild stages in DockerfilesA search for
FROM …cli_imagereturned zero results, so theadditional_contexts: clialias is consistent and there are no lingeringcli_imagestages. No changes needed here.
| args: | ||
| CLI_IMAGE: *cli-image | ||
| WEBROOT: "${WEBROOT:-web}" | ||
| additional_contexts: | ||
| cli: "service:cli" | ||
| <<: [*default-volumes, *default-user] |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Left-over single-value args: block can be inlined or removed.
With only WEBROOT remaining you can drop the args: wrapper and keep the stanza short:
- args:
- WEBROOT: "${WEBROOT:-web}"
+ args: { WEBROOT: "${WEBROOT:-web}" }Purely cosmetic but keeps this section tidy.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| args: | |
| CLI_IMAGE: *cli-image | |
| WEBROOT: "${WEBROOT:-web}" | |
| additional_contexts: | |
| cli: "service:cli" | |
| <<: [*default-volumes, *default-user] | |
| args: { WEBROOT: "${WEBROOT:-web}" } | |
| additional_contexts: | |
| cli: "service:cli" | |
| <<: [*default-volumes, *default-user] |
🤖 Prompt for AI Agents
In docker-compose.yml around lines 119 to 123, the args block contains only a
single WEBROOT variable, making the args: wrapper unnecessary. Remove the args:
line and inline the WEBROOT variable directly at the same indentation level to
simplify and tidy the configuration.
70faea3 to
b93db0e
Compare
AlexSkrypnyk
changed the title
Closes #1779
Based on https://docs.docker.com/compose/how-tos/dependent-images/#use-another-services-image-as-the-base-image
Summary by CodeRabbit