feat(platform-api): support API-level upstreamDefinitions pool#2155
Conversation
…API schema Declare the API-level upstreamDefinitions pool (ReusableUpstream + UpstreamTimeout) so an upstream.ref can resolve against a named definition, mirroring the gateway's upstreamDefinitions shape. Schema only; model, converters, deployment-YAML emission, and validation follow.
mehara-rothila
changed the title
📝 WalkthroughSummaryThis PR adds OpenAPI schema declarations for API-level upstream definitions to the platform-api, enabling support for reusable named upstream configurations at the API level. ChangesFile: RESTAPI Schema EnhancementAdded a new optional New Schema ComponentsReusableUpstream - Defines a reusable named upstream definition with the following properties:
UpstreamTimeout - Provides upstream timeout configuration:
ImpactThe schema additions enable API-level upstream pooling with support for:
This is a schema-only change; follow-up work will include internal model generation, converter implementation, deployment YAML emission, and validation logic. WalkthroughThis OpenAPI specification update extends the REST API schema to support reusable upstream definitions. The change introduces two new schema components— 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@platform-api/src/resources/openapi.yaml`:
- Around line 8455-8464: UpstreamTimeout.connect's OpenAPI regex only allows a
single unit but the runtime uses time.ParseDuration (see
spec.upstreamDefinitions[%d].timeout.connect), so update the OpenAPI schema for
UpstreamTimeout.connect to either (A) broaden the pattern to match Go
time.ParseDuration format (allow multiple sequential value+unit segments,
decimals, and units like ns|us|µs|ms|s|m|h, optional leading minus) or (B)
explicitly document/validate that only single-unit durations are accepted;
locate the UpstreamTimeout.connect property in the OpenAPI file and change the
pattern or description accordingly.
- Around line 8427-8447: Update the ReusableUpstream schema so its basePath
property is constrained to a path-only form and each upstream.url is constrained
to host[:port]-only (no path/query/fragment) to match gateway validation:
tighten basePath (property name basePath) to require a leading "/" and a path
pattern (e.g., path segment characters only) and change
upstreams.items.properties.url to validate host[:port] (e.g., restrict
format/pattern to host with optional port) instead of generic uri; keep timeout
($ref: '`#/components/schemas/UpstreamTimeout`') unchanged.
🪄 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: c978ac83-b305-4917-85d3-8826d792c30e
📒 Files selected for processing (1)
platform-api/src/resources/openapi.yaml
| @@ -10228,6 +8409,60 @@ components: | |||
| description: Authentication value (API key, Bearer token, or Base64 encoded credentials for basic auth) | |||
| example: my-api-key-value | |||
|
|
|||
| ReusableUpstream: | |||
There was a problem hiding this comment.
| ReusableUpstream: | |
| UpstreamDefinition: |
What about UpstreamDefinition here?
cc: @tharindu1st
There was a problem hiding this comment.
@tharindu1st @renuka-fernando this is a 3-way name swap, not a simple rename.
Today in openapi.yaml:
| Schema | Role | Line |
|---|---|---|
Upstream |
the {main, sandbox} wrapper |
L8363 |
UpstreamDefinition |
the url/ref endpoint |
L8374 |
ReusableUpstream |
the pool entry | L8412 |
(Upstream is shared by API / LLMProvider / MCPServer / WebSub, so it has to stay.)
Two things point the same way:
- The gateway-controller names the pool entry
UpstreamDefinition(management-openapi.yaml#L4210). - Our internal model already uses
UpstreamEndpoint(endpoint) +UpstreamDefinition(pool) , seeplatform-api/src/internal/model/upstream.go.
We can't fully match the gateway since it has no wrapper (it inlines {main, sandbox}).
Proposal: keep the Upstream wrapper, then rename:
- endpoint
UpstreamDefinition→UpstreamEndpoint - pool entry
ReusableUpstream→UpstreamDefinition
This aligns the schema with both the gateway's pool name and our internal model. Good to proceed?
| maxLength: 100 | ||
| pattern: '^[a-zA-Z0-9\-_]+$' | ||
| example: my-upstream-1 | ||
| basePath: |
There was a problem hiding this comment.
pattern for basePath?
There was a problem hiding this comment.
The gateway-controller validates upstream basePath in code (#2106), not via an OpenAPI pattern. Happy to add a matching pattern here for client-side validation , or keep it server-side to match the gateway. Which do you prefer?
| @@ -8006,6 +6326,11 @@ components: | |||
| example: "2023-10-12T10:30:00Z" | |||
| upstream: | |||
| $ref: "#/components/schemas/Upstream" | |||
There was a problem hiding this comment.
upstream should have to support ref as well right?
There was a problem hiding this comment.
Yes , main/sandbox resolve to UpstreamDefinition (line 8374), which is oneOf: [url] | [ref], so main: { ref: my-upstream-1 } already works. Same shape as the gateway's Upstream in #2012.
2 participants
Purpose
The gateway (data plane) fully supports
upstreamDefinitions: a pool of reusable, named backend definitions that an API'supstream.refresolves against by name. The control plane (platform-api) never declared or emitted this pool, so on the control-plane path it is currently broken:upstream.refis half-wired. Thereffield is accepted, but the pool it must resolve against is never emitted in the deployment YAML, so the gateway rejects it ("no upstreamDefinitions provided").dynamic-endpointpolicy is inert. Its requiredtargetUpstreamparameter must name an entry inupstreamDefinitions; with no pool emitted, the policy has nothing to resolve against.basePath, and per-upstreamtimeout(all carried by the pool) cannot be expressed at all through the control plane.This PR begins closing that control-plane gap by declaring the API-level
upstreamDefinitionspool in the platform-api OpenAPI schema.Goals
Make the control plane able to express the same
upstreamDefinitionspool the gateway already consumes, so a single API definition can declare named backends and reference them by name, andref-based routing plus thedynamic-endpointpolicy work through the product, not only by deploying directly to the gateway.This PR is the first step: the OpenAPI schema declaration. The follow-up steps (code generation, internal model, converters, deployment-YAML emission, validation) wire it through end to end on this same branch.
Approach
Scope of this PR (step 1 of the feature): OpenAPI schema only. It adds the
upstreamDefinitionsarray on the REST API plus theReusableUpstreamandUpstreamTimeoutcomponent schemas. No operation-level (per-operation) upstream override is included; that is intentionally deferred.Design decisions:
ReusableUpstreammirrors the gateway's authoritativeUpstreamDefinition(name, optionalbasePath, optionaltimeout, andupstreams[]withurl+ optionalweight), so the deployment YAML platform-api will emit is parseable by the gateway unchanged.UpstreamTimeoutis a direct copy of the gateway's.UpstreamDefinitionfor the per-endpoint upstream wrapper (url|ref+auth). The reusable pool entry is therefore namedReusableUpstreamso the existing type is untouched and existing endpoints/models are not broken.allOf.upstreamDefinitionsis added once to theRESTAPIschema; becauseCreateRESTAPIRequestandUpdateRESTAPIRequestinherit fromRESTAPIviaallOf, the field is registered for create, update, and read.weight: minimum: 0to match the gateway (0 means "never route here"), rather than imposing an artificial floor in the control plane.Remaining work (follow-up commits on this branch):
generated.go(make generate).ReusableUpstream,UpstreamTimeout, andUpstreamDefinitionson the config structs).BuildAPIDeploymentYAML(the line that makes ref and dynamic-endpoint functional end to end).User stories
As an API developer, I want to declare a set of named, reusable upstream definitions on my API and reference them by name from the API's
upstream.ref, so that backend URLs live in one place and ref-based and dynamic-endpoint routing work through the control plane, not just by deploying directly to the gateway.Documentation
N/A for this schema-only step. User-facing documentation (under the Platform Gateway docs) will accompany the functional wiring.
Automation tests
Security checks
Samples
Once fully wired, an API will be able to declare a pool and reference it:
Test environment
platform-api/src/go.mod)go build ./cmd/main.goandmake testpass (no new code paths in this schema-only step)