Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

THREESCALE-9976 Fix GRPC #1419

Merged
merged 5 commits into from
Nov 8, 2023
Merged

THREESCALE-9976 Fix GRPC #1419

merged 5 commits into from
Nov 8, 2023

Conversation

eguzki
Copy link
Member

@eguzki eguzki commented Oct 31, 2023

What

Fixes https://issues.redhat.com/browse/THREESCALE-9976

GRPC leverages the POST HTTP2 method (GRPC Spec) . APIcast reads the body looking for request params for POST based requests using ngx.req.read_body. However, ngx.req.read_body does not support HTTP2 and it blocks the request. The clients receive connection timeout errors.

Added development environment for GRPC scenario

Verification steps

  • Build docker image from this git branch
make runtime-image IMAGE_NAME=apicast-test
  • Run grpc dev environment
cd dev-environments/grpc
make gateway-certs
make upstream-certs
make gateway IMAGE_NAME=apicast-test
  • Download GRPC client
make grpcurl
  • Run request to echo endpoint
bin/grpcurl -vv -insecure -H "app_id: abc123" -H "app_key: abc123" -authority gateway.example.com 127.0.0.1:8443 main.HelloWorld/Greeting

The command should return the echo response

Resolved method descriptor:
rpc Greeting ( .main.GreetingMessage ) returns ( .main.GreetingReply );

Request metadata to send:
app_id: abc123
app_key: abc123

Response headers received:
content-type: application/grpc
date: Fri, 03 Nov 2023 11:41:16 GMT
server: openresty

Estimated response size: 32 bytes

Response contents:
{
  "hostname": "d7728ab86df0",
  "clientAddress": "172.19.0.3:53564"
}

Response trailers received:
(empty)
Sent 0 requests and received 1 response

GRPC relies on POST HTTP2 method and ngx.req.read_body does not support HTTP2

Signed-off-by: Eguzki Astiz Lezaun <eastizle@redhat.com>
@eguzki eguzki changed the title Dev environment GRPC THREESCALE-9976 Fix GRPC Nov 3, 2023
@eguzki eguzki marked this pull request as ready for review November 3, 2023 11:28
@eguzki eguzki requested a review from a team as a code owner November 3, 2023 11:28
gateway/src/apicast/configuration/service.lua Show resolved Hide resolved
dev-environments/grpc/Makefile Show resolved Hide resolved
gateway/src/apicast/configuration/service.lua Outdated Show resolved Hide resolved
gateway/src/apicast/configuration/service.lua Outdated Show resolved Hide resolved
@tkan145
Copy link
Contributor

tkan145 commented Nov 3, 2023

Not sure why GH print the review twice 😕

@eguzki eguzki requested a review from tkan145 November 3, 2023 15:46
@@ -22,11 +24,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- In boot mode on `init_worker` check configuration expiration [PR #1399](https://github.com/3scale/APIcast/pull/1399) [THREESCALE-9003](https://issues.redhat.com/browse/THREESCALE-9003)
- Removes the warning message at the bootstrap [PR #1398](https://github.com/3scale/APIcast/pull/1398) [THREESCALE-7942](https://issues.redhat.com/browse/THREESCALE-7942)
- Set NGiNX variable variables_hash_max_size to 2048 to avoid startup warning [PR #1395](https://github.com/3scale/APIcast/pull/1395) [THREESCALE-7941](https://issues.redhat.com/browse/THREESCALE-7941)
- Dev environment on aarch64 host [PR #1381](https://github.com/3scale/APIcast/pull/1381)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's just a small thing but I think this is an unrelated change?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just the IDE tool removing trailing spaces


### Added

- Doc: Policy Development Tutorial [PR #1384](https://github.com/3scale/APIcast/pull/1384)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just the IDE tool removing trailing spaces

@tkan145
Copy link
Contributor

tkan145 commented Nov 7, 2023

However, ngx.req.read_body does not support HTTP2 and it blocks the request.

Do you mean ngx.req.get_post_args? As the line client request body saved to a temporary file can be seen from the log, so I think ngx.req.read_body works with http2

Ignore my previous comment. just found this one openresty/lua-nginx-module#2172

@eguzki
Copy link
Member Author

eguzki commented Nov 8, 2023

However, ngx.req.read_body does not support HTTP2 and it blocks the request.

Do you mean ngx.req.get_post_args? As the line client request body saved to a temporary file can be seen from the log, so I think ngx.req.read_body works with http2

Ignore my previous comment. just found this one openresty/lua-nginx-module#2172

From https://github.com/openresty/lua-nginx-module#ngxreqread_body

Due to the stream processing feature of HTTP/2 or HTTP/3, this api could potentially block the entire request. Therefore, this api is effective only when HTTP/2 or HTTP/3 requests send content-length header. For requests with versions lower than HTTP/2, this api can still be used without any problems.

@eguzki eguzki merged commit c7476bb into master Nov 8, 2023
11 checks passed
@eguzki eguzki deleted the dev-environment-grpc branch November 8, 2023 08:40
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.

2 participants