Skip to content

grpc: Add grpc service support (A102)#9175

Open
mbissa wants to merge 6 commits into
grpc:masterfrom
mbissa:a102-grpc-service-support-rebased
Open

grpc: Add grpc service support (A102)#9175
mbissa wants to merge 6 commits into
grpc:masterfrom
mbissa:a102-grpc-service-support-rebased

Conversation

@mbissa

@mbissa mbissa commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

This PR adds support for grpc service as part of A102

RELEASE NOTES:

  • xds: Add grpc service support (A102)

mbissa added 3 commits June 8, 2026 10:32
fix vet script import alias check

gofmt and formatting fixes for vet check

Fix vet script issues: rename GRPCServiceConfig to Config, unexport HeaderValueOption, clean up package aliases, and rename unused test stream callback variables
…dsClient method, reuse bootstrap credentials structures, and move HeaderMutator to httpfilter
@mbissa mbissa added this to the 1.83 Release milestone Jun 8, 2026
@mbissa mbissa added the Type: Feature New features or improvements in behavior label Jun 8, 2026
@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.56757% with 157 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.93%. Comparing base (cd25711) to head (1a1163b).
⚠️ Report is 16 commits behind head on master.

Files with missing lines Patch % Lines
internal/xds/xdsclient/xdsresource/grpc_service.go 58.16% 44 Missing and 20 partials ⚠️
internal/xds/bootstrap/bootstrap.go 63.09% 22 Missing and 9 partials ⚠️
internal/xds/xdsclient/clientimpl.go 0.00% 30 Missing ⚠️
internal/xds/httpfilter/headers.go 65.51% 15 Missing and 5 partials ⚠️
internal/xds/resolver/xds_resolver.go 46.15% 3 Missing and 4 partials ⚠️
internal/xds/httpfilter/extproc/ext_proc.go 85.00% 2 Missing and 1 partial ⚠️
internal/xds/xdsclient/pool.go 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9175      +/-   ##
==========================================
- Coverage   83.20%   82.93%   -0.27%     
==========================================
  Files         418      420       +2     
  Lines       33741    34090     +349     
==========================================
+ Hits        28073    28272     +199     
- Misses       4250     4364     +114     
- Partials     1418     1454      +36     
Files with missing lines Coverage Δ
internal/xds/httpfilter/httpfilter.go 87.50% <ø> (ø)
internal/xds/xdsclient/client.go 100.00% <ø> (ø)
...nternal/xds/xdsclient/xdsresource/unmarshal_lds.go 93.62% <100.00%> (+0.05%) ⬆️
internal/xds/xdsclient/pool.go 82.30% <60.00%> (-1.04%) ⬇️
internal/xds/httpfilter/extproc/ext_proc.go 84.92% <85.00%> (+8.39%) ⬆️
internal/xds/resolver/xds_resolver.go 85.66% <46.15%> (-1.17%) ⬇️
internal/xds/httpfilter/headers.go 65.51% <65.51%> (ø)
internal/xds/xdsclient/clientimpl.go 63.52% <0.00%> (-14.78%) ⬇️
internal/xds/bootstrap/bootstrap.go 63.76% <63.09%> (-0.18%) ⬇️
internal/xds/xdsclient/xdsresource/grpc_service.go 58.16% <58.16%> (ø)

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mbissa

mbissa commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for whitelisting allowed gRPC services (allowed_grpc_services) in the xDS bootstrap configuration, enabling secure connections to external processing servers with specified channel and call credentials. It also adds a HeaderMutator utility to compile and apply header mutations on gRPC metadata. The review feedback highlights several critical issues, including potential resource leaks in AllowedGrpcService.UnmarshalJSON if credential building fails midway, and multiple potential nil pointer dereferences when handling allowed gRPC services or credential plugins. Additionally, the reviewer recommends caching the bootstrap configuration in parseGRPCServiceConfig to avoid highly inefficient, repeated file I/O and JSON parsing.

Comment thread internal/xds/bootstrap/bootstrap.go
Comment thread internal/xds/xdsclient/clientimpl.go
Comment thread internal/xds/xdsclient/pool.go
Comment thread internal/xds/xdsclient/xdsresource/grpc_service.go
Comment thread internal/xds/xdsclient/xdsresource/grpc_service.go
Comment thread internal/xds/xdsclient/xdsresource/grpc_service.go
Comment thread internal/xds/httpfilter/extproc/ext_proc.go
Comment thread internal/xds/httpfilter/extproc/ext_proc.go
@easwars

easwars commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

I'd like to request that this PR be split into smaller PRs:

  • Changes to the xDS credentials registry to support configuring credentials using proto messages.
    • I currently don't see changes in this PR at all
  • Implementation of allowed_grpc_services field in the bootstrap config.
  • Implementation of header mutations.
  • Implementation of parsing the GrpcService proto.

Some of these PRs can be in review in parallel.

@easwars easwars assigned mbissa and unassigned eshitachandwani Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Feature New features or improvements in behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants