Merge pull request #363 from opsmill/dga-20250416-fix-menu-cmd

Add `menu validate` command to validate the format of menu files.

Author: dgarros

.yamllint.yml

@@ -7,6 +7,7 @@ ignore: |
   tests/unit/sdk/test_data/schema_encoding_error.yml
   /**/node_modules/**
   tests/unit/sdk/test_data/multiple_files_valid_not_valid.yml
+  tests/fixtures/menus/invalid_yaml.yml
 
 rules:
   new-lines: disable

changelog/+menu-validate.added.md

@@ -0,0 +1 @@
+Add `menu validate` command to validate the format of menu files.

docs/docs/infrahubctl/infrahubctl-menu.mdx

@@ -17,6 +17,7 @@ $ infrahubctl menu [OPTIONS] COMMAND [ARGS]...
 **Commands**:
 
 * `load`: Load one or multiple menu files into...
+* `validate`: Validate one or multiple menu files.
 
 ## `infrahubctl menu load`
 
@@ -38,3 +39,24 @@ $ infrahubctl menu load [OPTIONS] MENUS...
 * `--branch TEXT`: Branch on which to load the menu.
 * `--config-file TEXT`: [env var: INFRAHUBCTL_CONFIG; default: infrahubctl.toml]
 * `--help`: Show this message and exit.
+
+## `infrahubctl menu validate`
+
+Validate one or multiple menu files.
+
+**Usage**:
+
+```console
+$ infrahubctl menu validate [OPTIONS] PATHS...
+```
+
+**Arguments**:
+
+* `PATHS...`: [required]
+
+**Options**:
+
+* `--debug / --no-debug`: [default: no-debug]
+* `--branch TEXT`: Branch on which to validate the objects.
+* `--config-file TEXT`: [env var: INFRAHUBCTL_CONFIG; default: infrahubctl.toml]
+* `--help`: Show this message and exit.

infrahub_sdk/ctl/menu.py

@@ -7,10 +7,14 @@
 from ..async_typer import AsyncTyper
 from ..ctl.client import initialize_client
 from ..ctl.utils import catch_exception, init_logging
-from ..exceptions import ObjectValidationError
+from ..exceptions import ObjectValidationError, ValidationError
 from ..spec.menu import MenuFile
 from .parameters import CONFIG_PARAM
-from .utils import load_yamlfile_from_disk_and_exit
+from .utils import (
+    display_object_validate_format_error,
+    display_object_validate_format_success,
+    load_yamlfile_from_disk_and_exit,
+)
 
 app = AsyncTyper()
 console = Console()
@@ -40,21 +44,54 @@ async def load(
     files = load_yamlfile_from_disk_and_exit(paths=menus, file_type=MenuFile, console=console)
     client = initialize_client()
 
+    has_errors = False
+
+    for file in files:
+        try:
+            await file.validate_format(client=client, branch=branch)
+        except ValidationError as exc:
+            has_errors = True
+            display_object_validate_format_error(file=file, error=exc, console=console)
+
+    if has_errors:
+        raise typer.Exit(1)
+
     for file in files:
-        file.validate_content()
-        schema = await client.schema.get(kind=file.spec.kind, branch=branch)
-
-        for idx, item in enumerate(file.spec.data):
-            try:
-                await file.spec.create_node(
-                    client=client,
-                    schema=schema,
-                    position=[idx + 1],
-                    data=item,
-                    branch=branch,
-                    default_schema_kind=file.spec.kind,
-                    context={"list_index": idx},
-                )
-            except ObjectValidationError as exc:
-                console.print(f"[red] {exc!s}")
-                raise typer.Exit(1)
+        try:
+            await file.process(client=client, branch=branch)
+        except ObjectValidationError as exc:
+            has_errors = True
+            console.print(f"[red] {exc!s}")
+
+    if has_errors:
+        raise typer.Exit(1)
+
+
+@app.command()
+@catch_exception(console=console)
+async def validate(
+    paths: list[Path],
+    debug: bool = False,
+    branch: str = typer.Option(None, help="Branch on which to validate the objects."),
+    _: str = CONFIG_PARAM,
+) -> None:
+    """Validate one or multiple menu files."""
+
+    init_logging(debug=debug)
+
+    logging.getLogger("infrahub_sdk").setLevel(logging.INFO)
+
+    files = load_yamlfile_from_disk_and_exit(paths=paths, file_type=MenuFile, console=console)
+    client = initialize_client()
+
+    has_errors = False
+    for file in files:
+        try:
+            await file.validate_format(client=client, branch=branch)
+            display_object_validate_format_success(file=file, console=console)
+        except ValidationError as exc:
+            has_errors = True
+            display_object_validate_format_error(file=file, error=exc, console=console)
+
+    if has_errors:
+        raise typer.Exit(1)

infrahub_sdk/spec/menu.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from ..yaml import InfrahubFile, InfrahubFileKind
-from .object import InfrahubObjectFileData
+from .object import InfrahubObjectFileData, ObjectFile
 
 
 class InfrahubMenuFileData(InfrahubObjectFileData):
@@ -18,7 +18,7 @@ def enrich_node(cls, data: dict, context: dict) -> dict:
         return data
 
 
-class MenuFile(InfrahubFile):
+class MenuFile(ObjectFile):
     _spec: InfrahubMenuFileData | None = None
 
     @property
@@ -28,7 +28,7 @@ def spec(self) -> InfrahubMenuFileData:
         return self._spec
 
     def validate_content(self) -> None:
-        super().validate_content()
+        InfrahubFile.validate_content(self)
         if self.kind != InfrahubFileKind.MENU:
             raise ValueError("File is not an Infrahub Menu file")
         self._spec = InfrahubMenuFileData(**self.data.spec)

infrahub_sdk/spec/object.py

@@ -6,7 +6,7 @@
 from pydantic import BaseModel, Field
 
 from ..exceptions import ObjectValidationError, ValidationError
-from ..schema import RelationshipSchema
+from ..schema import GenericSchemaAPI, RelationshipKind, RelationshipSchema
 from ..yaml import InfrahubFile, InfrahubFileKind
 
 if TYPE_CHECKING:
@@ -59,6 +59,11 @@ def is_bidirectional(self) -> bool:
     def is_mandatory(self) -> bool:
         if not self.peer_rel:
             return False
+        # For hierarchical node, currently the relationship to the parent is always optional in the schema even if it's mandatory
+        # In order to build the tree from top to bottom, we need to consider it as mandatory
+        # While it should technically work bottom-up, it created some unexpected behavior while loading the menu
+        if self.peer_rel.cardinality == "one" and self.peer_rel.kind == RelationshipKind.HIERARCHY:
+            return True
         return not self.peer_rel.optional
 
     @property
@@ -168,14 +173,28 @@ async def validate_format(self, client: InfrahubClient, branch: str | None = Non
         schema = await client.schema.get(kind=self.kind, branch=branch)
         for idx, item in enumerate(self.data):
             errors.extend(
-                await self.validate_object(client=client, position=[idx + 1], schema=schema, data=item, branch=branch)
+                await self.validate_object(
+                    client=client,
+                    position=[idx + 1],
+                    schema=schema,
+                    data=item,
+                    branch=branch,
+                    default_schema_kind=self.kind,
+                )
             )
         return errors
 
     async def process(self, client: InfrahubClient, branch: str | None = None) -> None:
         schema = await client.schema.get(kind=self.kind, branch=branch)
         for idx, item in enumerate(self.data):
-            await self.create_node(client=client, schema=schema, data=item, position=[idx + 1], branch=branch)
+            await self.create_node(
+                client=client,
+                schema=schema,
+                data=item,
+                position=[idx + 1],
+                branch=branch,
+                default_schema_kind=self.kind,
+            )
 
     @classmethod
     async def validate_object(
@@ -186,6 +205,7 @@ async def validate_object(
         position: list[int | str],
         context: dict | None = None,
         branch: str | None = None,
+        default_schema_kind: str | None = None,
     ) -> list[ObjectValidationError]:
         errors: list[ObjectValidationError] = []
         context = context.copy() if context else {}
@@ -234,6 +254,7 @@ async def validate_object(
                         data=value,
                         context=context,
                         branch=branch,
+                        default_schema_kind=default_schema_kind,
                     )
                 )
 
@@ -248,6 +269,7 @@ async def validate_related_nodes(
         data: dict | list[dict],
         context: dict | None = None,
         branch: str | None = None,
+        default_schema_kind: str | None = None,
     ) -> list[ObjectValidationError]:
         context = context.copy() if context else {}
         errors: list[ObjectValidationError] = []
@@ -260,7 +282,9 @@ async def validate_related_nodes(
 
         if isinstance(data, dict) and rel_info.format == RelationshipDataFormat.ONE_OBJ:
             peer_kind = data.get("kind") or rel_info.peer_kind
-            peer_schema = await client.schema.get(kind=peer_kind, branch=branch)
+            peer_schema = await cls.get_peer_schema(
+                client=client, peer_kind=peer_kind, branch=branch, default_schema_kind=default_schema_kind
+            )
 
             rel_info.find_matching_relationship(peer_schema=peer_schema)
             context.update(rel_info.get_context(value="placeholder"))
@@ -273,13 +297,16 @@ async def validate_related_nodes(
                     data=data["data"],
                     context=context,
                     branch=branch,
+                    default_schema_kind=default_schema_kind,
                 )
             )
             return errors
 
         if isinstance(data, dict) and rel_info.format == RelationshipDataFormat.MANY_OBJ_DICT_LIST:
             peer_kind = data.get("kind") or rel_info.peer_kind
-            peer_schema = await client.schema.get(kind=peer_kind, branch=branch)
+            peer_schema = await cls.get_peer_schema(
+                client=client, peer_kind=peer_kind, branch=branch, default_schema_kind=default_schema_kind
+            )
 
             rel_info.find_matching_relationship(peer_schema=peer_schema)
             context.update(rel_info.get_context(value="placeholder"))
@@ -294,6 +321,7 @@ async def validate_related_nodes(
                         data=peer_data,
                         context=context,
                         branch=branch,
+                        default_schema_kind=default_schema_kind,
                     )
                 )
             return errors
@@ -302,7 +330,9 @@ async def validate_related_nodes(
             for idx, item in enumerate(data):
                 context["list_index"] = idx
                 peer_kind = item.get("kind") or rel_info.peer_kind
-                peer_schema = await client.schema.get(kind=peer_kind, branch=branch)
+                peer_schema = await cls.get_peer_schema(
+                    client=client, peer_kind=peer_kind, branch=branch, default_schema_kind=default_schema_kind
+                )
 
                 rel_info.find_matching_relationship(peer_schema=peer_schema)
                 context.update(rel_info.get_context(value="placeholder"))
@@ -315,6 +345,7 @@ async def validate_related_nodes(
                         data=item["data"],
                         context=context,
                         branch=branch,
+                        default_schema_kind=default_schema_kind,
                     )
                 )
             return errors
@@ -345,7 +376,13 @@ async def create_node(
         context = context.copy() if context else {}
 
         errors = await cls.validate_object(
-            client=client, position=position, schema=schema, data=data, context=context, branch=branch
+            client=client,
+            position=position,
+            schema=schema,
+            data=data,
+            context=context,
+            branch=branch,
+            default_schema_kind=default_schema_kind,
         )
         if errors:
             messages = [str(error) for error in errors]
@@ -480,7 +517,9 @@ async def create_related_nodes(
 
         if isinstance(data, dict) and rel_info.format == RelationshipDataFormat.MANY_OBJ_DICT_LIST:
             peer_kind = data.get("kind") or rel_info.peer_kind
-            peer_schema = await client.schema.get(kind=peer_kind, branch=branch)
+            peer_schema = await cls.get_peer_schema(
+                client=client, peer_kind=peer_kind, branch=branch, default_schema_kind=default_schema_kind
+            )
 
             if parent_node:
                 rel_info.find_matching_relationship(peer_schema=peer_schema)
@@ -506,7 +545,9 @@ async def create_related_nodes(
                 context["list_index"] = idx
 
                 peer_kind = item.get("kind") or rel_info.peer_kind
-                peer_schema = await client.schema.get(kind=peer_kind, branch=branch)
+                peer_schema = await cls.get_peer_schema(
+                    client=client, peer_kind=peer_kind, branch=branch, default_schema_kind=default_schema_kind
+                )
 
                 if parent_node:
                     rel_info.find_matching_relationship(peer_schema=peer_schema)
@@ -529,6 +570,23 @@ async def create_related_nodes(
             f"Relationship {rel_info.rel_schema.name} doesn't have the right format {rel_info.rel_schema.cardinality} / {type(data)}"
         )
 
+    @classmethod
+    async def get_peer_schema(
+        cls, client: InfrahubClient, peer_kind: str, branch: str | None = None, default_schema_kind: str | None = None
+    ) -> MainSchemaTypesAPI:
+        peer_schema = await client.schema.get(kind=peer_kind, branch=branch)
+        if not isinstance(peer_schema, GenericSchemaAPI):
+            return peer_schema
+
+        if not default_schema_kind:
+            raise ValueError(f"Found a peer schema as a generic {peer_kind} but no default value was provided")
+
+        # if the initial peer_kind was a generic, we try the default_schema_kind
+        peer_schema = await client.schema.get(kind=default_schema_kind, branch=branch)
+        if isinstance(peer_schema, GenericSchemaAPI):
+            raise ValueError(f"Default schema kind {default_schema_kind} can't be a generic")
+        return peer_schema
+
 
 class ObjectFile(InfrahubFile):
     _spec: InfrahubObjectFileData | None = None

tests/fixtures/menus/invalid_menu.yml

@@ -0,0 +1,20 @@
+---
+apiVersion: infrahub.app/v1
+kind: Menu
+spec:
+  data:
+    - nampace: Testing
+      name: Animal
+      label: Animals
+      kind: TestingAnimal
+      children:
+        data:
+          - namespace: Testing
+            name: Dog
+            label: Dog
+            kind: TestingDog
+
+          - namespace: Testing
+            name: Cat
+            label: Cat
+            kind: TestingCat

tests/fixtures/menus/invalid_yaml.yml

@@ -0,0 +1,20 @@
+---
+apiVersion: infrahub.app/v1
+kind: Menu
+spec:
+    - namespace: Testing
+      - not valid
+      name: Animal
+      label: Animals
+      kind: TestingAnimal
+      children:
+        data:
+          - namespace: Testing
+            name: Dog
+            label: Dog
+            kind: TestingDog
+
+          - namespace: Testing
+            name: Cat
+            label: Cat
+            kind: TestingCat