sr/protobuf: Compatibility check: MULTIPLE_FIELDS_MOVED_TO_ONEOF

vbotbuildovich · Aug 12, 2024 · 9c1670a · 9c1670a
1 parent 689ebad
commit 9c1670a
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 0 deletions.
diff --git a/src/v/pandaproxy/schema_registry/protobuf.cc b/src/v/pandaproxy/schema_registry/protobuf.cc
@@ -499,6 +499,13 @@ struct compatibility_checker {
             }
         }
 
+        for (int i = 0; i < reader->real_oneof_decl_count(); ++i) {
+            auto r = reader->oneof_decl(i);
+            if (!check_compatible(r, writer)) {
+                return false;
+            }
+        }
+
         // check writer fields
         for (int i = 0; i < writer->field_count(); ++i) {
             auto w = writer->field(i);
@@ -525,6 +532,19 @@ struct compatibility_checker {
         return true;
     }
 
+    bool check_compatible(
+      const pb::OneofDescriptor* reader, const pb::Descriptor* writer) {
+        size_t count = 0;
+        for (int i = 0; i < reader->field_count(); ++i) {
+            auto r = reader->field(i);
+            auto w = writer->FindFieldByNumber(r->number());
+            if (w && !w->real_containing_oneof()) {
+                ++count;
+            }
+        }
+        return count <= 1;
+    }
+
     bool check_compatible(
       const pb::FieldDescriptor* reader, const pb::FieldDescriptor* writer) {
         switch (writer->type()) {

diff --git a/src/v/pandaproxy/schema_registry/test/compatibility_protobuf.cc b/src/v/pandaproxy/schema_registry/test/compatibility_protobuf.cc
@@ -724,3 +724,23 @@ SEASTAR_THREAD_TEST_CASE(test_protobuf_compatibility_field_unmade_reserved) {
       R"(syntax = "proto2"; message Simple { optional int32 id = 1; optional int32 new_id = 2; })",
       R"(syntax = "proto2"; message Simple { optional int32 id = 1; reserved 2; })"));
 }
+
+SEASTAR_THREAD_TEST_CASE(
+  test_protobuf_compatibility_multiple_fields_moved_to_oneof) {
+    BOOST_REQUIRE(check_compatible(
+      pps::compatibility_level::backward,
+      R"(syntax = "proto3"; message Simple { oneof wrapper { int32 id = 1; } })",
+      R"(syntax = "proto3"; message Simple { int32 id = 1; })"));
+    BOOST_REQUIRE(!check_compatible(
+      pps::compatibility_level::backward,
+      R"(syntax = "proto3"; message Simple { oneof wrapper { int32 id = 1; int32 new_id = 2; } })",
+      R"(syntax = "proto3"; message Simple { int32 id = 1; int32 new_id = 2; })"));
+}
+
+SEASTAR_THREAD_TEST_CASE(
+  test_protobuf_compatibility_fields_moved_out_of_oneof) {
+    BOOST_REQUIRE(check_compatible(
+      pps::compatibility_level::backward,
+      R"(syntax = "proto3"; message Simple { int32 id = 1; int32 new_id = 2; })",
+      R"(syntax = "proto3"; message Simple { oneof wrapper { int32 id = 1; int32 new_id = 2; } })"));
+}