feat(inventory): reject unread keys and a missing Cloudflare token before the deploy

The Keycloak stage on echoserver ran ten minutes before sys-svc-dns stopped
it with "API.cloudflare.api_token must be set": the inventory still kept the
token under the flat name CLOUDFLARE_API_TOKEN, which nothing reads any
more. The validator that runs before every deploy now catches that class.

- api.py reports a host that selects DNS_PROVIDER: cloudflare without
  API.cloudflare.api_token, with group_vars/all merged under the host_vars
- variables.py reports every top-level key of host_vars and group_vars that
  core does not read: not declared in group_vars or a role's defaults or
  vars, not tested with "is defined" or "| default(", not read by name in a
  plugin, and not a SYSTEM_EMAIL_<KEY> the email lookup builds; a flat
  credential name such as CLOUDFLARE_API_TOKEN names its API.<provider>.<key>
  path; ansible_* keys are exempt

A key that is read in a way the scan cannot see takes
"# nocheck: inventory-variable", documented in suppression.md.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

cli/administration/inventory/validate/__init__.py

@@ -6,6 +6,8 @@ Split by concern (DRY/SPOT/KISS):
 - ``applications``: applications-vs-defaults+variants validation
 - ``users``: user-vs-defaults validation
 - ``hosts``: ``all.children`` host-group validation
