From 51df4b018441370ebd7ee212d10786a552583c5b Mon Sep 17 00:00:00 2001 From: Elia Migliore Date: Tue, 17 Oct 2023 13:13:50 +0200 Subject: [PATCH 1/3] test: add test that checks new message addition are valid --- ...est_dependencies_compatibility_protobuf.py | 194 ++++++++++++++++++ 1 file changed, 194 insertions(+) diff --git a/tests/integration/test_dependencies_compatibility_protobuf.py b/tests/integration/test_dependencies_compatibility_protobuf.py index 51f5c558c..a783c5aa3 100644 --- a/tests/integration/test_dependencies_compatibility_protobuf.py +++ b/tests/integration/test_dependencies_compatibility_protobuf.py @@ -4,6 +4,7 @@ Copyright (c) 2019 Aiven Ltd See LICENSE for details """ + from karapace.client import Client from karapace.protobuf.kotlin_wrapper import trim_margin from tests.utils import create_subject_name_factory @@ -669,3 +670,196 @@ async def test_protobuf_customer_update_when_having_references(registry_async_cl res = await registry_async_client.post(f"subjects/{subject_customer}/versions", json=body) assert res.status_code == 200 + + +async def test_protobuf_schema_compatibility_full_path_renaming(): + dependency = """\ + package "my.awesome.customer.delivery"; + message RequestId { + string request_id = 1; + }\ + """ + + original_full_path = """\ + import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + message MessageRequest { + my.awesome.customer.delivery.v1beta1.RequestId request_id = 1; + }\ + """ + + evolved_partial_path = """\ + import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + message MessageRequest { + awesome.customer.delivery.v1beta1.RequestId request_id = 1; + }\ + """ + + # placeholder for a real test + assert dependency + original_full_path + evolved_partial_path != "" + + +async def test_protobuf_schema_compatibility_partial_path_renaming(): + dependency = """\ + package "my.awesome.customer.delivery"; + message RequestId { + string request_id = 1; + }\ + """ + + original_partial_path = """\ + import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + message MessageRequest { + my.awesome.customer.delivery.v1beta1.RequestId request_id = 1; + }\ + """ + + evolved_full_path = """\ + import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + message MessageRequest { + awesome.customer.delivery.v1beta1.RequestId request_id = 1; + }\ + """ + + # placeholder for a real test + assert dependency + original_partial_path + evolved_full_path != "" + + +async def test_protobuf_schema_compatibility_import_renaming_should_fail(): + dependency = """\ + package "my.awesome.customer.delivery"; + message RequestId { + string request_id = 1; + }\ + """ + + updated_dependency = """\ + package "awesome.customer.delivery"; + message RequestId { + string request_id = 1; + }\ + """ + + original_partial_path = """\ + import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + import "awesome/customer/delivery/v1beta1/request_id.proto"; + + message MessageRequest { + awesome.customer.delivery.v1beta1.RequestId request_id = 1; + }\ + """ + + evolved_partial_path = """\ + import "awesome/customer/delivery/v1beta1/request_id.proto"; + import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + + message MessageRequest { + awesome.customer.delivery.v1beta1.RequestId request_id = 1; + }\ + """ + + # placeholder for a real test + assert dependency + original_partial_path + evolved_partial_path + updated_dependency != "" + # this should fail because now it's referring to the other object. + + +@pytest.mark.parametrize("compatibility,expected_to_fail", [("FULL", True), ("FORWARD", True), ("BACKWARD", False)]) +async def test_protobuf_schema_update_add_message( + registry_async_client: Client, + compatibility: str, + expected_to_fail: bool, +) -> None: + subject_place = create_subject_name_factory("test_protobuf_place")() + subject_customer = create_subject_name_factory("test_protobuf_customer")() + + for subject in [subject_place, subject_customer]: + res = await registry_async_client.put(f"config/{subject}", json={"compatibility": compatibility}) + assert res.status_code == 200 + + place_proto = """\ +syntax = "proto3"; +package a1; +message Place { + string city = 1; + int32 zone = 2; +} +""" + + body = {"schemaType": "PROTOBUF", "schema": place_proto} + res = await registry_async_client.post(f"subjects/{subject_place}/versions", json=body) + + assert res.status_code == 200 + + customer_proto = """\ +syntax = "proto3"; +package a1; +import "place.proto"; +import "google/type/postal_address.proto"; +// @producer: another comment +message Customer { + string name = 1; + int32 code = 2; + Place place = 3; + google.type.PostalAddress address = 4; +} +""" + body = { + "schemaType": "PROTOBUF", + "schema": customer_proto, + "references": [ + { + "name": "place.proto", + "subject": subject_place, + "version": -1, + } + ], + } + res = await registry_async_client.post(f"subjects/{subject_customer}/versions", json=body) + + assert res.status_code == 200 + + customer_proto_updated = """\ +syntax = "proto3"; +package a1; + +import "place.proto"; +import "google/type/postal_address.proto"; + +// @consumer: the comment was incorrect, updating it now +message Customer { + string name = 1; + int32 code = 2; + Place place = 3; + google.type.PostalAddress address = 4; +} +message Bar { + string another = 1; +} +""" + + body = { + "schemaType": "PROTOBUF", + "schema": customer_proto_updated, + "references": [ + { + "name": "place.proto", + "subject": subject_place, + "version": -1, + } + ], + } + res = await registry_async_client.post(f"subjects/{subject_customer}/versions", json=body) + + if expected_to_fail: + assert res.status_code == 409 + assert res.json() == { + "message": f"Incompatible schema, compatibility_mode={compatibility} Incompatible modification " + f"Modification.MESSAGE_DROP found", + "error_code": 409, + } + else: + assert res.status_code == 200 + + res = await registry_async_client.get(f"subjects/{subject_customer}/versions/2") + + assert res.status_code == 200 + assert res.json()["schema"] == customer_proto_updated From b763750f125e386ca6072b0da50a42ef197709b1 Mon Sep 17 00:00:00 2001 From: Elia Migliore Date: Wed, 18 Oct 2023 11:27:15 +0200 Subject: [PATCH 2/3] test: add test that checks new message addition are valid --- ...est_dependencies_compatibility_protobuf.py | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/integration/test_dependencies_compatibility_protobuf.py b/tests/integration/test_dependencies_compatibility_protobuf.py index a783c5aa3..8e5f11a1d 100644 --- a/tests/integration/test_dependencies_compatibility_protobuf.py +++ b/tests/integration/test_dependencies_compatibility_protobuf.py @@ -674,23 +674,23 @@ async def test_protobuf_customer_update_when_having_references(registry_async_cl async def test_protobuf_schema_compatibility_full_path_renaming(): dependency = """\ - package "my.awesome.customer.delivery"; + package "my.awesome.customer.request"; message RequestId { string request_id = 1; }\ """ original_full_path = """\ - import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + import "my/awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { - my.awesome.customer.delivery.v1beta1.RequestId request_id = 1; + my.awesome.customer.request.v1beta1.RequestId request_id = 1; }\ """ evolved_partial_path = """\ - import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + import "my/awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { - awesome.customer.delivery.v1beta1.RequestId request_id = 1; + awesome.customer.request.v1beta1.RequestId request_id = 1; }\ """ @@ -700,23 +700,23 @@ async def test_protobuf_schema_compatibility_full_path_renaming(): async def test_protobuf_schema_compatibility_partial_path_renaming(): dependency = """\ - package "my.awesome.customer.delivery"; + package "my.awesome.customer.request"; message RequestId { string request_id = 1; }\ """ original_partial_path = """\ - import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + import "my/awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { - my.awesome.customer.delivery.v1beta1.RequestId request_id = 1; + my.awesome.customer.request.v1beta1.RequestId request_id = 1; }\ """ evolved_full_path = """\ - import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + import "my/awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { - awesome.customer.delivery.v1beta1.RequestId request_id = 1; + awesome.customer.request.v1beta1.RequestId request_id = 1; }\ """ @@ -726,34 +726,34 @@ async def test_protobuf_schema_compatibility_partial_path_renaming(): async def test_protobuf_schema_compatibility_import_renaming_should_fail(): dependency = """\ - package "my.awesome.customer.delivery"; + package "my.awesome.customer.request"; message RequestId { string request_id = 1; }\ """ updated_dependency = """\ - package "awesome.customer.delivery"; + package "awesome.customer.request"; message RequestId { string request_id = 1; }\ """ original_partial_path = """\ - import "my/awesome/customer/delivery/v1beta1/request_id.proto"; - import "awesome/customer/delivery/v1beta1/request_id.proto"; + import "my/awesome/customer/request/v1beta1/request_id.proto"; + import "awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { - awesome.customer.delivery.v1beta1.RequestId request_id = 1; + awesome.customer.request.v1beta1.RequestId request_id = 1; }\ """ evolved_partial_path = """\ - import "awesome/customer/delivery/v1beta1/request_id.proto"; - import "my/awesome/customer/delivery/v1beta1/request_id.proto"; + import "awesome/customer/request/v1beta1/request_id.proto"; + import "my/awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { - awesome.customer.delivery.v1beta1.RequestId request_id = 1; + awesome.customer.request.v1beta1.RequestId request_id = 1; }\ """ From f6994040729fa0de9d37f5b5d3b03f80c307d37c Mon Sep 17 00:00:00 2001 From: Elia Migliore Date: Wed, 18 Oct 2023 15:53:10 +0200 Subject: [PATCH 3/3] test: add test that checks new message addition are valid added tests that need to succed or fail based on the schema evolution logic --- ...est_dependencies_compatibility_protobuf.py | 189 ++++++++++++++++-- 1 file changed, 173 insertions(+), 16 deletions(-) diff --git a/tests/integration/test_dependencies_compatibility_protobuf.py b/tests/integration/test_dependencies_compatibility_protobuf.py index 8e5f11a1d..47c8b3564 100644 --- a/tests/integration/test_dependencies_compatibility_protobuf.py +++ b/tests/integration/test_dependencies_compatibility_protobuf.py @@ -7,6 +7,7 @@ from karapace.client import Client from karapace.protobuf.kotlin_wrapper import trim_margin +from karapace.typing import Subject from tests.utils import create_subject_name_factory import pytest @@ -672,15 +673,24 @@ async def test_protobuf_customer_update_when_having_references(registry_async_cl assert res.status_code == 200 -async def test_protobuf_schema_compatibility_full_path_renaming(): +async def test_protobuf_schema_compatibility_full_path_renaming(registry_async_client: Client) -> None: + subject_dependency = Subject("dependency") + subject_entity = Subject("entity") + + for subject in [subject_dependency, subject_entity]: + res = await registry_async_client.put(f"config/{subject}", json={"compatibility": "FULL"}) + assert res.status_code == 200 + dependency = """\ - package "my.awesome.customer.request"; + syntax = "proto3"; + package my.awesome.customer.request.v1beta1; message RequestId { string request_id = 1; }\ """ original_full_path = """\ + syntax = "proto3"; import "my/awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { my.awesome.customer.request.v1beta1.RequestId request_id = 1; @@ -688,25 +698,72 @@ async def test_protobuf_schema_compatibility_full_path_renaming(): """ evolved_partial_path = """\ + syntax = "proto3"; import "my/awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { awesome.customer.request.v1beta1.RequestId request_id = 1; }\ """ - # placeholder for a real test - assert dependency + original_full_path + evolved_partial_path != "" + # registering the dependency + body = {"schemaType": "PROTOBUF", "schema": dependency} + res = await registry_async_client.post(f"subjects/{subject_dependency}/versions", json=body) + + assert res.status_code == 200 + + # registering the entity + + body = { + "schemaType": "PROTOBUF", + "schema": original_full_path, + "references": [ + { + "name": "place.proto", + "subject": subject_dependency, + "version": -1, + } + ], + } + res = await registry_async_client.post(f"subjects/{subject_entity}/versions", json=body) + assert res.status_code == 200 + previous_id = res.json()["id"] + + # registering the evolved entity + + body = { + "schemaType": "PROTOBUF", + "schema": evolved_partial_path, + "references": [ + { + "name": "place.proto", + "subject": subject_dependency, + "version": -1, + } + ], + } + res = await registry_async_client.post(f"subjects/{subject_entity}/versions", json=body) + assert res.status_code == 200 + assert res.json()["id"] == previous_id + +async def test_protobuf_schema_compatibility_partial_path_renaming(registry_async_client: Client) -> None: + subject_dependency = Subject("dependency") + subject_entity = Subject("entity") + + for subject in [subject_dependency, subject_entity]: + res = await registry_async_client.put(f"config/{subject}", json={"compatibility": "FULL"}) + assert res.status_code == 200 -async def test_protobuf_schema_compatibility_partial_path_renaming(): dependency = """\ - package "my.awesome.customer.request"; + syntax = "proto3"; + package my.awesome.customer.request.v1beta1; message RequestId { string request_id = 1; }\ """ original_partial_path = """\ + syntax = "proto3"; import "my/awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { my.awesome.customer.request.v1beta1.RequestId request_id = 1; @@ -714,32 +771,83 @@ async def test_protobuf_schema_compatibility_partial_path_renaming(): """ evolved_full_path = """\ + syntax = "proto3"; import "my/awesome/customer/request/v1beta1/request_id.proto"; message MessageRequest { awesome.customer.request.v1beta1.RequestId request_id = 1; }\ """ - # placeholder for a real test - assert dependency + original_partial_path + evolved_full_path != "" + # registering the dependency + body = {"schemaType": "PROTOBUF", "schema": dependency} + res = await registry_async_client.post(f"subjects/{subject_dependency}/versions", json=body) + + assert res.status_code == 200 + # registering the entity -async def test_protobuf_schema_compatibility_import_renaming_should_fail(): - dependency = """\ - package "my.awesome.customer.request"; + body = { + "schemaType": "PROTOBUF", + "schema": original_partial_path, + "references": [ + { + "name": "place.proto", + "subject": subject_dependency, + "version": -1, + } + ], + } + res = await registry_async_client.post(f"subjects/{subject_entity}/versions", json=body) + assert res.status_code == 200 + previous_id = res.json()["id"] + + # registering the evolved entity + + body = { + "schemaType": "PROTOBUF", + "schema": evolved_full_path, + "references": [ + { + "name": "place.proto", + "subject": subject_dependency, + "version": -1, + } + ], + } + res = await registry_async_client.post(f"subjects/{subject_entity}/versions", json=body) + assert res.status_code == 200 + assert ( + res.json()["id"] == previous_id + ), "The registered schema should be recognized as the same, no evolutions are being applied" + + +async def test_protobuf_schema_compatibility_import_renaming_should_fail(registry_async_client: Client) -> None: + first_subject_dependency = Subject("first_dependency") + second_subject_dependency = Subject("second_dependency") + subject_entity = Subject("entity") + + for subject in [first_subject_dependency, second_subject_dependency, subject_entity]: + res = await registry_async_client.put(f"config/{subject}", json={"compatibility": "FULL"}) + assert res.status_code == 200 + + first_dependency = """\ + syntax = "proto3"; + package my.awesome.customer.request.v1beta1; message RequestId { string request_id = 1; }\ """ - updated_dependency = """\ - package "awesome.customer.request"; + second_dependency = """\ + syntax = "proto3"; + package awesome.customer.request.v1beta1; message RequestId { string request_id = 1; }\ """ original_partial_path = """\ + syntax = "proto3"; import "my/awesome/customer/request/v1beta1/request_id.proto"; import "awesome/customer/request/v1beta1/request_id.proto"; @@ -749,6 +857,7 @@ async def test_protobuf_schema_compatibility_import_renaming_should_fail(): """ evolved_partial_path = """\ + syntax = "proto3"; import "awesome/customer/request/v1beta1/request_id.proto"; import "my/awesome/customer/request/v1beta1/request_id.proto"; @@ -757,9 +866,57 @@ async def test_protobuf_schema_compatibility_import_renaming_should_fail(): }\ """ - # placeholder for a real test - assert dependency + original_partial_path + evolved_partial_path + updated_dependency != "" - # this should fail because now it's referring to the other object. + # registering the first dependency + body = {"schemaType": "PROTOBUF", "schema": first_dependency} + res = await registry_async_client.post(f"subjects/{first_subject_dependency}/versions", json=body) + + assert res.status_code == 200 + + # registering the second dependency + body = {"schemaType": "PROTOBUF", "schema": second_dependency} + res = await registry_async_client.post(f"subjects/{second_subject_dependency}/versions", json=body) + + assert res.status_code == 200 + + # registering the entity + body = { + "schemaType": "PROTOBUF", + "schema": original_partial_path, + "references": [ + { + "name": f"{first_subject_dependency}.proto", + "subject": first_subject_dependency, + "version": -1, + }, + { + "name": f"{second_subject_dependency}.proto", + "subject": second_subject_dependency, + "version": -1, + }, + ], + } + res = await registry_async_client.post(f"subjects/{subject_entity}/versions", json=body) + assert res.status_code == 200 + + # registering the evolved entity + body = { + "schemaType": "PROTOBUF", + "schema": evolved_partial_path, + "references": [ + { + "name": f"{first_subject_dependency}.proto", + "subject": first_subject_dependency, + "version": -1, + }, + { + "name": f"{second_subject_dependency}.proto", + "subject": second_subject_dependency, + "version": -1, + }, + ], + } + res = await registry_async_client.post(f"subjects/{subject_entity}/versions", json=body) + assert res.status_code != 200, "The real used type is changed due to the relative import." @pytest.mark.parametrize("compatibility,expected_to_fail", [("FULL", True), ("FORWARD", True), ("BACKWARD", False)])