-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] terraform deprecated 된 명령어인 name을 id로 변경 #192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughTerraform ARNs in ecs task role inline policies and SSM MySQL parameter policy were updated to use data.aws_region.current.id instead of data.aws_region.current.name for SSM parameters and KMS decrypt resources within terraform/common/locals.tf. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
leegwichan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! 사소한 부분까지 놓치지 않는 승로님 👍
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
terraform/common/locals.tf (2)
90-92: 동일 맥락: dev/prod 파라미터 ARNs도 region 속성으로 전환 고려
id로의 치환은 실행에 문제없으나, Terraform AWS Provider v6의 가이드에 맞춰region속성 사용으로 통일하는 것이 추후 업그레이드(v7) 대비에 유리합니다. 현재 PR 범위를 벗어난다면 후속 작업으로 분리합시다. (newreleases.io)- "arn:aws:ssm:${data.aws_region.current.id}:${data.aws_caller_identity.current.account_id}:parameter/dev/*", - "arn:aws:ssm:${data.aws_region.current.id}:${data.aws_caller_identity.current.account_id}:parameter/prod/*" + "arn:aws:ssm:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:parameter/dev/*", + "arn:aws:ssm:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:parameter/prod/*"
45-47: ✔️.name속성 미사용 확인 – 현 PR 승인 및 Optionalregion전환 제안레포 내 모든
.tf파일에서data.aws_region.*.name호출이 전혀 없음을 확인했습니다.
따라서 이번 PR에서 이미 적용하신${data.aws_region.current.id}사용은 문제없이 동작하며, 현 단계에서는 그대로 머지하셔도 좋습니다.다만 AWS Provider v6 권고(
.name대신.region사용)를 장기 호환성(v7 이상) 차원에서 Optional로 권장드립니다. 후속 PR에서 아래처럼 일괄 전환을 고려해 보세요:- "arn:aws:ssm:${data.aws_region.current.id}:${data.aws_caller_identity.current.account_id}:parameter/dev/MYSQL_URL", - "arn:aws:ssm:${data.aws_region.current.id}:${data.aws_caller_identity.current.account_id}:parameter/dev/MYSQL_USER_NAME", - "arn:aws:ssm:${data.aws_region.current.id}:${data.aws_caller_identity.current.account_id}:parameter/dev/MYSQL_PASSWORD" + "arn:aws:ssm:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:parameter/dev/MYSQL_URL", + "arn:aws:ssm:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:parameter/dev/MYSQL_USER_NAME", + "arn:aws:ssm:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:parameter/dev/MYSQL_PASSWORD"
- 검증 커맨드:
rg -nP -C2 -g '**/*.tf' 'data\.aws_region\.[A-Za-z0-9_-]+\.(name)\b'- 대상 파일:
terraform/common/locals.tf(45–47라인)
📜 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 (1)
terraform/common/locals.tf(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.
⏰ 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
|
🎉 This PR is included in version 1.8.0-develop.12 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.8.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |



✨ 개요
🧾 관련 이슈
Closed #185
🔍 참고 사항 (선택)
Summary by CodeRabbit