[ALL-Feat] 이사하기 #340
[ALL-Feat] 이사하기 #340
Conversation
WalkthroughThis pull request enhances deployment workflows for both frontend and backend applications. In the GitHub Actions workflows, new steps have been added to securely copy deployment scripts and Nginx configuration files to an EC2 instance. The working directories and trigger paths have been updated accordingly. Additionally, two new shell scripts have been introduced to automate the deployment processes for backend and frontend applications, including service management and rollback capabilities. A minor update in the pre-push hook changes the shell interpreter from sh to bash. Changes
Sequence Diagram(s)sequenceDiagram
participant GA as GitHub Actions (Frontend)
participant EC2 as EC2 Instance
participant FD as frontend_deploy.sh
participant Nginx as Nginx Server
GA->>EC2: Copy deploy script & Nginx config files
EC2->>FD: Execute frontend_deploy.sh
FD->>EC2: Clear old build & copy new build files
FD->>Nginx: Copy configuration and run config test
Nginx-->>FD: Return test result (pass/fail)
FD->>EC2: Reload config or perform rollback
sequenceDiagram
participant GA as GitHub Actions (Backend)
participant EC2 as EC2 Instance
participant BD as backend_deploy.sh
participant SD as Systemd
GA->>EC2: Copy backend_deploy.sh script
EC2->>BD: Execute backend_deploy.sh
BD->>EC2: Set up directories, copy backend JAR, and update symlink
BD->>SD: Create/modify systemd service and reload daemon
BD->>BD: Perform health checks (retry loop)
BD-->>EC2: Complete deployment or trigger rollback
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/fe-cd.yml (1)
49-56: Use a pinned version forappleboy/scp-action.Relying on
@mastercan lead to unexpected changes downstream. Pinning the version (e.g.,@v0.1.7) ensures consistent and reproducible deployments.You can unify the version with the existing usage for consistency, for example:
- uses: appleboy/scp-action@master + uses: appleboy/scp-action@v0.1.7frontend/nginx/frontend.conf (1)
2-14: Prefer separate server blocks overifdirectives for domain redirects.Using multiple
if ($host = ...)blocks can introduce complexity or unexpected behavior in Nginx. Instead, consider defining distinct server blocks to handle each domain or usingserver_namedirectives for cleaner, more maintainable redirection logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/fe-cd.yml(1 hunks)frontend/nginx/frontend.conf(1 hunks)
🔇 Additional comments (1)
frontend/nginx/frontend.conf (1)
25-25: Includeapi.unjemanna.sitein the 443 server block if SSL should cover that domain.Currently, only
unjemanna.siteandwww.unjemanna.siteappear underserver_namefor port 443. If theapi.unjemanna.sitedomain requires HTTPS, consider including it here, unless it’s fully handled in/etc/nginx/conf.d/api.conf.
hamo-o
left a comment
hamo-o
left a comment
There was a problem hiding this comment.
회의 내용과 같이 노션에 있는 다른 파일들도 추가 부탁드립니다!!
- backend_deploy.sh, frontend_deploy.sh - nginx/backend.conf frontend.conf - fe-cd.yml, be-cd.yml 수정 (설정 파일을 ec2 서버로 전송하는 코드 추가)
sh로 실행하면 스크립트에 지원하지 않는 커맨드들이 있어서 에러 발생함. 그래서 bash로 수정
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (11)
script/deploy/frontend_deploy.sh (1)
47-48: Add directory existence check before cleanupThe script attempts to clean up old backups without verifying the directory exists, which could cause errors if the directory is missing.
# 오래된 백업 정리 (최근 5개만 유지) echo "오래된 프론트엔드 백업 정리 중..." -cd $APP_DIR/frontend/backups && ls -t | tail -n +6 | xargs rm -f || true +if [ -d "$APP_DIR/frontend/backups" ]; then + cd $APP_DIR/frontend/backups && ls -t | tail -n +6 | xargs rm -f || true +else + echo "백업 디렉토리가 존재하지 않습니다." +fiscript/deploy/nginx/frontend.conf (1)
1-21: Simplify HTTP to HTTPS redirectionThe current configuration uses multiple
ifstatements for redirection when a singlereturndirective would be more efficient. Nginx recommends avoidingifstatements when possible for performance reasons.server { - if ($host = www.unjemanna.site) { - return 301 https://$host$request_uri; - } - - - if ($host = api.unjemanna.site) { - return 301 https://$host$request_uri; - } - - - if ($host = unjemanna.site) { - return 301 https://$host$request_uri; - } - - listen 80; server_name unjemanna.site www.unjemanna.site api.unjemanna.site; return 301 https://$host$request_uri; }script/deploy/nginx/backend.conf (1)
10-12: Consider adding www subdomain to CORS allowed originsThe CORS configuration only allows
https://unjemanna.sitebut nothttps://www.unjemanna.site, which might cause CORS issues when users access the site via the www subdomain.- add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site' always; + add_header 'Access-Control-Allow-Origin' '$http_origin' always; + add_header 'Access-Control-Allow-Credentials' 'true' always; add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PATCH, PUT' always; add_header 'Access-Control-Allow-Headers' 'Authorization, Content-Type' always; + + # Only allow specific origins + if ($http_origin !~ '^https://(www\.)?unjemanna\.site$') { + add_header 'Access-Control-Allow-Origin' '' always; + }Similarly, update the same pattern in the OPTIONS block.
.github/workflows/be-cd.yml (2)
70-71: Verify that backend_deploy.sh exists and has execute permissionsThe workflow now executes the backend deployment script from a new location, but there's no step ensuring the script has execute permissions.
script: | cd ~/app/deploy + chmod +x ./backend_deploy.sh bash ./backend_deploy.sh
9-10: Add Nginx configuration files to workflow trigger pathsThe workflow triggers when changes are made to
backend/**orscript/deploy/backend_deploy.sh, but should also trigger when the Nginx configuration files are changed since they're relevant for the backend deployment.paths: - "backend/**" - "script/deploy/backend_deploy.sh" + - "script/deploy/nginx/backend.conf"script/deploy/backend_deploy.sh (6)
4-9: Variable Initialization and Directory ConfigurationThe variables (e.g.,
APP_DIR,BACKEND_DIR,BACKEND_JAR,LOG_DIR) are set up clearly. However, consider quoting the variable references (for example,"${APP_DIR}"and"${BACKEND_JAR}") to guard against issues with spaces or special characters in paths.
15-18: Log Directory CreationCreating and setting permissions for the log directory using
sudois appropriate. It might be beneficial to check the command results or capture errors to handle any permission issues that could arise in different deployment environments.
25-29: Deploying the Backend JARCopying the latest JAR to the new release directory and updating the symlink is a correct approach.
Suggestion: Wrap variable references in quotes to avoid potential issues when file paths contain spaces. For example:-cp $BACKEND_JAR $BACKEND_RELEASE_DIR/application.jar +cp "$BACKEND_JAR" "$BACKEND_RELEASE_DIR/application.jar" -ln -sfn $BACKEND_RELEASE_DIR $BACKEND_CURRENT_LINK +ln -sfn "$BACKEND_RELEASE_DIR" "$BACKEND_CURRENT_LINK"
48-52: Service Deployment and RestartMoving the generated service file to
/etc/systemd/system/, reloading, and then restarting the service is demonstrated correctly.
Suggestion: Consider adding error checking around these commands to verify that the service file is correctly deployed and that the daemon reload is successful.
53-65: Backend Health Check LoopThe loop repeatedly checks the backend's health using
curlandgrepuntil it confirms the service is up or reaches the maximum retry limit.
Suggestion: To prevent a hanging curl call, include a timeout parameter (e.g.,--max-time 2) in the curl command:-if curl -s http://localhost:8080/health | grep -q "UP"; then +if curl -s --max-time 2 http://localhost:8080/health | grep -q "UP"; then
78-83: Cleanup of Old ReleasesThe command for cleaning up old releases (keeping only the five most recent) is efficient.
Suggestion: As with previous file operations, quoting the directory variable can prevent issues with directory names containing spaces:-cd $BACKEND_DIR/releases && ls -t | tail -n +6 | xargs rm -rf || true +cd "$BACKEND_DIR/releases" && ls -t | tail -n +6 | xargs rm -rf || true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/hooks/pre-push(1 hunks).github/workflows/be-cd.yml(3 hunks).github/workflows/fe-cd.yml(3 hunks)script/deploy/backend_deploy.sh(1 hunks)script/deploy/frontend_deploy.sh(1 hunks)script/deploy/nginx/backend.conf(1 hunks)script/deploy/nginx/frontend.conf(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/hooks/pre-push
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/fe-cd.yml
🔇 Additional comments (8)
script/deploy/nginx/frontend.conf (1)
56-57:Details
❓ Verification inconclusive
Missing cert.conf reference
The configuration includes a reference to
/etc/nginx/conf.d/cert.conf, but there's no mention of this file being created or copied in the deployment script.Please verify that
cert.confexists or is created during the deployment process. You should add a step in the deployment script to handle the SSL certificate configuration:
🏁 Script executed:
#!/bin/bash # Check if cert.conf exists on the target server ssh $EC2_USERNAME@$EC2_HOST "ls -la /etc/nginx/conf.d/cert.conf"Length of output: 138
Manual SSL Certificate Configuration Verification Needed
We've attempted to verify the presence of
/etc/nginx/conf.d/cert.confusing the provided script, but the check failed with an error indicating that thesshcommand is not available. This leaves us with inconclusive evidence on whether the file is created or deployed. Please manually verify in an environment wheresshis available thatcert.confexists (or is generated/copied) during the deployment process. Also, ensure that the deployment script includes the necessary step to handle the SSL certificate configuration if the file is missing..github/workflows/be-cd.yml (1)
43-51: LGTM: Good addition of deploy script copyingThe addition of a step to copy the deployment script to the EC2 instance is a good practice. This ensures that the latest version of the deployment script is used.
script/deploy/backend_deploy.sh (6)
1-3: File Header and Safety ConfigurationThe shebang and
set -edirective ensure that the script exits immediately on any error. This is a good safeguard for deployment procedures.
10-14: Timestamp and Release Directory SetupGenerating a timestamp with the
datecommand and using it to create a release directory is well implemented. This provides a clear mechanism for versioning and rollback. No changes are needed.
19-21: Backend Release Directory StructureUsing
mkdir -pto create the release directory is a straightforward, effective approach. This segment is correctly implemented.
22-24: Recording Previous Deployment StateCapturing the previous release path via
readlink(with a fallback to an empty string) is a good strategy for implementing rollback procedures. No changes are needed here.
30-47: Service File GenerationThe heredoc used for generating the systemd unit file is clear and effective. Variable substitution works as intended to insert paths into the service file. This segment is good to go.
66-77: Rollback MechanismThe rollback strategy checks for an available previous deployment and reverts the symlink if the backend fails to become healthy. This is a robust approach to ensure service availability.
| sudo rm -rf $FRONTEND_DIR/* | ||
| sudo tar -xzf $APP_DIR/frontend/backups/frontend_$TIMESTAMP.tar.gz -C $FRONTEND_DIR | ||
| echo "이전 버전으로 롤백했습니다." | ||
| exit 1 |
There was a problem hiding this comment.
Rollback functionality references an undefined variable
The rollback code uses $TIMESTAMP which is undefined in this script. The rollback will fail when needed because of this missing variable.
+# 배포 전 현재 프론트엔드의 백업 생성
+TIMESTAMP=$(date +%Y%m%d%H%M%S)
+mkdir -p $APP_DIR/frontend/backups
+sudo tar -czf $APP_DIR/frontend/backups/frontend_$TIMESTAMP.tar.gz -C $FRONTEND_DIR .
+echo "현재 프론트엔드 버전이 백업되었습니다."
+
# 새 버전 배포
sudo rm -rf $FRONTEND_DIR/*
sudo cp -r $APP_DIR/frontend/dist/* $FRONTEND_DIR/Committable suggestion skipped: line range outside the PR's diff.
| server { | ||
| listen 443 ssl; | ||
| server_name unjemanna.site www.unjemanna.site; | ||
|
|
There was a problem hiding this comment.
Server block is missing api.unjemanna.site
The server block for HTTPS doesn't include api.unjemanna.site in the server_name directive, while it's included in the HTTP block. This inconsistency might cause issues when accessing the API via HTTPS.
server {
listen 443 ssl;
- server_name unjemanna.site www.unjemanna.site;
+ server_name unjemanna.site www.unjemanna.site api.unjemanna.site;📝 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.
| server { | |
| listen 443 ssl; | |
| server_name unjemanna.site www.unjemanna.site; | |
| server { | |
| listen 443 ssl; | |
| server_name unjemanna.site www.unjemanna.site api.unjemanna.site; |
| add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site' always; | ||
| add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PATCH, PUT' always; | ||
| add_header 'Access-Control-Allow-Headers' 'Authorization, Content-Type' always; | ||
|
|
||
| if ($request_method = 'OPTIONS') { | ||
| add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site'; | ||
| add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PUT, OPTIONS, PATCH'; | ||
| add_header 'Access-Control-Allow-Headers' 'Content-Type, Authorization'; | ||
| add_header 'Access-Control-Max-Age' 86400; | ||
| return 204; | ||
| } |
There was a problem hiding this comment.
CORS headers inconsistency between normal responses and OPTIONS
The CORS headers are defined twice with slight differences between normal responses and OPTIONS requests. The Access-Control-Allow-Methods values are different, which could cause inconsistent behavior.
- add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site' always;
- add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PATCH, PUT' always;
- add_header 'Access-Control-Allow-Headers' 'Authorization, Content-Type' always;
+ add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site' always;
+ add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PUT, OPTIONS, PATCH' always;
+ add_header 'Access-Control-Allow-Headers' 'Content-Type, Authorization' always;
if ($request_method = 'OPTIONS') {
add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site';
add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PUT, OPTIONS, PATCH';
add_header 'Access-Control-Allow-Headers' 'Content-Type, Authorization';
add_header 'Access-Control-Max-Age' 86400;
return 204;
}📝 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.
| add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site' always; | |
| add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PATCH, PUT' always; | |
| add_header 'Access-Control-Allow-Headers' 'Authorization, Content-Type' always; | |
| if ($request_method = 'OPTIONS') { | |
| add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site'; | |
| add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PUT, OPTIONS, PATCH'; | |
| add_header 'Access-Control-Allow-Headers' 'Content-Type, Authorization'; | |
| add_header 'Access-Control-Max-Age' 86400; | |
| return 204; | |
| } | |
| add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site' always; | |
| add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PUT, OPTIONS, PATCH' always; | |
| add_header 'Access-Control-Allow-Headers' 'Content-Type, Authorization' always; | |
| if ($request_method = 'OPTIONS') { | |
| add_header 'Access-Control-Allow-Origin' 'https://unjemanna.site'; | |
| add_header 'Access-Control-Allow-Methods' 'GET, POST, DELETE, PUT, OPTIONS, PATCH'; | |
| add_header 'Access-Control-Allow-Headers' 'Content-Type, Authorization'; | |
| add_header 'Access-Control-Max-Age' 86400; | |
| return 204; | |
| } |
hamo-o
left a comment
hamo-o
left a comment
There was a problem hiding this comment.
처음에 서버에 패키지 까는 요부분은 없는데 이것도 올려주실 수 있나요?
pm2 설치해야 해서요
그거는 스크립트가 아니라 제가 하나씩 타이핑해서 설치할 거 모아둔 거에요. |
hamo-o
left a comment
hamo-o
left a comment
There was a problem hiding this comment.
아 그러면 일단 넘어가도 될 것 같습니다 수고하셨어요
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit
New Features
Chores