Refactor docker files / test images #3112
Conversation
github-actions
bot
added
backend
frontend
docker
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughConsolidates Docker assets under a unified /docker directory, repoints CI and Dependabot to the new paths, updates Makefiles and docker-compose to use root build contexts, adds a cspell Dockerfile, and expands test/run steps to use explicit env-files and read-only volume mounts. (34 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-12-26T06:08:58.549ZApplied to files:
📚 Learning: 2025-12-21T19:03:59.068ZApplied to files:
⏰ 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). (5)
🔇 Additional comments (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. 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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/dependabot.yml.github/workflows/run-ci-cd.yaml.github/workflows/update-nest-test-images.yamlMakefilebackend/Makefilebackend/entrypoint.shdocker-compose/local/compose.yamldocker/backend/Dockerfiledocker/backend/Dockerfile.localdocker/backend/Dockerfile.testdocker/cspell/Dockerfiledocker/docs/Dockerfile.localdocker/frontend/Dockerfiledocker/frontend/Dockerfile.e2e.testdocker/frontend/Dockerfile.localdocker/frontend/Dockerfile.unit.testfrontend/Makefile
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-26T12:50:50.512Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2429
File: backend/Makefile:30-32
Timestamp: 2025-10-26T12:50:50.512Z
Learning: The `exec-backend-e2e-command` and `exec-db-e2e-command` Makefile targets in the backend/Makefile are intended for local development and debugging only, not for CI/CD execution, so the `-it` flags are appropriate.
Applied to files:
frontend/Makefilebackend/Makefile
📚 Learning: 2025-12-26T06:08:58.549Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 3041
File: .github/workflows/run-ci-cd.yaml:233-243
Timestamp: 2025-12-26T06:08:58.549Z
Learning: For the OWASP/Nest repository, Redis image versions should remain consistent across all environments (production, staging, local, E2E, and CI/CD E2E tests). When upgrading Redis, update all docker-compose files and CI/CD workflow configurations together to maintain environment parity.
Applied to files:
.github/workflows/run-ci-cd.yaml.github/workflows/update-nest-test-images.yamlbackend/Makefile
📚 Learning: 2025-12-21T19:03:59.068Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1139
File: .github/workflows/setup-backend-environment/action.yaml:16-27
Timestamp: 2025-12-21T19:03:59.068Z
Learning: Composite actions (runs: using: composite) execute as steps within the calling job's context and can access the job context, including job.services.* properties (e.g., job.services.<service_id>.id, job.services.<service_id>.ports). Service containers must be defined at the job level, but a composite action's steps can reference them via the job context.
Applied to files:
.github/workflows/run-ci-cd.yaml.github/workflows/update-nest-test-images.yaml
📚 Learning: 2025-12-26T06:08:58.549Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 3041
File: .github/workflows/run-ci-cd.yaml:233-243
Timestamp: 2025-12-26T06:08:58.549Z
Learning: Ensure Redis image versions stay in sync across all environments by updating every relevant YAML file together (docker-compose files and CI/CD workflow configurations). When upgrading Redis, bump the image version in all docker-compose files and in all CI workflow YAMLs in one coordinated change to maintain parity across production, staging, local, E2E, and CI tests.
Applied to files:
docker-compose/local/compose.yaml
⏰ 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). (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (21)
docker/cspell/Dockerfile (4)
3-12: LGTM! Good security and performance practices.The environment setup correctly:
- Uses cache mounts for faster builds
- Configures retry logic for network resilience
- Employs
--ignore-scriptsto prevent execution of potentially malicious scripts during installation
14-19: LGTM! Proper layer caching and dependency management.The implementation follows Docker best practices by:
- Copying dependency manifests before installation (optimizes layer caching)
- Using
--frozen-lockfilefor reproducible builds- Leveraging cache mounts for the pnpm store
- Separating installation context (
/opt/node) from runtime context (/nest)
21-23: LGTM! Secure runtime configuration.The entrypoint correctly references the installed cspell binary, and using the non-root
nodeuser follows security best practices for containerized applications.
1-1: No action required—node:24-alpine is a valid LTS release.Node.js v24 is an officially supported LTS release (released October 2024) and follows the standard even-numbered LTS versioning scheme. The
node:24-alpineimage is a standard, stable offering on Docker Hub and is appropriate for production use.Likely an incorrect or invalid review comment.
Makefile (1)
53-53: Verify that removing COMPOSE_BAKE doesn't affect functionality.The
COMPOSE_BAKE=trueflag was removed, leaving onlyDOCKER_BUILDKIT=1. Confirm this change doesn't break any docker compose features or build optimizations that depended on the bake functionality.docker/backend/Dockerfile.test (1)
27-27: COPY paths correctly reflect the repository root build context.The updated paths with
backend/prefixes are consistent with the new build context structure where the context is at the repository root.Also applies to: 31-31
docker/frontend/Dockerfile.e2e.test (1)
16-16: LGTM! Improved security with explicit permissions.The updated COPY instruction correctly reflects the repository root build context and adds explicit read-only permissions (444) and root ownership, which is a security best practice for dependency manifests.
backend/Makefile (1)
120-133: Enhanced test workflow with improved volume mounting strategy.The updated test target includes several improvements:
- Dockerfile path aligns with the new Docker organization
- Explicit
DJANGO_SETTINGS_MODULEsetting improves clarity- Multiple read-only volume mounts enable testing against live code without image rebuilds
- The
--rmflag ensures proper cleanupNote that this changes the test strategy to mount host directories rather than copying them into the image. While this improves development workflow, verify that the test environment still matches production image behavior.
docker/backend/Dockerfile.local (1)
29-29: COPY paths correctly updated for the new build context.The change aligns with the repository-wide Docker reorganization, using
backend/prefixed paths and explicit permissions consistent with other backend Dockerfiles.docker/frontend/Dockerfile.local (1)
28-28: LGTM! Consistent with the Docker reorganization.The updated COPY instruction correctly uses
frontend/prefixed paths and adds explicit read-only permissions and root ownership, matching the security improvements in other frontend Dockerfiles..github/dependabot.yml (1)
4-4: Directory paths confirmed to exist at new locations.All four directories referenced in the dependabot.yml configuration have been verified to exist:
/docker/backend/docker/cspell/docker/docs/docker/frontendThe paths correctly reflect the consolidated
/dockerdirectory structure, and the dependabot configuration is properly aligned with the reorganization.docker/backend/Dockerfile (1)
39-39: No action required – the entrypoint.sh file exists at the expected location.The file exists at
backend/entrypoint.sh, which is the correct location for theCOPY entrypoint.sh entrypoint.shinstruction when the build context is the repository root. This is consistent with other COPY instructions in the builder stage that reference files in thebackend/directory..github/workflows/update-nest-test-images.yaml (2)
40-41: LGTM! Build contexts and Dockerfile paths correctly updated.All three test image builds now consistently use the repository root as context (
.) with Dockerfile paths pointing to the new centralizeddocker/directory structure. This aligns perfectly with the PR's refactoring goals.Also applies to: 55-56, 70-71
61-61: Minor naming consistency improvement.Renaming from "Update frontend end-to-end test image" to "Update frontend e2e test image" improves consistency with the step name on Line 244 of
.github/workflows/run-ci-cd.yaml.docker/frontend/Dockerfile.unit.test (1)
16-21: LGTM! Dockerfile correctly adapted for new build context.The COPY paths now correctly reference
frontend/package.jsonandfrontend/pnpm-lock.yamlsince the build context is the repository root. The minimal image (dependencies only, no source code) aligns with the runtime volume mount strategy seen infrontend/Makefile, where test files and source code are mounted at runtime. This approach improves development workflow by avoiding rebuilds when source code changes.docker-compose/local/compose.yaml (1)
11-12: LGTM! Docker Compose service definitions correctly updated.All service build configurations now consistently use the repository root as context with Dockerfile paths pointing to the new
docker/directory structure:
- Backend:
docker/backend/Dockerfile.local- Docs:
docker/docs/Dockerfile.local- Frontend:
docker/frontend/Dockerfile.localThis centralization improves maintainability and aligns with the PR's refactoring objectives.
Also applies to: 79-79, 95-96
frontend/Makefile (2)
60-63: LGTM! Build commands correctly updated for new Docker structure.Build commands now use repository root as context with Dockerfile paths pointing to
docker/frontend/. The removal of--build-arg NEXT_PUBLIC_ENVIRONMENT=localfrom build time is appropriate since it's now set at runtime (Lines 67, 87), improving image reusability.Also applies to: 80-83
64-77: Excellent runtime volume mount strategy.The extensive use of read-only volume mounts for source code, tests, and configuration files is a significant improvement:
- Eliminates need to rebuild images when source code changes
- Speeds up development workflow
- All mounts are read-only (
:ro), following security best practices- Aligns with the lightweight Docker images approach
This pattern works well with the minimal test images that only contain dependencies.
Also applies to: 84-95
.github/workflows/run-ci-cd.yaml (3)
167-168: LGTM! Test image builds correctly updated.All test image builds now consistently use repository root as context (
.) with Dockerfile paths pointing to the centralizeddocker/directory structure. This aligns with the refactoring goals.Also applies to: 209-210, 253-254
175-184: Excellent improvement to test execution strategy.The extensive use of bind mounts for source code, tests, and configuration files significantly improves the CI/CD workflow:
- Changes to source code don't require image rebuilds
- Faster iteration during development and debugging
- All mounts are read-only, following security best practices
- Aligns with the lightweight test image approach
Also applies to: 217-228, 261-274
333-334: LGTM! Staging backend build configuration correctly updated.Both context and Dockerfile path have been updated to use the repository root and centralized
docker/directory structure.
| context: frontend | ||
| file: frontend/docker/Dockerfile | ||
| file: docker/frontend/Dockerfile |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Step 1: Explore repository structure for Dockerfiles
echo "=== Finding all Dockerfiles ==="
fd -t f "Dockerfile" --max-depth 5
echo -e "\n=== Checking if docker/ directory exists at repo root ==="
ls -la docker/ 2>/dev/null || echo "docker/ directory not found at repo root"
echo -e "\n=== Repository root structure ==="
ls -la | head -20Repository: OWASP/Nest
Length of output: 2001
🏁 Script executed:
#!/bin/bash
# Step 2: Check the workflow file structure and extract the specific lines mentioned
echo "=== Checking workflow file size ==="
wc -l .github/workflows/run-ci-cd.yaml
echo -e "\n=== Lines 375-390 (staging frontend build) ==="
sed -n '375,390p' .github/workflows/run-ci-cd.yaml
echo -e "\n=== Lines 650-665 (production backend build) ==="
sed -n '650,665p' .github/workflows/run-ci-cd.yaml
echo -e "\n=== Lines 695-710 (production frontend build) ==="
sed -n '695,710p' .github/workflows/run-ci-cd.yamlRepository: OWASP/Nest
Length of output: 1914
🏁 Script executed:
#!/bin/bash
# Step 3: Use yq to extract the exact context and file values for the three builds
if command -v yq &> /dev/null; then
echo "=== Extracting build configurations using yq ==="
echo "Staging frontend:"
yq eval '.jobs.build-staging-images.steps[] | select(.name == "Build frontend image") | {context: .with.context, file: .with.file}' .github/workflows/run-ci-cd.yaml
echo -e "\nProduction backend:"
yq eval '.jobs.build-production-images.steps[] | select(.name == "Build backend image") | {context: .with.context, file: .with.file}' .github/workflows/run-ci-cd.yaml
echo -e "\nProduction frontend:"
yq eval '.jobs.build-production-images.steps[] | select(.name == "Build frontend image") | {context: .with.context, file: .with.file}' .github/workflows/run-ci-cd.yaml
else
echo "yq not available, will extract manually"
fiRepository: OWASP/Nest
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Step 1: Search for all build-push-action steps to identify patterns
echo "=== All docker/build-push-action steps in workflow ==="
grep -n "name: Build" .github/workflows/run-ci-cd.yaml
echo ""
echo "=== Checking all context values in workflow ==="
grep -n "context:" .github/workflows/run-ci-cd.yaml
echo ""
echo "=== Checking all file values in workflow ==="
grep -n "file:" .github/workflows/run-ci-cd.yamlRepository: OWASP/Nest
Length of output: 1115
🏁 Script executed:
#!/bin/bash
# Step 2: Check git history to understand when/if Dockerfiles were moved
echo "=== Recent commits mentioning docker or Dockerfile ==="
git log --oneline --all -20 -- docker/ | head -20
echo -e "\n=== Check if Dockerfiles exist at old locations (root level) ==="
ls -la | grep -i docker || echo "No Dockerfile at repo root"
echo -e "\n=== Check for backend/Dockerfile or frontend/Dockerfile at root ==="
if [ -f "backend/Dockerfile" ]; then
echo "Found backend/Dockerfile at root"
else
echo "backend/Dockerfile does NOT exist at root"
fi
if [ -f "frontend/Dockerfile" ]; then
echo "Found frontend/Dockerfile at root"
else
echo "frontend/Dockerfile does NOT exist at root"
fiRepository: OWASP/Nest
Length of output: 554
🏁 Script executed:
#!/bin/bash
# Step 3: Verify if docker-compose or other files reference Dockerfiles to understand the pattern
echo "=== Checking docker-compose files ==="
find . -maxdepth 3 -name "docker-compose*" -type f | head -10
if [ -f "docker-compose.yml" ] || [ -f "docker-compose.yaml" ]; then
echo ""
echo "=== docker-compose context patterns ==="
grep -A2 "build:" docker-compose.yml docker-compose.yaml 2>/dev/null | head -40
fiRepository: OWASP/Nest
Length of output: 91
Critical: Inconsistent build context and Dockerfile paths will break builds.
Three build configurations have mismatched context and Dockerfile paths:
-
Staging frontend (Lines 381-382):
- Context:
frontend - File:
docker/frontend/Dockerfile - Result: Docker will look for
frontend/docker/frontend/Dockerfile❌
- Context:
-
Production backend (Lines 658-659):
- Context:
backend - File:
docker/backend/Dockerfile - Result: Docker will look for
backend/docker/backend/Dockerfile❌
- Context:
-
Production frontend (Lines 704-705):
- Context:
frontend - File:
docker/frontend/Dockerfile - Result: Docker will look for
frontend/docker/frontend/Dockerfile❌
- Context:
The Dockerfile paths are relative to the specified context directory. Since all Dockerfiles are located in the centralized docker/ directory at the repository root (not at backend/Dockerfile or frontend/Dockerfile), the context should be . (repository root) for all three builds. Note that other builds in this workflow (test images and staging backend) already use the correct pattern with context: ..
🔎 Proposed fix
For staging frontend build:
- name: Build frontend image
uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83
with:
cache-from: |
type=gha
type=registry,ref=owasp/nest:frontend-staging-cache
cache-to: |
type=registry,ref=owasp/nest:frontend-staging-cache
- context: frontend
+ context: .
file: docker/frontend/DockerfileFor production backend build:
- name: Build backend image
uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83
with:
build-args: |
OWASP_GID=1002
OWASP_UID=1002
cache-from: |
type=gha
type=registry,ref=owasp/nest:backend-staging-cache
- context: backend
+ context: .
file: docker/backend/DockerfileFor production frontend build:
- name: Build frontend image
uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83
with:
cache-from: |
type=gha
type=registry,ref=owasp/nest:frontend-staging-cache
- context: frontend
+ context: .
file: docker/frontend/Dockerfile📝 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.
| context: frontend | |
| file: frontend/docker/Dockerfile | |
| file: docker/frontend/Dockerfile | |
| context: . | |
| file: docker/frontend/Dockerfile |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docker/frontend/Dockerfile.e2e.test (1)
22-31: Consider reducing duplication between stages.The environment variables (Lines 22-28) and pnpm installation (Lines 30-31) are duplicated from the builder stage. While not critical, you could refactor using a base image or ARGs to improve maintainability.
🔎 Example refactor using a common base
+FROM node:24-alpine AS base + +ENV FORCE_COLOR=1 \ + NPM_CACHE="/app/.npm" \ + PNPM_HOME="/pnpm" + +ENV NPM_CONFIG_RETRY=5 \ + NPM_CONFIG_TIMEOUT=30000 \ + PATH="$PNPM_HOME:$PATH" + +RUN --mount=type=cache,target=${NPM_CACHE} \ + npm install --ignore-scripts -g pnpm --cache ${NPM_CACHE} + -FROM node:24-alpine AS builder +FROM base AS builder - -ENV FORCE_COLOR=1 \ - NPM_CACHE="/app/.npm" \ - PNPM_HOME="/pnpm" - -ENV NPM_CONFIG_RETRY=5 \ - NPM_CONFIG_TIMEOUT=30000 \ - PATH="$PNPM_HOME:$PATH" - -RUN --mount=type=cache,target=${NPM_CACHE} \ - npm install --ignore-scripts -g pnpm --cache ${NPM_CACHE} WORKDIR /app COPY --chmod=444 --chown=root:root frontend/package.json frontend/pnpm-lock.yaml ./ RUN --mount=type=cache,id=pnpm,target=/pnpm/store \ pnpm install --frozen-lockfile --ignore-scripts FROM mcr.microsoft.com/playwright:v1.57.0-jammy -ENV FORCE_COLOR=1 \ - NPM_CACHE="/app/.npm" \ - PNPM_HOME="/pnpm" - -ENV NPM_CONFIG_RETRY=5 \ - NPM_CONFIG_TIMEOUT=30000 \ - PATH="$PNPM_HOME:$PATH" - -RUN --mount=type=cache,target=${NPM_CACHE} \ - npm install --ignore-scripts -g pnpm --cache ${NPM_CACHE} +COPY --from=base /usr/local/lib/node_modules/pnpm /usr/local/lib/node_modules/pnpm +COPY --from=base /usr/local/bin/pnpm /usr/local/bin/pnpm + +ENV PNPM_HOME="/pnpm" \ + PATH="/pnpm:$PATH" WORKDIR /app COPY --from=builder --chown=root:root /app/node_modules ./node_modules COPY --from=builder --chown=root:root /app/package.json ./package.jsonNote: This approach copies pnpm binaries from the base stage instead of reinstalling, but you may need to adjust paths if the Playwright base image has different system library requirements.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/run-ci-cd.yamldocker/frontend/Dockerfile.e2e.test
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-26T06:08:58.549Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 3041
File: .github/workflows/run-ci-cd.yaml:233-243
Timestamp: 2025-12-26T06:08:58.549Z
Learning: For the OWASP/Nest repository, Redis image versions should remain consistent across all environments (production, staging, local, E2E, and CI/CD E2E tests). When upgrading Redis, update all docker-compose files and CI/CD workflow configurations together to maintain environment parity.
Applied to files:
.github/workflows/run-ci-cd.yaml
📚 Learning: 2025-12-21T19:03:59.068Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1139
File: .github/workflows/setup-backend-environment/action.yaml:16-27
Timestamp: 2025-12-21T19:03:59.068Z
Learning: Composite actions (runs: using: composite) execute as steps within the calling job's context and can access the job context, including job.services.* properties (e.g., job.services.<service_id>.id, job.services.<service_id>.ports). Service containers must be defined at the job level, but a composite action's steps can reference them via the job context.
Applied to files:
.github/workflows/run-ci-cd.yaml
⏰ 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). (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (6)
docker/frontend/Dockerfile.e2e.test (2)
1-19: LGTM - Builder stage correctly optimized for caching.The builder stage properly leverages cache mounts and sets secure file ownership. The COPY instruction on Line 16 correctly assumes a root build context (
.), which aligns with the workflow changes that setcontext: ..
20-20: The Dockerfile is using Playwright v1.57.0, which is the latest stable release as of December 2025. No action required..github/workflows/run-ci-cd.yaml (4)
158-180: LGTM - Backend test build and execution correctly configured.The backend test image build correctly uses
context: .withdocker/backend/Dockerfile.test, and the test execution properly mounts all required directories as read-only volumes. The use of GHA caching with zstd compression optimizes CI performance.
196-220: LGTM - Frontend unit test configuration is correct.The unit test image build correctly uses
context: .withdocker/frontend/Dockerfile.unit.test, and the test execution mounts all necessary files and directories as read-only volumes. The configuration aligns well with the docker/ refactoring.
236-262: LGTM - Frontend e2e test configuration is comprehensive.The e2e test image build correctly uses
context: .withdocker/frontend/Dockerfile.e2e.test, and the test execution mounts all necessary configuration files and directories as read-only volumes. The configuration properly supports Playwright e2e testing.
310-326: LGTM - Staging backend build correctly configured.The staging backend build correctly uses
context: .withdocker/backend/Dockerfile, properly aligning with the centralized docker/ directory structure.
|
Proposed change
Refactor docker files.
Checklist
make check-testlocally and all tests passed