fix reverse proxy mapping and traefik disabled #1539
Conversation
Summary by CodeRabbit
WalkthroughUpdated Docker Compose configs: added a Traefik-disabling label for the createbuckets service in the dev essentials file, and changed the frontend service to expose container port 80 (not 3000) with a corresponding Traefik load balancer port label in the main compose file. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
|
Francesco Marchi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![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
🧹 Nitpick comments (5)
docker/docker-compose-dev-essentials.yaml (2)
139-151: RabbitMQ may still be auto-exposed via Traefik.Since
providers.docker.exposedbydefaultisn’t disabled in Traefik, services withouttraefik.enable=falsecan be exposed implicitly. Recommend explicitly disabling for RabbitMQ.Apply:
rabbitmq: image: rabbitmq:4.1.0-management container_name: unstract-rabbitmq hostname: unstract-rabbit restart: unless-stopped env_file: - ./essentials.env ports: - "5672:5672" # AMQP port - "15672:15672" # Management UI port volumes: - rabbitmq_data:/var/lib/rabbitmq + labels: + - traefik.enable=false
77-81: Harden Traefik discovery defaults.To prevent unintentional exposure, disable Docker “exposed by default” and rely on explicit labels.
Apply:
- command: --api.insecure=true + command: --api.insecure=true --accesslog=true --log.level=INFO - --providers.docker=true --providers.docker.network=unstract-network + --providers.docker=true --providers.docker.network=unstract-network --providers.docker.exposedbydefault=false --providers.file.filename=/proxy_overrides.yaml --providers.file.watch=truedocker/docker-compose.yaml (3)
33-35: Mirror the rule fix for backend for consistency.Backend uses
PathPrefix(/api/v1,/deployment)as well. Recommend the same explicit OR.Apply:
- - traefik.http.routers.backend.rule=Host(`frontend.unstract.localhost`) && PathPrefix(`/api/v1`, `/deployment`) + - traefik.http.routers.backend.rule=Host(`frontend.unstract.localhost`) && (PathPrefix(`/api/v1`) || PathPrefix(`/deployment`))
175-177: Optional: set explicit router priorities.Not strictly required, but helps avoid ambiguity if rules evolve.
- traefik.http.routers.frontend.rule=Host(`frontend.unstract.localhost`) && !(PathPrefix(`/api/v1`) || PathPrefix(`/deployment`)) + - traefik.http.routers.frontend.priority=10And for backend (above):
- traefik.http.routers.backend.rule=Host(`frontend.unstract.localhost`) && (PathPrefix(`/api/v1`) || PathPrefix(`/deployment`)) + - traefik.http.routers.backend.priority=20
167-168: Optional: avoid publishing 3000 when Traefik is present.If you only access the frontend via Traefik on host port 80, drop the host port exposure to reduce conflicts. Keep it only if you need direct access on 3000.
- ports: - - "3000:80" + # ports: + # - "3000:80" # Uncomment if you need direct access without Traefik
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
docker/docker-compose-dev-essentials.yaml(1 hunks)docker/docker-compose.yaml(2 hunks)
🔇 Additional comments (2)
docker/docker-compose-dev-essentials.yaml (1)
63-64: Good call disabling Traefik on createbuckets.This avoids accidental router/service creation for a short‑lived, non-HTTP job.
docker/docker-compose.yaml (1)
168-168: Confirm frontend listens on port 80 inside the image.Sandbox couldn't run Docker (docker: command not found); verification required locally. Run and paste results:
docker image inspect "unstract/frontend:${VERSION}" | jq '.[0].ContainerConfig.ExposedPorts, .[0].Config.ExposedPorts'
docker run --rm --name probe-frontend -d -p 8088:80 "unstract/frontend:${VERSION}" && sleep 3 && curl -i http://localhost:8088/ | sed -n '1,1p' && docker rm -f probe-frontendLocation: docker/docker-compose.yaml (around line 168) — current mapping: - "3000:80"
If the container actually serves on 3000, change compose to "- "3000:3000"" and update Traefik to forward to port 3000.
|
@Fra5792 the changes as such looks okay. Do you have any associated issues with this? Also can you add some screenshots of before and after of these changes? Also please accept the CLA. |
|
3 participants
What
An issue was causing reverse-proxy a result of bad gateway
Why
Traefik and port mapping issue
I have read and understood the Contribution Guidelines.