+- ``api``: API credential validation
+- ``variables``: top-level inventory variables core never reads
 - ``__main__``: argparse + orchestration
 """
 
@@ -16,11 +18,13 @@ from pathlib import Path
 PROJECT_ROOT: Path = Path(__file__).resolve().parents[4]
 
 from .__main__ import main  # noqa: E402
+from .api import validate_api_keys  # noqa: E402
 from .applications import compare_application_keys  # noqa: E402
 from .hosts import validate_host_keys  # noqa: E402
 from .keys import recursive_keys  # noqa: E402
 from .loaders import load_inventory_files, load_yaml_file  # noqa: E402
 from .users import compare_user_keys  # noqa: E402
+from .variables import validate_inventory_variables  # noqa: E402
 
 __all__ = [
     "PROJECT_ROOT",
@@ -30,5 +34,7 @@ __all__ = [
     "load_yaml_file",
     "main",
     "recursive_keys",
+    "validate_api_keys",
     "validate_host_keys",
+    "validate_inventory_variables",
 ]

cli/administration/inventory/validate/__main__.py

@@ -10,6 +10,10 @@ PROJECT_ROOT = Path(__file__).resolve().parents[4]
 if str(PROJECT_ROOT) not in sys.path:
     sys.path.insert(0, str(PROJECT_ROOT))
 
+from cli.administration.inventory.validate.api import (  # noqa: E402
+    legacy_api_keys,
+    validate_api_keys,
+)
 from cli.administration.inventory.validate.applications import (  # noqa: E402
     compare_application_keys,
 )
@@ -19,6 +23,10 @@ from cli.administration.inventory.validate.loaders import (  # noqa: E402
     load_yaml_file,
 )
 from cli.administration.inventory.validate.users import compare_user_keys  # noqa: E402
+from cli.administration.inventory.validate.variables import (  # noqa: E402
+    read_variables,
+    validate_inventory_variables,
+)
 from utils.cache.applications import (  # noqa: E402
     get_application_defaults,
     get_variants,
@@ -67,6 +75,11 @@ def main() -> None:
 
     host_errs = validate_host_keys(set(application_defaults), args.inventory_dir)
     app_errs.extend(host_errs)
+    known = read_variables()
+    app_errs.extend(
+        validate_inventory_variables(args.inventory_dir, known, legacy_api_keys(known))
+    )
+    app_errs.extend(validate_api_keys(args.inventory_dir))
 
     if app_errs or user_errs:
         if app_errs:

cli/administration/inventory/validate/api.py

@@ -0,0 +1,76 @@
+"""API credential validation: moved credential names and a missing Cloudflare token."""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+from utils.api import declared_api
+
+from .loaders import load_yaml_file
+
+
+def legacy_api_keys(declared: set[str]) -> dict[str, str]:
+    """Map the flat names older inventories gave API credentials to their ``API`` path.
+
+    Args:
+        declared: the variables core reads, see ``read_variables()``.
+
+    Returns:
+        ``<PROVIDER>_<KEY>`` in upper case to ``API.<provider>.<key>``, without
+        the names core still reads as a variable of their own.
+    """
+    legacy = {
+        f"{provider}_{key}".upper(): f"API.{provider}.{key}"
+        for provider, keys in declared_api().items()
+        for key in keys
+    }
+    return {name: path for name, path in legacy.items() if name not in declared}
+
+
+def _cloudflare_token(data: dict) -> str:
+    api = data.get("API")
+    provider = api.get("cloudflare") if isinstance(api, dict) else None
+    token = provider.get("api_token") if isinstance(provider, dict) else None
+    return str(token or "").strip()
+
+
+def _merged(paths: list[Path]) -> dict:
+    merged: dict = {}
+    for path in paths:
+        data = load_yaml_file(path)
+        if isinstance(data, dict):
+            merged.update(data)
+    return merged
+
+
+def validate_api_keys(inv_dir) -> list[str]:
+    """Report hosts that select Cloudflare DNS without a Cloudflare token.
+
+    Args:
+        inv_dir: the inventory directory.
+
+    Returns:
+        One message per host whose variables select ``DNS_PROVIDER: cloudflare``
+        without ``API.cloudflare.api_token``.
+    """
+    root = Path(inv_dir)
+    group_all = _merged(
+        sorted(root.glob("group_vars/all.yml"))
+        + sorted(root.glob("group_vars/all/*.yml"))
+    )
+    hosts = [(path, [path]) for path in sorted(root.glob("host_vars/*.yml"))]
+    hosts += [
+        (path, sorted(path.glob("*.yml")))
+        for path in sorted(root.glob("host_vars/*"))
+        if path.is_dir()
+    ]
+    errs: list[str] = []
+    for path, files in hosts or [(root / "group_vars", [])]:
+        effective = {**group_all, **_merged(files)}
+        if effective.get("DNS_PROVIDER") == "cloudflare" and not _cloudflare_token(
+            effective
+        ):
+            errs.append(
+                f"{path}: DNS_PROVIDER is cloudflare, but API.cloudflare.api_token is empty"
+            )
+    return errs

cli/administration/inventory/validate/variables.py

@@ -0,0 +1,123 @@
+"""Inventory variable validation: every top-level key needs a reader in core.
+
+A key counts as read when core declares it in ``group_vars`` or a role's
+``defaults`` or ``vars``, tests it with ``is defined`` or ``| default(`` in its
+YAML or Jinja, reads it by name from the variables in a plugin, or is one of
+the ``SYSTEM_EMAIL_<KEY>`` names the email lookup builds at run time.
+
+Suppression (see ``docs/contributing/actions/testing/suppression.md``):
+
+* ``# nocheck: inventory-variable`` on, or directly above, the key's line.
+"""
+
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+from utils import PROJECT_ROOT
+from utils.annotations.suppress import is_suppressed_at
+from utils.cache.files import iter_project_files, read_text
+from utils.cache.yaml import load_yaml_any
+
+from .loaders import load_yaml_file
+
+_RULE = "inventory-variable"
+_NAME = r"([A-Za-z_][A-Za-z0-9_]*)"
+_HOOK = re.compile(rf"\b{_NAME}\s+is\s+(?:not\s+)?defined|\b{_NAME}\s*\|\s*default\(")
+_PLUGIN_READ = re.compile(
+    rf"(?:variables|available_variables|hostvars\[[^\]]+\])\s*(?:\.get\(\s*|\[\s*)[\"']{_NAME}[\"']"
+)
+_EMAIL_KEYS = re.compile(r"RESOLUTION_ORDER = \(([^)]*)\)")
+
+
+def _relative(path: str) -> tuple[str, ...]:
+    return Path(path).relative_to(PROJECT_ROOT).parts
+
+
+def declared_variables() -> set[str]:
+    """Return every variable core declares in group_vars or a role's defaults and vars.
+
+    Returns:
+        The top-level keys of ``group_vars/**`` and ``roles/*/{defaults,vars}/**``.
+    """
+    names: set[str] = set()
+    for path in iter_project_files(extensions=(".yml", ".yaml")):
+        parts = _relative(path)
+        if parts[0] == "group_vars" or (
+            parts[0] == "roles" and len(parts) > 3 and parts[2] in ("defaults", "vars")
+        ):
+            data = load_yaml_any(path, default_if_missing=None)
+            if isinstance(data, dict):
+                names.update(data)
+    return names
+
+
+def read_variables() -> set[str]:
+    """Return every variable core reads, declared or not.
+
+    Returns:
+        ``declared_variables()`` plus the names core guards with ``is defined``
+        or ``| default(``, the names plugins read from the variables by name
+        and the ``SYSTEM_EMAIL_<KEY>`` names.
+    """
+    names = declared_variables()
+    for path in iter_project_files(
+        extensions=(".yml", ".yaml", ".j2"), exclude_tests=True
+    ):
+        for match in _HOOK.findall(read_text(path)):
+            names.update(name for name in match if name)
+    for path in iter_project_files(extensions=(".py",), exclude_tests=True):
+        if _relative(path)[0] in ("plugins", "utils", "roles", "filter_plugins"):
+            names.update(_PLUGIN_READ.findall(read_text(path)))
+    email = _EMAIL_KEYS.search(
+        read_text(str(PROJECT_ROOT / "plugins" / "lookup" / "email.py"))
+    )
+    names.update(
+        f"SYSTEM_EMAIL_{key.upper()}"
+        for key in re.findall(r"[\"'](\w+)[\"']", email.group(1) if email else "")
+    )
+    return names
+
+
+def validate_inventory_variables(
+    inv_dir, known: set[str], moved: dict[str, str]
+) -> list[str]:
+    """Report top-level inventory variables the deploy never reads.
+
+    Args:
+        inv_dir: the inventory directory.
+        known: the variables core reads, see ``read_variables()``.
+        moved: flat name to the path its value moved to, see ``legacy_api_keys()``.
+
+    Returns:
+        One message per unread key of ``host_vars/**`` and ``group_vars/**``
+        that carries no ``inventory-variable`` marker.
+    """
+    root = Path(inv_dir)
+    errs: list[str] = []
+    files = sorted(
+        {*root.glob("host_vars/**/*.yml"), *root.glob("group_vars/**/*.yml")}
+    )
+    for path in files:
+        data = load_yaml_file(path)
+        if not isinstance(data, dict):
+            continue
+        lines = read_text(str(path)).splitlines()
+        for key in data:
+            name = str(key)
+            if (name in known and name not in moved) or name.startswith("ansible_"):
+                continue
+            number = next(
+                (n for n, line in enumerate(lines, 1) if line.startswith(f"{name}:")),
+                0,
+            )
+            if number and is_suppressed_at(lines, number, _RULE):
+                continue
+            hint = (
+                f"move its value to {moved[name]}"
+                if name in moved
+                else "no group_vars, role defaults or role vars declare it and no plugin reads it"
+            )
+            errs.append(f"{path}: '{name}' is not read by the deploy; {hint}")
+    return errs

