[FEATURE] 테라폼 기반 배포 구현 - #203
Merged
Merged
Conversation
1 task
|
Warning Review limit reached
More reviews will be available in 20 minutes and 40 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
Walkthrough이 PR은 AWS Terraform 기반의 IaC 인프라 관리 파이프라인을 구축합니다. VPC, EC2, RDS PostgreSQL, S3, ECR을 정의하고, EC2 사용자 데이터로 Docker/Nginx/AWS CLI를 자동 설치합니다. Nginx는 HTTP/HTTPS/내부 모드 템플릿으로 요청 ID 추적 및 JSON 로깅을 제공하고, Caddy는 선택적 리버스 프록시로 제공됩니다. 배포 자동화 스크립트는 Docker 이미지 빌드·푸시 후 원격 EC2에서 인프라 설치 및 블루-그린 배포를 실행합니다. Prometheus, Alertmanager, Grafana로 구성한 경량 모니터링 스택은 SSD API 및 호스트 메트릭을 수집하고 Discord 웹훅으로 알림을 전달합니다. ChangesTerraform 기반 AWS 인프라 및 배포 파이프라인
🎯 4 (Complex) | ⏱️ ~75 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
Author
📝 Code Coverage
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
deploy/ec2/nginx/site.http.conf.template (1)
1-18: ⚡ Quick win세 nginx 템플릿 간 공통 설정 중복을 제거하는 것을 권장합니다.
Lines 1-18의 요청 ID 매핑 및 JSON 로그 포맷 정의가
site.http.conf.template,site.https.conf.template,site.internal.conf.template세 파일에서 동일하게 반복됩니다. 이는 유지보수 시 세 곳을 모두 수정해야 하며 불일치 위험이 있습니다.♻️ 제안하는 리팩터링
1단계: 공통 설정을 별도 파일로 분리
deploy/ec2/nginx/ssd-common.conf파일 생성:map $http_x_request_id $ssd_request_id { "~^(?:[A-Fa-f0-9]{32}|[A-Fa-f0-9-]{36})$" $http_x_request_id; default $request_id; } log_format ssd_json escape=json '{"timestamp":"$time_iso8601",' '"requestId":"$ssd_request_id",' '"remoteAddr":"$remote_addr",' '"method":"$request_method",' '"uri":"$request_uri",' '"status":$status,' '"bodyBytesSent":$body_bytes_sent,' '"requestTime":$request_time,' '"upstreamStatus":"$upstream_status",' '"upstreamResponseTime":"$upstream_response_time",' '"httpReferer":"$http_referer",' '"userAgent":"$http_user_agent"}';2단계: 각 템플릿에서 include로 참조
+include /etc/nginx/conf.d/ssd-common.conf; + -map $http_x_request_id $ssd_request_id { - "~^(?:[A-Fa-f0-9]{32}|[A-Fa-f0-9-]{36})$" $http_x_request_id; - default $request_id; -} - -log_format ssd_json escape=json - '{"timestamp":"$time_iso8601",' - ... - '"userAgent":"$http_user_agent"}'; - server {3단계:
install_infra.sh에서 공통 설정 파일 설치 추가copy_if_changed "${SCRIPT_DIR}/nginx/ssd-common.conf" "/etc/nginx/conf.d/ssd-common.conf" 644🤖 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 `@deploy/ec2/nginx/site.http.conf.template` around lines 1 - 18, Extract the duplicated mapping and log_format block (the map $http_x_request_id $ssd_request_id and log_format ssd_json definitions) into a new shared file (e.g., deploy/ec2/nginx/ssd-common.conf), replace the repeated blocks in site.http.conf.template, site.https.conf.template and site.internal.conf.template with an include of that shared file, and update the installation script (install_infra.sh) to copy the new ssd-common.conf into /etc/nginx/conf.d (use the existing copy_if_changed pattern) so all three templates use the single source of truth.
🤖 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 `@deploy/ec2/install_caddy.sh`:
- Around line 18-24: 설정 파일 경로
APP_CONFIG_SOURCE="/opt/ssd/config/application-dev.yml"가 하드코딩되어 있지만 존재 여부를 확인하지
않고 install_infra.sh를 호출하고 있어 오류가 발생할 수 있습니다; install_caddy.sh에서 SCRIPT_DIR/ 설치
호출 전에 해당 파일이 실제로 존재하는지(test -f "$APP_CONFIG_SOURCE") 검사하고, 파일이 없을 경우 적절한 error
message를 출력(processLogger 대신 echo/stderr)하고 비정상 종료(exit 1) 하도록 변경하세요; 검사 코드는
APP_CONFIG_SOURCE 변수와 기존 install_infra.sh 호출 블록 바로 위(현재 if [[
"${NGINX_INTERNAL_ONLY}" == "true" ]] 내부)에서 수행하면 됩니다.
In `@deploy/ec2/install_prod_monitoring_lite.sh`:
- Around line 23-31: The current block that extracts webhook_url from
APP_CONFIG_FILE and writes to DISCORD_WEBHOOK_FILE must validate the extracted
value and fail fast if empty: after computing webhook_url (the variable in this
code block), check that it is non-empty (and not just whitespace); if empty,
emit an error message to stderr or logs and exit with a non-zero status instead
of writing an empty file and continuing; only write to DISCORD_WEBHOOK_FILE and
chmod it when webhook_url contains a valid value.
- Around line 17-20: 현재 스크립트만으로는 monitoring-lite만 복사되어 Compose가 바인드 마운트하는
../monitoring/grafana 경로와 불일치해 Grafana 컨테이너가 실패할 수 있으니, MONITORING_SOURCE_DIR에서
공용 Grafana 자산(예: ../monitoring/grafana/*)도 tmp_dir로 함께 복사하도록 설치 흐름을 확장하고 기존
tmp_dir → MONITORING_DIR 복사와 chown 처리(변수: MONITORING_SOURCE_DIR, tmp_dir,
MONITORING_DIR, DEPLOY_USER)를 동일하게 적용하며 기존 .env 및
alertmanager/secrets/discord_webhook_url 삭제 로직을 유지해 주세요.
In `@deploy/ec2/nginx/site.https.conf.template`:
- Around line 56-60: The HTTPS nginx template (site.https.conf.template) assumes
/etc/letsencrypt/live/__SERVER_NAME__/fullchain.pem and privkey.pem exist before
nginx starts; ensure the deployment workflow provisions/renews certificates
prior to running nginx -t or systemd start. Fix by adding certificate
provisioning to the startup flow — either delegate TLS to install_caddy.sh and
use nginx only internally, or add certbot install/obtain/renew steps into
install_infra.sh (or the CI/CD job) and perform cert existence checks and nginx
-t after provisioning; update the bootstrap/start script to wait for/validate
the files referenced by ssl_certificate and ssl_certificate_key before launching
nginx.
In `@infra/terraform/main.tf`:
- Around line 257-260: The RDS subnet group aws_db_subnet_group.main is
currently using public subnets aws_subnet.public_a and aws_subnet.public_c
(which have map_public_ip_on_launch = true); update subnet_ids to reference the
dedicated private subnet resources (e.g., aws_subnet.private_a.id and
aws_subnet.private_c.id or whatever private subnet identifiers exist) so the DB
is placed in private subnets instead of public ones; ensure any variable/local
name (local.name_prefix) remains unchanged and run terraform fmt/validate after
updating aws_db_subnet_group.main.
- Around line 290-307: Add IMDSv2 enforcement and explicit root volume
encryption to the aws_instance.app resource: add a metadata_options block with
http_tokens = "required" (and optionally http_put_response_hop_limit) to force
IMDSv2, and set encrypted = true inside the root_block_device (and optionally
kms_key_id if a specific CMK is required) so the root volume does not rely on
account/region defaults; update the template for resource aws_instance.app,
referencing metadata_options and the existing root_block_device stanza.
- Around line 232-241: Locate the aws_ecr_repository resource named "app"
(resource "aws_ecr_repository" "app") and change the image_tag_mutability
attribute from "MUTABLE" to "IMMUTABLE" so tags cannot be overwritten; update
any related documentation or variable defaults (var.ecr_repository_name usage
can remain) and run terraform plan/apply to apply the change.
In `@infra/terraform/outputs.tf`:
- Around line 41-45: Remove the Terraform output block named "db_password" (the
output "db_password" that returns local.resolved_db_password) or replace it with
a secure secret-store export instead; update any automation that expects this
output to instead read the password from the chosen secret manager, and
remove/rename references to the db_password output in deployment scripts or CI
so nothing consumes terraform output -raw/-json for credentials. Ensure any
replacement uses a secret backend (e.g., AWS Secrets Manager / SSM Parameter
Store) and that the code paths which previously referenced
local.resolved_db_password are updated to fetch from that secret store.
In `@infra/terraform/scripts/deploy_ec2_app.sh`:
- Around line 72-87: The heredoc in deploy_ec2_app.sh currently uses an unquoted
EOF so the local shell expands variables before sending the script; update the
SSH heredoc to use a quoted delimiter (e.g. 'EOF') to prevent client-side
expansion and instead pass required values explicitly into the remote
environment (for example export or inline env for the remote commands that call
/home/${SSH_USER}/ec2/install_infra.sh and
/home/${SSH_USER}/ec2/blue_green_deploy.sh). Ensure the environment variables
referenced inside the heredoc—APP_CONFIG_SOURCE, NGINX_ENABLE_SSL,
NGINX_SERVER_NAME, AWS_REGION, ECR_URL, IMAGE_TAG, SPRING_PROFILE,
APP_GRAFANA_BASE_URL, HEALTH_PATH—are set on the remote side (via sudo -E,
export, or by prefixing the remote command) so the scripts install_infra.sh and
blue_green_deploy.sh receive them and no client-side expansion happens.
In `@infra/terraform/variables.tf`:
- Around line 86-105: Remove the hardcoded secret defaults for the variables
jwt_secret, discord_webhook_url, and sentry_dsn: delete the default = "..."
lines and keep sensitive = true so these vars must be supplied externally (via
terraform.tfvars, environment variables, or a secret manager); optionally add a
brief description or validation block if desired to enforce non-empty values
(refer to variables jwt_secret, discord_webhook_url, sentry_dsn to locate the
declarations).
---
Nitpick comments:
In `@deploy/ec2/nginx/site.http.conf.template`:
- Around line 1-18: Extract the duplicated mapping and log_format block (the map
$http_x_request_id $ssd_request_id and log_format ssd_json definitions) into a
new shared file (e.g., deploy/ec2/nginx/ssd-common.conf), replace the repeated
blocks in site.http.conf.template, site.https.conf.template and
site.internal.conf.template with an include of that shared file, and update the
installation script (install_infra.sh) to copy the new ssd-common.conf into
/etc/nginx/conf.d (use the existing copy_if_changed pattern) so all three
templates use the single source of truth.
🪄 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: CHILL
Plan: Pro
Run ID: 92a26415-77e6-44b0-9566-a991dc07d199
📒 Files selected for processing (25)
.gitignoredeploy/ec2/caddy/Caddyfile.templatedeploy/ec2/install_caddy.shdeploy/ec2/install_infra.shdeploy/ec2/install_prod_monitoring_lite.shdeploy/ec2/monitoring-lite/alertmanager/alertmanager.ymldeploy/ec2/monitoring-lite/alertmanager/secrets/.gitkeepdeploy/ec2/monitoring-lite/alertmanager/templates/discord.tmpldeploy/ec2/monitoring-lite/docker-compose.monitoring-lite.ymldeploy/ec2/monitoring-lite/prometheus/prometheus.ymldeploy/ec2/monitoring-lite/prometheus/rules/ssd-alerts.ymldeploy/ec2/monitoring/prometheus/prometheus.ymldeploy/ec2/nginx/site.http.conf.templatedeploy/ec2/nginx/site.https.conf.templatedeploy/ec2/nginx/site.internal.conf.templateinfra/terraform/.terraform.lock.hclinfra/terraform/README.mdinfra/terraform/main.tfinfra/terraform/outputs.tfinfra/terraform/providers.tfinfra/terraform/scripts/deploy_ec2_app.shinfra/terraform/templates/ec2_user_data.sh.tftplinfra/terraform/terraform.tfvars.exampleinfra/terraform/variables.tfinfra/terraform/versions.tf
Comment on lines
+56
to
+60
| listen 443 ssl; | ||
| ssl_certificate /etc/letsencrypt/live/__SERVER_NAME__/fullchain.pem; | ||
| ssl_certificate_key /etc/letsencrypt/live/__SERVER_NAME__/privkey.pem; | ||
| include /etc/letsencrypt/options-ssl-nginx.conf; | ||
| ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem; |
There was a problem hiding this comment.
❓ Verification inconclusive
🏁 Script executed:
#!/bin/bash
# 인증서 발급/관리 관련 코드 검색
echo "=== Searching for certbot/letsencrypt setup ==="
rg -n -C3 'certbot|letsencrypt|acme' --type=sh
echo ""
echo "=== Searching for SSL certificate provisioning ==="
rg -n -C3 'ssl_certificate|fullchain\.pem|privkey\.pem' --type=shRepository: capstone-ssd/ssd-server
Length of output: 1949
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Confirm target template exists and show relevant lines ==="
ls -la deploy/ec2/nginx/site.https.conf.template 2>/dev/null || true
nl -ba deploy/ec2/nginx/site.https.conf.template | sed -n '40,90p' || true
echo
echo "=== Search for certbot/letsencrypt/acme setup (all relevant script types) ==="
rg -n --hidden --no-ignore \
-S 'certbot|letsencrypt|acme' \
deploy . --glob '!.git/*' --type-add 'sh:*.sh' --type sh 2>/dev/null || true
echo
echo "=== Search for certificate file references ==="
rg -n --hidden --no-ignore \
-S 'ssl_certificate|fullchain\.pem|privkey\.pem|ssl_dhparam' \
. --glob '!.git/*' 2>/dev/null || true
echo
echo "=== Locate install scripts mentioned in the review comment ==="
fd -a --hidden --no-ignore --type f 'install_caddy.sh|install_infra.sh' . 2>/dev/null || true
echo
echo "=== Show those scripts if found ==="
for f in $(fd -a --hidden --no-ignore --type f 'install_caddy.sh|install_infra.sh' . 2>/dev/null || true); do
echo "----- $f -----"
nl -ba "$f" | sed -n '1,220p'
echo
doneRepository: capstone-ssd/ssd-server
Length of output: 1968
🏁 Script executed:
#!/bin/bash
echo "=== Searching for certbot/letsencrypt/acme setup ==="
rg -n -C3 'certbot|letsencrypt|acme' . --glob '!.git/*'
echo ""
echo "=== Searching for SSL certificate provisioning ==="
rg -n -C3 'ssl_certificate|fullchain\.pem|privkey\.pem' . --glob '!.git/*'Repository: capstone-ssd/ssd-server
Length of output: 1968
HTTPS 템플릿 전제(인증서 사전 존재) 보장 여부를 확인하세요.
listen 443 ssl;
ssl_certificate /etc/letsencrypt/live/__SERVER_NAME__/fullchain.pem;
ssl_certificate_key /etc/letsencrypt/live/__SERVER_NAME__/privkey.pem;
include /etc/letsencrypt/options-ssl-nginx.conf;
ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem;
위 설정은 fullchain.pem/privkey.pem가 nginx 기동 전에 해당 경로에 존재해야 합니다. 배포 스크립트(또는 워크플로우)에서 인증서 발급/갱신 로직이 nginx 시작 이전에 수행되도록 되어 있는지 확인해주세요. 그렇지 않으면 nginx 검증(nginx -t)/기동이 실패할 수 있습니다.
인증서 프로비저닝 책임이 레포에 없다면 아래 중 하나로 구조를 정리해야 합니다:
- Caddy 사용 (권장):
install_caddy.sh로 자동 HTTPS를 위임하고 nginx는 내부 전용으로 사용 - Certbot 추가:
install_infra.sh또는 별도 스크립트에서 certbot 설치 + 발급/갱신 로직 추가
🤖 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 `@deploy/ec2/nginx/site.https.conf.template` around lines 56 - 60, The HTTPS
nginx template (site.https.conf.template) assumes
/etc/letsencrypt/live/__SERVER_NAME__/fullchain.pem and privkey.pem exist before
nginx starts; ensure the deployment workflow provisions/renews certificates
prior to running nginx -t or systemd start. Fix by adding certificate
provisioning to the startup flow — either delegate TLS to install_caddy.sh and
use nginx only internally, or add certbot install/obtain/renew steps into
install_infra.sh (or the CI/CD job) and perform cert existence checks and nginx
-t after provisioning; update the bootstrap/start script to wait for/validate
the files referenced by ssl_certificate and ssl_certificate_key before launching
nginx.
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
1 participant
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.
📣 Related Issue
📝 Summary
Summary by CodeRabbit
릴리스 노트
New Features
Chores