diff --git a/karapace/protobuf/compare_result.py b/karapace/protobuf/compare_result.py index 0f43e0a54..ad7c0f526 100644 --- a/karapace/protobuf/compare_result.py +++ b/karapace/protobuf/compare_result.py @@ -4,6 +4,7 @@ """ from dataclasses import dataclass, field from enum import auto, Enum +from typing import List class Modification(Enum): @@ -44,7 +45,7 @@ def is_compatible(self) -> bool: self.FIELD_TYPE_ALTER, self.ONE_OF_FIELD_DROP, self.FEW_FIELDS_CONVERTED_TO_ONE_OF, - ] + ] # type: ignore[comparison-overlap] @dataclass @@ -65,9 +66,9 @@ def to_str(self) -> str: class CompareResult: def __init__(self) -> None: - self.result = [] - self.path = [] - self.canonical_name = [] + self.result: List[ModificationRecord] = [] + self.path: List[str] = [] + self.canonical_name: List[str] = [] def push_path(self, name_element: str, canonical: bool = False) -> None: if canonical: diff --git a/karapace/protobuf/dependency.py b/karapace/protobuf/dependency.py index f02e864b5..af5121488 100644 --- a/karapace/protobuf/dependency.py +++ b/karapace/protobuf/dependency.py @@ -44,15 +44,21 @@ def verify(self) -> DependencyVerifierResult: used_type_with_scope = used_type[:delimiter] + "." + used_type[delimiter + 1 :] used_type = used_type[delimiter + 1 :] - if not ( - used_type in DependenciesHardcoded.index - or KnownDependency.index_simple.get(used_type) is not None - or KnownDependency.index.get(used_type) is not None - or used_type in declared_index + if used_type in DependenciesHardcoded.index: + continue + + known_pkg = KnownDependency.index_simple.get(used_type) or KnownDependency.index.get(used_type) + if known_pkg is not None and known_pkg in self.import_path: + continue + + if ( + used_type in declared_index or (delimiter != -1 and used_type_with_scope in declared_index) or "." + used_type in declared_index ): - return DependencyVerifierResult(False, f"type {used_type} is not defined") + continue + + return DependencyVerifierResult(False, f'type "{used_type}" is not defined') return DependencyVerifierResult(True) diff --git a/karapace/protobuf/known_dependency.py b/karapace/protobuf/known_dependency.py index d74953c17..32d95e7e9 100644 --- a/karapace/protobuf/known_dependency.py +++ b/karapace/protobuf/known_dependency.py @@ -7,7 +7,6 @@ # Support of known dependencies -from enum import Enum from typing import Any, Dict, Set @@ -17,36 +16,6 @@ def static_init(cls: Any) -> object: return cls -class KnownDependencyLocation(Enum): - ANY_LOCATION = "google/protobuf/any.proto" - API_LOCATION = "google/protobuf/api.proto" - DESCRIPTOR_LOCATION = "google/protobuf/descriptor.proto" - DURATION_LOCATION = "google/protobuf/duration.proto" - EMPTY_LOCATION = "google/protobuf/empty.proto" - FIELD_MASK_LOCATION = "google/protobuf/field_mask.proto" - SOURCE_CONTEXT_LOCATION = "google/protobuf/source_context.proto" - STRUCT_LOCATION = "google/protobuf/struct.proto" - TIMESTAMP_LOCATION = "google/protobuf/timestamp.proto" - TYPE_LOCATION = "google/protobuf/type.proto" - WRAPPER_LOCATION = "google/protobuf/wrappers.proto" - CALENDAR_PERIOD_LOCATION = "google/type/calendar_period.proto" - COLOR_LOCATION = "google/type/color.proto" - DATE_LOCATION = "google/type/date.proto" - DATETIME_LOCATION = "google/type/datetime.proto" - DAY_OF_WEEK_LOCATION = "google/type/dayofweek.proto" - DECIMAL_LOCATION = "google/type/decimal.proto" - EXPR_LOCATION = "google/type/expr.proto" - FRACTION_LOCATION = "google/type/fraction.proto" - INTERVAL_LOCATION = "google/type/interval.proto" - LATLNG_LOCATION = "google/type/latlng.proto" - MONEY_LOCATION = "google/type/money.proto" - MONTH_LOCATION = "google/type/month.proto" - PHONE_NUMBER_LOCATION = "google/type/phone_number.proto" - POSTAL_ADDRESS_LOCATION = "google/type/postal_address.proto" - QUATERNION_LOCATION = "google/type/quaternion.proto" - TIME_OF_DAY_LOCATION = "google/type/timeofday.proto" - - @static_init class KnownDependency: index: Dict = dict() diff --git a/karapace/protobuf/one_of_element.py b/karapace/protobuf/one_of_element.py index 357308a06..3d7d1993f 100644 --- a/karapace/protobuf/one_of_element.py +++ b/karapace/protobuf/one_of_element.py @@ -4,14 +4,27 @@ """ # Ported from square/wire: # wire-library/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/OneOfElement.kt + +from __future__ import annotations + from itertools import chain from karapace.protobuf.compare_result import CompareResult, Modification from karapace.protobuf.compare_type_storage import CompareTypes +from karapace.protobuf.field_element import FieldElement +from karapace.protobuf.group_element import GroupElement +from karapace.protobuf.option_element import OptionElement from karapace.protobuf.utils import append_documentation, append_indented class OneOfElement: - def __init__(self, name: str, documentation: str = "", fields=None, groups=None, options=None) -> None: + def __init__( + self, + name: str, + documentation: str = "", + fields: list[FieldElement] | None = None, + groups: list[GroupElement] | None = None, + options: list[OptionElement] | None = None, + ) -> None: self.name = name self.documentation = documentation self.fields = fields or [] @@ -19,7 +32,7 @@ def __init__(self, name: str, documentation: str = "", fields=None, groups=None, self.groups = groups or [] def to_schema(self) -> str: - result = [] + result: list[str] = [] append_documentation(result, self.documentation) result.append(f"oneof {self.name} {{") if self.options: @@ -38,7 +51,7 @@ def to_schema(self) -> str: result.append("}\n") return "".join(result) - def compare(self, other: "OneOfElement", result: CompareResult, types: CompareTypes) -> None: + def compare(self, other: OneOfElement, result: CompareResult, types: CompareTypes) -> None: self_tags = {} other_tags = {} diff --git a/karapace/protobuf/schema.py b/karapace/protobuf/schema.py index dd9fa66db..ed7c7795d 100644 --- a/karapace/protobuf/schema.py +++ b/karapace/protobuf/schema.py @@ -130,8 +130,10 @@ def collect_dependencies(self, verifier: ProtobufDependencyVerifier) -> None: for key in self.dependencies: self.dependencies[key].schema.schema.collect_dependencies(verifier) - # verifier.add_import?? we have no access to own Kafka structure from this class... - # but we need data to analyse imports to avoid cyclic dependencies... + for i in self.proto_file_element.imports: + verifier.add_import(i) + for i in self.proto_file_element.public_imports: + verifier.add_import(i) package_name = self.proto_file_element.package_name if package_name is None: diff --git a/karapace/protobuf/syntax.py b/karapace/protobuf/syntax.py index 8b15ebf3e..bbf03de13 100644 --- a/karapace/protobuf/syntax.py +++ b/karapace/protobuf/syntax.py @@ -14,7 +14,7 @@ class Syntax(Enum): PROTO_3 = "proto3" @classmethod - def _missing_(cls, value) -> None: + def _missing_(cls, value: object) -> None: raise IllegalArgumentException(f"unexpected syntax: {value}") def __str__(self) -> str: diff --git a/karapace/schema_registry_apis.py b/karapace/schema_registry_apis.py index ad1177217..982406e9c 100644 --- a/karapace/schema_registry_apis.py +++ b/karapace/schema_registry_apis.py @@ -29,6 +29,7 @@ VersionNotFoundException, ) from karapace.karapace import KarapaceBase +from karapace.protobuf.exception import ProtobufUnresolvedDependencyException from karapace.rapu import HTTPRequest, JSON_CONTENT_TYPE, SERVER_NAME from karapace.schema_models import ParsedTypedSchema, SchemaType, SchemaVersion, TypedSchema, ValidatedTypedSchema from karapace.schema_references import Reference @@ -1167,7 +1168,7 @@ async def subject_post( ) except (InvalidReferences, InvalidSchema, InvalidSchemaType) as e: self.log.warning("Invalid schema: %r", body["schema"], exc_info=True) - if isinstance(e.__cause__, (SchemaParseException, JSONDecodeError)): + if isinstance(e.__cause__, (SchemaParseException, JSONDecodeError, ProtobufUnresolvedDependencyException)): human_error = f"{e.__cause__.args[0]}" # pylint: disable=no-member else: from_body_schema_str = body["schema"] diff --git a/mypy.ini b/mypy.ini index 7400c59df..163d88444 100644 --- a/mypy.ini +++ b/mypy.ini @@ -25,15 +25,6 @@ warn_unused_ignores = False [mypy-karapace.karapace_all] ignore_errors = True -[mypy-karapace.protobuf.one_of_element] -ignore_errors = True - -[mypy-karapace.protobuf.syntax] -ignore_errors = True - -[mypy-karapace.protobuf.field] -ignore_errors = True - [mypy-karapace.protobuf.kotlin_wrapper] ignore_errors = True @@ -79,9 +70,6 @@ ignore_errors = True [mypy-karapace.protobuf.enum_constant_element] ignore_errors = True -[mypy-karapace.protobuf.compare_result] -ignore_errors = True - [mypy-karapace.protobuf.location] ignore_errors = True diff --git a/tests/integration/test_schema_protobuf.py b/tests/integration/test_schema_protobuf.py index 7ccb9f695..76d219624 100644 --- a/tests/integration/test_schema_protobuf.py +++ b/tests/integration/test_schema_protobuf.py @@ -657,8 +657,7 @@ class ReferenceTestCase(BaseTestCase): subject="wr_nonexisting_s1_missing_references", references=None, expected=422, - expected_msg=f"Invalid PROTOBUF schema. Error: Invalid schema {SCHEMA_WITH_REF} " - f"with refs None of type {SchemaType.PROTOBUF}", + expected_msg='Invalid PROTOBUF schema. Error: type "NoReference" is not defined', expected_error_code=42201, ), ], @@ -1036,3 +1035,23 @@ async def test_protobuf_error(registry_async_client: Client) -> None: res = await registry_async_client.post(f"subjects/{testdata.subject}/versions", json=body) assert res.status_code == 200 + + +async def test_protobuf_missing_google_import(registry_async_client: Client) -> None: + subject = create_subject_name_factory("test_protobuf_missing_google_import")() + + unknown_proto = """\ +syntax = "proto3"; +package a1; +message UsingGoogleTypesWithoutImport { + string name = 1; + google.type.PostalAddress p = 2; +} +""" + body = {"schemaType": "PROTOBUF", "schema": unknown_proto} + res = await registry_async_client.post(f"subjects/{subject}/versions", json=body) + + assert res.status_code == 422 + + myjson = res.json() + assert myjson["error_code"] == 42201 and '"google.type.PostalAddress" is not defined' in myjson["message"]