Validated imports for known types

Validate that referred known types are imported before use.  Also
improve typing and mypy purity a bit.

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:

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)
 

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()

karapace/protobuf/one_of_element.py

@@ -4,22 +4,35 @@
 """
 # 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 []
         self.options = options or []
         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 = {}
 

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:

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:

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"]

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
 

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"]