Add cache mounts to docker files#1452
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis change updates several Dockerfiles across backend, frontend, and documentation components to optimize dependency installation and caching. For Python-based images, the installation of Poetry is modified to remove the Changes
Assessment against linked issues
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
⏰ Context from checks skipped due to timeout of 90000ms (5)
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. 🪧 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
Documentation and Community
|
github-actions
bot
added
backend
frontend
docker
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (10)
backend/docker/Dockerfile (2)
6-6: Pin Poetry version & cache pip for reproducibility
Rather than installing the latest Poetry arbitrarily, pin to a known version (e.g.poetry==1.6.1) and consider mounting pip’s cache directory to speed uppip install poetryacross builds.Example diff:
- python -m pip install poetry + python -m pip install "poetry==1.6.1" +RUN --mount=type=cache,target=/root/.cache/pip \ + python -m pip install "poetry==1.6.1"
37-37: Avoid overwriting.venvpermissions on second copy
The subsequentCOPY /home/owaspwill merge in.venvagain, potentially resetting its mode to555. Instead, copy application files explicitly to prevent re-copying the virtualenv:- COPY --from=builder --chmod=555 --chown=owasp:owasp /home/owasp /home/owasp + COPY --from=builder --chmod=555 --chown=owasp:owasp \ + /home/owasp/apps /home/owasp/manage.py /home/owasp/wsgi.py \ + /home/owasp/settings /home/owasp/static /home/owasp/templates \ + /home/owasp/entrypoint.sh \ + /home/owasp/backend/docker/Dockerfile.local (3)
7-7: Pin Poetry version & cache pip in builder
Consider pinning Poetry (poetry==1.6.1) for reproducible builds and mounting pip’s cache to speed up installs:- python -m pip install poetry +RUN --mount=type=cache,target=/root/.cache/pip \ + python -m pip install "poetry==1.6.1"
27-27: Install Poetry in runtime stage may be unnecessary
In a local dev image you may need Poetry, but duplicating the install inflates final image size. Consider removing or copying the binary from the builder stage instead.
37-37: Prevent double-copy of.venv
After copying.venv, the full-directory copy will merge it again. Extract only code assets instead to preserve permissions and reduce layer size.backend/docker/Dockerfile.test (2)
7-7: Pin Poetry version & cache pip
Lock Poetry to a specific version and shadow pip’s cache into a BuildKit mount to speed up repeated builds.- python -m pip install poetry +RUN --mount=type=cache,target=/root/.cache/pip \ + python -m pip install "poetry==1.6.1"
42-42: Selective copy to avoid .venv overwrite
Rather than copying the entire/home/owasp, explicitly list code & config files to ensure.venvstays intact with correct permissions.docs/docker/Dockerfile.local (2)
9-9: Pin Poetry version & cache pip
For deterministic docs builds, pin Poetry and mount pip’s cache:- python -m pip install poetry +RUN --mount=type=cache,target=/root/.cache/pip \ + python -m pip install "poetry==1.6.1"
40-40: Avoid double.venvcopy in docs image
Copying the full/home/owaspafter.venvmerges the venv twice. Use a selective copy of docs sources to keep layers minimal and permissions correct.frontend/docker/Dockerfile.local (1)
13-15: Consider adding explicit cache IDs for stability
Your cache mounts look correct for the local build, but adding anidensures the same cache is reused across layers and builds.Example enhancement:
-RUN --mount=type=cache,target=/root/.pnpm-store \ - --mount=type=cache,target=/home/owasp/node_modules \ - pnpm install --frozen-lockfile --ignore-scripts +RUN --mount=type=cache,id=pnpm-store,target=/root/.pnpm-store \ + --mount=type=cache,id=node-modules,target=/home/owasp/node_modules \ + pnpm install --frozen-lockfile --ignore-scripts
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/docker/Dockerfile(3 hunks)backend/docker/Dockerfile.local(4 hunks)backend/docker/Dockerfile.test(3 hunks)docs/docker/Dockerfile.local(3 hunks)frontend/docker/Dockerfile(1 hunks)frontend/docker/Dockerfile.e2e.test(1 hunks)frontend/docker/Dockerfile.local(1 hunks)frontend/docker/Dockerfile.unit.test(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
|
Oops, something I missed in the e2e and unit files. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cspell/Dockerfile(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
| RUN --mount=type=cache,id=pnpm,target=/pnpm/store \ | ||
| pnpm install --frozen-lockfile --ignore-scripts |
There was a problem hiding this comment.
Ensure cache mount aligns with pnpm’s actual store directory
The build cache is mounted at /pnpm/store, but pnpm’s default store path is typically under the user’s home (e.g., ~/.pnpm-store). Without explicitly configuring pnpm to use /pnpm/store, the cache mount will not be utilized.
Update the install command to direct pnpm’s store into your cache mount:
RUN --mount=type=cache,id=pnpm,target=/pnpm/store \
- pnpm install --frozen-lockfile --ignore-scripts
+ pnpm install --frozen-lockfile --ignore-scripts --store-dir=/pnpm/storeAlternatively, set it globally before installation:
RUN pnpm config set store-dir=/pnpm/storeThis ensures the cache mount is effective and speeds up subsequent builds.
🤖 Prompt for AI Agents (early access)
In cspell/Dockerfile around lines 9 to 10, the pnpm cache mount is set to /pnpm/store but pnpm by default uses a different store directory, so the cache is not utilized. Fix this by configuring pnpm to use /pnpm/store as its store directory either by adding a command before installation to set the store-dir config globally or by passing the store-dir option directly in the install command. This ensures the cache mount is effective and speeds up builds.
| COPY package.json pnpm-lock.yaml ./ | ||
|
|
||
| RUN npm install --ignore-scripts -g pnpm && \ | ||
| RUN npm install --ignore-scripts -g pnpm |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin pnpm version for reproducible builds
Installing the latest pnpm without a version specifier can lead to unpredictable build failures when a new pnpm release is published.
Consider pinning pnpm to a known stable version, for example:
RUN npm install --ignore-scripts -g pnpm
+RUN npm install --ignore-scripts -g pnpm@8.8.0You can bump the @8.8.0 to whichever version aligns with your team’s compatibility requirements.
📝 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.
| RUN npm install --ignore-scripts -g pnpm | |
| RUN npm install --ignore-scripts -g pnpm | |
| RUN npm install --ignore-scripts -g pnpm@8.8.0 |
🤖 Prompt for AI Agents (early access)
In cspell/Dockerfile at line 7, the RUN command installs pnpm without specifying a version, which can cause unpredictable build failures. Modify the command to pin pnpm to a specific stable version by appending the version number after the package name, for example, use "pnpm@8.8.0" instead of just "pnpm" to ensure reproducible builds.
|
@coderabbitai Do you see any issues? Is the cache dir for poetry specified correctly? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3504 🏁 Script executed: Length of output: 6186 @ahmedxgouda Looking at the Dockerfiles, I spotted a potential issue with the Poetry cache directory. While the cache mounts have been successfully implemented, there's a user/permissions mismatch that might cause problems:
According to Poetry's documentation, the default cache directory on Linux is The current implementation might work because Docker's BuildKit runs the RUN command with root privileges during build time (even when USER is set to For better alignment with best practices, consider one of these approaches: # Option 1: Set POETRY_CACHE_DIR to a location writable by owasp user
ENV POETRY_VIRTUALENVS_IN_PROJECT=true \
+ POETRY_CACHE_DIR=/tmp/poetry-cache \
PYTHONUNBUFFERED=1
# Option 2: Use uid/gid in the mount specification
-RUN --mount=type=cache,target=/root/.cache/pypoetry \
+RUN --mount=type=cache,target=/home/owasp/.cache/pypoetry,uid=1000,gid=1000 \
poetry install --no-root --without dev --without testThe implementation in frontend e2e and unit test files looks correct with the pnpm cache mounts properly configured. |
|
@arkid15r Could you review it please? |
|
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
Alright, let's try this 👍
* Add cache mounts to docker files * Fix docker e2e and unit test syntax * Apply coderabbit suggestion * Fix spelling in backend * Refactor Dockerfiles to use consistent cache mount for pnpm * Fix typo in cache mount path in docs Dockerfile.local * Fix typo in cache mount path in backend Dockerfile * Add cache mount to cspell * Fix poetry cache * Add PNPM Home environement variable * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
2 participants
Resolves #1449
The last build I made was 10x faster than the normal (The frontend container was built in milliseconds regarding the last update to the packages). The packages are persistent even between different builds (i.e. test and local).
Screencast from 2025-05-02 07-58-14.webm