Skip to content

Conversation

@elavol
Copy link

@elavol elavol commented Jan 5, 2026

This fix issue Swetrix/swetrix#460, by using nginx to proxy request then forward to either API or FE. It help Swetrix can be deploy on single domain with API is on /api/ instead of separate domain

Summary by CodeRabbit

  • Chores

    • Added an nginx reverse proxy so the app and API are served from a single public port (80); removed direct service port exposure.
    • Frontend served at /, backend available under /backend/; proxy enforces HTTP/1.1, standard proxy headers, gzip compression, and adds security headers.
  • Documentation

    • Renamed environment variable to BASE_URL (public URL, no trailing slash); updated setup prompts and README to reference BASE_URL.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 5, 2026

📝 Walkthrough

Walkthrough

Added an nginx reverse-proxy service to Docker Compose, removed host port mappings from swetrix and swetrix-api, renamed swetrix env API_URLBASE_URL, and updated nginx to route /backend/swetrix-api:5005 and /swetrix:3000 on port 80.

Changes

Cohort / File(s) Summary
Docker Compose
compose.yaml
Added nginx-proxy (nginx:1.29.4-alpine, restart: always) with depends_on health conditions for swetrix/swetrix-api, links to them, ports: 80:80, and volumes: ./nginx/config:/etc/nginx/conf.d/default.conf; removed host port mappings 80:3000 (swetrix) and 8080:5005 (swetrix-api); changed swetrix env key API_URLBASE_URL.
Nginx config
nginx/config
Consolidated to a single server listening on port 80; added global security headers and gzip settings; location /backend/ proxies to swetrix-api:5005 with full proxy headers and buffering; primary location / proxies to swetrix:3000 with HTTP/1.1 and proxy headers; removed separate 81/8081 server block.
Env / Scripts / Docs
.env.example, configure.sh, README.md
Replaced API_URL with BASE_URL and updated default to http://localhost (no trailing slash); configure.sh prompt/variable/output renamed to BASE_URL and trims trailing slash; README updated to reference BASE_URL for access instructions.

Sequence Diagram

sequenceDiagram
    participant Client
    participant nginx as nginx-proxy<br/>(port 80)
    participant api as swetrix-api<br/>(port 5005)
    participant app as swetrix<br/>(port 3000)

    Client->>nginx: HTTP request (port 80)
    
    rect rgba(200,220,240,0.5)
        Note over nginx: Route decision
        alt Path starts with /backend
            nginx->>api: proxy_pass to swetrix-api:5005 (HTTP/1.1)
            api-->>nginx: response
        else Other paths (root)
            nginx->>app: proxy_pass to swetrix:3000 (HTTP/1.1)
            app-->>nginx: response
        end
    end
    
    nginx-->>Client: HTTP response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A hop, a tweak, the ports moved in,
Proxy listens where the requests begin,
/backend greets the API door,
Root forwards front-end like before,
Rabbit cheers — configs aligned with a grin.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: introducing nginx as a reverse proxy to handle and route requests to both the API and Web UI services.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

Fix all issues with AI Agents 🤖
In @nginx/config:
- Around line 6-12: The /api/ location block is missing several proxy headers
and operational directives; inside the location /api/ block add proxy_set_header
X-Forwarded-Proto $scheme and proxy_set_header Upgrade $http_upgrade along with
proxy_set_header Connection $connection_upgrade (and ensure you add the
top-level map $http_upgrade -> $connection_upgrade if WebSocket support is
required), and also add client_max_body_size with an appropriate limit plus
timeout directives proxy_connect_timeout, proxy_send_timeout, and
proxy_read_timeout to make the proxy resilient.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
nginx/config (1)

14-20: Add missing proxy headers and consider operational directives.

The root location has the same gaps as the /api/ location:

Major concerns:

  • X-Forwarded-Proto header missing (critical for HTTPS setups)
  • Connection and Upgrade headers missing (required if the frontend uses WebSockets for real-time updates)

Recommended improvements:

  • Add client_max_body_size directive
  • Add timeout configurations for better reliability
🔎 Proposed enhancements
  location / {
    proxy_pass http://swetrix:3000;
    proxy_http_version 1.1;
    proxy_set_header Host $host;
    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;
+   proxy_set_header X-Forwarded-Host $host;
+   
+   # WebSocket support (if needed)
+   proxy_set_header Upgrade $http_upgrade;
+   proxy_set_header Connection $connection_upgrade;
+   
+   # Operational settings
+   client_max_body_size 10M;
+   proxy_connect_timeout 60s;
+   proxy_send_timeout 60s;
+   proxy_read_timeout 60s;
  }
🧹 Nitpick comments (2)
compose.yaml (2)

117-119: Replace deprecated links with depends_on.

The links directive is deprecated in Docker Compose v2+. Use depends_on instead for service dependencies, which provides the same networking functionality with clearer semantics.

🔎 Proposed refactor
  nginx-proxy:
    image: nginx:1.29.4-alpine
    restart: always
-   links:
-     - "swetrix-api"
-     - "swetrix"
+   depends_on:
+     swetrix:
+       condition: service_healthy
+     swetrix-api:
+       condition: service_healthy
    ports:
      - "80:80"

114-123: Consider adding a health check for the nginx-proxy service.

Adding a health check improves service orchestration and enables better monitoring of the reverse proxy.

