diff --git a/backend/infrahub/api/schema.py b/backend/infrahub/api/schema.py index 35c121fbeb..7a12b3b3d7 100644 --- a/backend/infrahub/api/schema.py +++ b/backend/infrahub/api/schema.py @@ -26,7 +26,15 @@ SchemaDiff, SchemaUpdateValidationResult, ) -from infrahub.core.schema import GenericSchema, MainSchemaTypes, NodeSchema, ProfileSchema, SchemaRoot, TemplateSchema +from infrahub.core.schema import ( + GenericSchema, + MainSchemaTypes, + NodeSchema, + ProfileSchema, + SchemaRoot, + SchemaWarning, + TemplateSchema, +) from infrahub.core.schema.constants import SchemaNamespace # noqa: TC001 from infrahub.core.validators.models.validate_migration import ( SchemaValidateMigrationData, @@ -130,6 +138,9 @@ class SchemaUpdate(BaseModel): hash: str = Field(..., description="The new hash for the entire schema") previous_hash: str = Field(..., description="The previous hash for the entire schema") diff: SchemaDiff = Field(..., description="The modifications to the schema") + warnings: list[SchemaWarning] = Field( + default_factory=list, description="Warnings encountered while loading the schema" + ) @computed_field def schema_updated(self) -> bool: @@ -307,8 +318,10 @@ async def load_schema( log.info("schema_load_request", branch=branch.name) errors: list[str] = [] + warnings: list[SchemaWarning] = [] for schema in schemas.schemas: errors += schema.validate_namespaces() + warnings += schema.gather_warnings() if errors: raise SchemaNotValidError(message=", ".join(errors)) @@ -402,7 +415,7 @@ async def load_schema( ) await service.event.send(event=event) - return SchemaUpdate(hash=updated_hash, previous_hash=original_hash, diff=result.diff) + return SchemaUpdate(hash=updated_hash, previous_hash=original_hash, diff=result.diff, warnings=warnings) @router.post("/check") @@ -417,8 +430,10 @@ async def check_schema( log.info("schema_check_request", branch=branch.name) errors: list[str] = [] + warnings: list[SchemaWarning] = [] for schema in schemas.schemas: errors += schema.validate_namespaces() + warnings += schema.gather_warnings() if errors: raise SchemaNotValidError(message=", ".join(errors)) @@ -445,4 +460,10 @@ async def check_schema( if error_messages: raise SchemaNotValidError(message=",\n".join(error_messages)) - return JSONResponse(status_code=202, content={"diff": result.diff.model_dump()}) + return JSONResponse( + status_code=202, + content={ + "diff": result.diff.model_dump(), + "warnings": [warning.model_dump(mode="json") for warning in warnings], + }, + ) diff --git a/backend/infrahub/core/schema/__init__.py b/backend/infrahub/core/schema/__init__.py index 5e1adba5c4..6a7111cdcb 100644 --- a/backend/infrahub/core/schema/__init__.py +++ b/backend/infrahub/core/schema/__init__.py @@ -1,6 +1,7 @@ from __future__ import annotations import uuid +from enum import Enum from typing import Any, TypeAlias from infrahub_sdk.utils import deep_merge_dict @@ -44,6 +45,21 @@ class SchemaExtension(HashableModel): nodes: list[NodeExtensionSchema] = Field(default_factory=list) +class SchemaWarningType(Enum): + DEPRECATION = "deprecation" + + +class SchemaWarningKind(BaseModel): + kind: str = Field(..., description="The kind impacted by the warning") + field: str | None = Field(default=None, description="The attribute or relationship impacted by the warning") + + +class SchemaWarning(BaseModel): + type: SchemaWarningType = Field(..., description="The type of warning") + kinds: list[SchemaWarningKind] = Field(default_factory=list, description="The kinds impacted by the warning") + message: str = Field(..., description="The message that describes the warning") + + class SchemaRoot(BaseModel): model_config = ConfigDict(extra="forbid") @@ -80,6 +96,46 @@ def validate_namespaces(self) -> list[str]: return errors + def gather_warnings(self) -> list[SchemaWarning]: + models = self.nodes + self.generics + warnings: list[SchemaWarning] = [] + for model in models: + if model.display_labels is not None: + warnings.append( + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind=model.kind)], + message="display_labels are deprecated, use display_label instead", + ) + ) + if model.default_filter is not None: + warnings.append( + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind=model.kind)], + message="default_filter is deprecated", + ) + ) + for attribute in model.attributes: + if attribute.max_length is not None: + warnings.append( + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind=model.kind, field=attribute.name)], + message="Use of 'max_length' on attributes is deprecated, use parameters instead", + ) + ) + if attribute.min_length is not None: + warnings.append( + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind=model.kind, field=attribute.name)], + message="Use of 'min_length' on attributes is deprecated, use parameters instead", + ) + ) + + return warnings + def generate_uuid(self) -> None: """Generate UUID for all nodes, attributes & relationships Mainly useful during unit tests.""" diff --git a/backend/tests/integration/schema_lifecycle/test_attribute_parameters_update.py b/backend/tests/integration/schema_lifecycle/test_attribute_parameters_update.py index 4d9a69b465..57c68fc8f4 100644 --- a/backend/tests/integration/schema_lifecycle/test_attribute_parameters_update.py +++ b/backend/tests/integration/schema_lifecycle/test_attribute_parameters_update.py @@ -297,7 +297,29 @@ async def test_schema02_load_update( }, }, "removed": {}, - } + }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingThingLegacy", "field": "value"}], + "message": "Use of 'max_length' on attributes is deprecated, use parameters instead", + }, + { + "type": "deprecation", + "kinds": [{"kind": "TestingThingLegacy", "field": "value"}], + "message": "Use of 'min_length' on attributes is deprecated, use parameters instead", + }, + { + "type": "deprecation", + "kinds": [{"kind": "TestingThing", "field": "value"}], + "message": "Use of 'max_length' on attributes is deprecated, use parameters instead", + }, + { + "type": "deprecation", + "kinds": [{"kind": "TestingThing", "field": "value"}], + "message": "Use of 'min_length' on attributes is deprecated, use parameters instead", + }, + ], } async def test_step02_load_schema_with_overrides( diff --git a/backend/tests/integration/schema_lifecycle/test_generic_migrations.py b/backend/tests/integration/schema_lifecycle/test_generic_migrations.py index 90f5111c77..687b246224 100644 --- a/backend/tests/integration/schema_lifecycle/test_generic_migrations.py +++ b/backend/tests/integration/schema_lifecycle/test_generic_migrations.py @@ -596,6 +596,7 @@ async def test_step02_check_add_specific_overrides( }, "removed": {}, }, + "warnings": [], } async def test_step02_load_schema_with_overrides( @@ -787,6 +788,7 @@ async def test_step03_check_delete_overridden_field( }, "removed": {}, }, + "warnings": [], } async def test_step03_load_schema_with_deleted_override( @@ -942,6 +944,7 @@ async def test_step04_check_generic_weight_updates( }, "removed": {}, }, + "warnings": [], } async def test_step04_load_schema_with_updated_generic_weight( @@ -1170,6 +1173,7 @@ async def test_step05_check_delete_generic_fields( }, "removed": {}, }, + "warnings": [], } async def test_step05_load_schema_with_generic_deletes( @@ -1331,6 +1335,7 @@ async def test_step06_check_deleted_overridden_fields( }, "removed": {}, }, + "warnings": [], } async def test_step06_load_schema_with_override_deletes( diff --git a/backend/tests/integration/schema_lifecycle/test_migration_attribute_branch.py b/backend/tests/integration/schema_lifecycle/test_migration_attribute_branch.py index 552752fd8d..18fb47a11e 100644 --- a/backend/tests/integration/schema_lifecycle/test_migration_attribute_branch.py +++ b/backend/tests/integration/schema_lifecycle/test_migration_attribute_branch.py @@ -222,6 +222,13 @@ async def test_step02_check_attr_add_rename( }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } async def test_step02_load_attr_add_rename( @@ -287,6 +294,13 @@ async def test_step03_check(self, db: InfrahubDatabase, client: InfrahubClient, }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } assert success diff --git a/backend/tests/integration/schema_lifecycle/test_migration_hierarchy_change.py b/backend/tests/integration/schema_lifecycle/test_migration_hierarchy_change.py index c1c049a42b..ec903e49de 100644 --- a/backend/tests/integration/schema_lifecycle/test_migration_hierarchy_change.py +++ b/backend/tests/integration/schema_lifecycle/test_migration_hierarchy_change.py @@ -156,6 +156,7 @@ async def test_check_schema_02(self, client: InfrahubClient, branch_1: Branch, l }, }, }, + "warnings": [], } async def test_load_schema_02( diff --git a/backend/tests/integration/schema_lifecycle/test_migration_kind_update.py b/backend/tests/integration/schema_lifecycle/test_migration_kind_update.py index ae5fefb7dc..272d432fae 100644 --- a/backend/tests/integration/schema_lifecycle/test_migration_kind_update.py +++ b/backend/tests/integration/schema_lifecycle/test_migration_kind_update.py @@ -219,7 +219,8 @@ async def test_step02_check_change_node_kind( "added": {}, "changed": {SPECIFIC_ONE_KIND_UPDATED: {"added": {}, "changed": {"name": None}, "removed": {}}}, "removed": {}, - } + }, + "warnings": [], } async def test_step02_load_node_kind_change( diff --git a/backend/tests/integration/schema_lifecycle/test_migration_relationship_branch.py b/backend/tests/integration/schema_lifecycle/test_migration_relationship_branch.py index 72dfaf54f4..b1c5e067a9 100644 --- a/backend/tests/integration/schema_lifecycle/test_migration_relationship_branch.py +++ b/backend/tests/integration/schema_lifecycle/test_migration_relationship_branch.py @@ -241,6 +241,13 @@ async def test_step02_check(self, db: InfrahubDatabase, client: InfrahubClient, }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } assert success @@ -297,6 +304,13 @@ async def test_step03_check(self, db: InfrahubDatabase, client: InfrahubClient, }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } assert success diff --git a/backend/tests/integration/schema_lifecycle/test_schema_attribute_remove_add.py b/backend/tests/integration/schema_lifecycle/test_schema_attribute_remove_add.py index f18ff14f21..dd58f6d976 100644 --- a/backend/tests/integration/schema_lifecycle/test_schema_attribute_remove_add.py +++ b/backend/tests/integration/schema_lifecycle/test_schema_attribute_remove_add.py @@ -152,6 +152,13 @@ async def test_step02_check_attr_add_rename( }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } async def test_step02_load(self, db: InfrahubDatabase, client: InfrahubClient, initial_dataset, schema_step02): @@ -185,6 +192,13 @@ async def test_step03_check(self, db: InfrahubDatabase, client: InfrahubClient, }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } assert success diff --git a/backend/tests/integration/schema_lifecycle/test_schema_migration_branch.py b/backend/tests/integration/schema_lifecycle/test_schema_migration_branch.py index 95a3ee8584..6390d880e4 100644 --- a/backend/tests/integration/schema_lifecycle/test_schema_migration_branch.py +++ b/backend/tests/integration/schema_lifecycle/test_schema_migration_branch.py @@ -195,6 +195,13 @@ async def test_step02_check_attr_add_rename( }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } async def test_step02_load_attr_add_rename( @@ -283,6 +290,13 @@ async def test_step03_check(self, db: InfrahubDatabase, client: InfrahubClient, }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } assert success @@ -357,7 +371,16 @@ async def test_step04_check(self, db: InfrahubDatabase, client: InfrahubClient, success, response = await client.schema.check(schemas=[schema_step04], branch=self.branch1.name) - assert response == {"diff": {"added": {}, "changed": {}, "removed": {"TestingTag": None}}} + assert response == { + "diff": {"added": {}, "changed": {}, "removed": {"TestingTag": None}}, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], + } assert success async def test_step04_load(self, db: InfrahubDatabase, client: InfrahubClient, initial_dataset, schema_step04): @@ -416,7 +439,14 @@ async def test_step05_check(self, db: InfrahubDatabase, client: InfrahubClient, "removed": {}, }, }, - } + }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } assert success diff --git a/backend/tests/integration/schema_lifecycle/test_schema_migration_main.py b/backend/tests/integration/schema_lifecycle/test_schema_migration_main.py index 7d400b996d..caed0232a3 100644 --- a/backend/tests/integration/schema_lifecycle/test_schema_migration_main.py +++ b/backend/tests/integration/schema_lifecycle/test_schema_migration_main.py @@ -124,6 +124,13 @@ async def test_step02_check_attr_add_rename( }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } async def test_step02_load_attr_add_rename( @@ -196,6 +203,13 @@ async def test_step03_check(self, db: InfrahubDatabase, client: InfrahubClient, }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } assert success @@ -239,7 +253,16 @@ async def test_step04_check(self, db: InfrahubDatabase, client: InfrahubClient, success, response = await client.schema.check(schemas=[schema_step04]) - assert response == {"diff": {"added": {}, "changed": {}, "removed": {"TestingTag": None}}} + assert response == { + "diff": {"added": {}, "changed": {}, "removed": {"TestingTag": None}}, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], + } assert success async def test_step04_load(self, db: InfrahubDatabase, client: InfrahubClient, initial_dataset, schema_step04): @@ -270,7 +293,14 @@ async def test_step05_check(self, db: InfrahubDatabase, client: InfrahubClient, "removed": {}, }, }, - } + }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } assert success diff --git a/backend/tests/integration/schema_lifecycle/test_schema_validator_main.py b/backend/tests/integration/schema_lifecycle/test_schema_validator_main.py index 47fae2baa6..1c4823ec3b 100644 --- a/backend/tests/integration/schema_lifecycle/test_schema_validator_main.py +++ b/backend/tests/integration/schema_lifecycle/test_schema_validator_main.py @@ -300,6 +300,13 @@ async def test_step_02_check_attr_regex_add_success( }, }, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingCar", "field": None}], + "message": "default_filter is deprecated", + } + ], } async def test_step_03_check_relationship_cardinality_change_failure( diff --git a/backend/tests/integration/schema_lifecycle/test_unique_field_updates.py b/backend/tests/integration/schema_lifecycle/test_unique_field_updates.py index 9946264937..a6a6d23197 100644 --- a/backend/tests/integration/schema_lifecycle/test_unique_field_updates.py +++ b/backend/tests/integration/schema_lifecycle/test_unique_field_updates.py @@ -268,6 +268,13 @@ async def test_step02_check_more_fields( }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingPerson", "field": None}], + "message": "display_labels are deprecated, use display_label instead", + } + ], } async def test_step02_load_more_fields( @@ -347,6 +354,13 @@ async def test_step03_check_unique_fields( }, "removed": {}, }, + "warnings": [ + { + "type": "deprecation", + "kinds": [{"kind": "TestingPerson", "field": None}], + "message": "display_labels are deprecated, use display_label instead", + } + ], } async def test_step03_load_unique_fields( @@ -428,6 +442,7 @@ async def test_step04_check_rename_name( }, "removed": {}, }, + "warnings": [], } async def test_step04_load_renamed_fields( @@ -502,6 +517,7 @@ async def test_step05_check_remove_original_unique( }, "removed": {}, }, + "warnings": [], } async def test_step05_load_remove_original_unique( @@ -569,6 +585,7 @@ async def test_step06_check_remove_unique_attr_from_generic( }, "removed": {}, }, + "warnings": [], } async def test_step06_load_remove_unique_attr_from_generic( diff --git a/backend/tests/unit/core/test_schema.py b/backend/tests/unit/core/test_schema.py index 233256974c..e8546ff642 100644 --- a/backend/tests/unit/core/test_schema.py +++ b/backend/tests/unit/core/test_schema.py @@ -1,3 +1,4 @@ +from dataclasses import dataclass from typing import Any, Hashable import pytest @@ -12,6 +13,9 @@ NodeSchema, RelationshipSchema, SchemaRoot, + SchemaWarning, + SchemaWarningKind, + SchemaWarningType, core_models, internal_schema, ) @@ -22,6 +26,118 @@ from infrahub.database import InfrahubDatabase +@dataclass +class SchemaWarningTestCaseData: + name: str + schema: SchemaRoot + warnings: list[SchemaWarning] + + +SCHEMA_WARNING_TESTCASES: list[SchemaWarningTestCaseData] = [ + SchemaWarningTestCaseData( + name="use_display_labels", + schema=SchemaRoot( + nodes=[ + NodeSchema( + namespace="Test", + name="Unit", + display_labels=["name__value"], + attributes=[AttributeSchema(name="name", kind="Text")], + ) + ] + ), + warnings=[ + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind="TestUnit")], + message="display_labels are deprecated, use display_label instead", + ) + ], + ), + SchemaWarningTestCaseData( + name="use_default_filter", + schema=SchemaRoot( + nodes=[ + NodeSchema( + namespace="Test", + name="Unit", + default_filter="name__value", + attributes=[AttributeSchema(name="name", kind="Text")], + ) + ] + ), + warnings=[ + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind="TestUnit")], + message="default_filter is deprecated", + ) + ], + ), + SchemaWarningTestCaseData( + name="use_default_filter_and_display_labels", + schema=SchemaRoot( + nodes=[ + NodeSchema( + namespace="Test", + name="Unit", + default_filter="name__value", + display_labels=["name__value"], + attributes=[AttributeSchema(name="name", kind="Text")], + ) + ] + ), + warnings=[ + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind="TestUnit")], + message="display_labels are deprecated, use display_label instead", + ), + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind="TestUnit")], + message="default_filter is deprecated", + ), + ], + ), + SchemaWarningTestCaseData( + name="use_min_max_length_on_attribute", + schema=SchemaRoot( + nodes=[ + NodeSchema( + namespace="Test", + name="Ticket", + attributes=[AttributeSchema(name="name", kind="Text", min_length=1, max_length=40)], + ) + ] + ), + warnings=[ + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind="TestTicket", field="name")], + message="Use of 'max_length' on attributes is deprecated, use parameters instead", + ), + SchemaWarning( + type=SchemaWarningType.DEPRECATION, + kinds=[SchemaWarningKind(kind="TestTicket", field="name")], + message="Use of 'min_length' on attributes is deprecated, use parameters instead", + ), + ], + ), +] + + +@pytest.mark.parametrize( + "test_case", + [pytest.param(tc, id=tc.name) for tc in SCHEMA_WARNING_TESTCASES], +) +async def test_schema_warnings( + test_case: SchemaWarningTestCaseData, +) -> None: + """Validate that the expected warnings show up for each schema.""" + assert test_case.schema.gather_warnings() == test_case.warnings + + def test_schema_root_no_generic(): FULL_SCHEMA = { "nodes": [ diff --git a/changelog/+45c6002b.added.md b/changelog/+45c6002b.added.md new file mode 100644 index 0000000000..573bf507de --- /dev/null +++ b/changelog/+45c6002b.added.md @@ -0,0 +1 @@ +Added a 'warnings' attribute to the API payload for /api/schema/load and /api/schema/check, so that deprecation warnings could be returned for non critical schema violations. diff --git a/changelog/+4968885a.added.md b/changelog/+4968885a.added.md new file mode 100644 index 0000000000..de21fda7ee --- /dev/null +++ b/changelog/+4968885a.added.md @@ -0,0 +1 @@ +Added warnings to API endpoints /api/schema/check and /api/schema/load, as a way to notify schema developers when they use deprecated fields.