Skip to content

chore: simplifies http routing#121

Merged
jland-redhat merged 1 commit intoopendatahub-io:mainfrom
bartoszmajsak:deployment/chore/duplicate-route
Oct 2, 2025
Merged

chore: simplifies http routing#121
jland-redhat merged 1 commit intoopendatahub-io:mainfrom
bartoszmajsak:deployment/chore/duplicate-route

Conversation

@bartoszmajsak
Copy link
Copy Markdown
Collaborator

@bartoszmajsak bartoszmajsak commented Oct 2, 2025

We only need one route for maas-api, routes for models are handled by LLMInferenceService controller, so creating separate one can cause confusion.

Summary by CodeRabbit

  • New Features

    • Unified access under /maas-api with automatic URL rewrite for cleaner URLs.
    • Gateway-level authentication is enforced for maas-api.
  • Refactor

    • Renamed the maas-api route and removed host-based routing; previous maas-api. URLs are no longer served—use the /maas-api path instead.
    • Removed domain-based routes for certain model endpoints (e.g., qwen3 and simulator), consolidating traffic through the primary gateway path.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 2, 2025

Walkthrough

Removed several networking HTTPRoute manifests, renamed and changed the maas-api HTTPRoute to use PathPrefix /maas-api with a URLRewrite, removed host-based routing and backend namespace, replaced an auth-override resource with maas-auth-policy, and deleted a gateway-class kustomization and its model-routing routes.

Changes

Cohort / File(s) Summary
maas-api HTTPRoute update
deployment/base/maas-api/httproute.yaml
Renamed resource (maas-api-domain-route → maas-api-route); removed hostnames; changed match to PathPrefix "/maas-api"; added URLRewrite ReplacePrefixMatch to "/"; removed backendRef namespace.
maas-api auth config swap
deployment/base/maas-api/kustomization.yaml, deployment/base/maas-api/auth-override.yaml
Removed auth-override.yaml from kustomization and added maas-auth-policy.yaml; deleted the AuthPolicy allow-all manifest.
Networking maas-api route removal
deployment/base/networking/httproute.yaml, deployment/base/networking/kustomization.yaml
Deleted the standalone HTTPRoute for maas-api and removed it from networking kustomization resources.
Gateway-class kustomization removal
deployment/base/networking/gateway-class/kustomization.yaml
Deleted the gateway-class kustomization file that referenced model-routing.yaml.
Model routing deletions
deployment/base/networking/gateway-class/model-routing.yaml
Removed two domain-based HTTPRoute manifests (Qwen3 and Simulator) routing to llm backends.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant G as Gateway
  participant R as HTTPRoute (maas-api-route)
  participant S as Service (maas-api:8080)

  C->>G: HTTP GET /maas-api/...
  G->>R: Match PathPrefix "/maas-api"
  Note over R: URLRewrite ReplacePrefixMatch "/maas-api" → "/"
  R->>S: Forward request with path "/..."
  S-->>G: Response
  G-->>C: Response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I twitch my nose and trim the trail,
Swap old routes for a cleaner tale.
Prefix rewrites and hosts set free,
Hop—requests find home, as quick as can be. 🐇✨

Pre-merge checks and finishing touches

✅ 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 “chore: simplifies http routing” directly reflects the primary intent of the changeset by highlighting the removal of duplicate HTTP routes and consolidation of routing configuration, and it is concise, clear, and specific enough for teammates to understand the main change at a glance.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3d6dca0 and c2f47b9.

📒 Files selected for processing (7)
  • deployment/base/maas-api/auth-override.yaml (0 hunks)
  • deployment/base/maas-api/httproute.yaml (1 hunks)
  • deployment/base/maas-api/kustomization.yaml (1 hunks)
  • deployment/base/networking/gateway-class/kustomization.yaml (0 hunks)
  • deployment/base/networking/gateway-class/model-routing.yaml (0 hunks)
  • deployment/base/networking/httproute.yaml (0 hunks)
  • deployment/base/networking/kustomization.yaml (0 hunks)
💤 Files with no reviewable changes (5)
  • deployment/base/networking/kustomization.yaml
  • deployment/base/networking/gateway-class/model-routing.yaml
  • deployment/base/networking/httproute.yaml
  • deployment/base/maas-api/auth-override.yaml
  • deployment/base/networking/gateway-class/kustomization.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • deployment/base/maas-api/httproute.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-12T22:19:36.331Z
Learnt from: bartoszmajsak
PR: opendatahub-io/maas-billing#73
File: maas-api/deploy/overlays/dev/patches/sa-token-provider.yaml:5-12
Timestamp: 2025-09-12T22:19:36.331Z
Learning: In maas-api/deploy/overlays/dev/patches/sa-token-provider.yaml, the patch file exists but is intentionally not referenced in the kustomization.yaml. This is by design - the patch is meant to be applied conditionally during development workflows (e.g., via "kustomize edit add patch" commands) rather than being permanently included in the dev overlay.

Applied to files:

  • deployment/base/maas-api/kustomization.yaml

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.

@bartoszmajsak bartoszmajsak force-pushed the deployment/chore/duplicate-route branch from 0141cc0 to 3d6dca0 Compare October 2, 2025 06:48
Copy link
Copy Markdown
Collaborator

@dmytro-zaharnytskyi dmytro-zaharnytskyi left a comment

Choose a reason for hiding this comment

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

legit

We only need one route for maas-api, routes for models are handled by LLMInferenceService controller, so creating separate one can cause confusion.

Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>

; Conflicts:
;	deployment/base/networking/kustomization.yaml

Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>
@bartoszmajsak bartoszmajsak force-pushed the deployment/chore/duplicate-route branch from 3d6dca0 to c2f47b9 Compare October 2, 2025 14:29
Copy link
Copy Markdown
Contributor

@jland-redhat jland-redhat left a comment

Choose a reason for hiding this comment

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

LGTM

@jland-redhat jland-redhat merged commit 2743423 into opendatahub-io:main Oct 2, 2025
2 checks passed
SB159 pushed a commit to SB159/maas-billing that referenced this pull request Oct 15, 2025
We only need one route for maas-api, routes for models are handled by LLMInferenceService controller, so creating separate one can cause confusion.



; Conflicts:
;	deployment/base/networking/kustomization.yaml

Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>
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.

3 participants