🔎 Proposed enhancement
  nginx-proxy:
    image: nginx:1.29.4-alpine
    restart: always
    links:
      - "swetrix-api"
      - "swetrix"
    ports:
      - "80:80"
    volumes:
      - ./nginx/config:/etc/nginx/conf.d/default.conf
+   healthcheck:
+     test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://localhost:80/ || exit 1"]
+     interval: 30s
+     timeout: 10s
+     retries: 3
+     start_period: 10s
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b3c8db and f9af660.

📒 Files selected for processing (2)
  • compose.yaml
  • nginx/config
🔇 Additional comments (3)
nginx/config (1)

2-3: LGTM! Standard listen configuration.

The IPv4 and IPv6 listen directives on port 80 are correctly configured for the single-domain architecture.

compose.yaml (2)

9-9: LGTM! API_URL correctly configured for single-domain setup.

The hardcoded /api value aligns with the nginx routing configuration that proxies /api/ requests to the backend API service.


115-115: No action required. The image version nginx:1.29.4-alpine is valid—nginx 1.29.4 (mainline) was released on December 9, 2025. If using a mainline release in production is a concern, consider pinning to the latest stable version (1.28.1 as of January 2026) instead, but the current version specification itself is not invalid.

@Blaumaus
Copy link
Member

Blaumaus commented Jan 5, 2026

wow that look fantastic! thanks for the PR, will be merged before the v5 release soon!

@Blaumaus Blaumaus self-assigned this Jan 13, 2026
Copy link

@coderabbitai coderabbitai bot left a 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

🤖 Fix all issues with AI agents
In `@compose.yaml`:
- Line 9: The frontend is configured with API_URL=/backend but nginx only
proxies location /api/ to the API service, causing requests to /backend to hit
the frontend; fix by either changing API_URL to "/api" in the compose
environment (update the API_URL value) or add a matching nginx location block
(add location /backend { proxy_pass http://swetrix-api:5005/; } in nginx/config)
so that requests to /backend are forwarded to swetrix-api:5005.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@compose.yaml`:
- Line 9: In the swetrix-fe container environment block in compose.yaml, replace
the unsupported environment key BASE_URL with API_URL so the frontend can locate
the backend (i.e., change the mapping from BASE_URL=${BASE_URL} to
API_URL=${BASE_URL} or set API_URL to the public Swetrix API base URL); update
the environment entry for the swetrix-fe service accordingly.

In `@README.md`:
- Around line 23-25: Replace the non-descriptive link text "here" that points to
https://docs.swetrix.com/selfhosting/configuring with a descriptive phrase like
"Self-hosting configuration guide" (or "Swetrix self-hosting configuration
guide") so the sentence "A table explaining what each value means can be found
here" becomes accessible and lint-compliant; update the markdown link text while
keeping the same URL and leave the surrounding sentences about BASE_URL and
docker unchanged to satisfy MD059.
🧹 Nitpick comments (1)
compose.yaml (1)

114-128: Consider mounting the nginx config read-only.
Prevents accidental writes in the container and signals intent.

🔒 Suggested change
-      - ./nginx/config:/etc/nginx/conf.d/default.conf
+      - ./nginx/config:/etc/nginx/conf.d/default.conf:ro

Copy link

@coderabbitai coderabbitai bot left a 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

🤖 Fix all issues with AI agents
In `@nginx/config`:
- Around line 19-27: The proxy headers use undefined variables
$connection_upgrade and $proxy_add_forwarded; fix by either (A) replacing the
non-standard Forwarded header (proxy_set_header Forwarded) with a supported
header or removing it and set Connection to a static value (e.g., "upgrade" when
appropriate), or (B, preferred) add a map for $http_upgrade to define
$connection_upgrade at the http level (via a map directive in the http block or
top of this conf.d include) and remove/replace $proxy_add_forwarded with a
constructed Forwarded value or omit it; update the proxy_set_header lines
(proxy_set_header Connection and proxy_set_header Forwarded) accordingly so
Connection uses the defined $connection_upgrade and Forwarded does not reference
$proxy_add_forwarded.
♻️ Duplicate comments (1)
nginx/config (1)

37-58: Same undefined variable issues apply here.

The $connection_upgrade and $proxy_add_forwarded issues flagged in the /backend/ block apply equally to this location block. Apply the same fix to both locations.

🧹 Nitpick comments (1)
nginx/config (1)

29-35: Consider adding timeout and body size limits.

The proxy buffering configuration looks good. However, for production resilience, consider adding:

  • client_max_body_size to limit upload sizes
  • Timeout directives to prevent hanging connections
♻️ Proposed additions
     # Proxy buffering (prevents truncated responses for large files)
     proxy_buffering                    on;
     proxy_buffer_size                  256k;
     proxy_buffers                      32 512k;
     proxy_busy_buffers_size            4m;
     proxy_max_temp_file_size           1024m;
+
+    # Operational limits
+    client_max_body_size               10m;
+    proxy_connect_timeout              60s;
+    proxy_send_timeout                 60s;
+    proxy_read_timeout                 60s;
   }

@Blaumaus Blaumaus changed the base branch from main to swetrix-v5 January 26, 2026 23:31
@Blaumaus Blaumaus merged commit c11ca8e into Swetrix:swetrix-v5 Jan 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants