nginx-gateway-fabric
nginx-gateway-fabric copied to clipboard
Add request header filter support for gRPC
Proposed changes
Problem: As a user of NGF with applications that require GRPC traffic I want NGF to support a GRPCRouteRule that can handle a request HTTPHeaderFilter So that I can append specific headers required by my applications.
Solution: Add support for RequestHeaderFilters for GRPCRoutes. This mostly reuses the logic that already exists for HTTPRoutes by converting the GRPCFilter to HTTPFilter, and adapting the existing validation. This makes most of the request header validation shared, so it has been moved to the route_common.go
(as well as the corresponding unit tests).
Also added base gRPC headers - Host
(which maps to the :authority
pseudo-header under the hood), Authority
(which needs to be the same value as Host
) and X-Forwarded-For
which works the same as HTTP and preserves the client IP.
Testing: As well as unit test coverage, local testing using the conformance test echo server:
Config:
apiVersion: gateway.networking.k8s.io/v1alpha2
kind: GRPCRoute
metadata:
name: backend-v1
spec:
parentRefs:
- name: grpcroute-listener-hostname-matching
sectionName: listener-1
rules:
- matches:
filters:
- type: RequestHeaderModifier
requestHeaderModifier:
set:
- name: My-Overwrite-Header
value: this-is-the-only-value
add:
- name: Accept-Encoding
value: compress
- name: My-cool-header
value: this-is-an-appended-value
remove:
- User-Agent
backendRefs:
- name: grpc-infra-backend-v1
port: 8080
Request and response:
grpcurl -plaintext -proto grpc.proto -authority bar.com -H "my-cool-header: hello" -H "my-overwrite-header: don't see this" ${GW_IP}:${GW_PORT} gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo
{
"assertions": {
"fullyQualifiedMethod": "/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo",
"headers": [
{
"key": "grpc-accept-encoding",
"value": "gzip"
},
{
"key": ":authority",
"value": "bar.com"
},
{
"key": "accept-encoding",
"value": "compress"
},
{
"key": "my-cool-header",
"value": "hello,this-is-an-appended-value"
},
{
"key": "content-type",
"value": "application/grpc"
},
{
"key": "my-overwrite-header",
"value": "this-is-the-only-value"
},
{
"key": "x-forwarded-for",
"value": "172.18.0.3"
},
{
"key": "authority",
"value": "bar.com"
}
],
"authority": "bar.com",
"context": {
"namespace": "default",
"pod": "grpc-infra-backend-v1-68866c4db9-vd4ln"
}
}
}
Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide specific feedback, add them here.
Closes #1766
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.
Added support for RequestHeaderFilter for GRPCRoutes
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 86.90%. Comparing base (
bc01fdf
) to head (44f565a
).
Additional details and impacted files
@@ Coverage Diff @@
## main #1909 +/- ##
==========================================
+ Coverage 86.83% 86.90% +0.06%
==========================================
Files 88 88
Lines 6025 6056 +31
Branches 50 50
==========================================
+ Hits 5232 5263 +31
Misses 741 741
Partials 52 52
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
are we adding or updating any examples to test this?
LGTM otherwise!
@salonichf5 No, configuring this filter is identical to how it's done for HTTPRoute, so adding a further example wouldn't add much value. Additionally, we don't have an easy way to create an echo server like we do for HTTP (in my testing I am using the echo server from the conformance test suite), so there would be additional overhead in creating and maintaining one for use in examples where we want to see the headers.