feat: nginx config for self hosting #560
Conversation
WalkthroughThe nginx configuration file was updated to streamline and centralize its handling of frontend and backend requests. The server block now explicitly sets the server name and document root. The frontend location serving was simplified, and all backend API-related proxying was consolidated into a single regex-based location block. Additional proxy headers for real IP and forwarding were added, and some timeout and cache bypass directives were removed. Proxy connection handling was clarified, and error handling directives were removed from the proxy block. Changes
Assessment against linked issues
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 (
|
amhsirak
added
Type: Infra
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
nginx.conf (4)
3-3: Consider specifying explicit server names for production.While
server_name _;works as a catch-all configuration for development, in production environments it's generally better to specify the exact domain names your application will serve. This improves security by preventing unexpected domain name binding.- server_name _; + server_name yourdomain.com www.yourdomain.com;
15-15: Consider parameterizing the backend service location.The hardcoded
localhost:8080with an inline comment "change as per your setup" suggests this might need customization in different environments. Consider using environment variables or including conditional configurations for different deployment environments.- proxy_pass http://localhost:8080; # change as per your setup + proxy_pass ${BACKEND_URL}; # Configured via environment variableOr alternatively, create environment-specific include files:
- proxy_pass http://localhost:8080; # change as per your setup + include /etc/nginx/conf.d/backend_location.conf;
1-28: Add performance optimizations for static assets.Consider adding gzip compression and caching headers for static assets to improve performance. This is particularly important for a production environment.
Add these directives to the server block:
# Enable gzip compression gzip on; gzip_types text/plain text/css application/json application/javascript text/xml application/xml application/xml+rss text/javascript; gzip_comp_level 6; gzip_min_length 1000; # Cache static assets location ~* \.(jpg|jpeg|png|gif|ico|css|js)$ { expires 7d; add_header Cache-Control "public, max-age=604800, immutable"; }
1-28: Add request size limit for security.Consider adding a
client_max_body_sizedirective to limit upload sizes and prevent potential DoS attacks through large file uploads.Add this to the server block:
# Limit request size client_max_body_size 10M;Adjust the size limit according to your application's requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
nginx.conf(1 hunks)
🔇 Additional comments (3)
nginx.conf (3)
5-7: The frontend configuration looks good.Explicitly setting the root directory and index file improves clarity. This is a good practice for maintainability.
9-11: The frontend location block is well structured.The simplified location block with
try_filesis the standard approach for serving single-page applications, allowing the client-side routing to handle paths.
20-22: Good addition of proxy headers for proper request forwarding.The added headers
X-Real-IP,X-Forwarded-For, andX-Forwarded-Protoare essential for correctly identifying client information when using a reverse proxy. This ensures the backend application can correctly identify client IPs and protocol information.
nginx.conf
Outdated
| server { | ||
| listen 80; | ||
| server_name _; | ||
|
|
||
| root /var/www/maxun; | ||
| index index.html; | ||
|
|
||
| # Serve the frontend | ||
| location / { | ||
| root /usr/share/nginx/html; | ||
| try_files $uri $uri/ /index.html; | ||
| } | ||
|
|
||
| location /api { | ||
| proxy_pass http://localhost:8080; | ||
|
|
||
| # Proxy for backend | ||
| location ^/(auth|storage|record|workflow|robot|proxy|api-docs|api)(/|$) { | ||
| proxy_pass http://localhost:8080; # change as per your setup | ||
| proxy_http_version 1.1; | ||
| proxy_set_header Upgrade $http_upgrade; | ||
| proxy_set_header Connection 'upgrade'; | ||
| proxy_set_header Host $host; | ||
| proxy_cache_bypass $http_upgrade; | ||
|
|
||
| # Add timeout configurations | ||
| proxy_connect_timeout 60s; | ||
| proxy_send_timeout 60s; | ||
| proxy_read_timeout 60s; | ||
|
|
||
| # Add error handling | ||
| proxy_intercept_errors on; | ||
| error_page 502 503 504 /50x.html; | ||
| } | ||
| proxy_set_header X-Real-IP $remote_addr; | ||
| proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; | ||
| proxy_set_header X-Forwarded-Proto $scheme; | ||
|
|
||
| location ~ ^/(record|workflow|storage|auth|integration|proxy|api-docs) { | ||
| proxy_pass http://localhost:8080; | ||
| proxy_http_version 1.1; | ||
| proxy_set_header Upgrade $http_upgrade; | ||
| proxy_set_header Connection 'keep-alive'; # Ensure connections remain open | ||
| proxy_set_header Connection 'upgrade'; | ||
| proxy_set_header Host $host; | ||
| proxy_cache_bypass $http_upgrade; | ||
|
|
||
| # Timeout configurations | ||
| proxy_connect_timeout 60s; | ||
| proxy_send_timeout 60s; | ||
| proxy_read_timeout 60s; | ||
|
|
||
| # Error handling for these routes | ||
| # Add error handling | ||
| proxy_intercept_errors on; | ||
| error_page 502 503 504 /50x.html; | ||
| } | ||
| } No newline at end of file | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding HTTPS configuration for production.
The current configuration only listens on port 80 (HTTP). For production use, especially with user authentication, it's strongly recommended to add HTTPS support with proper SSL certificate configuration.
You could add an additional server block like this:
server {
listen 80;
server_name yourdomain.com www.yourdomain.com;
return 301 https://$host$request_uri;
}
server {
listen 443 ssl;
server_name yourdomain.com www.yourdomain.com;
ssl_certificate /path/to/fullchain.pem;
ssl_certificate_key /path/to/privkey.pem;
ssl_protocols TLSv1.2 TLSv1.3;
ssl_prefer_server_ciphers on;
# Rest of your configuration...
root /var/www/maxun;
index index.html;
# Remaining location blocks...
}For easier SSL management, consider using Certbot/Let's Encrypt.
nginx.conf
Outdated
| # Add error handling | ||
| proxy_intercept_errors on; | ||
| error_page 502 503 504 /50x.html; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the existence of the error page.
The configuration references /50x.html for error handling, but it's not clear if this file exists in the specified root directory (/var/www/maxun). Ensure this file exists or create it with appropriate content.
🏁 Script executed:
#!/bin/bash
# Check if the 50x.html file exists in common locations
# Look for 50x.html in the specified root directory
echo "Checking if 50x.html exists in project files..."
fd "50x.html" --type fLength of output: 127
Add the missing /50x.html error page
The error_page 502 503 504 /50x.html; directive refers to /50x.html, but no such file was found in the repository. Please add a 50x.html file under your configured document root (/var/www/maxun) with appropriate content (or update the path to point to an existing error page).
• nginx.conf (around lines 24–27): error_page 502 503 504 /50x.html;
• Missing file: 50x.html in /var/www/maxun/50x.html
| proxy_pass http://localhost:8080; | ||
|
|
||
| # Proxy for backend | ||
| location ^/(auth|storage|record|workflow|robot|proxy|api-docs|api)(/|$) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that the regex location pattern captures all required API endpoints.
The regex pattern ^/(auth|storage|record|workflow|robot|proxy|api-docs|api)(/|$) consolidates multiple API endpoints into a single location block, which is good for maintainability. However, ensure it captures all the backend endpoints your application requires.
🏁 Script executed:
#!/bin/bash
# Verify if there might be other API endpoints in the codebase that should be included in the nginx regex
# Look for API route definitions in the codebase
echo "Searching for potential API routes defined in the codebase..."
rg -g "*.{js,ts,jsx,tsx}" "router\.(get|post|put|delete|patch)\s*\(\s*['\"]\/[^/]+" --json | jq -r ".data.lines.text" | sort | uniqLength of output: 434
🏁 Script executed:
#!/bin/bash
# Corrected search for API route prefixes in JS/TS files using PCRE
echo "Searching for API route prefixes defined in the codebase..."
rg -P -g '*.{js,ts,jsx,tsx}' "router\.(?:get|post|put|delete|patch)\(\s*['\"]\/([^\/]+)" --only-matching --replace '$1' | sort -uLength of output: 1975
Update nginx location regex to include missing API prefixes
The current pattern only matches /robot (singular) and doesn’t cover the /robots and /integration routes found in the codebase. Please update the location block on line 14 of nginx.conf to include these:
• /robots for the routes in server/src/api/record.ts
• /integration for the routes in server/src/routes/integration.ts
Suggested diff:
- location ^/(auth|storage|record|workflow|robot|proxy|api-docs|api)(/|$) {
+ location ^/(auth|storage|record|workflow|robots|integration|proxy|api-docs|api)(/|$) {📝 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.
| location ^/(auth|storage|record|workflow|robot|proxy|api-docs|api)(/|$) { | |
| location ^/(auth|storage|record|workflow|robots|integration|proxy|api-docs|api)(/|$) { |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
nginx.conf (1)
14-15: Regex location still misses/robots&/integrationendpoints (re-raised)A previous review already pointed out that plural
/robotsand the/integrationprefix exist in the codebase.
They are still absent, so those routes will 404.-location ^/(auth|storage|record|workflow|robot|proxy|api-docs|api)(/|$) { +location ^/(auth|storage|record|workflow|robot|robots|integration|proxy|api-docs|api)(/|$) {
🧹 Nitpick comments (2)
nginx.conf (2)
20-23: Optional: normalise headers & indentationMinor nits:
• Mixed tabs/spaces make the file harder to diff.
• If you terminate SSL elsewhere (load balancer), also set
proxy_set_header X-Forwarded-Host $host;andX-Forwarded-Ssl on;for clearer downstream logging.
8-11: Optional: enable long-term caching for static assetsFor SPA bundles you can dramatically cut bandwidth by adding:
location / { try_files $uri $uri/ /index.html; expires 7d; add_header Cache-Control "public, must-revalidate"; }(Adjust duration to match your cache-busting strategy.)
| proxy_set_header Upgrade $http_upgrade; | ||
| proxy_set_header Connection 'keep-alive'; # Ensure connections remain open | ||
| proxy_set_header Connection 'upgrade'; | ||
| proxy_set_header Host $host; |
There was a problem hiding this comment.
Hard-coding Connection 'upgrade' may break normal HTTP requests
Sending Connection: upgrade on every proxied request can confuse back-ends that do not support WebSockets.
Use the recommended map-based switch instead:
# place this once in the http{} block
map $http_upgrade $connection_upgrade {
default close;
websocket upgrade;
}
# inside the location block
proxy_set_header Upgrade $http_upgrade;
proxy_set_header Connection $connection_upgrade;| listen 80; | ||
| server_name _; |
There was a problem hiding this comment.
Add default_server or a concrete hostname to avoid name-based routing issues
With server_name _; Nginx will only serve this block when no other server_name matches.
If another vhost is added later, requests may unexpectedly be routed elsewhere.
Safest options:
-listen 80;
+listen 80 default_server;or replace _ with your real hostname(s) so that routing is explicit.
📝 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.
| listen 80; | |
| server_name _; | |
| listen 80 default_server; | |
| server_name _; |
closes #551
Summary by CodeRabbit