Add per-operation upstream override for REST API operations - #2012
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-operation main and sandbox upstream references, shared validation and lookup utilities, deterministic API-scoped cluster names, REST/xDS routing support, persistence coverage, runtime tests, integration scenarios, and updated cluster identity documentation. ChangesPer-Operation Upstream Routing
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant APIValidator
participant RESTTransformer
participant XDSTranslator
participant Envoy
Client->>APIValidator: Submit operation upstream reference
APIValidator->>RESTTransformer: Accept validated configuration
RESTTransformer->>XDSTranslator: Pass referenced cluster and base path
XDSTranslator->>Envoy: Emit cluster-header route
Envoy-->>Client: Route request to referenced upstream
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@gateway/gateway-controller/api/management-openapi.yaml`:
- Around line 4139-4148: OperationUpstream currently allows an empty object
which is invalid; update the OperationUpstream schema (the OperationUpstream
object that defines properties main and sandbox) to require at least one of
those properties by adding an anyOf clause such as anyOf: - required: [main] -
required: [sandbox] (or an equivalent oneOf/anyOf expression) so the schema
enforces presence of main or sandbox and prevents {} being valid.
In `@gateway/gateway-controller/pkg/policy/builder.go`:
- Around line 185-186: The current condition treats any non-nil
op.Upstream.Sandbox as active; change it to perform a content-based check like
apiSandboxHasContent so empty sandbox objects are ignored. Introduce/compute a
per-op predicate (e.g., perOpSandboxHasContent) that mirrors the api-level
sandbox content test and use if apiSandboxHasContent || (op.Upstream != nil &&
perOpSandboxHasContent) to decide whether to append effectiveSandboxVHost.
In `@gateway/gateway-controller/pkg/xds/translator_test.go`:
- Around line 2061-2074: Update TestResolvePerOpUpstream_DedupSameURL to ensure
URL-stability by creating two distinct api.Upstream instances with different Url
values (e.g., "http://shared-svc:8080" and "http://shared-svc:8081") but the
same apiID, method, path and env, then call translator.resolvePerOpUpstream for
both and assert the returned cluster names are equal; keep the test name and use
the Translator.resolvePerOpUpstream and translator variable references so the
test still targets the same logic and verifies that cluster key derivation does
not depend on the exact URL string.
🪄 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: 4012d4ee-58df-4773-b82e-fa11e65290d8
📒 Files selected for processing (16)
gateway/gateway-controller/api/management-openapi.yamlgateway/gateway-controller/pkg/api/management/generated.gogateway/gateway-controller/pkg/config/api_validator.gogateway/gateway-controller/pkg/config/validator_test.gogateway/gateway-controller/pkg/models/runtime_deploy_config.gogateway/gateway-controller/pkg/policy/builder.gogateway/gateway-controller/pkg/policy/builder_test.gogateway/gateway-controller/pkg/transform/restapi.gogateway/gateway-controller/pkg/transform/restapi_test.gogateway/gateway-controller/pkg/utils/commonutils.gogateway/gateway-controller/pkg/utils/commonutils_test.gogateway/gateway-controller/pkg/xds/translator.gogateway/gateway-controller/pkg/xds/translator_test.gogateway/it/features/per-op-upstream-basic.featuregateway/it/features/per-op-upstream-ref.featuregateway/it/features/per-op-upstream-validation.feature
- OperationUpstream schema now requires at least one of main or sandbox
via anyOf, rejecting empty {} at the schema layer as defense in depth
alongside the validator.
- Policy builder per-op sandbox check is now content-based (trims and
checks Url/Ref non-empty) matching the apiSandboxHasContent pattern
used elsewhere. Fixes asymmetric pointer-only check that would emit
sandbox policy chains for empty sandbox wrappers.
- TestResolvePerOpUpstream_DedupSameURL now uses two distinct URLs with
identical apiID|METHOD|path|env, asserting cluster name independence
from URL. Pins the EDS-stable design contract.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gateway/gateway-controller/pkg/policyxds/policyxds_test.go (1)
120-120: 💤 Low valueConsider using a hash-like cluster key suffix for test realism.
The cluster key
"main_fixture"has the correctmain_prefix for an API-level main environment cluster, but the suffixfixturedoesn't reflect the production naming pattern introduced in this PR. According to the PR objectives, API-level clusters follow the pattern<env>_<sha256(apiID|env)[:8]>(e.g.,main_a1b2c3d4). While the current fixture name is functionally correct and clear, using a hash-like suffix such as"main_12ab34cd"would make the test more representative of production behavior.📝 Proposed update for more realistic fixture naming
Upstream: models.RouteUpstream{ - ClusterKey: "main_fixture", + ClusterKey: "main_12ab34cd", },UpstreamClusters: map[string]*models.UpstreamCluster{ - "main_fixture": { + "main_12ab34cd": { BasePath: "/",Also applies to: 132-132
🤖 Prompt for 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. In `@gateway/gateway-controller/pkg/policyxds/policyxds_test.go` at line 120, Update the test fixture cluster key strings that currently use "main_fixture" to a realistic hash-like suffix such as "main_12ab34cd" to match the production pattern; locate the occurrences in the policyxds_test setup where ClusterKey: "main_fixture" is declared (the test fixture variables/constants used in Policy XDS tests) and replace them with "main_12ab34cd" (or another 8-char hex-like hash) to make the test names reflect the <env>_<sha256(... )[:8]> convention.
🤖 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.
Nitpick comments:
In `@gateway/gateway-controller/pkg/policyxds/policyxds_test.go`:
- Line 120: Update the test fixture cluster key strings that currently use
"main_fixture" to a realistic hash-like suffix such as "main_12ab34cd" to match
the production pattern; locate the occurrences in the policyxds_test setup where
ClusterKey: "main_fixture" is declared (the test fixture variables/constants
used in Policy XDS tests) and replace them with "main_12ab34cd" (or another
8-char hex-like hash) to make the test names reflect the <env>_<sha256(...
)[:8]> convention.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8729c50c-d6aa-40f4-b0b0-efec0f9c903c
📒 Files selected for processing (13)
gateway/gateway-controller/cmd/controller/main.gogateway/gateway-controller/pkg/config/api_validator.gogateway/gateway-controller/pkg/policyxds/policyxds_test.gogateway/gateway-controller/pkg/transform/restapi.gogateway/gateway-controller/pkg/transform/restapi_test.gogateway/gateway-controller/pkg/utils/clusterkey/clusterkey.gogateway/gateway-controller/pkg/utils/clusterkey/clusterkey_test.gogateway/gateway-controller/pkg/utils/commonutils_test.gogateway/gateway-controller/pkg/xds/translator.gogateway/gateway-controller/pkg/xds/translator_test.gogateway/it/features/api-level-eds-stable.featuregateway/it/features/per-op-upstream-basic.featuregateway/it/suite_test.go
💤 Files with no reviewable changes (1)
- gateway/gateway-controller/pkg/utils/commonutils_test.go
✅ Files skipped from review due to trivial changes (2)
- gateway/gateway-controller/cmd/controller/main.go
- gateway/it/suite_test.go
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
gateway/gateway-controller/pkg/config/validator_test.go (1)
773-796: ⚡ Quick winIsolate invalid-timeout cases with subtests.
Using
requireinside the loop means the first failing case can stop execution before the next case is checked. Wrapping eachbadTimeoutint.Run(...)keeps both cases independently validated and easier to diagnose.Proposed test refactor
- for _, badTimeout := range []string{"0s", "-5s"} { + for _, badTimeout := range []string{"0s", "-5s"} { + t.Run(badTimeout, func(t *testing.T) { connect := badTimeout definitions := &[]api.UpstreamDefinition{ { Name: "my-upstream", Timeout: &api.UpstreamTimeout{ Connect: &connect, }, Upstreams: []struct { Url string `json:"url" yaml:"url"` Weight *int `json:"weight,omitempty" yaml:"weight,omitempty"` }{ { Url: "http://backend:8080", }, }, }, } errors := validator.validateUpstreamDefinitions(definitions) require.Len(t, errors, 1, "timeout %q must be rejected", badTimeout) assert.Equal(t, "spec.upstreamDefinitions[0].timeout.connect", errors[0].Field) assert.Contains(t, errors[0].Message, "must be a positive duration") + }) }🤖 Prompt for 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. In `@gateway/gateway-controller/pkg/config/validator_test.go` around lines 773 - 796, The test loop in validator_test.go uses require inside a for-range which can abort the whole loop on first failure; refactor by wrapping each badTimeout iteration in a subtest using t.Run(fmt.Sprintf("timeout=%s", badTimeout), func(t *testing.T) { ... }) and move the assertions (require.Len, assert.Equal, assert.Contains) into that subtest, capturing the loop variable (e.g., tt := badTimeout) before using it to build the definitions passed to validator.validateUpstreamDefinitions so each case runs and reports independently.
🤖 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 `@gateway/gateway-controller/api/management-openapi.yaml`:
- Around line 4136-4137: The schemas that currently use a $ref with a sibling
description (referencing "`#/components/schemas/RestAPIOperationUpstream`") must
be changed so the description is not a sibling of $ref; instead wrap the
reference in an allOf and move the description onto the enclosing schema object.
Concretely, replace occurrences where a property has "$ref:
'`#/components/schemas/RestAPIOperationUpstream`'" alongside "description" by
creating an object with "description: <same text>" and "allOf: [{ $ref:
'`#/components/schemas/RestAPIOperationUpstream`' }]" (apply this change for every
place referencing RestAPIOperationUpstream in the file).
In `@gateway/gateway-controller/pkg/config/api_validator.go`:
- Around line 656-667: The per-operation upstream ref checks (using
upstreamRefRegex and length >100 in the validate path that returns
ValidationError for field/refName) are stricter than names allowed by
validateUpstreamDefinitions, causing valid definition names to become
unreferencable; to fix, apply the same validation rules to
spec.upstreamDefinitions[*].name inside validateUpstreamDefinitions (reuse
upstreamRefRegex and the 100-character limit and return a ValidationError with
the same message format when a definition name violates them) OR remove the
extra constraints from the per-operation check so both places use the same
contract; update validateUpstreamDefinitions to reference the same
upstreamRefRegex and error messages (Field and Message) so names and refs are
aligned.
---
Nitpick comments:
In `@gateway/gateway-controller/pkg/config/validator_test.go`:
- Around line 773-796: The test loop in validator_test.go uses require inside a
for-range which can abort the whole loop on first failure; refactor by wrapping
each badTimeout iteration in a subtest using t.Run(fmt.Sprintf("timeout=%s",
badTimeout), func(t *testing.T) { ... }) and move the assertions (require.Len,
assert.Equal, assert.Contains) into that subtest, capturing the loop variable
(e.g., tt := badTimeout) before using it to build the definitions passed to
validator.validateUpstreamDefinitions so each case runs and reports
independently.
🪄 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: a0b21e0b-ca37-459a-b596-04dce4de1349
📒 Files selected for processing (18)
gateway/gateway-controller/api/management-openapi.yamlgateway/gateway-controller/pkg/api/management/generated.gogateway/gateway-controller/pkg/config/api_validator.gogateway/gateway-controller/pkg/config/validator_test.gogateway/gateway-controller/pkg/policy/builder.gogateway/gateway-controller/pkg/policy/builder_test.gogateway/gateway-controller/pkg/transform/restapi.gogateway/gateway-controller/pkg/transform/restapi_test.gogateway/gateway-controller/pkg/utils/upstreamref/upstreamref.gogateway/gateway-controller/pkg/utils/upstreamref/upstreamref_test.gogateway/gateway-controller/pkg/xds/translator.gogateway/gateway-controller/pkg/xds/translator_test.gogateway/gateway-controller/tests/integration/storage_test.gogateway/gateway-runtime/policy-engine/internal/kernel/translator.gogateway/gateway-runtime/policy-engine/internal/kernel/translator_test.gogateway/it/features/per-op-upstream-basic.featuregateway/it/features/per-op-upstream-ref.featuregateway/it/features/per-op-upstream-validation.feature
🚧 Files skipped from review as they are similar to previous changes (4)
- gateway/gateway-controller/pkg/policy/builder.go
- gateway/gateway-controller/pkg/policy/builder_test.go
- gateway/gateway-controller/pkg/transform/restapi.go
- gateway/gateway-controller/pkg/xds/translator.go
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
4cfd850 to
4caca38
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
f707aad to
afa9253
Compare
f7bd4f1 to
07fd3e4
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
e847c52 to
bde4caf
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
gateway/gateway-controller/pkg/config/api_validator.go (2)
34-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo parallel regexes enforce the same UpstreamReference contract.
upstreamRefRegex(used byvalidateUpstreamRefNamefor API-level and per-op refs) and the pre-existingupstreamDefinitionNameRegex(used inline invalidateUpstreamDefinitionsList, ~line 300) both encode^[a-zA-Z0-9\-_]+$, but are separate compiled regexes with separate, differently-worded error messages ("must not exceed 100 characters" / "must match pattern ..." vs. "must be 1-100 characters" / "must match ^[a-zA-Z0-9\-_]+$ ..."). This is exactly the kind of contract-drift risk a prior review flagged for this ref/name pair — if one pattern is updated later, the other can silently diverge.Consider having
validateUpstreamDefinitionsListcallvalidateUpstreamRefNamefordef.Nametoo, so there is a single source of truth for the ref/name contract (accepting the resulting message-text change, and updatingTestValidateUpstreamDefinitions_NameRules's expected substrings accordingly).🤖 Prompt for 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. In `@gateway/gateway-controller/pkg/config/api_validator.go` around lines 34 - 54, Update validateUpstreamDefinitionsList to validate each def.Name through validateUpstreamRefName instead of applying upstreamDefinitionNameRegex inline, making upstreamRefRegex the single source of truth for upstream reference and definition-name validation. Remove the redundant definition-name validation path if no longer needed, and update TestValidateUpstreamDefinitions_NameRules expected error substrings to match the shared validator messages.
436-447: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
upstreamRefResolveshelper
upstreamRefResolvesis no longer referenced in the repository, and its formerMCP/LLMcallers can move to the sharedupstreamref.FindByNamepath already used byvalidateUpstreamRef.🤖 Prompt for 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. In `@gateway/gateway-controller/pkg/config/api_validator.go` around lines 436 - 447, Remove the unused upstreamRefResolves helper from api_validator.go. Update its former MCP/LLM callers to use the shared upstreamref.FindByName path already used by validateUpstreamRef, preserving the existing upstream-name resolution behavior.
🤖 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.
Nitpick comments:
In `@gateway/gateway-controller/pkg/config/api_validator.go`:
- Around line 34-54: Update validateUpstreamDefinitionsList to validate each
def.Name through validateUpstreamRefName instead of applying
upstreamDefinitionNameRegex inline, making upstreamRefRegex the single source of
truth for upstream reference and definition-name validation. Remove the
redundant definition-name validation path if no longer needed, and update
TestValidateUpstreamDefinitions_NameRules expected error substrings to match the
shared validator messages.
- Around line 436-447: Remove the unused upstreamRefResolves helper from
api_validator.go. Update its former MCP/LLM callers to use the shared
upstreamref.FindByName path already used by validateUpstreamRef, preserving the
existing upstream-name resolution behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e1adef5-8064-4f6a-a288-91eab9c5da26
📒 Files selected for processing (9)
gateway/gateway-controller/api/management-openapi.yamlgateway/gateway-controller/pkg/api/management/generated.gogateway/gateway-controller/pkg/config/api_validator.gogateway/gateway-controller/pkg/config/validator_test.gogateway/gateway-controller/pkg/xds/translator.gogateway/gateway-controller/pkg/xds/translator_test.gogateway/gateway-runtime/policy-engine/internal/kernel/translator_test.gogateway/it/features/per-op-upstream.featuregateway/it/suite_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- gateway/gateway-runtime/policy-engine/internal/kernel/translator_test.go
- gateway/gateway-controller/pkg/api/management/generated.go
- gateway/gateway-controller/pkg/xds/translator.go
- gateway/gateway-controller/pkg/xds/translator_test.go
…ted contract The management validator deliberately accepts a zero connect timeout and enforces only the duration format, so deploying a definition with connect 0s returns 201. The scenario previously expected a 400 from the stricter pre-rebase contract and failed once CI ran the suite against the merged main.
…ne feature file Merge per-op-upstream-basic, per-op-upstream-ref, per-op-upstream-validation, and api-level-url-stable into a single features/per-op-upstream.feature and register it once in suite_test.go. All 26 scenarios are preserved verbatim; the four files already ran together in the same suite, so this is behavior-neutral.
Add scenarios proving per-operation upstream refs apply to Gateway-API-style match operations (method + path.value + header matchers): a main ref on a match operation with a sibling match op falling back to the API-level upstream, a header matcher selecting the per-op ref on a shared path, and a sandbox ref on a match operation. Confirms the transform resolves the route from the match block and still applies the per-operation cluster.
Note that the ref pattern backs only API-level and per-operation upstream refs, and that definition names are validated separately, so the two checks are not mistaken for one shared rule.
The management validator accepted a zero connect timeout while the transformer requires a positive value, so a definition carrying "0s" was accepted at deploy time and then failed when the deployment was translated. Because runtime configs are rebuilt from stored configurations on startup, such a definition also stopped the controller from starting again. Validate the connect timeout as positive alongside the existing format check, and cover the rejection in the validator unit test and the integration scenario.
bde4caf to
c400a6f
Compare
Summary
Adds a per-operation
upstreamoverride on REST API operations: an operation can route itsmainand/orsandboxtraffic to a different backend than the API-level upstream. Per-op targets are ref-only: each references a named entry inspec.upstreamDefinitionsrather than carrying an inline URL. Operations without a per-op upstream fall back to the API-level upstream, exactly as before.A per-op route reuses the referenced
upstreamDefinition's cluster (one cluster per definition, so operations sharing arefshare it) rather than minting a cluster per operation. API-levelmain/sandboxcluster names are URL-stable (derived from a hash of the API's identity, not the URL). Both are URL-independent, so URL edits update Envoy endpoints in place instead of recreating clusters.Why
Real-world APIs often need individual operations to reach different backend services. Today that needs path-based workarounds or splitting into multiple APIs. This lets the API definition declare per-operation routing directly, while backend URLs stay defined once in
upstreamDefinitionsand are referenced by name.Design decisions
Ref-only at the operation level.
operations[].upstream.main/.sandboxcarry only arefto a namedupstreamDefinition, with no inlineurland no per-target timeout/hostRewrite. Backends are declared once inspec.upstreamDefinitions; connect timeout lives on the definition. (API-levelupstreamstill accepts eitherurlorref, unchanged.)Per-op routes reuse the definition cluster. A per-op
refdoes not create a new cluster. It points the operation's route at the cluster the referencedupstreamDefinitionalready owns (upstream_<kind>_<apiID>_<defName>, built once per definition). N operations sharing arefcost one cluster, not N, and the route inherits the definition's authoritative base path.cluster_headerstays on with that cluster as the default, so a dynamic-endpoint policy can still override the operation (precedence: op-policy > api-policy > per-op ref > api-level upstream).URL-stable API-level cluster naming. API-level
main/sandboxcluster names derive from the fullsha256(apiID)(64 hex), viaclusterkey.HashedName(env, apiID), producingmain_<hash>/sandbox_<hash>. Theenvis a plaintext prefix, not part of the hash, somainandsandboxshare the same hash. The URL is intentionally excluded, so a URL edit keeps the same cluster name and Envoy updates its endpoints in place (no connection drain) rather than recreating the cluster. The clusters areSTRICT_DNSwith an inline load assignment. Rollout: a one-time cluster rebuild on the first config push after upgrade (URL-derived to identity-based names), handled gracefully by Envoy's CDS warming; stable thereafter.Single source of truth. Ref resolution and connect-timeout parsing go through
pkg/utils/upstreamref(FindByName,ParseConnectTimeout); API-level cluster-name hashing goes throughpkg/utils/clusterkey(Hashreturns the full SHA-256 hash,HashedNamejoins a prefix to it). Both are consumed by the validator, the RDC transformer, and the xDS translator so they cannot drift. The policy-engine kernel (a separate Go module that cannot importclusterkey) reconstructs the definition cluster name for dynamic-endpoint targets; a kernel unit test pins its output byte-for-byte toclusterkey.DefinitionNameso the two modules cannot drift either.What changed (gateway-controller layer)
management-openapi.yaml): optionalupstreamonOperationresolving toOperationUpstream(main/sandbox), each an inline ref-only object ({ ref }, no named target schema). Wrapper and each sub-field locked withadditionalProperties: false, plusminProperties: 1on the wrapper (an emptyupstream: {}is rejected).api_validator.go): rejects an emptyupstream: {}wrapper; requires at least one ofmain/sandbox; rejects arefthat does not resolve; and enforces one name contract (^[a-zA-Z0-9\-_]+$, max 100 chars) on per-op refs, API-level refs, andupstreamDefinitionnames so every valid definition name is referenceable.transform/restapi.go) and xDS translator (xds/translator.go): per-op routes reuse the referenced definition's cluster (upstream_<kind>_<apiID>_<defName>, built once per definition); API-level clusters use URL-stable namesmain_<hash>/sandbox_<hash>; operations without a per-op upstream fall back to the API-level cluster.pkg/utils/upstreamref,pkg/utils/clusterkey): new leaf packages.upstreamrefhandles ref resolution and connect-timeout parsing; the stdlib-onlyclusterkeyhandles URL-stable API-level cluster-key hashing (Hash/HashedName) and definition cluster naming (DefinitionName).Cluster names: before / after
For an API with UUID
0190b3e2-7b1c-7c2a-9b3d-1a2b3c4d5e6f(sha256(apiID)=54a9b3e5ce2b6ccb97168e5948a66f48e084213b38eb8c7dc01c6f624a63c2f2):cluster_http_default-backend_8080(URL-derived)main_54a9b3e5ce2b6ccb97168e5948a66f48e084213b38eb8c7dc01c6f624a63c2f2(identity-based)cluster_http_sandbox-backend_8080(URL-derived)sandbox_54a9b3e5ce2b6ccb97168e5948a66f48e084213b38eb8c7dc01c6f624a63c2f2(identity-based)user-serviceupstream_RestApi_0190b3e2-7b1c-7c2a-9b3d-1a2b3c4d5e6f_user-serviceclusterkey.DefinitionNamehelper)Only the API-level names change. Editing a backend URL previously renamed the API-level cluster, so Envoy dropped and recreated it; with identity-based names the cluster name never changes and Envoy updates the endpoints in place. Definition cluster names were already URL-independent.
Test coverage
pkg/config,pkg/transform,pkg/xds,pkg/utils/upstreamref, andpkg/utils/clusterkey: deterministic hash tests (Hash), validator rejection tests (empty/unknown/pattern/length refs, with exact error reasons), URL-stable cluster-name contract tests (same name across a URL edit), definition-cluster reuse and cross-API isolation tests, per-op route tests asserting the exact referenced definition cluster, a policy chain-order test (API-level policies precede operation-level, so the operation-level policy wins as the last write), and a policy-engine kernel test pinning thex-target-upstreamvalue for a dotted/coloned definition name byte-for-byte to the controller'sclusterkey.DefinitionName.Example
One API exercising every capability: API-level fallback, per-op main override, per-op main+sandbox overrides, definition base path and connect timeout, host rewrite, and a dynamic-endpoint policy overriding a per-op ref.
Routing result:
GET /productsgoes todefault-backend(no per-op upstream, API-level fallback)GET /users/{id}goes touser-serviceunder/api/v2(per-op main ref, inherits the definition's basePath)POST /ordersgoes toorder-serviceon main anduser-serviceon sandbox (per-op overrides per environment)GET /recommendationsgoes topromo-service(the operation-level dynamic-endpoint policy overrides the per-op ref)