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

PerRPCCredentials should have access to more data about the call than just the “audience”. #3019

Closed
shanel-at-google opened this issue Sep 12, 2019 · 1 comment

Comments

@shanel-at-google
Copy link
Contributor

If decisions made by PerRPCCredentials need to know either the Peer’s host or the full method name currently there is no way to access that information. (The “audience” generated by the createAudiance method cuts off the method data after the slash in the method name.)

I’d propose attaching additional data pulled from the CallHdr to the context so PerRPCCredentials could make use of it.

Use case(s) - what problem will this feature solve?

If for example you wanted to decide to fail an RPC to a particular host, or to a particular port, or prevent the use of a specific method this would allow that.

Proposed Solution

package credentials

type RequestInfo struct {
	Peer string
	Method string
}

We would build a RequestInfo based on the CallHdr and attach it to the ctx at the start of the createHeaderFields method in internal/transport/http2_client.go using context.WithValue

This would allow any GetRequestMetadata calls to have access to the RequestInfo data by way of the passed context value.

This would be similar to the Java implementation: https://grpc.github.io/grpc-java/javadoc/io/grpc/CallCredentials.RequestInfo.html

Alternatives Considered

It could be argued that we could attach this data to the context someplace earlier. I am a believer in YAGNI, so would prefer to make the most minimal change possible. That said, if something further up does eventually need this data, moving where it is attached should not be too difficult of a process.

Another option that is possible is attaching the necessary data in the Peer object and let the call in NewStream that attaches the peer to the context do the work for us. In some ways it seems a bit weird to have the service/method being called in the individual RPC be in an object meant to store the remote server’s info, but I could possibly be persuaded...

Additional Context

@dfawley dfawley added the P2 label Sep 26, 2019
shanel-at-google added a commit to shanel-at-google/grpc-go that referenced this issue Sep 30, 2019
… request method (though could later be expanded to pass more info) so that things like GetRequestMetadata can be used to apply logic based on that data.

This is a fix for grpc#3019
shanel-at-google added a commit to shanel-at-google/grpc-go that referenced this issue Oct 1, 2019
… request method (though could later be expanded to pass more info) so that things like GetRequestMetadata can be used to apply logic based on that data.

This is a fix for grpc#3019
shanel-at-google added a commit to shanel-at-google/grpc-go that referenced this issue Oct 1, 2019
… request method (though could later be expanded to pass more info) so that things like GetRequestMetadata can be used to apply logic based on that data.

This is a fix for grpc#3019
shanel-at-google added a commit to shanel-at-google/grpc-go that referenced this issue Oct 1, 2019
… request method (though could later be expanded to pass more info) so that things like GetRequestMetadata can be used to apply logic based on that data.

This is a fix for grpc#3019
dfawley pushed a commit that referenced this issue Oct 4, 2019
Add a RequestInfo struct which initially is used for passing the full request method (though could later be expanded to pass more info) so that things like GetRequestMetadata can be used to apply logic based on that data.

This is a fix for #3019
@dfawley
Copy link
Member

dfawley commented Oct 4, 2019

Fixed by #3057

@dfawley dfawley closed this as completed Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants