Uh oh!
There was an error while loading. Please reload this page.
fix: nginx 인증서 자동 갱신 안정화 - #55
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughmonitoring_stack 및 app_stack의 nginx 설정이 standalone Certbot에서 webroot 기반 HTTP-01 challenge 방식으로 전환되었다. monitoring_stack에는 IAM instance profile과 SSM RunCommand 기반 nginx 설정 재실행 경로가 추가되었으며, 관련 변수와 모듈 배선 및 secrets 서브프로젝트 참조가 갱신되었다. ChangesNginx SSL 갱신 및 SSM 재실행
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Terraform
participant SSM as AWS SSM
participant EC2 as monitoring_server
participant Nginx
participant Certbot
Terraform->>SSM: SSM 에이전트 온라인 대기
Terraform->>SSM: send-command(setup_nginx.sh)
SSM->>EC2: nginx 설정 스크립트 실행
EC2->>Nginx: challenge 설정 작성 및 reload
EC2->>Certbot: certonly --webroot 인증서 발급
EC2->>Nginx: full 설정 작성 및 reload
Terraform->>SSM: 실행 상태 폴링
SSM-->>Terraform: Success 또는 실패 상태
Certbot->>Nginx: renew deploy-hook reload
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Terraform Plan: |
Terraform Plan: |
Terraform Plan: |
Terraform Plan: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:7b05728b97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
modules/monitoring_stack/scripts/nginx_setup.sh.tftpl (1)
22-45: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win인증서가 있으면 80 전용 설정으로 먼저 덮어쓰지 마세요
modules/monitoring_stack/scripts/nginx_setup.sh.tftpl:22-45에서 매번 80 전용 설정으로reload한 뒤 full 설정을 다시 쓰고 있어서, 기존 인증서가 있는 재실행에서도 443 서버 블록이 잠깐 사라집니다. 인증서가 이미 있으면 처음부터 full 설정을 적용하고, challenge location만 추가하는 분기로 바꾸는 편이 좋습니다.🤖 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 `@modules/monitoring_stack/scripts/nginx_setup.sh.tftpl` around lines 22 - 45, The nginx setup flow in the script template is overwriting the existing config with an HTTP-only server block before restoring the full TLS config, which can temporarily drop the 443 block on reruns. Update the branching around the nginx config generation so the existing-cert path writes the full server configuration from the start, and only adds the ACME challenge location needed for certbot; keep the no-cert path on the current 80-only bootstrap. Use the nginx config generation and reload logic in the script template to locate the change.modules/app_stack/scripts/nginx_setup.sh.tftpl (1)
154-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value중복된 full 설정 작성/심볼릭 링크 처리.
인증서가 이미 존재하는 경로(154-162행)에서
write_full_nginx_conf+ 심볼릭 링크 + default 제거를 수행한 뒤, 176-180행에서 동일 작업이 다시 반복됩니다. challenge→full 전환이 필요한 최초 발급 경로를 제외하면 중복이므로, 발급이 실제로 필요한 경우에만 재작성하도록 정리하면 흐름이 단순해집니다.🤖 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 `@modules/app_stack/scripts/nginx_setup.sh.tftpl` around lines 154 - 180, The nginx setup flow is duplicating the full config write and site symlink/default cleanup in the certificate-present branch and again after certbot runs. Refactor the logic around write_full_nginx_conf, write_http_challenge_conf, the ln -sf /etc/nginx/sites-available/$CONF_NAME step, and reload_nginx so the full configuration and symlink cleanup happen only once after a successful issuance/renewal, while the challenge config remains only for the initial HTTP-01 path.
🤖 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 `@modules/monitoring_stack/scripts/nginx_setup.sh.tftpl`:
- Around line 102-108: Split the cron file name used in the certbot setup from
CONF_NAME, since the current /etc/cron.d/certbot-$CONF_NAME naming can inherit
dots from nginx_conf_name and be ignored by cron. Update the
nginx_setup.sh.tftpl block that writes the cron entry so the /etc/cron.d
filename is a fixed or sanitized value, while keeping CONF_NAME only for the
rest of the setup logic. Use the certbot renewal cron stanza as the location to
make this change.
---
Nitpick comments:
In `@modules/app_stack/scripts/nginx_setup.sh.tftpl`:
- Around line 154-180: The nginx setup flow is duplicating the full config write
and site symlink/default cleanup in the certificate-present branch and again
after certbot runs. Refactor the logic around write_full_nginx_conf,
write_http_challenge_conf, the ln -sf /etc/nginx/sites-available/$CONF_NAME
step, and reload_nginx so the full configuration and symlink cleanup happen only
once after a successful issuance/renewal, while the challenge config remains
only for the initial HTTP-01 path.
In `@modules/monitoring_stack/scripts/nginx_setup.sh.tftpl`:
- Around line 22-45: The nginx setup flow in the script template is overwriting
the existing config with an HTTP-only server block before restoring the full TLS
config, which can temporarily drop the 443 block on reruns. Update the branching
around the nginx config generation so the existing-cert path writes the full
server configuration from the start, and only adds the ACME challenge location
needed for certbot; keep the no-cert path on the current 80-only bootstrap. Use
the nginx config generation and reload logic in the script template to locate
the change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 60bcfba8-8100-433c-8ed4-208cdfeb15f6
📒 Files selected for processing (7)
config/secretsenvironment/monitoring/main.tfenvironment/monitoring/variables.tfmodules/app_stack/scripts/nginx_setup.sh.tftplmodules/monitoring_stack/ec2.tfmodules/monitoring_stack/scripts/nginx_setup.sh.tftplmodules/monitoring_stack/variables.tf
Uh oh!
There was an error while loading. Please reload this page.
wibaek
commented
Jul 8, 2026
👍 Caddy 사용도 고려해볼 수 있을 것 같네요 |
Hexeong
commented
Jul 9, 2026
찾아본 결과, Caddy가 현재 nginx의 역할을 대체할 수 있다는 점 확인했습니다! 이번 PR이 반영되고 동일한 문제가 발생한다면 해당 작업 진행해보는 방향으로 하겠습니다! |
lsy1307
left a comment
There was a problem hiding this comment.
전체 nginx/webroot 전환 흐름은 괜찮아 보입니다. 기존 인증서가 있으면 full nginx config를 유지한 상태에서 challenge location만 포함하고, 인증서가 없을 때만 HTTP challenge bootstrap config를 쓰는 분기도 적절해 보입니다.
다만 config/secrets submodule 포인터 변경에 monitoring.tfvars뿐 아니라 prod.tfvars 변경도 함께 포함되어 있습니다. prod plan 결과가 null_resource.update_nginx 교체만 나온다고 적어주셔서 실제 영향은 없어 보이지만, PR 설명에는 monitoring secrets 갱신만 언급되어 있어 prod tfvars 변경도 의도된 최신화인지 한 줄 남겨주면 더 명확할 것 같습니다.
Hexeong
commented
Jul 14, 2026
해당 내용은 다른 PR이 머지됨과 함께 서브 모듈 포인터 충돌을 해결하고자 반영된 내용입니다. 포인터를 최신화하는 과정에서 반영되었기에 해당 PR의 작업내용은 아니지만, nginx 설정이 반영되는 과정에 있어 문제가 없게 최신화를 진행했습니다. |
Uh oh!
There was an error while loading. Please reload this page.
관련 이슈
작업 내용
Monitoring Nginx 인증서 발급/갱신 방식을
standalone에서webroot방식으로 변경했습니다./.well-known/acme-challenge/요청을 처리합니다./etc/cron.d/certbot-$CONF_NAME파일로 관리하도록 변경해 중복 등록을 방지했습니다.Monitoring EC2에 nginx 설정 변경 반영 경로를 추가했습니다.
nginx_setup.sh.tftpl렌더링 결과의 hash를terraform_data.update_nginx트리거로 사용합니다.ec2_iam_instance_profile변수를 추가했습니다.Prod/Stage 공용 app_stack Nginx 인증서 갱신도 동일하게 안정화했습니다.
certbot --standalone과systemctl stop nginx흐름을 제거했습니다.null_resource.update_nginx의 기존 script hash 기반 재실행 구조는 유지했습니다.Monitoring secrets submodule을 갱신했습니다.
특이 사항
reload로 수행되며, reload 실패 시에만restartfallback을 사용합니다.nginx_setup.sh.tftpl내용이 바뀌면 script hash 변경으로null_resource.update_nginx가 재실행됩니다.terraform_data.update_nginx를 통해 동일한 hash 기반 재실행 구조를 갖습니다.검증 결과:
bash -n modules/monitoring_stack/scripts/nginx_setup.sh.tftplbash -n modules/app_stack/scripts/nginx_setup.sh.tftplterraform validateforenvironment/monitoringterraform validateforenvironment/stageterraform validateforenvironment/prodterraform_data.update_nginx생성null_resource.update_nginxscript hash replacement onlynull_resource.update_nginxscript hash replacement only리뷰 요구사항 (선택)
Summary by CodeRabbit