Send provider-template map to policy engine #818
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n for LLM provider/proxy scenarios
Contributor
WalkthroughAdds provider-to-template lazy-resource handling across the gateway: publishes/removes provider→template mappings in the gateway controller, threads provider metadata through XDS routes, exposes lazy resources in policy-engine config dumps, and adds integration tests exercising lazy-resource xDS synchronization and provider-template mapping behavior. Changes
Sequence DiagramsequenceDiagram
actor User
participant GWCtrl as Gateway Controller
participant LRS as Lazy Resource Store
participant XDS as XDS Translator
participant PE as Policy Engine
participant ConfigDump as Config Dump
User->>GWCtrl: Deploy LLM Provider (providerName, templateHandle)
GWCtrl->>LRS: publishProviderTemplateMappingAsLazyResource(providerName, templateHandle, correlationID)
LRS-->>GWCtrl: ACK (mapping stored)
User->>XDS: Create route for API
XDS->>XDS: extractProviderName() / extractTemplateHandle()
XDS->>XDS: createRoute(..., provider_name metadata)
XDS-->>PE: Envoy route with provider_name & template_handle metadata
PE->>ConfigDump: Dump configuration
ConfigDump->>LRS: dumpLazyResources()
LRS-->>ConfigDump: ResourcesByType (incl. ProviderTemplateMapping)
ConfigDump-->>PE: ConfigDumpResponse{..., LazyResources}
User->>GWCtrl: Delete LLM Provider (providerName)
GWCtrl->>LRS: removeProviderTemplateMappingLazyResource(providerName, correlationID)
LRS-->>GWCtrl: ACK (mapping removed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@gateway/gateway-controller/pkg/utils/llm_deployment.go`:
- Around line 732-742: The removal routine
removeProviderTemplateMappingLazyResource currently calls
lazyResourceManager.RemoveResource(providerName, correlationID) which only
removes by ID and risks colliding with other resource types (via
store.RemoveByID); update the call so the removal filters by ResourceType (use
the same ResourceType constant used when publishing, e.g.,
ResourceTypeProviderTemplateMapping) or add/use a RemoveResourceByType(id,
resourceType, correlationID) method so only provider-template mapping resources
are deleted, and change the empty providerName handling to mirror
publishProviderTemplateMappingAsLazyResource by returning an error for empty
providerName instead of silently returning nil.
🧹 Nitpick comments (3)
gateway/gateway-controller/pkg/xds/translator.go (2)
437-445: Consider consolidating LLM metadata extraction.
extractTemplateHandleandextractProviderNameboth traverseSourceConfiguration(with JSON marshal/unmarshal). A single helper that parses once and returns both values would reduce repeated work per config.
1005-1112: LGTM: provider_name propagated into route metadata.
Reminder: since gateway components changed, rebuild local images withcd gateway && make build-local.gateway/it/features/lazy-resources-xds.feature (1)
64-66: Consider using polling with timeout instead of fixed sleep for xDS propagation.The 3-second fixed wait may cause flakiness in slow CI environments or be unnecessarily slow in fast environments. A polling mechanism with exponential backoff and timeout would be more robust.
However, if fixed delays are the established pattern in this test suite and have proven reliable, this can be addressed in a follow-up.
Tharsanan1
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@gateway/it/steps_llm.go`:
- Around line 350-367: The assertion function assertLazyResourceNotExists should
fail if the config dump is missing the lazy_resources section instead of
silently passing; after unmarshalling in assertLazyResourceNotExists check that
response.LazyResources is non-nil and that
response.LazyResources.ResourcesByType is present (e.g., len==0 or nil) and, if
missing, return a descriptive error like "config dump missing lazy_resources" so
the test surfaces shape regressions; apply the same guard to the analogous "not
exists" assertion at the other spot (the function around lines 552-572) to
ensure both functions validate presence of LazyResources before iterating.
🧹 Nitpick comments (1)
gateway/it/features/lazy-resources-xds.feature (1)
64-66: Prefer polling over fixed sleeps to reduce flakiness.
Given xDS propagation variability, consider an “eventually” step that polls config_dump until the expected condition is met (and reuse it for other waits in this feature).
Comment on lines
+350
to
+367
| // assertLazyResourceNotExists checks that a lazy resource with the given ID does not exist | ||
| func assertLazyResourceNotExists(body []byte, templateID string) error { | ||
| var response ConfigDumpResponse | ||
| if err := json.Unmarshal(body, &response); err != nil { | ||
| return fmt.Errorf("failed to parse config dump JSON: %w", err) | ||
| } | ||
|
|
||
| for resourceType, resources := range response.LazyResources.ResourcesByType { | ||
| for _, resource := range resources { | ||
| if resource.ID == templateID { | ||
| return fmt.Errorf("template %q should not exist but was found in lazy resources of type %q", | ||
| templateID, resourceType) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
Contributor
There was a problem hiding this comment.
Guard against missing lazy_resources in “not exists” assertions.
If the field is absent, these checks currently pass, which can hide regressions in config-dump shape.
✅ Suggested hardening
func assertLazyResourceNotExists(body []byte, templateID string) error {
var response ConfigDumpResponse
if err := json.Unmarshal(body, &response); err != nil {
return fmt.Errorf("failed to parse config dump JSON: %w", err)
}
+ if response.LazyResources.ResourcesByType == nil {
+ return fmt.Errorf("lazy_resources missing from config dump")
+ }
for resourceType, resources := range response.LazyResources.ResourcesByType {
for _, resource := range resources {
if resource.ID == templateID {
return fmt.Errorf("template %q should not exist but was found in lazy resources of type %q",
templateID, resourceType)
}
}
}
return nil
}func assertLazyResourceNotExistsWithType(body []byte, resourceID, resourceType string) error {
var response ConfigDumpResponse
if err := json.Unmarshal(body, &response); err != nil {
return fmt.Errorf("failed to parse config dump JSON: %w", err)
}
+ if response.LazyResources.ResourcesByType == nil {
+ return fmt.Errorf("lazy_resources missing from config dump")
+ }
resources, exists := response.LazyResources.ResourcesByType[resourceType]
if !exists {
// Type doesn't exist, so resource definitely doesn't exist
return nil
}Also applies to: 552-572
🤖 Prompt for AI Agents
In `@gateway/it/steps_llm.go` around lines 350 - 367, The assertion function
assertLazyResourceNotExists should fail if the config dump is missing the
lazy_resources section instead of silently passing; after unmarshalling in
assertLazyResourceNotExists check that response.LazyResources is non-nil and
that response.LazyResources.ResourcesByType is present (e.g., len==0 or nil)
and, if missing, return a descriptive error like "config dump missing
lazy_resources" so the test surfaces shape regressions; apply the same guard to
the analogous "not exists" assertion at the other spot (the function around
lines 552-572) to ensure both functions validate presence of LazyResources
before iterating.
Krishanx92
approved these changes
Jan 27, 2026
renuka-fernando
approved these changes
Jan 27, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.