Skip to content

Conversation

@bzp2010
Copy link
Contributor

@bzp2010 bzp2010 commented Feb 27, 2025

Add a patch to the nginx upstream module to allow grpc_pass to be used without upstream trailers being passed through to the client.

For apache/apisix#11988

@bzp2010 bzp2010 requested a review from Copilot March 29, 2025 18:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds support to use gRPC without passing upstream trailers by introducing a gRPC server implementation for testing purposes. The changes include a new gRPC server written in Go (server.go), protocol definitions (route.proto) along with generated code (route.pb.go), and a CI configuration update to run the required gRPC setup script.

Reviewed Changes

Copilot reviewed 5 out of 13 changed files in this pull request and generated no comments.

File Description
t/assets/grpc/server.go Introduces a simple gRPC server implementation for routing.
t/assets/grpc/a6/route.proto Defines the protocol for route queries and responses.
t/assets/grpc/a6/route.pb.go Generated Go code for the defined protobuf messages and service.
.github/workflows/ci.yml Updates CI workflow to run an additional setup script for gRPC.
Files not reviewed (8)
  • lib/resty/apisix/upstream.lua: Language not supported
  • patch/1.25.3.1/nginx-upstream_pass_trailers.patch: Language not supported
  • patch/1.27.1.1/nginx-upstream_pass_trailers.patch: Language not supported
  • src/ngx_http_apisix_module.c: Language not supported
  • src/ngx_http_apisix_module.h: Language not supported
  • t/assets/grpc/go.mod: Language not supported
  • t/assets/grpc/setup.sh: Language not supported
  • t/upstream_pass_trailers.t: Language not supported
Comments suppressed due to low confidence (2)

t/assets/grpc/server.go:40

  • [nitpick] Consider clarifying the error message to be more descriptive, e.g., 'Route name cannot be empty'.
return nil, status.Errorf(codes.InvalidArgument, "query params invalid")

.github/workflows/ci.yml:35

  • Ensure that the 'setup.sh' script exists at the specified path to avoid CI failures.
cd t/assets/grpc && ./setup.sh > build.log 2>&1 || (cat build.log && exit 1) && cd ../../..

@bzp2010 bzp2010 requested review from AlinsRan and membphis March 30, 2025 02:47
@bzp2010 bzp2010 marked this pull request as ready for review March 30, 2025 02:47
@bzp2010 bzp2010 changed the title feat: allow not pass trailers in nginx upstream feat: allow skip pass trailers in nginx grpc upstream Mar 30, 2025
@bzp2010
Copy link
Contributor Author

bzp2010 commented Mar 30, 2025

GM test failure has nothing to do with the current PR, so please ignore it.

@bzp2010 bzp2010 requested review from Revolyssup and nic-6443 March 31, 2025 03:15
@bzp2010 bzp2010 merged commit 802cc05 into main Apr 16, 2025
4 of 5 checks passed
@bzp2010 bzp2010 deleted the bzp/feat-block-pass-trailers branch April 16, 2025 09:34
@bzp2010 bzp2010 restored the bzp/feat-block-pass-trailers branch April 16, 2025 09:34
@bzp2010 bzp2010 deleted the bzp/feat-block-pass-trailers branch April 16, 2025 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants