Skip to content
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
1 change: 1 addition & 0 deletions .yamllint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions changelog/+menu-validate.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `menu validate` command to validate the format of menu files.
22 changes: 22 additions & 0 deletions docs/docs/infrahubctl/infrahubctl-menu.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand All @@ -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.
75 changes: 56 additions & 19 deletions infrahub_sdk/ctl/menu.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -40,21 +44,54 @@
files = load_yamlfile_from_disk_and_exit(paths=menus, file_type=MenuFile, console=console)
client = initialize_client()

has_errors = False

Check warning on line 47 in infrahub_sdk/ctl/menu.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/ctl/menu.py#L47

Added line #L47 was not covered by tests

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)

Check warning on line 54 in infrahub_sdk/ctl/menu.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/ctl/menu.py#L50-L54

Added lines #L50 - L54 were not covered by tests

if has_errors:
raise typer.Exit(1)

Check warning on line 57 in infrahub_sdk/ctl/menu.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/ctl/menu.py#L57

Added line #L57 was not covered by tests

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}")

Check warning on line 64 in infrahub_sdk/ctl/menu.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/ctl/menu.py#L60-L64

Added lines #L60 - L64 were not covered by tests

if has_errors:
raise typer.Exit(1)

Check warning on line 67 in infrahub_sdk/ctl/menu.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/ctl/menu.py#L67

Added line #L67 was not covered by tests


@app.command()
@catch_exception(console=console)
async def validate(
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice with a unit test even if it's just for the failure scenario

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I added some unit tests

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)
6 changes: 3 additions & 3 deletions infrahub_sdk/spec/menu.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
Expand All @@ -28,7 +28,7 @@ def spec(self) -> InfrahubMenuFileData:
return self._spec

def validate_content(self) -> None:
super().validate_content()
InfrahubFile.validate_content(self)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we making this change? Swapping from inheriting from InfrahubFile and moving to ObjectFile instead but we override the spec method from ObjectFile completely and then call the .validate_content on InfrahubFile I assume to avoid using the one from ObjectFile. Am I missing something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree this isn't ideal, and it calls for a larger redesign / cleaned of the ObjectFile / YamlFile / MenuFile but I think this is a larger effort and I would prefer to manage that separately
I will open a dedicated issue to track that

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if self.kind != InfrahubFileKind.MENU:
raise ValueError("File is not an Infrahub Menu file")
self._spec = InfrahubMenuFileData(**self.data.spec)
76 changes: 67 additions & 9 deletions infrahub_sdk/spec/object.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -59,6 +59,11 @@
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
Expand Down Expand Up @@ -168,14 +173,28 @@
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(
Expand All @@ -186,6 +205,7 @@
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 {}
Expand Down Expand Up @@ -234,6 +254,7 @@
data=value,
context=context,
branch=branch,
default_schema_kind=default_schema_kind,
)
)

Expand All @@ -248,6 +269,7 @@
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] = []
Expand All @@ -260,7 +282,9 @@

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"))
Expand All @@ -273,13 +297,16 @@
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"))
Expand All @@ -294,6 +321,7 @@
data=peer_data,
context=context,
branch=branch,
default_schema_kind=default_schema_kind,
)
)
return errors
Expand All @@ -302,7 +330,9 @@
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"))
Expand All @@ -315,6 +345,7 @@
data=item["data"],
context=context,
branch=branch,
default_schema_kind=default_schema_kind,
)
)
return errors
Expand Down Expand Up @@ -345,7 +376,13 @@
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]
Expand Down Expand Up @@ -480,7 +517,9 @@

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)
Expand All @@ -506,7 +545,9 @@
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)
Expand All @@ -529,6 +570,23 @@
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")

Check warning on line 582 in infrahub_sdk/spec/object.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/spec/object.py#L582

Added line #L582 was not covered by tests

# 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")

Check warning on line 587 in infrahub_sdk/spec/object.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/spec/object.py#L587

Added line #L587 was not covered by tests
return peer_schema


class ObjectFile(InfrahubFile):
_spec: InfrahubObjectFileData | None = None
Expand Down
20 changes: 20 additions & 0 deletions tests/fixtures/menus/invalid_menu.yml
Original file line number Diff line number Diff line change
@@ -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
20 changes: 20 additions & 0 deletions tests/fixtures/menus/invalid_yaml.yml
Original file line number Diff line number Diff line change
@@ -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
Loading