[All-Feat] Nginx 리버스 프록시 설정 #350
Conversation
WalkthroughThe pull request updates the Nginx configuration in Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Nginx
participant Backend as Local Server (Port 5173)
participant Static as Static Asset Handler (/var/www/html)
Client->>Nginx: Send HTTP Request
alt Request is for dynamic content
Nginx->>Backend: Proxy request with WebSocket support
Backend-->>Nginx: Processed Response
Nginx-->>Client: Return Response
else Request is for static content
Nginx->>Static: Serve static asset from /var/www/html
Static-->>Nginx: Asset content
Nginx-->>Client: Return Static Content
end
Suggested reviewers
Poem
🪧 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
script/deploy/nginx/frontend.conf(2 hunks)
🔇 Additional comments (2)
script/deploy/nginx/frontend.conf (2)
37-42: Great implementation of Nginx reverse proxy for SSR!The proxy setup is correctly configured to forward requests to your SSR server running locally on port 5173. The configuration includes all the necessary headers for WebSocket support (
UpgradeandConnection: 'upgrade'), and preserves the original host header.
54-60:Details
❓ Verification inconclusive
Revise the static file handling configuration.
The current configuration has a couple of potential issues:
The
try_files $uri /index.html;directive for static assets will cause 404 errors for missing static files to serve the index.html instead. This is typically not desired for static assets and could cause confusion.Consider adding the same security headers that are present in the root location.
# 정적 assets location ~* \.(js|css|png|jpg|jpeg|gif|ico|svg)$ { root /var/www/html; expires 7d; add_header Cache-Control "public, max-age=604800"; - try_files $uri /index.html; + try_files $uri =404; + + # 보안 헤더 추가 + add_header X-Content-Type-Options nosniff; + add_header X-Frame-Options SAMEORIGIN; + add_header X-XSS-Protection "1; mode=block"; }Does your frontend application need the fallback to index.html for static assets? If this is required for your application architecture, please clarify.
Attention: Confirm the static fallback behavior for missing assets
The updated configuration now returns a 404 for missing static files and adds security headers to align with your root settings. This helps avoid serving the index page as a fallback for missing assets—a behavior that is generally undesired for static resources. Please verify that your frontend application doesn't require the fallback to
/index.htmlin this location; if it does for SPA routing or another purpose, indicate accordingly.
- File affected:
script/deploy/nginx/frontend.conf(Lines 54-60)- Changes made:
- Replaced
try_files $uri /index.html;withtry_files $uri =404;to prevent serving the index page on 404 errors.- Added security headers:
add_header X-Content-Type-Options nosniff;add_header X-Frame-Options SAMEORIGIN;add_header X-XSS-Protection "1; mode=block";If the SPA architectural requirements necessitate the fallback behavior, please clarify and adjust the configuration accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
script/deploy/nginx/frontend.conf (2)
51-52: Consider using a separate directory for static assetsThe current configuration uses the same root directory (
/var/www/html) for both the SSR application and static assets. Consider using a dedicated subdirectory for static assets (e.g.,/var/www/html/assetsor/var/www/html/static) to better organize your deployment and avoid potential conflicts.
51-51: Expand file type coverage for static assetsThe current regex only covers common static files but misses some important web asset types like fonts (woff, woff2, ttf, eot), videos (mp4, webm), and documents (pdf). Consider expanding the file types to ensure comprehensive static asset handling.
- location ~* \.(js|css|png|jpg|jpeg|gif|ico|svg)$ { + location ~* \.(js|css|png|jpg|jpeg|gif|ico|svg|woff|woff2|ttf|eot|pdf|mp4|webm)$ {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
script/deploy/nginx/frontend.conf(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
script/deploy/nginx/frontend.conf (1)
Learnt from: dioo1461
PR: softeer5th/Team4-enDolphin#350
File: script/deploy/nginx/frontend.conf:49-51
Timestamp: 2025-03-25T07:32:54.031Z
Learning: For Nginx configurations with SSR (Server-Side Rendering), caching directives like 'expires' and 'Cache-Control' headers should be removed from proxy location blocks to prevent API requests from being cached by browsers. Static assets should have their own location blocks with appropriate caching settings.
🔇 Additional comments (1)
script/deploy/nginx/frontend.conf (1)
37-42: Well-implemented proxy configuration for SSRThe proxy configuration for the SSR server is correctly implemented with essential headers for WebSocket support and proper host header forwarding. Good job avoiding caching directives in this proxy block, which aligns with the best practices for SSR implementations to prevent unintended caching of dynamic content and API requests.
| root /var/www/html; | ||
| expires 7d; | ||
| add_header Cache-Control "public, max-age=604800"; | ||
| try_files $uri /index.html; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reconsider fallback behavior for static assets
The try_files $uri /index.html; directive will serve the index.html file when a static asset is not found. This could cause confusing behavior for missing assets and potentially hide 404 errors. For static assets, it's generally better to return a 404 status when the file is not found.
- try_files $uri /index.html;
+ try_files $uri =404;📝 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.
| try_files $uri /index.html; | |
| try_files $uri =404; |
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
SSR 구현을 위해서 Nginx 프록시 설정을 합니다. SSR 서버를 띄우는 부분은 #340 에 같이 올라갔습니다.
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
이해를 돕기 위해 포스팅을 작성했습니다.
https://hamo0.tistory.com/99 (비밀번호 1111, 성공하면 공개로 바꾸려고요....)
Summary by CodeRabbit