Missing rules / bugs with Protobuf schema compatibility module

[hhkkxxx133]

Looking at the Protobuf documentation of updating a schema for proto2 and proto3, I notice that the following rules are missing for Protobuf schema compatibility module:

Rule	Description
Adding non-required field(s) is compatible	For proto2, new fields should be optional or repeated. For proto3, any new field can be added.
Removing non-required field(s) is compatible, as long as the field number is not used again in the updated message type	For proto2, fields to be removed should be optional or repeated. For proto3, any existing field can be removed.
For string, bytes, and message fields, optional is compatible with repeated
A non-required field can be converted to an extension and vice versa, as long as the type and number stay the same.	proto2 only
int32, uint32, int64, uint64, and bool are all compatible
sint32 and sint64 are compatible with each other but are not compatible with the other integer types
string and bytes are compatible as long as the bytes are valid UTF-8
fixed32 is compatible with sfixed32, and fixed64 with sfixed64
enum is compatible with int32, uint32, int64, and uint64 in terms of wire format (note that values will be truncated if they don't fit)	This depends on the client code when the message is deserialized. Need more discussion.
Changing a single optional value into a member of a new oneof is safe and binary compatible
Moving multiple optional fields into a new oneof may not be safe; Moving any fields into an existing oneof is not safe.

In addition, some other rules I think they make senses to be add as well:

Rule	Description
Adding package is not compatible	This could cause deserialization failure.
Removing package is not compatible	This could cause deserialization failure.
Changing package is not compatible	This could cause deserialization failure.
Adding message(s) is compatible
Removing message(s) is compatible
Adding enum(s) is compatible
Removing enum(s) is compatible
Adding enum constant(s) is compatible
Removing enum constant(s) is compatible	During deserialization, unrecognized enum values will be preserved in the message.
Adding oneof(s) is compatible
Removing oneof(s) is compatible
Adding oneof field(s) is compatible
Removing oneof field(s) is incompatible

Some bugs were observed during testing:

1. IntRange for reserved fields is not considered here
2. NullPointerException will be thrown when the field is removed here

Questions

1. Are you aware of these issues and working on fixing them?
2. Do you see any concerns with adding the rules mentioned above?
3. Some existing rules, for example those related to reserved and RPC, are more strict than the general Protobuf compatibility guidelines. We could offer different modes for the users to select the rules they want to enforce. What do you think?
4. I am willing to contribute to some of these issues. Do you have any suggestions on how to take this forward?

[jmartin127]

I believe some of the bugs have been resolved. For example, the NPE when removing a field for a Protobuf schema is no longer an issue as of version 2.2.3.Final. However, it definitely was an issue in version 2.0.0.Final, that the default configuration of the operator deploys with.

[carlesarnal]

Hi @hhkkxxx133, sorry, we had some releases and I've been busy with them. We're aware of some of the issues, some already addressed (and others not.). Let me try to answer your questions:

2. I don't see any issue with adding those rules.
3. Same, I don't see issues adding that mode, but if you can expand on that a bit, that would be great.
4. If you want to contribute, you should look here, this is the place where most of the checks/rules are implemented.

Thanks!