Skip to content

Commit 68b2a2f

Browse files
fix: align ExternalModel BBR auth, body rewrite, and header match with /v1/models identity (opendatahub-io#401)
* fix: align ExternalModel BBR auth, body rewrite, and header match with /v1/models identity * remove debug loggers * Triggering CI build
1 parent efde850 commit 68b2a2f

4 files changed

Lines changed: 28 additions & 14 deletions

File tree

pkg/controller/externalmodel/reconciler.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ func (r *Reconciler) reconcileHTTPRoute(ctx context.Context, logger logr.Logger,
139139
provider.Spec.Endpoint,
140140
provider.Name,
141141
model.Name,
142-
ref.TargetModel,
143142
model.Namespace,
144143
ctrlcommon.DefaultTLSPort,
145144
r.gatewayName(),
@@ -243,7 +242,7 @@ func commonLabels(modelName string) map[string]string {
243242
}
244243
}
245244

246-
func buildHTTPRoute(providerEndpoint, providerName, modelName, targetModel, namespace string, port int32, gatewayName, gatewayNamespace, routeTimeout string, labels map[string]string) *gatewayapiv1.HTTPRoute {
245+
func buildHTTPRoute(providerEndpoint, providerName, modelName, namespace string, port int32, gatewayName, gatewayNamespace, routeTimeout string, labels map[string]string) *gatewayapiv1.HTTPRoute {
247246
gwNamespace := gatewayapiv1.Namespace(gatewayNamespace)
248247
pathType := gatewayapiv1.PathMatchPathPrefix
249248
pathPrefix := "/" + namespace + "/" + modelName
@@ -313,7 +312,7 @@ func buildHTTPRoute(providerEndpoint, providerName, modelName, targetModel, name
313312
{
314313
Name: "X-Gateway-Model-Name",
315314
Type: &headerType,
316-
Value: targetModel,
315+
Value: modelName,
317316
},
318317
},
319318
},

pkg/controller/externalmodel/reconciler_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func TestReconcile_CreatesHTTPRoute(t *testing.T) {
190190
// Two rules: path + header
191191
require.Len(t, hr.Spec.Rules, 2)
192192
assert.Equal(t, "/"+ns+"/gpt4", *hr.Spec.Rules[0].Matches[0].Path.Value)
193-
assert.Equal(t, "gpt-4o", hr.Spec.Rules[1].Matches[0].Headers[0].Value)
193+
assert.Equal(t, "gpt4", hr.Spec.Rules[1].Matches[0].Headers[0].Value)
194194

195195
// Backend ref points to provider's Service
196196
assert.Equal(t, "my-openai", string(hr.Spec.Rules[0].BackendRefs[0].Name))
@@ -336,9 +336,9 @@ func TestReconcile_TwoModelsOneProvider(t *testing.T) {
336336
assert.Equal(t, "shared-provider", string(hr1.Spec.Rules[0].BackendRefs[0].Name))
337337
assert.Equal(t, "shared-provider", string(hr2.Spec.Rules[0].BackendRefs[0].Name))
338338

339-
// But different target models in header match
340-
assert.Equal(t, "gpt-4o", hr1.Spec.Rules[1].Matches[0].Headers[0].Value)
341-
assert.Equal(t, "gpt-3.5-turbo", hr2.Spec.Rules[1].Matches[0].Headers[0].Value)
339+
// Header match uses modelName (ExternalModel CR name)
340+
assert.Equal(t, "gpt4", hr1.Spec.Rules[1].Matches[0].Headers[0].Value)
341+
assert.Equal(t, "gpt35", hr2.Spec.Rules[1].Matches[0].Headers[0].Value)
342342

343343
// Different path prefixes
344344
assert.Equal(t, "/"+ns+"/gpt4", *hr1.Spec.Rules[0].Matches[0].Path.Value)
@@ -363,7 +363,7 @@ func TestCommonLabels(t *testing.T) {
363363
func TestBuildHTTPRoute(t *testing.T) {
364364
hr := buildHTTPRoute(
365365
"api.openai.com", "my-openai",
366-
"gpt4", "gpt-4o",
366+
"gpt4",
367367
"models", 443,
368368
"default-gateway", "openshift-ingress", "300s",
369369
commonLabels("gpt4"),
@@ -385,10 +385,10 @@ func TestBuildHTTPRoute(t *testing.T) {
385385
rule1 := hr.Spec.Rules[0]
386386
assert.Equal(t, "/models/gpt4", *rule1.Matches[0].Path.Value)
387387

388-
// Rule 2: header-based match uses targetModel
388+
// Rule 2: header-based match uses modelName (what /v1/models returns)
389389
rule2 := hr.Spec.Rules[1]
390390
assert.Equal(t, "X-Gateway-Model-Name", string(rule2.Matches[0].Headers[0].Name))
391-
assert.Equal(t, "gpt-4o", rule2.Matches[0].Headers[0].Value)
391+
assert.Equal(t, "gpt4", rule2.Matches[0].Headers[0].Value)
392392

393393
// Backend ref points to the PROVIDER's Service, not the model
394394
for i, rule := range hr.Spec.Rules {
@@ -409,7 +409,7 @@ func TestBuildHTTPRoute(t *testing.T) {
409409
func TestBuildHTTPRoute_TargetModelDiffersFromName(t *testing.T) {
410410
hr := buildHTTPRoute(
411411
"bedrock.us-east-1.amazonaws.com", "my-bedrock",
412-
"claude", "anthropic.claude-3-opus",
412+
"claude",
413413
"models", 443,
414414
"my-gateway", "gateway-ns", "300s",
415415
commonLabels("claude"),
@@ -419,8 +419,8 @@ func TestBuildHTTPRoute_TargetModelDiffersFromName(t *testing.T) {
419419
assert.Equal(t, "claude", hr.Name)
420420
assert.Equal(t, "/models/claude", *hr.Spec.Rules[0].Matches[0].Path.Value)
421421

422-
// Header match uses targetModel (provider-side name)
423-
assert.Equal(t, "anthropic.claude-3-opus", hr.Spec.Rules[1].Matches[0].Headers[0].Value)
422+
// Header match uses modelName (what /v1/models returns to clients)
423+
assert.Equal(t, "claude", hr.Spec.Rules[1].Matches[0].Headers[0].Value)
424424

425425
// Backend points to provider Service
426426
assert.Equal(t, "my-bedrock", string(hr.Spec.Rules[0].BackendRefs[0].Name))

pkg/plugins/model-provider-resolver/external_provider_reconciler.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ func (r *externalProviderReconciler) Reconcile(ctx context.Context, req ctrl.Req
6464
}
6565

6666
config := buildConfigWithDefaults(provider.Spec.Provider, provider.Spec.Config)
67+
authType := mapCRDAuthType(provider.Spec.Auth.Type)
6768
r.store.addOrUpdateProvider(req.NamespacedName, &providerInfo{
6869
provider: provider.Spec.Provider,
6970
endpoint: provider.Spec.Endpoint,
70-
auth: auth.Auth(provider.Spec.Auth.Type),
71+
auth: authType,
7172
secretName: provider.Spec.Auth.SecretRef.Name,
7273
secretNamespace: req.Namespace,
7374
config: config,
@@ -77,6 +78,16 @@ func (r *externalProviderReconciler) Reconcile(ctx context.Context, req ctrl.Req
7778
return ctrl.Result{}, nil
7879
}
7980

81+
// mapCRDAuthType maps CRD auth.type values to internal auth constants.
82+
// The ExternalProvider CRD uses "simple" for API-key auth, while the
83+
// internal auth package uses "apikey".
84+
func mapCRDAuthType(crdType string) auth.Auth {
85+
if crdType == "simple" {
86+
return auth.APIKey
87+
}
88+
return auth.Auth(crdType)
89+
}
90+
8091
// buildConfigWithDefaults returns a config map that starts from provider-specific
8192
// defaults and then applies any user-supplied values from the CR on top.
8293
func buildConfigWithDefaults(providerName string, userConfig map[string]string) map[string]string {

pkg/plugins/model-provider-resolver/plugin.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,10 @@ func (p *ModelProviderResolverPlugin) ProcessRequest(ctx context.Context, cycleS
171171

172172
ref := selectByWeight(modelInfo.refs)
173173

174+
if model != ref.targetModel {
175+
request.SetBodyField("model", ref.targetModel)
176+
}
177+
174178
cycleState.Write(state.ProviderKey, ref.provider)
175179
cycleState.Write(state.ModelKey, ref.targetModel)
176180
cycleState.Write(state.APIFormatKey, ref.apiFormat)

0 commit comments

Comments
 (0)