sr/protobuf: Compatibility check: MULTIPLE_FIELDS_MOVED_TO_ONEOF

Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
redpanda-data · Aug 8, 2024 · ef09c89 · ef09c89
1 parent dd7c66b
commit ef09c89
Show file tree

Hide file tree

Showing 2 changed files with 35 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) {
             // TODO(oren): Need special handling here?
@@ -532,6 +539,22 @@ struct compatibility_checker {
         return true;
     }
 
+    bool check_compatible(
+      const pb::OneofDescriptor* reader, const pb::Descriptor* writer) {
+        // NOTE(oren): if a oneof disappears altogether, that doesn't cause
+        // incompatibility, even if the fields themselves are not present
+
+        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
@@ -692,3 +692,15 @@ SEASTAR_THREAD_TEST_CASE(
       R"(syntax = "proto2"; message Simple { optional int32 id = 1; })",
       R"(syntax = "proto2"; message Simple { optional int32 id = 1; required int32 new_id = 2; })"));
 }
+
+SEASTAR_THREAD_TEST_CASE(
+  test_protobuf_compatibility_multiple_fields_moved_to_oneof) {
+    BOOST_REQUIRE(check_compatible(
+      pps::compatibility_level::forward,
+      R"(syntax = "proto3"; message Simple { int32 id = 1; })",
+      R"(syntax = "proto3"; message Simple { oneof wrapper { int32 id = 1; } })"));
+    BOOST_REQUIRE(!check_compatible(
+      pps::compatibility_level::forward,
+      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; } })"));
+}