-
Notifications
You must be signed in to change notification settings - Fork 293
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
Restore pclientd
GRPC proxy functionality
#3120
Comments
If I run pclientd and pcli locally, using
vs custody mode:
How did you discover this error? I'm surprised that our CI hasn't failed on this yet, because we shipped the refactored protos as part of v0.61.0, and interchaintest has been updated to use those new import paths, and ostensibly the proto-blob -> pclientd -> pd exercise within that test suite is still passing. |
I didn't do anything to discover the error, I just thought about it and realized it would be a problem. I would also expect it to be a problem that InterchainTest would notice. I'm not sure what InterchainTest is actually running, but we don't exercise this code path in our own smoke tests. |
Closing as completed because of the stopgap situation. A real HTTP proxy would be cool and is in no way an important use of time. |
Is your feature request related to a problem? Please describe.
pclientd
has an integrated GRPC proxy that allowspclientd
to act as a single endpoint for client requests:pclientd
connects to.Unfortunately, the way this feature was originally implemented caused it to be broken by protobuf refactoring in #3091, which split up and renamed all of the query services.
The original (current) implementation created tuple structs wrapping a shared
Channel
, each implementing Tonic'sNamedService
trait, which uses a&'static str
associated constant to name the service, e.g.,We have one of these for each proxied service, and then plug them into the Tonic server builder, which constructs a router that routes over all of them.
The problem with this approach is that each service to be proxied needs its own type and implementation and to be plugged in separately. So when we split up the queries, it breaks -- because it only proxies specific services -- and restoring functionality would mean creating many more services.
Describe the solution you'd like
Ideally, we'd have a proxy that would be generic over the service name, allowing us to have the following behavior:
Having a filter list is nice but non-essential, the important part is being able to configure the proxy as a generic fallback.
In the Tokio discord, @LucioFranco pointed to the
Routes::into_router
method as a way to solve the problem. This converts into an Axum router, which has afallback
method that can be configured however we want:To support a filter list, we could use a
RegexSet
as the configuration for whatever object we pass as the custom handler. Then our code could specify service filters like["penumbra.core.component.*"]
to proxy any kind of component query service.Describe alternatives you've considered
If we run into obstacles with the above approach, we can always use copy-paste technology to extend the current solution across all of our current services. This is suboptimal but it's better than having broken functionality. Our InterchainTest integration relies on
pclientd
having this behavior, so we shouldn't release without some kind of fix (cc @conorsch).The text was updated successfully, but these errors were encountered: