grpc / grpc-go

The Go language implementation of gRPC. HTTP/2 based RPC
https://grpc.io
Apache License 2.0
21.14k stars 4.4k forks source link

Exporting full methods #5876

Closed KenxinKun closed 1 year ago

KenxinKun commented 1 year ago

Please see the FAQ in our main README.md before submitting your issue.

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

Hi, we have a use case using grpc middleware / interceptors to check for permissions/authorization server-side when calling a particular gRPC endpoint. We're doing something like:

func NewPermissionsInterceptor(permission PermissionChecker) grpc.UnaryServerInterceptor {
    return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
        if !permission.HasPermission(ctx, info.FullMethod) {
            return nil, errors.New("permissions denied")
        }

        return handler(ctx, req)
    }
}

where the .HasPermission(...) method contains a hard coded copy of the methods used by the protoc-gen-gofullmethods command.

Would it be possible to allow the protoc command to export the method strings directly, so that we can access them? (they're currently written twice for client and server respectively, but not exposed as a package exported constant).

I believe this might be useful to others trying to make similar middleware / interceptors. If this is something you'd like to support I'd be happy to fork and contribute the development time with a PR :)

Proposed Solution

The protoc-gen-go-grpc plugin could export as constants the methods used by each rpc endpoint (see below for example).

Alternatives Considered

Create a custom plugin that just does this (we actually have one here, but it feels like its just an extension of the original plugin and other users might benefit from a similar solution).

Additional Context

Example minimal proto input:

syntax = "proto3";

package pet.v1;

import "google/type/datetime.proto";

option go_package = "github.com/bufbuild/buf-tour/petstore/gen/proto/go/pet/v1;petv1";

// ... omitted message types ...

service PetStore {
  rpc GetPet(GetPetRequest) returns (GetPetResponse) {}
  rpc PutPet(PutPetRequest) returns (PutPetResponse) {}
  rpc DeletePet(DeletePetRequest) returns (DeletePetResponse) {}
}

we then export the methods such as:

package petv1

const (
    PetStore_GetPet    = "/pet.v1.PetStore/GetPet"
    PetStore_PutPet    = "/pet.v1.PetStore/PutPet"
    PetStore_DeletePet = "/pet.v1.PetStore/DeletePet"
)

var (
    FullMethods = []string{
        PetStore_GetPet,
        PetStore_PutPet,
        PetStore_DeletePet,
    }
)
easwars commented 1 year ago

The output of protoc-gen-go-grpc does export a service description of this type, which contains method description for every method in the service.

For a service which looks like this: https://github.com/grpc/grpc-go/blob/master/test/grpc_testing/test.proto#L126-L156, the plugin exports a method description which looks like this: https://github.com/grpc/grpc-go/blob/master/test/grpc_testing/test_grpc.pb.go#L415-L456.

Does this not serve your needs? If so, can you please explain why.

If you would still like to go ahead and contribute a PR, you are welcome to do so. One thing to keep in mind when making such a change is to ensure that the newly added symbols are unique and do not clash with any existing names or have the possibility of clashing with some names in the future. Thanks.

KenxinKun commented 1 year ago

Hi @easwars thanks for the links! I just double checked and I don't think it quite works out because:

The remaining issue though is that we want to configure this interceptors so to avoid hardcoding a string with the method name ideally we want to just pick it from an export that is identifiable. Although I could loop over the array of method descriptors I wouldn't be able to select it with a well named variable:

easwars commented 1 year ago

@KenxinKun

Thanks for your reply. I'm still not convinced that an exported symbol is required. But as you mentioned, when you are ready to contribute a PR, please check back with us and we can have a more concrete discussion. Thank you.