docs/contributing/actions/testing/suppression.md

@@ -112,6 +112,7 @@ The placement rule is per check. The catalog "Position" column uses these labels
 | `min-storage-headroom`      | same line OR comment block above key  | [test_min_storage_headroom.py](../../../../tests/external/update/docker/test_min_storage_headroom.py)                                                                                                                                    | Allows a `roles/*/meta/services.yml` `min_storage` to exceed the measured compressed size of its own `image:`/`version:` by more than 1 GB. MUST be paired with a `Reason:` on the same comment line naming what occupies the space. Reserved for services whose data volume genuinely dominates the footprint; the default path is to lower the value, because an over-declared budget makes a host look too small and prunes a cache the deploy did not need to lose. |
 | `healthcheck-binary`        | same or above                         | [test_healthcheck_binary_on_slim_base.py](../../../../tests/lint/ansible/roles/meta/test_healthcheck_binary_on_slim_base.py)                                                                                                             | Exempts a `roles/*/meta/services.yml` healthcheck flavor from the requirement that the role's `files/Dockerfile` install the binary the probe runs, when the service sits on a `-slim` image. MUST name where the binary comes from instead. A `-slim` tag carries the runtime and nothing around it, so `curl` is absent and the probe fails from the first beat; under swarm the task is then reaped with exit 137 and every deployment holding the service in its closure fails on the convergence wait, naming the service rather than the missing binary. |
 | `galaxy-git-parity`         | same or above                         | [test_galaxy_entries_have_git_source.py](../../../../tests/lint/repository/dependencies/test_galaxy_entries_have_git_source.py)                                                                                                          | Exempts a `requirements/requirements.galaxy.yml` collection from the requirement to also carry a `type: git` entry in `requirements/requirements.git.yml`. Reserved for collections that publish no usable git remote; without the git entry a `galaxy.ansible.com` outage fails `make install`, because the fallback in `scripts/install/ansible.sh` cannot resolve the collection either. |
