Unable to mark the body parameter as REQUIRED when part of the proto message is path param

[HoaiDang-work]

🐛 Bug Report

We have the below proto description with openAPI annotations. FederatedIdentity identity = 4 [(google.api.field_behavior) = REQUIRED]; is request body param which is required. However, part of the identity, the type field is also path param /v1/identity/{identity.type}/register-by-link/cancel

rpc cancelRegisterIdentityByInstructionLink(MessagingIdentityRequest) returns (CancelRegistrationByLinkResponse) {
    option (google.api.method_visibility).restriction = "EXTERNAL";
    option (google.api.http) = {
      post: "/v1/identity/{identity.type}/register-by-link/cancel"
      body: "*"
    };
}

message MessagingIdentityRequest {
  common.ServiceRequestIdentity requestIdentity = 1 [(google.api.field_visibility).restriction = "HIDDEN"];
  string userId = 2 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "The provisioned User ID."}, (google.api.field_behavior) = REQUIRED];
  string companyId = 3 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "The company ID where the corporate phone number belongs."}, (google.api.field_behavior) = REQUIRED];
  //Contains 2 properties: `id` (string, required) and `attributes` (array of objects). Property `id` is the corporate phone number. Property `attributes` contains `com.leapxpert.official.account.phone.number` for key `name`, `STRING` for key `type` and corporate phone number for key `values`.
  FederatedIdentity identity = 4 [(google.api.field_behavior) = REQUIRED];
  string corporateAccountId = 5 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "The corporate phone number ID."}];
}

message FederatedIdentity {
  IdentityProviderType type = 1;
  string id = 2 [(common.sensitive) = true, (common.secureLogFormatter) = PHONE_OR_EMAIL];
  repeated MultiValueAttribute attributes = 3;
  IdentityScope scope = 4;
  // database entity id
  string objectId = 5;
  string companyId = 6;
  FederatedIdentityVerificationState verificationState = 7;
}

In the generated openapi spec, the identity is not marked as required.

Expected behavior

The remaining parts of identity should be marked as required.

Actual Behavior

Only the part including in the path param is marked as required, the remaining parts as request body params are not marked as required.

Your Environment

MacOS M1

[johanbrandhorst]

Thanks for your detailed bug report. This sounds like a reasonable bug, I expect the logic for parsing the path parameters and marking them required doesn't take this annotation into account. Would you be willing to help contribute a fix? The logic for this is in here: https://github.com/grpc-ecosystem/grpc-gateway/blob/main/protoc-gen-openapiv2/internal/genopenapi/template.go.

[HoaiDang-work]

I'm not familiar with Golang to contribute quality code to fix the problem 😓

[MahikaJaguste]

@johanbrandhorst - I would like to try to fix this

[johanbrandhorst]

Go for it

[MahikaJaguste]

Hello @johanbrandhorst , I was able to write a fix for this issue. To test it, I installed protoc-gen-openapiv2 locally and used it with protoc to generate the swagger.json for an example .proto file and it gave the expected output.

However, I am unable to write a test case in template_test.go which incorporates the "*" wildcard body feature in the above fashion. Example:

books.proto

syntax = "proto3";

package notes.v1;
option go_package = "./books/v1";

import "google/api/annotations.proto";
import "protoc-gen-openapiv2/options/annotations.proto";
import "google/api/field_behavior.proto";

option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_swagger) = {
    info: {
        title: "Notes";
        version: "1.0";
        contact: {
            name: "abc";
            url: "https://github.com/abc/notes";
            email: "[email protected]";
        };
        license: {
            name: "BSD 3-Clause License";
            url: "https://github.com/abc/notes/LICENSE";
        };
    };

    schemes: HTTP;
    schemes: HTTPS;
    consumes: "application/json";
    produces: "application/json";
};

service BookService {
    rpc AddBook(AddBookRequest) returns (Book) {
        option (google.api.http) = {
            post: "/v1/books/{book.type}"
            body: "*"
        };
    };
}

message Book {
    string name = 1;
    string type = 2;
}

message AddBookRequest {
    Book book = 1 [(google.api.field_behavior) = REQUIRED];
    uint32 libraryId = 2 [(google.api.field_behavior) = REQUIRED];
    bool isLatestEdition = 3;
}

To simulate this in a test,

