grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.45k stars 760 forks source link

`credentials` parameter not implemented/unused when generating grpc-web code #1387

Closed Manuelraa closed 2 weeks ago

Manuelraa commented 7 months ago

When compiling grpc-web code the generated client's credentials argument is unused.

Therefore I have to manually provide credentials in request metadata.

Example/Reproduced in this repo: https://github.com/Manuelraa/grpc_web_creds_bug

Files

Example proto

syntax = "proto3";

package grpc_web_cred_bug.v1;

import "google/api/annotations.proto";

message Test {
    string name = 1;
}

message TestRequest {
    string name = 1;
}

message TestResponse {
    Test test = 1;
}

service TestService {
    rpc Test(TestRequest) returns (TestResponse) {
        option (google.api.http) = {
            post: "/v1/test"
        };
    };
}

Dockerfile for generation

FROM golang:1.21.3

WORKDIR /tmp

ENV GO111MODULE=on GOBIN=/usr/local/bin

# buf
RUN go install github.com/bufbuild/buf/cmd/buf@v1.28.1

# grpc-web
ADD \
    --chmod='755' \
    https://github.com/grpc/grpc-web/releases/download/1.5.0/protoc-gen-grpc-web-1.5.0-linux-x86_64 \
    /usr/local/bin/protoc-gen-grpc-web

# protobuf-javascript
ADD \
    https://github.com/protocolbuffers/protobuf-javascript/releases/download/v3.21.2/protobuf-javascript-3.21.2-linux-x86_64.tar.gz \
    /tmp/protobuf-javascript.tar.gz
RUN tar -xvzf /tmp/protobuf-javascript.tar.gz -C /usr/local/bin/ --strip-components 1 bin/protoc-gen-js

WORKDIR /workspace
ENV HOME=/tmp
ENTRYPOINT [ "buf" ]

generate script

#!/usr/bin/env bash

set -xe

IMAGE='manuelraa/buf_grpc_test'
USER="$(id -u):$(id -g)"

# Build buf docker image
docker build --file Dockerfile-buf --tag "$IMAGE" './'

# Buf generate
docker run \
    --volume "$(pwd):/workspace" \
    --workdir /workspace/api \
    --user "$USER" \
    "$IMAGE" generate

Code generation and the problem

Generating files

[user@rockydever9 grpc_web_creds_bug]$ ./buf_generate.sh 
+ IMAGE=manuelraa/buf_grpc_test
++ id -u
++ id -g
+ USER=1000:1000
+ docker build --file Dockerfile-buf --tag manuelraa/buf_grpc_test ./
[+] Building 0.6s (13/13) FINISHED                                                                                                                                                                                                                                                                       docker:default
 => [internal] load build definition from Dockerfile-buf                                                                                                                                                                                                                                                           0.0s
 => => transferring dockerfile: 716B                                                                                                                                                                                                                                                                               0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                                                                                                  0.0s
 => => transferring context: 2B                                                                                                                                                                                                                                                                                    0.0s
 => [internal] load metadata for docker.io/library/golang:1.21.3                                                                                                                                                                                                                                                   0.4s
 => [1/7] FROM docker.io/library/golang:1.21.3@sha256:b113af1e8b06f06a18ad41a6b331646dff587d7a4cf740f4852d16c49ed8ad73                                                                                                                                                                                             0.0s
 => https://github.com/grpc/grpc-web/releases/download/1.5.0/protoc-gen-grpc-web-1.5.0-linux-x86_64                                                                                                                                                                                                                0.2s
 => https://github.com/protocolbuffers/protobuf-javascript/releases/download/v3.21.2/protobuf-javascript-3.21.2-linux-x86_64.tar.gz                                                                                                                                                                                0.2s
 => CACHED [2/7] WORKDIR /tmp                                                                                                                                                                                                                                                                                      0.0s
 => CACHED [3/7] RUN go install github.com/bufbuild/buf/cmd/buf@v1.28.1                                                                                                                                                                                                                                            0.0s
 => CACHED [4/7] ADD     --chmod=755     https://github.com/grpc/grpc-web/releases/download/1.5.0/protoc-gen-grpc-web-1.5.0-linux-x86_64     /usr/local/bin/protoc-gen-grpc-web                                                                                                                                    0.0s
 => CACHED [5/7] ADD     https://github.com/protocolbuffers/protobuf-javascript/releases/download/v3.21.2/protobuf-javascript-3.21.2-linux-x86_64.tar.gz     /tmp/protobuf-javascript.tar.gz                                                                                                                       0.0s
 => CACHED [6/7] RUN tar -xvzf /tmp/protobuf-javascript.tar.gz -C /usr/local/bin/ --strip-components 1 bin/protoc-gen-js                                                                                                                                                                                           0.0s
 => CACHED [7/7] WORKDIR /workspace                                                                                                                                                                                                                                                                                0.0s
 => exporting to image                                                                                                                                                                                                                                                                                             0.0s
 => => exporting layers                                                                                                                                                                                                                                                                                            0.0s
 => => writing image sha256:30a936785e5c45ba423451b858a0b2aba74c2670f70e081491ce3dbfbd677fe7                                                                                                                                                                                                                       0.0s
 => => naming to docker.io/manuelraa/buf_grpc_test                                                                                                                                                                                                                                                                 0.0s
