Verify that gateway ReST-ish API can still be used! #6150
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 29, 2023
absoludity
added a commit
that referenced
this pull request
Mar 29, 2023
<!-- Before you open the request please review the following guidelines and tips to help it be more easily integrated: - Describe the scope of your change - i.e. what the change does. - Describe any known limitations with your change. - Please run any tests or examples that can exercise your modified code. Thank you for contributing! --> ### Description of the change <!-- Describe the scope of your change - i.e. what the change does. --> This is split out from the investigation done in #6097 and will be followed by a bunch more switching other services and plugins away from the improbable-eng gRPC backend (see #6013) ### Benefits <!-- What benefits will be realized by the code change? --> This PR sets a clear example of switching one of the gRPC servers, the core plugin service, away from the improbable-eng server to the connect gRPC server. The switch of the server is trivial, the difficulty was in getting our kubeapps-apis service working correctly with a combination of improbable and connect gRPC servers. As a side-effect, I updated the liveness and readiness checks to use a grpc request (since we no longer have the gateway endpoint for the core plugin service). Initially I'd tried using the existing server as the default and routing certain routes for transitioned servers to the new handler, but reading http2 headers requires writing the http2 settings frame, which meant the new handler, for some reason that I wasn't able to determine, did not accept the connections. I then switched to default to the new server and proxying to the old improbable for unhandled routes, which worked for all requests that are initiated by the dashboard, but not those initiated from within the kubeapps-apis server itself (like where the GetResources handler makes a gRPC request to the packages plugin for the resource refs). In those cases it failed. I initially assumed this was something to do with the proxying, but I couldn't find the issue at first, so instead transitioned the resources plugin over as well (in #6097), which ended up needing much more changes than I wanted in a PR, including handling the different ways auth creds are passed by the two libraries etc. But even after transitioning the resources plugin, I was still hitting an error for proxied requests (this time when the resources plugin called the packaging plugin). After much more digging, I finally found it was because the [golang httputil.ReverseProxy proxies requests not transports](golang/go#33452). So the gRPC (http2) requests were being proxied using an http1 transport which was resulting in a 404 as the server was not recognising the request (gRPC assumes http2). Once understood, the solution to use a different reverse proxy (just different transport) depending on the request was straight forward. Once I understood that, I was then able to go back to my original intention of a small PR demonstrating changing just a single service, the plugin service, which is what this PR is :) ### Possible drawbacks <!-- Describe any known limitations with your change --> ~~We lose the Gateway ReST-ish http endpoints. The only place we were using the gateway http endponts was in the liveness/readiness checks, which I've switched here too. We do publish the openapi docs for the gateway, but have not made any commitment to supporting it. Some devs have used the gateway with Postman for testing kubeapps-apis locally, but postman also supports gRPC now for a year.~~ Actually, I may even be able to re-use the proxy that I've setup here to keep the gateway available too. I'll check. EDIT: Indeed we can - #6150 ### Applicable issues <!-- Enter any applicable Issues here (You can reference an issue using #) --> - ref #6013 ### Additional information <!-- If there's anything else that's important and relevant to your pull request, mention that information here.--> I'll follow up with a PR for switching the resources gRPC service, then PRs for packaging, repository and plugins etc. --------- Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
absoludity
force-pushed
the
6013-connect-grpc-backend-3
branch
from
ca82111 to
af221c2
Compare
✅ Deploy Preview for kubeapps-dev canceled.
|
Signed-off-by: Michael Nelson <minelson@vmware.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the change
Just verifying that we actually don't need to lose the gRPC Gateway functionality (of the ReST-ish API translation to gRPC).
In the end, all that was required was to update the gateway args so that it points to the new gRPC connect service, and then it just continues to work (I'd thought it was dependent on a piece that we'd be removing, but it's not).
Local example check:
I assume I'll be able to hook it up similarly when using other plugin services with auth.
Benefits
We keep the OpenAPI gateway API :)
Possible drawbacks
None
Applicable issues
Additional information