func TestRenderServicesOpenapiRequiredBodyContainingPathParam(t *testing.T) {

	var fieldBehaviorRequired = []annotations.FieldBehavior{annotations.FieldBehavior_REQUIRED}
	var requiredFieldOptions = new(descriptorpb.FieldOptions)
	proto.SetExtension(requiredFieldOptions, annotations.E_FieldBehavior, fieldBehaviorRequired)

	bookDesc := &descriptorpb.DescriptorProto{
		Name: proto.String("Book"),
		Field: []*descriptorpb.FieldDescriptorProto{
			{
				Name:   proto.String("name"),
				Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
				Number: proto.Int32(1),
			},
			{
				Name:   proto.String("type"),
				Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
				Number: proto.Int32(2),
			},
		},
	}
	addBookReqDesc := &descriptorpb.DescriptorProto{
		Name: proto.String("AddBookReq"),
		Field: []*descriptorpb.FieldDescriptorProto{
			{
				Name:     proto.String("book"),
				Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
				TypeName: proto.String(".example.Book"),
				Number:   proto.Int32(1),
				Options:  requiredFieldOptions,
			},
			{
				Name:     proto.String("libraryId"),
				Type:     descriptorpb.FieldDescriptorProto_TYPE_UINT32.Enum(),
				Number:   proto.Int32(2),
				Options:  requiredFieldOptions,
			},
			{
				Name:     proto.String("isLatestEdition"),
				Type:     descriptorpb.FieldDescriptorProto_TYPE_BOOL.Enum(),
				Number:   proto.Int32(3),
			},
		},
	}
	meth := &descriptorpb.MethodDescriptorProto{
		Name:       proto.String("AddBookMethod"),
		InputType:  proto.String("AddBookReq"),
		OutputType: proto.String("Book"),
	}
	svc := &descriptorpb.ServiceDescriptorProto{
		Name:   proto.String("BookService"),
		Method: []*descriptorpb.MethodDescriptorProto{meth},
	}
	bookMsg := &descriptor.Message{
		DescriptorProto: bookDesc,
	}
	addBookReqMsg := &descriptor.Message{
		DescriptorProto: addBookReqDesc,
	}

	nameField := &descriptor.Field{
		Message:              bookMsg,
		FieldDescriptorProto: bookMsg.GetField()[0],
	}
	nameField.JsonName = proto.String("name")
	typeField := &descriptor.Field{
		Message:              bookMsg,
		FieldDescriptorProto: bookMsg.GetField()[1],
	}
	typeField.JsonName = proto.String("type")
	bookMsg.Fields = []*descriptor.Field{nameField, typeField}

	bookField := &descriptor.Field{
		Message:              addBookReqMsg,
		FieldMessage:         bookMsg,
		FieldDescriptorProto: addBookReqMsg.GetField()[0],
	}
	bookField.JsonName = proto.String("book")
	libraryIdField := &descriptor.Field{
		Message:              addBookReqMsg,
		FieldDescriptorProto: addBookReqMsg.GetField()[1],
	}
	libraryIdField.JsonName = proto.String("libraryId")
	isLatestEditionField := &descriptor.Field{
		Message:              addBookReqMsg,
		FieldDescriptorProto: addBookReqMsg.GetField()[2],
	}
	isLatestEditionField.JsonName = proto.String("isLatestEdition")
	addBookReqMsg.Fields = []*descriptor.Field{bookField, libraryIdField, isLatestEditionField}

	file := descriptor.File{
		FileDescriptorProto: &descriptorpb.FileDescriptorProto{
			SourceCodeInfo: &descriptorpb.SourceCodeInfo{},
			Package:        proto.String("example"),
			Name:           proto.String("book.proto"),
			MessageType:    []*descriptorpb.DescriptorProto{bookDesc, addBookReqDesc},
			Service:        []*descriptorpb.ServiceDescriptorProto{svc},
			Options: &descriptorpb.FileOptions{
				GoPackage: proto.String("github.com/grpc-ecosystem/grpc-gateway/runtime/internal/examplepb;example"),
			},
		},
		GoPkg: descriptor.GoPackage{
			Path: "example.com/path/to/example/example.pb",
			Name: "example_pb",
		},
		Messages: []*descriptor.Message{bookMsg, addBookReqMsg},
		Services: []*descriptor.Service{
			{
				ServiceDescriptorProto: svc,
				Methods: []*descriptor.Method{
					{
						MethodDescriptorProto: meth,
						RequestType:           addBookReqMsg,
						ResponseType:          bookMsg,
						Bindings: []*descriptor.Binding{
							{
								HTTPMethod: "POST",
								PathTmpl: httprule.Template{
									Version:  1,
									OpCodes:  []int{0, 0},
									Template: "/v1/books/{book.type}",
								},
								PathParams: []descriptor.Parameter{
									{
										FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{
											{
												Name: "book",
											},
											{
												Name: "type",
											},
										}),
										Target: typeField,
									},
								},
								Body: &descriptor.Body{
									FieldPath:  ???
								},
							},
						},
					},
				},
			},
		},
	}
        ...
}

What should be written in the body part ?

Putting an empty array FieldPath: []descriptor.FieldPathComponent{}, gives the following error - panic: failed to resolve method FQN: '.example.BookService.AddBookMethod' presumably because this is not a top-level message type.

I did not find any other test case with a similar example.
Kindly help me with what I could be missing.

[johanbrandhorst]

Hi, looks like you should be able to use an empty slice:

FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{}),

Here's an example test:

grpc-gateway/protoc-gen-openapiv2/internal/genopenapi/template_test.go

Line 9725 in 5469869

FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{}),

.

Here's the parser for the body parameter when its an empty slice:

grpc-gateway/protoc-gen-openapiv2/internal/genopenapi/template.go

Line 1297 in 5469869

if len(b.Body.FieldPath) == 0 {

.

[MahikaJaguste]

Hey @johanbrandhorst , thanks for pointing the example above.

I have opened PR #4850 . Kindly provide feedback. Thanks