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

fix: support GRPC with Gateway's HTTP listener #5128

Merged
merged 1 commit into from
Nov 27, 2023
Merged

fix: support GRPC with Gateway's HTTP listener #5128

merged 1 commit into from
Nov 27, 2023

Conversation

programmer04
Copy link
Member

@programmer04 programmer04 commented Nov 8, 2023

What this PR does / why we need it:

This adds support for GRPC over HTTP (without TLS). The only supported version of the HTTP protocol is currently HTTP/2 for GRPC. GRPC without encryption is much less popular but supported.

For Kong Gateway, the HTTP listener must have the http2 option enabled (configured with env var PROXY_LISTEN) to support GRPC. By default it is only enabled for HTTPS, but not for HTTP. Currently, Kong Gateway 3.5 doesn't offer simultaneous support of HTTP/1.1 and HTTP/2 without TLS on a single TCP socket (PROXY_LISTEN='0.0.0.0:80 http2') for the listener is set. A client has to connect with HTTP/2 from the start which breaks all other HTTP-related tests (the same would be with users set-ups). Thus it is tested on an isolated instance of Kong Gateway. There is a separate issue for documenting the required setting and its implication in Kong docs - #5134 for the feature introduced in this PR.

For HTTPS both HTTP/1 and HTTP/2 work as expected with http2 on a single socket, because the mechanism of protocol negation during TLS handshake (TLS-ALPN) is used as described in RFC 7540 Section 3.3, hence having PROXY_LISTEN='0.0.0.0:8443 http2 ssl' as default is sensible.

Which issue this PR fixes:

Fixes #4273

Special notes for your reviewer:

Opportunistically, all integration tests related to GRPC support have been moved to the isolated since we intend to migrate all tests someday. See #4823.

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@programmer04 programmer04 added area/feature New feature or request do not merge let the author merge this, don't merge for them. fix labels Nov 8, 2023
@programmer04 programmer04 added this to the KIC v3.1.x milestone Nov 8, 2023
@programmer04 programmer04 self-assigned this Nov 8, 2023
Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (250c634) 76.8% compared to head (37af9ee) 75.7%.

Files Patch % Lines
...ternal/dataplane/translator/translate_grpcroute.go 72.7% 2 Missing and 1 partial ⚠️
internal/controllers/gateway/route_utils.go 0.0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #5128     +/-   ##
=======================================
- Coverage   76.8%   75.7%   -1.1%     
=======================================
  Files        170     170             
  Lines      19056   19060      +4     
=======================================
- Hits       14639   14436    -203     
- Misses      3588    3801    +213     
+ Partials     829     823      -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@programmer04 programmer04 removed do not merge let the author merge this, don't merge for them. fix labels Nov 24, 2023
@programmer04 programmer04 marked this pull request as ready for review November 24, 2023 16:53
@programmer04 programmer04 requested a review from a team as a code owner November 24, 2023 16:53
@programmer04 programmer04 enabled auto-merge (squash) November 24, 2023 16:53
Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/feature New feature or request size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GRPCRoute: Not supported by HTTP Listener.
2 participants