Skip to content

Commit 30be4f4

Browse files
committed
Fix issue where wildcard policy not engaging for provider
1 parent 5f590ee commit 30be4f4

3 files changed

Lines changed: 139 additions & 48 deletions

File tree

gateway/gateway-controller/pkg/utils/llm_provider_transformer_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4857,7 +4857,7 @@ func TestTransform_PathMatchingEdgeCases_AllowAll_PolicyMoreGeneral(t *testing.T
48574857

48584858
func TestTransform_PathMatchingEdgeCases_AllowAll_NestedWildcardPolicies(t *testing.T) {
48594859
// AllowAll: Multiple nested wildcard policies with no exceptions
4860-
// All policies should attach to catch-all operations
4860+
// Broader wildcard policies should also apply to nested wildcard operations
48614861
transformer, _ := setupTestTransformer(t)
48624862

48634863
policies := []api.LLMPolicy{
@@ -4916,8 +4916,9 @@ func TestTransform_PathMatchingEdgeCases_AllowAll_NestedWildcardPolicies(t *test
49164916
nestedOp := findOperation(spec.Operations, "api/v1/*", "GET")
49174917
require.NotNil(t, nestedOp, "api/v1/* GET should exist")
49184918
require.NotNil(t, nestedOp.Policies)
4919-
assert.Len(t, *nestedOp.Policies, 1)
4920-
assert.Equal(t, "NestedPolicy", (*nestedOp.Policies)[0].Name)
4919+
assert.Len(t, *nestedOp.Policies, 2)
4920+
assert.Equal(t, "TopLevelPolicy", (*nestedOp.Policies)[0].Name)
4921+
assert.Equal(t, "NestedPolicy", (*nestedOp.Policies)[1].Name)
49214922

49224923
// Find api/* operation
49234924
topOp := findOperation(spec.Operations, "api/*", "GET")

gateway/gateway-controller/pkg/utils/llm_transformer.go

Lines changed: 61 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -224,18 +224,11 @@ func (t *LLMProviderTransformer) transformProxy(proxy *api.LLMProxyConfiguration
224224

225225
// Phase 2: Process User-Defined Policies
226226
if proxy.Spec.Policies != nil {
227+
registerExplicitLLMPolicyOperations(operationRegistry, *proxy.Spec.Policies, nil)
228+
227229
for _, llmPol := range *proxy.Spec.Policies {
228230
for _, pathEntry := range llmPol.Paths {
229-
// Expand wildcard methods in policy
230-
var policyMethods []string
231-
if len(pathEntry.Methods) == 1 && string(pathEntry.Methods[0]) == "*" {
232-
policyMethods = constants.WILDCARD_HTTP_METHODS
233-
} else {
234-
policyMethods = make([]string, len(pathEntry.Methods))
235-
for i, m := range pathEntry.Methods {
236-
policyMethods[i] = string(m)
237-
}
238-
}
231+
policyMethods := expandLLMPolicyMethods(pathEntry.Methods)
239232

240233
for _, policyMethod := range policyMethods {
241234
attachedPolicyPaths := make(map[string]bool)
@@ -244,11 +237,7 @@ func (t *LLMProviderTransformer) transformProxy(proxy *api.LLMProxyConfiguration
244237
// Create operation if it doesn't exist (dynamic operation creation)
245238
key := pathMethodKey{path: pathEntry.Path, method: policyMethod}
246239
if _, exists := operationRegistry[key]; !exists {
247-
op := &api.Operation{
248-
Path: pathEntry.Path,
249-
Method: api.OperationMethod(policyMethod),
250-
}
251-
operationRegistry[key] = op
240+
op := ensureOperation(operationRegistry, pathEntry.Path, policyMethod)
252241
methodOperations = append(methodOperations, op)
253242
}
254243

@@ -451,18 +440,13 @@ func (t *LLMProviderTransformer) transformProvider(provider *api.LLMProviderConf
451440

452441
// Phase 3: Process User-Defined Policies
453442
if provider.Spec.Policies != nil {
443+
registerExplicitLLMPolicyOperations(operationRegistry, *provider.Spec.Policies, func(path, method string) bool {
444+
return !isDeniedByException(path, method, deniedPathMethods)
445+
})
446+
454447
for _, llmPol := range *provider.Spec.Policies {
455448
for _, pathEntry := range llmPol.Paths {
456-
// Expand wildcard methods in policy
457-
var policyMethods []string
458-
if len(pathEntry.Methods) == 1 && string(pathEntry.Methods[0]) == "*" {
459-
policyMethods = constants.WILDCARD_HTTP_METHODS
460-
} else {
461-
policyMethods = make([]string, len(pathEntry.Methods))
462-
for i, m := range pathEntry.Methods {
463-
policyMethods[i] = string(m)
464-
}
465-
}
449+
policyMethods := expandLLMPolicyMethods(pathEntry.Methods)
466450

467451
for _, policyMethod := range policyMethods {
468452
attachedPolicyPaths := make(map[string]bool)
@@ -477,11 +461,7 @@ func (t *LLMProviderTransformer) transformProvider(provider *api.LLMProviderConf
477461
key := pathMethodKey{path: pathEntry.Path, method: policyMethod}
478462
if _, exists := operationRegistry[key]; !exists {
479463
// Create operation for user policy
480-
op := &api.Operation{
481-
Path: pathEntry.Path,
482-
Method: api.OperationMethod(policyMethod),
483-
}
484-
operationRegistry[key] = op
464+
op := ensureOperation(operationRegistry, pathEntry.Path, policyMethod)
485465
methodOperations = append(methodOperations, op)
486466
}
487467

@@ -567,18 +547,13 @@ func (t *LLMProviderTransformer) transformProvider(provider *api.LLMProviderConf
567547

568548
// Phase 3: Process Policies with Dynamic Operation Creation
569549
if provider.Spec.Policies != nil {
550+
registerExplicitLLMPolicyOperations(operationRegistry, *provider.Spec.Policies, func(path, method string) bool {
551+
return isAllowedByAccessControl(path, method, normalizedExceptions)
552+
})
553+
570554
for _, llmPol := range *provider.Spec.Policies {
571555
for _, pathEntry := range llmPol.Paths {
572-
// Expand wildcard methods in policy
573-
var policyMethods []string
574-
if len(pathEntry.Methods) == 1 && string(pathEntry.Methods[0]) == "*" {
575-
policyMethods = constants.WILDCARD_HTTP_METHODS
576-
} else {
577-
policyMethods = make([]string, len(pathEntry.Methods))
578-
for i, m := range pathEntry.Methods {
579-
policyMethods[i] = string(m)
580-
}
581-
}
556+
policyMethods := expandLLMPolicyMethods(pathEntry.Methods)
582557

583558
for _, policyMethod := range policyMethods {
584559
attachedPolicyPaths := make(map[string]bool)
@@ -590,11 +565,7 @@ func (t *LLMProviderTransformer) transformProvider(provider *api.LLMProviderConf
590565
key := pathMethodKey{path: pathEntry.Path, method: policyMethod}
591566
if _, exists := operationRegistry[key]; !exists {
592567
// Create operation for specific policy path covered by wildcard access control
593-
op := &api.Operation{
594-
Path: pathEntry.Path,
595-
Method: api.OperationMethod(policyMethod),
596-
}
597-
operationRegistry[key] = op
568+
op := ensureOperation(operationRegistry, pathEntry.Path, policyMethod)
598569
methodOperations = append(methodOperations, op)
599570
}
600571
}
@@ -803,6 +774,51 @@ func appendOperationPolicy(op *api.Operation, pol api.Policy) {
803774
op.Policies = &existing
804775
}
805776

777+
// ensureOperation checks if an operation for the given path+method exists in the registry, and creates it if not
778+
func ensureOperation(operationRegistry map[pathMethodKey]*api.Operation, path, method string) *api.Operation {
779+
key := pathMethodKey{path: path, method: method}
780+
if op, exists := operationRegistry[key]; exists {
781+
return op
782+
}
783+
784+
op := &api.Operation{
785+
Path: path,
786+
Method: api.OperationMethod(method),
787+
}
788+
operationRegistry[key] = op
789+
return op
790+
}
791+
792+
// expandLLMPolicyMethods takes the methods defined in an LLM policy and expands them to actual HTTP methods if wildcard is used
793+
func expandLLMPolicyMethods(methods []api.LLMPolicyPathMethods) []string {
794+
if len(methods) == 1 && string(methods[0]) == constants.WILD_CARD {
795+
return append([]string(nil), constants.WILDCARD_HTTP_METHODS...)
796+
}
797+
798+
expanded := make([]string, len(methods))
799+
for i, method := range methods {
800+
expanded[i] = string(method)
801+
}
802+
return expanded
803+
}
804+
805+
// registerExplicitLLMPolicyOperations iterates through the explicitly defined policies in the LLM policy and ensures that operations
806+
// exist for their paths and methods in the operation registry. The shouldRegister callback allows conditional registration based on
807+
// path and method (e.g., to skip paths/methods denied by access control exceptions).
808+
func registerExplicitLLMPolicyOperations(operationRegistry map[pathMethodKey]*api.Operation, policies []api.LLMPolicy,
809+
shouldRegister func(path, method string) bool) {
810+
for _, llmPol := range policies {
811+
for _, pathEntry := range llmPol.Paths {
812+
for _, method := range expandLLMPolicyMethods(pathEntry.Methods) {
813+
if shouldRegister != nil && !shouldRegister(pathEntry.Path, method) {
814+
continue
815+
}
816+
ensureOperation(operationRegistry, pathEntry.Path, method)
817+
}
818+
}
819+
}
820+
}
821+
806822
func getOperationsForMethod(operationRegistry map[pathMethodKey]*api.Operation, method string) []*api.Operation {
807823
ops := make([]*api.Operation, 0)
808824
for key, op := range operationRegistry {

gateway/gateway-controller/pkg/utils/llm_transformer_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,6 +1006,80 @@ func TestTransformProvider_ExpandsWildcardPolicyPathWithTemplateMappings(t *test
10061006
assert.Equal(t, "value", (*wildcardPolicy.Params)["userParam"])
10071007
}
10081008

1009+
func TestTransformProvider_PolicyOrderDoesNotAffectWildcardCoverage(t *testing.T) {
1010+
store := storage.NewConfigStore()
1011+
db := newTestMockDB()
1012+
routerConfig := &config.RouterConfig{ListenerPort: 8080}
1013+
transformer := NewLLMProviderTransformer(store, db, routerConfig, newTestPolicyVersionResolver())
1014+
1015+
template := &models.StoredLLMProviderTemplate{
1016+
UUID: "0000-template-1-0000-000000000001",
1017+
Configuration: api.LLMProviderTemplate{
1018+
Metadata: api.Metadata{Name: "openai"},
1019+
Spec: api.LLMProviderTemplateData{},
1020+
},
1021+
}
1022+
db.SaveLLMProviderTemplate(template)
1023+
err := store.AddTemplate(template)
1024+
require.NoError(t, err)
1025+
1026+
upstreamURL := "https://api.openai.com"
1027+
provider := &api.LLMProviderConfiguration{
1028+
Metadata: api.Metadata{Name: "openai-provider"},
1029+
Spec: api.LLMProviderConfigData{
1030+
DisplayName: "OpenAI Provider",
1031+
Version: "1.0.0",
1032+
Template: "openai",
1033+
Upstream: api.LLMProviderConfigData_Upstream{
1034+
Url: &upstreamURL,
1035+
},
1036+
AccessControl: api.LLMAccessControl{Mode: api.AllowAll},
1037+
Policies: &[]api.LLMPolicy{
1038+
{
1039+
Name: "set-headers-all",
1040+
Version: "v1",
1041+
Paths: []api.LLMPolicyPath{{
1042+
Path: "/*",
1043+
Methods: []api.LLMPolicyPathMethods{"POST"},
1044+
Params: map[string]interface{}{"scope": "all"},
1045+
}},
1046+
},
1047+
{
1048+
Name: "set-headers",
1049+
Version: "v1",
1050+
Paths: []api.LLMPolicyPath{{
1051+
Path: "/chat/completions",
1052+
Methods: []api.LLMPolicyPathMethods{"POST"},
1053+
Params: map[string]interface{}{"scope": "specific"},
1054+
}},
1055+
},
1056+
},
1057+
},
1058+
}
1059+
1060+
result, err := transformer.Transform(provider, &api.RestAPI{})
1061+
require.NoError(t, err)
1062+
require.NotNil(t, result)
1063+
1064+
chatOp := findOperation(result.Spec.Operations, "/chat/completions", "POST")
1065+
require.NotNil(t, chatOp)
1066+
require.NotNil(t, chatOp.Policies)
1067+
1068+
policyNames := make(map[string]bool)
1069+
for _, policy := range *chatOp.Policies {
1070+
policyNames[policy.Name] = true
1071+
}
1072+
1073+
assert.True(t, policyNames["set-headers-all"], "/chat/completions should inherit the wildcard policy")
1074+
assert.True(t, policyNames["set-headers"], "/chat/completions should keep its specific policy")
1075+
1076+
wildcardOp := findOperation(result.Spec.Operations, "/*", "POST")
1077+
require.NotNil(t, wildcardOp)
1078+
require.NotNil(t, wildcardOp.Policies)
1079+
assert.Len(t, *wildcardOp.Policies, 1)
1080+
assert.Equal(t, "set-headers-all", (*wildcardOp.Policies)[0].Name)
1081+
}
1082+
10091083
func TestTransformProvider_WithUpstreamAuth(t *testing.T) {
10101084
store := storage.NewConfigStore()
10111085
db := newTestMockDB()

0 commit comments

Comments
 (0)