From 28ba8e135d0856dd1918a8c78a6bf6eb97958146 Mon Sep 17 00:00:00 2001 From: Guillaume Mulocher Date: Thu, 16 May 2024 12:20:12 +0200 Subject: [PATCH] refactor: Fix FAQ and other pieces of code (#679) --- Dockerfile | 4 +- anta/catalog.py | 8 ++-- anta/cli/exec/utils.py | 53 ++++++++++++++------------- anta/cli/utils.py | 5 +-- anta/custom_types.py | 8 +--- anta/tests/field_notices.py | 1 - docs/faq.md | 4 +- docs/stylesheets/extra.material.css | 9 ----- tests/units/cli/nrfu/test_commands.py | 2 +- tests/units/test_models.py | 1 - 10 files changed, 40 insertions(+), 55 deletions(-) diff --git a/Dockerfile b/Dockerfile index 2a0ef53f3..f3d241cca 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ ARG IMG_OPTION=alpine ### BUILDER -FROM python:${PYTHON_VER}-${IMG_OPTION} as BUILDER +FROM python:${PYTHON_VER}-${IMG_OPTION} AS BUILDER RUN pip install --upgrade pip @@ -19,7 +19,7 @@ RUN pip --no-cache-dir install --user . ### BASE -FROM python:${PYTHON_VER}-${IMG_OPTION} as BASE +FROM python:${PYTHON_VER}-${IMG_OPTION} AS BASE # Opencontainer labels # Labels version and revision will be updating diff --git a/anta/catalog.py b/anta/catalog.py index 025654bba..142640ecb 100644 --- a/anta/catalog.py +++ b/anta/catalog.py @@ -176,12 +176,12 @@ def flatten_modules(data: dict[str, Any], package: str | None = None) -> dict[Mo if isinstance(tests, dict): # This is an inner Python module modules.update(AntaCatalogFile.flatten_modules(data=tests, package=module.__name__)) - else: - if not isinstance(tests, list): - msg = f"Syntax error when parsing: {tests}\nIt must be a list of ANTA tests. Check the test catalog." - raise ValueError(msg) # noqa: TRY004 pydantic catches ValueError or AssertionError, no TypeError + elif isinstance(tests, list): # This is a list of AntaTestDefinition modules[module] = tests + else: + msg = f"Syntax error when parsing: {tests}\nIt must be a list of ANTA tests. Check the test catalog." + raise ValueError(msg) # noqa: TRY004 pydantic catches ValueError or AssertionError, no TypeError return modules # ANN401 - Any ok for this validator as we are validating the received data diff --git a/anta/cli/exec/utils.py b/anta/cli/exec/utils.py index 5a28912e1..34156e2d3 100644 --- a/anta/cli/exec/utils.py +++ b/anta/cli/exec/utils.py @@ -106,12 +106,12 @@ async def collect(device: AntaDevice) -> None: cmd += f" | head -{latest}" command = AntaCommand(command=cmd, ofmt="text") await device.collect(command=command) - if command.collected and command.text_output: - filenames = [Path(f"{EOS_SCHEDULED_TECH_SUPPORT}/{f}") for f in command.text_output.splitlines()] - else: + if not (command.collected and command.text_output): logger.error("Unable to get tech-support filenames on %s: verify that %s is not empty", device.name, EOS_SCHEDULED_TECH_SUPPORT) return + filenames = [Path(f"{EOS_SCHEDULED_TECH_SUPPORT}/{f}") for f in command.text_output.splitlines()] + # Create directories outdir = Path() / root_dir / f"{device.name.lower()}" outdir.mkdir(parents=True, exist_ok=True) @@ -122,31 +122,32 @@ async def collect(device: AntaDevice) -> None: if command.collected and not command.text_output: logger.debug("'aaa authorization exec default local' is not configured on device %s", device.name) - if configure: - commands = [] - # TODO: @mtache - add `config` field to `AntaCommand` object to handle this use case. - # Otherwise mypy complains about enable as it is only implemented for AsyncEOSDevice - # TODO: Should enable be also included in AntaDevice? - if not isinstance(device, AsyncEOSDevice): - msg = "anta exec collect-tech-support is only supported with AsyncEOSDevice for now." - raise UsageError(msg) - if device.enable and device._enable_password is not None: # pylint: disable=protected-access - commands.append({"cmd": "enable", "input": device._enable_password}) # pylint: disable=protected-access - elif device.enable: - commands.append({"cmd": "enable"}) - commands.extend( - [ - {"cmd": "configure terminal"}, - {"cmd": "aaa authorization exec default local"}, - ], - ) - logger.warning("Configuring 'aaa authorization exec default local' on device %s", device.name) - command = AntaCommand(command="show running-config | include aaa authorization exec default local", ofmt="text") - await device._session.cli(commands=commands) # pylint: disable=protected-access - logger.info("Configured 'aaa authorization exec default local' on device %s", device.name) - else: + if not configure: logger.error("Unable to collect tech-support on %s: configuration 'aaa authorization exec default local' is not present", device.name) return + + commands = [] + # TODO: @mtache - add `config` field to `AntaCommand` object to handle this use case. + # Otherwise mypy complains about enable as it is only implemented for AsyncEOSDevice + # TODO: Should enable be also included in AntaDevice? + if not isinstance(device, AsyncEOSDevice): + msg = "anta exec collect-tech-support is only supported with AsyncEOSDevice for now." + raise UsageError(msg) + if device.enable and device._enable_password is not None: # pylint: disable=protected-access + commands.append({"cmd": "enable", "input": device._enable_password}) # pylint: disable=protected-access + elif device.enable: + commands.append({"cmd": "enable"}) + commands.extend( + [ + {"cmd": "configure terminal"}, + {"cmd": "aaa authorization exec default local"}, + ], + ) + logger.warning("Configuring 'aaa authorization exec default local' on device %s", device.name) + command = AntaCommand(command="show running-config | include aaa authorization exec default local", ofmt="text") + await device._session.cli(commands=commands) # pylint: disable=protected-access + logger.info("Configured 'aaa authorization exec default local' on device %s", device.name) + logger.debug("'aaa authorization exec default local' is already configured on device %s", device.name) await device.copy(sources=filenames, destination=outdir, direction="from") diff --git a/anta/cli/utils.py b/anta/cli/utils.py index 53a97190f..71d99b8ae 100644 --- a/anta/cli/utils.py +++ b/anta/cli/utils.py @@ -12,7 +12,6 @@ from typing import TYPE_CHECKING, Any, Callable import click -from pydantic import ValidationError from yaml import YAMLError from anta.catalog import AntaCatalog @@ -254,7 +253,7 @@ def wrapper( insecure=insecure, disable_cache=disable_cache, ) - except (ValidationError, TypeError, ValueError, YAMLError, OSError, InventoryIncorrectSchemaError, InventoryRootKeyError): + except (TypeError, ValueError, YAMLError, OSError, InventoryIncorrectSchemaError, InventoryRootKeyError): ctx.exit(ExitCode.USAGE_ERROR) return f(*args, inventory=i, tags=tags, **kwargs) @@ -292,7 +291,7 @@ def wrapper( return f(*args, catalog=None, **kwargs) try: c = AntaCatalog.parse(catalog) - except (ValidationError, TypeError, ValueError, YAMLError, OSError): + except (TypeError, ValueError, YAMLError, OSError): ctx.exit(ExitCode.USAGE_ERROR) return f(*args, catalog=c, **kwargs) diff --git a/anta/custom_types.py b/anta/custom_types.py index 944b85143..48711c2ce 100644 --- a/anta/custom_types.py +++ b/anta/custom_types.py @@ -24,7 +24,7 @@ def interface_autocomplete(v: str) -> str: - `po` will be changed to `Port-Channel` - `lo` will be changed to `Loopback` """ - intf_id_re = re.compile(r"[0-9]+(\/[0-9]+)*(\.[0-9]+)?") + intf_id_re = re.compile(r"\d+(\/\d+)*(\.\d+)?") m = intf_id_re.search(v) if m is None: msg = f"Could not parse interface ID in interface '{v}'" @@ -33,11 +33,7 @@ def interface_autocomplete(v: str) -> str: alias_map = {"et": "Ethernet", "eth": "Ethernet", "po": "Port-Channel", "lo": "Loopback"} - for alias, full_name in alias_map.items(): - if v.lower().startswith(alias): - return f"{full_name}{intf_id}" - - return v + return next((f"{full_name}{intf_id}" for alias, full_name in alias_map.items() if v.lower().startswith(alias)), v) def interface_case_sensitivity(v: str) -> str: diff --git a/anta/tests/field_notices.py b/anta/tests/field_notices.py index 5653deeb0..71a11749f 100644 --- a/anta/tests/field_notices.py +++ b/anta/tests/field_notices.py @@ -197,4 +197,3 @@ def test(self) -> None: return # We should never hit this point self.result.is_error("Error in running test - FixedSystemvrm1 not found") - return diff --git a/docs/faq.md b/docs/faq.md index 1cce990bc..c71d1c6f2 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -1,5 +1,5 @@ --- -toc_depth: 4 +toc_depth: 2 ---