[bbr] add integration tests for response plugin execution by abdallahsamabd · Pull Request #2450 · kubernetes-sigs/gateway-api-inference-extension

abdallahsamabd · 2026-03-02T13:33:32Z

What this PR does / why we need it:

Follow-up to PR #2369 ([pluggable bbr] mirror request plugins execution to the response path).

This PR adds integration tests that validate response plugin execution over a real gRPC ext_proc stream, covering:

Unary mode with plugin: a response plugin mutates the response body (e.g., guardrail adding a field)
Unary mode without plugins: no response plugins configured results in passthrough behavior

These tests exercise the full Process loop (request body → response headers → response body) to ensure end-to-end correctness.

Also adds:

NewBBRHarnessWithPlugins helper to support configuring request/response plugins in test harnesses
Response phase expectation helpers (ExpectResponseHeadersPassThrough, ExpectResponseBodyPassThrough, ExpectResponseBodyMutation)

Which issue(s) this PR fixes:

Fixes #2449

Does this PR introduce a user-facing change?:

k8s-ci-robot · 2026-03-02T13:33:36Z

@abdallahsamabd: The label(s) kind/test cannot be applied, because the repository doesn't have them.

Details

In response to this:

/kind test

What this PR does / why we need it:

Follow-up to PR #2369 ([pluggable bbr] mirror request plugins execution to the response path).

This PR adds integration tests that validate response plugin execution over a real gRPC ext_proc stream, covering:

Unary mode with plugin: a response plugin mutates the response body (e.g., guardrail adding a field)

Unary mode without plugins: no response plugins configured results in passthrough behavior

These tests exercise the full Process loop (request body → response headers → response body) to ensure end-to-end correctness.

Also adds:

NewBBRHarnessWithPlugins helper to support configuring request/response plugins in test harnesses

Response phase expectation helpers (ExpectResponseHeadersPassThrough, ExpectResponseBodyPassThrough, ExpectResponseBodyMutation)

Which issue(s) this PR fixes:

Fixes #2449

Does this PR introduce a user-facing change?:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

netlify · 2026-03-02T13:33:39Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`4bf8870`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/69ca8da3f8b5c10008dae9ca
😎 Deploy Preview	https://deploy-preview-2450--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2026-03-02T13:33:43Z

Hi @abdallahsamabd. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

nirrozenbaum · 2026-03-12T11:42:02Z

/ok-to-test

nirrozenbaum · 2026-03-29T18:51:00Z

@abdallahsamabd can you please rebase?

nirrozenbaum · 2026-03-29T18:52:01Z

+	if requestPlugins == nil {
+		modelToHeaderPlugin, err := plugins.NewBodyFieldToHeaderPlugin(handlers.ModelField, handlers.ModelHeader)
+		require.NoError(t, err, "failed to create body-field-to-header plugin")
+
+		baseModelPlugin := &basemodelextractor.BaseModelToHeaderPlugin{AdaptersStore: basemodelextractor.NewAdaptersStore()}
+		requestPlugins = []framework.RequestProcessor{modelToHeaderPlugin, baseModelPlugin}
+	}


why do we need this part?

Without it, the server would start with no request plugins at all, breaking basic model header extraction. The fallback ensures a working minimum request pipeline regardless of what the caller passes

not sure I understand.
This function is called from few places. all these places instantiate request plugins and pass them as arg to this function.
it's the caller responsibility to pass plugins. we shouldn't fallback here.
I also double checked the code and all callers (3 places) are calling with plugins, as expected.

nirrozenbaum · 2026-03-30T06:26:11Z

 	baseModelToHeaderPlugin := &basemodelextractor.BaseModelToHeaderPlugin{AdaptersStore: store}

-	return NewBBRHarnessWithPlugins(t, ctx, streaming, []framework.RequestProcessor{modelToHeaderPlugin, baseModelToHeaderPlugin})
+	return NewBBRHarnessWithPlugins(t, ctx, streaming, []framework.RequestProcessor{modelToHeaderPlugin, baseModelToHeaderPlugin}, nil)


nit: pass empty slice, not nil.

Suggested change

return NewBBRHarnessWithPlugins(t, ctx, streaming, []framework.RequestProcessor{modelToHeaderPlugin, baseModelToHeaderPlugin}, nil)

return NewBBRHarnessWithPlugins(t, ctx, streaming, []framework.RequestProcessor{modelToHeaderPlugin, baseModelToHeaderPlugin}, []framework.ResponseProcessor{})

nirrozenbaum · 2026-03-30T09:27:07Z

 	plugin := &bodyMutatingPlugin{fieldName: "injected", fieldValue: "test-value"}
 	baseModelToHeaderPlugin := &basemodelextractor.BaseModelToHeaderPlugin{AdaptersStore: basemodelextractor.NewAdaptersStore()}
-	h := NewBBRHarnessWithPlugins(t, ctx, false, []framework.RequestProcessor{plugin, baseModelToHeaderPlugin})
+	h := NewBBRHarnessWithPlugins(t, ctx, false, []framework.RequestProcessor{plugin, baseModelToHeaderPlugin}, nil)


can we fix here too?

Suggested change

h := NewBBRHarnessWithPlugins(t, ctx, false, []framework.RequestProcessor{plugin, baseModelToHeaderPlugin}, nil)

h := NewBBRHarnessWithPlugins(t, ctx, false, []framework.RequestProcessor{plugin, baseModelToHeaderPlugin}, []framework.ResponseProcessor{})

nirrozenbaum · 2026-03-30T09:27:30Z

 	plugin := &bodyMutatingPlugin{fieldName: "injected", fieldValue: "test-value"}
 	baseModelToHeaderPlugin := &basemodelextractor.BaseModelToHeaderPlugin{AdaptersStore: basemodelextractor.NewAdaptersStore()}
-	h := NewBBRHarnessWithPlugins(t, ctx, true, []framework.RequestProcessor{plugin, baseModelToHeaderPlugin})
+	h := NewBBRHarnessWithPlugins(t, ctx, true, []framework.RequestProcessor{plugin, baseModelToHeaderPlugin}, nil)


ditto:

Suggested change

h := NewBBRHarnessWithPlugins(t, ctx, true, []framework.RequestProcessor{plugin, baseModelToHeaderPlugin}, nil)

h := NewBBRHarnessWithPlugins(t, ctx, true, []framework.RequestProcessor{plugin, baseModelToHeaderPlugin}, []framework.ResponseProcessor{})

nirrozenbaum · 2026-03-30T11:43:18Z

+
+// TestResponsePlugins_NoPlugins_Unary validates that when no response plugins
+// are configured, the response body is passed through without mutation.
+func TestResponsePlugins_NoPlugins_Unary(t *testing.T) {


this is already tested in TestBodyBasedRouting, isn't it?
if something is missing from that test - can be extend the test instead of reimplementing it?

nirrozenbaum · 2026-03-30T11:50:41Z

+
+// TestResponsePlugins_Unary validates that response plugins can mutate the
+// response body in unary (non-streaming) mode over a real gRPC ext_proc stream.
+func TestResponsePlugins_Unary(t *testing.T) {


can we reuse TestBodyBasedRouting test suite?
e.g., extend it to have createHarnessFunc func() *BBRHarness. then in different tests call it differently (in some case with defaults, in other with your response test plugins).
ideally we should have single test suite driving the tests, and just different configurations.

nirrozenbaum · 2026-03-30T14:34:57Z

+	guardrailPlugin := &testResponsePlugin{
+		name: "guardrail",


can we call it with a name making it clear it's a response plugin?
e.g., responsePlugin?

nirrozenbaum · 2026-03-30T14:41:02Z

 			}
-			require.NoError(t, err, "unexpected error during request processing")
+
+			responses, err := integration.StreamedRequest(t, h.Client, tc.reqs, expectedCount)


I think there's a bit confusion in the PR.
this test is for unary request. it should use a single request object and call function integration.SendRequest as it was before.

integration/StreamedRequest is for testing streaming case.

The reason we switched to StreamedRequest is that testing the response phase (ResponseHeaders → ResponseBody) requires sending 3 messages on the same gRPC stream, which SendRequest can't do.

will be fixed:
refactor: restore TestBodyBasedRouting to use SendRequest for request-phase-only tests, and create a separate TestResponsePlugins using StreamedRequest for full lifecycle tests.

Follow-up to kubernetes-sigs#2354 / PR kubernetes-sigs#2369. Exercises the full Process loop (request body -> response headers -> response body) to validate that response plugins can mutate the response body over a real gRPC ext_proc stream. Covers: - Unary mode: response plugin mutates the response body - Unary mode: no response plugins results in passthrough behavior Signed-off-by: Abdallah Samara <abdallahsamabd@gmail.com> Made-with: Cursor

nirrozenbaum · 2026-03-31T13:15:15Z

+				respHeaders,
+				respBodyReq(map[string]any{"choices": []any{map[string]any{"text": "Hello!"}}}),


non blocking - maybe it's worth extracting these to integration package and expose as public functions?
we might reuse them in different tests.

nirrozenbaum

/lgtm
/approve

left one minor nit. no need to block the PR on that.

k8s-ci-robot · 2026-03-31T13:15:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abdallahsamabd, nirrozenbaum

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:

~~OWNERS~~ [nirrozenbaum]

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

…gateway-api-inference-extension#2450) Follow-up to kubernetes-sigs/gateway-api-inference-extension#2354 / PR kubernetes-sigs/gateway-api-inference-extension#2369. Exercises the full Process loop (request body -> response headers -> response body) to validate that response plugins can mutate the response body over a real gRPC ext_proc stream. Covers: - Unary mode: response plugin mutates the response body - Unary mode: no response plugins results in passthrough behavior Made-with: Cursor Signed-off-by: Abdallah Samara <abdallahsamabd@gmail.com>

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 2, 2026

k8s-ci-robot requested review from liu-cong and shmuelk March 2, 2026 13:33

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 2, 2026

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 2, 2026

abdallahsamabd marked this pull request as draft March 2, 2026 13:55

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 2, 2026

abdallahsamabd changed the title ~~[pluggable bbr] add integration tests for response plugin execution~~ [bbr] add integration tests for response plugin execution Mar 9, 2026

abdallahsamabd marked this pull request as ready for review March 9, 2026 11:15

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2026

k8s-ci-robot requested review from nirrozenbaum and robscott March 9, 2026 11:15

abdallahsamabd force-pushed the feat/2449 branch from 8507b0a to 2277e1f Compare March 9, 2026 11:24

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 9, 2026

abdallahsamabd marked this pull request as draft March 9, 2026 11:24

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2026

abdallahsamabd marked this pull request as ready for review March 12, 2026 10:17

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 12, 2026

abdallahsamabd force-pushed the feat/2449 branch from 2277e1f to 1e257e1 Compare March 12, 2026 11:16

abdallahsamabd force-pushed the feat/2449 branch from 1e257e1 to a2697c8 Compare March 15, 2026 20:06

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 18, 2026

abdallahsamabd force-pushed the feat/2449 branch from 2ee1c8e to 8bfb3d6 Compare March 24, 2026 10:47

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 27, 2026

nirrozenbaum reviewed Mar 29, 2026

View reviewed changes

abdallahsamabd force-pushed the feat/2449 branch from 8bfb3d6 to 26b612b Compare March 29, 2026 20:20

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 29, 2026

abdallahsamabd force-pushed the feat/2449 branch from 26b612b to a71a09a Compare March 29, 2026 20:25