confluentinc / confluent-kafka-go

Confluent's Apache Kafka Golang client
Apache License 2.0
4.65k stars 659 forks source link

error handling protobuf option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema) #946

Open baganokodo2022 opened 1 year ago

baganokodo2022 commented 1 year ago

Description

confluent-kafka-go version v2.0.2 We make use of protobuf.Serializer in confluent-kafka-go to register proto schemas on Confluent Schema Registry. The serializer breaks on the option, option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema), which should be rendered as,

  option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema) = {
    json_schema: {
      title: "SimpleMessage"
      description: "A simple message."
      required: ["id"]
    }
  };

instead, it ended up with,

  option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema) = {
    json_schema: <
      required: "order_id"
      required: "portfolio_id"
      required: "product_id"
      required: "type"
      required: "side"
      required: "created_time"
      required: "status"
    >
  };

CSR returned the following error message, details: Could not parse Protobuf - Syntax error in :217:18: expected a word line 217 points to json_schema: <

How to reproduce

Create a simple protobuf message with the above message-level option like this,

syntax = "proto3";

package org.order.common;

import "google/protobuf/descriptor.proto";
import "protoc-gen-swagger/options/openapiv2.proto";

message Order {
  option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema) = {
    json_schema: {
      title: "SimpleMessage"
      description: "A simple message."
      required: ["id"]
    }
  };

  string id = 1; 
}

In golang,

client, err := schemaregistry.NewClient(schemaregistry.NewConfig(srUrl))
ser, err := protobuf.NewSerializer(client, serde.ValueSerde, protobuf.NewSerializerConfig())
payload, err := serializer.ser.Serialize(serializer.topic, <protoc_generated_Order_value>)

Checklist

Please provide the following information:

baganokodo2022 commented 1 year ago

Today, I successfully reproduce the same error in a different code path,

Screenshot 2023-02-16 at 2 43 35 PM

baganokodo2022 commented 1 year ago

The problem seems to be originated from the line below,

https://github.com/golang/protobuf/blob/master/proto/text_encode.go#L373

If we replace it with, var bra, ket byte = '{', '}'

It generated the desired schema, which is accepted by Confluent Schema Registry.

syntax = "proto3";

package org.order.common;

import "google/protobuf/descriptor.proto";
import "protoc-gen-swagger/options/openapiv2.proto";

message Order {
  option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema) = {
    json_schema: {
      title: "SimpleMessage"
      description: "A simple message."
      required: ["id"]
    }
  };
  string id = 1; 
}
baganokodo2022 commented 1 year ago

according to the responses from https://github.com/golang/protobuf/issues/1523, the root cause is clearly on jhump/protoreflect.

baganokodo2022 commented 1 year ago

filed the issue to jhump, https://github.com/jhump/protoreflect/issues/549

jhump commented 1 year ago

the root cause is clearly on jhump/protoreflect

@baganokodo2022, I don't agree. The output in your examples is perfectly valid Protobuf source. Message literals in custom option values are allowed to use either angle brackets or curly braces for nested messages/groups.

So I think the root cause is more likely to be whatever is processing/parsing the proto sources in Confluent's schema registry. It is likely not completely compliant/compatible with what protoc (the reference compiler) considers valid source.

baganokodo2022 commented 1 year ago

thanks @jhump for the inputs.

baganokodo2022 commented 1 year ago

This issue is proven fixed in https://github.com/jhump/protoreflect/releases/tag/v1.15.0-rc2

jhump commented 1 year ago

FWIW, I think there is still a bug in Confluent that needs fixing: it should not be complaining about the use of angle brackets as that is allowed in protobuf sources.

The above mentioned release of github.comjhump/protoreflectsimply works around the bug, in that its protoprint package will no longer produce files that use the angle brackets.