fix: reject empty router model requests

[pm-ju]

/kind bug

Reject empty or whitespace-only model values in router requests.

ParseModelRequest already rejected missing or non-string model fields, but accepted "" or " ". Those requests then continued into routing/scheduling with an unusable model name.

This change treats blank model names as model not found and adds focused unit coverage for:

• valid model
• missing model
• non-string model
• empty model
• whitespace-only model

Verification:
go test ./pkg/kthena-router/router

[gemini-code-assist]

Code Review

This pull request enhances model name validation in ParseModelRequest by ensuring the model name is not empty or composed solely of whitespace. It also introduces a comprehensive suite of table-driven tests to verify these validation rules. Feedback was provided to improve the test structure by parameterizing the expected model name in the test cases to increase flexibility and maintainability.

[gemini-code-assist]

While the new test cases are great for covering the new validation logic, the test implementation could be made more robust and maintainable. Currently, the expected model name is hardcoded in the assertion (assert.Equal(t, "test-model", got["model"])), which makes the test less flexible for adding new valid test cases in the future.

To improve this, I suggest adding an expectedModel field to the test struct and using it in the assertion for the success case. This will make the test more data-driven and easier to extend.

 	tests := []struct {
 		name          string
 		body          string
 		wantErr       bool
 		expectedModel string
 	}{
 		{
 			name:          "valid model",
 			body:          `{"model": "test-model", "prompt": "hello"}`,
 			wantErr:       false,
 			expectedModel: "test-model",
 		},
 		{
 			name:    "missing model",
 			body:    `{"prompt": "hello"}`,
 			wantErr: true,
 		},
 		{
 			name:    "non-string model",
 			body:    `{"model": 123, "prompt": "hello"}`,
 			wantErr: true,
 		},
 		{
 			name:    "empty model",
 			body:    `{"model": "", "prompt": "hello"}`,
 			wantErr: true,
 		},
 		{
 			name:    "whitespace model",
 			body:    `{"model": "  ", "prompt": "hello"}`,
 			wantErr: true,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			w := httptest.NewRecorder()
 			c, _ := gin.CreateTestContext(w)
 			c.Request, _ = http.NewRequest("POST", "/v1/chat/completions", bytes.NewBufferString(tt.body))
 
 			got, err := ParseModelRequest(c)
 			if tt.wantErr {
 				assert.Error(t, err)
 				assert.Nil(t, got)
 				assert.Equal(t, http.StatusNotFound, w.Code)
 				assert.Contains(t, w.Body.String(), "model not found")
 				return
 			}
 			assert.NoError(t, err)
 			assert.Equal(t, tt.expectedModel, got["model"])
 		})
 	}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[hzxuzhonghu]

/lgtm

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• pkg/OWNERS [hzxuzhonghu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment