nginx-gateway-fabric
nginx-gateway-fabric copied to clipboard
Add Support for Gateway-API HTTP Request Mirror
Proposed changes
Add support for Gateway-API HTTP Request Mirroring spec, as described in this guide.
Problem: currently, NGF, does not support http-request mirroring filter.
Solution:
The end goal was to modify the dataplane.Location
struct so that servers_template.go
file will be able to include a MirrorPath
field populated in the dataplane.Location
. This field specifies the path to where mirrored requests should be sent:
{{- if $l.MirrorPath }}
mirror {{ $l.MirrorPath }};
{{- end }}
To achieve this, the following steps were taken:
- Create a
MirrorPath
field in thedataplane.Location
struct. - Create an actual location route entry as target for the mirror directive call:
- With the same specification as the "source" location route (headers, filters, etc.)
- Without the mirror filter that caused the route creation
- With a distinct name based on the "target" service name, namespace, and route
Additionally, the gateway-api specification uses a backendRef
structure as the mechanism for backend specification.
Therefore, the filter also creates a new BackendRef
(if it doesn't exist) mapped to the created target location route.
The biggest challenge, which was solved in a not-so-elegant way, was that target route creation is done by adding new path matches to the Graph
(separately from the path holding the filter information). So, when the Graph
is converted to a dataplane.Location
and the mirror /{target}
directive needs to be populated in the template (as shown above), the source path needs to perform similar logic as it did during the Graph
creation step for building the target path. In short, two pieces of code from different packages (graph and dataplane ) needed to be doing identical logic at different phases. The not-so-elegant solution was to call methods created in the helpers
package.
Testing: Numerous but probably not enough. A lot of investment made to test the graph creation. I manually tested the implementation locally over k8s (actually I'm using k3s), using the resources defined in the examples/cafe-mirror-example
, dug into the NGF pod to cheerfully witness the creation of the mirror directive, and the actual mirroring to be working.
Please focus on (optional): Think of another approach to solving the creation of the target path and the source path mirror {/target}
directive at different phases. During the testing in graph_test.go
I didn't quite understand the logic behind the tls policies and I just made it work, so possibly there are mistakes there
Closes #533
Checklist
Before creating a PR, run through this checklist and mark each as complete.
- [X] I have read the CONTRIBUTING doc
- [X] I have added tests that prove my fix is effective or that my feature works
- [X] I have checked that all unit tests pass after adding my changes
- [X] I have updated necessary documentation
- [X] I have rebased my branch onto main
- [X] I will ensure my PR is targeting the main branch and pulling from my branch from my own fork
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes, please add a brief note that summarizes the change.
Add support for Gateway-API http-request-mirror filter
Hi @amimimor! Welcome to the project! 🎉
Thanks for opening this pull request! Be sure to check out our Contributing Guidelines while you wait for someone on the team to review this.
@amimimor Can you enable the conformance tests for mirroring? Add HTTPRouteRequestMirror
and HTTPRouteRequestMultipleMirrors
to the list of supported features in the conformance Makefile. You can then follow the steps in the conformance README to run the tests and verify everything passes.
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.
This PR was closed because it has been stalled for 14 days with no activity.
Hi maintainers, I was off to a long vacation... can you reopen this draft so we could continue?
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.
This PR was closed because it has been stalled for 14 days with no activity.