⚡️(devx) improve "make bootstrap" setup time & update deps#745
Closed
sylvinus wants to merge 7 commits into
Closed
⚡️(devx) improve "make bootstrap" setup time & update deps#745sylvinus wants to merge 7 commits into
sylvinus wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR reorganizes the dev workflow into light/full stack variants across the Makefile and compose.yaml, introduces a shared uv-managed Python base image consumed by backend and MTA Dockerfiles, pins numerous container images to digests, adds a frontend node_modules dependency guardrail, extends CI workflows with new build inputs and MTA test jobs, and relocates environment files from env.d to deploy/env alongside refactored PaaS deployment scripts. ChangesDev stack, frontend checks, and shared Python runtime
Container image digest pinning
Environment file relocation and PaaS deployment scripts
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@compose.yaml`:
- Around line 271-301: Extract the duplicated worker settings from
worker-dev-light into a shared YAML anchor, keeping the service self-contained
without using extends. Define a base anchor for the common build, user, ulimits,
command, env_file, volumes, and mem_limit fields, then merge it into worker-dev
and worker-dev-light; preserve each service’s specific depends_on and profiles,
and remember that worker-dev-light must override environment while repeating
DJANGO_CONFIGURATION alongside OPENSEARCH_INDEX_THREADS.
In `@Makefile`:
- Around line 168-198: The start target leaves full-stack services running when
switching back from start-full, so update the start stop step to include the
orphan containers started by start-full. In the Makefile’s start and start-full
targets, make sure the stop command covers opensearch, objectstorage,
mailcatcher, mta-in-py, and mpa in addition to backend-dev, worker-dev,
worker-ui, so the light stack starts from a clean state.
In `@src/e2e/Dockerfile`:
- Line 7: The Docker image currently uses Node 24.18.0 while `@playwright/test`
is still pinned to 1.57.0, which can cause `npm install` to hang during
`playwright install --with-deps`. Update the Playwright version in the e2e setup
to 1.60.0 or newer, or change the base image in `src/e2e/Dockerfile` to a Node
version below 24.16.0, so the `npm install` flow completes reliably.
In `@src/frontend/Dockerfile`:
- Around line 12-21: The frozen frontend install path is missing the same
dependency policy check used by the normal install flow. Update the
frontend-deps stage in the Dockerfile and the install-frozen-front target in the
Makefile so they run the check:deps guardrail before or alongside npm ci,
matching the behavior already present in install-front. Use the existing
frontend install symbols (frontend-deps, install-frozen-front, and check:deps)
to keep the frozen/production path aligned with the package policy.
In `@src/frontend/scripts/check-node-modules.mjs`:
- Around line 39-60: The bare catch in walk suppresses directory read failures
and can silently truncate the scan. Update walk() in check-node-modules.mjs so
readdirSync errors are surfaced or recorded with enough context to mark the scan
as incomplete, instead of immediately returning; keep the existing traversal
logic for entries, flagIfForbidden, and bytes accounting, but make partial
failures visible to the caller or log them explicitly.
- Around line 18-19: The NODE_MODULES_LIMIT_GB parsing in check-node-modules.mjs
can turn malformed input into NaN and silently disable the size check. Update
the limit initialization around LIMIT_GB and LIMIT_BYTES to validate the
environment value explicitly, and make the script fail loudly with a clear error
when NODE_MODULES_LIMIT_GB is missing, non-numeric, or non-finite. Keep the
guardrail behavior in the main size-check logic unchanged, but ensure any
invalid limit is detected before the comparison so the check cannot no-op.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c832dd05-a245-49b9-aa8c-685025bc15a8
⛔ Files ignored due to path filters (2)
src/e2e/package-lock.jsonis excluded by!**/package-lock.jsonsrc/frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
Makefilecompose.yamlsrc/e2e/Dockerfilesrc/e2e/package.jsonsrc/frontend/.npmrcsrc/frontend/Dockerfilesrc/frontend/package.jsonsrc/frontend/scripts/check-node-modules.mjssrc/jmap-email/Dockerfilesrc/keycloak/Dockerfilesrc/mpa/rspamd/Dockerfilesrc/mpa/tests/Dockerfilesrc/mta-in/Dockerfilesrc/mta-out/Dockerfilesrc/socks-proxy/tests/Dockerfile
sylvinus
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/messages.yml:
- Around line 88-117: The new test jobs use actions/checkout@v6 without
disabling credential persistence, which leaves the GITHUB_TOKEN available to
subsequent make test-* commands. Update each checkout step in the test-mta-in,
test-mta-in-py, and test-mta-out jobs to set persist-credentials to false on the
Checkout repository action so the token is not written into .git/config.
In `@docker/python-uv/Dockerfile`:
- Line 15: The base image reference in the Dockerfile is still a mutable tag,
which leaves a reproducibility gap. Update the FROM declaration that uses
debian:trixie-slim to a digest-pinned reference, matching the supply-chain
approach already used for the uv image, and keep the rest of the build logic
unchanged.
In `@src/mta-out/Dockerfile`:
- Around line 90-93: The current hardening step only strips the uv-managed
interpreter via strip-python and leaves pip available from the copied /venv.
Update the runtime hardening so the Dockerfile’s strip-python step (or the
strip-python.sh helper it invokes) also removes the venv-installed pip
entrypoints and pip package metadata under /venv, using the existing
strip-python flow and related /venv copy in the Dockerfile as the place to
verify the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 398f3a31-782c-4345-a134-834f3e2fac5e
⛔ Files ignored due to path filters (4)
src/backend/uv.lockis excluded by!**/*.locksrc/e2e/package-lock.jsonis excluded by!**/package-lock.jsonsrc/mta-in/uv.lockis excluded by!**/*.locksrc/mta-out/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.github/workflows/docker-publish.yml.github/workflows/messages-ghcr.yml.github/workflows/messages.ymlMakefilebin/release-jmap-email.shcompose.yamldocker/python-uv/Dockerfiledocker/python-uv/strip-python.shsrc/backend/Dockerfilesrc/backend/pyproject.tomlsrc/e2e/package.jsonsrc/frontend/scripts/check-node-modules.mjssrc/jmap-email/CONTRIBUTING.mdsrc/jmap-email/Dockerfilesrc/jmap-email/README.mdsrc/jmap-email/pyproject.tomlsrc/mta-in/Dockerfilesrc/mta-in/Dockerfile.pymtasrc/mta-in/pyproject.tomlsrc/mta-out/Dockerfilesrc/mta-out/pyproject.toml
Comment on lines
+90
to
+93
| # Hardening: whiteout pip/tkinter/tests/headers from the runtime (see | ||
| # strip-python). Not a size change (full Python is in an inherited layer), just | ||
| # keeps those tools out of the running container. | ||
| RUN strip-python |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
strip-python does not remove pip from the /venv virtual environment.
strip-python.sh targets $PYDIR (the uv-managed interpreter at /opt/python/...), not /venv. After COPY --from=base-with-deps /venv /venv (line 95), the venv's own pip remains accessible in the runtime image. If the intent is to keep pip out of the running container, consider also removing /venv/bin/pip* and the pip dist-info from /venv/lib/.
🔒 Proposed addition to strip-python.sh
rm -f /bin/uv /bin/uvx
+
+# Also strip pip from the venv if it exists (runtime-prod images copy /venv
+# after strip-python runs, but the venv's pip is separate from $PYDIR's).
+rm -rf \
+ /venv/bin/pip* \
+ /venv/lib/python3.*/site-packages/pip \
+ /venv/lib/python3.*/site-packages/pip-*.dist-info🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mta-out/Dockerfile` around lines 90 - 93, The current hardening step only
strips the uv-managed interpreter via strip-python and leaves pip available from
the copied /venv. Update the runtime hardening so the Dockerfile’s strip-python
step (or the strip-python.sh helper it invokes) also removes the venv-installed
pip entrypoints and pip package metadata under /venv, using the existing
strip-python flow and related /venv copy in the Dockerfile as the place to
verify the fix.
jbpenrath
reviewed
Jul 8, 2026
Contributor
There was a problem hiding this comment.
What was the root of the issue ?
Member
Author
There was a problem hiding this comment.
mismatched old deps, 3.2GB node_modules pulling @adobe .. I've opened issues in ui-kit & cunningham
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/mta-in/Dockerfile.pymta (1)
96-109: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
strip-pythonruns before/venvis copied — venv pip survives in the production runtime.In
runtime-prod,strip-python(line 104) targets/opt/pythonbut executes beforeCOPY --from=base-with-deps /venv /venv(line 108). The venv's ownpipentrypoints and metadata are therefore present in the final image, contradicting the hardening intent described in the comment on lines 99-102. This same ordering affectssrc/mta-out/Dockerfile(line 93) and likelysrc/backend/Dockerfileas well.Even if
strip-python.shwas updated to handle/venv, the current Dockerfile ordering would still leave pip accessible because/venvis copied after stripping. The fix needs a Dockerfile-level change: either reorder theCOPYbeforestrip-python(and ensurestrip-python.shhandles/venv), or add an explicit removal step after the copy.🔒 Proposed fix: strip venv pip after COPY
USER root RUN strip-python -USER ${DOCKER_USER} - -# The distroless target below copies in an already-stripped Python instead. COPY --from=base-with-deps /venv /venv +RUN rm -rf /venv/bin/pip* \ + /venv/lib/python3.*/site-packages/pip \ + /venv/lib/python3.*/site-packages/pip-*.dist-info +USER ${DOCKER_USER} + COPY --from=app-prod /app/src /app/src#!/bin/bash # Check whether strip-python.sh handles /venv and the ordering in all prod Dockerfiles cat deploy/python-uv/strip-python.sh echo "=== Ordering in production Dockerfiles ===" for f in src/backend/Dockerfile src/mta-in/Dockerfile src/mta-in/Dockerfile.pymta src/mta-out/Dockerfile; do echo "--- $f ---" grep -n 'strip-python\|COPY.*\/venv' "$f" done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mta-in/Dockerfile.pymta` around lines 96 - 109, The production runtime ordering in runtime-prod leaves the /venv pip artifacts unstripped because strip-python runs before the COPY of /venv, so fix the Dockerfile flow in Dockerfile.pymta by ensuring the venv is copied before hardening or by adding a post-COPY removal step that strips /venv as well as /opt/python. Use the existing runtime-prod stage and strip-python step as the anchor for the change, and make the same ordering adjustment in the other production Dockerfiles that follow this pattern so the final image does not retain pip entrypoints or metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/mta-in/Dockerfile.pymta`:
- Around line 96-109: The production runtime ordering in runtime-prod leaves the
/venv pip artifacts unstripped because strip-python runs before the COPY of
/venv, so fix the Dockerfile flow in Dockerfile.pymta by ensuring the venv is
copied before hardening or by adding a post-COPY removal step that strips /venv
as well as /opt/python. Use the existing runtime-prod stage and strip-python
step as the anchor for the change, and make the same ordering adjustment in the
other production Dockerfiles that follow this pattern so the final image does
not retain pip entrypoints or metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 99e4092e-f237-455c-ba4a-fe3b2f64990d
📒 Files selected for processing (46)
.dockerignore.github/workflows/crowdin_download.yml.github/workflows/messages-ghcr.yml.gitignoreMakefileProcfilebin/release-jmap-email.shbin/scalingo_postcompilebin/scalingo_postfrontendcompose.yamldeploy/env/backend.defaultsdeploy/env/backend.e2edeploy/env/crowdin.defaultsdeploy/env/frontend.defaultsdeploy/env/frontend.e2edeploy/env/keycloak.defaultsdeploy/env/keycloak.e2edeploy/env/mta-in-py.defaultsdeploy/env/mta-in.defaultsdeploy/env/mta-in.e2edeploy/env/mta-out.defaultsdeploy/env/postgresql.defaultsdeploy/env/socks-proxy.defaultsdeploy/paas/scalingo_postcompiledeploy/paas/scalingo_postfrontenddeploy/paas/scalingo_run_webdeploy/python-uv/Dockerfiledeploy/python-uv/strip-python.shdocs/env.mddocs/self-hosting.mdsrc/backend/Dockerfilesrc/backend/entrypointsrc/backend/messages/settings.pysrc/e2e/.npmrcsrc/e2e/Dockerfilesrc/e2e/README.mdsrc/e2e/compose.yamlsrc/frontend/.npmrcsrc/frontend/Dockerfilesrc/jmap-email/Dockerfilesrc/mpa/rspamd/Dockerfilesrc/mpa/tests/Dockerfilesrc/mta-in/Dockerfilesrc/mta-in/Dockerfile.pymtasrc/mta-out/Dockerfilesrc/socks-proxy/tests/Dockerfile
💤 Files with no reviewable changes (4)
- .github/workflows/crowdin_download.yml
- deploy/python-uv/strip-python.sh
- bin/release-jmap-email.sh
- deploy/python-uv/Dockerfile
Member
Author
|
merged as f4d2358 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrade or pin a few dependencies & add a "light" container set by default
Use a shared python base image
Upgrade dependencies
Summary by CodeRabbit
New Features
Bug Fixes
Chores