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:
Walkthrough웹훅 기반 배포 서비스와 배포/상태 스크립트 추가, Docker Compose 분할 재구성(애플리케이션·웹훅·Datadog 포함), Spring Compose 삭제, Traefik letsencrypt 경로 및 .gitignore 항목 추가, Datadog 헬스체크·호스트명 변수 변경. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Traefik
participant Webhook as WebhookService
participant Script as DeployScript
participant Docker as DockerHost
Client->>Traefik: POST/GET /hooks (X-Deploy-Token)
Traefik->>Webhook: Forward request
Webhook->>Webhook: validate SECRET_TOKEN
alt token valid
Webhook->>Script: execute deploy-docker.sh / check-deploy-status.sh
Script->>Docker: git fetch/reset, docker pull, docker-compose up -d
Docker-->>Script: container started
loop health check
Script->>Docker: inspect container health
Docker-->>Script: healthy / unhealthy
end
Script-->>Webhook: return result/status
Webhook-->>Client: HTTP response (status/output)
else token invalid
Webhook-->>Client: 401 JSON error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Summary of ChangesHello @leegwichan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 프로젝트의 인프라 구조를 Docker 기반으로 전환하는 리팩토링의 일환으로, 기존의 Spring 애플리케이션 배포 방식을 Docker Compose를 활용한 서비스 기반 아키텍처로 변경합니다. 이는 애플리케이션, 모니터링(Datadog), 리버스 프록시(Traefik) 및 자동 배포 웹훅 서비스를 포함하며, 향후 CI/CD 파이프라인 구축을 위한 기반을 마련합니다. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 기존의 Blue-Green 무중단 배포 방식에서 Docker Compose를 활용한 중단 배포 방식으로 인프라 구조를 리팩토링하는 내용을 담고 있습니다. 전체적으로 Docker 기반 환경으로 전환하기 위한 설정들이 잘 구성되었습니다. 다만, 보안 강화를 위해 몇 가지 개선할 점이 보입니다. webhook 컨테이너의 root 사용자 실행 및 Docker 인증 정보 마운트 방식은 보안에 취약할 수 있어 주의가 필요합니다. 또한 Datadog 연동 시 버전 추적을 용이하게 하기 위해 DD_VERSION을 동적으로 관리하고, 배포 스크립트가 항상 최신 인프라 구성을 사용하도록 개선하는 것을 권장합니다. 자세한 내용은 각 파일에 남긴 코멘트를 참고해주세요.
| context: .. | ||
| dockerfile: ./webhook/webhook.Dockerfile | ||
| container_name: webhook | ||
| user: root |
There was a problem hiding this comment.
webhook 컨테이너를 root 사용자로 실행하는 것은 보안상 위험합니다. 컨테이너가 탈취될 경우 호스트 시스템에 대한 root 권한을 갖게 될 수 있습니다. Docker 소켓에 접근해야 하더라도, root가 아닌 별도의 사용자를 생성하고 docker 그룹에 추가하여 최소한의 권한만 부여하는 것이 안전합니다.
There was a problem hiding this comment.
후순위긴 하지만 이거 나중에 시도해봐도 좋을 듯 👍
| - "traefik.http.services.webhook.loadbalancer.server.port=9000" | ||
| volumes: | ||
| - /var/run/docker.sock:/var/run/docker.sock | ||
| - /home/ubuntu/.docker:/root/.docker:ro |
There was a problem hiding this comment.
호스트의 .docker 디렉토리를 컨테이너에 마운트하면 private registry의 인증 정보가 노출될 수 있어 보안상 위험합니다. read-only로 마운트하더라도 컨테이너 내부에서 인증 정보를 읽을 수 있습니다. docker login을 CI/CD 파이프라인에서 수행하거나, DOCKER_AUTH_CONFIG 환경 변수를 통해 인증 정보를 안전하게 전달하는 방법을 고려해 보세요.
| - DD_AGENT_HOST=datadog-agent | ||
| - DD_SERVICE=debate-timer | ||
| - DD_ENV=${ENV:-dev} | ||
| - DD_VERSION=1.0.0 |
There was a problem hiding this comment.
DD_VERSION이 1.0.0으로 하드코딩되어 있습니다. 이 경우 Datadog에서 배포 버전을 추적하기 어렵습니다. 배포 시점에 Git commit hash나 tag를 환경 변수로 전달하여 동적으로 버전을 설정하는 것이 좋습니다. 예를 들어, 이 값을 APP_VERSION 환경 변수로 전달받도록 변경할 수 있습니다.
- DD_VERSION=${APP_VERSION:-1.0.0}
Test Results135 files 135 suites 15s ⏱️ Results for commit aa75b10. ♻️ This comment has been updated with latest results. |
📝 Test Coverage Report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
scripts/deploy/check-deploy-status.sh (1)
10-19: 조기 종료 시 파일 디스크립터가 정리되지 않을 수 있습니다.Line 5에서
exit 0으로 스크립트가 종료되면 Line 19의exec 200>&-가 실행되지 않습니다.trap을 사용하여 종료 시 항상 파일 디스크립터를 정리하는 것이 좋습니다.♻️ trap을 사용한 정리 로직 추가
+cleanup() { + exec 200>&- 2>/dev/null +} +trap cleanup EXIT + if [ -z "$SECRET_TOKEN" ] || [ "$REQUEST_TOKEN" != "$SECRET_TOKEN" ]; then echo '{"status": "error", "message": "Unauthorized request. Invalid Token."}' exit 0 fi LOCK_FILE="/tmp/deploy-application.lock" exec 200>"$LOCK_FILE" if ! flock -n 200; then echo '{"status": "running", "message": "배포 스크립트가 현재 실행 중입니다."}' else flock -u 200 echo '{"status": "idle", "message": "현재 진행 중인 배포가 없습니다. 대기 중입니다."}' fi - -exec 200>&-🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/deploy/check-deploy-status.sh` around lines 10 - 19, Open the lock file descriptor (exec 200>"$LOCK_FILE") and then register a trap for EXIT and relevant signals (e.g., INT, TERM) that always releases and closes fd 200 (perform flock -u 200 if held and exec 200>&-); move or add the exec 200>&- cleanup into that trap and ensure the trap is set immediately after opening the descriptor so early exits still run the cleanup, leaving the rest of the flock -n 200 logic unchanged.scripts/deploy/deploy-docker.sh (4)
18-18: 변수 사용 시 따옴표로 감싸는 것이 안전합니다.
$PROJECT_DIR과$TARGET_SERVICE같은 변수들이 따옴표 없이 사용되고 있습니다. 경로나 값에 공백이 포함될 경우 예상치 못한 동작이 발생할 수 있습니다.♻️ 따옴표 추가 예시
-cd $PROJECT_DIR || { echo "디렉토리 이동 실패"; exit 1; } +cd "$PROJECT_DIR" || { echo "디렉토리 이동 실패"; exit 1; }스크립트 전체에서 변수 참조 시
"$VAR"형태로 일관되게 사용하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/deploy/deploy-docker.sh` at line 18, 현재 스크립트에서 변수 참조가 따옴표 없이 사용되어 공백 포함 경로 등에서 문제가 발생할 수 있습니다; scripts/deploy/deploy-docker.sh 내의 cd $PROJECT_DIR 뿐만 아니라 모든 변수 참조(예: $PROJECT_DIR, $TARGET_SERVICE, 기타 ENV/ARGS)를 일관되게 "$PROJECT_DIR", "$TARGET_SERVICE" 형태로 감싸고, 명령어 인자나 파일 경로로 전달되는 곳에서도 동일하게 따옴표를 적용하도록 수정하세요.
9-12: 스크립트 종료 시 락 파일 정리를 위한 trap 설정을 권장합니다.현재 파일 디스크립터 200이 스크립트 종료 시 자동으로 해제되지만, 명시적인
trap을 사용하면 비정상 종료 시에도 정리가 보장됩니다.♻️ trap을 사용한 정리 로직
echo "스크립트 실행 여부를 확인합니다." LOCK_FILE="/tmp/deploy-application.lock" + +cleanup() { + exec 200>&- 2>/dev/null +} +trap cleanup EXIT + exec 200>"$LOCK_FILE" flock -n 200 || { echo "⚠️ 이미 배포 스크립트가 실행 중입니다. 중복 실행을 차단합니다."; exit 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/deploy/deploy-docker.sh` around lines 9 - 12, Add a cleanup trap so the lock is always released and the lock file removed on exit: after creating LOCK_FILE and opening FD 200 (exec 200>"$LOCK_FILE") register a trap for EXIT that runs flock -u 200 to unlock, closes FD 200 (exec 200>&-) and removes "$LOCK_FILE" (rm -f "$LOCK_FILE"); keep existing non-blocking flock -n 200 check as-is and ensure the trap is installed immediately after opening the descriptor to cover abnormal exits.
50-54: 배포 실패 시 롤백 메커니즘 부재.헬스체크 실패 시 로그만 출력하고 종료하는데, 이전 이미지로 롤백하거나 관리자에게 알림을 보내는 메커니즘이 없습니다. 프로덕션 환경에서는 자동 롤백이나 알림 시스템 연동을 고려해 보세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/deploy/deploy-docker.sh` around lines 50 - 54, When the health check loop reaches the retry limit the script currently only prints logs and exits; modify the block that checks COUNT vs MAX_RETRIES in deploy-docker.sh to perform an automated rollback and/or notify ops: capture the current/previous image (store it before updating into a PREVIOUS_IMAGE variable), and on failure stop/remove the failed container ($TARGET_SERVICE), re-deploy the PREVIOUS_IMAGE (e.g., docker run or docker service update --image $PREVIOUS_IMAGE depending on your deployment type), and send a notification (e.g., curl to your alert webhook or mail) including $TARGET_SERVICE and the last docker logs; ensure rollback and notify steps are executed before exit 1 so failures restore the prior state and alert administrators.
24-31:docker-compose대신docker compose사용으로 전환하세요.
docker-compose(v1)은 2023년 6월 이후 지원이 종료되었으며, Docker는 모든 사용자에게docker composev2로의 마이그레이션을 권장합니다.
⚠️ 마이그레이션 시 주의사항: Docker Compose v2는 기본적으로 컨테이너 이름 구분자가 변경되었습니다(언더스코어 → 하이픈). 이 스크립트의 25-26줄에서 컨테이너 이름으로 정확히 매칭하는 로직이 있으므로, 마이그레이션 후 컨테이너 이름 지정 방식을 확인하세요. 필요시docker compose --compatibility플래그를 사용하여 v1 스타일의 이름 지정을 유지할 수 있습니다.♻️ docker compose로 전환
echo "최근 이미지 가져오는 중 (ENV: $TARGET_ENV)" -docker-compose pull $TARGET_SERVICE +docker compose pull $TARGET_SERVICE if docker ps --format '{{.Names}}' | grep -q "^${TARGET_SERVICE}$"; then echo "기존 $TARGET_SERVICE 중지 및 제거 중..." - docker-compose stop $TARGET_SERVICE + docker compose stop $TARGET_SERVICE fi echo "새 컨테이너 실행 중 - $TARGET_SERVICE..." -docker-compose up -d --no-deps $TARGET_SERVICE +docker compose up -d --no-deps $TARGET_SERVICE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/deploy/deploy-docker.sh` around lines 24 - 31, Replace all uses of the legacy "docker-compose" CLI in this script (the calls around docker-compose pull, stop, and up that reference TARGET_SERVICE) with the v2 "docker compose" form; after switching, verify container name matching logic that uses docker ps --format '{{.Names}}' | grep -q "^${TARGET_SERVICE}$" because Compose v2 may change project/name separators (underscore → hyphen); if you need v1-style names preserve compatibility by invoking "docker compose --compatibility" or adjust the grep to match the new name pattern (or use "docker compose ps" to identify the running service) so the stop/remove detection continues to work with TARGET_SERVICE.docker/application/docker-compose.application.yml (2)
13-13: DD_VERSION이 하드코딩되어 있습니다.
DD_VERSION=1.0.0이 고정값으로 설정되어 있어 Datadog에서 버전별 추적이 어려울 수 있습니다. 이미지 태그나 Git 커밋 해시를 사용하여 동적으로 설정하는 것을 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/application/docker-compose.application.yml` at line 13, DD_VERSION is hardcoded to "1.0.0"; change it to be set dynamically by using environment variable substitution or an env_file and defaulting to a build-time/Git-derived value (image tag or short commit hash) instead of a fixed literal. Replace the literal DD_VERSION=1.0.0 with a reference to an external env var (e.g., DD_VERSION) or injection from the CI/build step and ensure the compose/launch pipeline populates that variable from the image tag or git commit hash so Datadog receives version-specific values.
47-50: 모니터링 라우터의 PathPrefix 규칙이 너무 광범위합니다.
PathPrefix('/')규칙은 모든 경로와 일치하며, Host 제약이 없어서 의도치 않은 트래픽이 모니터링 엔드포인트로 라우팅될 수 있습니다. 내부 모니터링용으로만 사용할 경우 더 구체적인 경로(예:/monitoring또는/actuator)로 제한하거나, 메인 라우터처럼 Host 규칙을 추가하는 것이 좋습니다.♻️ 권장 수정안
- - "traefik.http.routers.application-monitor.rule=PathPrefix(`/`)" + - "traefik.http.routers.application-monitor.rule=PathPrefix(`/monitoring`) || PathPrefix(`/actuator`)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/application/docker-compose.application.yml` around lines 47 - 50, The Traefik router label traefik.http.routers.application-monitor.rule currently uses PathPrefix(`/`) which matches everything; change it to a more specific rule such as PathPrefix(`/monitoring`) or combine Host(...) + PathPrefix(...) to restrict access (update the label traefik.http.routers.application-monitor.rule), keep the router name application-monitor and service application-monitor-svc and port mapping (traefik.http.services.application-monitor-svc.loadbalancer.server.port=8083) unchanged, and ensure the entrypoint traefik.http.routers.application-monitor.entrypoints remains set to the monitoring entrypoint so the monitor is only reachable via the intended path/host.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/datadog/docker-compose.datadog.yml`:
- Line 6: Remove the silent default for the Datadog API key and make DD_API_KEY
a required env var so deployments fail fast; specifically replace the current
assignment that provides a fallback (the line setting DD_API_KEY with ":-blank")
with a required substitution (no ":-blank" fallback) or add an explicit
pre-deploy validation that errors when DD_API_KEY is unset so the Datadog
container cannot start quietly without a real key.
In `@docker/webhook/docker-compose.webhook.yml`:
- Line 11: 현재 docker-compose 환경에서 SECRET_TOKEN에 기본값 "blank"를 주어 WEBHOOK_KEY가
누락되면 예측 가능한 토큰이 설정됩니다; 수정 방법은 docker-compose 환경 설정에서 SECRET_TOKEN의 기본값 할당을 제거(예:
SECRET_TOKEN=${WEBHOOK_KEY})하고 컨테이너 시작 시 SECRET_TOKEN이 비어 있으면 즉시 종료하도록 엔트리포인트 또는
시작 스크립트에 검사 로직을 추가해 배포를 차단하는 것입니다; 참조 대상: 환경변수 이름 SECRET_TOKEN, 원본 변수
WEBHOOK_KEY, 그리고 서비스의 entrypoint/startup 스크립트(컨테이너 초기화 코드)에서 이 검사 구현을 추가하세요.
In `@docker/webhook/hooks.json`:
- Around line 3-35: The deploy and status hooks currently accept all HTTP
methods because they lack an http-methods field; update the hook objects with an
"http-methods" entry: for the "deploy" hook (id "deploy", execute-command
"/home/ubuntu/scripts/deploy-docker.sh") add "http-methods": ["POST"], and for
the "status" hook (id "status", execute-command
"/home/ubuntu/scripts/check-deploy-status.sh") add "http-methods": ["GET"];
ensure you insert these fields at the top level of each hook object so the
webhook server enforces the allowed methods.
In `@docker/webhook/webhook.Dockerfile`:
- Around line 2-4: Remove the hardcoded root execution: in webhook.Dockerfile
stop using USER root and instead create a non-root user (e.g., webhook), create
or match a docker group (ensure group id can be overridden at runtime to match
host docker.sock gid), add the webhook user to that docker group, chown
/etc/webhook and any runtime-required dirs to the webhook user, and keep package
installation steps (RUN apk add ...) as root during image build before switching
to the non-root USER; also remove the user: root setting from
docker-compose.webhook.yml and instead rely on the container's USER and mounting
the docker socket with the host docker group GID (or pass GID via
environment/compose to set group at runtime). Ensure the image switches to USER
webhook at the end so the webhook process runs non-root but has access to
docker.sock via group membership.
In `@scripts/deploy/check-deploy-status.sh`:
- Around line 3-6: The authentication failure branch using SECRET_TOKEN and
REQUEST_TOKEN currently calls exit 0 which signals success; change the exit to a
non-zero value (e.g., exit 1) so the script returns a failure status on
unauthorized requests—update the block that checks [ -z "$SECRET_TOKEN" ] || [
"$REQUEST_TOKEN" != "$SECRET_TOKEN" ] to print the same JSON error and then call
exit 1 instead of exit 0.
---
Nitpick comments:
In `@docker/application/docker-compose.application.yml`:
- Line 13: DD_VERSION is hardcoded to "1.0.0"; change it to be set dynamically
by using environment variable substitution or an env_file and defaulting to a
build-time/Git-derived value (image tag or short commit hash) instead of a fixed
literal. Replace the literal DD_VERSION=1.0.0 with a reference to an external
env var (e.g., DD_VERSION) or injection from the CI/build step and ensure the
compose/launch pipeline populates that variable from the image tag or git commit
hash so Datadog receives version-specific values.
- Around line 47-50: The Traefik router label
traefik.http.routers.application-monitor.rule currently uses PathPrefix(`/`)
which matches everything; change it to a more specific rule such as
PathPrefix(`/monitoring`) or combine Host(...) + PathPrefix(...) to restrict
access (update the label traefik.http.routers.application-monitor.rule), keep
the router name application-monitor and service application-monitor-svc and port
mapping
(traefik.http.services.application-monitor-svc.loadbalancer.server.port=8083)
unchanged, and ensure the entrypoint
traefik.http.routers.application-monitor.entrypoints remains set to the
monitoring entrypoint so the monitor is only reachable via the intended
path/host.
In `@scripts/deploy/check-deploy-status.sh`:
- Around line 10-19: Open the lock file descriptor (exec 200>"$LOCK_FILE") and
then register a trap for EXIT and relevant signals (e.g., INT, TERM) that always
releases and closes fd 200 (perform flock -u 200 if held and exec 200>&-); move
or add the exec 200>&- cleanup into that trap and ensure the trap is set
immediately after opening the descriptor so early exits still run the cleanup,
leaving the rest of the flock -n 200 logic unchanged.
In `@scripts/deploy/deploy-docker.sh`:
- Line 18: 현재 스크립트에서 변수 참조가 따옴표 없이 사용되어 공백 포함 경로 등에서 문제가 발생할 수 있습니다;
scripts/deploy/deploy-docker.sh 내의 cd $PROJECT_DIR 뿐만 아니라 모든 변수 참조(예:
$PROJECT_DIR, $TARGET_SERVICE, 기타 ENV/ARGS)를 일관되게 "$PROJECT_DIR",
"$TARGET_SERVICE" 형태로 감싸고, 명령어 인자나 파일 경로로 전달되는 곳에서도 동일하게 따옴표를 적용하도록 수정하세요.
- Around line 9-12: Add a cleanup trap so the lock is always released and the
lock file removed on exit: after creating LOCK_FILE and opening FD 200 (exec
200>"$LOCK_FILE") register a trap for EXIT that runs flock -u 200 to unlock,
closes FD 200 (exec 200>&-) and removes "$LOCK_FILE" (rm -f "$LOCK_FILE"); keep
existing non-blocking flock -n 200 check as-is and ensure the trap is installed
immediately after opening the descriptor to cover abnormal exits.
- Around line 50-54: When the health check loop reaches the retry limit the
script currently only prints logs and exits; modify the block that checks COUNT
vs MAX_RETRIES in deploy-docker.sh to perform an automated rollback and/or
notify ops: capture the current/previous image (store it before updating into a
PREVIOUS_IMAGE variable), and on failure stop/remove the failed container
($TARGET_SERVICE), re-deploy the PREVIOUS_IMAGE (e.g., docker run or docker
service update --image $PREVIOUS_IMAGE depending on your deployment type), and
send a notification (e.g., curl to your alert webhook or mail) including
$TARGET_SERVICE and the last docker logs; ensure rollback and notify steps are
executed before exit 1 so failures restore the prior state and alert
administrators.
- Around line 24-31: Replace all uses of the legacy "docker-compose" CLI in this
script (the calls around docker-compose pull, stop, and up that reference
TARGET_SERVICE) with the v2 "docker compose" form; after switching, verify
container name matching logic that uses docker ps --format '{{.Names}}' | grep
-q "^${TARGET_SERVICE}$" because Compose v2 may change project/name separators
(underscore → hyphen); if you need v1-style names preserve compatibility by
invoking "docker compose --compatibility" or adjust the grep to match the new
name pattern (or use "docker compose ps" to identify the running service) so the
stop/remove detection continues to work with TARGET_SERVICE.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.gitignoredocker/application/docker-compose.application.ymldocker/datadog/docker-compose.datadog.ymldocker/docker-compose.spring.ymldocker/docker-compose.ymldocker/traefik/docker-compose.traefik.ymldocker/webhook/docker-compose.webhook.ymldocker/webhook/hooks.jsondocker/webhook/webhook.Dockerfilescripts/deploy/check-deploy-status.shscripts/deploy/deploy-docker.shscripts/init/init-letsencrypt.sh
💤 Files with no reviewable changes (1)
- docker/docker-compose.spring.yml
| USER root | ||
| RUN apk add --no-cache docker-cli docker-cli-compose curl bash util-linux | ||
| WORKDIR /etc/webhook |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify USER declarations, docker.sock mounts, and socket access configuration
rg -n "^\s*USER\s+|^\s*user:\s*|/var/run/docker.sock|group_add" docker/webhook/webhook.Dockerfile docker/webhook/docker-compose.webhook.ymlRepository: debate-timer/debate-timer-be
Length of output: 271
🏁 Script executed:
#!/bin/bash
# Read the full Dockerfile to understand the complete context
cat -n docker/webhook/webhook.DockerfileRepository: debate-timer/debate-timer-be
Length of output: 229
🏁 Script executed:
#!/bin/bash
# Check if docker-compose file exists and review its contents
if [ -f docker/webhook/docker-compose.webhook.yml ]; then
cat -n docker/webhook/docker-compose.webhook.yml
else
# Try other common naming patterns
find docker/webhook -name "*compose*" -type f 2>/dev/null | head -10
fiRepository: debate-timer/debate-timer-be
Length of output: 1516
🏁 Script executed:
#!/bin/bash
# Check for any entrypoint or command specifications
rg -n "ENTRYPOINT|CMD|entrypoint|command:" docker/webhook/ -A 2Repository: debate-timer/debate-timer-be
Length of output: 871
웹훅 프로세스가 루트로 실행되는 문제를 모두 해결해야 합니다.
Dockerfile의 USER root (Line 2)뿐만 아니라 docker-compose.yml의 user: root (Line 7)도 제거해야 합니다. Dockerfile에서만 사용자를 변경해도 Compose 파일의 user 설정이 이를 무시합니다.
웹훅이 docker.sock에 접근해야 하므로, 비루트 사용자를 docker 그룹에 추가하는 방식으로 권한을 분리하세요:
권장 수정
docker/webhook/webhook.Dockerfile:
FROM almir/webhook:2.8.3
USER root
RUN apk add --no-cache docker-cli docker-cli-compose curl bash util-linux
+RUN addgroup -S webhook && adduser -S -G webhook webhook && addgroup webhook docker
WORKDIR /etc/webhook
+USER webhookdocker/webhook/docker-compose.webhook.yml:
webhook:
build:
context: ..
dockerfile: ./webhook/webhook.Dockerfile
container_name: webhook
- user: root
command: ["-verbose", "-hooks=/etc/webhook/hooks.json", "-hotreload"]📝 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.
| USER root | |
| RUN apk add --no-cache docker-cli docker-cli-compose curl bash util-linux | |
| WORKDIR /etc/webhook | |
| FROM almir/webhook:2.8.3 | |
| USER root | |
| RUN apk add --no-cache docker-cli docker-cli-compose curl bash util-linux | |
| RUN addgroup -S webhook && adduser -S -G webhook webhook && addgroup webhook docker | |
| WORKDIR /etc/webhook | |
| USER webhook |
🧰 Tools
🪛 Trivy (0.69.1)
[error] 2-2: Image user should not be 'root'
Last USER command in Dockerfile should not be 'root'
Rule: DS-0002
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/webhook/webhook.Dockerfile` around lines 2 - 4, Remove the hardcoded
root execution: in webhook.Dockerfile stop using USER root and instead create a
non-root user (e.g., webhook), create or match a docker group (ensure group id
can be overridden at runtime to match host docker.sock gid), add the webhook
user to that docker group, chown /etc/webhook and any runtime-required dirs to
the webhook user, and keep package installation steps (RUN apk add ...) as root
during image build before switching to the non-root USER; also remove the user:
root setting from docker-compose.webhook.yml and instead rely on the container's
USER and mounting the docker socket with the host docker group GID (or pass GID
via environment/compose to set group at runtime). Ensure the image switches to
USER webhook at the end so the webhook process runs non-root but has access to
docker.sock via group membership.
unifolio0
left a comment
unifolio0
left a comment
There was a problem hiding this comment.
/noti
@leegwichan approve 합니다
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docker/webhook/docker-compose.webhook.yml (2)
29-34: 헬스체크 간격이 120초로 설정되어 있습니다.2분 간격은 서비스 장애 감지가 늦어질 수 있습니다. 웹훅 서비스 특성상 빈번한 체크가 필요 없다면 현재 설정도 괜찮지만, 일반적으로 30~60초가 권장됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/webhook/docker-compose.webhook.yml` around lines 29 - 34, The healthcheck interval in the healthcheck block (the healthcheck: stanza using test: ["CMD-SHELL", "nc -z localhost 9000 || exit 1"]) is set to 120s which delays failure detection; update the interval to a shorter value (typically 30s or 60s) and optionally adjust timeout/retries/start_period accordingly (e.g., interval: 30s, timeout: 30s, retries: 3) so the webhook container is monitored more frequently without causing false positives.
23-23: 호스트 프로젝트 경로가 하드코딩되어 있습니다.
../..:/home/ubuntu/debate-timer볼륨 마운트가/home/ubuntu/debate-timer경로에 의존합니다. 배포 스크립트도 동일한 경로를 사용하므로 일관성은 있지만, 환경 변수로 관리하면 유연성이 향상됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/webhook/docker-compose.webhook.yml` at line 23, 현재 docker-compose.webhook.yml의 볼륨 마운트 문자열 '../..:/home/ubuntu/debate-timer'이 하드코딩되어 있으므로 이를 환경변수로 대체하세요: 교체 대상 문자열('../..:/home/ubuntu/debate-timer')을 예컨대 '${HOST_PROJECT_DIR}:/home/ubuntu/debate-timer' 형식으로 바꾸고 .env 또는 CI/CD 배포 스크립트에서 HOST_PROJECT_DIR 값을 설정하도록 업데이트(또는 기본값을 제공)하며, 배포 스크립트와 문서에서 HOST_PROJECT_DIR 사용을 반영해 경로를 일관되게 주입하도록 수정하세요.scripts/deploy/deploy-docker.sh (1)
20-20: 경로 변수에 따옴표를 추가하세요.
$PROJECT_DIR와$DOCKER_DIR변수가 공백이 포함된 경로일 경우 예기치 않은 동작이 발생할 수 있습니다.🔧 권장 수정
-cd $PROJECT_DIR || { echo "프로젝트 디렉토리 이동 실패"; exit 1; } +cd "$PROJECT_DIR" || { echo "프로젝트 디렉토리 이동 실패"; exit 1; }-cd $DOCKER_DIR || { echo "디렉토리 이동 실패"; exit 1; } +cd "$DOCKER_DIR" || { echo "디렉토리 이동 실패"; exit 1; }Also applies to: 39-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/deploy/deploy-docker.sh` at line 20, The script uses unquoted path variables which break on spaces; update all uses of PROJECT_DIR and DOCKER_DIR (e.g., the cd commands around the cd $PROJECT_DIR and cd $DOCKER_DIR calls) to use quoted expansions like "$PROJECT_DIR" and "$DOCKER_DIR" and similarly quote any other path variables in commands (cp, mv, etc.) to ensure paths containing spaces are handled safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/deploy/deploy-docker.sh`:
- Line 44: The script currently runs "docker compose pull $TARGET_SERVICE"
without aborting on failure, allowing deployment to continue with an old image;
change the deployment flow so that after the docker compose pull command
(referencing TARGET_SERVICE and the docker compose pull invocation) the script
checks the command exit status and exits non‑zero on failure (or enable strict
failure mode like set -e at top) so the deployment halts if the image pull
fails.
- Around line 32-37: git branch --show-current can return an empty string (or
"HEAD") in a detached HEAD state causing git fetch/reset to be called with empty
ref; update the script around CURRENT_BRANCH to detect detached HEAD (check if
CURRENT_BRANCH is empty or equals "HEAD") and branch the logic: when a normal
branch name exists keep the existing git fetch origin "$CURRENT_BRANCH" and git
reset --hard origin/"$CURRENT_BRANCH", but when detached use a safe fallback
such as running git fetch origin && git reset --hard HEAD (or git fetch origin
&& git reset --hard FETCH_HEAD) instead; implement this conditional around the
CURRENT_BRANCH variable to prevent invoking git with an empty ref.
---
Nitpick comments:
In `@docker/webhook/docker-compose.webhook.yml`:
- Around line 29-34: The healthcheck interval in the healthcheck block (the
healthcheck: stanza using test: ["CMD-SHELL", "nc -z localhost 9000 || exit 1"])
is set to 120s which delays failure detection; update the interval to a shorter
value (typically 30s or 60s) and optionally adjust timeout/retries/start_period
accordingly (e.g., interval: 30s, timeout: 30s, retries: 3) so the webhook
container is monitored more frequently without causing false positives.
- Line 23: 현재 docker-compose.webhook.yml의 볼륨 마운트 문자열
'../..:/home/ubuntu/debate-timer'이 하드코딩되어 있으므로 이를 환경변수로 대체하세요: 교체 대상
문자열('../..:/home/ubuntu/debate-timer')을 예컨대
'${HOST_PROJECT_DIR}:/home/ubuntu/debate-timer' 형식으로 바꾸고 .env 또는 CI/CD 배포 스크립트에서
HOST_PROJECT_DIR 값을 설정하도록 업데이트(또는 기본값을 제공)하며, 배포 스크립트와 문서에서 HOST_PROJECT_DIR 사용을
반영해 경로를 일관되게 주입하도록 수정하세요.
In `@scripts/deploy/deploy-docker.sh`:
- Line 20: The script uses unquoted path variables which break on spaces; update
all uses of PROJECT_DIR and DOCKER_DIR (e.g., the cd commands around the cd
$PROJECT_DIR and cd $DOCKER_DIR calls) to use quoted expansions like
"$PROJECT_DIR" and "$DOCKER_DIR" and similarly quote any other path variables in
commands (cp, mv, etc.) to ensure paths containing spaces are handled safely.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docker/webhook/docker-compose.webhook.ymldocker/webhook/hooks.jsondocker/webhook/webhook.Dockerfilescripts/deploy/deploy-docker.sh
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/deploy/deploy-docker.sh`:
- Around line 24-31: The git sync block handling TARGET_ENV currently continues
on errors from git fetch/switch/reset; change it to fail fast by either enabling
strict shell error handling (set -euo pipefail at script top) or explicitly
check each git command's exit status and abort with an error message and
non-zero exit; ensure the git fetch, git switch and git reset commands in the
TARGET_ENV branches (the git fetch/switch/reset sequence) are wrapped so any
failure logs which command failed and exits immediately to prevent proceeding
with an unintended commit.
- Line 55: The deploy script currently runs "docker compose up -d --no-deps
$TARGET_SERVICE" without handling failures immediately; modify the script so the
docker compose invocation is checked and the deployment aborts on failure:
ensure the script either enables strict error handling (set -euo pipefail at the
top) or explicitly test the exit status of the docker compose command (when
invoking the command for TARGET_SERVICE) and echo an error and exit 1 if it
fails, so the deploy stops immediately instead of failing later in healthcheck
loops.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docker/application/docker-compose.application.ymldocker/datadog/docker-compose.datadog.ymldocker/webhook/docker-compose.webhook.ymldocker/webhook/hooks.jsonscripts/deploy/deploy-docker.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- docker/datadog/docker-compose.datadog.yml
- docker/webhook/hooks.json
- docker/webhook/docker-compose.webhook.yml
- docker/application/docker-compose.application.yml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docker/webhook/webhook.Dockerfile (1)
2-2:⚠️ Potential issue | 🟠 Major컨테이너 런타임을 root로 두면 안 됩니다.
Line 2의
USER root가 마지막 사용자로 유지되어 webhook 프로세스가 root 권한으로 실행됩니다.docker.sock접근이 필요하더라도 빌드 단계만 root로 두고, 실행 단계는 비루트 사용자로 전환해야 합니다.권장 수정안
FROM almir/webhook:2.8.3 USER root RUN apk add --no-cache docker-cli-compose curl bash util-linux git +RUN addgroup -S webhook && adduser -S -G webhook webhook \ + && mkdir -p /etc/webhook \ + && chown -R webhook:webhook /etc/webhook WORKDIR /etc/webhook RUN git config --global --add safe.directory /home/ubuntu/debate-timer +USER webhook🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/webhook/webhook.Dockerfile` at line 2, The Dockerfile currently leaves USER root as the final image user; change the image to run as a non-root user by creating a dedicated user/group in the Dockerfile during build (e.g., add a user like "webhook" and a group), adjust ownership/permissions of any needed sockets or runtime dirs (e.g., chown /var/run/docker.sock or add the user to the docker group) during the build stage, and set USER to that non-root username instead of root before the container start (replace the final "USER root" with "USER webhook" or equivalent); ensure any build-time root operations remain in earlier layers but the runtime user is non-root so webhook does not run as root.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/webhook/webhook.Dockerfile`:
- Line 7: Hardcoded deployment paths (the RUN git config --global --add
safe.directory /home/ubuntu/debate-timer and the /home/ubuntu/debate-timer
references inside hooks.json) break portability; change the Dockerfile to accept
a build ARG (e.g., APP_DIR) and use it when setting git safe.directory (prefer
--system scope if appropriate), and update the Docker build step to template or
generate hooks.json at build/run time using that same ARG/ENV so all references
to /home/ubuntu/debate-timer are parameterized; ensure unique identifiers
mentioned: the RUN git config command, the safe.directory value, and the
hooks.json file are updated to use the injected ARG/ENV instead of a hardcoded
path.
---
Duplicate comments:
In `@docker/webhook/webhook.Dockerfile`:
- Line 2: The Dockerfile currently leaves USER root as the final image user;
change the image to run as a non-root user by creating a dedicated user/group in
the Dockerfile during build (e.g., add a user like "webhook" and a group),
adjust ownership/permissions of any needed sockets or runtime dirs (e.g., chown
/var/run/docker.sock or add the user to the docker group) during the build
stage, and set USER to that non-root username instead of root before the
container start (replace the final "USER root" with "USER webhook" or
equivalent); ensure any build-time root operations remain in earlier layers but
the runtime user is non-root so webhook does not run as root.
|
|
||
| WORKDIR /etc/webhook | ||
|
|
||
| RUN git config --global --add safe.directory /home/ubuntu/debate-timer |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 목적: safe.directory 설정 경로와 실제 마운트 경로 정합성 점검(읽기 전용)
set -e
echo "[1] safe.directory 관련 설정 검색"
rg -n -C2 'safe\.directory|/home/ubuntu/debate-timer|GIT_SAFE_DIRECTORY' docker
echo
echo "[2] webhook compose의 볼륨/경로 설정 검색"
rg -n -C3 'webhook|volumes:|/home/ubuntu/debate-timer|hooks\.json' dockerRepository: debate-timer/debate-timer-be
Length of output: 6613
🏁 Script executed:
#!/bin/bash
# Check full Dockerfile content
echo "[1] Full webhook.Dockerfile"
cat -n docker/webhook/webhook.Dockerfile
echo
echo "[2] Full hooks.json content"
cat -n docker/webhook/hooks.json
echo
echo "[3] Full docker-compose.webhook.yml"
cat -n docker/webhook/docker-compose.webhook.ymlRepository: debate-timer/debate-timer-be
Length of output: 3040
배포 이식성을 깨뜨리는 하드코딩된 경로는 safe.directory 뿐만 아니라 hooks.json까지 광범위합니다.
Line 7의 /home/ubuntu/debate-timer 고정 경로가 safe.directory에만 있는 것이 아니라, hooks.json의 실행 명령과 작업 디렉토리(5, 6, 21, 22줄)에도 동일하게 하드코딩되어 있습니다. 서버 계정이나 디렉토리 구조가 바뀔 경우 Git 명령뿐만 아니라 배포 스크립트 실행 자체가 실패합니다. Dockerfile의 경로는 ARG로 주입하고 --system 범위로 설정하되, hooks.json도 동시에 템플릿화하거나 환경 변수로 매개변수화해야 합니다.
권장 수정안
FROM almir/webhook:2.8.3
USER root
RUN apk add --no-cache docker-cli-compose curl bash util-linux git
+ARG GIT_SAFE_DIRECTORY=/home/ubuntu/debate-timer
WORKDIR /etc/webhook
-RUN git config --global --add safe.directory /home/ubuntu/debate-timer
+RUN git config --system --add safe.directory "${GIT_SAFE_DIRECTORY}"(추가로 hooks.json의 /home/ubuntu/debate-timer 참조도 환경 변수 치환이나 동적 생성으로 매개변수화 권장)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/webhook/webhook.Dockerfile` at line 7, Hardcoded deployment paths (the
RUN git config --global --add safe.directory /home/ubuntu/debate-timer and the
/home/ubuntu/debate-timer references inside hooks.json) break portability;
change the Dockerfile to accept a build ARG (e.g., APP_DIR) and use it when
setting git safe.directory (prefer --system scope if appropriate), and update
the Docker build step to template or generate hooks.json at build/run time using
that same ARG/ENV so all references to /home/ubuntu/debate-timer are
parameterized; ensure unique identifiers mentioned: the RUN git config command,
the safe.directory value, and the hooks.json file are updated to use the
injected ARG/ENV instead of a hardcoded path.
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti
- CD 시, 스크립트 & docker-compose.yml 업데이트 기능 추가 완료
- 새로운 EC2에서 초기 스크립트 ~ CD까지 정상작동 확인 완료
coli-geonwoo
left a comment
coli-geonwoo
left a comment
There was a problem hiding this comment.
/noti
커찬.. 고생많았슴다.. PR 올리고 거의 몇 주만에 리뷰다네요.. 전 리드자격이 없는 팀원이에요 ㅠㅠㅠ
몇가지 궁금한점 리뷰 남겼으니 확인부탁드립니당.. 그리고 커찬 의견이 궁금한 부분이 있어 염치없는 RC로 남겨둡니다.. 아마 한번 핑퐁하고 Approve 바로 할듯!
| retries: 10 | ||
| start_period: 450s |
There was a problem hiding this comment.
[사소한 의견]
오... 거의 7분 30초동안은 실패 카운트 안하는 것 같은데 혹시 기준점이 따로 있엇나요?
10분 동안 애플리케이션 healthcheck 시도할 수도 있을 것 같아서 period와 retries 횟수 조금 줄이는 건 어떨까... 하는 사견입니다.
There was a problem hiding this comment.
메모리 1GB, 스왑 메모리 2GB 기준 평균 Spring Boot 로딩 시간이 5분입니다. (H2 내장 DB 사용 기준)
그래서 일단 7.5분으로 진행하고, 나중에 DEV, PROD 부팅 평균 시간을 측정해서 개선해 나가는게 좋을 것 같아요!
| - "traefik.http.routers.application.entrypoints=websecure" | ||
| - "traefik.http.routers.application.tls=true" | ||
| - "traefik.http.routers.application.tls.certresolver=myresolver" | ||
| - "traefik.http.routers.application.service=application" |
There was a problem hiding this comment.
이거 뭔가 했는데 Nginx에서 편집해줬던 443 라우팅 정책이네요 ㄷㄷ traefik이 미래다..
|
|
||
| - "com.datadoghq.ad.logs=[{\"source\": \"java\", \"service\": \"debate-timer\"}]" | ||
|
|
||
| restart: always |
There was a problem hiding this comment.
[사소한 의견..]
always 쓰다가 Docker Stop 명령어로 개발자가 수동 정지해도 계속 재시도하는 경우가 있더라고요
unless-stopped 같은 경우에는 always처럼 실패 경우에 재시도 하는데 stop 명령어처럼 수동정지하는 경우에는 재시도 안하는 걸로 알고 있어서 이 설정으로 바꾸는 건 어떨까요?
restart: unless-stopped
| context: .. | ||
| dockerfile: ./webhook/webhook.Dockerfile | ||
| container_name: webhook | ||
| user: root |
There was a problem hiding this comment.
후순위긴 하지만 이거 나중에 시도해봐도 좋을 듯 👍
scripts/deploy/deploy-docker.sh
Outdated
| docker compose up -d --no-deps $TARGET_SERVICE | ||
|
|
||
| echo "헬스 체크 진행 중 - $TARGET_SERVICE" | ||
| MAX_RETRIES=100 |
There was a problem hiding this comment.
최대 100번 시도...? 좀 줄이는 것도 좋을 것 같아요. 이거 100번 시도하면 17분 정도 계속 시도할 것 같아서..
There was a problem hiding this comment.
반영했습니다~ 일단 50번으로 줄여놓았습니다. dev, prod 구축 후, 배포 평균 시간에 맞춰 조정하도록 하죠!
There was a problem hiding this comment.
/noti @coli-geonwoo
작성해 준 리뷰 반영했습니다. 헬스 체크 시간은 dev, prod 구축 후 평균 시작 시간에 따라 조정하도록 하겠습니다.
| retries: 10 | ||
| start_period: 450s |
There was a problem hiding this comment.
메모리 1GB, 스왑 메모리 2GB 기준 평균 Spring Boot 로딩 시간이 5분입니다. (H2 내장 DB 사용 기준)
그래서 일단 7.5분으로 진행하고, 나중에 DEV, PROD 부팅 평균 시간을 측정해서 개선해 나가는게 좋을 것 같아요!
scripts/deploy/deploy-docker.sh
Outdated
| docker compose up -d --no-deps $TARGET_SERVICE | ||
|
|
||
| echo "헬스 체크 진행 중 - $TARGET_SERVICE" | ||
| MAX_RETRIES=100 |
There was a problem hiding this comment.
반영했습니다~ 일단 50번으로 줄여놓았습니다. dev, prod 구축 후, 배포 평균 시간에 맞춰 조정하도록 하죠!
3 participants
🚩 연관 이슈
#235 #236
🗣️ 리뷰 요구사항 (선택)
더 해야할 작업 리스트
PR 머지 전
서버 이관 작업
기타 개선 작업
Summary by CodeRabbit
New Features
Bug Fixes / Security
Chores
Style