feat: add management endpoint forwarding to GenericRouter (closes #276)#3631
feat: add management endpoint forwarding to GenericRouter (closes #276)#3631dungnthe130669 wants to merge 1 commit into
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ManagementConfig-based passthrough routing and provider-specific management route factories; RegisterRoutes registers management handlers, createManagementHandler builds filtered passthrough requests, and OpenAI, Anthropic, and GenAI routers append their management routes. ChangesManagement Endpoints Support
Sequence DiagramsequenceDiagram
participant Client
participant RegisterRoutes
participant createManagementHandler
participant handlePassthroughNonStream
participant UpstreamProvider
Client->>RegisterRoutes: HTTP request to management route
RegisterRoutes->>createManagementHandler: detected ManagementConfig
createManagementHandler->>createManagementHandler: filter headers, strip prefix, resolve provider
createManagementHandler->>handlePassthroughNonStream: BifrostPassthroughRequest
handlePassthroughNonStream->>UpstreamProvider: forward with provider auth
UpstreamProvider-->>handlePassthroughNonStream: response
handlePassthroughNonStream-->>Client: proxied response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Confidence Score: 5/5The change is a safe, additive layer that reuses the well-tested passthrough infrastructure; no existing routes are modified or removed. The implementation correctly delegates to handlePassthroughNonStream, which owns context cancellation via its own defer cancel(). Header filtering follows the same allow/drop rules already used in handlePassthrough. New route paths (/v1/models/{model_id}, /v1/usage, /v1/organizations) do not overlap with any pre-existing registered routes. The GenAI stub returning nil is a valid no-op append in Go and is clearly documented. No files require special attention. Important Files Changed
Reviews (4): Last reviewed commit: "feat: add management endpoint forwarding..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@transports/bifrost-http/integrations/openai.go`:
- Around line 2923-2929: The new route block adding a management GET for
pathPrefix + "/v1/models" duplicates the existing
CreateOpenAIListModelsRouteConfigs registration and causes ambiguous routing;
remove this duplicate RouteConfig (the block with Type: RouteConfigTypeOpenAI,
Path: pathPrefix + "/v1/models", Method: "GET", ManagementConfig: mgmtCfg) or
change it to a distinct management-only path, and instead rely on
CreateOpenAIListModelsRouteConfigs for the public /v1/models handler so there is
only one registration for that method/path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3584ad73-da8c-48e0-b139-e1e5fa282ebe
📒 Files selected for processing (2)
transports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
…imhq#276) - Add ManagementConfig struct with Provider and StripPrefix fields - Add createManagementHandler() using existing Passthrough infrastructure - Register OpenAI management routes: GET/DELETE /v1/models/{model_id}, GET /v1/usage, GET /v1/organizations - Register Anthropic management routes: GET /v1/models/{model_id}, GET /v1/usage - Add empty GenAI stub (existing routes cover model listing) - No duplicate routes: GET /v1/models (list) stays with CreateOpenAIListModelsRouteConfigs Closes maximhq#276
dungnthe130669
force-pushed
the
feat/management-endpoints-276
branch
from
d91c0e2 to
f9b5f98
Compare
2 participants
Summary
Closes #276
Bifrost currently only supports operational POST endpoints (completions, embeddings, etc.) but lacks management GET endpoints needed for a complete drop-in replacement.
This PR adds management endpoint forwarding to
GenericRouterusing the existingPassthroughinfrastructure — no new HTTP client needed.Changes
router.goManagementConfigstruct withProviderandStripPrefixfieldsManagementConfigfield toRouteConfigcreateManagementHandler()— strips prefix, buildsBifrostPassthroughRequest, proxies viahandlePassthroughNonStreamRegisterRoutesbefore normal Bifrost pipeline validationopenai.goGET /openai/v1/models/{model_id}— retrieve model detailsDELETE /openai/v1/models/{model_id}— delete fine-tuned modelGET /openai/v1/organizations— list organizationsGET /openai/v1/usage— usage statsanthropic.goGET /anthropic/v1/models/{model_id}— retrieve model detailsGET /anthropic/v1/usage— usage statsgenai.goHow it works
createManagementHandlerreuses the existinghandlePassthroughNonStreampath:/openai) from incoming pathBifrostPassthroughRequestwith provider + auth from configured keysTesting
The repo requires
go 1.26.1+go.workworkspace setup. Logic was verified to have zero new compilation errors beyond pre-existing workspace setup issues (confirmed viagit stashbaseline comparison).