Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validated imports for known types #627

Merged
merged 1 commit into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions karapace/protobuf/compare_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""
from dataclasses import dataclass, field
from enum import auto, Enum
from typing import List


class Modification(Enum):
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
18 changes: 12 additions & 6 deletions karapace/protobuf/dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
31 changes: 0 additions & 31 deletions karapace/protobuf/known_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

# Support of known dependencies

from enum import Enum
from typing import Any, Dict, Set


Expand All @@ -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()
Expand Down
19 changes: 16 additions & 3 deletions karapace/protobuf/one_of_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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 = {}

Expand Down
6 changes: 4 additions & 2 deletions karapace/protobuf/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion karapace/protobuf/syntax.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion karapace/schema_registry_apis.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"]
Expand Down
12 changes: 0 additions & 12 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -79,9 +70,6 @@ ignore_errors = True
[mypy-karapace.protobuf.enum_constant_element]
ignore_errors = True

[mypy-karapace.protobuf.compare_result]
ignore_errors = True

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⭐️

[mypy-karapace.protobuf.location]
ignore_errors = True

Expand Down
23 changes: 21 additions & 2 deletions tests/integration/test_schema_protobuf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
],
Expand Down Expand Up @@ -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"]