+| `inventory-variable`        | same or above                         | [variables.py](../../../../cli/administration/inventory/validate/variables.py) | Allows a top-level key in an inventory's `host_vars/**` or `group_vars/**` that no `group_vars`, role `defaults` or role `vars` declares and that the validator does not list as read without a declaration. Default path: move the value to the variable the deploy reads, or delete it. Reserved for a helper the inventory reads itself, such as a domain its own templates interpolate. Place it on or directly above the key's line. |
 | `localhost-become`          | same or above                         | [test_localhost_delegation_drops_become.py](../../../../tests/lint/ansible/tasks/test_localhost_delegation_drops_become.py) | Allows a task delegated to `localhost` to keep the play's `become`. Default path: set `become: false` on the task or its enclosing block. Reserved for a step that needs root on the controller, such as a mount or a write below `/var/lib/infinito`; the marker MUST name that step. Place it on or directly above the `delegate_to:` line. |
 | `loop-var-shadow`           | same or above                         | [test_loop_var_shadowing.py](../../../../tests/lint/ansible/tasks/test_loop_var_shadowing.py)                                                                                                                                            | Allows a `loop_var` to take the name of a variable declared in `group_vars/**` or in a role's `vars`/`defaults`. Inside the loop that name carries the item, so every lookup, template and nested include reading it sees the item instead, and ansible only warns. Reserved for a loop whose body reads the name as the item on purpose, typically because the item IS that value: state which meaning the body relies on. |
 | `firewall-fragment-accept`  | same or above                         | [test_firewall_fragment_verdicts.py](../../../../tests/lint/ansible/jinja/test_firewall_fragment_verdicts.py)                                                                                                                            | Allows a rule in a `roles/*/templates/nftables.conf.j2` fragment to end in an `accept` verdict. Default path: express the intent as a `drop` of what must not pass, or put the rule in the chain carrying the policy it has to overrule, because an `accept` terminates only its own base chain and every other chain at that hook still runs, docker's `drop` policy on forwarded packets included. Reserved for an accept whose only job is to skip the fragment's own later rules. |

tests/unit/python/cli/administration/inventory/validate/test_api.py

@@ -0,0 +1,78 @@
+import shutil
+import tempfile
+import unittest
+from pathlib import Path
+
+from cli.administration.inventory.validate.api import (
+    legacy_api_keys,
+    validate_api_keys,
+)
+
+VAULTED_TOKEN = (
+    "API:\n"
+    "  cloudflare:\n"
+    "    api_token: !vault |\n"
+    "      $ANSIBLE_VAULT;1.1;AES256\n"
+    "      3133\n"
+)
+
+
+class TestValidateApiKeys(unittest.TestCase):
+    def setUp(self):
+        self.root = Path(tempfile.mkdtemp())
+        (self.root / "host_vars").mkdir()
+        (self.root / "group_vars").mkdir()
+
+    def tearDown(self):
+        shutil.rmtree(self.root)
+
+    def write(self, relative: str, text: str) -> None:
+        (self.root / relative).write_text(text, encoding="utf-8")
+
+    def test_cloudflare_without_a_token_is_reported(self):
+        self.write("host_vars/echo.yml", "DNS_PROVIDER: cloudflare\n")
+        errors = validate_api_keys(self.root)
+        self.assertEqual(len(errors), 1)
+        self.assertIn("API.cloudflare.api_token is empty", errors[0])
+
+    def test_the_legacy_key_does_not_count_as_a_token(self):
+        self.write(
+            "host_vars/echo.yml",
+            "DNS_PROVIDER: cloudflare\nCLOUDFLARE_API_TOKEN: abc\n",
+        )
+        self.assertEqual(len(validate_api_keys(self.root)), 1)
+
+    def test_a_vaulted_token_in_the_host_file_passes(self):
+        self.write("host_vars/echo.yml", "DNS_PROVIDER: cloudflare\n" + VAULTED_TOKEN)
+        self.assertEqual(validate_api_keys(self.root), [])
+
+    def test_a_token_in_group_vars_all_passes(self):
+        self.write("host_vars/echo.yml", "DNS_PROVIDER: cloudflare\n")
+        self.write("group_vars/all.yml", VAULTED_TOKEN)
+        self.assertEqual(validate_api_keys(self.root), [])
+
+    def test_a_host_api_mapping_replaces_the_group_one(self):
+        self.write(
+            "host_vars/echo.yml",
+            "DNS_PROVIDER: cloudflare\nAPI:\n  telegram:\n    bot_token: x\n",
+        )
+        self.write("group_vars/all.yml", VAULTED_TOKEN)
+        self.assertEqual(len(validate_api_keys(self.root)), 1)
+
+    def test_another_dns_provider_needs_no_token(self):
+        self.write("host_vars/echo.yml", "DNS_PROVIDER: localhost\n")
+        self.assertEqual(validate_api_keys(self.root), [])
+
+
+class TestLegacyApiKeys(unittest.TestCase):
+    def test_a_moved_credential_names_its_api_path(self):
+        self.assertEqual(
+            legacy_api_keys(set())["CLOUDFLARE_API_TOKEN"], "API.cloudflare.api_token"
+        )
+
+    def test_a_name_core_still_declares_is_not_moved(self):
+        self.assertNotIn("TELEGRAM_BOT_TOKEN", legacy_api_keys({"TELEGRAM_BOT_TOKEN"}))
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/unit/python/cli/administration/inventory/validate/test_variables.py

@@ -0,0 +1,94 @@
+import shutil
+import tempfile
+import unittest
+from pathlib import Path
+
+from cli.administration.inventory.validate.variables import (
+    declared_variables,
+    read_variables,
+    validate_inventory_variables,
+)
+
+KNOWN = {"DOMAIN_PRIMARY", "applications"}
+MOVED = {"CLOUDFLARE_API_TOKEN": "API.cloudflare.api_token"}
+
+
+class TestValidateInventoryVariables(unittest.TestCase):
+    def setUp(self):
+        self.root = Path(tempfile.mkdtemp())
+        (self.root / "host_vars").mkdir()
+        (self.root / "group_vars").mkdir()
+
+    def tearDown(self):
+        shutil.rmtree(self.root)
+
+    def errors(self, relative: str, text: str) -> list[str]:
+        (self.root / relative).write_text(text, encoding="utf-8")
+        return validate_inventory_variables(self.root, KNOWN, MOVED)
+
+    def test_a_read_variable_passes(self):
+        text = "DOMAIN_PRIMARY: x\napplications: {}\n"
+        self.assertEqual(self.errors("host_vars/echo.yml", text), [])
+
+    def test_an_unread_constant_fails(self):
+        errors = self.errors("host_vars/echo.yml", "SYSTEM_EMAIL: x\n")
+        self.assertEqual(len(errors), 1)
+        self.assertIn("'SYSTEM_EMAIL' is not read by the deploy", errors[0])
+
+    def test_a_moved_credential_names_its_new_path(self):
+        errors = self.errors("host_vars/echo.yml", "CLOUDFLARE_API_TOKEN: x\n")
+        self.assertIn("move its value to API.cloudflare.api_token", errors[0])
+
+    def test_ansible_variables_pass(self):
+        text = "ansible_host: x\nansible_become_password: y\n"
+        self.assertEqual(self.errors("host_vars/echo.yml", text), [])
+
+    def test_group_vars_are_checked_too(self):
+        self.assertEqual(len(self.errors("group_vars/all.yml", "LEGACY: x\n")), 1)
+
+    def test_a_host_vars_directory_is_checked(self):
+        (self.root / "host_vars" / "echo").mkdir()
+        self.assertEqual(len(self.errors("host_vars/echo/main.yml", "LEGACY: x\n")), 1)
+
+    def test_a_marker_on_the_key_line_allows_it(self):
+        text = "OLD_DOMAIN: x # nocheck: inventory-variable\n"
+        self.assertEqual(self.errors("host_vars/echo.yml", text), [])
+
+    def test_a_marker_above_the_key_allows_it(self):
+        text = "# nocheck: inventory-variable\nOLD_DOMAIN: x\n"
+        self.assertEqual(self.errors("host_vars/echo.yml", text), [])
+
+    def test_a_vaulted_unread_value_fails(self):
+        text = "LEGACY_TOKEN: !vault |\n  $ANSIBLE_VAULT;1.1;AES256\n  3133\n"
+        self.assertEqual(len(self.errors("host_vars/echo.yml", text)), 1)
+
+
+class TestReadVariables(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.read = read_variables()
+
+    def test_declarations_count(self):
+        self.assertLessEqual(declared_variables(), self.read)
+        self.assertIn("TELEGRAM_BOT_TOKEN", self.read)
+
+    def test_guarded_overrides_count(self):
+        self.assertIn("OIDC_OVERRIDE", self.read)
+        self.assertIn("design", self.read)
+
+    def test_plugin_reads_count(self):
+        self.assertIn("networks", self.read)
+        self.assertIn("SYSTEM_EMAIL_DOMAIN", self.read)
+
+    def test_the_email_lookup_names_count(self):
+        self.assertIn("SYSTEM_EMAIL_HOST", self.read)
+        self.assertIn("SYSTEM_EMAIL_EXTERNAL", self.read)
+
+    def test_retired_names_do_not_count(self):
+        for name in ("CLOUDFLARE_API_TOKEN", "SYSTEM_EMAIL", "internet_interfaces"):
+            with self.subTest(name=name):
+                self.assertNotIn(name, self.read)
+
+
+if __name__ == "__main__":
+    unittest.main()