Runtime JSON Schema validation of settings file (#1099)

* Runtime json scheam validation

* Move jsonschema dep to requirements

* Test updates

* Changelog

* Replay empty

* Delete ansible-navigator.yml

* Test fix

Co-authored-by: Ganesh Nalawade <ganesh634@gmail.com>

docs/changelog-fragments.d/1099.feature.md

@@ -0,0 +1,4 @@
+Added settings file validation against the settings file JSON Schema during application
+initialization.
+
+-- by {user}`cidrblock`

setup.cfg

@@ -37,6 +37,7 @@ install_requires =
     ansible-builder >=1, <2
     ansible-runner >=2, <3
     jinja2
+    jsonschema
     onigurumacffi >=1.1.0, <2
     pyyaml
     importlib-resources; python_version < "3.9.0"

src/ansible_navigator/configuration_subsystem/configurator.py

@@ -13,11 +13,13 @@
 from ..utils.functions import LogMessage
 from ..utils.functions import oxfordcomma
 from ..utils.functions import shlex_join
+from ..utils.json_schema import validate
 from ..utils.serialize import SafeLoader
 from ..utils.serialize import yaml
 from .definitions import ApplicationConfiguration
 from .definitions import Constants as C
 from .parser import Parser
+from .transform import to_schema
 
 
 class Configurator:
@@ -137,12 +139,15 @@ def _apply_defaults(self) -> None:
                 self._messages.append(LogMessage(level=logging.INFO, message=message))
 
     def _apply_settings_file(self) -> None:
+        # pylint: disable=too-many-locals
         settings_filesystem_path = self._config.internals.settings_file_path
         if isinstance(settings_filesystem_path, str):
             with open(settings_filesystem_path, "r", encoding="utf-8") as fh:
                 try:
                     config = yaml.load(fh, Loader=SafeLoader)
-                except (yaml.scanner.ScannerError, yaml.parser.ParserError) as exc:
+                    if config is None:
+                        raise ValueError("Settings file cannot be empty.")
+                except (yaml.scanner.ScannerError, yaml.parser.ParserError, ValueError) as exc:
                     exit_msg = (
                         f"Settings file found {settings_filesystem_path}, but failed to load it."
                     )
@@ -157,6 +162,31 @@ def _apply_settings_file(self) -> None:
                         ExitMessage(message=exit_msg, prefix=ExitPrefix.HINT),
                     )
                     return
+
+            schema = to_schema(settings=self._config)
+            errors = validate(schema=schema, data=config)
+            if errors:
+                msg = (
+                    "The following errors were found in the settings file"
+                    f" ({settings_filesystem_path}):"
+                )
+                self._exit_messages.append(ExitMessage(message=msg))
+                self._exit_messages.extend(error.to_exit_message() for error in errors)
+                hint = "Check the settings file and compare it to the current version."
+                self._exit_messages.append(ExitMessage(message=hint, prefix=ExitPrefix.HINT))
+                hint = (
+                    "The current version can be found here:"
+                    " (https://ansible-navigator.readthedocs.io/en/latest/settings/"
+                    "#ansible-navigator-settings)"
+                )
+                self._exit_messages.append(ExitMessage(message=hint, prefix=ExitPrefix.HINT))
+                hint = (
+                    "The schema used for validation can be seen with"
+                    " 'ansible-navigator settings --schema'"
+                )
+                self._exit_messages.append(ExitMessage(message=hint, prefix=ExitPrefix.HINT))
+                return
+
             for entry in self._config.entries:
                 settings_file_path = entry.settings_file_path(self._config.application_name)
                 path_parts = settings_file_path.split(".")

src/ansible_navigator/configuration_subsystem/navigator_configuration.py

@@ -424,7 +424,7 @@ class Internals:
             environment_variable_override="ansible_navigator_pass_environment_variables",
             settings_file_path_override="execution-environment.environment-variables.pass",
             short_description=(
-                "Specify an exiting environment variable to be passed through"
+                "Specify an existing environment variable to be passed through"
                 " to and set within the execution environment (--penv MY_VAR)"
             ),
             value=SettingsEntryValue(),

src/ansible_navigator/configuration_subsystem/transform.py

@@ -60,6 +60,8 @@ def to_schema(settings: ApplicationConfiguration) -> str:
             subschema[dot_parts[-1]]["enum"] = entry.choices
         if entry.value.default is not Constants.NOT_SET:
             subschema[dot_parts[-1]]["default"] = entry.value.default