++ pwd
+ docker run --volume /tmp/grpc_web_creds_bug:/workspace --workdir /workspace/api --user 1000:1000 manuelraa/buf_grpc_test generate
Warning: Generated file name "./grpc_web_cred_bug/v1/test_pb.js" does not conform to the Protobuf generation specification. Note that the file name must be relative, use "/" instead of "\", and not use "." or ".." as part of the file name. Buf will continue without error here, but please raise an issue with the maintainer of the plugin and reference https://github.com/protocolbuffers/protobuf/blob/95e6c5b4746dd7474d540ce4fb375e3f79a086f8/src/google/protobuf/compiler/plugin.proto#L122
Warning: Generated file name "./google/api/http_pb.js" does not conform to the Protobuf generation specification. Note that the file name must be relative, use "/" instead of "\", and not use "." or ".." as part of the file name. Buf will continue without error here, but please raise an issue with the maintainer of the plugin and reference https://github.com/protocolbuffers/protobuf/blob/95e6c5b4746dd7474d540ce4fb375e3f79a086f8/src/google/protobuf/compiler/plugin.proto#L122
Warning: Generated file name "./google/api/annotations_pb.js" does not conform to the Protobuf generation specification. Note that the file name must be relative, use "/" instead of "\", and not use "." or ".." as part of the file name. Buf will continue without error here, but please raise an issue with the maintainer of the plugin and reference https://github.com/protocolbuffers/protobuf/blob/95e6c5b4746dd7474d540ce4fb375e3f79a086f8/src/google/protobuf/compiler/plugin.proto#L122

Checking the generated code

Then checking the generated test_grpc_web_pb.js file we see the credentials parameter is not handled.

...

/**
 * @param {string} hostname
 * @param {?Object} credentials
 * @param {?grpc.web.ClientOptions} options
 * @constructor
 * @struct
 * @final
 */
proto.grpc_web_cred_bug.v1.TestServiceClient =
    function(hostname, credentials, options) {
  if (!options) options = {};
  options.format = 'text';

  /**
   * @private @const {!grpc.web.GrpcWebClientBase} The client
   */
  this.client_ = new grpc.web.GrpcWebClientBase(options);

  /**
   * @private @const {string} The hostname
   */
  this.hostname_ = hostname.replace(/\/+$/, '');

};

...
kobofare commented 6 months ago

me too.

sampajano commented 2 months ago

@Manuelraa Thanks so much for filing a detailed bug, with a repo for reproducing as well!

I think you're indeed right that the credentials is not being used.

I don't have all the history of the project, but from what i can see, even from the very early versions of the generator code (example) , the credentials field was not used.

@stanley-cheung Hey Stanley I wonder if you have some background on the credentials parameter? Was it ever utilized in grpc web client? Thanks!

sampajano commented 2 weeks ago

I guess this is working-as-intended so closing for now.. :) We will keep in mind to clean-up the credentials field in future API iterations.

Feel free to re-open and follow up later if there are more updates. Thanks!