[BBR] test: add request plugin header mutation tests by noalimoy · Pull Request #2544 · kubernetes-sigs/gateway-api-inference-extension

noalimoy · 2026-03-10T14:31:45Z

What type of PR is this?

/kind test

What this PR does / why we need it:

Adds unit and integration tests to verify that SetHeader/RemoveHeader interactions across multiple request plugins work correctly in all combinations.

TestHandleRequestBody_MultiPluginHeaderMutations — 8 unit test scenarios calling HandleRequestBody directly with fakeRequestPlugin pairs.
TestRequestPluginHeaderMutations_Unary — 8 matching integration test scenarios over a real gRPC ext_proc server.

Test scenarios (8 total, mirrored in unit + integration):

#	Scenario	Expected Result
1	Plugin1: `Set(A)` → Plugin2: `Remove(A)`	Cancels out
2	Plugin1: `Set(A)` → Plugin2: `Remove(B)`	Both apply
3	Plugin1: `Remove(non-existing)`	No-op
4	Plugin1: `Remove(A)` → Plugin2: `Set(A, new)`	New value wins
5	Plugin1: `Set(A, v1)` → Plugin2: `Set(A, v2)`	Last writer wins (v2)
6	Plugin1: `Set(A)` + Plugin2: `Set(B)`	Both apply
7	Plugin1: `Remove(A)` → Plugin2: `Remove(A)`	Idempotent (single remove)
8	Plugin1: `Set(A, same_value)`	No mutation (optimized)

Which issue(s) this PR fixes:

Fixes #2526

Does this PR introduce a user-facing change?:
None

k8s-ci-robot · 2026-03-10T14:31:49Z

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

Details

In response to this:

What type of PR is this?

/kind test

What this PR does / why we need it:

Adds unit and integration tests to verify that SetHeader/RemoveHeader interactions across multiple request plugins work correctly in all combinations.

TestHandleRequestBody_MultiPluginHeaderMutations — 8 unit test scenarios calling HandleRequestBody directly with fakeRequestPlugin pairs.

TestRequestPluginHeaderMutations_Unary — 8 matching integration test scenarios over a real gRPC ext_proc server.

Test scenarios (8 total, mirrored in unit + integration):

# Scenario Expected Result

1 Plugin1: Set(A) → Plugin2: Remove(A) Cancels out

2 Plugin1: Set(A) → Plugin2: Remove(B) Both apply

3 Plugin1: Remove(non-existing) No-op

4 Plugin1: Remove(A) → Plugin2: Set(A, new) New value wins

5 Plugin1: Set(A, v1) → Plugin2: Set(A, v2) Last writer wins (v2)

6 Plugin1: Set(A) + Plugin2: Set(B) Both apply

7 Plugin1: Remove(A) → Plugin2: Remove(A) Idempotent (single remove)

8 Plugin1: Set(A, same_value) No mutation (optimized)

Which issue(s) this PR fixes:

Fixes #2526

Does this PR introduce a user-facing change?:
None

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-10T14:31:52Z

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

Name	Link
🔨 Latest commit	`3658eae`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/69c4f74e55cb9300089aa482
😎 Deploy Preview	https://deploy-preview-2544--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-10T14:31:54Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: noalimoy
Once this PR has been reviewed and has the lgtm label, please assign kfswain for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

k8s-ci-robot · 2026-03-10T14:31:56Z

Hi @noalimoy. 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.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

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-12T08:20:37Z

/ok-to-test

nirrozenbaum · 2026-03-12T08:23:19Z

test/integration/bbr/harness.go

-	require.NoError(t, err, "failed to create body-field-to-header plugin")
-	runner.RequestPlugins = []framework.RequestProcessor{modelToHeaderPlugin}
+
+	if requestPlugins == nil {


is there a case this is not nil?
I see the calls in hermetic_test are always called with nil.

Yes — in plugins_hermetic_test.go#L247, each test case passes tc.plugins (custom fakeRequestPlugin pairs) to NewBBRHarness. When nil, it falls back to the default BodyFieldToHeaderPlugin, keeping existing hermetic_test.go calls untouched.

nirrozenbaum · 2026-03-12T08:24:24Z

test/integration/bbr/plugins_hermetic_test.go

+)
+
+// fakeRequestPlugin implements framework.RequestProcessor for integration testing.
+type fakeRequestPlugin struct {


where is this plugin used?

It's used in every test case in this file (plugins_hermetic_test.go#L66-L242) as tc.plugins.
each scenario creates 1–2 fakeRequestPlugin instances with different inline mutation functions to test header set/remove combinations.

{ name: "set then remove same header - cancels out", plugins: []framework.RequestProcessor{ &fakeRequestPlugin{ name: "setter", mutateFn: func(_ context.Context, req *framework.InferenceRequest) error { req.SetHeader("X-Custom", "value1") return nil }, }, &fakeRequestPlugin{ name: "remover", mutateFn: func(_ context.Context, req *framework.InferenceRequest) error { req.RemoveHeader("X-Custom") return nil }, }, }, wantSetHeaders: map[string]string{}, wantRemoved: []string{"X-Custom"}, },

whoops. right.
can you have a look and check why test fails?

Yep, already fixed. The test expectations were missing BodyMutation and Content-Length added by #2551, rebased on latest main..

noalimoy · 2026-03-12T11:50:25Z

/retest

Add unit and integration tests to verify SetHeader/RemoveHeader interactions across multiple request plugins in all combinations. Changes: - Add request_plugins_test.go with 8 unit test scenarios: set→remove, remove→set, double-remove (idempotent), set same value (no-op optimization), last-writer-wins, and mixed header operations - Add plugins_hermetic_test.go with matching 8 integration test scenarios over a real gRPC ext_proc server - Extend NewBBRHarness to accept custom request plugins (nil = default BodyFieldToHeaderPlugin) - Update bbr_success_total metric assertion to account for shared global Prometheus counter across test files Signed-off-by: noalimoy <nlimoy@redhat.com>

k8s-ci-robot · 2026-03-26T09:21:00Z

@noalimoy: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gateway-api-inference-extension-verify-main	`3658eae`	link	true	`/test pull-gateway-api-inference-extension-verify-main`
pull-gateway-api-inference-extension-test-unit-main	`3658eae`	link	true	`/test pull-gateway-api-inference-extension-test-unit-main`
pull-gateway-api-inference-extension-test-e2e-main	`3658eae`	link	true	`/test pull-gateway-api-inference-extension-test-e2e-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

nirrozenbaum · 2026-03-29T11:50:26Z

@noalimoy can you fix failing checks?

nirrozenbaum · 2026-03-29T12:37:19Z

test/integration/bbr/plugins_hermetic_test.go

+
+// TestRequestPluginHeaderMutations_Unary validates header set/remove interactions
+// across multiple request plugins in non-streaming (unary) mode via a real gRPC server.
+func TestRequestPluginHeaderMutations_Unary(t *testing.T) {


feels to me like this is repeating the exact same tests from request.
I think we can remove the tests from here and use the regular hermetic tests to test the built in plugins.

nirrozenbaum · 2026-03-29T12:38:25Z

pkg/bbr/handlers/request_test.go

 	# HELP bbr_success_total [ALPHA] Count of time the request was processed successfully.
 	# TYPE bbr_success_total counter
-	bbr_success_total{} 4
+	bbr_success_total{} 12


I understand this is changed because 8 new tests were added in a different file.
this is very much non maintainable (a maintainer cannot reasonable figure that out).
can we move the other tests to this file?

nirrozenbaum · 2026-03-29T12:39:08Z

pkg/bbr/handlers/request_plugins_test.go

+// TestHandleRequestBody_MultiPluginHeaderMutations tests the end-to-end behavior of
+// HandleRequestBody when multiple request plugins set and/or remove headers.
+// Each sub-test verifies the HeaderMutation in the resulting ProcessingResponse.
+func TestHandleRequestBody_MultiPluginHeaderMutations(t *testing.T) {


after seeing the impl - can we move this to request_test.go?
otherwise some things are fragmented, making it very hard to follow.

the tests themselves are great :)

k8s-ci-robot · 2026-04-01T19:58:38Z

PR needs rebase.

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.

k8s-ci-robot requested review from danehans and kfswain March 10, 2026 14:31

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 10, 2026

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 10, 2026

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 12, 2026

nirrozenbaum reviewed Mar 12, 2026

View reviewed changes

noalimoy force-pushed the bbr-header-mutation-tests/2526 branch from 4e0d1a0 to abf9024 Compare March 12, 2026 11:35

noalimoy requested a review from nirrozenbaum March 12, 2026 14:51

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

noalimoy force-pushed the bbr-header-mutation-tests/2526 branch from abf9024 to e087bfe Compare March 15, 2026 11:15

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

noalimoy force-pushed the bbr-header-mutation-tests/2526 branch from e087bfe to 56945ba Compare March 15, 2026 13:39

noalimoy force-pushed the bbr-header-mutation-tests/2526 branch from 56945ba to c8afde7 Compare March 25, 2026 16:56

noalimoy force-pushed the bbr-header-mutation-tests/2526 branch from c8afde7 to 3658eae Compare March 26, 2026 09:07

nirrozenbaum reviewed Mar 29, 2026

View reviewed changes

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

nirrozenbaum changed the title ~~test(bbr): add request plugin header mutation tests~~ [BBR] test: add request plugin header mutation tests Apr 7, 2026

Conversation

noalimoy commented Mar 10, 2026

Uh oh!

k8s-ci-robot commented Mar 10, 2026

Uh oh!

netlify bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

k8s-ci-robot commented Mar 10, 2026

Uh oh!

k8s-ci-robot commented Mar 10, 2026

Uh oh!

nirrozenbaum commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noalimoy commented Mar 12, 2026

Uh oh!

k8s-ci-robot commented Mar 26, 2026

Uh oh!

nirrozenbaum commented Mar 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify bot commented Mar 10, 2026 •

edited

Loading