+
+    PARTIAL_SCHEMA["version"] = settings.application_version
     return serialize(
         content=PARTIAL_SCHEMA,
         content_view=ContentView.NORMAL,

src/ansible_navigator/utils/json_schema.py

@@ -0,0 +1,118 @@
+"""Functionality to perform json schema validation."""
+
+import json
+
+from dataclasses import dataclass
+from typing import Any
+from typing import Deque
+from typing import Dict
+from typing import List
+from typing import Union
+
+from jsonschema import SchemaError
+from jsonschema import ValidationError
+from jsonschema.validators import validator_for
+
+from .functions import ExitMessage
+
+
+def to_path(schema_path: Deque[str]):
+    """Flatten a path to a dot delimited string.
+
+    :param schema_path: The schema path
+    :returns: The dot delimited path
+    """
+    return ".".join(str(index) for index in schema_path)
+
+
+def json_path(absolute_path: Deque[str]):
+    """Flatten a data path to a dot delimited string.
+
+    :param absolute_path: The path
+    :returns: The dot delimited string
+    """
+    path = "$"
+    for elem in absolute_path:
+        if isinstance(elem, int):
+            path += "[" + str(elem) + "]"
+        else:
+            path += "." + elem
+    return path
+
+
+@dataclass
+class JsonSchemaError:
+    # pylint: disable=too-many-instance-attributes
+    """Data structure to hold a json schema validation error."""
+
+    message: str
+    data_path: str
+    json_path: str
+    schema_path: str
+    relative_schema: str
+    expected: Union[bool, int, str]
+    validator: str
+    found: str
+
+    def to_friendly(self):
+        """Provide a friendly explanation of the error.
+
+        :returns: The error message
+        """
+        return f"In '{self.data_path}': {self.message}."
+
+    def to_exit_message(self):
+        """Provide an exit message for a schema validation failure.
+
+        :returns: The exit message
+        """
+        return ExitMessage(message=self.to_friendly())
+
+
+def validate(schema: Union[str, Dict[str, Any]], data: Dict[str, Any]) -> List[JsonSchemaError]:
+    """Validate some data against a JSON schema.
+
+    :param schema: the JSON schema to use for validation
+    :param data: The data to validate
+    :returns: Any errors encountered
+    """
+    errors: List[JsonSchemaError] = []
+
+    if isinstance(schema, str):
+        schema = json.loads(schema)
+    validator = validator_for(schema)
+    try:
+        validator.check_schema(schema)
+    except SchemaError as exc:
+        error = JsonSchemaError(
+            message=str(exc),
+            data_path="schema sanity check",
+            json_path="",
+            schema_path="",
+            relative_schema="",
+            expected="",
+            validator="",
+            found="",
+        )
+        errors.append(error)
+        return errors
+
+    validation_errors = sorted(validator(schema).iter_errors(data), key=lambda e: e.path)
+
+    if not validation_errors:
+        return errors
+
+    for validation_error in validation_errors:
+        if isinstance(validation_error, ValidationError):
+            error = JsonSchemaError(
+                message=validation_error.message,
+                data_path=to_path(validation_error.absolute_path),
+                json_path=json_path(validation_error.absolute_path),
+                schema_path=to_path(validation_error.relative_schema_path),
+                relative_schema=validation_error.schema,
+                expected=validation_error.validator_value,
+                validator=validation_error.validator,
+                found=validation_error.instance,
+            )
+            errors.append(error)
+    return errors

test-requirements.txt

@@ -2,7 +2,6 @@ ansible-core
 darglint
 flake8-docstrings
 flake8-quotes
-jsonschema
 libtmux
 lxml
 pre-commit

tests/fixtures/integration/actions/replay/ansible-navigator.yml

@@ -1,3 +1,2 @@
 ---
-ansible-navigator:
-  empty: True
+ansible-navigator: {}

tests/fixtures/integration/execution_environment/ansible-navigator_empty.yml

@@ -1,3 +1,2 @@
 ---
-ansible-navigator:
-  empty: True
+ansible-navigator: {}

tests/fixtures/integration/execution_environment_image/ansible-navigator_empty.yml

@@ -1,3 +1,2 @@
 ---
-ansible-navigator:
-  empty: True
+ansible-navigator: {}

tests/fixtures/integration/pass_environment_variable/ansible-navigator_empty.yml

@@ -1,3 +1,2 @@
 ---
-ansible-navigator:
-  empty: True
+ansible-navigator: {}

tests/fixtures/integration/set_environment_variable/ansible-navigator_empty.yml

@@ -1,3 +1,2 @@
 ---
-ansible-navigator:
-  empty: True
+ansible-navigator: {}

tests/fixtures/unit/cli/ansible-navigator.yml

@@ -1,17 +1,20 @@
 ---
 ansible-navigator:
+  documentation:
+    plugin:
+      type: become
   editor:
     command: emacs -nw +{line_number} {filename}
     console: False
-  doc-plugin-type: become
   execution-environment:
     container-engine: podman
-    enable: False
+    enabled: False
     image: quay.io/ansible/creator-ee:v0.2.0
   inventory-columns:
     - ansible_network_os
     - ansible_network_cli_ssh_type
     - ansible_connection
-  osc4: True
+  color:
+    osc4: True
   logging:
     level: critical

tests/fixtures/unit/cli/ansible-navigator_empty.yml

@@ -1,3 +1,2 @@
 ---
-ansible-navigator:
-  empty: True
+ansible-navigator: {}

tests/fixtures/unit/configuration_subsystem/ansible-navigator_empty.yml

@@ -1,3 +1,2 @@
 ---
-ansible-navigator:
-  empty: True
+ansible-navigator: {}

tests/fixtures/unit/configuration_subsystem/ansible-navigator_unknown_key.yml

@@ -0,0 +1,3 @@
+---
+ansible-navigator:
+  unknown: key

tests/integration/settings_from_cli/__init__.py

@@ -0,0 +1 @@
+"""Tests various scenarios from the CLI."""

tests/integration/settings_from_cli/test_json_schema_errors.py

@@ -0,0 +1,99 @@
+"""Check exit messages for json schema validation."""
+import os
+import subprocess
+
+from dataclasses import dataclass
+from pathlib import Path
+from typing import Any
+from typing import Tuple
+
+import pytest
+
+from ansible_navigator.utils.functions import shlex_join
+from ...defaults import FIXTURES_DIR
+
+
+TEST_FIXTURE_DIR = Path(FIXTURES_DIR) / "unit" / "configuration_subsystem"
+
+
+@dataclass
+class Scenario:
+    """Data for the tests."""
+
+    comment: str
+    """The comment for the test"""
+    settings_file: Path
+    """The settings file path"""
+    messages: Tuple[str, ...]
+    """Messages expected to be found"""
+
+    command: Tuple[str, ...] = ("ansible-navigator", "-m", "stdout")
+    """The command to run"""
+
+    def __str__(self):
+        """Provide a test id.
+
+        :returns: The test id
+        """
+        return self.comment
+
+
+test_data = (
+    Scenario(
+        comment="Empty settings file",
+        messages=("Settings file cannot be empty",),
+        settings_file=TEST_FIXTURE_DIR / "ansible-navigator_broken.yml",
+    ),
+    Scenario(
+        comment="Unrecognized key",
+        messages=("'unknown' was unexpected",),
+        settings_file=TEST_FIXTURE_DIR / "ansible-navigator_unknown_key.yml",
+    ),
+    Scenario(
+        comment="Unrecognized app",
+        messages=("'non_app' is not one of ['builder',",),
+        settings_file=TEST_FIXTURE_DIR / "ansible-navigator_no_app.yml",
+    ),
+    Scenario(
+        comment="EE enabled is not a bool",
+        messages=("5 is not one of [True, False]",),
+        settings_file=TEST_FIXTURE_DIR / "ansible-navigator_not_bool.yml",
+    ),
+)
+
+
+@pytest.mark.parametrize("data", test_data, ids=str)
+def test(data: Scenario, subtests: Any, tmp_path: Path):
+    """Test for json schema errors.
+
+    :param data: The test data
+    :param tmp_path: The temporary path fixture
+    :param subtests: The pytest subtest fixture
+
+    :raises AssertionError: When tests fails
+    """
+    assert data.settings_file.exists()
+    venv_path = os.environ.get("VIRTUAL_ENV")
+    if venv_path is None:
+        raise AssertionError(
+            "VIRTUAL_ENV environment variable was not set but tox should have set it.",
+        )
+    venv = Path(venv_path, "bin", "activate")
+    log_file = tmp_path / "log.txt"
+
+    command = list(data.command) + ["--lf", str(log_file)]
+
+    bash_wrapped = f"/bin/bash -c 'source {venv!s} && {shlex_join(command)}'"
+    env = {"ANSIBLE_NAVIGATOR_CONFIG": str(data.settings_file), "NO_COLOR": "true"}
+    proc_out = subprocess.run(
+        bash_wrapped,
+        check=False,
+        env=env,
+        shell=True,
+        stderr=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        universal_newlines=True,
+    )
+    for value in data.messages:
+        with subtests.test(msg=value, value=value):
+            assert value in proc_out.stdout

tests/unit/configuration_subsystem/conftest.py

@@ -55,6 +55,7 @@ def _generate_config(params=None, setting_file_name=None, initial=True) -> Gener
     # make a deep copy here to ensure we do not modify the original
     application_configuration = deepcopy(NavigatorConfiguration)
     application_configuration.internals.initializing = initial
+    application_configuration.application_version = "test"
     application_configuration.internals.settings_file_path = settings_file_path or None
     configurator = Configurator(
         application_configuration=application_configuration,