Establish an e2e backend instance locally and in CI/CD #2429
Conversation
github-actions
bot
added
backend
docker
|
Warning Rate limit exceeded@ahmedxgouda has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Summary by CodeRabbitRelease Notes
WalkthroughAdds end-to-end backend test infrastructure: new Makefile targets to run and manage e2e containers and DB data, a dedicated docker-compose e2e stack, a GitHub Actions job to run backend e2e tests, removal of the default CMD in the backend Dockerfile, and explicit entrypoints added to production and staging compose files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 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
🧹 Nitpick comments (7)
docker-compose/backend.e2e.yaml (4)
16-22: Set DJANGO_SETTINGS_MODULE default for e2e.Make the e2e stack independent of dev/prod .env.
environment: + DJANGO_SETTINGS_MODULE: ${DJANGO_SETTINGS_MODULE:-settings.e2e} DJANGO_DB_HOST: ${DJANGO_DB_HOST:-db} DJANGO_DB_NAME: ${DJANGO_DB_NAME:-nest_db_e2e} DJANGO_DB_PASSWORD: ${DJANGO_DB_PASSWORD:-nest_user_e2e_password} DJANGO_DB_PORT: ${DJANGO_DB_PORT:-5432} DJANGO_DB_USER: ${DJANGO_DB_USER:-nest_user_e2e}
22-25: Add a backend healthcheck to enable compose --wait and reliable test gating.Without a healthcheck, CI can race the server startup.
ports: - 8000:8000 + healthcheck: + test: ["CMD-SHELL", "python -c 'import socket,time,sys; \ +for _ in range(30): \ + try: socket.create_connection((\"localhost\",8000),2).close(); sys.exit(0) \ + except OSError: time.sleep(1) \ +sys.exit(1)'"] + interval: 5s + timeout: 3s + retries: 10 + start_period: 10s
41-43: E2E DB volume persists across runs; add a clean target to avoid stale state.Persistent volume improves speed but harms repeatability. Add a Makefile clean target that runs
docker compose -p nest-e2e -f docker-compose/backend.e2e.yaml down -v.
17-21: Default DB creds committed (ok for test) — but prefer .env.e2e to avoid spreading defaults.Low risk, but moving these to an
.env.e2ekeeps compose cleaner and avoids accidental reuse.Also applies to: 29-33
backend/docker/Dockerfile.e2e (2)
23-24: Pin Poetry to a version for reproducible builds.Unpinned Poetry can break builds unexpectedly.
-RUN --mount=type=cache,target=${PIP_CACHE_DIR} \ - python -m pip install poetry --cache-dir ${PIP_CACHE_DIR} +RUN --mount=type=cache,target=${PIP_CACHE_DIR} \ + python -m pip install 'poetry==1.8.3' --cache-dir ${PIP_CACHE_DIR}
31-32: Install only runtime deps for a slimmer image.If dev deps exist, prefer:
poetry install --no-root --only main.- poetry install --no-root + poetry install --no-root --only mainbackend/Makefile (1)
14-20: Add a clean target for the e2e stack (containers + volume).Ensures repeatable runs and easy teardown.
clean-backend-docker: @docker container rm -f nest-backend >/dev/null 2>&1 || true @docker container rm -f nest-cache >/dev/null 2>&1 || true @docker container rm -f nest-db >/dev/null 2>&1 || true @docker image rm -f nest-local-backend >/dev/null 2>&1 || true @docker volume rm -f nest-local_backend-venv >/dev/null 2>&1 || true + +clean-backend-e2e-docker: + @docker compose --project-name nest-e2e -f docker-compose/backend.e2e.yaml down -v
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/data/nest-e2e-data.sql.gzis excluded by!**/*.gz
📒 Files selected for processing (3)
backend/Makefile(4 hunks)backend/docker/Dockerfile.e2e(1 hunks)docker-compose/backend.e2e.yaml(1 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). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (2)
backend/Makefile (1)
84-87: load-data-e2e is fine as long as backend is healthy.No changes needed; verify it follows
run-backend-e2ein CI to avoid races.backend/docker/Dockerfile.e2e (1)
1-1: Ensure registry mirror sync and BuildKit enabled
python:3.13.7-alpineexists on Docker Hub—verify your mirror has this tag- CI environment didn’t expose BuildKit variables—enable BuildKit (
DOCKER_BUILDKIT=1) forRUN --mountsupport
backend/docker/Dockerfile.e2e
Outdated
| COPY --from=builder --chmod=555 --chown=root:root /home/owasp /home/owasp | ||
|
|
||
| USER owasp |
There was a problem hiding this comment.
Copying app as root:root with chmod=555 makes $HOME read‑only for USER owasp.
Runtime user cannot write caches/temp; can cause odd failures. Chown to owasp and avoid over‑restrictive 555.
-COPY --from=builder --chmod=555 --chown=root:root /home/owasp /home/owasp
+COPY --from=builder --chown=owasp:owasp /home/owasp /home/owasp
+# Optional hardening without breaking writes:
+# RUN chmod -R u+rwX,g+rX,o+rX /home/owasp📝 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.
| COPY --from=builder --chmod=555 --chown=root:root /home/owasp /home/owasp | |
| USER owasp | |
| COPY --from=builder --chown=owasp:owasp /home/owasp /home/owasp | |
| # Optional hardening without breaking writes: | |
| # RUN chmod -R u+rwX,g+rX,o+rX /home/owasp | |
| USER owasp |
🤖 Prompt for AI Agents
In backend/docker/Dockerfile.e2e around lines 59-61, the COPY uses root:root and
chmod=555 which makes /home/owasp read-only for USER owasp; change the COPY so
files are owned by the owasp user and not world-read-only: use
--chown=owasp:owasp and remove or relax the restrictive chmod (e.g.,
--chmod=0755 or omit it) and ensure any runtime-writable dirs (home, .cache,
tmp) are writable by the owner (set owner write bit or explicitly chown/chmod
those subdirs after copy) so USER owasp can write caches/temp.
| exec-backend-e2e-command: | ||
| @docker exec -it nest-backend-e2e $(CMD) | ||
|
|
There was a problem hiding this comment.
Use non‑TTY exec for CI‑safety.
-t fails on non‑TTY runners; switch to -i.
exec-backend-e2e-command:
- @docker exec -it nest-backend-e2e $(CMD)
+ @docker exec -i nest-backend-e2e $(CMD)
exec-db-e2e-command:
- @docker exec -it nest-db-e2e $(CMD)
+ @docker exec -i nest-db-e2e $(CMD)If you still need interactive shells locally, add separate *-it variants.
Also applies to: 37-39
🤖 Prompt for AI Agents
In backend/Makefile around lines 30 to 32 (and similarly lines 37 to 39), the
docker exec targets use -it which breaks on CI non‑TTY runners; remove the -t
flag so they use only -i (e.g. docker exec -i) for CI‑safe execution, and if
interactive local shells are still needed add separate targets with -it suffix
that keep -it for local use.
| dump-db-data-e2e: | ||
| @echo "Dumping Nest e2e data" | ||
| @CMD="pg_dumpall -U nest_user_e2e --clean | gzip -9 > backend/data/nest-e2e-data.sql.gz" $(MAKE) exec-db-e2e-command | ||
|
|
There was a problem hiding this comment.
db dump is broken (shell metacharacters not interpreted; redirection inside container).
Run the exec pipeline from the host and avoid TTY.
dump-db-data-e2e:
@echo "Dumping Nest e2e data"
- @CMD="pg_dumpall -U nest_user_e2e --clean | gzip -9 > backend/data/nest-e2e-data.sql.gz" $(MAKE) exec-db-e2e-command
+ @mkdir -p backend/data
+ @docker exec -i nest-db-e2e pg_dumpall -U nest_user_e2e --clean | gzip -9 > backend/data/nest-e2e-data.sql.gzNotes:
- No
-tto avoid TTY artifacts. - No
-hso it uses local socket (no password prompt).
📝 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.
| dump-db-data-e2e: | |
| @echo "Dumping Nest e2e data" | |
| @CMD="pg_dumpall -U nest_user_e2e --clean | gzip -9 > backend/data/nest-e2e-data.sql.gz" $(MAKE) exec-db-e2e-command | |
| dump-db-data-e2e: | |
| @echo "Dumping Nest e2e data" | |
| @mkdir -p backend/data | |
| @docker exec -i nest-db-e2e pg_dumpall -U nest_user_e2e --clean | gzip -9 > backend/data/nest-e2e-data.sql.gz |
🤖 Prompt for AI Agents
In backend/Makefile around lines 63-66, the current target runs the redirection
inside the container (and uses the exec wrapper with a TTY/host options), so
change it to run the pg_dump pipeline from the host and call docker exec without
-t or -h; specifically, invoke docker exec -i <db-container> pg_dumpall -U
nest_user_e2e --clean on the host and pipe that output into gzip -9 >
backend/data/nest-e2e-data.sql.gz (no -t, no -h), replacing the
CMD/exec-db-e2e-command usage so the redirection happens on the host side.
| run-backend-e2e: | ||
| @DOCKER_BUILDKIT=1 \ | ||
| docker compose --project-name nest-e2e -f docker-compose/backend.e2e.yaml up --build --remove-orphans | ||
|
|
There was a problem hiding this comment.
up blocks; run detached and wait on health for CI.
Prevents pipelines from proceeding.
run-backend-e2e:
@DOCKER_BUILDKIT=1 \
- docker compose --project-name nest-e2e -f docker-compose/backend.e2e.yaml up --build --remove-orphans
+ docker compose --project-name nest-e2e -f docker-compose/backend.e2e.yaml up -d --build --wait --remove-orphansRequires a backend healthcheck (see compose review).
📝 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-backend-e2e: | |
| @DOCKER_BUILDKIT=1 \ | |
| docker compose --project-name nest-e2e -f docker-compose/backend.e2e.yaml up --build --remove-orphans | |
| run-backend-e2e: | |
| @DOCKER_BUILDKIT=1 \ | |
| docker compose --project-name nest-e2e -f docker-compose/backend.e2e.yaml up -d --build --wait --remove-orphans |
🤖 Prompt for AI Agents
In backend/Makefile around lines 115-118, the make target currently brings up
compose in the foreground which blocks CI; change it to start the services
detached and then wait for health: run docker compose --project-name nest-e2e -f
docker-compose/backend.e2e.yaml up --build --detach (or -d) followed by docker
compose --project-name nest-e2e -f docker-compose/backend.e2e.yaml wait
--timeout 300s (or suitable timeout) so the job only proceeds once services are
healthy; also ensure the backend compose file defines a proper healthcheck for
the service(s) so the wait succeeds.
| depends_on: | ||
| db: | ||
| condition: service_healthy | ||
| env_file: ../backend/.env |
There was a problem hiding this comment.
env_file path likely breaks CI; use an example/e2e file or drop it.
CI runners may not have ../backend/.env; compose will fail. Either point to a committed example (or e2e‑specific) env file or rely on explicit environment defaults.
Apply one of:
- env_file: ../backend/.env
+ # Use a committed example env for e2e
+ env_file: ../backend/.env.exampleOr remove and rely on explicit env:
- env_file: ../backend/.env📝 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.
| env_file: ../backend/.env | |
| # Use a committed example env for e2e | |
| env_file: ../backend/.env.example |
github-actions
bot
added
ci
and removed
docker
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/run-ci-cd.yaml(1 hunks)backend/docker/Dockerfile(0 hunks)docker-compose/backend.e2e.yaml(1 hunks)docker-compose/production.yaml(1 hunks)docker-compose/staging.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- backend/docker/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (1)
- docker-compose/backend.e2e.yaml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/run-ci-cd.yaml
188-188: unexpected key "environment" for "services" section. expected one of "credentials", "env", "image", "options", "ports", "volumes"
(syntax-check)
🔇 Additional comments (3)
docker-compose/production.yaml (1)
4-4: LGTM — verify entrypoint script exists in the backend image.The explicit entrypoint aligns with the backend Dockerfile changes and provides consistent startup across environments.
Confirm that
/home/owasp/entrypoint.shis present in the backend Docker image (backend/docker/Dockerfile context).docker-compose/staging.yaml (1)
4-4: LGTM — mirrors production entrypoint.Consistent with production.yaml and backend Dockerfile updates.
.github/workflows/run-ci-cd.yaml (1)
233-238: The e2e test data file is properly committed and available.The file
backend/data/nest-e2e-data.sql.gzis tracked in git (commit 4002a19) and present on the PR branch. The workflow will successfully access it during execution. No action needed.
|
There was a problem hiding this comment.
The backend image should not contain non-production code
| @CMD="sed -E -i 's/\"email\": *\"([^\"]|\\\")*\"/\"email\": \"\"/g' data/nest.json" $(MAKE) exec-backend-command | ||
| @CMD="gzip -f data/nest.json" $(MAKE) exec-backend-command | ||
|
|
||
| dump-db-data-e2e: |
There was a problem hiding this comment.
Use consistent naming
| dump-db-data-e2e: | |
| dump-data-e2e: |
|
|
||
| dump-db-data-e2e: | ||
| @echo "Dumping Nest e2e data" | ||
| @CMD="pg_dumpall -U nest_user_e2e --clean | gzip -9 > backend/data/nest-e2e-data.sql.gz" $(MAKE) exec-db-e2e-command |
There was a problem hiding this comment.
it's already under the data/
| @CMD="pg_dumpall -U nest_user_e2e --clean | gzip -9 > backend/data/nest-e2e-data.sql.gz" $(MAKE) exec-db-e2e-command | |
| @CMD="pg_dumpall -U nest_user_e2e --clean | gzip -9 > backend/data/nest-e2e.sql.gz" $(MAKE) exec-db-e2e-command |
2 participants
Proposed change
Resolves #2422
Add the PR description here.
Checklist
make check-testlocally; all checks and tests passed.