[Fix] prod db 마이그레이션을 위한 테라폼 동기화 작업 #187
Conversation
[Feat] 로그인 기능 메인 서버 배포
[Feat] 1차 스프린트 배포
[Feat] 1차 스프린트 추가 api 배포
[Feat] 2차 프로덕션 선행 배포
[Hotfix] 임시적으로 응원 최대 개수를 인 당 10,000개까지 허용
- dev 환경에서 먼저 적용됨
- dev 환경에서 적용됨
- dev에서 반영됨
- 복제 버킷 강제 삭제 옵션 추가 - cloudwatch log 그룹 관리 정책 추가 - ssm 구성 추가
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
.github/workflows/deploy-migration-test.yml.yml (1)
30-33: actions/setup-python은 v5로 올려 주세요 (actionlint 오류).현재 v4는 최신 러너와 호환성 이슈가 있습니다. actionlint 경고대로 v5로 업그레이드해야 합니다.
적용 diff:
- - name: Set up Python - uses: actions/setup-python@v4 + - name: Set up Python + uses: actions/setup-python@v5 with: python-version: '3.12'terraform-db-migration/main.tf (2)
142-162: 스냅샷 복원에allocated_storage = 20은 실패 위험 — 제거 권장.스냅샷 크기보다 작은 값이면 즉시 실패합니다. 미지정 시 스냅샷 원본 크기가 적용됩니다. 이전 코멘트와 동일 이슈입니다.
적용 diff:
storage_type = "gp3" - allocated_storage = 20 multi_az = false storage_encrypted = true
180-188: SSH 22 포트 0.0.0.0/0 개방 — 최소한 CIDR 제한 필요.임시라도 전역 개방은 고위험입니다. 고정 사무실/배스천 IP로 제한하거나 SSM Session Manager 전환을 권장합니다.
적용 예시:
resource "aws_security_group_rule" "allow_ssh_to_jump_host" { type = "ingress" from_port = 22 to_port = 22 protocol = "tcp" - cidr_blocks = ["0.0.0.0/0"] + cidr_blocks = [var.operator_office_cidr] # 예: "203.0.113.0/24" security_group_id = module.migration_sg.security_group_ids["jump-host"] description = "Allow SSH from anywhere for temporary access" }또는 본 규칙을 제거하고 Session Manager만 사용.
🧹 Nitpick comments (5)
.github/workflows/deploy-migration-test.yml.yml (2)
165-177: 태스크 정의 새 리비전 등록 플로우 적합하지만, YAML trailing space를 정리하세요.ECS가 이미지 override를 미지원하는 점을 고려해 새 리비전을 등록하는 접근은 정확합니다. 다만 YAMLlint가 지적한 trailing spaces(라인 169, 174)를 제거해 주세요.
적용 예시(공백 정리만 수행):
- + ... - +
101-113: TF outputs 키 가용성 확인 제안(방어적).
jq -r '.<key>.value'가null을 반환하면 빈/문자열 값이 주입되어 이후 단계 실패 가능성이 있습니다. 초기 실패를 빠르게 감지하려면 존재성/비null 검증을 추가해 주세요.예시:
TF_OUTPUTS='${{ needs.terraform-apply-migration.outputs.tf_outputs_json }}' - echo "ECR_REPOSITORY=$(echo "$TF_OUTPUTS" | jq -r '.ecr_repository_name.value')" >> $GITHUB_ENV + ECR_REPOSITORY=$(echo "$TF_OUTPUTS" | jq -r '.ecr_repository_name.value') + test "$ECR_REPOSITORY" != "null" -a -n "$ECR_REPOSITORY" + echo "ECR_REPOSITORY=$ECR_REPOSITORY" >> $GITHUB_ENV같은 패턴을 다른 키에도 반복 적용 권장.
terraform-db-migration/locals.tf (2)
99-132: 모든 egress 0.0.0.0/0 — 임시 환경이지만 향후 축소 권장.현 PR 범위상 유지해도 되나, 후속 PR에서 VPC 엔드포인트 경로로 국소화하거나 목적지 기반으로 줄이는 것을 고려해 주세요.
168-174: Jump Host AMI 하드코딩 — data 소스로 지역/최신성 보장 권장.
ami-012ea6058806ff688고정은 이미지 갱신/지역차 이슈를 유발할 수 있습니다.data "aws_ami"로 owner/이름 필터 기반 조회를 권장합니다.예시(별도 파일 적용):
data "aws_ami" "al2023" { most_recent = true owners = ["137112412989"] # Amazon filter { name = "name"; values = ["al2023-ami-*-x86_64"] } } # locals.jump_host.ami_id = data.aws_ami.al2023.idterraform-db-migration/main.tf (1)
190-198: SSM 파라미터 KMS CMK 사용 고려(선택).
type = "SecureString"기본 KMS는 aws/ssm입니다. 보안/감사 요건이 있다면 CMK를 생성하여key_id를 명시하세요.예시:
resource "aws_ssm_parameter" "temp_migration_rds_url" { name = "/prod/MIGRATION_RDS_URL" type = "SecureString" + key_id = aws_kms_key.migration_params.arn value = "jdbc:mysql://${aws_db_instance.cloned_rds_for_migration.address}/eatda?useUnicode=true&characterEncoding=UTF-8" }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.github/workflows/deploy-migration-test.yml.yml(1 hunks).github/workflows/destroy-migration-test.yml(1 hunks)terraform-db-migration/lambda_src/handler.py(1 hunks)terraform-db-migration/lambda_src/requirements.txt(1 hunks)terraform-db-migration/locals.tf(1 hunks)terraform-db-migration/main.tf(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- terraform-db-migration/lambda_src/requirements.txt
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/destroy-migration-test.yml
- terraform-db-migration/lambda_src/handler.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryResponse.java:29-32
Timestamp: 2025-08-17T18:38:26.970Z
Learning: lvalentine6 prefers to manage PR scope carefully and defer non-critical improvements to future refactoring when the current PR is already large in scope, showing good judgment in prioritizing deliverable completion over scope expansion.
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryRegisterRequest.java:11-17
Timestamp: 2025-08-17T18:37:21.953Z
Learning: lvalentine6 prefers to apply validation annotations consistently across all record classes in the project rather than implementing them piecemeal, prioritizing architectural consistency and maintainability.
🪛 actionlint (1.7.7)
.github/workflows/deploy-migration-test.yml.yml
31-31: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/deploy-migration-test.yml.yml
[error] 169-169: trailing spaces
(trailing-spaces)
[error] 174-174: trailing spaces
(trailing-spaces)
🪛 Checkov (3.2.334)
terraform-db-migration/main.tf
[HIGH] 94-129: Ensure AWS Lambda function is configured to validate code-signing
(CKV_AWS_272)
[MEDIUM] 142-162: Ensure that AWS database instances have deletion protection enabled
(CKV_AWS_293)
[HIGH] 142-162: Ensure RDS Performance Insights are encrypted using KMS CMKs
(CKV_AWS_354)
[MEDIUM] 164-178: Ensure that detailed monitoring is enabled for EC2 instances
(CKV_AWS_126)
[HIGH] 164-178: EC2 instance should not have public IP.
(CKV_AWS_88)
[HIGH] 164-178: Ensure Instance Metadata Service Version 1 is not enabled
(CKV_AWS_79)
[HIGH] 190-198: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
🔇 Additional comments (7)
.github/workflows/deploy-migration-test.yml.yml (3)
35-42: Lambda 패키징 경로 수정 LGTM.lambda_src 경로/파일을 올바르게 패키징하도록 조정한 부분이 이전 지적사항을 정확히 해소합니다.
157-164: ECS 태스크 정의 정보 조회 로직 적절합니다.서비스의 최신 태스크 정의 ARN/컨테이너명을 조회해 후속 단계에서 재사용하는 흐름이 깔끔합니다.
75-89: Terraform 출력 전달 로직 안정성 OK.GITHUB_OUTPUT 멀티라인 heredoc 패턴이 올바르게 사용되었습니다. 후속 잡에서
needs.terraform-apply-migration.outputs.tf_outputs_json로 소비되는 것도 확인했습니다.terraform-db-migration/locals.tf (3)
22-31: S3 List 권한 추가 LGTM — 목록 조회 AccessDenied 리스크 해소.버킷 ARN(“/*” 제외)에 대한
s3:ListBucket허용이 추가되어 람다의 프리픽스 나열/페이지네이션이 안전해졌습니다.
33-45: 오브젝트 작업 권한 범위 적절.prod 버킷/클론 버킷에 대한 Get/Put/Copy/Delete/Head가 최소 필요 권한으로 깔끔히 제한되어 있습니다.
88-97: SSM VPCE 통신용 인바운드 규칙 정의 적절.Interface VPC Endpoint SG에 443 self-reference를 추가하여 Lambda→SSM 트래픽이 보장됩니다.
terraform-db-migration/main.tf (1)
77-92: SSM Interface VPC Endpoint 구성 타당.프라이빗 서브넷에 ENI를 두고, SG로 제한하여 프라이빗 네트워크에서 SSM API에 접근하도록 한 구성이 목적에 부합합니다.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
terraform-db-migration/main.tf (4)
84-86: SSM Interface VPC Endpoint SG 인바운드 443 확인 필요엔드포인트에
lambda-migrationSG를 직접 부착하고 있습니다. Interface VPC Endpoint의 ENI는 클라이언트(SG)로부터 TCP/443 인바운드를 허용해야 합니다.module "migration_sg"에ingress_rules = local.ingress_rules가 연결된 점은 고무적이지만, 실제로 443 self-reference(또는 람다 SG → 엔드포인트 SG) 규칙이 있는지 locals.tf에서 확인이 필요합니다.안전책(선호): 엔드포인트 전용 SG를 분리해 443 인바운드를 람다 SG에서만 허용하도록 구성.
원한다면 해당 SG/룰 블록 예시 diff를 제공하겠습니다.
150-154: 스냅샷 복원 시 allocated_storage=20은 실패 위험 — 제거 권장스냅샷 복원에서는
allocated_storage가 스냅샷 크기 이상이어야 합니다. 고정 20GB는 스냅샷 크기에 따라 즉시 실패합니다. 미지정 시 스냅샷 크기를 사용하므로 삭제가 가장 안전합니다.storage_type = "gp3" - allocated_storage = 20 multi_az = false storage_encrypted = true또한 원본 스냅샷이 CMK로 암호화돼 있다면 동일 KMS 키를 지정해야 하니(
kms_key_id) 키 일치도 함께 확인해 주세요.
164-178: 점프호스트 퍼블릭 IP + SSH 0.0.0.0/0 개방은 고위험 — 최소 IP 제한 또는 SSM 전환 권장현재 퍼블릭 서브넷에 퍼블릭 IP를 부여하고, SG에서 SSH를 전 세계에 개방하고 있습니다. 임시라도 알려진 사무실/운영망 CIDR로 제한하거나 SSM Session Manager로 대체하는 것이 안전합니다.
IP 제한 예시:
resource "aws_security_group_rule" "allow_ssh_to_jump_host" { type = "ingress" from_port = 22 to_port = 22 protocol = "tcp" - cidr_blocks = ["0.0.0.0/0"] + cidr_blocks = [var.operator_office_cidr] # 예: "203.0.113.0/24" security_group_id = module.migration_sg.security_group_ids["jump-host"] description = "Allow SSH from anywhere for temporary access" }또는 SSM으로 전환하면 본 규칙과 퍼블릭 IP 자체가 불필요합니다.
100-101: Lambda 번들 경로 불일치(상대경로 기준) — Terraform에서 zip을 찾지 못함Terraform은 경로를 “모듈 디렉터리 기준”으로 해석합니다. 워크플로우가 아티팩트를 리포지토리 루트의
./build/에 내려놓고, Terraform은./terraform-db-migration에서 실행되므로 현재 설정(./build/...)은terraform-db-migration/build/...를 가리켜 파일을 찾지 못합니다.두 가지 중 택1:
- 권장: Terraform 쪽 경로를
../build/...로 수정- 대안: 워크플로우의 다운로드 경로를
./terraform-db-migration/build로 변경Terraform 수정안:
- filename = "./build/migration_lambda.zip" - source_code_hash = filebase64sha256("./build/migration_lambda.zip") + filename = "../build/migration_lambda.zip" + source_code_hash = filebase64sha256("../build/migration_lambda.zip").github/workflows/deploy-migration-test.yml.yml (1)
30-33: actions/setup-python 최신 버전으로 업데이트actionlint 지적대로
actions/setup-python@v4는 최신 러너 호환성이 떨어집니다.@v5로 올려 주세요.- - name: Set up Python - uses: actions/setup-python@v4 + - name: Set up Python + uses: actions/setup-python@v5 with: python-version: '3.12'
🧹 Nitpick comments (7)
terraform-db-migration/main.tf (3)
155-157: 삭제 보호 비활성화(Deletion protection) — 임시 리소스라도 오조작 방지 차원에서 켜는 것을 고려체크오프가 경고하는 항목입니다. 에페메럴 스택이라 의도적일 수 있으나, 운영 중 실수 삭제 리스크가 크다면
deletion_protection = true를 고려해 주세요.
190-198: SSM SecureString KMS CMK 지정 누락 — 계정 기본키 대신 CMK 명시 권장
type = "SecureString"만 지정되어 있어 계정 기본 KMS 키에 의존합니다. 보안/감사 요건이 있다면 CMK를 명시하세요(예: bootstrap에서 관리하는 KMS).예시:
resource "aws_ssm_parameter" "temp_migration_rds_url" { name = "/prod/MIGRATION_RDS_URL" type = "SecureString" + key_id = data.terraform_remote_state.bootstrap_infra.outputs.ssm_kms_key_id value = "jdbc:mysql://${aws_db_instance.cloned_rds_for_migration.address}/eatda?useUnicode=true&characterEncoding=UTF-8"동일하게
temp_migration_s3_bucket이 민감값이라면 SecureString + KMS 적용을 고려해 주세요.
94-129: 코드사이닝 미구성(선택) + 로그 보존/태깅 등 전반은 양호
- Lambda: 코드사이닝은 현재 미구성(Checkov CKV_AWS_272). 에페메럴 목적이면 생략 가능하나, 장기 사용 시 코드사이닝 검증을 고려해 주세요.
- 그 외 VPC 구성, 환경변수, 의존 VPC Endpoint 명시, 로그그룹 별도 생성/1일 보존은 목적에 부합합니다.
.github/workflows/deploy-migration-test.yml.yml (4)
101-113: TF Outputs 파싱 및 환경 변수 주입 로직 적절 — null 가드만 추후 검토
jq -r로 각 value를 추출해$GITHUB_ENV에 기록하는 접근은 깔끔합니다. 다만 출력 키가 비어 있을 때 빈 문자열이 들어갈 수 있으니, 추후 안정화를 위해 필수 키에 대한 null 체크를 고려해 주세요(선택).
178-181: 사전/사후 Lambda 호출 단계 적절 — LGTM전처리/후처리 람다를 명시적으로 호출하는 구조 명확합니다. 재시도 전략이 필요하면
--qualifier, DLQ/알람 등을 후속 PR에서 고려해도 좋겠습니다.Also applies to: 198-200
169-169: 사소: 공백(lint) 정리YAMLlint가 두 줄의 트레일링 스페이스를 지적합니다. 제거하면 린트 잡음이 줄어듭니다.
Also applies to: 174-174
61-65: 대안 제안: Terraform 경로 문제를 워크플로우에서 해결하는 옵션Terraform 파일을 건드리지 않으려면, 람다 아티팩트를
./terraform-db-migration/build로 다운로드하도록 바꾸는 것도 방법입니다.- - name: Download Lambda artifact + - name: Download Lambda artifact uses: actions/download-artifact@v5 with: name: lambda-build - path: ./build + path: ./terraform-db-migration/build이 경우, Terraform의
filename/source_code_hash를 현재 값(./build/...) 그대로 유지할 수 있습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/deploy-migration-test.yml.yml(1 hunks)terraform-db-migration/main.tf(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryResponse.java:29-32
Timestamp: 2025-08-17T18:38:26.970Z
Learning: lvalentine6 prefers to manage PR scope carefully and defer non-critical improvements to future refactoring when the current PR is already large in scope, showing good judgment in prioritizing deliverable completion over scope expansion.
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryRegisterRequest.java:11-17
Timestamp: 2025-08-17T18:37:21.953Z
Learning: lvalentine6 prefers to apply validation annotations consistently across all record classes in the project rather than implementing them piecemeal, prioritizing architectural consistency and maintainability.
🪛 actionlint (1.7.7)
.github/workflows/deploy-migration-test.yml.yml
31-31: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/deploy-migration-test.yml.yml
[error] 169-169: trailing spaces
(trailing-spaces)
[error] 174-174: trailing spaces
(trailing-spaces)
🪛 Checkov (3.2.334)
terraform-db-migration/main.tf
[HIGH] 94-129: Ensure AWS Lambda function is configured to validate code-signing
(CKV_AWS_272)
[MEDIUM] 142-162: Ensure that AWS database instances have deletion protection enabled
(CKV_AWS_293)
[HIGH] 142-162: Ensure RDS Performance Insights are encrypted using KMS CMKs
(CKV_AWS_354)
[MEDIUM] 164-178: Ensure that detailed monitoring is enabled for EC2 instances
(CKV_AWS_126)
[HIGH] 164-178: EC2 instance should not have public IP.
(CKV_AWS_88)
[HIGH] 164-178: Ensure Instance Metadata Service Version 1 is not enabled
(CKV_AWS_79)
[HIGH] 190-198: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
🔇 Additional comments (3)
.github/workflows/deploy-migration-test.yml.yml (3)
35-42: Lambda 패키징 스텝은 경로 정정 완료 — LGTM
terraform-db-migration/lambda_src기준으로 의존성 설치 및handler.py를 포함해 zip 작성하는 부분은 적절합니다.
60-66: Terraform 아티팩트 전달/출력 처리 흐름 양호
- Lambda zip 아티팩트를 업로드 후
./build로 다운로드하는 흐름,terraform init/apply실행,terraform output -json을 GITHUB_OUTPUT 멀티라인으로 전달하는 방식 모두 적절합니다.Also applies to: 75-82, 83-90
165-177: ECS 태스크 정의 리비전 생성 플로우 적절 — LGTM기존 태스크 정의 JSON을 정제하고 이미지 교체 후 새 리비전을 등록하는 방식은 ECS 제약사항을 올바르게 처리합니다. 좋은 개선입니다.
|
/noti 내일 저녁까지 작업이 완료되야 하기에 충안님 바쁘시면 리뷰 없이 머지할까요? |
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
해당 PR 확인을 미처 못했네요!
Approve 하고 천천히 확인하겠습니다!
# Conflicts: # src/test/java/eatda/service/cheer/CheerServiceTest.java # terraform/common/locals.tf # terraform/dev/outputs.tf # terraform/dev/s3/outputs.tf # terraform/prod/outputs.tf # terraform/prod/s3/outputs.tf
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
src/main/java/eatda/service/cheer/CheerService.java (2)
38-38: 상수값을 환경/프로필별 설정으로 분리해 되돌림 리스크를 줄여주세요하드코딩된 상수를 PR마다 바꾸는 방식은 회귀 위험과 릴리즈 피로도를 높입니다. 이미 migration 전용 프로필(
application-migration.yml)이 있는 만큼:
- 기본/운영 프로필: 낮은 기본값(예: 3)
- migration 프로필: 10_000
처럼 설정에서 주입받도록 하는 편이 안전합니다. 현재 변경은 유지하되, 다음 작업으로 전환하시길 제안드립니다. 이전 코멘트의 “초기 데이터 작업 완료 후 변경” 계획과도 일치합니다.원하시면 후속 이슈를 생성해드리거나, 설정 반영 PR 스캐폴딩을 만들어드릴게요.
예시 구현:
// 설정 주입 @Value("${cheer.max-size-per-member:3}") private int maxCheerSize; // 사용처 if (cheerRepository.countByMember(member) >= maxCheerSize) { throw new BusinessException(BusinessErrorCode.FULL_CHEER_SIZE_PER_MEMBER); }예시 설정(application-migration.yml):
cheer: max-size-per-member: 10000검증 스크립트:
#!/bin/bash # 설정 키와 하드코딩 상수 사용 여부를 점검 rg -nC2 'max-size-per-member|MAX_CHEER_SIZE' src/main/java src/main/resourcesAlso applies to: 72-75
57-58: 상점 조회/저장 동시성 TODO — 일정 상 후속 처리 재확인
findByKakaoId(...).orElseGet(save(...))는 경합 시 중복 저장 가능성이 있어 TODO가 남아있는 상태입니다. 본 PR 범위 밖인 점 이해합니다. 마이그레이션 완료 시점에 처리 계획(락 전략/DB 제약/UPSERT 등)만 이슈로 트래킹 부탁드립니다.
🧹 Nitpick comments (8)
.github/workflows/deploy-migration-test.yml (5)
169-169: 불필요한 트레일링 스페이스 제거.YAMLlint 에러가 납니다. 공백만 있는 라인의 후행 공백을 제거하세요.
- + @@ - +Also applies to: 174-174
6-10: 기본 브랜치 입력값 확인 요청: 'dev' vs 'develop'.레포 브랜치 전략이
develop중심이면 기본값을develop으로 맞추는 게 안전합니다. 현재 설정이 의도 맞는지 확인 부탁드립니다. 필요 시 아래처럼 변경하세요.- default: 'dev' + default: 'develop'
114-120: AWS 자격 증명 방식 개선 제안: OIDC 기반 단기 크레덴셜로 전환.현재 고정형 Access Key를 사용 중입니다. 보안/회전 부담을 줄이기 위해 GitHub OIDC +
role-to-assume로 전환을 권장합니다. 이 경우 워크플로우 permissions에id-token: write가 필요합니다.예시:
permissions: contents: read id-token: write # ... - uses: aws-actions/configure-aws-credentials@v4 with: role-to-assume: arn:aws:iam::<ACCOUNT_ID>:role/<GHA_OIDC_ROLE> aws-region: ${{ env.AWS_REGION }}
101-113: 단일 서브넷 출력 사용 → 가용성/스케줄링 안정성 저하 가능.
awsvpcConfiguration에 서브넷을 2개 이상(AZ 분산) 제공하는 것이 모범 사례입니다. Terraformoutputs에서 배열(private_subnet_ids)을 노출하고, run-task에 복수 서브넷을 넘기도록 개선을 고려해 주세요.
147-156: Docker 빌드 성능 개선(선택): 캐시와 빌드 컨텍스트 최적화.
.dockerignore정리로 컨텍스트 축소docker/build-push-action+ 캐시 사용- - name: Build, tag, and push image to Amazon ECR - id: build-image - env: - ECR_REGISTRY: ${{ steps.login-ecr.outputs.registry }} - run: | - IMAGE_TAG="migration-test-$(date +'%Y%m%d-%H%M%S')" - docker build -t $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG . - docker push $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG - echo "image_uri=$ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG" >> $GITHUB_OUTPUT + - name: Build, tag, and push image to Amazon ECR + id: build-image + uses: docker/build-push-action@v6 + with: + context: . + push: true + tags: ${{ steps.login-ecr.outputs.registry }}/${{ env.ECR_REPOSITORY }}:migration-test-${{ github.run_id }} + cache-from: type=gha + cache-to: type=gha,mode=maxsrc/main/java/eatda/service/cheer/CheerService.java (3)
72-79: DB 레벨 유니크 제약 및 예외 핸들링 추가 제안현재
cheer테이블에는(member_id, store_kakao_id)에 대한 유니크 제약이 없어, 동시성 상황에서 중복 저장이 발생할 수 있습니다. 아래와 같이 DDL 마이그레이션과 서비스 레이어 예외 핸들링을 추가하면, 애플리케이션 일관성을 크게 높일 수 있습니다.
src/main/resources/db/migration/V8__add_unique_constraint_cheer_member_store.sql (신규 마이그레이션 파일)
ALTER TABLE `cheer` ADD CONSTRAINT `uk_cheer_member_store` UNIQUE (`member_id`, `store_kakao_id`);src/main/java/eatda/service/cheer/CheerService.java
public Cheer registerCheer(Member member, String storeKakaoId) { validateRegisterCheer(member, storeKakaoId); Cheer cheer = Cheer.create(member, storeKakaoId); - return cheerRepository.save(cheer); + try { + return cheerRepository.save(cheer); + } catch (DataIntegrityViolationException e) { + // DB 유니크 제약 위반 시, 이미 응원된 상태로 간주 + throw new BusinessException(BusinessErrorCode.ALREADY_CHEERED, e); + } }기존
validateRegisterCheer로직은 그대로 유지하여, 일반적인 경우에는 선검증을 수행하고
동시성 충돌이 발생한 경우에는 위 예외 핸들링으로 안전망을 갖추도록 합니다.
91-95: moveTempFilesToPermanent 메서드의 입력 순서 보장 검증됨
- FileClient.moveTempFilesToPermanent 구현부를 살펴보면,
• tempImageKeys.stream() → CompletableFuture 리스트 생성 → futures.stream() → join() → toList() 순으로 호출되며,
• Java Stream API의 ‘encounter order(순회 순서)’가 그대로 유지됩니다.- 실제로 FileClientTest에서도 결과 리스트의 인덱스별 키 일치를 검증하고 있어(예:
assertThat(result.get(0)).isEqualTo("cheer/123/temp1.jpg")) 순서 보장이 테스트로 보장되어 있습니다.따라서 현재 구현만으로도
tempKeys.get(i)와permanentKeys.get(i)의 1:1 순서 매핑이 안전하게 유지됩니다.• (선택사항) 코드 가독성과 계약 명시를 위해 moveTempFilesToPermanent 메서드 Javadoc에 “반환 리스트는 입력 리스트의 순서를 유지한다”는 설명을 추가하면 좋겠습니다.
113-114: saveCheerImages() 내 중복 save 호출 제거 검토
Cheer.images연관관계에cascade = CascadeType.ALL(=PERSIST+MERGE등 모든 전이)가 적용되어 있어, 이미 관리되는Cheer엔티티에 새CheerImage를 추가하면 트랜잭션 커밋 시점에 JPA의 Dirty Checking에 의해 자동으로 저장됩니다.
따라서saveCheerImages()메서드 마지막줄의cheerRepository.save(cheer);호출은 불필요한 DB 라운드트립을 야기하므로 제거를 검토해주세요.
대상 위치
- 파일: src/main/java/eatda/service/cheer/CheerService.java
- 메서드:
private void saveCheerImages(...)끝부분,cheerRepository.save(cheer);호출검증
Cheer엔티티(src/main/java/eatda/domain/cheer/Cheer.java)에 아래 설정 확인 완료@OneToMany(mappedBy = "cheer", cascade = CascadeType.ALL, orphanRemoval = true) private Set<CheerImage> images = new HashSet<>();registerCheer()메서드에서 최초save후 동일 트랜잭션 내 상태 유지 및 Dirty Checking 동작 보장제거 후에는 연관 이미지 저장 로직이 정상 동작하는지 통합 테스트를 통해 검증하시기 바랍니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/deploy-migration-test.yml(1 hunks)src/main/java/eatda/service/cheer/CheerService.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryResponse.java:29-32
Timestamp: 2025-08-17T18:38:26.970Z
Learning: lvalentine6 prefers to manage PR scope carefully and defer non-critical improvements to future refactoring when the current PR is already large in scope, showing good judgment in prioritizing deliverable completion over scope expansion.
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryRegisterRequest.java:11-17
Timestamp: 2025-08-17T18:37:21.953Z
Learning: lvalentine6 prefers to apply validation annotations consistently across all record classes in the project rather than implementing them piecemeal, prioritizing architectural consistency and maintainability.
🪛 actionlint (1.7.7)
.github/workflows/deploy-migration-test.yml
31-31: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/deploy-migration-test.yml
[error] 169-169: trailing spaces
(trailing-spaces)
[error] 174-174: trailing spaces
(trailing-spaces)
⏰ 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). (1)
- GitHub Check: test
🔇 Additional comments (1)
src/main/java/eatda/service/cheer/CheerService.java (1)
38-38: 마이그레이션 대응을 위한 상한 상향(10_000) — 현재 목적에는 적합해 보입니다이번 PR의 목표(데이터 마이그레이션/이관) 관점에서 per-member 제한을 일시적으로 완화하는 변경은 합리적으로 보입니다. 영향 범위도
validateRegisterCheer의 비교 한 군데로 제한됩니다.
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
.github/workflows/deploy-migration-test.yml (4)
30-33: actions/setup-python 버전 v5로 업데이트 필요actionlint 경고가 그대로이며 최신 러너와의 비호환 이슈가 보고됩니다. v5로 올려주세요.
적용 diff:
- - name: Set up Python - uses: actions/setup-python@v4 + - name: Set up Python + uses: actions/setup-python@v5 with: python-version: '3.12'
130-135: SSM 시크릿 마스킹 및 범위 축소 필요SSM에서 읽은 값은 기본 마스킹 대상이 아닙니다. 즉시 마스킹하고, 가능하면 GITHUB_ENV에 퍼시스트하지 말고 다음 단일 스텝의 env로만 전달하세요.
최소 변경(diff) — 마스킹 추가:
- name: Get TEST_JWT_SECRET_KEY from SSM id: get-test-secret run: | SECRET_VALUE=$(aws ssm get-parameter --name "/common/TEST_JWT_SECRET_KEY" --with-decryption --query "Parameter.Value" --output text) + echo "::add-mask::$SECRET_VALUE" echo "TEST_JWT_SECRET_KEY=$SECRET_VALUE" >> $GITHUB_ENV권장(범위 축소) — GITHUB_ENV 사용 제거 + 다음 스텝에서 outputs로 참조:
- name: Get TEST_JWT_SECRET_KEY from SSM id: get-test-secret run: | SECRET_VALUE=$(aws ssm get-parameter --name "/common/TEST_JWT_SECRET_KEY" --with-decryption --query "Parameter.Value" --output text) echo "::add-mask::$SECRET_VALUE" - echo "TEST_JWT_SECRET_KEY=$SECRET_VALUE" >> $GITHUB_ENV + echo "secret=$SECRET_VALUE" >> $GITHUB_OUTPUT @@ - name: Build with Gradle run: | chmod +x gradlew ./gradlew clean build -Dspring.profiles.active=migration env: - TEST_JWT_SECRET_KEY: ${{ env.TEST_JWT_SECRET_KEY }} + TEST_JWT_SECRET_KEY: ${{ steps.get-test-secret.outputs.secret }}
204-207: ECS 태스크 성공/실패 판정 스텝 누락wait 만으로는 성공 여부를 알 수 없습니다. describe-tasks로 exitCode/ stoppedReason 을 확인해 실패시 워크플로우를 실패로 처리하세요.
추가 diff:
- name: Wait for ECS Task to Complete run: | aws ecs wait tasks-stopped --cluster ${{ env.PROD_ECS_CLUSTER_NAME }} --tasks ${{ steps.ecs-run-task.outputs.task_arn }} + + - name: Verify ECS Task Succeeded + run: | + DESC=$(aws ecs describe-tasks --cluster "${{ env.PROD_ECS_CLUSTER_NAME }}" --tasks "${{ steps.ecs-run-task.outputs.task_arn }}") + echo "$DESC" | jq . + EXIT_CODE=$(echo "$DESC" | jq -r '.tasks[0].containers[0].exitCode // empty') + STOPPED_REASON=$(echo "$DESC" | jq -r '.tasks[0].stoppedReason // empty') + if [ -z "$EXIT_CODE" ] || [ "$EXIT_CODE" != "0" ]; then + echo "Task failed. exitCode=${EXIT_CODE}, stoppedReason=${STOPPED_REASON}"; exit 1; + fi
208-210: Post-processing Lambda도 --cli-binary-format 및 에러 검증 필요Pre-processing과 동일하게 보강되지 않아 실패/오동작 가능성이 있습니다. 동일 패턴으로 수정하세요.
적용 diff:
- name: Invoke Post-processing Lambda run: | - aws lambda invoke --function-name ${{ env.LAMBDA_FUNCTION_NAME }} --payload '{"task": "postprocess"}' response.json + META=$(aws lambda invoke \ + --function-name "${{ env.LAMBDA_FUNCTION_NAME }}" \ + --payload '{"task":"postprocess"}' \ + --cli-binary-format raw-in-base64-out \ + response.json) + echo "$META" | jq . + echo "$META" | jq -e '.FunctionError == null' >/dev/null || { echo "Postprocess Lambda failed"; exit 1; }
🧹 Nitpick comments (7)
src/test/java/eatda/service/cheer/CheerServiceTest.java (1)
41-43: 일시 비활성화는 OK. 재활성화 보증(TO-DO/이슈/기한)도 같이 남겨주세요운영 한시 정책(MAX_CHEER_SIZE=10000) 때문에 테스트를 비활성화한 판단은 합리적입니다. 다만 커버리지 공백이 길어지지 않도록 다음을 권장합니다.
- 테스트 위에 명시적 TODO + 목표 날짜 + 추적 이슈 링크 추가
- 가능하면 환경변수/프로퍼티 기반의 조건부 스킵으로 전환해 코드 변경 없이 CI에서만 Skip (향후 별도 PR에서도 OK)
제안 diff(사소 변경, 추적성 강화):
- @Disabled("현재 한시적으로 MAX_CHEER_SIZE=10000 으로 운영 중") + // TODO(ops-override): 운영 한시 정책 종료 시 테스트 재활성화. target: 2025-08-31, tracking: #182 + @Disabled("OPS 임시: MAX_CHEER_SIZE=10000 적용 중 — 종료 시 복구")참고: 본 PR 스코프가 인프라 동기화인 점 감안하여 테스트 로직 리팩토링(예: 설정 주입형 한도값으로 전환)은 추후 작은 PR로 분리하는 것을 제안드립니다.
원하시면 제가 추적 이슈를 생성하고(제목/본문 초안 포함) 해당 날짜에 리마인드하도록 도와드릴게요. 또한 CI에서만 Skip되도록 @disabled 대신 Assumption/조건부 어노테이션으로 바꾸는 간단 패치도 제공 가능합니다.
.github/workflows/deploy-migration-test.yml (6)
6-9: workflow 입력 기본 브랜치 재확인 ('dev' vs 'develop')리포지토리의 기본 통합 브랜치가 'develop'로 보입니다. 사용자가 입력을 생략할 때 오작동을 방지하려면 default 값을 실제 운영 관례와 일치시키는지 확인해 주세요. 필요 시 'develop'로 변경 권장합니다.
적용 diff(필요 시):
- default: 'dev' + default: 'develop'
101-113: Terraform outputs 파싱 후 필수 값 검증 추가jq 파싱 결과가 null/빈 문자열이면 이후 단계에서 난해한 실패가 발생합니다. 필수 키들(ECR_REPOSITORY 등)에 대해 빈 값 검증을 추가해 조기 실패하도록 하세요. 또한 set -euo pipefail 사용을 고려해 에러 전파를 강화하세요.
적용 diff:
- name: Parse Terraform Outputs and Set Env id: parse-tf run: | + set -euo pipefail TF_OUTPUTS='${{ needs.terraform-apply-migration.outputs.tf_outputs_json }}' echo "ECR_REPOSITORY=$(echo "$TF_OUTPUTS" | jq -r '.ecr_repository_name.value')" >> $GITHUB_ENV echo "PROD_ECS_CLUSTER_NAME=$(echo "$TF_OUTPUTS" | jq -r '.prod_ecs_cluster_name.value')" >> $GITHUB_ENV echo "PROD_ECS_SERVICE_NAME=$(echo "$TF_OUTPUTS" | jq -r '.prod_ecs_api_service_name.value')" >> $GITHUB_ENV echo "LAMBDA_FUNCTION_NAME=$(echo "$TF_OUTPUTS" | jq -r '.migration_lambda_function_name.value')" >> $GITHUB_ENV echo "VPC_SUBNET=$(echo "$TF_OUTPUTS" | jq -r '.private_subnet_id.value')" >> $GITHUB_ENV echo "TASK_SECURITY_GROUP=$(echo "$TF_OUTPUTS" | jq -r '.task_security_group_id.value')" >> $GITHUB_ENV echo "CLONED_RDS_ADDRESS=$(echo "$TF_OUTPUTS" | jq -r '.cloned_rds_address.value')" >> $GITHUB_ENV echo "CLONED_S3_BUCKET_NAME=$(echo "$TF_OUTPUTS" | jq -r '.cloned_s3_bucket_name.value')" >> $GITHUB_ENV + # 필수 값 검증 + missing=0 + for k in ECR_REPOSITORY PROD_ECS_CLUSTER_NAME PROD_ECS_SERVICE_NAME LAMBDA_FUNCTION_NAME VPC_SUBNET TASK_SECURITY_GROUP CLONED_RDS_ADDRESS CLONED_S3_BUCKET_NAME; do + v=$(grep "^$k=" "$GITHUB_ENV" | tail -1 | cut -d= -f2-) + if [ -z "${v:-}" ] || [ "$v" = "null" ]; then + echo "Missing TF output: $k"; missing=1 + fi + done + [ "$missing" -eq 0 ] || exit 1검증 스크립트가 필요하시면 생성해 드립니다.
157-164: TaskDefinition/ContainerName 확보 실패 시 즉시 실패 처리 추가 권장AWS API에서 빈 값이 돌아오면 이후 단계가 모호하게 실패/대기합니다. 값 검증을 추가해 주세요.
적용 diff:
- name: Get Latest PROD Task Definition Info id: get-prod-info run: | TASK_DEF_ARN=$(aws ecs describe-services --cluster "${{ env.PROD_ECS_CLUSTER_NAME }}" --services "${{ env.PROD_ECS_SERVICE_NAME }}" --query "services[0].taskDefinition" --output text) CONTAINER_NAME=$(aws ecs describe-task-definition --task-definition "$TASK_DEF_ARN" --query "taskDefinition.containerDefinitions[0].name" --output text) echo "task_definition_arn=$TASK_DEF_ARN" >> $GITHUB_OUTPUT echo "container_name=$CONTAINER_NAME" >> $GITHUB_OUTPUT + if [ -z "$TASK_DEF_ARN" ] || [ "$TASK_DEF_ARN" = "None" ] || [ -z "$CONTAINER_NAME" ] || [ "$CONTAINER_NAME" = "None" ]; then + echo "Failed to fetch task definition/container name"; exit 1; + fi
165-176: jq 파이프라인의 인용 개선 및 trailing spaces 정리echo "$TASK_DEF" 사용으로 word-splitting 이슈는 방지되었지만, YAMLlint가 지적한 공백(trailing spaces)이 라인 169, 174에 남아 있습니다. 또한 here-string 사용이 조금 더 견고합니다.
적용 diff(공백 제거):
- + @@ - +적용 diff(선호안: here-string):
- NEW_TASK_DEF=$(echo "$TASK_DEF" | jq --arg IMAGE "${{ steps.build-image.outputs.image_uri }}" ' + NEW_TASK_DEF=$(jq --arg IMAGE "${{ steps.build-image.outputs.image_uri }}" ' .containerDefinitions[0].image = $IMAGE | del(.taskDefinitionArn, .revision, .status, .requiresAttributes, .placementConstraints, .compatibilities, .registeredAt, .registeredBy) - ') + ' <<< "$TASK_DEF")
14-16: 동시성 그룹 키에 브랜치 포함 고려(선택)여러 브랜치에서 병행 테스트를 돌릴 경우 동일 그룹으로 서로 블로킹될 수 있습니다. 그룹을 branch 입력값과 결합하는 방안을 고려해 주세요.
예시:
concurrency: - group: terraform-migration + group: terraform-migration-${{ github.event.inputs.branch }} cancel-in-progress: false
136-142: Gradle 빌드 시 시크릿 노출 면역성 확보(선택)위 스텝 변경으로 TEST_JWT_SECRET_KEY를 outputs로 전달한다면, Gradle 로그에 비의도적 출력이 없는지 확인하세요. 필요 시 org.gradle.logging.level=quiet 등을 적용해 로그 노출 표면을 줄일 수 있습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/deploy-migration-test.yml(1 hunks)src/test/java/eatda/service/cheer/CheerServiceTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryResponse.java:29-32
Timestamp: 2025-08-17T18:38:26.970Z
Learning: lvalentine6 prefers to manage PR scope carefully and defer non-critical improvements to future refactoring when the current PR is already large in scope, showing good judgment in prioritizing deliverable completion over scope expansion.
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryRegisterRequest.java:11-17
Timestamp: 2025-08-17T18:37:21.953Z
Learning: lvalentine6 prefers to apply validation annotations consistently across all record classes in the project rather than implementing them piecemeal, prioritizing architectural consistency and maintainability.
🪛 actionlint (1.7.7)
.github/workflows/deploy-migration-test.yml
31-31: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/deploy-migration-test.yml
[error] 169-169: trailing spaces
(trailing-spaces)
[error] 174-174: trailing spaces
(trailing-spaces)
⏰ 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). (1)
- GitHub Check: test
🔇 Additional comments (3)
src/test/java/eatda/service/cheer/CheerServiceTest.java (1)
28-28: @disabled 임포트 추가 적절일시 비활성화 목적이 명확하고, JUnit 5 표준 어노테이션 사용이 일관적입니다.
.github/workflows/deploy-migration-test.yml (2)
178-186: Pre-processing Lambda 호출 보강 적용 완료AWS CLI v2용 --cli-binary-format과 메타데이터 검증이 잘 추가되었습니다. 이 단계는 신뢰도 있게 동작할 것으로 보입니다.
188-203: ECS run-task 실패 시 fail-fast 처리 추가 확인taskArn 비어 있음 검사 로직이 추가되어 모호한 대기/실패를 방지합니다. 좋습니다.
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
terraform-db-migration/main.tf (4)
180-188: SSH 22번 포트 전역 개방(0.0.0.0/0) — 임시라도 IP 제한 또는 SSM 대체 권장점프호스트 접근을 위해 열어둔 것으로 보이지만, 운영·보안 관점에서 고위험입니다. 알려진 소스 CIDR로 제한하거나 SSM Session Manager로 대체하는 편이 안전합니다. 기한이 촉박하면 최소한 특정 CIDR로 제한하는 방향 추천합니다.
예시:
- cidr_blocks = ["0.0.0.0/0"] + cidr_blocks = [var.operator_office_cidr] # 예: "203.0.113.0/24"
67-71: data.aws_region.current 미정의로 plan/apply 실패 — region 데이터 소스 추가 필요
service_name = "com.amazonaws.${data.aws_region.current.id}.s3"참조가 있으나data "aws_region" "current"가 이 파일에 없습니다. 동일 문제는 79행(.ssm)에도 발생합니다. 파일 상단에 지역 데이터 소스를 추가해 주세요.적용 예:
+data "aws_region" "current" {}
77-92: SSM Interface VPC Endpoint SG 인바운드(443) 누락 — Lambda→SSM 통신 차단 위험
aws_vpc_endpoint.ssm_interface에module.migration_sg.security_group_ids["lambda-migration"]를 부착했지만, 이 SG(또는 엔드포인트 전용 SG)에 443 인바운드 허용 규칙이 없습니다. Interface ENI는 클라이언트(Lambda ENI)로부터의 443 인바운드를 허용해야 합니다.최소 변경(이 파일에서 규칙 추가):
+resource "aws_security_group_rule" "allow_https_to_ssm_vpce_from_lambda" { + type = "ingress" + from_port = 443 + to_port = 443 + protocol = "tcp" + security_group_id = module.migration_sg.security_group_ids["lambda-migration"] + source_security_group_id = module.migration_sg.security_group_ids["lambda-migration"] + description = "Allow HTTPS from Lambda ENI to SSM Interface VPC Endpoint ENI" +}또는 보안그룹 모듈에 self-reference 규칙을 추가(선호 시 locals → cross_reference_rules로).
142-152: 스냅샷 복원 시 allocated_storage=20은 실패 위험 — 제거 권장RDS 스냅샷 복원에서는
allocated_storage가 스냅샷 크기보다 작으면 실패합니다. 지정하지 않으면 스냅샷의 원본 크기를 사용하므로 안전합니다.storage_type = "gp3" - allocated_storage = 20 multi_az = false storage_encrypted = true
🧹 Nitpick comments (5)
terraform/prod/s3/main.tf (1)
92-100: Managed Cache Policy ID 하드코딩 — 의도 확인 및 파라미터화 고려
cache_policy_id = "658327ea-f89d-4fab-a63d-7e88639e58f6"는 AWS Managed CachingOptimized 정책입니다. 정적 S3 오리진이면 적합하지만, 쿼리스트링/쿠키/헤더에 의존하는 경우 캐시 키에 반영되지 않아 동작이 바뀔 수 있습니다. 의도대로 “완전 캐시 최적화”가 맞는지 한 번만 확인해 주세요. 필요 시 변동성을 줄이기 위해 policy ID를 변수/locals로 추출하는 것도 좋습니다.
또한 CloudFront에서 OPTIONS를 허용하고 있으나 S3 CORS는 ["GET"]만 허용 중입니다(41-47): 프리플라이트 요청이 필요 없다면 괜찮지만, 필요하다면 S3 CORS에 OPTIONS 추가가 필요합니다.예시(선호 시 최소 변경):
- cache_policy_id = "658327ea-f89d-4fab-a63d-7e88639e58f6" + # AWS Managed CachingOptimized + cache_policy_id = "658327ea-f89d-4fab-a63d-7e88639e58f6"또는 변수화:
- cache_policy_id = "658327ea-f89d-4fab-a63d-7e88639e58f6" + cache_policy_id = var.cloudfront_cache_policy_idterraform/common/security-group/variables.tf (1)
24-25: cidr_blocks optional 전환에 따른 모듈 내부 null 처리·검증 필요
cidr_blocks = optional(list(string))로 완화된 것은 SG↔SG 규칙(cross_reference_rules) 활용을 쉽게 해줍니다. 다만 모듈 내부에서cidr_blocks가 null/빈 리스트일 때aws_security_group_rule에 잘못 전달되면 프로바이더 에러가 납니다(ingress는 cidr_blocks 또는 source_security_group_id 중 하나는 필수). 모듈 구현부에서 다음을 보장해 주세요.
- null인 경우 속성 자체를 생략하거나 대안 경로(예: source_security_group_id)를 사용
- 규칙 객체에 대한 검증: cidr_blocks가 없으면 cross_reference_rules로 동일 포트가 정의되어야 함
가능한 보완(모듈 구현부 예시):
# ingress_rules 전개 시 resource "aws_security_group_rule" "ingress_cidr" { for_each = { for k, v in var.ingress_rules : k => v if try(length(v.cidr_blocks) > 0, false) } type = "ingress" from_port = each.value.from_port to_port = each.value.to_port protocol = each.value.protocol cidr_blocks = each.value.cidr_blocks security_group_id = module.this.security_group_ids[each.value.security_group_key] description = each.value.description }필요 시
variable "ingress_rules"에 validation을 추가하여 빈 규칙을 막는 방법도 있습니다(교차 변수 참조 난이도가 있어 선택). 현재 PR 범위를 넘으면 추후 리팩토링으로 분리하는 것도 OK입니다.Also applies to: 35-36
terraform-db-migration/main.tf (3)
94-129: 선택: Lambda 로그 보존/보안 강화(비차단)
- CloudWatch Log Group 명시 및
retention_in_days설정으로 로그 무기한 보존 방지- 필요 시 코드 서명 검증(CKV_AWS_272) 고려 — 규정 준수 요구가 없다면 이번 PR에선 생략 가능
빠른 보완 예:
resource "aws_cloudwatch_log_group" "migration_task" { name = "/aws/lambda/${aws_lambda_function.migration_task.function_name}" retention_in_days = 14 tags = { Purpose = "Ephemeral" } }
190-198: 선택: SSM SecureString에 고객 관리형 KMS(CMK) 사용 고려현재 키 미지정으로 AWS 관리형 키(alias/aws/ssm)를 사용합니다. 사내 정책에 CMK 의무가 있다면
key_id를 지정해 주세요(예: 부트스트랩 스택의 KMS ARN을 원격 상태에서 가져와 사용). 규정이 없다면 현 설정으로도 동작에 문제는 없습니다.
56-63: 선택: 임시 마이그레이션에서 CloudFront 생성 최소화 검토
module "cloned_s3_bucket"가 prod S3 모듈을 재사용하여 CloudFront까지 생성할 가능성이 높습니다. 마이그레이션 테스트에 CloudFront가 불필요하다면, “버킷만” 생성하는 경량 모듈로 분리(또는 플래그로 생성 제어)하여 비용/대기 시간을 줄일 수 있습니다. 일정 여유가 생기면 리팩토링 후보로 고려해 주세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
terraform-db-migration/locals.tf(1 hunks)terraform-db-migration/main.tf(1 hunks)terraform/common/security-group/variables.tf(2 hunks)terraform/prod/s3/main.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- terraform-db-migration/locals.tf
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryResponse.java:29-32
Timestamp: 2025-08-17T18:38:26.970Z
Learning: lvalentine6 prefers to manage PR scope carefully and defer non-critical improvements to future refactoring when the current PR is already large in scope, showing good judgment in prioritizing deliverable completion over scope expansion.
Learnt from: lvalentine6
PR: YAPP-Github/26th-Web-Team-1-BE#181
File: src/main/java/eatda/controller/story/StoryRegisterRequest.java:11-17
Timestamp: 2025-08-17T18:37:21.953Z
Learning: lvalentine6 prefers to apply validation annotations consistently across all record classes in the project rather than implementing them piecemeal, prioritizing architectural consistency and maintainability.
🪛 Checkov (3.2.334)
terraform-db-migration/main.tf
[HIGH] 94-129: Ensure AWS Lambda function is configured to validate code-signing
(CKV_AWS_272)
[MEDIUM] 142-162: Ensure that AWS database instances have deletion protection enabled
(CKV_AWS_293)
[HIGH] 142-162: Ensure RDS Performance Insights are encrypted using KMS CMKs
(CKV_AWS_354)
[MEDIUM] 164-178: Ensure that detailed monitoring is enabled for EC2 instances
(CKV_AWS_126)
[HIGH] 164-178: EC2 instance should not have public IP.
(CKV_AWS_88)
[HIGH] 164-178: Ensure Instance Metadata Service Version 1 is not enabled
(CKV_AWS_79)
[HIGH] 190-198: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
🔇 Additional comments (1)
terraform-db-migration/main.tf (1)
100-101: Lambda 번들 경로 확인 필요 — 실행 디렉터리에 따라 zip 경로가 달라질 수 있음현재
filename/source_code_hash = "./build/migration_lambda.zip"입니다. 워크플로우가 Terraform을 repo 루트에서-chdir=terraform-db-migration로 실행하면 현 경로가 유효합니다. 루트 기준으로 실행하면../build/...가 맞습니다. CI에서 zip 경로 미일치로 plan/apply가 실패할 수 있으니 실제 워크플로우의 작업 디렉터리를 확인해 주세요.두 경로 중 하나로 정합:
- filename = "./build/migration_lambda.zip" - source_code_hash = filebase64sha256("./build/migration_lambda.zip") + # (루트에서 실행 시) + # filename = "../build/migration_lambda.zip" + # source_code_hash = filebase64sha256("../build/migration_lambda.zip")
|
🎉 This PR is included in version 1.8.0-develop.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
✨ 개요
현재까지 반영사항
🧾 관련 이슈
#182
🔍 참고 사항 (선택)
Summary by CodeRabbit
신기능
작업(Chores)
테스트