libopenstorage / openstorage

A multi-host clustered implementation of the open storage specification
Apache License 2.0
523 stars 118 forks source link

Change OpenStorageSchedule API parameter type to string #2454

Closed twang-ps closed 3 months ago

twang-ps commented 3 months ago

What this PR does / why we need it:
This PR is to resolve a compilation issue on release branch where it doesn't support enum type path parameter in protobuf REST API.

This fix is to cast Job.Type enum type to string type in request data structures.

Issue: After I cherry pick PR https://github.com/libopenstorage/openstorage/pull/2426 onto release-9.8, it failed to build. It complains the following error, which means protoc-gen-grpc-gateway of this version doesn't support a enum type in path parameter.

service OpenStorageSchedule {
  // Inspect queries information of a schedule by type and ID
  rpc Inspect(SdkInspectScheduleRequest)
    returns (SdkInspectScheduleResponse) {
      option(google.api.http) = {
        get: "/v1/schedules/{type}/{id}"
      };
    }
message SdkInspectScheduleRequest {
  // ID of the schedule
  string id = 1;
  // Type is schedule type
  Job.Type type = 2;
}
root@ip-10-13-177-24:~/git/go/src/github.com/libopenstorage/openstorage# make docker-proto
docker pull quay.io/openstorage/osd-proto:pre-gomodules
pre-gomodules: Pulling from openstorage/osd-proto
Digest: sha256:76b09a1a0231f9cf78f22eecd243d7ddb2bcc464e0cea9493a2d08dfe534ef14
Status: Image is up to date for quay.io/openstorage/osd-proto:pre-gomodules
quay.io/openstorage/osd-proto:pre-gomodules
docker run \
        --privileged --rm \
        -v /root/git/go/src/github.com/libopenstorage/openstorage:/go/src/github.com/libopenstorage/openstorage \
        -e "GOPATH=/go" \
        -e "DOCKER_PROTO=yes" \
        -e "PATH=/bin:/usr/bin:/usr/local/bin:/go/bin:/usr/local/go/bin" \
        quay.io/openstorage/osd-proto:pre-gomodules \
                make proto mockgen
>>> Generating protobuf definitions from api/api.proto
protoc -I /go/src/github.com/libopenstorage/openstorage \
        -I /usr/local/include \
        -I /go/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
        --go_out=plugins=grpc:. \
        /go/src/github.com/libopenstorage/openstorage/api/api.proto
protoc -I /go/src/github.com/libopenstorage/openstorage \
        -I /usr/local/include \
        -I /go/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
        --grpc-gateway_out=logtostderr=true:. \
        /go/src/github.com/libopenstorage/openstorage/api/api.proto
--grpc-gateway_out: template: client-rpc-request-func:30:53: executing "client-rpc-request-func" at <$param.ConvertFuncEx...>: error calling ConvertFuncExpr: unsupported field type TYPE_ENUM of parameter type in OpenStorageSchedule.Inspect
make: *** [Makefile:201: proto] Error 1
Makefile:184: recipe for target 'docker-proto' failed
make: *** [docker-proto] Error 2

Cause analysis libopenstorage/openstorage is built inside a container. But the images used for master branch and release branch are different. Master branch uses quay.io/openstorage/osd-proto:lastest while release branch uses quay.io/openstorage/osd-proto:pro-gomodules The version of protoc-gen-grpc-gateway is v1.16.0 on master branch while it's v1.4.1 on release branch On master branch, it’s able to generate the following code to parse the enum type:

func request_OpenStorageSchedule_Inspect_0(ctx context.Context, marshaler runtime.Marshaler, client OpenStorageScheduleClient, req *http.Request, pathParams map[string]string) (proto.Message, runtime.ServerMetadata, error) {
    var protoReq SdkInspectScheduleRequest
    var metadata runtime.ServerMetadata

    var (
        val string
        e   int32
        ok  bool
        err error
        _   = err
    )

    val, ok = pathParams["type"]
    if !ok {
        return nil, metadata, status.Errorf(codes.InvalidArgument, "missing parameter %s", "type")
    }

    e, err = runtime.Enum(val, Job_Type_value)

    if err != nil {
        return nil, metadata, status.Errorf(codes.InvalidArgument, "type mismatch, parameter: %s, error: %v", "type", err)
    }

    protoReq.Type = Job_Type(e)

On release branch, the code generated could not handle the parsing

func request_OpenStorageSchedule_Inspect_0(ctx context.Context, marshaler runtime.Marshaler, client OpenStorageScheduleClient, req *http.Request, pathParams map[string]string) (proto.Message, runtime.ServerMetadata, error) {
    var protoReq SdkInspectScheduleRequest
    var metadata runtime.ServerMetadata

    var (
        val string
        ok  bool
        err error
        _   = err
    )

    val, ok = pathParams["type"]
    if !ok {
        return nil, metadata, status.Errorf(codes.InvalidArgument, "missing parameter %s", "type")
    }

    protoReq.Type, err = runtime.String(val)

    if err != nil {
        return nil, metadata, status.Errorf(codes.InvalidArgument, "type mismatch, parameter: %s, error: %v", "type", err)
    }

Testing Notes
Add testing output or passing unit test output here.

Special notes for your reviewer:
Add any notes for the reviewer here.