From 06c7491ac590c99d37af0ecc26ace9ba052e1875 Mon Sep 17 00:00:00 2001 From: amstee Date: Fri, 26 Jul 2024 17:13:35 +0100 Subject: [PATCH 1/5] chore: allow definition of two new fields when schema is registered to keep compatibility with confluent clients --- karapace/schema_registry_apis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/karapace/schema_registry_apis.py b/karapace/schema_registry_apis.py index 4ef7b884a..d3b90dac6 100644 --- a/karapace/schema_registry_apis.py +++ b/karapace/schema_registry_apis.py @@ -991,7 +991,7 @@ def _validate_schema_request_body(self, content_type: str, body: dict | Any) -> status=HTTPStatus.BAD_REQUEST, ) for field in body: - if field not in {"schema", "schemaType", "references"}: + if field not in {"schema", "schemaType", "references", "metadata", "ruleSet"}: self.r( body={ "error_code": SchemaErrorCodes.HTTP_UNPROCESSABLE_ENTITY.value, From b1cd1d64d317de3c757cd8f75790427c8607abc0 Mon Sep 17 00:00:00 2001 From: amstee Date: Fri, 26 Jul 2024 21:44:54 +0100 Subject: [PATCH 2/5] test: validate_schema_request_body --- tests/unit/test_schema_registry_api.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/unit/test_schema_registry_api.py b/tests/unit/test_schema_registry_api.py index 5b8e11c45..f09009fe8 100644 --- a/tests/unit/test_schema_registry_api.py +++ b/tests/unit/test_schema_registry_api.py @@ -11,6 +11,30 @@ from unittest.mock import ANY, AsyncMock, Mock, patch, PropertyMock import asyncio +import pytest + + +async def test_validate_schema_request_body(): + controller = KarapaceSchemaRegistryController(config=set_config_defaults(DEFAULTS)) + + controller._validate_schema_request_body("application/json", { + "schema": "{}", + "schemaType": "JSON", + "references": [], + "metadata": {}, + "ruleSet": {} + }) + + with pytest.raises(HTTPResponse) as exc_info: + controller._validate_schema_request_body("application/json", { + "schema": "{}", + "schemaType": "JSON", + "references": [], + "unexpected_field_name": {}, + "ruleSet": {} + }) + assert exc_info.type is HTTPResponse + assert str(exc_info.value) == "HTTPResponse 422" async def test_forward_when_not_ready(): From 9f7685bc638c25e85ade96e911b715e57d1e064e Mon Sep 17 00:00:00 2001 From: amstee Date: Fri, 26 Jul 2024 21:51:56 +0100 Subject: [PATCH 3/5] chore: lint --- tests/unit/test_schema_registry_api.py | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/tests/unit/test_schema_registry_api.py b/tests/unit/test_schema_registry_api.py index f09009fe8..6d850f5fc 100644 --- a/tests/unit/test_schema_registry_api.py +++ b/tests/unit/test_schema_registry_api.py @@ -17,22 +17,15 @@ async def test_validate_schema_request_body(): controller = KarapaceSchemaRegistryController(config=set_config_defaults(DEFAULTS)) - controller._validate_schema_request_body("application/json", { - "schema": "{}", - "schemaType": "JSON", - "references": [], - "metadata": {}, - "ruleSet": {} - }) + controller._validate_schema_request_body( # pylint: disable=W0212 + "application/json", {"schema": "{}", "schemaType": "JSON", "references": [], "metadata": {}, "ruleSet": {}} + ) with pytest.raises(HTTPResponse) as exc_info: - controller._validate_schema_request_body("application/json", { - "schema": "{}", - "schemaType": "JSON", - "references": [], - "unexpected_field_name": {}, - "ruleSet": {} - }) + controller._validate_schema_request_body( # pylint: disable=W0212 + "application/json", + {"schema": "{}", "schemaType": "JSON", "references": [], "unexpected_field_name": {}, "ruleSet": {}}, + ) assert exc_info.type is HTTPResponse assert str(exc_info.value) == "HTTPResponse 422" From c85f4aa842a4ed5f2cc73fa72b9e897276c3498d Mon Sep 17 00:00:00 2001 From: amstee Date: Wed, 31 Jul 2024 16:44:10 +0100 Subject: [PATCH 4/5] chore: lint --- .../schema_registry/test_jsonschema.py | 13 ++++++++++++- tests/integration/test_schema_protobuf.py | 15 ++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/integration/schema_registry/test_jsonschema.py b/tests/integration/schema_registry/test_jsonschema.py index 4c80e20fd..4b6440bb3 100644 --- a/tests/integration/schema_registry/test_jsonschema.py +++ b/tests/integration/schema_registry/test_jsonschema.py @@ -97,6 +97,7 @@ TYPES_STRING_SCHEMA, ) from tests.utils import new_random_name +from typing import Any, Dict import json import pytest @@ -234,8 +235,14 @@ async def not_schemas_are_backward_compatible( @pytest.mark.parametrize("trail", ["", "/"]) @pytest.mark.parametrize("compatibility", [CompatibilityModes.FORWARD, CompatibilityModes.BACKWARD, CompatibilityModes.FULL]) +@pytest.mark.parametrize("metadata", [None, {}]) +@pytest.mark.parametrize("rule_set", [None, {}]) async def test_same_jsonschema_must_have_same_id( - registry_async_client: Client, compatibility: CompatibilityModes, trail: str + registry_async_client: Client, + compatibility: CompatibilityModes, + trail: str, + metadata: Dict[str, Any], + rule_set: Dict[str, Any], ) -> None: for schema in ALL_SCHEMAS: subject = new_random_name("subject") @@ -248,6 +255,8 @@ async def test_same_jsonschema_must_have_same_id( json={ "schema": json.dumps(schema.schema), "schemaType": SchemaType.JSONSCHEMA.value, + "metadata": metadata, + "ruleSet": rule_set, }, ) assert first_res.status_code == 200 @@ -259,6 +268,8 @@ async def test_same_jsonschema_must_have_same_id( json={ "schema": json.dumps(schema.schema), "schemaType": SchemaType.JSONSCHEMA.value, + "metadata": metadata, + "ruleSet": rule_set, }, ) assert second_res.status_code == 200 diff --git a/tests/integration/test_schema_protobuf.py b/tests/integration/test_schema_protobuf.py index 10df70637..c14650a2d 100644 --- a/tests/integration/test_schema_protobuf.py +++ b/tests/integration/test_schema_protobuf.py @@ -12,7 +12,7 @@ from karapace.typing import JsonData from tests.base_testcase import BaseTestCase from tests.utils import create_subject_name_factory -from typing import List, Optional, Union +from typing import Any, Dict, List, Optional, Union import logging import pytest @@ -963,11 +963,20 @@ class ReferenceTestCase(BaseTestCase): ], ids=str, ) -async def test_references(testcase: ReferenceTestCase, registry_async_client: Client): +@pytest.mark.parametrize("metadata", [None, {}]) +@pytest.mark.parametrize("rule_set", [None, {}]) +async def test_references( + testcase: ReferenceTestCase, registry_async_client: Client, metadata: Dict[str, Any], rule_set: Dict[str, Any] +): for testdata in testcase.schemas: if isinstance(testdata, TestCaseSchema): print(f"Adding new schema, subject: '{testdata.subject}'\n{testdata.schema_str}") - body = {"schemaType": testdata.schema_type, "schema": testdata.schema_str} + body = { + "schemaType": testdata.schema_type, + "schema": testdata.schema_str, + "metadata": metadata, + "ruleSet": rule_set, + } if testdata.references: body["references"] = testdata.references res = await registry_async_client.post(f"subjects/{testdata.subject}/versions", json=body) From 9c0f0ba057c34c9cb8ce4d09f4688ef6eed80932 Mon Sep 17 00:00:00 2001 From: amstee Date: Fri, 2 Aug 2024 15:50:52 +0100 Subject: [PATCH 5/5] feat: define proper types for SchemaMetadata & SchemaRuleSet to facilitate future impl --- karapace/typing.py | 4 +++- tests/integration/schema_registry/test_jsonschema.py | 6 +++--- tests/integration/test_schema_protobuf.py | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/karapace/typing.py b/karapace/typing.py index 4daf4739e..77058cce2 100644 --- a/karapace/typing.py +++ b/karapace/typing.py @@ -6,7 +6,7 @@ from enum import Enum, unique from karapace.errors import InvalidVersion -from typing import ClassVar, Dict, List, Mapping, NewType, Sequence, Union +from typing import Any, ClassVar, Dict, List, Mapping, NewType, Sequence, Union from typing_extensions import TypeAlias import functools @@ -23,6 +23,8 @@ Subject = NewType("Subject", str) VersionTag = Union[str, int] +SchemaMetadata = NewType("SchemaMetadata", Dict[str, Any]) +SchemaRuleSet = NewType("SchemaRuleSet", Dict[str, Any]) # note: the SchemaID is a unique id among all the schemas (and each version should be assigned to a different id) # basically the same SchemaID refer always to the same TypedSchema. diff --git a/tests/integration/schema_registry/test_jsonschema.py b/tests/integration/schema_registry/test_jsonschema.py index 4b6440bb3..d765cb585 100644 --- a/tests/integration/schema_registry/test_jsonschema.py +++ b/tests/integration/schema_registry/test_jsonschema.py @@ -6,6 +6,7 @@ from karapace.client import Client from karapace.compatibility import CompatibilityModes from karapace.schema_reader import SchemaType +from karapace.typing import SchemaMetadata, SchemaRuleSet from tests.schemas.json_schemas import ( A_DINT_B_DINT_OBJECT_SCHEMA, A_DINT_B_INT_OBJECT_SCHEMA, @@ -97,7 +98,6 @@ TYPES_STRING_SCHEMA, ) from tests.utils import new_random_name -from typing import Any, Dict import json import pytest @@ -241,8 +241,8 @@ async def test_same_jsonschema_must_have_same_id( registry_async_client: Client, compatibility: CompatibilityModes, trail: str, - metadata: Dict[str, Any], - rule_set: Dict[str, Any], + metadata: SchemaMetadata, + rule_set: SchemaRuleSet, ) -> None: for schema in ALL_SCHEMAS: subject = new_random_name("subject") diff --git a/tests/integration/test_schema_protobuf.py b/tests/integration/test_schema_protobuf.py index c14650a2d..716a03e12 100644 --- a/tests/integration/test_schema_protobuf.py +++ b/tests/integration/test_schema_protobuf.py @@ -9,10 +9,10 @@ from karapace.errors import InvalidTest from karapace.protobuf.kotlin_wrapper import trim_margin from karapace.schema_type import SchemaType -from karapace.typing import JsonData +from karapace.typing import JsonData, SchemaMetadata, SchemaRuleSet from tests.base_testcase import BaseTestCase from tests.utils import create_subject_name_factory -from typing import Any, Dict, List, Optional, Union +from typing import List, Optional, Union import logging import pytest @@ -966,7 +966,7 @@ class ReferenceTestCase(BaseTestCase): @pytest.mark.parametrize("metadata", [None, {}]) @pytest.mark.parametrize("rule_set", [None, {}]) async def test_references( - testcase: ReferenceTestCase, registry_async_client: Client, metadata: Dict[str, Any], rule_set: Dict[str, Any] + testcase: ReferenceTestCase, registry_async_client: Client, metadata: SchemaMetadata, rule_set: SchemaRuleSet ): for testdata in testcase.schemas: if isinstance(testdata, TestCaseSchema):