From bdd967d85bb9364c6904eee50bd4566e6c827948 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 15:38:50 +0200 Subject: [PATCH 01/12] Add kolla drift-detection framework OSISM assembles each container image's version from a chain of separate repos: openstack/kolla defines the images, container-images-kolla pins concrete versions, and container-image-kolla-ansible consumes them through its versions.yml.j2 template. Nothing checks that these stay in agreement, so a service can fall out of the chain silently and only surface at deploy or run time. Add a local-first, plugin-based drift detector to catch that automatically: - source: read a file or list a directory from any OSISM repo, transparently from a local checkout (--base-dir) or, with no checkout, from GitHub raw + API, so checks run offline or online unchanged. Reads pinned repos and release-range refs from a local git clone via git object reads; resolves a release to its upstream ref (treating 422 as an absent ref). - model: DriftEntry records one disagreement between an authoritative source and a consumer and carries allowlist suppression plus optional per-entry summary/remediation overrides. - plugin protocol: a check declares the files it reads and returns the drift it finds; the driver discovers checks from a registry. - driver (check-kolla-drift.py): selects enabled checks (or --plugin), runs them, applies the allowlist, renders a grouped, lifecycle-ordered text report (or JSON), flags stale allowlist entries as a hard error, and exits 0 (clean) / 1 (drift) / 2 (input error) for CI use. - config: remote defaults with per-repo source overrides, base_dirs for local reads, and releases/release_refs keys for range-aware checks. - allowlist: known-intentional exceptions, each requiring a reason. This lands the infrastructure only; the plugin registry is empty, so it flags nothing on its own. The kolla checks that use it follow in the next commits, each reviewable in isolation. Adds a kolla-drift-test tox env and the pytest/responses test deps. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Roger Luethi --- .flake8 | 2 +- .gitignore | 1 + requirements.txt | 2 + src/check-kolla-drift.py | 244 ++++++ src/kolla-drift-allowlist.yml | 3 + src/kolla-drift-config.yml | 17 + src/osism_drift/__init__.py | 0 src/osism_drift/config.py | 230 +++++ src/osism_drift/drift/__init__.py | 3 + src/osism_drift/drift/plugin.py | 19 + src/osism_drift/model.py | 44 + src/osism_drift/report.py | 84 ++ src/osism_drift/source.py | 354 ++++++++ tests/kolla_drift/__init__.py | 0 tests/kolla_drift/conftest.py | 7 + .../environments/kolla/secrets.yml.A | 5 + .../environments/kolla/secrets.yml.B | 4 + .../files/src/templates/versions.yml.j2 | 7 + .../src/tag-images-with-the-version.py | 5 + .../fixtures/defaults/all/050-images.yml | 3 + .../fixtures/defaults/all/099-kolla.yml | 7 + .../fixtures/generics/inventory/50-kolla | 5 + .../fixtures/generics/inventory/51-kolla | 2 + .../kolla-ansible/ansible/inventory/multinode | 14 + .../fixtures/kolla/docker/foo/.gitkeep | 0 .../kolla/docker/ignored-svc/.gitkeep | 0 .../fixtures/kolla/docker/macros.j2 | 1 + .../fixtures/kolla/docker/newsvc/.gitkeep | 0 .../fixtures/kolla/docker/off/.gitkeep | 0 .../fixtures/kolla/docker/present_a/.gitkeep | 0 .../fixtures/release/latest/openstack-A.yml | 4 + .../fixtures/release/latest/openstack-B.yml | 4 + tests/kolla_drift/test_config.py | 384 ++++++++ tests/kolla_drift/test_model.py | 36 + tests/kolla_drift/test_plugin_registry.py | 37 + tests/kolla_drift/test_report.py | 127 +++ tests/kolla_drift/test_source.py | 818 ++++++++++++++++++ tox.ini | 6 + 38 files changed, 2478 insertions(+), 1 deletion(-) create mode 100755 src/check-kolla-drift.py create mode 100644 src/kolla-drift-allowlist.yml create mode 100644 src/kolla-drift-config.yml create mode 100644 src/osism_drift/__init__.py create mode 100644 src/osism_drift/config.py create mode 100644 src/osism_drift/drift/__init__.py create mode 100644 src/osism_drift/drift/plugin.py create mode 100644 src/osism_drift/model.py create mode 100644 src/osism_drift/report.py create mode 100644 src/osism_drift/source.py create mode 100644 tests/kolla_drift/__init__.py create mode 100644 tests/kolla_drift/conftest.py create mode 100644 tests/kolla_drift/fixtures/cfg-cookiecutter/{{cookiecutter.project_name}}/environments/kolla/secrets.yml.A create mode 100644 tests/kolla_drift/fixtures/cfg-cookiecutter/{{cookiecutter.project_name}}/environments/kolla/secrets.yml.B create mode 100644 tests/kolla_drift/fixtures/container-image-kolla-ansible/files/src/templates/versions.yml.j2 create mode 100644 tests/kolla_drift/fixtures/container-images-kolla/src/tag-images-with-the-version.py create mode 100644 tests/kolla_drift/fixtures/defaults/all/050-images.yml create mode 100644 tests/kolla_drift/fixtures/defaults/all/099-kolla.yml create mode 100644 tests/kolla_drift/fixtures/generics/inventory/50-kolla create mode 100644 tests/kolla_drift/fixtures/generics/inventory/51-kolla create mode 100644 tests/kolla_drift/fixtures/kolla-ansible/ansible/inventory/multinode create mode 100644 tests/kolla_drift/fixtures/kolla/docker/foo/.gitkeep create mode 100644 tests/kolla_drift/fixtures/kolla/docker/ignored-svc/.gitkeep create mode 100644 tests/kolla_drift/fixtures/kolla/docker/macros.j2 create mode 100644 tests/kolla_drift/fixtures/kolla/docker/newsvc/.gitkeep create mode 100644 tests/kolla_drift/fixtures/kolla/docker/off/.gitkeep create mode 100644 tests/kolla_drift/fixtures/kolla/docker/present_a/.gitkeep create mode 100644 tests/kolla_drift/fixtures/release/latest/openstack-A.yml create mode 100644 tests/kolla_drift/fixtures/release/latest/openstack-B.yml create mode 100644 tests/kolla_drift/test_config.py create mode 100644 tests/kolla_drift/test_model.py create mode 100644 tests/kolla_drift/test_plugin_registry.py create mode 100644 tests/kolla_drift/test_report.py create mode 100644 tests/kolla_drift/test_source.py diff --git a/.flake8 b/.flake8 index 1d06d44..9480cbe 100644 --- a/.flake8 +++ b/.flake8 @@ -1,3 +1,3 @@ [flake8] max-line-length = 200 -ignore = W503 E722 +ignore = W503 E722 E203 diff --git a/.gitignore b/.gitignore index 45e300a..e8107a0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ release +!tests/kolla_drift/fixtures/release/ .venv venv diff --git a/requirements.txt b/requirements.txt index 926494c..391b936 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,6 +3,8 @@ PyYAML==6.0.3 ansible==11.13.0 packaging==26.2 pwgen==0.8.2.post0 +pytest==8.3.4 python-gilt==1.2.3 requests==2.34.2 +responses==0.25.3 tabulate==0.10.0 diff --git a/src/check-kolla-drift.py b/src/check-kolla-drift.py new file mode 100755 index 0000000..e3513ef --- /dev/null +++ b/src/check-kolla-drift.py @@ -0,0 +1,244 @@ +#!/usr/bin/env python3 +"""Drift detector for the OSISM kolla container-image toolchain. + +Catches divergence between the OSISM consumer repos (`defaults`, `release`, +`generics`, the container-image repos) and upstream `openstack/kolla` / +`openstack/kolla-ansible` *when it is introduced* — before it breaks a deploy or +a CI gate. + +Each check is a plugin (see `osism_drift.drift`) that compares one +authoritative source against one consumer and emits `DriftEntry` items. The +plugins run in the order a service travels from upstream definition to a +deployed container: + +- `kolla_enablement_orphan` — the OSISM enable flag still exists upstream +- `kolla_enablement_build` — the enabled service is actually built +- `kolla_version_chain_upstream` — the built image has a version-pin line +- `kolla_version_chain_inner` — that pin line resolves to a real version +- `kolla_inventory` — the service is present in the deploy inventory + +Sources resolve remotely (GitHub) by default. A repeatable `--base-dir DIR` +reads local checkouts instead, with pinned upstream repos read from git objects +at the release refs (fully offline). An allowlist marks known or +intentional drift as expected; allowlist entries that match nothing are +reported as stale. + +Findings print two ways: + +- text (default) — grouped, narrated blocks per check (`osism_drift.report`) +- `--format json` — one JSON object per entry, for machines + +Exit codes: + +- 0 — no actionable drift and no stale allowlist entries +- 1 — actionable drift or stale allowlist entries found +- 2 — input error (missing file, unparseable, bad config) + +Run with `-h` for the per-plugin reference and the inputs each one reads. +""" + +import argparse +import dataclasses +import json +import sys +from pathlib import Path + +from osism_drift.config import ( + Allowlist, + ConfigError, + load_allowlist, + load_config, + to_remote_only, +) +from osism_drift.drift import PLUGINS +from osism_drift import report, source +from osism_drift.source import SourceError + + +def _build_parser(): + p = argparse.ArgumentParser( + description="Check OSISM container image versions for drift across repos.", + formatter_class=argparse.RawDescriptionHelpFormatter, + epilog=_format_plugin_help() + "\n" + _exit_codes_help(), + ) + here = Path(__file__).resolve().parent + p.add_argument( + "--config", + default=here / "kolla-drift-config.yml", + help="config file (default: alongside script)", + ) + p.add_argument( + "--allowlist", + default=here / "kolla-drift-allowlist.yml", + help="allowlist (default: alongside script)", + ) + p.add_argument( + "--plugin", + action="append", + help="run only this plugin (repeatable); default: all enabled", + ) + p.add_argument("--format", choices=("text", "json"), default="text") + p.add_argument( + "--no-allowlist", + action="store_true", + help="ignore allowlist; report everything", + ) + p.add_argument( + "--base-dir", + action="append", + metavar="DIR", + help="local checkout root (repeatable); repos found by dir name, first match wins", + ) + p.add_argument( + "--remote-fallback", + action="store_true", + help="for repos not found under any --base-dir, fetch remotely instead of erroring", + ) + p.add_argument( + "--remote-only", + action="store_true", + help="ignore --base-dir; HTTP fetch everywhere", + ) + p.add_argument("-v", "--verbose", action="store_true") + p.add_argument("-q", "--quiet", action="store_true") + return p + + +def _format_plugin_help() -> str: + lines = ["Plugins:"] + for plugin in PLUGINS: + lines.append(f" {plugin.NAME}") + lines.append(f" {plugin.DESCRIPTION}") + lines.append(" Reads:") + col_w = max(len(repo) for repo, _ in plugin.INPUT_FILES) + 4 + for repo, rel in plugin.INPUT_FILES: + lines.append(f" {repo:<{col_w}}{rel}") + lines.append("") + return "\n".join(lines) + + +def _exit_codes_help() -> str: + return ( + "Exit codes:\n" + " 0 no actionable drift and no stale allowlist entries\n" + " 1 actionable drift or stale allowlist entries found\n" + " 2 input error (missing file, unparseable, bad config)" + ) + + +def _format_stale_text(stale) -> list[str]: + out = [] + if stale: + out.append("STALE ALLOWLIST (entries that matched no drift):") + for e in stale: + extra = [] + if e.alias is not None: + extra.append(f"alias={e.alias}") + if e.found_src is not None: + extra.append(f"found_src={e.found_src}") + suffix = (" " + " ".join(extra)) if extra else "" + out.append(f" {e.plugin}: {e.image}{suffix} -- {e.reason}") + out.append("") + return out + + +def _load_runtime(args): + """Build the resolved config and allowlist from parsed CLI args. + + Raises ConfigError on invalid config or allowlist input. + """ + config = load_config(args.config) + config = dataclasses.replace( + config, + base_dirs=tuple(args.base_dir or ()), + remote_fallback=args.remote_fallback, + ) + if args.remote_only: + config = to_remote_only(config) + allowlist = ( + Allowlist(entries=()) if args.no_allowlist else load_allowlist(args.allowlist) + ) + return config, allowlist + + +def _emit(args, drifts, actionable, allowlisted, stale): + """Print the findings and stale-allowlist report in the chosen format.""" + if args.format == "json": + for d in drifts if args.verbose else actionable: + print(json.dumps(d.to_dict())) + for e in stale: + print( + json.dumps( + { + "type": "stale_allowlist", + "plugin": e.plugin, + "image": e.image, + "alias": e.alias, + "found_src": e.found_src, + "reason": e.reason, + } + ) + ) + return + for line in report.format_text(drifts, PLUGINS): + print(line) + for line in _format_stale_text(stale): + print(line) + if not args.quiet: + plural = "entry" if len(stale) == 1 else "entries" + print( + f"Summary: {len(actionable)} to act on, {len(allowlisted)} " + f"allowlisted, {len(stale)} stale allowlist {plural} " + f"({len(drifts)} total)" + ) + + +def main(argv=None) -> int: + """CLI entry point: run the configured plugins and report drift.""" + args = _build_parser().parse_args(argv) + try: + config, allowlist = _load_runtime(args) + except ConfigError as e: + print(f"config error: {e}", file=sys.stderr) + return 2 + + selected = [ + p + for p in PLUGINS + if (args.plugin is None or p.NAME in args.plugin) + and config.plugins.get(p.NAME) is not None + and config.plugins[p.NAME].enabled + ] + + repos = {repo for p in selected for repo, _ in p.INPUT_FILES} + try: + resolution = source.describe_resolution(repos, config) + except SourceError as e: + print(f"source error: {e}", file=sys.stderr) + return 2 + if not args.quiet: + print( + f"Resolving sources ({len(config.base_dirs)} base dir(s)):", file=sys.stderr + ) + for line in resolution: + print(line, file=sys.stderr) + + drifts = [] + try: + for plugin in selected: + drifts.extend(plugin.run(config, allowlist, verbose=args.verbose)) + except SourceError as e: + print(f"source error: {e}", file=sys.stderr) + return 2 + + actionable = [d for d in drifts if not d.allowlisted] + allowlisted = [d for d in drifts if d.allowlisted] + ran = {p.NAME for p in selected} + stale = allowlist.stale(drifts, ran) + + _emit(args, drifts, actionable, allowlisted, stale) + return 1 if (actionable or stale) else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/kolla-drift-allowlist.yml b/src/kolla-drift-allowlist.yml new file mode 100644 index 0000000..91b742f --- /dev/null +++ b/src/kolla-drift-allowlist.yml @@ -0,0 +1,3 @@ +# Known-intentional exceptions. Each entry needs a non-empty `reason`. +# An entry that suppresses no real drift is reported as stale (a hard error). +allow: [] diff --git a/src/kolla-drift-config.yml b/src/kolla-drift-config.yml new file mode 100644 index 0000000..b1d949d --- /dev/null +++ b/src/kolla-drift-config.yml @@ -0,0 +1,17 @@ +# Default config for the drift detector. +# Override with `--config PATH` if needed. Reads remote (GitHub) by default; +# pass `--base-dir DIR` (repeatable) to read OSISM repos from local checkouts. + +remote: + github_raw: https://raw.githubusercontent.com/ + github_api: https://api.github.com/repos/ + default_owner: osism + branch: main + +sources: + kolla: {owner: openstack, branch: stable/2025.2} + kolla_ansible: {owner: openstack, branch: stable/2025.2} + +release_version: latest + +plugins: {} diff --git a/src/osism_drift/__init__.py b/src/osism_drift/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/osism_drift/config.py b/src/osism_drift/config.py new file mode 100644 index 0000000..a053df7 --- /dev/null +++ b/src/osism_drift/config.py @@ -0,0 +1,230 @@ +"""Config + allowlist loaders with schema validation.""" + +from dataclasses import dataclass, field, replace +from pathlib import Path +from typing import Optional + +import yaml + + +class ConfigError(Exception): + """Raised on invalid config or allowlist content.""" + + +_ALLOWED_TOP_KEYS = { + "remote", + "release_version", + "plugins", + "sources", + "releases", + "release_refs", +} +_ALLOWED_REMOTE_KEYS = {"github_raw", "github_api", "branch", "default_owner"} +_ALLOWED_SOURCE_KEYS = {"owner", "branch"} + + +@dataclass(frozen=True) +class Remote: + """GitHub raw/API base URLs and the default owner for remote reads.""" + + github_raw: str + github_api: str + branch: str + default_owner: str = "osism" + + +@dataclass(frozen=True) +class SourceCfg: + """Per-repo owner/ref override. A set `branch` pins the repo (read remotely).""" + + owner: Optional[str] = None + branch: Optional[str] = None + + +@dataclass(frozen=True) +class PluginCfg: + """Per-plugin configuration (currently only the enabled flag).""" + + enabled: bool + + +@dataclass(frozen=True) +class Config: # pylint: disable=too-many-instance-attributes # data record + """Resolved drift-detector configuration (file plus CLI overrides).""" + + remote: Remote + release_version: str + plugins: dict # name -> PluginCfg + sources: dict = field(default_factory=dict) # repo -> SourceCfg + releases: tuple = () # explicit supported-release override; () -> derive + release_refs: dict = field(default_factory=dict) # {repo: {release: ref}} override + base_dirs: tuple = () # local checkout roots (CLI --base-dir); () -> all remote + remote_fallback: bool = False # CLI --remote-fallback: not-found-local -> remote + + +def load_config(path) -> Config: + """Parse and schema-validate a config YAML file into a Config.""" + raw = yaml.safe_load(Path(path).read_text(encoding="utf-8")) + extra = set(raw) - _ALLOWED_TOP_KEYS + if extra: + raise ConfigError(f"unknown top-level keys: {sorted(extra)}") + + remote_raw = raw["remote"] + extra_r = set(remote_raw) - _ALLOWED_REMOTE_KEYS + if extra_r: + raise ConfigError(f"unknown remote keys: {sorted(extra_r)}") + remote = Remote(**remote_raw) + + sources = {} + for name, spec in (raw.get("sources") or {}).items(): + spec = spec or {} + extra_s = set(spec) - _ALLOWED_SOURCE_KEYS + if extra_s: + raise ConfigError(f"source {name!r}: unknown keys {sorted(extra_s)}") + sources[name] = SourceCfg(owner=spec.get("owner"), branch=spec.get("branch")) + + plugin_raw = raw.get("plugins", {}) + plugins = { + name: PluginCfg(enabled=bool(spec.get("enabled", True))) + for name, spec in plugin_raw.items() + } + + releases = tuple(str(r) for r in (raw.get("releases") or [])) + release_refs = { + repo: {str(rel): str(ref) for rel, ref in (mapping or {}).items()} + for repo, mapping in (raw.get("release_refs") or {}).items() + } + + return Config( + remote=remote, + release_version=raw["release_version"], + plugins=plugins, + sources=sources, + releases=releases, + release_refs=release_refs, + ) + + +@dataclass(frozen=True) +class AllowEntry: + """One allowlist rule that marks matching drift entries as allowlisted.""" + + plugin: str + image: str + reason: str + alias: Optional[str] = None + found_src: Optional[str] = None + match: str = "exact" + + def _image_matches(self, image: str) -> bool: + # Boundary-aware prefix: a prefix covers a service and its sub-groups + # (separators - and :) without swallowing an adjacent name (cyborgx). + if self.match == "prefix": + return ( + image == self.image + or image.startswith(self.image + "-") + or image.startswith(self.image + ":") + ) + return image == self.image + + def matches(self, drift) -> bool: + """True if this entry covers `drift` (plugin, image, alias, source).""" + if drift.plugin != self.plugin or not self._image_matches(drift.image): + return False + if self.alias is not None and drift.alias != self.alias: + return False + if self.found_src is not None and drift.found_src != self.found_src: + return False + return True + + +@dataclass(frozen=True) +class Allowlist: + """An ordered collection of AllowEntry allowlist rules.""" + + entries: tuple + + def match(self, drift): + """Return the first entry matching `drift`, or None.""" + for e in self.entries: + if e.matches(drift): + return e + return None + + def apply(self, drift): + """Return `drift`, marked allowlisted if an entry matches it. + + Folds the match-then-mark idiom every plugin repeats per finding into + one call: unmatched drifts are returned unchanged, matched ones as an + allowlisted copy carrying the match reason. + """ + match = self.match(drift) + if match is not None: + return drift.as_allowlisted(match.reason) + return drift + + def stale(self, drifts, plugins): + """Entries whose plugin ran but which matched none of the drifts. + + A stale entry matches no real drift and is a hard error. Scoped to + `plugins` (the names of the plugins that actually ran) so a filtered + run does not flag entries belonging to plugins that did not run. + """ + return [ + e + for e in self.entries + if e.plugin in plugins and not any(e.matches(d) for d in drifts) + ] + + +def to_remote_only(config: Config) -> Config: + """Config for --remote-only: clear base_dirs so every read goes remote. + + Preserves all other fields (sources/releases/release_refs/remote_fallback) + via dataclasses.replace. + """ + return replace(config, base_dirs=()) + + +_ALLOWED_ALLOW_KEYS = {"plugin", "image", "reason", "alias", "found_src", "match"} + + +def load_allowlist(path) -> Allowlist: + """Parse and validate an allowlist YAML file into an Allowlist. + + A missing file yields an empty allowlist. + """ + p = Path(path) + if not p.exists(): + return Allowlist(entries=()) + raw = yaml.safe_load(p.read_text(encoding="utf-8")) or {} + out = [] + for i, item in enumerate(raw.get("allow", [])): + extra = set(item) - _ALLOWED_ALLOW_KEYS + if extra: + raise ConfigError(f"allowlist entry {i}: unknown keys {sorted(extra)}") + for required in ("plugin", "image", "reason"): + if required not in item: + raise ConfigError( + f"allowlist entry {i}: missing required field {required!r}" + ) + if not item["image"]: + raise ConfigError(f"allowlist entry {i}: image must be non-empty") + if not item["reason"]: + raise ConfigError(f"allowlist entry {i}: reason must be non-empty") + match = item.get("match", "exact") + if match not in ("exact", "prefix"): + raise ConfigError( + f"allowlist entry {i}: match must be 'exact' or 'prefix', got {match!r}" + ) + out.append( + AllowEntry( + plugin=item["plugin"], + image=item["image"], + reason=item["reason"], + alias=item.get("alias"), + found_src=item.get("found_src"), + match=match, + ) + ) + return Allowlist(entries=tuple(out)) diff --git a/src/osism_drift/drift/__init__.py b/src/osism_drift/drift/__init__.py new file mode 100644 index 0000000..ac2a349 --- /dev/null +++ b/src/osism_drift/drift/__init__.py @@ -0,0 +1,3 @@ +"""Plugin registry. Plugins are appended here as they are added.""" + +PLUGINS = [] diff --git a/src/osism_drift/drift/plugin.py b/src/osism_drift/drift/plugin.py new file mode 100644 index 0000000..f9ba9f3 --- /dev/null +++ b/src/osism_drift/drift/plugin.py @@ -0,0 +1,19 @@ +"""Plugin protocol for drift checks.""" + +from typing import Protocol + + +class Plugin(Protocol): # pylint: disable=too-few-public-methods # interface + """Structural type a drift-check plugin module must satisfy.""" + + NAME: str + DESCRIPTION: str + INPUT_FILES: list # list of (repo, rel_path) tuples + + @staticmethod + def run(config, allowlist, verbose: bool = False) -> list: + """Return list[DriftEntry] for all drift this plugin finds. + + verbose=True authorizes plugins to emit advisory messages to + stderr (e.g. unresolved Jinja warnings). + """ diff --git a/src/osism_drift/model.py b/src/osism_drift/model.py new file mode 100644 index 0000000..6a5e4b6 --- /dev/null +++ b/src/osism_drift/model.py @@ -0,0 +1,44 @@ +"""Drift detector data model.""" + +from dataclasses import asdict, dataclass +from typing import Optional + + +@dataclass(frozen=True) +class DriftEntry: # pylint: disable=too-many-instance-attributes # data record + """One drift between an authoritative source and a consumer.""" + + plugin: str + image: str # release key, after alias resolution + alias: str # _tag prefix as found in the source + expected: str # value from the authoritative source + found: str # value from the consumer + expected_src: str # human-readable path of the authoritative source + found_src: str # human-readable path of the consumer + allowlisted: bool = False + reason: Optional[str] = None + # Optional per-entry overrides of the plugin's SUMMARY/REMEDIATION, for + # plugins whose findings split into distinct actions (e.g. add vs remove). + # When None the report falls back to the plugin-level constants. + summary: Optional[str] = None + remediation: Optional[str] = None + + def as_allowlisted(self, reason: str) -> "DriftEntry": + """Return a copy marked allowlisted, carrying the match reason.""" + return DriftEntry( + plugin=self.plugin, + image=self.image, + alias=self.alias, + expected=self.expected, + found=self.found, + expected_src=self.expected_src, + found_src=self.found_src, + allowlisted=True, + reason=reason, + summary=self.summary, + remediation=self.remediation, + ) + + def to_dict(self) -> dict: + """Return the entry's fields as a plain, JSON-serialisable dict.""" + return asdict(self) diff --git a/src/osism_drift/report.py b/src/osism_drift/report.py new file mode 100644 index 0000000..cadf636 --- /dev/null +++ b/src/osism_drift/report.py @@ -0,0 +1,84 @@ +"""Grouped, narrated text rendering of drift findings. + +Pure presentation, no I/O: groups actionable (non-allowlisted) DriftEntry items +by (plugin, expected_src, found_src) and renders each group as a lead sentence +(plugin.SUMMARY), the sorted name list, a Fix line (plugin.REMEDIATION), and +the two source paths (Refs). Block order follows the given `plugins` list. The +orientation header precedes the blocks. Returns [] when there are no actionable +drifts. The caller owns the stale-allowlist block and the summary. +""" + +import textwrap + +HEADER = "Checks follow a service's path: enabled → built → version-pinned → deployed." + +_WIDTH = 76 +_NAME_INDENT = " " + + +def format_text(drifts, plugins): + """Render actionable (non-allowlisted) drifts as grouped, narrated lines.""" + actionable = [d for d in drifts if not d.allowlisted] + if not actionable: + return [] + + by_name = {p.NAME: p for p in plugins} + order = {p.NAME: i for i, p in enumerate(plugins)} + + groups = {} + for d in actionable: + groups.setdefault((d.plugin, d.expected_src, d.found_src), []).append(d) + + out = [HEADER, ""] + for key in sorted(groups, key=lambda k: (order.get(k[0], len(order)), k[1], k[2])): + plugin_name, expected_src, found_src = key + out.extend( + _format_group(by_name[plugin_name], expected_src, found_src, groups[key]) + ) + return out + + +def _format_group(plugin, expected_src, found_src, entries): + """Render one (plugin, expected_src, found_src) group as text lines.""" + names = sorted(d.image for d in entries) + + # A group shares one (expected_src, found_src); entries in it carry the + # same summary/remediation. Take the first's override, else the plugin's. + first = entries[0] + summary = first.summary if first.summary is not None else plugin.SUMMARY + remediation = ( + first.remediation if first.remediation is not None else plugin.REMEDIATION + ) + + lead = f"{plugin.NAME} — {summary.format(n=len(names))}" + lines = list( + textwrap.wrap( + lead, width=_WIDTH, break_long_words=False, break_on_hyphens=False + ) + ) + lines.append("") + lines.extend( + textwrap.wrap( + ", ".join(names), + width=_WIDTH, + initial_indent=_NAME_INDENT, + subsequent_indent=_NAME_INDENT, + break_long_words=False, + break_on_hyphens=False, + ) + ) + lines.append("") + lines.extend( + textwrap.wrap( + f"Fix: {remediation}", + width=_WIDTH, + initial_indent=" ", + subsequent_indent=" ", + break_long_words=False, + break_on_hyphens=False, + ) + ) + lines.append(f" Refs: {expected_src}") + lines.append(f" {found_src}") + lines.append("") + return lines diff --git a/src/osism_drift/source.py b/src/osism_drift/source.py new file mode 100644 index 0000000..376e8d3 --- /dev/null +++ b/src/osism_drift/source.py @@ -0,0 +1,354 @@ +"""Local-or-remote source reads for OSISM repos. + +A repo may carry a per-repo override in config.sources (owner and/or branch). +A set `branch` *pins* the repo: it is always read remotely at that ref, so the +result is deterministic regardless of any local checkout's current branch. +""" + +import subprocess +from pathlib import Path + +import requests + + +class SourceError(Exception): + """Raised on any read/list failure that should abort the run.""" + + +def _source(repo: str, config): + return config.sources.get(repo) + + +def _owner(repo: str, config) -> str: + s = _source(repo, config) + if s is not None and s.owner: + return s.owner + return config.remote.default_owner + + +def _ref(repo: str, config) -> str: + s = _source(repo, config) + if s is not None and s.branch: + return s.branch + return config.remote.branch + + +def _is_pinned(repo: str, config) -> bool: + s = _source(repo, config) + return s is not None and s.branch is not None + + +def _local_repo_dir(repo: str, config) -> Path | None: + """First --base-dir (in order) that contains (hyphenated name).""" + name = repo.replace("_", "-") + for base in config.base_dirs: + cand = Path(base).expanduser() / name + if cand.is_dir(): + return cand + return None + + +def _resolve(repo: str, config): + """('local', dir) | ('remote', None); raise SourceError on mode-B not-found. + + A pinned (upstream) repo resolves local only if its discovered dir is a git + checkout — it is read at named refs via git objects, so a non-git dir cannot + serve it (it falls to --remote-fallback / mode B). Unpinned (consumer) repos + resolve local from any discovered dir (read as the working tree). + """ + if not config.base_dirs: + return ("remote", None) + d = _local_repo_dir(repo, config) + usable = d is not None and (not _is_pinned(repo, config) or (d / ".git").exists()) + if usable: + return ("local", d) + if config.remote_fallback: + return ("remote", None) + raise SourceError( + f"repo {repo!r} not found under any --base-dir " + f"({', '.join(config.base_dirs)}); pass --remote-fallback to fetch it remotely" + ) + + +def _git(d, *args): + return subprocess.run( + ["git", "-C", str(d), *args], capture_output=True, check=False + ) + + +def _resolve_local_ref(d, ref): + """Clone-local name that resolves `ref` to a commit, or None. Tries the ref + as-given, then / for EVERY configured remote, so a ref held only + under a non-origin remote (e.g. gerrit/unmaintained/2024.1) still resolves.""" + cands = [ref] + cands += [f"{r}/{ref}" for r in _git(d, "remote").stdout.decode().split()] + for cand in cands: + if ( + _git(d, "rev-parse", "--verify", "--quiet", f"{cand}^{{commit}}").returncode + == 0 + ): + return cand + return None + + +def _git_show(d, ref, rel_path, optional=False): + rref = _resolve_local_ref(d, ref) + if rref is None: + raise SourceError( + f"ref {ref!r} not found in {d} — fetch it " + f"(this repo is read at named refs via git)" + ) + r = _git(d, "show", f"{rref}:{rel_path}") + if r.returncode != 0: + if optional: + return None + raise SourceError(f"{rel_path} absent at {ref} in {d}") + return r.stdout + + +def _git_ls_tree(d, ref, rel_path, dirs_only=False): + rref = _resolve_local_ref(d, ref) + if rref is None: + raise SourceError( + f"ref {ref!r} not found in {d} — fetch it " + f"(this repo is read at named refs via git)" + ) + # Colon (subtree) form lists DIRECT CHILDREN by BASENAME (not full paths). + r = _git(d, "ls-tree", f"{rref}:{rel_path}") + if r.returncode != 0: + raise SourceError(f"cannot list {rel_path} at {ref} in {d}") + out = [] + for line in r.stdout.decode().splitlines(): + meta, _, name = line.partition("\t") # " \t" + if not dirs_only or meta.split()[1] == "tree": + out.append(name) + return out + + +def _git_ref_exists(d, ref): + return _resolve_local_ref(d, ref) is not None + + +def _remote_url(repo: str, rel_path: str, config) -> str: + owner = _owner(repo, config) + return ( + f"{config.remote.github_raw}{owner}/{repo.replace('_', '-')}/" + f"{_ref(repo, config)}/{rel_path}" + ) + + +def read(repo: str, rel_path: str, config) -> bytes: + """Read `rel_path` from `repo`; raise SourceError if it is absent.""" + where, d = _resolve(repo, config) + if where == "local": + if _is_pinned(repo, config): + return _git_show(d, _ref(repo, config), rel_path) + p = d / rel_path + if not p.exists(): + raise SourceError(f"{rel_path} not found in local {repo} ({d})") + return p.read_bytes() + url = _remote_url(repo, rel_path, config) + try: + r = requests.get(url, timeout=30) + except requests.RequestException as e: + raise SourceError(f"network error fetching {url}: {e}") from e + if r.status_code == 404: + raise SourceError(f"404 not found: {url}") + if not r.ok: + raise SourceError(f"HTTP {r.status_code} fetching {url}") + return r.content + + +def read_optional(repo: str, rel_path: str, config) -> bytes | None: + """Like read(), but return None instead of raising when absent.""" + where, d = _resolve(repo, config) + if where == "local": + if _is_pinned(repo, config): + return _git_show(d, _ref(repo, config), rel_path, optional=True) + p = d / rel_path + return p.read_bytes() if p.exists() else None + url = _remote_url(repo, rel_path, config) + try: + r = requests.get(url, timeout=30) + except requests.RequestException as e: + raise SourceError(f"network error fetching {url}: {e}") from e + if r.status_code == 404: + return None + if not r.ok: + raise SourceError(f"HTTP {r.status_code} fetching {url}") + return r.content + + +def list_dir(repo: str, rel_path: str, config, dirs_only: bool = False) -> list[str]: + """List entries under `rel_path` in `repo` (directories only if `dirs_only`).""" + where, d = _resolve(repo, config) + if where == "local": + if _is_pinned(repo, config): + return _git_ls_tree(d, _ref(repo, config), rel_path, dirs_only) + p = d / rel_path + if not p.is_dir(): + raise SourceError(f"{rel_path} not a directory in local {repo} ({d})") + return [x.name for x in p.iterdir() if (not dirs_only or x.is_dir())] + owner = _owner(repo, config) + url = ( + f"{config.remote.github_api}{owner}/{repo.replace('_', '-')}/" + f"contents/{rel_path}?ref={_ref(repo, config)}" + ) + try: + r = requests.get( + url, timeout=30, headers={"Accept": "application/vnd.github.v3+json"} + ) + except requests.RequestException as e: + raise SourceError(f"network error listing {url}: {e}") from e + if r.status_code == 404: + raise SourceError(f"404 not found: {url}") + if not r.ok: + raise SourceError(f"HTTP {r.status_code} listing {url}") + items = r.json() + if dirs_only: + items = [it for it in items if it.get("type") == "dir"] + return [item["name"] for item in items] + + +def list_dir_at_ref( + repo: str, rel_path: str, ref: str, config, dirs_only: bool = False +) -> list[str]: + """List a repo directory at an explicit git ref. + + Local (the repo resolves under a --base-dir): list the git tree at `ref` + (objects, never the working tree). Remote: the GitHub contents API at `ref`. + Either way the explicit ref is read, ignoring any per-repo pin, so a range + check is deterministic. + """ + where, d = _resolve(repo, config) + if where == "local" and _is_pinned(repo, config): + return _git_ls_tree(d, ref, rel_path, dirs_only) + owner = _owner(repo, config) + url = ( + f"{config.remote.github_api}{owner}/{repo.replace('_', '-')}/" + f"contents/{rel_path}?ref={ref}" + ) + try: + r = requests.get( + url, timeout=30, headers={"Accept": "application/vnd.github.v3+json"} + ) + except requests.RequestException as e: + raise SourceError(f"network error listing {url}: {e}") from e + if r.status_code == 404: + raise SourceError(f"404 not found: {url}") + if not r.ok: + raise SourceError(f"HTTP {r.status_code} listing {url}") + items = r.json() + if dirs_only: + items = [it for it in items if it.get("type") == "dir"] + return [item["name"] for item in items] + + +def ref_exists(repo: str, ref: str, config) -> bool: + """True if `ref` (branch/tag/sha) resolves in the upstream repo (local clone + when it resolves under a --base-dir, else the GitHub commits API).""" + where, d = _resolve(repo, config) + if where == "local" and _is_pinned(repo, config): + return _git_ref_exists(d, ref) + owner = _owner(repo, config) + url = f"{config.remote.github_api}{owner}/{repo.replace('_', '-')}/commits/{ref}" + try: + r = requests.get( + url, timeout=30, headers={"Accept": "application/vnd.github.v3+json"} + ) + except requests.RequestException as e: + raise SourceError(f"network error checking ref {url}: {e}") from e + # GitHub's commits API returns 422 (not 404) for a ref that does not + # resolve; treat both as "absent" so the resolver probes the next candidate. + if r.status_code in (404, 422): + return False + if not r.ok: + raise SourceError(f"HTTP {r.status_code} checking ref {url}") + return True + + +_REF_CANDIDATES = ("stable/{r}", "unmaintained/{r}", "{r}-eol", "{r}-eom") + + +def release_to_ref(repo: str, release: str, config) -> str: + """Resolve an OSISM release (e.g. '2024.2') to an existing upstream ref. + + OSISM builds releases upstream has moved past EOL, so ref naming is + non-uniform: a release_refs override wins, else probe stable/ -> + unmaintained/ -> -eol -> -eom and take the first that exists. None + exists -> SourceError (loud, never a silent 404 mid-listing). Each + (repo, release) is resolved once per run, so no caching is needed. + """ + override = (config.release_refs.get(repo) or {}).get(release) + if override: + return override + for tmpl in _REF_CANDIDATES: + cand = tmpl.format(r=release) + if ref_exists(repo, cand, config): + return cand + tried = ", ".join(t.format(r=release) for t in _REF_CANDIDATES) + raise SourceError( + f"no upstream ref for {repo} release {release}: tried {tried} " + f"(set release_refs to override)" + ) + + +def read_at_ref( + repo: str, rel_path: str, ref: str, config, optional: bool = False +) -> bytes | None: + """Read a repo file at an explicit git ref. Always remote. + + Local (the repo resolves under a --base-dir): read the git object at `ref`. + Remote: github_raw at `ref`. The explicit ref is read either way, ignoring + any per-repo pin. optional=True maps an absent path (local) or a 404 (remote) + to None so the caller can probe an alternative (e.g. monolithic all.yml -> + split all/ dir). + """ + where, d = _resolve(repo, config) + if where == "local" and _is_pinned(repo, config): + return _git_show(d, ref, rel_path, optional=optional) + owner = _owner(repo, config) + url = ( + f"{config.remote.github_raw}{owner}/{repo.replace('_', '-')}/" + f"{ref}/{rel_path}" + ) + try: + r = requests.get(url, timeout=30) + except requests.RequestException as e: + raise SourceError(f"network error fetching {url}: {e}") from e + if r.status_code == 404: + if optional: + return None + raise SourceError(f"404 not found: {url}") + if not r.ok: + raise SourceError(f"HTTP {r.status_code} fetching {url}") + return r.content + + +def describe_resolution(repos, config) -> list[str]: + """One human log line per repo (sorted). Raises SourceError on a mode-B + not-found, so the driver can abort before any comparison runs.""" + lines = [] + for repo in sorted(repos): + where, d = _resolve(repo, config) + if where == "local" and _is_pinned(repo, config): + lines.append( + f" {repo:<32} local {d} @ {_ref(repo, config)} " + f"(+per-release range refs) [git refs, must be current]" + ) + elif where == "local": + lines.append(f" {repo:<32} local {d} [working tree, as-is]") + elif _is_pinned(repo, config): + owner = _owner(repo, config) + lines.append( + f" {repo:<32} remote {owner}/{repo.replace('_', '-')} " + f"@ {_ref(repo, config)} (+per-release range refs) [remote]" + ) + else: + owner = _owner(repo, config) + tail = ", not found locally" if config.base_dirs else "" + lines.append( + f" {repo:<32} remote {owner}/{repo.replace('_', '-')} " + f"@ {config.remote.branch} [remote{tail}]" + ) + return lines diff --git a/tests/kolla_drift/__init__.py b/tests/kolla_drift/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/kolla_drift/conftest.py b/tests/kolla_drift/conftest.py new file mode 100644 index 0000000..576c136 --- /dev/null +++ b/tests/kolla_drift/conftest.py @@ -0,0 +1,7 @@ +"""Shared test config: makes src/ importable as `osism_drift`.""" + +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(ROOT / "src")) diff --git a/tests/kolla_drift/fixtures/cfg-cookiecutter/{{cookiecutter.project_name}}/environments/kolla/secrets.yml.A b/tests/kolla_drift/fixtures/cfg-cookiecutter/{{cookiecutter.project_name}}/environments/kolla/secrets.yml.A new file mode 100644 index 0000000..960eac5 --- /dev/null +++ b/tests/kolla_drift/fixtures/cfg-cookiecutter/{{cookiecutter.project_name}}/environments/kolla/secrets.yml.A @@ -0,0 +1,5 @@ +--- +# kolla secrets for release A +keystone_password: +foo_password: "x" +orphan_a_password: diff --git a/tests/kolla_drift/fixtures/cfg-cookiecutter/{{cookiecutter.project_name}}/environments/kolla/secrets.yml.B b/tests/kolla_drift/fixtures/cfg-cookiecutter/{{cookiecutter.project_name}}/environments/kolla/secrets.yml.B new file mode 100644 index 0000000..a51227b --- /dev/null +++ b/tests/kolla_drift/fixtures/cfg-cookiecutter/{{cookiecutter.project_name}}/environments/kolla/secrets.yml.B @@ -0,0 +1,4 @@ +--- +# kolla secrets for release B +keystone_password: +orphan_b_password: diff --git a/tests/kolla_drift/fixtures/container-image-kolla-ansible/files/src/templates/versions.yml.j2 b/tests/kolla_drift/fixtures/container-image-kolla-ansible/files/src/templates/versions.yml.j2 new file mode 100644 index 0000000..8023b71 --- /dev/null +++ b/tests/kolla_drift/fixtures/container-image-kolla-ansible/files/src/templates/versions.yml.j2 @@ -0,0 +1,7 @@ +openstack_version: "2025.2" +kolla_present_a_version: "{{ versions['present_a']|default(openstack_version) }}" +kolla_present_b_version: "{{ versions['present_b']|default(openstack_version) }}" +kolla_common_version: "{{ versions['kolla_toolbox']|default(openstack_version) }}" +kolla_inert_x_version: "{{ versions['inert_x']|default(openstack_version) }}" +kolla_foo_version: "{{ versions['foo']|default(openstack_version) }}" +kolla_off_version: "{{ versions['off']|default(openstack_version) }}" diff --git a/tests/kolla_drift/fixtures/container-images-kolla/src/tag-images-with-the-version.py b/tests/kolla_drift/fixtures/container-images-kolla/src/tag-images-with-the-version.py new file mode 100644 index 0000000..ca40cff --- /dev/null +++ b/tests/kolla_drift/fixtures/container-images-kolla/src/tag-images-with-the-version.py @@ -0,0 +1,5 @@ +SBOM_IMAGE_TO_VERSION = { + "present_a": "present-a-image", + "present_b": "present-b-image", + "kolla-toolbox": "kolla-toolbox", +} diff --git a/tests/kolla_drift/fixtures/defaults/all/050-images.yml b/tests/kolla_drift/fixtures/defaults/all/050-images.yml new file mode 100644 index 0000000..9a4b1e1 --- /dev/null +++ b/tests/kolla_drift/fixtures/defaults/all/050-images.yml @@ -0,0 +1,3 @@ +--- +feat_api_image: "url/feat-api" +live_image: "url/live" diff --git a/tests/kolla_drift/fixtures/defaults/all/099-kolla.yml b/tests/kolla_drift/fixtures/defaults/all/099-kolla.yml new file mode 100644 index 0000000..cd51b43 --- /dev/null +++ b/tests/kolla_drift/fixtures/defaults/all/099-kolla.yml @@ -0,0 +1,7 @@ +enable_foo: "yes" +enable_bar: "yes" +enable_feat: "yes" +enable_off: "no" +enable_multi_word: "yes" +feat_internal_fqdn: "internal" +feat_api_port: "9999" diff --git a/tests/kolla_drift/fixtures/generics/inventory/50-kolla b/tests/kolla_drift/fixtures/generics/inventory/50-kolla new file mode 100644 index 0000000..c67c3fb --- /dev/null +++ b/tests/kolla_drift/fixtures/generics/inventory/50-kolla @@ -0,0 +1,5 @@ +[control] +ctl01 + +[nova:children] +control diff --git a/tests/kolla_drift/fixtures/generics/inventory/51-kolla b/tests/kolla_drift/fixtures/generics/inventory/51-kolla new file mode 100644 index 0000000..451def1 --- /dev/null +++ b/tests/kolla_drift/fixtures/generics/inventory/51-kolla @@ -0,0 +1,2 @@ +[compute] +cmp01 diff --git a/tests/kolla_drift/fixtures/kolla-ansible/ansible/inventory/multinode b/tests/kolla_drift/fixtures/kolla-ansible/ansible/inventory/multinode new file mode 100644 index 0000000..608ff93 --- /dev/null +++ b/tests/kolla_drift/fixtures/kolla-ansible/ansible/inventory/multinode @@ -0,0 +1,14 @@ +[control] +ctl01 + +[nova:children] +control + +[cyborg:children] +cyborg-agent + +[cyborg-agent:children] +compute + +[ironic-dnsmasq:children] +control diff --git a/tests/kolla_drift/fixtures/kolla/docker/foo/.gitkeep b/tests/kolla_drift/fixtures/kolla/docker/foo/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/tests/kolla_drift/fixtures/kolla/docker/ignored-svc/.gitkeep b/tests/kolla_drift/fixtures/kolla/docker/ignored-svc/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/tests/kolla_drift/fixtures/kolla/docker/macros.j2 b/tests/kolla_drift/fixtures/kolla/docker/macros.j2 new file mode 100644 index 0000000..c6c62eb --- /dev/null +++ b/tests/kolla_drift/fixtures/kolla/docker/macros.j2 @@ -0,0 +1 @@ +{# jinja macros #} diff --git a/tests/kolla_drift/fixtures/kolla/docker/newsvc/.gitkeep b/tests/kolla_drift/fixtures/kolla/docker/newsvc/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/tests/kolla_drift/fixtures/kolla/docker/off/.gitkeep b/tests/kolla_drift/fixtures/kolla/docker/off/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/tests/kolla_drift/fixtures/kolla/docker/present_a/.gitkeep b/tests/kolla_drift/fixtures/kolla/docker/present_a/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/tests/kolla_drift/fixtures/release/latest/openstack-A.yml b/tests/kolla_drift/fixtures/release/latest/openstack-A.yml new file mode 100644 index 0000000..2125594 --- /dev/null +++ b/tests/kolla_drift/fixtures/release/latest/openstack-A.yml @@ -0,0 +1,4 @@ +infrastructure_projects: + bar: +openstack_projects: + multi-word: stable-A diff --git a/tests/kolla_drift/fixtures/release/latest/openstack-B.yml b/tests/kolla_drift/fixtures/release/latest/openstack-B.yml new file mode 100644 index 0000000..2844ec7 --- /dev/null +++ b/tests/kolla_drift/fixtures/release/latest/openstack-B.yml @@ -0,0 +1,4 @@ +openstack_projects: + foo: stable-B + bar: stable-B + multi-word: stable-B diff --git a/tests/kolla_drift/test_config.py b/tests/kolla_drift/test_config.py new file mode 100644 index 0000000..25d5273 --- /dev/null +++ b/tests/kolla_drift/test_config.py @@ -0,0 +1,384 @@ +import pytest +from osism_drift.config import load_config, ConfigError, load_allowlist, Allowlist +from osism_drift.model import DriftEntry + + +def test_load_minimal_config(tmp_path): + cfg_path = tmp_path / "c.yml" + cfg_path.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/osism/ + github_api: https://api.github.com/repos/osism/ + branch: main +release_version: latest +plugins: + release_vs_manager: {enabled: true} +""") + cfg = load_config(cfg_path) + assert cfg.release_version == "latest" + assert cfg.plugins["release_vs_manager"].enabled is True + assert cfg.remote.branch == "main" + assert cfg.base_dirs == () + assert cfg.remote_fallback is False + + +def test_osism_root_and_paths_are_unknown_keys(tmp_path): + # The hardcoded local-path config was removed; --base-dir replaces it. + cfg_path = tmp_path / "c.yml" + cfg_path.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/osism/ + github_api: https://api.github.com/repos/osism/ + branch: main +osism_root: /tmp/repos +paths: {} +release_version: latest +plugins: {} +""") + with pytest.raises(ConfigError, match="osism_root"): + load_config(cfg_path) + + +def test_unknown_top_level_key_errors(tmp_path): + cfg_path = tmp_path / "c.yml" + cfg_path.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/osism/ + github_api: https://api.github.com/repos/osism/ + branch: main +release_version: latest +plugins: + release_vs_manager: {enabled: true} +typo_field: oops +""") + with pytest.raises(ConfigError, match="typo_field"): + load_config(cfg_path) + + +def _entry( + plugin="release_vs_manager", + image="redis", + alias="manager_redis", + found_src="testbed/environments/manager/images.yml", +): + return DriftEntry( + plugin=plugin, + image=image, + alias=alias, + expected="x", + found="y", + expected_src="release/latest/base.yml", + found_src=found_src, + ) + + +def test_allowlist_broad_match(tmp_path): + p = tmp_path / "a.yml" + p.write_text(""" +allow: + - plugin: release_vs_manager + image: redis + reason: "everywhere intentional" +""") + a = load_allowlist(p) + match = a.match(_entry()) + assert match is not None + assert match.reason == "everywhere intentional" + + +def test_apply_marks_matching_drift_allowlisted(tmp_path): + p = tmp_path / "a.yml" + p.write_text(""" +allow: + - plugin: release_vs_manager + image: redis + reason: "everywhere intentional" +""") + a = load_allowlist(p) + result = a.apply(_entry()) + assert result.allowlisted is True + assert result.reason == "everywhere intentional" + + +def test_apply_returns_unmatched_drift_unchanged(tmp_path): + a = load_allowlist(tmp_path / "missing.yml") # empty allowlist + drift = _entry() + result = a.apply(drift) + assert result is drift + assert result.allowlisted is False + + +def test_allowlist_narrow_by_alias(tmp_path): + p = tmp_path / "a.yml" + p.write_text(""" +allow: + - plugin: release_vs_manager + image: redis + alias: manager_redis + reason: "only manager_redis is intentional" +""") + a = load_allowlist(p) + assert a.match(_entry(alias="manager_redis")) is not None + assert a.match(_entry(alias="netbox_redis")) is None + + +def test_allowlist_very_narrow(tmp_path): + p = tmp_path / "a.yml" + p.write_text(""" +allow: + - plugin: role_shadows + image: redis + alias: manager_redis + found_src: ansible-collection-services/roles/manager/defaults/main.yml + reason: "operational pin" +""") + a = load_allowlist(p) + assert ( + a.match( + _entry( + plugin="role_shadows", + alias="manager_redis", + found_src="ansible-collection-services/roles/manager/defaults/main.yml", + ) + ) + is not None + ) + assert ( + a.match( + _entry( + plugin="role_shadows", + alias="manager_redis", + found_src="ansible-collection-services/roles/other/defaults/main.yml", + ) + ) + is None + ) + + +def test_allowlist_empty_reason_errors(tmp_path): + p = tmp_path / "a.yml" + p.write_text(""" +allow: + - plugin: release_vs_manager + image: redis + reason: "" +""") + with pytest.raises(ConfigError, match="reason"): + load_allowlist(p) + + +def test_allowlist_no_file_returns_empty(tmp_path): + a = load_allowlist(tmp_path / "missing.yml") + assert a.match(_entry()) is None + + +from osism_drift.config import AllowEntry # noqa: E402 (used by stale tests in PR2) + + +def test_sources_parsed(tmp_path): + cfg_path = tmp_path / "c.yml" + cfg_path.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/ + github_api: https://api.github.com/repos/ + default_owner: osism + branch: main +sources: + kolla: {owner: openstack, branch: stable/2025.2} +release_version: latest +plugins: {} +""") + cfg = load_config(cfg_path) + assert cfg.remote.default_owner == "osism" + assert cfg.sources["kolla"].owner == "openstack" + assert cfg.sources["kolla"].branch == "stable/2025.2" + + +def test_default_owner_defaults_when_absent(tmp_path): + cfg_path = tmp_path / "c.yml" + cfg_path.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/ + github_api: https://api.github.com/repos/ + branch: main +release_version: latest +plugins: {} +""") + cfg = load_config(cfg_path) + assert cfg.remote.default_owner == "osism" + assert cfg.sources == {} + + +def test_unknown_source_key_errors(tmp_path): + cfg_path = tmp_path / "c.yml" + cfg_path.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/ + github_api: https://api.github.com/repos/ + branch: main +sources: + kolla: {owner: openstack, ref: stable/2025.2} +release_version: latest +plugins: {} +""") + with pytest.raises(ConfigError, match="ref"): + load_config(cfg_path) + + +def test_stale_flags_unused_entry(): + al = Allowlist( + ( + AllowEntry(plugin="p", image="used", reason="r"), + AllowEntry(plugin="p", image="dead", reason="r"), + ) + ) + d = DriftEntry( + plugin="p", + image="used", + alias="used", + expected="e", + found="f", + expected_src="s", + found_src="t", + ) + stale = al.stale([d], {"p"}) + assert [e.image for e in stale] == ["dead"] + + +def test_stale_scoped_to_ran_plugins(): + al = Allowlist((AllowEntry(plugin="q", image="z", reason="r"),)) + # Plugin q did not run, so its entry is not considered stale this run. + assert al.stale([], {"p"}) == [] + + +def _pdrift(image, plugin="kolla_inventory"): + return DriftEntry( + plugin=plugin, + image=image, + alias=image, + expected="e", + found="f", + expected_src="s", + found_src="t", + ) + + +def test_prefix_matches_service_and_subgroups(): + e = AllowEntry(plugin="kolla_inventory", image="cyborg", reason="r", match="prefix") + assert e.matches(_pdrift("cyborg")) is True + assert e.matches(_pdrift("cyborg:children")) is True + assert e.matches(_pdrift("cyborg-agent:children")) is True + + +def test_prefix_does_not_match_adjacent_name(): + e = AllowEntry(plugin="kolla_inventory", image="cyborg", reason="r", match="prefix") + assert e.matches(_pdrift("cyborgx:children")) is False + assert e.matches(_pdrift("cyborg2")) is False + + +def test_exact_is_default_and_unchanged(): + e = AllowEntry(plugin="kolla_inventory", image="control", reason="r") + assert e.match == "exact" + assert e.matches(_pdrift("control")) is True + assert e.matches(_pdrift("control-plane")) is False + + +def test_allowlist_unknown_match_value_errors(tmp_path): + p = tmp_path / "a.yml" + p.write_text(""" +allow: + - {plugin: kolla_inventory, image: cyborg, match: glob, reason: "x"} +""") + with pytest.raises(ConfigError, match="match"): + load_allowlist(p) + + +def test_allowlist_empty_image_errors(tmp_path): + p = tmp_path / "a.yml" + p.write_text(""" +allow: + - {plugin: kolla_inventory, image: "", match: prefix, reason: "x"} +""") + with pytest.raises(ConfigError, match="image"): + load_allowlist(p) + + +def test_prefix_entry_stale_when_no_match(): + al = Allowlist( + ( + AllowEntry( + plugin="kolla_inventory", image="cyborg", reason="r", match="prefix" + ), + AllowEntry( + plugin="kolla_inventory", image="ghost", reason="r", match="prefix" + ), + ) + ) + drifts = [_pdrift("cyborg-agent:children")] + stale = al.stale(drifts, {"kolla_inventory"}) + assert [e.image for e in stale] == ["ghost"] + + +def _write_cfg(tmp_path, body): + p = tmp_path / "c.yml" + p.write_text(body) + return p + + +def test_releases_and_release_refs_parse(tmp_path): + cfg = load_config( + _write_cfg( + tmp_path, + """ +remote: {github_raw: "https://raw/", github_api: "https://api/", branch: main} +release_version: latest +plugins: {} +releases: ["2024.1", "2025.2"] +release_refs: + kolla: + "2024.2": "2024.2-eol" +""", + ) + ) + assert cfg.releases == ("2024.1", "2025.2") + assert cfg.release_refs == {"kolla": {"2024.2": "2024.2-eol"}} + + +def test_releases_default_empty(tmp_path): + cfg = load_config( + _write_cfg( + tmp_path, + """ +remote: {github_raw: "https://raw/", github_api: "https://api/", branch: main} +release_version: latest +plugins: {} +""", + ) + ) + assert cfg.releases == () + assert cfg.release_refs == {} + + +def test_to_remote_only_clears_base_dirs_preserves_rest(tmp_path): + import dataclasses + from osism_drift.config import to_remote_only + + cfg = load_config( + _write_cfg( + tmp_path, + """ +remote: {github_raw: "https://raw/", github_api: "https://api/", branch: main} +release_version: latest +plugins: {} +releases: ["2025.2"] +release_refs: {kolla: {"2024.2": "2024.2-eol"}} +""", + ) + ) + cfg = dataclasses.replace(cfg, base_dirs=("/some/root",), remote_fallback=True) + ro = to_remote_only(cfg) + assert ro.base_dirs == () + assert ro.remote_fallback is True # only base_dirs is cleared + assert ro.releases == ("2025.2",) + assert ro.release_refs == {"kolla": {"2024.2": "2024.2-eol"}} + assert ro.sources == cfg.sources and ro.plugins == cfg.plugins diff --git a/tests/kolla_drift/test_model.py b/tests/kolla_drift/test_model.py new file mode 100644 index 0000000..81e2b9c --- /dev/null +++ b/tests/kolla_drift/test_model.py @@ -0,0 +1,36 @@ +from osism_drift.model import DriftEntry + + +def test_drift_entry_has_all_fields(): + entry = DriftEntry( + plugin="release_vs_manager", + image="redis", + alias="manager_redis", + expected="7.5.0", + found="7.4.7-alpine", + expected_src="release/latest/base.yml", + found_src="testbed/environments/manager/images.yml", + ) + assert entry.plugin == "release_vs_manager" + assert entry.image == "redis" + assert entry.alias == "manager_redis" + assert entry.expected == "7.5.0" + assert entry.found == "7.4.7-alpine" + assert entry.expected_src == "release/latest/base.yml" + assert entry.found_src == "testbed/environments/manager/images.yml" + + +def test_drift_entry_to_dict_for_json(): + entry = DriftEntry( + plugin="role_shadows", + image="adminer", + alias="adminer", + expected="5.4.2", + found="4.7", + expected_src="release/latest/base.yml", + found_src="ansible-collection-services/roles/adminer/defaults/main.yml", + ) + d = entry.to_dict() + assert d["plugin"] == "role_shadows" + assert d["alias"] == "adminer" + assert "expected_src" in d diff --git a/tests/kolla_drift/test_plugin_registry.py b/tests/kolla_drift/test_plugin_registry.py new file mode 100644 index 0000000..5eadafb --- /dev/null +++ b/tests/kolla_drift/test_plugin_registry.py @@ -0,0 +1,37 @@ +from pathlib import Path + +from osism_drift.config import load_config +from osism_drift.drift import PLUGINS + + +def test_registry_is_a_list(): + assert isinstance(PLUGINS, list) + + +def test_each_plugin_has_required_metadata(): + for p in PLUGINS: + assert isinstance(p.NAME, str) and p.NAME + assert isinstance(p.DESCRIPTION, str) and p.DESCRIPTION + assert isinstance(p.INPUT_FILES, list) and p.INPUT_FILES + assert callable(p.run) + + +def test_every_plugin_has_summary_and_remediation(): + for p in PLUGINS: + assert isinstance(p.SUMMARY, str) and p.SUMMARY.strip(), p.NAME + assert "{n}" in p.SUMMARY, p.NAME + assert isinstance(p.REMEDIATION, str) and p.REMEDIATION.strip(), p.NAME + + +def test_default_config_enables_every_registered_plugin(): + # The driver only runs a plugin that is present and enabled in the config, so + # a plugin registered in PLUGINS but missing from the default config would + # silently never run. Guard against that gap. + cfg_path = Path(__file__).resolve().parents[2] / "src" / "kolla-drift-config.yml" + config = load_config(cfg_path) + for p in PLUGINS: + entry = config.plugins.get(p.NAME) + assert entry is not None, f"{p.NAME} missing from default config" + assert entry.enabled, f"{p.NAME} not enabled in default config" + + diff --git a/tests/kolla_drift/test_report.py b/tests/kolla_drift/test_report.py new file mode 100644 index 0000000..e29e3e4 --- /dev/null +++ b/tests/kolla_drift/test_report.py @@ -0,0 +1,127 @@ +import types + +from osism_drift import report +from osism_drift.model import DriftEntry + + +def _plugin(name, summary="{n} things missing:", remediation="do X."): + return types.SimpleNamespace(NAME=name, SUMMARY=summary, REMEDIATION=remediation) + + +def _entry( + plugin, image, expected_src="E", found_src="F", allowlisted=False, alias=None +): + return DriftEntry( + plugin=plugin, + image=image, + alias=alias if alias is not None else image, + expected="exp", + found="found", + expected_src=expected_src, + found_src=found_src, + allowlisted=allowlisted, + ) + + +def test_single_plugin_groups_sorts_and_renders(): + p = _plugin("plug_a") + lines = report.format_text( + [_entry("plug_a", "zebra"), _entry("plug_a", "alpha")], [p] + ) + text = "\n".join(lines) + assert report.HEADER in lines + assert "plug_a — 2 things missing:" in text + assert "alpha, zebra" in text # sorted, comma-joined + assert " Fix: do X." in text + assert " Refs: E" in text + assert " F" in text + + +def test_orders_blocks_by_plugins_list(): + pa = _plugin("a_plug", "{n} a:") + pb = _plugin("b_plug", "{n} b:") + lines = report.format_text([_entry("a_plug", "x"), _entry("b_plug", "y")], [pb, pa]) + text = "\n".join(lines) + assert text.index("b_plug — 1 b:") < text.index("a_plug — 1 a:") + + +def test_build_style_splits_one_block_per_ref_pair(): + p = _plugin("build", "{n} svc:") + drifts = [ + _entry( + "build", + "aodh", + expected_src="openstack/kolla docker/ @ ref-2024.1", + found_src="osism/release latest/openstack-2024.1.yml", + ), + _entry( + "build", + "barbican", + expected_src="openstack/kolla docker/ @ ref-2025.1", + found_src="osism/release latest/openstack-2025.1.yml", + ), + ] + text = "\n".join(report.format_text(drifts, [p])) + assert text.count("build — 1 svc:") == 2 # two separate blocks + assert text.index("2024.1") < text.index("2025.1") # sorted by src pair + + +def test_allowlisted_excluded_from_blocks_and_count(): + p = _plugin("plug") + drifts = [_entry("plug", "shown"), _entry("plug", "hidden", allowlisted=True)] + text = "\n".join(report.format_text(drifts, [p])) + assert "shown" in text + assert "hidden" not in text + assert "plug — 1 things missing:" in text # count excludes allowlisted + + +def test_empty_returns_empty_list(): + p = _plugin("plug") + assert report.format_text([], [p]) == [] + assert report.format_text([_entry("plug", "x", allowlisted=True)], [p]) == [] + + +def test_entry_summary_remediation_override_plugin_default(): + # A plugin emitting two flavours of finding can override the per-plugin + # SUMMARY/REMEDIATION on the entry; the report uses the override when set + # and falls back to the plugin default when it is None. + p = _plugin("plug", summary="{n} default:", remediation="default fix.") + add = DriftEntry( + plugin="plug", + image="valkey", + alias="valkey", + expected="e", + found="f", + expected_src="SBOM", + found_src="TMPL", + summary="{n} to wire:", + remediation="add the key.", + ) + rm = DriftEntry( + plugin="plug", + image="zun", + alias="zun", + expected="e", + found="f", + expected_src="NOIMG", + found_src="TMPL", + summary="{n} dead:", + remediation="remove the line.", + ) + fallback = _entry("plug", "alpha", expected_src="OTHER", found_src="TMPL") + text = "\n".join(report.format_text([add, rm, fallback], [p])) + assert "plug — 1 to wire:" in text + assert "Fix: add the key." in text + assert "plug — 1 dead:" in text + assert "Fix: remove the line." in text + assert "plug — 1 default:" in text # fallback entry uses plugin default + assert "Fix: default fix." in text + + +def test_lists_image_not_alias(): + p = _plugin("plug") + text = "\n".join( + report.format_text([_entry("plug", "real_key", alias="some_alias")], [p]) + ) + assert "real_key" in text + assert "some_alias" not in text diff --git a/tests/kolla_drift/test_source.py b/tests/kolla_drift/test_source.py new file mode 100644 index 0000000..ea86dc0 --- /dev/null +++ b/tests/kolla_drift/test_source.py @@ -0,0 +1,818 @@ +import dataclasses +import subprocess as _sub + +import pytest +import requests +import responses +from osism_drift.source import read, SourceError, read_optional, list_dir +from osism_drift.config import load_config, SourceCfg + + +def _cfg(tmp_path, base_dirs=(), remote_fallback=False): + cfg = tmp_path / "c.yml" + cfg.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/ + github_api: https://api.github.com/repos/ + default_owner: osism + branch: main +release_version: latest +plugins: + release_vs_manager: {enabled: true} +""") + return dataclasses.replace( + load_config(cfg), + base_dirs=tuple(str(b) for b in base_dirs), + remote_fallback=remote_fallback, + ) + + +def _make_cfg(tmp_path, sources=None, release_refs=None): + import yaml + + p = tmp_path / "src.yml" + body = { + "remote": { + "github_raw": "https://raw.githubusercontent.com/", + "github_api": "https://api.github.com/repos/", + "branch": "main", + "default_owner": "osism", + }, + "release_version": "latest", + "plugins": {}, + } + if sources: + body["sources"] = sources + if release_refs: + body["release_refs"] = release_refs + p.write_text(yaml.safe_dump(body)) + return p + + +def test_read_local_working_tree_when_found(tmp_path): + (tmp_path / "release" / "latest").mkdir(parents=True) + (tmp_path / "release" / "latest" / "base.yml").write_text("hello") + cfg = _cfg(tmp_path, base_dirs=(tmp_path,)) + assert read("release", "latest/base.yml", cfg) == b"hello" + + +def test_read_not_found_in_base_dir_is_mode_b_error(tmp_path): + # release dir absent under the only base-dir, no --remote-fallback -> hard error. + cfg = _cfg(tmp_path, base_dirs=(tmp_path,)) + with pytest.raises(SourceError, match="not found under any --base-dir"): + read("release", "latest/base.yml", cfg) + + +def test_read_local_file_absent_is_error_not_remote(tmp_path): + # repo dir present, the file itself absent -> hard error, never silent remote. + (tmp_path / "release").mkdir() + cfg = _cfg(tmp_path, base_dirs=(tmp_path,)) + with pytest.raises(SourceError, match="not found in local"): + read("release", "latest/base.yml", cfg) + + +@responses.activate +def test_read_remote_fallback_when_not_found(tmp_path): + responses.add( + responses.GET, + "https://raw.githubusercontent.com/osism/release/main/latest/base.yml", + body="remote-body", + status=200, + ) + cfg = _cfg(tmp_path, base_dirs=(tmp_path,), remote_fallback=True) + assert read("release", "latest/base.yml", cfg) == b"remote-body" + + +@responses.activate +def test_read_no_base_dir_is_remote(tmp_path): + responses.add( + responses.GET, + "https://raw.githubusercontent.com/osism/release/main/latest/base.yml", + body="remote-only", + status=200, + ) + cfg = _cfg(tmp_path) + assert read("release", "latest/base.yml", cfg) == b"remote-only" + + +def test_first_base_dir_with_the_repo_wins(tmp_path): + a, b = tmp_path / "a", tmp_path / "b" + (b / "release" / "latest").mkdir(parents=True) + (b / "release" / "latest" / "base.yml").write_text("from-b") + (a).mkdir() # a/ exists but has no release/ + cfg = _cfg(tmp_path, base_dirs=(a, b)) + assert read("release", "latest/base.yml", cfg) == b"from-b" + + +def _pinned_kolla_cfg(tmp_path, **kw): + return dataclasses.replace( + _cfg(tmp_path, base_dirs=(tmp_path,), **kw), + sources={"kolla": SourceCfg(owner="openstack", branch="stable/2025.2")}, + ) + + +def test_pinned_local_git_repo_reads_at_pin_ref(tmp_path): + # kolla is a real git clone under the base-dir -> read at the pin ref via git. + from osism_drift.source import list_dir + + _make_repo( + tmp_path / "kolla", + [ + ("main", "branch", {"docker/old.txt": "x"}), + ( + "stable/2025.2", + "branch", + {"docker/nova/Dockerfile.j2": "n", "docker/base/Dockerfile.j2": "b"}, + ), + ], + ) + cfg = _pinned_kolla_cfg(tmp_path) + assert read("kolla", "docker/nova/Dockerfile.j2", cfg) == b"n" + assert sorted(list_dir("kolla", "docker", cfg, dirs_only=True)) == ["base", "nova"] + + +@responses.activate +def test_pinned_non_git_dir_falls_back_to_remote(tmp_path): + # A plain (non-git) kolla/ dir cannot serve a pinned repo -> --remote-fallback goes remote. + (tmp_path / "kolla" / "docker").mkdir(parents=True) + (tmp_path / "kolla" / "docker" / "x.txt").write_text("local") + responses.add( + responses.GET, + "https://raw.githubusercontent.com/openstack/kolla/stable/2025.2/docker/x.txt", + body="remote", + status=200, + ) + cfg = _pinned_kolla_cfg(tmp_path, remote_fallback=True) + assert read("kolla", "docker/x.txt", cfg) == b"remote" + + +def test_pinned_non_git_dir_no_fallback_is_mode_b(tmp_path): + (tmp_path / "kolla").mkdir() # plain dir, not a git repo, no fallback + cfg = _pinned_kolla_cfg(tmp_path) + with pytest.raises(SourceError, match="not found under any --base-dir"): + read("kolla", "docker/x.txt", cfg) + + +@responses.activate +def test_read_remote_404_errors(tmp_path): + responses.add( + responses.GET, + "https://raw.githubusercontent.com/osism/release/main/latest/base.yml", + body="not found", + status=404, + ) + cfg = _cfg(tmp_path) + with pytest.raises(SourceError, match="404"): + read("release", "latest/base.yml", cfg) + + +@responses.activate +def test_read_network_error(tmp_path): + responses.add( + responses.GET, + "https://raw.githubusercontent.com/osism/release/main/latest/base.yml", + body=requests.ConnectionError("boom"), + ) + cfg = _cfg(tmp_path) + with pytest.raises(SourceError, match="boom"): + read("release", "latest/base.yml", cfg) + + +@responses.activate +def test_read_underscore_repo_translates_to_hyphen(tmp_path): + responses.add( + responses.GET, + "https://raw.githubusercontent.com/osism/ansible-collection-services/main/roles/x/defaults/main.yml", + body="ok", + status=200, + ) + cfg = _cfg(tmp_path) + assert ( + read("ansible_collection_services", "roles/x/defaults/main.yml", cfg) == b"ok" + ) + + +def test_read_optional_present_local(tmp_path): + d = tmp_path / "acs" / "roles" / "x" / "defaults" + d.mkdir(parents=True) + (d / "main.yml").write_text("ok") + cfg = _cfg(tmp_path, base_dirs=(tmp_path,)) + assert read_optional("acs", "roles/x/defaults/main.yml", cfg) == b"ok" + + +def test_read_optional_missing_local_returns_none(tmp_path): + (tmp_path / "acs" / "roles" / "x").mkdir(parents=True) # repo present, file absent + cfg = _cfg(tmp_path, base_dirs=(tmp_path,)) + assert read_optional("acs", "roles/x/defaults/main.yml", cfg) is None + + +@responses.activate +def test_read_optional_404_returns_none(tmp_path): + responses.add( + responses.GET, + "https://raw.githubusercontent.com/osism/acs/main/roles/x/defaults/main.yml", + status=404, + ) + cfg = _cfg(tmp_path) + assert read_optional("acs", "roles/x/defaults/main.yml", cfg) is None + + +@responses.activate +def test_read_optional_network_error_still_raises(tmp_path): + responses.add( + responses.GET, + "https://raw.githubusercontent.com/osism/acs/main/roles/x/defaults/main.yml", + body=requests.ConnectionError("dns"), + ) + cfg = _cfg(tmp_path) + with pytest.raises(SourceError): + read_optional("acs", "roles/x/defaults/main.yml", cfg) + + +@responses.activate +def test_read_optional_underscore_repo_translates_to_hyphen(tmp_path): + responses.add( + responses.GET, + "https://raw.githubusercontent.com/osism/ansible-collection-services/main/roles/x/defaults/main.yml", + status=404, + ) + cfg = _cfg(tmp_path) + assert ( + read_optional("ansible_collection_services", "roles/x/defaults/main.yml", cfg) + is None + ) + + +def test_list_dir_local(tmp_path): + for r in ("a", "b", "c"): + (tmp_path / "acs" / "roles" / r).mkdir(parents=True) + cfg = _cfg(tmp_path, base_dirs=(tmp_path,)) + assert sorted(list_dir("acs", "roles", cfg)) == ["a", "b", "c"] + + +@responses.activate +def test_list_dir_remote(tmp_path): + responses.add( + responses.GET, + "https://api.github.com/repos/osism/acs/contents/roles?ref=main", + json=[{"name": "a", "type": "dir"}, {"name": "b", "type": "dir"}], + status=200, + ) + cfg = _cfg(tmp_path) + assert sorted(list_dir("acs", "roles", cfg)) == ["a", "b"] + + +@responses.activate +def test_list_dir_remote_404_errors(tmp_path): + responses.add( + responses.GET, + "https://api.github.com/repos/osism/acs/contents/roles?ref=main", + status=404, + ) + cfg = _cfg(tmp_path) + with pytest.raises(SourceError, match="404"): + list_dir("acs", "roles", cfg) + + +@responses.activate +def test_list_dir_underscore_repo_translates_to_hyphen(tmp_path): + responses.add( + responses.GET, + "https://api.github.com/repos/osism/ansible-collection-services/contents/roles?ref=main", + json=[{"name": "a", "type": "dir"}], + status=200, + ) + cfg = _cfg(tmp_path) + assert list_dir("ansible_collection_services", "roles", cfg) == ["a"] + + +@responses.activate +def test_source_override_redirects_owner_and_ref(tmp_path): + responses.add( + responses.GET, + "https://raw.githubusercontent.com/openstack/kolla/stable/2025.2/docker/x/Dockerfile.j2", + body="ok", + status=200, + ) + cfg_path = tmp_path / "c.yml" + cfg_path.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/ + github_api: https://api.github.com/repos/ + default_owner: osism + branch: main +sources: + kolla: {owner: openstack, branch: stable/2025.2} +release_version: latest +plugins: {} +""") + cfg = load_config(cfg_path) + assert read("kolla", "docker/x/Dockerfile.j2", cfg) == b"ok" + + +def test_list_dir_local_dirs_only_excludes_files(tmp_path): + # kolla unpinned here -> working-tree listing discovered under the base-dir. + (tmp_path / "kolla" / "docker" / "nova").mkdir(parents=True) + (tmp_path / "kolla" / "docker" / "base").mkdir(parents=True) + (tmp_path / "kolla" / "docker" / "macros.j2").write_text("{# jinja #}") + cfg = _cfg(tmp_path, base_dirs=(tmp_path,)) + assert sorted(list_dir("kolla", "docker", cfg, dirs_only=True)) == ["base", "nova"] + assert "macros.j2" in list_dir("kolla", "docker", cfg) + + +@responses.activate +def test_list_dir_remote_dirs_only_filters_type(tmp_path): + responses.add( + responses.GET, + "https://api.github.com/repos/openstack/kolla/contents/docker?ref=stable/2025.2", + json=[{"name": "nova", "type": "dir"}, {"name": "macros.j2", "type": "file"}], + status=200, + ) + cfg_path = tmp_path / "c.yml" + cfg_path.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/ + github_api: https://api.github.com/repos/ + default_owner: osism + branch: main +sources: + kolla: {owner: openstack, branch: stable/2025.2} +release_version: latest +plugins: {} +""") + cfg = load_config(cfg_path) + assert list_dir("kolla", "docker", cfg, dirs_only=True) == ["nova"] + + +@responses.activate +def test_list_dir_at_ref_uses_explicit_ref_and_owner(tmp_path): + # kolla pinned to a DIFFERENT branch; list_dir_at_ref must ignore the pin. + # responses ignores the query string when matching, so register the bare URL + # and assert the ref/owner via the recorded request (no matcher API needed). + cfg = load_config( + _make_cfg( + tmp_path, + sources={"kolla": {"owner": "openstack", "branch": "stable/2025.2"}}, + ) + ) + responses.add( + responses.GET, + "https://api.github.com/repos/openstack/kolla/contents/docker", + json=[ + {"name": "valkey", "type": "dir"}, + {"name": "README.rst", "type": "file"}, + ], + status=200, + ) + from osism_drift.source import list_dir_at_ref + + out = list_dir_at_ref("kolla", "docker", "unmaintained/2024.1", cfg, dirs_only=True) + assert out == ["valkey"] + assert "/openstack/kolla/contents/docker" in responses.calls[0].request.url + assert "ref=unmaintained/2024.1" in responses.calls[0].request.url + + +@responses.activate +def test_list_dir_at_ref_404_raises(tmp_path): + cfg = load_config(_make_cfg(tmp_path, sources={"kolla": {"owner": "openstack"}})) + responses.add( + responses.GET, + "https://api.github.com/repos/openstack/kolla/contents/docker", + status=404, + ) + from osism_drift.source import list_dir_at_ref, SourceError + + with pytest.raises(SourceError): + list_dir_at_ref("kolla", "docker", "stable/2099.1", cfg) + + +def _commits_url(owner, repo, ref): + return f"https://api.github.com/repos/{owner}/{repo}/commits/{ref}" + + +@responses.activate +def test_release_to_ref_prefers_stable(tmp_path): + cfg = load_config(_make_cfg(tmp_path, sources={"kolla": {"owner": "openstack"}})) + responses.add( + responses.GET, _commits_url("openstack", "kolla", "stable/2025.2"), status=200 + ) + from osism_drift.source import release_to_ref + + assert release_to_ref("kolla", "2025.2", cfg) == "stable/2025.2" + + +@responses.activate +def test_release_to_ref_falls_back_through_candidates(tmp_path): + cfg = load_config(_make_cfg(tmp_path, sources={"kolla": {"owner": "openstack"}})) + responses.add( + responses.GET, _commits_url("openstack", "kolla", "stable/2024.1"), status=404 + ) + responses.add( + responses.GET, + _commits_url("openstack", "kolla", "unmaintained/2024.1"), + status=200, + ) + from osism_drift.source import release_to_ref + + assert release_to_ref("kolla", "2024.1", cfg) == "unmaintained/2024.1" + + +@responses.activate +def test_release_to_ref_treats_422_as_absent(tmp_path): + # GitHub's commits API returns 422 (not 404) for a ref that does not resolve + # (e.g. stable/2024.1 once a release moves to unmaintained/). The probe must + # treat it as absent and fall through, not abort. + cfg = load_config(_make_cfg(tmp_path, sources={"kolla": {"owner": "openstack"}})) + responses.add( + responses.GET, _commits_url("openstack", "kolla", "stable/2024.1"), status=422 + ) + responses.add( + responses.GET, + _commits_url("openstack", "kolla", "unmaintained/2024.1"), + status=200, + ) + from osism_drift.source import release_to_ref + + assert release_to_ref("kolla", "2024.1", cfg) == "unmaintained/2024.1" + + +@responses.activate +def test_release_to_ref_uses_eom_tag_last(tmp_path): + cfg = load_config(_make_cfg(tmp_path, sources={"kolla": {"owner": "openstack"}})) + for ref, st in [ + ("stable/2024.1", 404), + ("unmaintained/2024.1", 404), + ("2024.1-eol", 404), + ("2024.1-eom", 200), + ]: + responses.add(responses.GET, _commits_url("openstack", "kolla", ref), status=st) + from osism_drift.source import release_to_ref + + assert release_to_ref("kolla", "2024.1", cfg) == "2024.1-eom" + + +@responses.activate +def test_release_to_ref_none_raises(tmp_path): + cfg = load_config(_make_cfg(tmp_path, sources={"kolla": {"owner": "openstack"}})) + for ref in ["stable/2099.1", "unmaintained/2099.1", "2099.1-eol", "2099.1-eom"]: + responses.add( + responses.GET, _commits_url("openstack", "kolla", ref), status=404 + ) + from osism_drift.source import release_to_ref, SourceError + + with pytest.raises(SourceError, match="no upstream ref"): + release_to_ref("kolla", "2099.1", cfg) + + +def test_release_to_ref_override_wins(tmp_path): + # No responses registered: override must short-circuit before any HTTP call. + cfg = load_config( + _make_cfg( + tmp_path, + sources={"kolla": {"owner": "openstack"}}, + release_refs={"kolla": {"2024.2": "2024.2-eol"}}, + ) + ) + from osism_drift.source import release_to_ref + + assert release_to_ref("kolla", "2024.2", cfg) == "2024.2-eol" + + +@responses.activate +def test_read_at_ref_returns_bytes_at_explicit_ref(tmp_path): + # kolla_ansible pinned to a DIFFERENT branch; read_at_ref must ignore the pin + # and read the supplied ref, honouring the owner override. + cfg = load_config( + _make_cfg( + tmp_path, + sources={ + "kolla_ansible": {"owner": "openstack", "branch": "stable/2025.2"} + }, + ) + ) + responses.add( + responses.GET, + "https://raw.githubusercontent.com/openstack/kolla-ansible/" + "unmaintained/2024.1/ansible/group_vars/all.yml", + body=b'enable_redis: "no"\n', + status=200, + ) + from osism_drift.source import read_at_ref + + out = read_at_ref( + "kolla_ansible", "ansible/group_vars/all.yml", "unmaintained/2024.1", cfg + ) + assert out == b'enable_redis: "no"\n' + assert ( + "/openstack/kolla-ansible/unmaintained/2024.1/ansible/group_vars/all.yml" + in responses.calls[0].request.url + ) + + +@responses.activate +def test_read_at_ref_optional_404_returns_none(tmp_path): + cfg = load_config( + _make_cfg(tmp_path, sources={"kolla_ansible": {"owner": "openstack"}}) + ) + responses.add( + responses.GET, + "https://raw.githubusercontent.com/openstack/kolla-ansible/" + "stable/2025.2/ansible/group_vars/all.yml", + status=404, + ) + from osism_drift.source import read_at_ref + + assert ( + read_at_ref( + "kolla_ansible", + "ansible/group_vars/all.yml", + "stable/2025.2", + cfg, + optional=True, + ) + is None + ) + + +@responses.activate +def test_read_at_ref_404_raises_when_not_optional(tmp_path): + cfg = load_config( + _make_cfg(tmp_path, sources={"kolla_ansible": {"owner": "openstack"}}) + ) + responses.add( + responses.GET, + "https://raw.githubusercontent.com/openstack/kolla-ansible/" + "stable/2025.2/ansible/group_vars/all/x.yml", + status=404, + ) + from osism_drift.source import read_at_ref, SourceError + + with pytest.raises(SourceError): + read_at_ref( + "kolla_ansible", "ansible/group_vars/all/x.yml", "stable/2025.2", cfg + ) + + +def test_describe_resolution_local_and_remote(tmp_path): + from osism_drift.source import describe_resolution + + (tmp_path / "defaults").mkdir() + # kolla is pinned but has no local git clone -> remote_fallback lets it read remotely + cfg = dataclasses.replace( + _cfg(tmp_path, base_dirs=(tmp_path,), remote_fallback=True), + sources={"kolla": SourceCfg(owner="openstack", branch="stable/2025.2")}, + ) + lines = describe_resolution(["kolla", "defaults"], cfg) + joined = "\n".join(lines) + assert "defaults" in joined and "local" in joined and "working tree" in joined + assert "kolla" in joined and "remote" in joined and "stable/2025.2" in joined + + +def test_describe_resolution_mode_b_raises(tmp_path): + from osism_drift.source import describe_resolution, SourceError + + cfg = _cfg(tmp_path, base_dirs=(tmp_path,)) # defaults/ absent, no fallback + with pytest.raises(SourceError, match="not found under any --base-dir"): + describe_resolution(["defaults"], cfg) + + +def _git_init(path): + path.mkdir(parents=True, exist_ok=True) + import os as _os + + # Strip agent env vars so the OSISM commit-msg hook does not fire on + # throwaway fixture repos (hook only enforces when CLAUDECODE/AI_AGENT is set). + _env = {k: v for k, v in _os.environ.items() if k not in ("CLAUDECODE", "AI_AGENT")} + + def g(*a): + _sub.run( + ["git", "-C", str(path), *a], check=True, capture_output=True, env=_env + ) + + g("init", "-q") + g("config", "user.email", "t@t") + g("config", "user.name", "T") + return g + + +def _make_repo(path, refs): + # refs: list of (ref_name, kind 'branch'|'tag', {relpath: content}); commit + # each snapshot in order and label it. We read objects, not the work tree. + g = _git_init(path) + for i, (ref, kind, files) in enumerate(refs): + g( + "rm", "-r", "--quiet", "--ignore-unmatch", "." + ) # clear prior snapshot -> each ref's tree is exactly `files` + for rel, content in files.items(): + f = path / rel + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(content) + g("add", rel) + g("commit", "-q", "-m", ref) + if i == 0: + g( + "checkout", "-q", "--detach" + ) # no current branch, so `branch -f ` works for any name (incl. the default) + g("branch" if kind == "branch" else "tag", "-f", ref) + return path + + +def test_git_show_multi_ref_from_one_clone(tmp_path): + from osism_drift.source import _git_show + + repo = _make_repo( + tmp_path / "kolla-ansible", + [ + ( + "stable/2025.1", + "branch", + {"ansible/group_vars/all.yml": 'enable_redis: "no"\n'}, + ), + ( + "stable/2025.2", + "branch", + {"ansible/group_vars/all.yml": 'enable_valkey: "no"\n'}, + ), + ], + ) + assert ( + _git_show(repo, "stable/2025.1", "ansible/group_vars/all.yml") + == b'enable_redis: "no"\n' + ) + assert ( + _git_show(repo, "stable/2025.2", "ansible/group_vars/all.yml") + == b'enable_valkey: "no"\n' + ) + + +def test_git_show_optional_absent_path(tmp_path): + from osism_drift.source import _git_show, SourceError + + repo = _make_repo( + tmp_path / "kolla-ansible", + [ + ( + "stable/2025.2", + "branch", + {"ansible/group_vars/all/valkey.yml": 'enable_valkey: "no"\n'}, + ), + ], + ) + # ref exists, path absent (monolithic all.yml not present at 2025.2): + assert ( + _git_show(repo, "stable/2025.2", "ansible/group_vars/all.yml", optional=True) + is None + ) + with pytest.raises(SourceError): + _git_show(repo, "stable/2025.2", "ansible/group_vars/all.yml") + + +def test_git_ls_tree_returns_basenames_and_dirs_only(tmp_path): + from osism_drift.source import _git_ls_tree + + repo = _make_repo( + tmp_path / "kolla", + [ + ( + "stable/2025.2", + "branch", + { + "docker/nova/Dockerfile.j2": "x", + "docker/base/Dockerfile.j2": "y", + "docker/macros.j2": "{# jinja #}", + }, + ), + ], + ) + assert sorted(_git_ls_tree(repo, "stable/2025.2", "docker")) == [ + "base", + "macros.j2", + "nova", + ] + assert sorted(_git_ls_tree(repo, "stable/2025.2", "docker", dirs_only=True)) == [ + "base", + "nova", + ] + + +def test_resolve_local_ref_finds_non_origin_remote(tmp_path): + from osism_drift.source import _resolve_local_ref, _git_show + + # upstream clone holds the canonical ref; main clone has it only as a + # remote-tracking ref under a remote named "gerrit". + up = _make_repo( + tmp_path / "up", + [ + ("unmaintained/2024.1", "branch", {"f.txt": "hi"}), + ], + ) + main = tmp_path / "kolla-ansible" + _git_init(main) + _sub.run( + ["git", "-C", str(main), "remote", "add", "gerrit", str(up)], + check=True, + capture_output=True, + ) + _sub.run( + ["git", "-C", str(main), "fetch", "-q", "gerrit"], + check=True, + capture_output=True, + ) + # a bare "unmaintained/2024.1" is not a local branch here, only gerrit/unmaintained/2024.1: + assert ( + _resolve_local_ref(main, "unmaintained/2024.1") == "gerrit/unmaintained/2024.1" + ) + assert _git_show(main, "unmaintained/2024.1", "f.txt") == b"hi" + + +def test_git_show_unresolvable_ref_raises(tmp_path): + from osism_drift.source import _git_show, SourceError + + repo = _make_repo(tmp_path / "kolla", [("main", "branch", {"f": "x"})]) + with pytest.raises(SourceError, match="not found"): + _git_show(repo, "stable/2099.1", "f") + + +def test_git_ref_exists(tmp_path): + from osism_drift.source import _git_ref_exists + + repo = _make_repo(tmp_path / "kolla", [("stable/2025.2", "branch", {"f": "x"})]) + assert _git_ref_exists(repo, "stable/2025.2") is True + assert _git_ref_exists(repo, "stable/2099.1") is False + + +def test_read_at_ref_local_git(tmp_path): + from osism_drift.source import read_at_ref, list_dir_at_ref, ref_exists + + _make_repo( + tmp_path / "kolla-ansible", + [ + ( + "stable/2025.1", + "branch", + {"ansible/group_vars/all.yml": 'enable_redis: "no"\n'}, + ), + ( + "stable/2025.2", + "branch", + {"ansible/group_vars/all/valkey.yml": 'enable_valkey: "no"\n'}, + ), + ], + ) + cfg = dataclasses.replace( + _cfg(tmp_path, base_dirs=(tmp_path,)), + sources={"kolla_ansible": SourceCfg(owner="openstack", branch="main")}, + ) + # explicit ref, ignoring the pin: + assert ( + read_at_ref("kolla_ansible", "ansible/group_vars/all.yml", "stable/2025.1", cfg) + == b'enable_redis: "no"\n' + ) + # optional miss at a ref where the path is absent: + assert ( + read_at_ref( + "kolla_ansible", + "ansible/group_vars/all.yml", + "stable/2025.2", + cfg, + optional=True, + ) + is None + ) + assert list_dir_at_ref( + "kolla_ansible", "ansible/group_vars/all", "stable/2025.2", cfg + ) == ["valkey.yml"] + assert ref_exists("kolla_ansible", "stable/2025.1", cfg) is True + assert ref_exists("kolla_ansible", "stable/2099.1", cfg) is False + + +def test_describe_resolution_pinned_local_tag(tmp_path): + from osism_drift.source import describe_resolution + + _make_repo( + tmp_path / "kolla", + [("stable/2025.2", "branch", {"docker/x/Dockerfile.j2": "x"})], + ) + cfg = dataclasses.replace( + _cfg(tmp_path, base_dirs=(tmp_path,)), + sources={"kolla": SourceCfg(owner="openstack", branch="stable/2025.2")}, + ) + line = "\n".join(describe_resolution(["kolla"], cfg)) + assert "kolla" in line and "local" in line and "git refs, must be current" in line + + +@responses.activate +def test_read_at_ref_unpinned_local_dir_is_remote(tmp_path): + # An UNPINNED plain dir under --base-dir must not be git-read by read_at_ref + # (it has no .git); it reads remotely at the explicit ref instead of crashing git. + from osism_drift.source import read_at_ref + + (tmp_path / "acs").mkdir() + responses.add( + responses.GET, + "https://raw.githubusercontent.com/osism/acs/stable/2025.2/x.yml", + body="remote", + status=200, + ) + cfg = _cfg(tmp_path, base_dirs=(tmp_path,)) # acs unpinned (no sources) + assert read_at_ref("acs", "x.yml", "stable/2025.2", cfg) == b"remote" diff --git a/tox.ini b/tox.ini index 1465a7d..860aa93 100644 --- a/tox.ini +++ b/tox.ini @@ -37,3 +37,9 @@ allowlist_externals = * commands = bash src/test-seed.sh + +[testenv:kolla-drift-test] +basepython = python3 +deps = -r requirements.txt +commands = + pytest {posargs:tests/kolla_drift/ -v} From 9231bbc0c1669cfa9c7e4c9f1751555a7f559f71 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 15:50:43 +0200 Subject: [PATCH 02/12] Add kolla_enablement_orphan drift plugin Flag OSISM enable_* flags whose service is absent from upstream kolla-ansible's top-level enable-defaults across all supported releases (the service was removed or renamed upstream). Orphan = absent from the union over the supported range; per-release keys come from enablement.upstream_enable_keys (monolithic group_vars/all.yml at 2024.1/2024.2/2025.1, split group_vars/all/*.yml at 2025.2), resolved via release_to_ref. Guard an empty release range, which would otherwise report every flag as orphan. The SCOPE selects which OSISM enable flags are eligible. Ship the "explicit" scope: it flags the dead enable_X: "no" cleanup flags upstream kolla-ansible no longer defines at any supported release (freezer, murano, sahara, senlin, solum, vitrage and their horizon plugins, ironic_pxe_uefi, outward_rabbitmq, skydive), which is what drives the osism/defaults cleanup. The OSISM-invented common and kolla_operations flags have no upstream counterpart by design and are kept via the allowlist. Add the I/O-free enablement parse helpers (parse_enable_flags, truthy_enables, canon, release_range) and the upstream enable-defaults resolver. Expose orphan_ids() for reuse by kolla_orphan_config. Register and enable the plugin. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Roger Luethi --- src/kolla-drift-allowlist.yml | 7 +- src/kolla-drift-config.yml | 3 +- src/osism_drift/drift/__init__.py | 4 +- .../drift/kolla_enablement_orphan.py | 101 +++++++++++ src/osism_drift/enablement.py | 99 +++++++++++ tests/kolla_drift/test_driver.py | 18 ++ tests/kolla_drift/test_enablement.py | 158 ++++++++++++++++++ .../test_kolla_enablement_orphan.py | 125 ++++++++++++++ tests/kolla_drift/test_plugin_registry.py | 4 + 9 files changed, 516 insertions(+), 3 deletions(-) create mode 100644 src/osism_drift/drift/kolla_enablement_orphan.py create mode 100644 src/osism_drift/enablement.py create mode 100644 tests/kolla_drift/test_driver.py create mode 100644 tests/kolla_drift/test_enablement.py create mode 100644 tests/kolla_drift/test_kolla_enablement_orphan.py diff --git a/src/kolla-drift-allowlist.yml b/src/kolla-drift-allowlist.yml index 91b742f..a691ecb 100644 --- a/src/kolla-drift-allowlist.yml +++ b/src/kolla-drift-allowlist.yml @@ -1,3 +1,8 @@ # Known-intentional exceptions. Each entry needs a non-empty `reason`. # An entry that suppresses no real drift is reported as stale (a hard error). -allow: [] +allow: + # --- kolla_enablement_orphan --- + # OSISM-invented enable flags with no upstream counterpart by design (not a + # removed-upstream service). Truthy, so they surface in the truthy-only scope. + - {plugin: kolla_enablement_orphan, image: common, reason: "OSISM-invented flag; no upstream counterpart by design"} + - {plugin: kolla_enablement_orphan, image: kolla_operations, reason: "OSISM-invented flag; no upstream counterpart by design"} diff --git a/src/kolla-drift-config.yml b/src/kolla-drift-config.yml index b1d949d..9296add 100644 --- a/src/kolla-drift-config.yml +++ b/src/kolla-drift-config.yml @@ -14,4 +14,5 @@ sources: release_version: latest -plugins: {} +plugins: + kolla_enablement_orphan: {enabled: true} diff --git a/src/osism_drift/drift/__init__.py b/src/osism_drift/drift/__init__.py index ac2a349..3dbfb7f 100644 --- a/src/osism_drift/drift/__init__.py +++ b/src/osism_drift/drift/__init__.py @@ -1,3 +1,5 @@ """Plugin registry. Plugins are appended here as they are added.""" -PLUGINS = [] +from osism_drift.drift import kolla_enablement_orphan + +PLUGINS = [kolla_enablement_orphan] diff --git a/src/osism_drift/drift/kolla_enablement_orphan.py b/src/osism_drift/drift/kolla_enablement_orphan.py new file mode 100644 index 0000000..b997b73 --- /dev/null +++ b/src/osism_drift/drift/kolla_enablement_orphan.py @@ -0,0 +1,101 @@ +"""kolla_enablement_orphan: OSISM enables a service upstream no longer defines. + +An OSISM enable flag (defaults all/099-kolla.yml enable_X) is an orphan when its +service X is absent from upstream kolla-ansible's enable-defaults at every +supported release: upstream removed or renamed the service, so the OSISM flag is +stale and should be cleaned up or migrated. + +A service defined in any in-range release is still needed, so the test is against +the union of upstream top-level enable-defaults across the release range, not any +single release. The per-release upstream keys come from +enablement.upstream_enable_keys (the monolithic group_vars/all.yml, else the split +group_vars/all/*.yml), resolved per release via source.release_to_ref for +kolla-ansible. Only top-level defaults count; reference-only mentions in tasks or +tests do not. + +SCOPE selects which OSISM enable flags are eligible, differing only in the OSISM +input set (enablement.osism_enable_ids): + + "truthy" -- flags set to a truthy value. The only such orphan candidates are + the OSISM-invented flags common and kolla_operations, both + allowlisted, so this scope yields a clean, gateable baseline. + "explicit" -- additionally includes dead enable_X: "no" cleanup flags, which + are real orphans but not yet removed from osism/defaults. +""" + +from osism_drift import enablement, source +from osism_drift.model import DriftEntry + +NAME = "kolla_enablement_orphan" +DESCRIPTION = ( + "Flag OSISM enable_* flags whose service is absent from upstream " + "kolla-ansible enable-defaults across all supported releases (orphan)." +) +INPUT_FILES = [ + ("defaults", "all/099-kolla.yml"), + ("kolla_ansible", "ansible/group_vars/all[.yml|/*.yml] (per resolved release ref)"), +] +SUMMARY = ( + "{n} OSISM enable flags whose service upstream kolla-ansible no longer " + "defines at any supported release; the service was removed or renamed " + "upstream, leaving the flag orphaned:" +) +REMEDIATION = ( + "remove the stale enable_ from osism/defaults, or migrate it to the " + "upstream replacement. Some flags (e.g. common, kolla_operations) are OSISM " + "inventions with no upstream counterpart — keep those allowlisted rather " + "than removed." +) + +_ENABLE = "all/099-kolla.yml" +SCOPE = "explicit" # see module docstring; also flags dead enable_X: "no" cruft + + +def run(config, allowlist, verbose: bool = False) -> list[DriftEntry]: + """Return orphan-flag drifts: OSISM enables a service upstream dropped.""" + return _run(config, allowlist, SCOPE, verbose) + + +def orphan_ids(config, scope: str = SCOPE) -> set: + """Service ids OSISM enables that upstream defines at no supported release. + + The raw orphan set (before the allowlist), shared with kolla_orphan_config + so the companion/image sweep keys off the same dead-service determination. + """ + osism = enablement.osism_enable_ids( + enablement.parse_enable_flags(source.read("defaults", _ENABLE, config)), scope + ) + releases = enablement.release_range(config) + if not releases: + # set().union(*[]) is empty, which would report EVERY selected flag as an + # orphan (mass false positive). Fail loud instead. + raise source.SourceError( + "empty supported release range; cannot compute the upstream " + "enable-defaults union" + ) + upstream = set().union( + *(enablement.upstream_enable_keys(r, config) for r in releases) + ) + return osism - upstream + + +def _run(config, allowlist, scope, verbose: bool = False) -> list[DriftEntry]: + drifts = [] + for sid in sorted(orphan_ids(config, scope)): + d = DriftEntry( + plugin=NAME, + image=sid, + alias=sid, + expected=( + "defined in upstream kolla-ansible enable-defaults at some " + "supported release" + ), + found=( + "absent from upstream enable-defaults across all supported " + "releases (orphan)" + ), + expected_src="openstack/kolla-ansible group_vars enable-defaults @ supported refs", + found_src="osism/defaults all/099-kolla.yml", + ) + drifts.append(allowlist.apply(d)) + return drifts diff --git a/src/osism_drift/enablement.py b/src/osism_drift/enablement.py new file mode 100644 index 0000000..92ff0a9 --- /dev/null +++ b/src/osism_drift/enablement.py @@ -0,0 +1,99 @@ +"""Shared parse helpers for the enablement-drift checks (no I/O policy). + +Pure functions over file bytes: parse OSISM enable flags and the OSISM build +set, plus the supported-release range. `canon` normalises the hyphen/underscore +split between key spaces (enable_* uses underscores; docker/ dirs and release +build keys use hyphens) so every cross-space comparison is on one form. +""" + +import yaml + +from osism_drift import source + + +def canon(name: str) -> str: + """Canonical id for cross-key-space compares: hyphens -> underscores.""" + return name.replace("-", "_") + + +def parse_enable_flags(body: bytes) -> dict: + """{service_id: raw_value} for every enable_ in an OSISM vars file.""" + data = yaml.safe_load(body) or {} + return { + k[len("enable_") :]: v + for k, v in data.items() + if isinstance(k, str) and k.startswith("enable_") + } + + +def truthy_enables(flags: dict) -> set: + """canon ids whose value is a literal yes/true (skip no/false/jinja).""" + out = set() + for sid, val in flags.items(): + if val is True: + out.add(canon(sid)) + elif isinstance(val, str) and val.strip().lower() in ("yes", "true"): + out.add(canon(sid)) + return out + + +def parse_build_set(body: bytes) -> set: + """canon keys OSISM builds at a release: infra ∪ openstack project keys.""" + data = yaml.safe_load(body) or {} + keys = set() + for block in ("infrastructure_projects", "openstack_projects"): + keys |= {canon(k) for k in (data.get(block) or {})} + return keys + + +def release_range(config) -> list: + """Supported releases: config.releases override, else derived from the + osism/release latest/openstack-*.yml file set (sorted).""" + if config.releases: + return list(config.releases) + names = source.list_dir("release", "latest", config) + rels = [ + n[len("openstack-") : -len(".yml")] + for n in names + if n.startswith("openstack-") and n.endswith(".yml") + ] + return sorted(rels) + + +def upstream_enable_keys(release, config) -> set: + """canon ids of every top-level enable_ default in upstream kolla-ansible + at `release`'s resolved ref. + + The enable-defaults layer moves between releases: a monolithic + ansible/group_vars/all.yml (2024.1/2024.2/2025.1) or a split + ansible/group_vars/all/*.yml dir (2025.2+). Probe the monolithic file first; + on a 404 fall through to the split dir. Top-level defaults only — never + role-defaults/tasks/tests/releasenotes — so a reference-only mention of an + enable var is not counted as a definition. Always remote. + """ + ref = source.release_to_ref("kolla_ansible", release, config) + mono = source.read_at_ref( + "kolla_ansible", "ansible/group_vars/all.yml", ref, config, optional=True + ) + if mono is not None: + return {canon(k) for k in parse_enable_flags(mono)} + keys = set() + for name in source.list_dir_at_ref( + "kolla_ansible", "ansible/group_vars/all", ref, config + ): + if name.endswith(".yml"): + body = source.read_at_ref( + "kolla_ansible", f"ansible/group_vars/all/{name}", ref, config + ) + keys |= {canon(k) for k in parse_enable_flags(body)} + return keys + + +def osism_enable_ids(flags, scope) -> set: + """OSISM enable ids selected by scope. 'truthy' -> literal yes/true only; + 'explicit' -> every enable_* key (canon-normalized) regardless of value.""" + if scope == "truthy": + return truthy_enables(flags) + if scope == "explicit": + return {canon(k) for k in flags} + raise ValueError(f"unknown scope {scope!r}") diff --git a/tests/kolla_drift/test_driver.py b/tests/kolla_drift/test_driver.py new file mode 100644 index 0000000..45a8829 --- /dev/null +++ b/tests/kolla_drift/test_driver.py @@ -0,0 +1,18 @@ +import importlib.util +from pathlib import Path + +_SPEC = importlib.util.spec_from_file_location( + "check_kolla_drift", + Path(__file__).resolve().parents[2] / "src" / "check-kolla-drift.py", +) +driver = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(driver) + + +def test_base_dir_not_found_exits_2(capsys, tmp_path): + # --base-dir given but the OSISM repos aren't under it, no --remote-fallback. + rc = driver.main( + ["--base-dir", str(tmp_path), "--plugin", "kolla_enablement_orphan"] + ) + assert rc == 2 + assert "not found under any --base-dir" in capsys.readouterr().err diff --git a/tests/kolla_drift/test_enablement.py b/tests/kolla_drift/test_enablement.py new file mode 100644 index 0000000..c815b07 --- /dev/null +++ b/tests/kolla_drift/test_enablement.py @@ -0,0 +1,158 @@ +import pytest +import responses +from osism_drift.config import Config, Remote, SourceCfg +from osism_drift import enablement + + +def test_canon_normalizes_hyphen_to_underscore(): + assert enablement.canon("kolla-toolbox") == "kolla_toolbox" + assert enablement.canon("redis") == "redis" + + +def test_parse_enable_flags_strips_prefix(): + body = b'enable_redis: "yes"\nenable_heat: "no"\nother_var: 1\n' + assert enablement.parse_enable_flags(body) == {"redis": "yes", "heat": "no"} + + +def test_truthy_enables_literal_only_and_canon(): + flags = { + "redis": "yes", + "heat": "no", + "grafana": True, + "off2": False, + "senlin": "{{ enable_x | bool }}", + "multi_word": "yes", + } + assert enablement.truthy_enables(flags) == {"redis", "grafana", "multi_word"} + + +def test_parse_build_set_unions_both_blocks_canon(): + body = b""" +infrastructure_projects: + redis: + kolla-toolbox: +openstack_projects: + cinder: stable-2025.2 +""" + assert enablement.parse_build_set(body) == {"redis", "kolla_toolbox", "cinder"} + + +def _cfg_with_release_dir(tmp_path, releases=()): + # Isolated release dir per test — never the shared fixtures dir. + rel = tmp_path / "release" / "latest" + rel.mkdir(parents=True) + (rel / "openstack-2024.1.yml").write_text("openstack_projects: {cinder: x}\n") + (rel / "openstack-2025.2.yml").write_text("openstack_projects: {cinder: y}\n") + (rel / "openstack.yml").write_text("# alias file; must be ignored by the filter\n") + return Config( + remote=Remote("https://raw/", "https://api/", "main", "osism"), + base_dirs=(str(tmp_path),), + release_version="latest", + plugins={}, + sources={}, + releases=releases, + ) + + +def test_release_range_derives_from_file_set(tmp_path): + assert enablement.release_range(_cfg_with_release_dir(tmp_path)) == [ + "2024.1", + "2025.2", + ] + + +def test_release_range_override_wins(tmp_path): + cfg = _cfg_with_release_dir(tmp_path, releases=("2025.2",)) + assert enablement.release_range(cfg) == ["2025.2"] + + +def _ka_config(): + # kolla_ansible owner override; no local roots so every read is remote and + # responses-mockable. release_to_ref/read_at_ref/list_dir_at_ref are all + # always-remote regardless. + return Config( + remote=Remote( + "https://raw.githubusercontent.com/", + "https://api.github.com/repos/", + "main", + "osism", + ), + release_version="latest", + plugins={}, + sources={"kolla_ansible": SourceCfg(owner="openstack")}, + ) + + +@responses.activate +def test_upstream_enable_keys_monolithic(): + cfg = _ka_config() + responses.add( + responses.GET, + "https://api.github.com/repos/openstack/kolla-ansible/commits/stable/2025.1", + status=200, + ) + responses.add( + responses.GET, + "https://raw.githubusercontent.com/openstack/kolla-ansible/" + "stable/2025.1/ansible/group_vars/all.yml", + body=b'enable_redis: "no"\nenable_valkey: "no"\nother_var: 1\n', + status=200, + ) + assert enablement.upstream_enable_keys("2025.1", cfg) == {"redis", "valkey"} + + +@responses.activate +def test_upstream_enable_keys_split_dir(): + cfg = _ka_config() + responses.add( + responses.GET, + "https://api.github.com/repos/openstack/kolla-ansible/commits/stable/2025.2", + status=200, + ) + # monolithic absent -> 404 -> fall through to the split dir + responses.add( + responses.GET, + "https://raw.githubusercontent.com/openstack/kolla-ansible/" + "stable/2025.2/ansible/group_vars/all.yml", + status=404, + ) + responses.add( + responses.GET, + "https://api.github.com/repos/openstack/kolla-ansible/contents/ansible/group_vars/all", + json=[ + {"name": "valkey.yml", "type": "file"}, + {"name": "redis.yml", "type": "file"}, + {"name": "README.md", "type": "file"}, + ], + status=200, + ) + responses.add( + responses.GET, + "https://raw.githubusercontent.com/openstack/kolla-ansible/" + "stable/2025.2/ansible/group_vars/all/valkey.yml", + body=b'enable_valkey: "no"\n', + status=200, + ) + responses.add( + responses.GET, + "https://raw.githubusercontent.com/openstack/kolla-ansible/" + "stable/2025.2/ansible/group_vars/all/redis.yml", + body=b'enable_redis: "no"\n', + status=200, + ) + assert enablement.upstream_enable_keys("2025.2", cfg) == {"valkey", "redis"} + + +def test_osism_enable_ids_truthy(): + flags = {"redis": "yes", "off": "no", "grafana": True, "jinja": "{{ x | bool }}"} + assert enablement.osism_enable_ids(flags, "truthy") == {"redis", "grafana"} + + +def test_osism_enable_ids_explicit_includes_all_keys(): + flags = {"redis": "yes", "off": "no", "grafana": True} + assert enablement.osism_enable_ids(flags, "explicit") == {"redis", "off", "grafana"} + + +def test_osism_enable_ids_unknown_scope_raises(): + with pytest.raises(ValueError): + enablement.osism_enable_ids({}, "bogus") diff --git a/tests/kolla_drift/test_kolla_enablement_orphan.py b/tests/kolla_drift/test_kolla_enablement_orphan.py new file mode 100644 index 0000000..e391ade --- /dev/null +++ b/tests/kolla_drift/test_kolla_enablement_orphan.py @@ -0,0 +1,125 @@ +from pathlib import Path +import pytest +import responses +from osism_drift.config import ( + Config, + Remote, + PluginCfg, + SourceCfg, + Allowlist, + AllowEntry, +) +from osism_drift.drift import kolla_enablement_orphan as plugin + +FIXT = Path(__file__).parent / "fixtures" +API = "https://api.github.com/repos" +RAW = "https://raw.githubusercontent.com" + + +@pytest.fixture +def cfg(): + return Config( + remote=Remote(f"{RAW}/", f"{API}/", "main", "osism"), + base_dirs=(str(FIXT),), + remote_fallback=True, # FIXT/kolla-ansible is a plain dir, not a git repo -> remote + release_version="latest", + plugins={"kolla_enablement_orphan": PluginCfg(enabled=True)}, + sources={"kolla_ansible": SourceCfg(owner="openstack", branch="stable/2025.2")}, + releases=("A", "B"), + ) + + +def _mock_release(ref, defines): + # release_to_ref probes stable/ first (200), then read_at_ref the + # monolithic all.yml. Distinct refs -> distinct URLs, no ordering reliance. + body = ("".join(f'enable_{k}: "no"\n' for k in sorted(defines))).encode() + responses.add( + responses.GET, f"{API}/openstack/kolla-ansible/commits/{ref}", status=200 + ) + responses.add( + responses.GET, + f"{RAW}/openstack/kolla-ansible/{ref}/ansible/group_vars/all.yml", + body=body, + status=200, + ) + + +def _mock_upstream(defines): + # Same definitions at both releases A and B. + for ref in ("stable/A", "stable/B"): + _mock_release(ref, defines) + + +@responses.activate +def test_truthy_scope_flags_absent_truthy_services(cfg): + _mock_upstream({"foo", "bar"}) + drifts = plugin._run(cfg, Allowlist(()), "truthy") + assert sorted(d.image for d in drifts) == ["feat", "multi_word"] + assert all(not d.allowlisted for d in drifts) + + +@responses.activate +def test_explicit_scope_also_flags_dead_no_flags(cfg): + _mock_upstream({"foo", "bar"}) + drifts = plugin._run(cfg, Allowlist(()), "explicit") + assert sorted(d.image for d in drifts) == ["feat", "multi_word", "off"] + + +@responses.activate +def test_present_upstream_is_not_orphan(cfg): + # "multi-word" (hyphen) upstream must match the underscore OSISM id via canon. + _mock_upstream({"foo", "bar", "feat", "multi-word"}) + drifts = plugin._run(cfg, Allowlist(()), "truthy") + assert drifts == [] + + +@responses.activate +def test_union_across_releases_not_intersection(cfg): + # feat is defined upstream ONLY at release A, multi-word ONLY at B. The union + # over the range covers both, so neither is an orphan. This pins the union + # semantics: an impl that reads only one release, only the last, or the + # intersection would wrongly flag the one missing from its chosen release. + _mock_release("stable/A", {"foo", "bar", "feat"}) + _mock_release("stable/B", {"foo", "bar", "multi-word"}) + drifts = plugin._run(cfg, Allowlist(()), "truthy") + assert drifts == [] + + +@responses.activate +def test_allowlist_marks_allowlisted(cfg): + _mock_upstream({"foo", "bar"}) + al = Allowlist( + ( + AllowEntry( + plugin="kolla_enablement_orphan", image="feat", reason="OSISM-invented" + ), + ) + ) + drifts = plugin._run(cfg, al, "truthy") + feat = [d for d in drifts if d.image == "feat"][0] + assert feat.allowlisted is True + + +@responses.activate +def test_run_uses_explicit_default(cfg): + _mock_upstream({"foo", "bar"}) + drifts = plugin.run(cfg, Allowlist(())) # public run() -> SCOPE == "explicit" + # explicit scope also flags dead enable_X: "no" flags, so "off" is included. + assert sorted(d.image for d in drifts) == ["feat", "multi_word", "off"] + + +def test_empty_release_range_raises(tmp_path): + from osism_drift.source import SourceError + + empty = tmp_path / "release" / "latest" + empty.mkdir(parents=True) + c = Config( + remote=Remote(f"{RAW}/", f"{API}/", "main", "osism"), + base_dirs=(str(tmp_path), str(FIXT)), + release_version="latest", + plugins={}, + sources={}, + releases=(), + ) + with pytest.raises(SourceError, match="empty supported release range"): + plugin._run(c, Allowlist(()), "truthy") diff --git a/tests/kolla_drift/test_plugin_registry.py b/tests/kolla_drift/test_plugin_registry.py index 5eadafb..d73e7e7 100644 --- a/tests/kolla_drift/test_plugin_registry.py +++ b/tests/kolla_drift/test_plugin_registry.py @@ -23,6 +23,10 @@ def test_every_plugin_has_summary_and_remediation(): assert isinstance(p.REMEDIATION, str) and p.REMEDIATION.strip(), p.NAME +def test_kolla_enablement_orphan_plugin_registered(): + assert "kolla_enablement_orphan" in [p.NAME for p in PLUGINS] + + def test_default_config_enables_every_registered_plugin(): # The driver only runs a plugin that is present and enabled in the config, so # a plugin registered in PLUGINS but missing from the default config would From dcef683d6062214d1232f86ccafd8e89a52dc5cf Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 15:57:02 +0200 Subject: [PATCH 03/12] Add kolla_orphan_config drift plugin kolla_enablement_orphan flags the enable_ flag of a service upstream kolla-ansible no longer defines, but osism/defaults also carries that service's companion vars (e.g. senlin_api_port, outward_rabbitmq_port) and image-definition blocks (_tag, _*_image/_tag). Those are equally dead, yet nothing enumerated them, so a cleanup that deleted only the enable flag left them dangling. For each genuinely dead service (an orphan per kolla_enablement_orphan, excluding the OSISM-invented ones kept via that check's allowlist) report every top-level var across osism/defaults all/*.yml whose name is the service id or begins with "_", grouped by the file it lives in. The enable flag itself is left to kolla_enablement_orphan (it never matches the "_" prefix). Reuse kolla_enablement_orphan.orphan_ids() for the dead-service set and secrets_map.parse_secret_keys for the top-level key parse; match on the canon (hyphen/underscore) form. Register it second in the lifecycle PLUGINS order, right after kolla_enablement_orphan, and enable it in the default config. A registry test now guards that every registered plugin is enabled in the default config, so a plugin in PLUGINS but missing from the config can no longer silently never run. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Roger Luethi --- src/kolla-drift-config.yml | 1 + src/osism_drift/drift/__init__.py | 3 +- src/osism_drift/drift/kolla_orphan_config.py | 100 ++++++++++++++++ src/osism_drift/secrets_map.py | 18 +++ tests/kolla_drift/test_kolla_orphan_config.py | 110 ++++++++++++++++++ tests/kolla_drift/test_plugin_registry.py | 4 + tests/kolla_drift/test_secrets_map.py | 22 ++++ 7 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 src/osism_drift/drift/kolla_orphan_config.py create mode 100644 src/osism_drift/secrets_map.py create mode 100644 tests/kolla_drift/test_kolla_orphan_config.py create mode 100644 tests/kolla_drift/test_secrets_map.py diff --git a/src/kolla-drift-config.yml b/src/kolla-drift-config.yml index 9296add..a1acaef 100644 --- a/src/kolla-drift-config.yml +++ b/src/kolla-drift-config.yml @@ -16,3 +16,4 @@ release_version: latest plugins: kolla_enablement_orphan: {enabled: true} + kolla_orphan_config: {enabled: true} diff --git a/src/osism_drift/drift/__init__.py b/src/osism_drift/drift/__init__.py index 3dbfb7f..d01e836 100644 --- a/src/osism_drift/drift/__init__.py +++ b/src/osism_drift/drift/__init__.py @@ -1,5 +1,6 @@ """Plugin registry. Plugins are appended here as they are added.""" from osism_drift.drift import kolla_enablement_orphan +from osism_drift.drift import kolla_orphan_config -PLUGINS = [kolla_enablement_orphan] +PLUGINS = [kolla_enablement_orphan, kolla_orphan_config] diff --git a/src/osism_drift/drift/kolla_orphan_config.py b/src/osism_drift/drift/kolla_orphan_config.py new file mode 100644 index 0000000..0c264d7 --- /dev/null +++ b/src/osism_drift/drift/kolla_orphan_config.py @@ -0,0 +1,100 @@ +"""kolla_orphan_config: dead companion/image config for orphaned services. + +kolla_enablement_orphan flags the enable_X flag of a service upstream dropped, +but osism/defaults usually also carries that service's companion vars and image +definitions (X_*, e.g. senlin_api_port, senlin_api_image/_tag). Those are +equally dead and must be removed with the flag, yet nothing enumerated them, so +a cleanup that only deleted the enable flag left them dangling. + +This sweep closes that gap. For each genuinely dead service (an orphan per +kolla_enablement_orphan, excluding the OSISM-invented ones kept via that check's +allowlist) it reports every top-level var defined across osism/defaults +all/*.yml whose name is the service id or begins with "_". The +enable_X flag itself is left to kolla_enablement_orphan (it never matches the +"_" prefix). Keys are read with the shared top-level-key parser, not a +YAML load, and matched on the canon (hyphen/underscore) form. +""" + +from osism_drift import enablement, secrets_map, source +from osism_drift.drift import kolla_enablement_orphan +from osism_drift.model import DriftEntry + +NAME = "kolla_orphan_config" +DESCRIPTION = ( + "Flag dead companion/image config vars (_*) in osism/defaults for " + "services kolla_enablement_orphan reports as orphaned." +) +INPUT_FILES = [ + ("defaults", "all/*.yml"), + ("defaults", "all/099-kolla.yml (enable flags, via kolla_enablement_orphan)"), +] +SUMMARY = ( + "{n} dead companion/image config vars for services upstream removed (their " + "enable_ flag is reported separately by kolla_enablement_orphan); " + "these must be removed too:" +) +REMEDIATION = ( + "remove these vars from the listed osism/defaults file, or allowlist any " + "that are intentionally kept (an OSISM invention with no upstream service)." +) + +_DEFAULTS_DIR = "all" + + +def _owning_service(var: str, dead: set) -> str | None: + """The dead service that owns `var` (var == sid or var starts sid+'_'), + longest match wins; None if no dead service owns it.""" + v = enablement.canon(var) + owners = [ + sid for sid in dead if v == sid or v.startswith(f"{enablement.canon(sid)}_") + ] + return max(owners, key=len) if owners else None + + +def run(config, allowlist, verbose: bool = False) -> list[DriftEntry]: + """Return dead-config drifts: _* vars of orphaned services.""" + # Genuinely dead services: orphaned AND not kept via the orphan allowlist + # (so OSISM inventions like common/kolla_operations are excluded). + dead = set() + for sid in kolla_enablement_orphan.orphan_ids(config): + probe = DriftEntry( + plugin=kolla_enablement_orphan.NAME, + image=sid, + alias=sid, + expected="", + found="", + expected_src="", + found_src="", + ) + if allowlist.match(probe) is None: + dead.add(sid) + if not dead: + return [] + + # Every top-level var across defaults all/*.yml, remembering its file. + var_file = {} + for fn in sorted(source.list_dir("defaults", _DEFAULTS_DIR, config)): + if not fn.endswith(".yml"): + continue + body = source.read("defaults", f"{_DEFAULTS_DIR}/{fn}", config) + for key in secrets_map.parse_secret_keys(body): + var_file.setdefault(key, f"{_DEFAULTS_DIR}/{fn}") + + drifts = [] + for var in sorted(var_file): + if var.startswith("enable_"): + continue # enable flags belong to kolla_enablement_orphan + owner = _owning_service(var, dead) + if owner is None: + continue + d = DriftEntry( + plugin=NAME, + image=var, + alias=var, + expected=f"removed with the orphaned service '{owner}'", + found=f"dead companion/image config in {var_file[var]}", + expected_src="openstack/kolla-ansible (service removed upstream)", + found_src=f"osism/defaults {var_file[var]}", + ) + drifts.append(allowlist.apply(d)) + return drifts diff --git a/src/osism_drift/secrets_map.py b/src/osism_drift/secrets_map.py new file mode 100644 index 0000000..f9f9d3c --- /dev/null +++ b/src/osism_drift/secrets_map.py @@ -0,0 +1,18 @@ +"""Parser for the top-level key set of an OSISM secrets or kolla passwords file. + +Both osism cfg-cookiecutter secrets templates and kolla-ansible's +etc/kolla/passwords.yml are flat YAML mappings of var names to (often +jinja/templated) values. Only the var names matter for the orphan comparison, +and the values may not be valid YAML (cookiecutter/jinja placeholders), so the +keys are extracted by line regex rather than a YAML load. Top-level keys only: +an indented (nested) line is not a secret var name. +""" + +import re + +_KEY = re.compile(rb"^([A-Za-z_][A-Za-z0-9_]*)[ \t]*:", re.MULTILINE) + + +def parse_secret_keys(body: bytes) -> set[str]: + """Return the set of top-level YAML keys (secret/password var names).""" + return {m.decode() for m in _KEY.findall(body)} diff --git a/tests/kolla_drift/test_kolla_orphan_config.py b/tests/kolla_drift/test_kolla_orphan_config.py new file mode 100644 index 0000000..5f1a60d --- /dev/null +++ b/tests/kolla_drift/test_kolla_orphan_config.py @@ -0,0 +1,110 @@ +from pathlib import Path +import pytest +import responses +from osism_drift.config import ( + Config, + Remote, + PluginCfg, + SourceCfg, + Allowlist, + AllowEntry, +) +from osism_drift.drift import kolla_orphan_config as plugin + +FIXT = Path(__file__).parent / "fixtures" +API = "https://api.github.com/repos" +RAW = "https://raw.githubusercontent.com" + + +@pytest.fixture +def cfg(): + return Config( + remote=Remote(f"{RAW}/", f"{API}/", "main", "osism"), + base_dirs=(str(FIXT),), + remote_fallback=True, # FIXT/kolla-ansible is a plain dir -> remote (mocked) + release_version="latest", + plugins={"kolla_orphan_config": PluginCfg(enabled=True)}, + sources={"kolla_ansible": SourceCfg(owner="openstack", branch="stable/2025.2")}, + releases=("A", "B"), + ) + + +def _mock_upstream(defines): + # upstream defines these enable_* at both releases; orphan_ids = osism - upstream. + body = ("".join(f'enable_{k}: "no"\n' for k in sorted(defines))).encode() + for ref in ("stable/A", "stable/B"): + responses.add( + responses.GET, f"{API}/openstack/kolla-ansible/commits/{ref}", status=200 + ) + responses.add( + responses.GET, + f"{RAW}/openstack/kolla-ansible/{ref}/ansible/group_vars/all.yml", + body=body, + status=200, + ) + + +@responses.activate +def test_flags_dead_service_companion_vars(cfg): + # upstream defines only foo/bar -> dead = {feat, off, multi_word}. Only feat + # has companion/image vars in the fixtures. + _mock_upstream({"foo", "bar"}) + drifts = plugin.run(cfg, Allowlist(())) + assert sorted(d.image for d in drifts) == [ + "feat_api_image", + "feat_api_port", + "feat_internal_fqdn", + ] + # the live var and the enable flags are never swept + images = [d.image for d in drifts] + assert "live_image" not in images + assert not any(i.startswith("enable_") for i in images) + + +@responses.activate +def test_found_src_points_to_each_file(cfg): + _mock_upstream({"foo", "bar"}) + by = {d.image: d for d in plugin.run(cfg, Allowlist(()))} + assert "all/050-images.yml" in by["feat_api_image"].found_src + assert "all/099-kolla.yml" in by["feat_api_port"].found_src + + +@responses.activate +def test_no_dead_services_returns_empty(cfg): + # upstream defines everything OSISM enables -> nothing orphaned -> no sweep. + _mock_upstream({"foo", "bar", "feat", "off", "multi-word"}) + assert plugin.run(cfg, Allowlist(())) == [] + + +@responses.activate +def test_respects_orphan_allowlist(cfg): + # Allowlisting feat as an orphan (OSISM invention) excludes it from the dead + # set, so its companion vars are not swept either. + _mock_upstream({"foo", "bar"}) + al = Allowlist( + ( + AllowEntry( + plugin="kolla_enablement_orphan", + image="feat", + reason="OSISM-invented", + ), + ) + ) + assert plugin.run(cfg, al) == [] + + +@responses.activate +def test_own_allowlist_marks_allowlisted(cfg): + _mock_upstream({"foo", "bar"}) + al = Allowlist( + ( + AllowEntry( + plugin="kolla_orphan_config", + image="feat_api_port", + reason="kept intentionally", + ), + ) + ) + by = {d.image: d for d in plugin.run(cfg, al)} + assert by["feat_api_port"].allowlisted is True + assert by["feat_internal_fqdn"].allowlisted is False diff --git a/tests/kolla_drift/test_plugin_registry.py b/tests/kolla_drift/test_plugin_registry.py index d73e7e7..b29e4de 100644 --- a/tests/kolla_drift/test_plugin_registry.py +++ b/tests/kolla_drift/test_plugin_registry.py @@ -27,6 +27,10 @@ def test_kolla_enablement_orphan_plugin_registered(): assert "kolla_enablement_orphan" in [p.NAME for p in PLUGINS] +def test_kolla_orphan_config_plugin_registered(): + assert "kolla_orphan_config" in [p.NAME for p in PLUGINS] + + def test_default_config_enables_every_registered_plugin(): # The driver only runs a plugin that is present and enabled in the config, so # a plugin registered in PLUGINS but missing from the default config would diff --git a/tests/kolla_drift/test_secrets_map.py b/tests/kolla_drift/test_secrets_map.py new file mode 100644 index 0000000..b9c7b1c --- /dev/null +++ b/tests/kolla_drift/test_secrets_map.py @@ -0,0 +1,22 @@ +from osism_drift.secrets_map import parse_secret_keys + + +def test_extracts_top_level_keys(): + body = b"---\n# comment\nkeystone_password:\nrabbitmq_password: secret\n" + assert parse_secret_keys(body) == {"keystone_password", "rabbitmq_password"} + + +def test_ignores_comments_indented_and_markers(): + body = ( + b"---\n" + b"# heading\n" + b"outer_password:\n" + b" nested_key: x\n" # indented -> not a top-level var + b"\n" + b"other_password: '{{ lookup() }}'\n" # jinja value, key still parsed + ) + assert parse_secret_keys(body) == {"outer_password", "other_password"} + + +def test_empty_input(): + assert parse_secret_keys(b"") == set() From 626e2affa93ee69e7b871460abc5f5a7522cfd14 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 16:01:43 +0200 Subject: [PATCH 04/12] Add kolla_secrets_orphan drift plugin Detect OSISM kolla secrets with no upstream counterpart. The enablement and version-chain checks already reconcile OSISM enable flags and version pins against upstream, but stale secret vars left behind by removed services (e.g. outward_rabbitmq_password) went undetected. For each supported release the plugin compares the top-level keys of the cfg-cookiecutter kolla secrets template (environments/kolla/secrets.yml.) against kolla-ansible's authoritative etc/kolla/passwords.yml at that release's resolved ref. A key present in the OSISM template but absent upstream is an orphaned secret. Both sides are release-specific, so the comparison is per release, not unioned. Keys are extracted with secrets_map.parse_secret_keys (a line regex, not a YAML load, because the template values are jinja/cookiecutter placeholders that do not parse as YAML); canon normalises the hyphen/underscore split. OSISM-invented secrets with no upstream counterpart are kept via the allowlist; a release with no OSISM template is skipped, not an error. Register it next to the sibling kolla_enablement_orphan and enable it in the default config. The current real-world baseline is clean across 2024.1-2025.2, so it serves as a regression gate. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Roger Luethi --- src/kolla-drift-config.yml | 1 + src/osism_drift/drift/__init__.py | 3 +- src/osism_drift/drift/kolla_secrets_orphan.py | 84 ++++++++++++++++ .../kolla_drift/test_kolla_secrets_orphan.py | 97 +++++++++++++++++++ tests/kolla_drift/test_plugin_registry.py | 4 + 5 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 src/osism_drift/drift/kolla_secrets_orphan.py create mode 100644 tests/kolla_drift/test_kolla_secrets_orphan.py diff --git a/src/kolla-drift-config.yml b/src/kolla-drift-config.yml index a1acaef..745d3cc 100644 --- a/src/kolla-drift-config.yml +++ b/src/kolla-drift-config.yml @@ -17,3 +17,4 @@ release_version: latest plugins: kolla_enablement_orphan: {enabled: true} kolla_orphan_config: {enabled: true} + kolla_secrets_orphan: {enabled: true} diff --git a/src/osism_drift/drift/__init__.py b/src/osism_drift/drift/__init__.py index d01e836..980e160 100644 --- a/src/osism_drift/drift/__init__.py +++ b/src/osism_drift/drift/__init__.py @@ -2,5 +2,6 @@ from osism_drift.drift import kolla_enablement_orphan from osism_drift.drift import kolla_orphan_config +from osism_drift.drift import kolla_secrets_orphan -PLUGINS = [kolla_enablement_orphan, kolla_orphan_config] +PLUGINS = [kolla_enablement_orphan, kolla_orphan_config, kolla_secrets_orphan] diff --git a/src/osism_drift/drift/kolla_secrets_orphan.py b/src/osism_drift/drift/kolla_secrets_orphan.py new file mode 100644 index 0000000..763a227 --- /dev/null +++ b/src/osism_drift/drift/kolla_secrets_orphan.py @@ -0,0 +1,84 @@ +"""kolla_secrets_orphan: OSISM ships a secret upstream no longer expects. + +For each supported release R, an OSISM kolla secret is an orphan when it has no +counterpart in upstream kolla-ansible's authoritative secret list: + + - OSISM set -- top-level keys of the cfg-cookiecutter kolla secrets template + for R (environments/kolla/secrets.yml.R) + - upstream -- top-level keys of kolla-ansible etc/kolla/passwords.yml at R's + resolved ref (the canonical list of every secret it expects) + +A key in the OSISM template but absent upstream is a leftover from a removed +service (e.g. outward_rabbitmq_password). Both sides are release-specific, so the +comparison is per release, not unioned. + +Mirrors kolla_enablement_orphan in shape (OSISM-vs-upstream reconciliation) but +on the secret key space: keys come from secrets_map.parse_secret_keys (line +regex, not a YAML load, because the template values are jinja/cookiecutter +placeholders). canon normalises hyphen/underscore. OSISM-invented secrets with +no upstream counterpart are kept via the allowlist rather than removed. +""" + +from osism_drift import enablement, secrets_map, source +from osism_drift.model import DriftEntry + +NAME = "kolla_secrets_orphan" +DESCRIPTION = ( + "Flag OSISM kolla secret vars absent from upstream kolla-ansible " + "passwords.yml at a supported release (orphaned secrets)." +) +INPUT_FILES = [ + ("cfg_cookiecutter", "environments/kolla/secrets.yml."), + ("kolla_ansible", "etc/kolla/passwords.yml (per resolved release ref)"), +] +SUMMARY = ( + "{n} OSISM kolla secrets defined for this release with no counterpart in " + "upstream kolla-ansible's passwords.yml, so the secret is orphaned (the " + "service that consumed it was removed upstream):" +) +REMEDIATION = ( + "remove the orphaned secret from the cfg-cookiecutter kolla secrets template " + "(and regenerate environments), or allowlist it if it is an OSISM-invented " + "secret with no upstream counterpart." +) + +_SECRETS = "{{cookiecutter.project_name}}/environments/kolla/secrets.yml" +_PASSWORDS = "etc/kolla/passwords.yml" + + +def run(config, allowlist, verbose: bool = False) -> list[DriftEntry]: + """Return orphan-secret drifts: OSISM ships a secret upstream dropped.""" + drifts = [] + for release in enablement.release_range(config): + osism_body = source.read_optional( + "cfg_cookiecutter", f"{_SECRETS}.{release}", config + ) + if osism_body is None: + continue # no OSISM kolla secrets template for this release + osism_keys = { + enablement.canon(k) for k in secrets_map.parse_secret_keys(osism_body) + } + + ref = source.release_to_ref("kolla_ansible", release, config) + upstream_keys = { + enablement.canon(k) + for k in secrets_map.parse_secret_keys( + source.read_at_ref("kolla_ansible", _PASSWORDS, ref, config) + ) + } + + for key in sorted(osism_keys - upstream_keys): + d = DriftEntry( + plugin=NAME, + image=key, + alias=key, + expected=f"present in upstream passwords.yml at {ref}", + found=( + "absent upstream; orphaned in cfg-cookiecutter " + f"environments/kolla/secrets.yml.{release}" + ), + expected_src=f"openstack/kolla-ansible {_PASSWORDS} @ {ref}", + found_src=f"cfg-cookiecutter environments/kolla/secrets.yml.{release}", + ) + drifts.append(allowlist.apply(d)) + return drifts diff --git a/tests/kolla_drift/test_kolla_secrets_orphan.py b/tests/kolla_drift/test_kolla_secrets_orphan.py new file mode 100644 index 0000000..f2c44ec --- /dev/null +++ b/tests/kolla_drift/test_kolla_secrets_orphan.py @@ -0,0 +1,97 @@ +import dataclasses +from pathlib import Path +import pytest +import responses +from osism_drift.config import ( + Config, + Remote, + PluginCfg, + SourceCfg, + Allowlist, + AllowEntry, +) +from osism_drift.drift import kolla_secrets_orphan as plugin + +FIXT = Path(__file__).parent / "fixtures" +API = "https://api.github.com/repos" +RAW = "https://raw.githubusercontent.com" + + +@pytest.fixture +def cfg(): + return Config( + remote=Remote(f"{RAW}/", f"{API}/", "main", "osism"), + base_dirs=(str(FIXT),), + remote_fallback=True, # FIXT/kolla-ansible is a plain dir, not a git repo -> remote + release_version="latest", + plugins={"kolla_secrets_orphan": PluginCfg(enabled=True)}, + sources={"kolla_ansible": SourceCfg(owner="openstack", branch="stable/2025.2")}, + releases=("A", "B"), + ) + + +def _mock_passwords(ref, keys): + # release_to_ref probes stable/ first (200), then read_at_ref the + # passwords.yml at that ref. Distinct refs -> distinct URLs. + body = ("".join(f"{k}:\n" for k in sorted(keys))).encode() + responses.add( + responses.GET, f"{API}/openstack/kolla-ansible/commits/{ref}", status=200 + ) + responses.add( + responses.GET, + f"{RAW}/openstack/kolla-ansible/{ref}/etc/kolla/passwords.yml", + body=body, + status=200, + ) + + +@responses.activate +def test_flags_orphan_secrets_per_release(cfg): + # fixtures: A secrets {keystone, foo, orphan_a}, B secrets {keystone, orphan_b} + _mock_passwords("stable/A", {"keystone_password", "foo_password"}) + _mock_passwords("stable/B", {"keystone_password"}) + drifts = plugin.run(cfg, Allowlist(())) + assert sorted(d.image for d in drifts) == ["orphan_a_password", "orphan_b_password"] + by = {d.image: d for d in drifts} + assert "secrets.yml.A" in by["orphan_a_password"].found_src + assert "passwords.yml" in by["orphan_a_password"].expected_src + assert all(not d.allowlisted for d in drifts) + + +@responses.activate +def test_present_secret_is_not_orphan(cfg): + # keystone_password exists upstream at both releases -> never flagged. + _mock_passwords("stable/A", {"keystone_password", "foo_password"}) + _mock_passwords("stable/B", {"keystone_password"}) + drifts = plugin.run(cfg, Allowlist(())) + assert all(d.image != "keystone_password" for d in drifts) + + +@responses.activate +def test_allowlist_marks_allowlisted(cfg): + _mock_passwords("stable/A", {"keystone_password", "foo_password"}) + _mock_passwords("stable/B", {"keystone_password"}) + al = Allowlist( + ( + AllowEntry( + plugin="kolla_secrets_orphan", + image="orphan_a_password", + reason="OSISM-invented secret", + ), + ) + ) + drifts = plugin.run(cfg, al) + a = [d for d in drifts if d.image == "orphan_a_password"][0] + assert a.allowlisted is True + assert a.reason == "OSISM-invented secret" + + +@responses.activate +def test_missing_template_skips_release(cfg): + # A release with no cfg-cookiecutter secrets template is skipped, not an error. + c = dataclasses.replace(cfg, releases=("A", "B", "Z")) # Z has no fixture template + _mock_passwords("stable/A", {"keystone_password", "foo_password"}) + _mock_passwords("stable/B", {"keystone_password"}) + drifts = plugin.run(c, Allowlist(())) + # Z contributes nothing; A and B behave as before. + assert sorted(d.image for d in drifts) == ["orphan_a_password", "orphan_b_password"] diff --git a/tests/kolla_drift/test_plugin_registry.py b/tests/kolla_drift/test_plugin_registry.py index b29e4de..803ff1e 100644 --- a/tests/kolla_drift/test_plugin_registry.py +++ b/tests/kolla_drift/test_plugin_registry.py @@ -31,6 +31,10 @@ def test_kolla_orphan_config_plugin_registered(): assert "kolla_orphan_config" in [p.NAME for p in PLUGINS] +def test_kolla_secrets_orphan_plugin_registered(): + assert "kolla_secrets_orphan" in [p.NAME for p in PLUGINS] + + def test_default_config_enables_every_registered_plugin(): # The driver only runs a plugin that is present and enabled in the config, so # a plugin registered in PLUGINS but missing from the default config would From be942e1636db66eb44219a337a37044c22b1c4a1 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 16:06:58 +0200 Subject: [PATCH 05/12] Add kolla_enablement_build drift plugin Flag OSISM-enabled kolla services (defaults all/099-kolla.yml enable_X truthy) that are buildable upstream at a supported release (a docker/X dir exists in openstack/kolla at the release's resolved ref) but absent from that release's OSISM build set (release latest/openstack-R.yml infrastructure_projects / openstack_projects). The docker/ universe is the scope filter, so feature flags are out of scope with no alias table. Range-aware via release_range + release_to_ref. Wire the defaults and release paths and enable the plugin. The live baseline is clean today (enable_valkey is not yet in defaults); the post-valkey gap is exercised by the synthetic fixtures. Add a per-plugin SUMMARY/REMEDIATION so the report names the file an operator must edit. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Roger Luethi --- src/kolla-drift-config.yml | 1 + src/osism_drift/drift/__init__.py | 3 +- .../drift/kolla_enablement_build.py | 78 +++++++++++++++++++ .../test_kolla_enablement_build.py | 74 ++++++++++++++++++ tests/kolla_drift/test_plugin_registry.py | 4 + 5 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 src/osism_drift/drift/kolla_enablement_build.py create mode 100644 tests/kolla_drift/test_kolla_enablement_build.py diff --git a/src/kolla-drift-config.yml b/src/kolla-drift-config.yml index 745d3cc..a73b951 100644 --- a/src/kolla-drift-config.yml +++ b/src/kolla-drift-config.yml @@ -18,3 +18,4 @@ plugins: kolla_enablement_orphan: {enabled: true} kolla_orphan_config: {enabled: true} kolla_secrets_orphan: {enabled: true} + kolla_enablement_build: {enabled: true} diff --git a/src/osism_drift/drift/__init__.py b/src/osism_drift/drift/__init__.py index 980e160..3bd4dbe 100644 --- a/src/osism_drift/drift/__init__.py +++ b/src/osism_drift/drift/__init__.py @@ -3,5 +3,6 @@ from osism_drift.drift import kolla_enablement_orphan from osism_drift.drift import kolla_orphan_config from osism_drift.drift import kolla_secrets_orphan +from osism_drift.drift import kolla_enablement_build -PLUGINS = [kolla_enablement_orphan, kolla_orphan_config, kolla_secrets_orphan] +PLUGINS = [kolla_enablement_orphan, kolla_orphan_config, kolla_secrets_orphan, kolla_enablement_build] diff --git a/src/osism_drift/drift/kolla_enablement_build.py b/src/osism_drift/drift/kolla_enablement_build.py new file mode 100644 index 0000000..25a7168 --- /dev/null +++ b/src/osism_drift/drift/kolla_enablement_build.py @@ -0,0 +1,78 @@ +"""kolla_enablement_build: enabled service not built for a release. + +For each supported release R, an OSISM-enabled service is a build gap when it is +buildable upstream at R yet missing from OSISM's build set for R: + + - enabled -- enable_X is truthy in defaults all/099-kolla.yml + - buildable -- a docker/X dir exists in openstack/kolla at R's resolved ref + - built -- X appears in release latest/openstack-R.yml under + infrastructure_projects or openstack_projects + +The upstream docker/ universe is the scope filter, so feature flags (which have +no image) are out of scope and no alias table is needed. + +Range-aware: the upstream ref per release comes from source.release_to_ref. The +remote is consulted only for the docker/ listing and the ref probe; defaults and +release are read at their pins. +""" + +from osism_drift import enablement, source +from osism_drift.model import DriftEntry + +NAME = "kolla_enablement_build" +DESCRIPTION = ( + "Flag OSISM-enabled kolla services that are buildable upstream at a " + "supported release but absent from that release's OSISM build set." +) +INPUT_FILES = [ + ("defaults", "all/099-kolla.yml"), + ("release", "latest/openstack-.yml"), + ("kolla", "docker/ (per resolved release ref)"), +] +SUMMARY = ( + "{n} services enabled in OSISM and buildable upstream at this release, but " + "absent from the release's OSISM build set, so no image is built for them:" +) +REMEDIATION = ( + "add the service to infrastructure_projects or openstack_projects in the " + "release file, or allowlist it if it is intentionally not built." +) + +_ENABLE = "all/099-kolla.yml" + + +def run(config, allowlist, verbose: bool = False) -> list[DriftEntry]: + """Return build-gap drifts: enabled and upstream-buildable but not built.""" + enabled = enablement.truthy_enables( + enablement.parse_enable_flags(source.read("defaults", _ENABLE, config)) + ) + drifts = [] + for release in enablement.release_range(config): + ref = source.release_to_ref("kolla", release, config) + buildable = { + enablement.canon(d) + for d in source.list_dir_at_ref( + "kolla", "docker", ref, config, dirs_only=True + ) + } + built = enablement.parse_build_set( + source.read("release", f"latest/openstack-{release}.yml", config) + ) + for svc in sorted((enabled & buildable) - built): + d = DriftEntry( + plugin=NAME, + image=svc, + alias=svc, + expected=( + f"buildable upstream at {ref} (docker/{svc}) and enabled in OSISM; " + f"expected in release/latest/openstack-{release}.yml build set" + ), + found=( + "absent from infrastructure_projects/openstack_projects " + f"@ openstack-{release}.yml" + ), + expected_src=f"openstack/kolla docker/ @ {ref}", + found_src=f"osism/release latest/openstack-{release}.yml", + ) + drifts.append(allowlist.apply(d)) + return drifts diff --git a/tests/kolla_drift/test_kolla_enablement_build.py b/tests/kolla_drift/test_kolla_enablement_build.py new file mode 100644 index 0000000..d0caffe --- /dev/null +++ b/tests/kolla_drift/test_kolla_enablement_build.py @@ -0,0 +1,74 @@ +from pathlib import Path +import pytest +import responses +from osism_drift.config import ( + Config, + Remote, + PluginCfg, + SourceCfg, + Allowlist, + AllowEntry, +) +from osism_drift.drift import kolla_enablement_build as plugin + +FIXT = Path(__file__).parent / "fixtures" +API = "https://api.github.com/repos" + + +@pytest.fixture +def cfg(): + return Config( + remote=Remote("https://raw.githubusercontent.com/", f"{API}/", "main", "osism"), + base_dirs=(str(FIXT),), + remote_fallback=True, # FIXT/kolla is a plain dir, not a git repo -> remote + release_version="latest", + plugins={"kolla_enablement_build": PluginCfg(enabled=True)}, + sources={"kolla": SourceCfg(owner="openstack", branch="stable/2025.2")}, + releases=("A", "B"), + ) + + +def _mock_commit(ref): + responses.add(responses.GET, f"{API}/openstack/kolla/commits/{ref}", status=200) + + +def _mock_docker(dirs): + # responses serves duplicate-URL registrations in registration order, and + # ignores the ?ref= query string when matching. release_range is ["A","B"], + # so the first docker listing is consumed by release A, the second by B. + responses.add( + responses.GET, + f"{API}/openstack/kolla/contents/docker", + json=[{"name": d, "type": "dir"} for d in dirs], + status=200, + ) + + +def _mock_all(): + _mock_commit("stable/A") + _mock_commit("stable/B") + _mock_docker(["foo", "bar", "multi-word"]) # release A + _mock_docker(["foo", "bar", "baz", "multi-word"]) # release B + + +@responses.activate +def test_flags_enabled_buildable_not_built(cfg): + _mock_all() + drifts = plugin.run(cfg, Allowlist(())) + assert [(d.image, d.found_src) for d in drifts] == [ + ("foo", "osism/release latest/openstack-A.yml") + ] + + +@responses.activate +def test_allowlist_marks_allowlisted(cfg): + _mock_all() + al = Allowlist( + ( + AllowEntry( + plugin="kolla_enablement_build", image="foo", reason="tracked in #289" + ), + ) + ) + drifts = plugin.run(cfg, al) + assert len(drifts) == 1 and drifts[0].allowlisted is True diff --git a/tests/kolla_drift/test_plugin_registry.py b/tests/kolla_drift/test_plugin_registry.py index 803ff1e..2648a72 100644 --- a/tests/kolla_drift/test_plugin_registry.py +++ b/tests/kolla_drift/test_plugin_registry.py @@ -35,6 +35,10 @@ def test_kolla_secrets_orphan_plugin_registered(): assert "kolla_secrets_orphan" in [p.NAME for p in PLUGINS] +def test_kolla_enablement_build_plugin_registered(): + assert "kolla_enablement_build" in [p.NAME for p in PLUGINS] + + def test_default_config_enables_every_registered_plugin(): # The driver only runs a plugin that is present and enabled in the config, so # a plugin registered in PLUGINS but missing from the default config would From e791a55006ee8fb532c55421dee3f068a9d26976 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 16:13:20 +0200 Subject: [PATCH 06/12] Add kolla_version_chain_upstream plugin Flag openstack/kolla docker/ services (pinned stable/2025.2) that have no versions['K'] line in the kolla-ansible template -- unwired pins that silently default. One-way: producer keys are not folded in, so blazar and masakari (present in the producer, absent from the template) still flag. Pin kolla to openstack/stable/2025.2 via sources, add the docker/ service parser and the versions.yml.j2 template parser, enable the plugin, and seed the allowlist with the ignore-tier services from the empirical stable/2025.2-vs-template delta: not-deployed services (cyborg, tacker, telegraf, zun -- zun has a kolla docker image but OSISM does not build or deploy it), build base layers (base, openstack_base), and naming/variant or supporting images with no distinct versions['K'] key (hacluster, letsencrypt, networking_baremetal, ovsdpdk, httpd, mariadb_server). Every seed entry maps to a real delta item so none is stale; valkey/blazar/ masakari are intentionally left to flag. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Roger Luethi --- src/kolla-drift-allowlist.yml | 15 ++++ src/kolla-drift-config.yml | 1 + src/osism_drift/drift/__init__.py | 3 +- .../drift/kolla_version_chain_upstream.py | 68 +++++++++++++++++++ src/osism_drift/kolla_docker.py | 13 ++++ src/osism_drift/versions_template.py | 10 +++ tests/kolla_drift/test_kolla_docker.py | 13 ++++ .../test_kolla_version_chain_upstream.py | 46 +++++++++++++ tests/kolla_drift/test_plugin_registry.py | 4 ++ tests/kolla_drift/test_versions_template.py | 18 +++++ 10 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 src/osism_drift/drift/kolla_version_chain_upstream.py create mode 100644 src/osism_drift/kolla_docker.py create mode 100644 src/osism_drift/versions_template.py create mode 100644 tests/kolla_drift/test_kolla_docker.py create mode 100644 tests/kolla_drift/test_kolla_version_chain_upstream.py create mode 100644 tests/kolla_drift/test_versions_template.py diff --git a/src/kolla-drift-allowlist.yml b/src/kolla-drift-allowlist.yml index a691ecb..22a6e7c 100644 --- a/src/kolla-drift-allowlist.yml +++ b/src/kolla-drift-allowlist.yml @@ -1,6 +1,21 @@ # Known-intentional exceptions. Each entry needs a non-empty `reason`. # An entry that suppresses no real drift is reported as stale (a hard error). allow: + # Not deployed in this OSISM series (cf. inventory IGNORE_NOT_DEPLOYED). + - {plugin: kolla_version_chain_upstream, image: cyborg, reason: "not deployed in this series"} + - {plugin: kolla_version_chain_upstream, image: tacker, reason: "not deployed in this series"} + - {plugin: kolla_version_chain_upstream, image: telegraf, reason: "not deployed in this series"} + - {plugin: kolla_version_chain_upstream, image: zun, reason: "not deployed in this series"} + # Build base layer, not a deployable service. + - {plugin: kolla_version_chain_upstream, image: base, reason: "build base layer, not a service"} + - {plugin: kolla_version_chain_upstream, image: openstack_base, reason: "build base layer, not a service"} + # Naming/variant or supporting image, no distinct versions['K'] key. + - {plugin: kolla_version_chain_upstream, image: hacluster, reason: "pacemaker/corosync variant, no distinct version key"} + - {plugin: kolla_version_chain_upstream, image: letsencrypt, reason: "wired via versions['letsencrypt_lego'] / ['letsencrypt_webserver']"} + - {plugin: kolla_version_chain_upstream, image: networking_baremetal, reason: "neutron plugin image, no distinct version key"} + - {plugin: kolla_version_chain_upstream, image: ovsdpdk, reason: "openvswitch dpdk variant, no distinct version key"} + - {plugin: kolla_version_chain_upstream, image: httpd, reason: "apache supporting image, not independently pinned"} + - {plugin: kolla_version_chain_upstream, image: mariadb_server, reason: "upstream dir mariadb-server; wired via versions['mariadb']"} # --- kolla_enablement_orphan --- # OSISM-invented enable flags with no upstream counterpart by design (not a # removed-upstream service). Truthy, so they surface in the truthy-only scope. diff --git a/src/kolla-drift-config.yml b/src/kolla-drift-config.yml index a73b951..b88a685 100644 --- a/src/kolla-drift-config.yml +++ b/src/kolla-drift-config.yml @@ -19,3 +19,4 @@ plugins: kolla_orphan_config: {enabled: true} kolla_secrets_orphan: {enabled: true} kolla_enablement_build: {enabled: true} + kolla_version_chain_upstream: {enabled: true} diff --git a/src/osism_drift/drift/__init__.py b/src/osism_drift/drift/__init__.py index 3bd4dbe..115f2ea 100644 --- a/src/osism_drift/drift/__init__.py +++ b/src/osism_drift/drift/__init__.py @@ -4,5 +4,6 @@ from osism_drift.drift import kolla_orphan_config from osism_drift.drift import kolla_secrets_orphan from osism_drift.drift import kolla_enablement_build +from osism_drift.drift import kolla_version_chain_upstream -PLUGINS = [kolla_enablement_orphan, kolla_orphan_config, kolla_secrets_orphan, kolla_enablement_build] +PLUGINS = [kolla_enablement_orphan, kolla_orphan_config, kolla_secrets_orphan, kolla_enablement_build, kolla_version_chain_upstream] diff --git a/src/osism_drift/drift/kolla_version_chain_upstream.py b/src/osism_drift/drift/kolla_version_chain_upstream.py new file mode 100644 index 0000000..621310e --- /dev/null +++ b/src/osism_drift/drift/kolla_version_chain_upstream.py @@ -0,0 +1,68 @@ +"""kolla_version_chain_upstream: upstream services with no template pin. + +Compares the top-level docker/ service dirs of openstack/kolla (pinned to +stable/2025.2) against the versions['K'] keys in the kolla-ansible template. A +service present upstream but absent from the template key space has no version +pin wired (e.g. valkey, blazar, masakari) and its line would silently default. + +The comparison is one-way (upstream -> template): producer keys are not folded +in, so a service present in the producer yet absent from the template still +flags here. Hyphen/underscore is normalised on the key space only. +""" + +from osism_drift import kolla_docker, source, versions_template +from osism_drift.model import DriftEntry + +NAME = "kolla_version_chain_upstream" +DESCRIPTION = ( + "Flag openstack/kolla docker/ services (stable/2025.2) with no " + "versions['K'] line in the kolla-ansible template (unwired pins)." +) +INPUT_FILES = [ + ("kolla", "docker"), + ("container_image_kolla_ansible", "files/src/templates/versions.yml.j2"), +] +SUMMARY = ( + "{n} services that ship a docker/ image in openstack/kolla but have no " + "versions[''] line in the kolla-ansible template, so their image tag " + "is never pinned:" +) +REMEDIATION = ( + "add a versions[''] line to the template to pin the image, or " + "allowlist the name if it is intentionally unpinned." +) + +_DOCKER = "docker" +_TEMPLATE = "files/src/templates/versions.yml.j2" +_EXPECTED_SRC = "openstack/kolla/docker/ @ stable/2025.2" +_FOUND_SRC = "container-image-kolla-ansible/files/src/templates/versions.yml.j2" + + +def _norm(key: str) -> str: + return key.replace("-", "_") + + +def run(config, allowlist, verbose: bool = False) -> list[DriftEntry]: + """Return unwired-pin drifts: upstream services with no template key.""" + docker_names = source.list_dir("kolla", _DOCKER, config, dirs_only=True) + upstream = kolla_docker.parse(docker_names) + template_bytes = source.read("container_image_kolla_ansible", _TEMPLATE, config) + template_keys = { + _norm(k) for k in versions_template.parse_versions_keys(template_bytes) + } + + drifts = [] + for service in sorted(upstream): + if service in template_keys: + continue + d = DriftEntry( + plugin=NAME, + image=service, + alias=service, + expected="present in openstack/kolla docker/ at stable/2025.2", + found="absent (no versions['...'] template key)", + expected_src=_EXPECTED_SRC, + found_src=_FOUND_SRC, + ) + drifts.append(allowlist.apply(d)) + return drifts diff --git a/src/osism_drift/kolla_docker.py b/src/osism_drift/kolla_docker.py new file mode 100644 index 0000000..99432d1 --- /dev/null +++ b/src/osism_drift/kolla_docker.py @@ -0,0 +1,13 @@ +"""Parser for openstack/kolla's docker/ service directory listing. + +The input is the directory listing (already filtered to directories by +source.list_dir(..., dirs_only=True), which drops files such as +docker/macros.j2). The output is the normalised service key space +(hyphens → underscores) for comparison against the template's versions['K'] +keys. Naming/variant directories are filtered later by the allowlist. +""" + + +def parse(names) -> set[str]: + """Return the set of service names normalised to the underscore key space.""" + return {n.replace("-", "_") for n in names} diff --git a/src/osism_drift/versions_template.py b/src/osism_drift/versions_template.py new file mode 100644 index 0000000..6cde225 --- /dev/null +++ b/src/osism_drift/versions_template.py @@ -0,0 +1,10 @@ +"""Parser for container-image-kolla-ansible's versions.yml.j2 template.""" + +import re + +_KEY_RE = re.compile(r"versions\['([a-z0-9_]+)'\]") + + +def parse_versions_keys(body: bytes) -> set[str]: + """Return the distinct keys K referenced as versions['K'] in the template.""" + return set(_KEY_RE.findall(body.decode("utf-8"))) diff --git a/tests/kolla_drift/test_kolla_docker.py b/tests/kolla_drift/test_kolla_docker.py new file mode 100644 index 0000000..e0614d6 --- /dev/null +++ b/tests/kolla_drift/test_kolla_docker.py @@ -0,0 +1,13 @@ +from osism_drift.kolla_docker import parse + + +def test_normalises_hyphen_to_underscore(): + assert parse(["nova", "mariadb-server", "openstack-base"]) == { + "nova", + "mariadb_server", + "openstack_base", + } + + +def test_empty(): + assert parse([]) == set() diff --git a/tests/kolla_drift/test_kolla_version_chain_upstream.py b/tests/kolla_drift/test_kolla_version_chain_upstream.py new file mode 100644 index 0000000..3871636 --- /dev/null +++ b/tests/kolla_drift/test_kolla_version_chain_upstream.py @@ -0,0 +1,46 @@ +from pathlib import Path +import pytest +from osism_drift.config import Allowlist, AllowEntry, Config, Remote, PluginCfg +from osism_drift.drift import kolla_version_chain_upstream as plugin + +FIXT = Path(__file__).parent / "fixtures" + + +@pytest.fixture +def cfg(): + # kolla is NOT pinned here, so list_dir reads the local fixture dir (offline). + return Config( + remote=Remote("https://raw/", "https://api/", "main", "osism"), + base_dirs=(str(FIXT),), + release_version="latest", + plugins={"kolla_version_chain_upstream": PluginCfg(enabled=True)}, + sources={}, + ) + + +def test_flags_services_without_template_key(cfg): + drifts = plugin.run(cfg, Allowlist(())) + images = sorted(d.image for d in drifts) + assert images == ["ignored_svc", "newsvc"] # present_a has a key + assert all("macros" not in d.image for d in drifts) # docker/macros.j2 excluded + + +def test_present_service_not_flagged(cfg): + drifts = plugin.run(cfg, Allowlist(())) + assert all(d.image != "present_a" for d in drifts) + + +def test_allowlist_marks_allowlisted(cfg): + al = Allowlist( + ( + AllowEntry( + plugin="kolla_version_chain_upstream", + image="ignored_svc", + reason="variant, not a service", + ), + ) + ) + drifts = plugin.run(cfg, al) + by = {d.image: d for d in drifts} + assert by["ignored_svc"].allowlisted is True + assert by["newsvc"].allowlisted is False diff --git a/tests/kolla_drift/test_plugin_registry.py b/tests/kolla_drift/test_plugin_registry.py index 2648a72..e808bff 100644 --- a/tests/kolla_drift/test_plugin_registry.py +++ b/tests/kolla_drift/test_plugin_registry.py @@ -39,6 +39,10 @@ def test_kolla_enablement_build_plugin_registered(): assert "kolla_enablement_build" in [p.NAME for p in PLUGINS] +def test_kolla_upstream_plugin_registered(): + assert "kolla_version_chain_upstream" in [p.NAME for p in PLUGINS] + + def test_default_config_enables_every_registered_plugin(): # The driver only runs a plugin that is present and enabled in the config, so # a plugin registered in PLUGINS but missing from the default config would diff --git a/tests/kolla_drift/test_versions_template.py b/tests/kolla_drift/test_versions_template.py new file mode 100644 index 0000000..06e6843 --- /dev/null +++ b/tests/kolla_drift/test_versions_template.py @@ -0,0 +1,18 @@ +from osism_drift.versions_template import parse_versions_keys + +SAMPLE = b"""\ +openstack_version: "2025.2" +kolla_aodh_version: "{{ versions['aodh']|default(openstack_version) }}" +kolla_heat_version: "{{ versions['heat']|default(openstack_version) }}" +kolla_common_version: "{{ versions['kolla_toolbox']|default(openstack_version) }}" +kolla_ironic_version: "{{ versions['ironic']|default(openstack_version) }}" +kolla_ironic_prometheus_exporter_version: "{{ versions['ironic']|default(openstack_version) }}" +""" + + +def test_extracts_distinct_keys(): + assert parse_versions_keys(SAMPLE) == {"aodh", "heat", "kolla_toolbox", "ironic"} + + +def test_empty_input(): + assert parse_versions_keys(b"") == set() From d04fd50c4d30ae501a1bc14f8c83abfaa9bb4049 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 16:19:02 +0200 Subject: [PATCH 07/12] Add kolla_version_chain_inner plugin A versions['K'] key the kolla-ansible template references but container-images-kolla's SBOM_IMAGE_TO_VERSION map never emits resolves to no value, so the template line silently falls back to openstack_version. The service looks pinned but is not -- an inert pin no error or log reveals, typically noticed only when the wrong version ships. Compare the two key spaces and nothing else: add the versions.yml.j2 template parser and the SBOM_IMAGE_TO_VERSION producer parser, normalise hyphen vs underscore, and deliberately ignore output-variable and image names (not keys; treating them as aliases would mask real drift). Classify each inert pin and route it to the right fix. A pin is an ADD only when OSISM actually deploys a built image (enable_X truthy in defaults AND a docker/X dir exists upstream): then the missing SBOM key should be wired. Otherwise the template line is dead and should be removed -- in the kolla-ansible template repo, not container-images-kolla. DriftEntry's optional summary/remediation carry the two distinct hints, and the add and remove buckets render as separate report blocks, each naming the file to edit. Register and enable the plugin. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Roger Luethi --- src/kolla-drift-config.yml | 1 + src/osism_drift/drift/__init__.py | 3 +- .../drift/kolla_version_chain_inner.py | 131 ++++++++++++++++++ src/osism_drift/sbom_map.py | 21 +++ tests/kolla_drift/test_driver_stale.py | 85 ++++++++++++ .../test_kolla_version_chain_inner.py | 74 ++++++++++ tests/kolla_drift/test_plugin_registry.py | 4 + tests/kolla_drift/test_sbom_map.py | 29 ++++ 8 files changed, 347 insertions(+), 1 deletion(-) create mode 100644 src/osism_drift/drift/kolla_version_chain_inner.py create mode 100644 src/osism_drift/sbom_map.py create mode 100644 tests/kolla_drift/test_driver_stale.py create mode 100644 tests/kolla_drift/test_kolla_version_chain_inner.py create mode 100644 tests/kolla_drift/test_sbom_map.py diff --git a/src/kolla-drift-config.yml b/src/kolla-drift-config.yml index b88a685..70783ed 100644 --- a/src/kolla-drift-config.yml +++ b/src/kolla-drift-config.yml @@ -20,3 +20,4 @@ plugins: kolla_secrets_orphan: {enabled: true} kolla_enablement_build: {enabled: true} kolla_version_chain_upstream: {enabled: true} + kolla_version_chain_inner: {enabled: true} diff --git a/src/osism_drift/drift/__init__.py b/src/osism_drift/drift/__init__.py index 115f2ea..86f395b 100644 --- a/src/osism_drift/drift/__init__.py +++ b/src/osism_drift/drift/__init__.py @@ -5,5 +5,6 @@ from osism_drift.drift import kolla_secrets_orphan from osism_drift.drift import kolla_enablement_build from osism_drift.drift import kolla_version_chain_upstream +from osism_drift.drift import kolla_version_chain_inner -PLUGINS = [kolla_enablement_orphan, kolla_orphan_config, kolla_secrets_orphan, kolla_enablement_build, kolla_version_chain_upstream] +PLUGINS = [kolla_enablement_orphan, kolla_orphan_config, kolla_secrets_orphan, kolla_enablement_build, kolla_version_chain_upstream, kolla_version_chain_inner] diff --git a/src/osism_drift/drift/kolla_version_chain_inner.py b/src/osism_drift/drift/kolla_version_chain_inner.py new file mode 100644 index 0000000..a028c36 --- /dev/null +++ b/src/osism_drift/drift/kolla_version_chain_inner.py @@ -0,0 +1,131 @@ +"""kolla_version_chain_inner: detect inert version pins, split by fix direction. + +A versions['K'] key referenced in the kolla-ansible template but absent from +container-images-kolla's SBOM_IMAGE_TO_VERSION map is never emitted, so the +template line silently defaults to openstack_version. Compares key spaces only, +normalising hyphen/underscore; output-variable names and image names are NOT keys. + +An inert pin has two opposite fixes, and which one is right depends on whether +OSISM actually deploys a built image for the service: + + - ADD a pin -- the service is enabled in OSISM (enable_X truthy) AND kolla + can build an image for it (a docker/X dir exists upstream). + The pin should resolve; wire the missing SBOM key. + - REMOVE the line -- otherwise (not enabled, or no upstream image). No + OSISM-built image backs the pin, so the template line is dead. + +Emitting one generic "add or remove" hint conflates these (and points at the +wrong repo for the common remove case), so each finding carries its own +SUMMARY/REMEDIATION and the source pair of the file the operator must edit. +""" + +from osism_drift import ( + enablement, + kolla_docker, + sbom_map, + source, + versions_template, +) +from osism_drift.model import DriftEntry + +NAME = "kolla_version_chain_inner" +DESCRIPTION = ( + "Flag versions['K'] template keys absent from the producer's " + "SBOM_IMAGE_TO_VERSION map (inert pins), split into wire-the-pin vs " + "remove-the-dead-line by OSISM enablement and upstream buildability." +) +INPUT_FILES = [ + ("container_image_kolla_ansible", "files/src/templates/versions.yml.j2"), + ("container_images_kolla", "src/tag-images-with-the-version.py"), + ("defaults", "all/099-kolla.yml"), + ("kolla", "docker/"), +] +# Module-level fallback for the report; every entry below sets a per-finding +# override, so these render only if an entry ever omits them. +SUMMARY = ( + "{n} image-version keys that the kolla-ansible template reads via " + "versions[''] but container-images-kolla's SBOM map never sets:" +) +REMEDIATION = ( + "wire the key into SBOM_IMAGE_TO_VERSION if OSISM deploys it, else remove " + "the dead template line. Allowlist keys that are meant to default." +) + +_TEMPLATE = "files/src/templates/versions.yml.j2" +_SBOM = "src/tag-images-with-the-version.py" +_ENABLE = "all/099-kolla.yml" +_DOCKER = "docker" + +_ADD_SUMMARY = ( + "{n} image-version keys the kolla-ansible template reads via versions[''] " + "for services OSISM enables and kolla can build, but container-images-kolla's " + "SBOM map never sets them, so the pin stays inert:" +) +_ADD_REMEDIATION = ( + "add the key to SBOM_IMAGE_TO_VERSION in tag-images-with-the-version.py so the " + "pin is produced." +) +_ADD_EXPECTED_SRC = "container-images-kolla/src/tag-images-with-the-version.py" +_ADD_FOUND_SRC = "container-image-kolla-ansible/files/src/templates/versions.yml.j2" + +_DEAD_SUMMARY = ( + "{n} versions[''] template lines for services OSISM does not build (not " + "enabled, or no upstream kolla image), so the pin can never resolve, dead lines:" +) +_DEAD_REMEDIATION = ( + "remove the dead versions[''] line from the kolla-ansible template, or " + "allowlist it if it is intentionally kept." +) +_DEAD_EXPECTED_SRC = "openstack/kolla docker/ @ stable/2025.2 (no OSISM-built image)" +_DEAD_FOUND_SRC = "container-image-kolla-ansible/files/src/templates/versions.yml.j2" + + +def _norm(key: str) -> str: + return key.replace("-", "_") + + +def run(config, allowlist, verbose: bool = False) -> list[DriftEntry]: + """Return inert-pin drifts, each routed to its add-vs-remove fix.""" + template_bytes = source.read("container_image_kolla_ansible", _TEMPLATE, config) + sbom_bytes = source.read("container_images_kolla", _SBOM, config) + template_keys = versions_template.parse_versions_keys(template_bytes) + sbom_keys = {_norm(k) for k in sbom_map.parse_sbom_keys(sbom_bytes)} + + enabled = enablement.truthy_enables( + enablement.parse_enable_flags(source.read("defaults", _ENABLE, config)) + ) + buildable = kolla_docker.parse( + source.list_dir("kolla", _DOCKER, config, dirs_only=True) + ) + + drifts = [] + for key in sorted(template_keys): + nkey = _norm(key) + if nkey in sbom_keys: + continue + if nkey in enabled and nkey in buildable: + d = DriftEntry( + plugin=NAME, + image=key, + alias=key, + expected="emitted by SBOM_IMAGE_TO_VERSION", + found="absent (enabled & buildable, but pin defaults to openstack_version)", + expected_src=_ADD_EXPECTED_SRC, + found_src=_ADD_FOUND_SRC, + summary=_ADD_SUMMARY, + remediation=_ADD_REMEDIATION, + ) + else: + d = DriftEntry( + plugin=NAME, + image=key, + alias=key, + expected="no OSISM-built image (not enabled and/or not buildable upstream)", + found="dead versions['...'] line defaulting to openstack_version", + expected_src=_DEAD_EXPECTED_SRC, + found_src=_DEAD_FOUND_SRC, + summary=_DEAD_SUMMARY, + remediation=_DEAD_REMEDIATION, + ) + drifts.append(allowlist.apply(d)) + return drifts diff --git a/src/osism_drift/sbom_map.py b/src/osism_drift/sbom_map.py new file mode 100644 index 0000000..14a221b --- /dev/null +++ b/src/osism_drift/sbom_map.py @@ -0,0 +1,21 @@ +"""Static parser for the SBOM_IMAGE_TO_VERSION map in +container-images-kolla/src/tag-images-with-the-version.py. + +Read via ast.literal_eval — the producer script is never imported or executed. +""" + +import ast + + +def parse_sbom_keys(body: bytes) -> set[str]: + """Return the set of keys of the SBOM_IMAGE_TO_VERSION dict literal.""" + tree = ast.parse(body) + for node in ast.walk(tree): + if isinstance(node, ast.Assign): + for target in node.targets: + if ( + isinstance(target, ast.Name) + and target.id == "SBOM_IMAGE_TO_VERSION" + ): + return set(ast.literal_eval(node.value).keys()) + raise ValueError("SBOM_IMAGE_TO_VERSION assignment not found") diff --git a/tests/kolla_drift/test_driver_stale.py b/tests/kolla_drift/test_driver_stale.py new file mode 100644 index 0000000..49bed03 --- /dev/null +++ b/tests/kolla_drift/test_driver_stale.py @@ -0,0 +1,85 @@ +import importlib.util +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +FIXT = Path(__file__).parent / "fixtures" + + +def _load_driver(): + spec = importlib.util.spec_from_file_location( + "drift_driver", ROOT / "src" / "check-kolla-drift.py" + ) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def _write_cfg(tmp_path): + cfg = tmp_path / "c.yml" + cfg.write_text(""" +remote: + github_raw: https://raw.githubusercontent.com/ + github_api: https://api.github.com/repos/ + default_owner: osism + branch: main +release_version: latest +plugins: + kolla_version_chain_inner: {enabled: true} +""") + return cfg + + +def test_stale_entry_forces_exit_1_and_is_reported(tmp_path, capsys): + driver = _load_driver() + cfg = _write_cfg(tmp_path) + al = tmp_path / "a.yml" + al.write_text(""" +allow: + - {plugin: kolla_version_chain_inner, image: inert_x, reason: "allowlist the real drift"} + - {plugin: kolla_version_chain_inner, image: ghost, reason: "dead entry"} +""") + rc = driver.main( + [ + "--config", + str(cfg), + "--allowlist", + str(al), + "--base-dir", + str(FIXT), + "--plugin", + "kolla_version_chain_inner", + ] + ) + out = capsys.readouterr().out + # inert_x (the only real drift) is allowlisted → no actionable drift, + # but the `ghost` entry matched nothing → stale → exit 1. + assert rc == 1 + assert "STALE ALLOWLIST" in out + assert "ghost" in out + + +def test_no_allowlist_ignores_invalid_file(tmp_path, capsys): + driver = _load_driver() + cfg = _write_cfg(tmp_path) + bad = tmp_path / "bad.yml" + bad.write_text( + "allow:\n - {plugin: x}\n" + ) # missing image/reason → ConfigError if parsed + rc = driver.main( + [ + "--config", + str(cfg), + "--allowlist", + str(bad), + "--no-allowlist", + "--base-dir", + str(FIXT), + "--plugin", + "kolla_version_chain_inner", + ] + ) + out = capsys.readouterr().out + # --no-allowlist must NOT read the file: exit 1 from the real inert_x drift, + # not exit 2 from a config error. (Old behavior parsed it and returned 2.) + assert rc == 1 + assert "inert_x" in out diff --git a/tests/kolla_drift/test_kolla_version_chain_inner.py b/tests/kolla_drift/test_kolla_version_chain_inner.py new file mode 100644 index 0000000..a167bac --- /dev/null +++ b/tests/kolla_drift/test_kolla_version_chain_inner.py @@ -0,0 +1,74 @@ +from pathlib import Path +import pytest +from osism_drift.config import Allowlist, AllowEntry, Config, Remote, PluginCfg +from osism_drift.drift import kolla_version_chain_inner as plugin + +FIXT = Path(__file__).parent / "fixtures" + + +@pytest.fixture +def cfg(): + return Config( + remote=Remote("https://raw/", "https://api/", "main"), + base_dirs=(str(FIXT),), + release_version="latest", + plugins={"kolla_version_chain_inner": PluginCfg(enabled=True)}, + ) + + +def test_flags_all_inert_keys(cfg): + # present_a/present_b/kolla_toolbox are in the SBOM; foo/off/inert_x are not. + drifts = plugin.run(cfg, Allowlist(())) + assert sorted(d.image for d in drifts) == ["foo", "inert_x", "off"] + assert all(d.plugin == "kolla_version_chain_inner" for d in drifts) + assert all(d.allowlisted is False for d in drifts) + + +def test_enabled_and_buildable_key_is_an_add(cfg): + # foo: enable_foo truthy AND docker/foo exists -> wire the SBOM key. + d = [x for x in plugin.run(cfg, Allowlist(())) if x.image == "foo"][0] + assert "tag-images-with-the-version.py" in d.expected_src + assert "versions.yml.j2" in d.found_src + assert "SBOM_IMAGE_TO_VERSION" in d.remediation + assert "remove" not in d.remediation.lower() + + +def test_disabled_or_unbuildable_key_is_a_remove(cfg): + # off: buildable (docker/off) but enable_off is "no" -> dead line. + # inert_x: neither enabled nor buildable -> dead line. + by = {x.image: x for x in plugin.run(cfg, Allowlist(()))} + for key in ("off", "inert_x"): + d = by[key] + assert "remove" in d.remediation.lower() + assert "versions.yml.j2" in d.found_src # the template line to delete + assert "tag-images-with-the-version.py" not in d.expected_src + + +def test_add_and_remove_render_as_separate_blocks(cfg): + # The two buckets carry distinct (expected_src, found_src), so the grouped + # report renders one add block and one remove block, never one merged hint. + drifts = plugin.run(cfg, Allowlist(())) + add = [d for d in drifts if d.image == "foo"][0] + rm = [d for d in drifts if d.image == "off"][0] + assert (add.expected_src, add.found_src) != (rm.expected_src, rm.found_src) + + +def test_hyphen_underscore_is_not_drift(cfg): + # kolla_toolbox (template) vs kolla-toolbox (SBOM) — normalised match. + drifts = plugin.run(cfg, Allowlist(())) + assert all(d.image != "kolla_toolbox" for d in drifts) + + +def test_allowlist_marks_allowlisted(cfg): + al = Allowlist( + ( + AllowEntry( + plugin="kolla_version_chain_inner", + image="inert_x", + reason="tracked in issue X", + ), + ) + ) + by = {d.image: d for d in plugin.run(cfg, al)} + assert by["inert_x"].allowlisted is True + assert by["inert_x"].reason == "tracked in issue X" diff --git a/tests/kolla_drift/test_plugin_registry.py b/tests/kolla_drift/test_plugin_registry.py index e808bff..9d760b1 100644 --- a/tests/kolla_drift/test_plugin_registry.py +++ b/tests/kolla_drift/test_plugin_registry.py @@ -43,6 +43,10 @@ def test_kolla_upstream_plugin_registered(): assert "kolla_version_chain_upstream" in [p.NAME for p in PLUGINS] +def test_kolla_inner_plugin_registered(): + assert "kolla_version_chain_inner" in [p.NAME for p in PLUGINS] + + def test_default_config_enables_every_registered_plugin(): # The driver only runs a plugin that is present and enabled in the config, so # a plugin registered in PLUGINS but missing from the default config would diff --git a/tests/kolla_drift/test_sbom_map.py b/tests/kolla_drift/test_sbom_map.py new file mode 100644 index 0000000..36c6398 --- /dev/null +++ b/tests/kolla_drift/test_sbom_map.py @@ -0,0 +1,29 @@ +import pytest +from osism_drift.sbom_map import parse_sbom_keys + +SAMPLE = b"""\ +import logging + +SBOM_IMAGE_TO_VERSION = { + "heat": "heat-api", + "ironic": "ironic-api", + "kolla_toolbox": "kolla-toolbox", + "kolla-toolbox": "kolla-toolbox", +} + +OTHER = {"nope": 1} +""" + + +def test_extracts_keys(): + assert parse_sbom_keys(SAMPLE) == { + "heat", + "ironic", + "kolla_toolbox", + "kolla-toolbox", + } + + +def test_missing_map_raises(): + with pytest.raises(ValueError): + parse_sbom_keys(b"X = 1\n") From 863d3b20a467f7d3a2858c53776d5c48afc75cc4 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 16:24:45 +0200 Subject: [PATCH 08/12] Add kolla_inventory drift plugin Re-home the retired check-kolla-inventory.py as a framework plugin: flag ansible inventory groups in upstream kolla-ansible multinode (pinned stable/2025.2) that are absent from the OSISM 50/51-kolla inventory in the generics repo. One-way; each flagged group's upstream members ride in `expected` so a maintainer knows what to add. Add the ansible INI inventory group parser (configparser, as the original script used), pin kolla_ansible to openstack/stable/2025.2, add the generics path, enable the plugin, and seed the allowlist with the empirical delta: 7 exact base-infra groups the OSISM overlay omits (compute, control, network, storage, monitoring, deployment, baremetal:children) plus 4 prefix not-deployed services (cyborg, tacker, telegraf, collectd, prefix covering their sub-groups). All 16 current delta items are suppressed and none is stale; the original's 8 dead IGNORE prefixes are dropped. The standalone script was never wired into CI or any tox env -- it was run by hand during inventory maintenance -- and this plugin fully subsumes it, so remove it. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Roger Luethi --- src/check-kolla-inventory.py | 50 -------------- src/kolla-drift-allowlist.yml | 15 +++++ src/kolla-drift-config.yml | 7 +- src/osism_drift/drift/__init__.py | 25 +++++-- src/osism_drift/drift/kolla_inventory.py | 69 ++++++++++++++++++++ src/osism_drift/inventory_sections.py | 19 ++++++ tests/kolla_drift/test_inventory_sections.py | 24 +++++++ tests/kolla_drift/test_kolla_inventory.py | 54 +++++++++++++++ tests/kolla_drift/test_plugin_registry.py | 48 +++++++++----- 9 files changed, 234 insertions(+), 77 deletions(-) delete mode 100644 src/check-kolla-inventory.py create mode 100644 src/osism_drift/drift/kolla_inventory.py create mode 100644 src/osism_drift/inventory_sections.py create mode 100644 tests/kolla_drift/test_inventory_sections.py create mode 100644 tests/kolla_drift/test_kolla_inventory.py diff --git a/src/check-kolla-inventory.py b/src/check-kolla-inventory.py deleted file mode 100644 index 1db6623..0000000 --- a/src/check-kolla-inventory.py +++ /dev/null @@ -1,50 +0,0 @@ -#!/usr/bin/env python3 - -from configparser import ConfigParser -import io - -import requests - -IGNORE = [ - "baremetal", - "blazar", - "collectd", - "compute", - "control", - "cyborg", - "deployment", - "freezer", - "masakari", - "monitoring", - "murano", - "network", - "sahara", - "solum", - "storage", - "tacker", - "telegraf", - "venus", - "vitrage", -] - -config = ConfigParser(allow_no_value=True) -config.read("inventory/50-kolla") -sections = config.sections() - -config = ConfigParser(allow_no_value=True) -config.read("inventory/51-kolla") -sections += config.sections() - -url = "https://raw.githubusercontent.com/openstack/kolla-ansible/master/ansible/inventory/multinode" # noqa -response = requests.get(url, stream=True) - -config = ConfigParser(allow_no_value=True) -config.read_file(io.StringIO(response.text)) -upstream_sections = config.sections() - -for section in [x for x in upstream_sections if x not in sections]: - if not [x for x in IGNORE if section.startswith(x)]: - print(f"[{section}]") - for x in config[section]: - print(x) - print() diff --git a/src/kolla-drift-allowlist.yml b/src/kolla-drift-allowlist.yml index 22a6e7c..18e935b 100644 --- a/src/kolla-drift-allowlist.yml +++ b/src/kolla-drift-allowlist.yml @@ -16,6 +16,21 @@ allow: - {plugin: kolla_version_chain_upstream, image: ovsdpdk, reason: "openvswitch dpdk variant, no distinct version key"} - {plugin: kolla_version_chain_upstream, image: httpd, reason: "apache supporting image, not independently pinned"} - {plugin: kolla_version_chain_upstream, image: mariadb_server, reason: "upstream dir mariadb-server; wired via versions['mariadb']"} + # --- kolla_inventory (Flow A) --- + # OSISM's overlay model assumes these base-infra groups exist in the + # environment inventory, so they are intentionally absent from 50/51-kolla. + - {plugin: kolla_inventory, image: compute, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: control, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: network, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: storage, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: monitoring, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: deployment, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: "baremetal:children", reason: "baremetal aggregator; omitted by the OSISM overlay model"} + # Services not deployed in this series (prefix covers their sub-groups). + - {plugin: kolla_inventory, image: cyborg, match: prefix, reason: "not deployed in this series"} + - {plugin: kolla_inventory, image: tacker, match: prefix, reason: "not deployed in this series"} + - {plugin: kolla_inventory, image: telegraf, match: prefix, reason: "not deployed in this series"} + - {plugin: kolla_inventory, image: collectd, match: prefix, reason: "not deployed in this series"} # --- kolla_enablement_orphan --- # OSISM-invented enable flags with no upstream counterpart by design (not a # removed-upstream service). Truthy, so they surface in the truthy-only scope. diff --git a/src/kolla-drift-config.yml b/src/kolla-drift-config.yml index 70783ed..e46ee27 100644 --- a/src/kolla-drift-config.yml +++ b/src/kolla-drift-config.yml @@ -15,9 +15,10 @@ sources: release_version: latest plugins: + kolla_version_chain_inner: {enabled: true} + kolla_version_chain_upstream: {enabled: true} + kolla_inventory: {enabled: true} + kolla_enablement_build: {enabled: true} kolla_enablement_orphan: {enabled: true} kolla_orphan_config: {enabled: true} kolla_secrets_orphan: {enabled: true} - kolla_enablement_build: {enabled: true} - kolla_version_chain_upstream: {enabled: true} - kolla_version_chain_inner: {enabled: true} diff --git a/src/osism_drift/drift/__init__.py b/src/osism_drift/drift/__init__.py index 86f395b..3f98c81 100644 --- a/src/osism_drift/drift/__init__.py +++ b/src/osism_drift/drift/__init__.py @@ -1,10 +1,21 @@ """Plugin registry. Plugins are appended here as they are added.""" -from osism_drift.drift import kolla_enablement_orphan -from osism_drift.drift import kolla_orphan_config -from osism_drift.drift import kolla_secrets_orphan -from osism_drift.drift import kolla_enablement_build -from osism_drift.drift import kolla_version_chain_upstream -from osism_drift.drift import kolla_version_chain_inner +from osism_drift.drift import ( + kolla_enablement_build, + kolla_enablement_orphan, + kolla_inventory, + kolla_orphan_config, + kolla_secrets_orphan, + kolla_version_chain_inner, + kolla_version_chain_upstream, +) -PLUGINS = [kolla_enablement_orphan, kolla_orphan_config, kolla_secrets_orphan, kolla_enablement_build, kolla_version_chain_upstream, kolla_version_chain_inner] +PLUGINS = [ + kolla_enablement_orphan, + kolla_orphan_config, + kolla_secrets_orphan, + kolla_enablement_build, + kolla_version_chain_upstream, + kolla_version_chain_inner, + kolla_inventory, +] diff --git a/src/osism_drift/drift/kolla_inventory.py b/src/osism_drift/drift/kolla_inventory.py new file mode 100644 index 0000000..5a6e23d --- /dev/null +++ b/src/osism_drift/drift/kolla_inventory.py @@ -0,0 +1,69 @@ +"""kolla_inventory: detect upstream ansible inventory groups missing locally. + +Compares the group (INI section) names of upstream openstack/kolla-ansible +ansible/inventory/multinode (pinned stable/2025.2) against the union of group +names in the OSISM inventory files generics/inventory/50-kolla and 51-kolla. A +group present upstream but absent locally is flagged, and its upstream members +ride in `expected` so a maintainer knows what to add. + +The comparison is one-way (upstream -> local). Intentional omissions are +covered by the allowlist (exact base groups plus prefix not-deployed +services). +""" + +from osism_drift import inventory_sections, source +from osism_drift.model import DriftEntry + +NAME = "kolla_inventory" +DESCRIPTION = ( + "Flag ansible inventory groups in upstream kolla-ansible multinode " + "(stable/2025.2) that are absent from the OSISM 50/51-kolla inventory." +) +INPUT_FILES = [ + ("kolla_ansible", "ansible/inventory/multinode"), + ("generics", "inventory/50-kolla"), + ("generics", "inventory/51-kolla"), +] +SUMMARY = ( + "{n} ansible inventory groups present in upstream kolla-ansible multinode " + "but missing from the OSISM 50/51-kolla inventory:" +) +REMEDIATION = ( + "add the group and its members to generics/inventory/50-kolla or 51-kolla, " + "or allowlist it if the service is intentionally not deployed." +) + +_MULTINODE = "ansible/inventory/multinode" +_50 = "inventory/50-kolla" +_51 = "inventory/51-kolla" +_EXPECTED_SRC = "openstack/kolla-ansible/ansible/inventory/multinode @ stable/2025.2" +_FOUND_SRC = "generics/inventory/{50,51}-kolla" + + +def run(config, allowlist, verbose: bool = False) -> list[DriftEntry]: + """Return drifts for kolla-ansible inventory groups missing from OSISM.""" + upstream = inventory_sections.parse_groups( + source.read("kolla_ansible", _MULTINODE, config) + ) + local = set( + inventory_sections.parse_groups(source.read("generics", _50, config)) + ) | set(inventory_sections.parse_groups(source.read("generics", _51, config))) + + drifts = [] + for group in sorted(set(upstream) - local): + members = upstream[group] + member_str = ", ".join(members) if members else "(none)" + d = DriftEntry( + plugin=NAME, + image=group, + alias=group, + expected=( + "present in upstream kolla-ansible multinode at " + f"stable/2025.2; members: {member_str}" + ), + found="absent from OSISM inventory (50-kolla/51-kolla)", + expected_src=_EXPECTED_SRC, + found_src=_FOUND_SRC, + ) + drifts.append(allowlist.apply(d)) + return drifts diff --git a/src/osism_drift/inventory_sections.py b/src/osism_drift/inventory_sections.py new file mode 100644 index 0000000..54e5164 --- /dev/null +++ b/src/osism_drift/inventory_sections.py @@ -0,0 +1,19 @@ +"""Parser for ansible-style INI inventory files (kolla-ansible multinode and +the OSISM 50/51-kolla files). + +Returns each group (INI section) mapped to its sorted members — child-group +names for a :children group, host names for a host group — via configparser, +the same parser the original check-kolla-inventory.py used. The group-name set +for the drift comparison is set(parse_groups(body)); the members make a flagged +group actionable. optionxform is overridden to preserve member-name case. +""" + +import configparser + + +def parse_groups(body: bytes) -> dict[str, list[str]]: + """Return {section: sorted(member names)} for every INI section.""" + cp = configparser.ConfigParser(allow_no_value=True) + cp.optionxform = str # preserve case of member names + cp.read_string(body.decode("utf-8")) + return {section: sorted(cp[section].keys()) for section in cp.sections()} diff --git a/tests/kolla_drift/test_inventory_sections.py b/tests/kolla_drift/test_inventory_sections.py new file mode 100644 index 0000000..3c4c3b6 --- /dev/null +++ b/tests/kolla_drift/test_inventory_sections.py @@ -0,0 +1,24 @@ +from osism_drift.inventory_sections import parse_groups + +SAMPLE = b"""\ +[control] +ctl01 +ctl02 + +[nova:children] +control +compute + +[empty:children] +""" + + +def test_returns_groups_with_sorted_members(): + groups = parse_groups(SAMPLE) + assert set(groups) == {"control", "nova:children", "empty:children"} + assert groups["nova:children"] == ["compute", "control"] + assert groups["control"] == ["ctl01", "ctl02"] + + +def test_empty_group_has_no_members(): + assert parse_groups(SAMPLE)["empty:children"] == [] diff --git a/tests/kolla_drift/test_kolla_inventory.py b/tests/kolla_drift/test_kolla_inventory.py new file mode 100644 index 0000000..c975f0d --- /dev/null +++ b/tests/kolla_drift/test_kolla_inventory.py @@ -0,0 +1,54 @@ +from pathlib import Path +import pytest +from osism_drift.config import Allowlist, AllowEntry, Config, Remote, PluginCfg +from osism_drift.drift import kolla_inventory as plugin + +FIXT = Path(__file__).parent / "fixtures" + + +@pytest.fixture +def cfg(): + # kolla_ansible is NOT pinned here, so it reads the local fixture (offline). + return Config( + remote=Remote("https://raw/", "https://api/", "main", "osism"), + base_dirs=(str(FIXT),), + release_version="latest", + plugins={"kolla_inventory": PluginCfg(enabled=True)}, + sources={}, + ) + + +def test_flags_upstream_only_groups(cfg): + drifts = plugin.run(cfg, Allowlist(())) + assert sorted(d.image for d in drifts) == [ + "cyborg-agent:children", + "cyborg:children", + "ironic-dnsmasq:children", + ] + + +def test_red_entry_carries_members(cfg): + drifts = plugin.run(cfg, Allowlist(())) + red = next(d for d in drifts if d.image == "ironic-dnsmasq:children") + assert "members:" in red.expected + assert "control" in red.expected + + +def test_exact_and_prefix_allowlist_match(cfg): + al = Allowlist( + ( + AllowEntry( + plugin="kolla_inventory", + image="cyborg", + reason="not deployed", + match="prefix", + ), + ) + ) + drifts = plugin.run(cfg, al) + by = {d.image: d for d in drifts} + assert by["cyborg:children"].allowlisted is True # prefix matches the service group + assert ( + by["cyborg-agent:children"].allowlisted is True + ) # prefix covers the sub-group + assert by["ironic-dnsmasq:children"].allowlisted is False diff --git a/tests/kolla_drift/test_plugin_registry.py b/tests/kolla_drift/test_plugin_registry.py index 9d760b1..e9731f3 100644 --- a/tests/kolla_drift/test_plugin_registry.py +++ b/tests/kolla_drift/test_plugin_registry.py @@ -16,35 +16,39 @@ def test_each_plugin_has_required_metadata(): assert callable(p.run) -def test_every_plugin_has_summary_and_remediation(): - for p in PLUGINS: - assert isinstance(p.SUMMARY, str) and p.SUMMARY.strip(), p.NAME - assert "{n}" in p.SUMMARY, p.NAME - assert isinstance(p.REMEDIATION, str) and p.REMEDIATION.strip(), p.NAME - - -def test_kolla_enablement_orphan_plugin_registered(): - assert "kolla_enablement_orphan" in [p.NAME for p in PLUGINS] +def test_kolla_inner_plugin_registered(): + assert "kolla_version_chain_inner" in [p.NAME for p in PLUGINS] -def test_kolla_orphan_config_plugin_registered(): - assert "kolla_orphan_config" in [p.NAME for p in PLUGINS] +def test_kolla_upstream_plugin_registered(): + assert "kolla_version_chain_upstream" in [p.NAME for p in PLUGINS] -def test_kolla_secrets_orphan_plugin_registered(): - assert "kolla_secrets_orphan" in [p.NAME for p in PLUGINS] +def test_kolla_inventory_plugin_registered(): + assert "kolla_inventory" in [p.NAME for p in PLUGINS] def test_kolla_enablement_build_plugin_registered(): assert "kolla_enablement_build" in [p.NAME for p in PLUGINS] -def test_kolla_upstream_plugin_registered(): - assert "kolla_version_chain_upstream" in [p.NAME for p in PLUGINS] +def test_kolla_enablement_orphan_plugin_registered(): + assert "kolla_enablement_orphan" in [p.NAME for p in PLUGINS] -def test_kolla_inner_plugin_registered(): - assert "kolla_version_chain_inner" in [p.NAME for p in PLUGINS] +def test_kolla_secrets_orphan_plugin_registered(): + assert "kolla_secrets_orphan" in [p.NAME for p in PLUGINS] + + +def test_kolla_orphan_config_plugin_registered(): + assert "kolla_orphan_config" in [p.NAME for p in PLUGINS] + + +def test_every_plugin_has_summary_and_remediation(): + for p in PLUGINS: + assert isinstance(p.SUMMARY, str) and p.SUMMARY.strip(), p.NAME + assert "{n}" in p.SUMMARY, p.NAME + assert isinstance(p.REMEDIATION, str) and p.REMEDIATION.strip(), p.NAME def test_default_config_enables_every_registered_plugin(): @@ -59,3 +63,13 @@ def test_default_config_enables_every_registered_plugin(): assert entry.enabled, f"{p.NAME} not enabled in default config" +def test_plugins_in_lifecycle_order(): + assert [p.NAME for p in PLUGINS] == [ + "kolla_enablement_orphan", + "kolla_orphan_config", + "kolla_secrets_orphan", + "kolla_enablement_build", + "kolla_version_chain_upstream", + "kolla_version_chain_inner", + "kolla_inventory", + ] From 4eb26c016417b0f21daaf22ee72ce65f36bf26ff Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 16:56:24 +0200 Subject: [PATCH 09/12] Run kolla drift detector in periodic-daily The check-kolla-drift.py detector existed in the repo but nothing ran it: the kolla-drift-test tox env only exercises the unit tests, never the detector against the live repos. Drift it is meant to catch (an OSISM enable flag, version pin, or inventory group diverging from upstream openstack/kolla and kolla-ansible) would therefore go unnoticed until a deploy or gate broke. Wire the detector into Zuul's periodic-daily pipeline: - add a kolla-drift tox env that runs `python3 src/check-kolla-drift.py` with no args, so every repo is read remotely from GitHub (no checkout needed) and the configured allowlist applies. The script exits 0 when clean, 1 on actionable drift or a stale allowlist entry, and 2 on an input error, which maps directly onto a CI gate. - add a generics-tox-kolla-drift job wrapping that env. - run the job in periodic-daily, which catches drift introduced upstream after a change merged, independent of any PR. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi --- .zuul.yaml | 7 +++++++ tox.ini | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/.zuul.yaml b/.zuul.yaml index 39539ed..d036f7b 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -35,6 +35,12 @@ vars: tox_envlist: seed +- job: + name: generics-tox-kolla-drift + parent: tox + vars: + tox_envlist: kolla-drift + - project: merge-mode: squash-merge default-branch: main @@ -53,6 +59,7 @@ jobs: - generics-tox-check - generics-tox-seed + - generics-tox-kolla-drift - generics-tox-test-latest - generics-tox-test-stable - generics-tox-test-stable-legacy diff --git a/tox.ini b/tox.ini index 860aa93..47c5336 100644 --- a/tox.ini +++ b/tox.ini @@ -43,3 +43,9 @@ basepython = python3 deps = -r requirements.txt commands = pytest {posargs:tests/kolla_drift/ -v} + +[testenv:kolla-drift] +basepython = python3 +deps = -r requirements.txt +commands = + python3 src/check-kolla-drift.py {posargs} From 5d79a3b38c2957a012a6c0a0acc5c845077ae128 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 17:23:27 +0200 Subject: [PATCH 10/12] Format kolla-drift config files for yamllint The repo's yamllint Zuul job (check and periodic-daily pipelines) flagged 26 errors in the two drift config files: "too many spaces after colon" in kolla-drift-config.yml and "too many spaces after comma" in kolla-drift-allowlist.yml. Both stem from column-aligning the values for readability, which exceeds yamllint's one-space cap on the colons and commas rules. The files had never been linted before, so the violations surfaced only once the branch reached CI. Collapse the alignment padding to a single space after each colon and comma, and add the `---` document-start header to both files. This is a whitespace-only change: the parsed YAML (plugin enable flags and allowlist entries) is identical, and the unit suite still passes (146 tests). Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi --- src/kolla-drift-allowlist.yml | 33 +++++++++++++++++---------------- src/kolla-drift-config.yml | 21 +++++++++++---------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/kolla-drift-allowlist.yml b/src/kolla-drift-allowlist.yml index 18e935b..3c72ce7 100644 --- a/src/kolla-drift-allowlist.yml +++ b/src/kolla-drift-allowlist.yml @@ -1,38 +1,39 @@ +--- # Known-intentional exceptions. Each entry needs a non-empty `reason`. # An entry that suppresses no real drift is reported as stale (a hard error). allow: # Not deployed in this OSISM series (cf. inventory IGNORE_NOT_DEPLOYED). - - {plugin: kolla_version_chain_upstream, image: cyborg, reason: "not deployed in this series"} - - {plugin: kolla_version_chain_upstream, image: tacker, reason: "not deployed in this series"} + - {plugin: kolla_version_chain_upstream, image: cyborg, reason: "not deployed in this series"} + - {plugin: kolla_version_chain_upstream, image: tacker, reason: "not deployed in this series"} - {plugin: kolla_version_chain_upstream, image: telegraf, reason: "not deployed in this series"} - - {plugin: kolla_version_chain_upstream, image: zun, reason: "not deployed in this series"} + - {plugin: kolla_version_chain_upstream, image: zun, reason: "not deployed in this series"} # Build base layer, not a deployable service. - - {plugin: kolla_version_chain_upstream, image: base, reason: "build base layer, not a service"} + - {plugin: kolla_version_chain_upstream, image: base, reason: "build base layer, not a service"} - {plugin: kolla_version_chain_upstream, image: openstack_base, reason: "build base layer, not a service"} # Naming/variant or supporting image, no distinct versions['K'] key. - - {plugin: kolla_version_chain_upstream, image: hacluster, reason: "pacemaker/corosync variant, no distinct version key"} - - {plugin: kolla_version_chain_upstream, image: letsencrypt, reason: "wired via versions['letsencrypt_lego'] / ['letsencrypt_webserver']"} + - {plugin: kolla_version_chain_upstream, image: hacluster, reason: "pacemaker/corosync variant, no distinct version key"} + - {plugin: kolla_version_chain_upstream, image: letsencrypt, reason: "wired via versions['letsencrypt_lego'] / ['letsencrypt_webserver']"} - {plugin: kolla_version_chain_upstream, image: networking_baremetal, reason: "neutron plugin image, no distinct version key"} - - {plugin: kolla_version_chain_upstream, image: ovsdpdk, reason: "openvswitch dpdk variant, no distinct version key"} - - {plugin: kolla_version_chain_upstream, image: httpd, reason: "apache supporting image, not independently pinned"} - - {plugin: kolla_version_chain_upstream, image: mariadb_server, reason: "upstream dir mariadb-server; wired via versions['mariadb']"} + - {plugin: kolla_version_chain_upstream, image: ovsdpdk, reason: "openvswitch dpdk variant, no distinct version key"} + - {plugin: kolla_version_chain_upstream, image: httpd, reason: "apache supporting image, not independently pinned"} + - {plugin: kolla_version_chain_upstream, image: mariadb_server, reason: "upstream dir mariadb-server; wired via versions['mariadb']"} # --- kolla_inventory (Flow A) --- # OSISM's overlay model assumes these base-infra groups exist in the # environment inventory, so they are intentionally absent from 50/51-kolla. - - {plugin: kolla_inventory, image: compute, reason: "base infra group; provided by the environment inventory"} - - {plugin: kolla_inventory, image: control, reason: "base infra group; provided by the environment inventory"} - - {plugin: kolla_inventory, image: network, reason: "base infra group; provided by the environment inventory"} - - {plugin: kolla_inventory, image: storage, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: compute, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: control, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: network, reason: "base infra group; provided by the environment inventory"} + - {plugin: kolla_inventory, image: storage, reason: "base infra group; provided by the environment inventory"} - {plugin: kolla_inventory, image: monitoring, reason: "base infra group; provided by the environment inventory"} - {plugin: kolla_inventory, image: deployment, reason: "base infra group; provided by the environment inventory"} - {plugin: kolla_inventory, image: "baremetal:children", reason: "baremetal aggregator; omitted by the OSISM overlay model"} # Services not deployed in this series (prefix covers their sub-groups). - - {plugin: kolla_inventory, image: cyborg, match: prefix, reason: "not deployed in this series"} - - {plugin: kolla_inventory, image: tacker, match: prefix, reason: "not deployed in this series"} + - {plugin: kolla_inventory, image: cyborg, match: prefix, reason: "not deployed in this series"} + - {plugin: kolla_inventory, image: tacker, match: prefix, reason: "not deployed in this series"} - {plugin: kolla_inventory, image: telegraf, match: prefix, reason: "not deployed in this series"} - {plugin: kolla_inventory, image: collectd, match: prefix, reason: "not deployed in this series"} # --- kolla_enablement_orphan --- # OSISM-invented enable flags with no upstream counterpart by design (not a # removed-upstream service). Truthy, so they surface in the truthy-only scope. - - {plugin: kolla_enablement_orphan, image: common, reason: "OSISM-invented flag; no upstream counterpart by design"} + - {plugin: kolla_enablement_orphan, image: common, reason: "OSISM-invented flag; no upstream counterpart by design"} - {plugin: kolla_enablement_orphan, image: kolla_operations, reason: "OSISM-invented flag; no upstream counterpart by design"} diff --git a/src/kolla-drift-config.yml b/src/kolla-drift-config.yml index e46ee27..599bc41 100644 --- a/src/kolla-drift-config.yml +++ b/src/kolla-drift-config.yml @@ -1,24 +1,25 @@ +--- # Default config for the drift detector. # Override with `--config PATH` if needed. Reads remote (GitHub) by default; # pass `--base-dir DIR` (repeatable) to read OSISM repos from local checkouts. remote: - github_raw: https://raw.githubusercontent.com/ - github_api: https://api.github.com/repos/ + github_raw: https://raw.githubusercontent.com/ + github_api: https://api.github.com/repos/ default_owner: osism - branch: main + branch: main sources: - kolla: {owner: openstack, branch: stable/2025.2} + kolla: {owner: openstack, branch: stable/2025.2} kolla_ansible: {owner: openstack, branch: stable/2025.2} release_version: latest plugins: - kolla_version_chain_inner: {enabled: true} + kolla_version_chain_inner: {enabled: true} kolla_version_chain_upstream: {enabled: true} - kolla_inventory: {enabled: true} - kolla_enablement_build: {enabled: true} - kolla_enablement_orphan: {enabled: true} - kolla_orphan_config: {enabled: true} - kolla_secrets_orphan: {enabled: true} + kolla_inventory: {enabled: true} + kolla_enablement_build: {enabled: true} + kolla_enablement_orphan: {enabled: true} + kolla_orphan_config: {enabled: true} + kolla_secrets_orphan: {enabled: true} From 59bccdce2a396a38fc078af090f2455fac7dca72 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 18:15:26 +0200 Subject: [PATCH 11/12] Document the kolla drift detector The drift detector shipped with per-commit slices of docs/check-kolla-drift.md spread across the framework and plugin commits. Those slices had drifted out of sync with the code: two of the seven plugins (kolla_orphan_config, kolla_secrets_orphan) were never documented at all, the "Adding a plugin" guide listed the wrong plugin protocol (omitting the required SUMMARY/REMEDIATION names and showing the obsolete allowlist.match/as_allowlisted idiom instead of allowlist.apply), and several passages referenced removed concepts (the dropped paths.* config, a "deferred" orphan signal, and a docs/findings/ file that does not exist in this repo). Plugin sections also followed no common template and were ordered inconsistently. The slices were dropped from the earlier commits; this commit adds one authoritative document in their place. It presents all seven plugins in lifecycle order (enabled -> built -> version-pinned -> deployed, matching the report) under a uniform template -- description, --plugin command, the inputs it reads, and the fix/allowlist remediation -- and documents the framework: running it, remote/--base-dir input resolution, per-repo source overrides, the allowlist (fields, exact/prefix match, stale detection), and an accurate "Adding a plugin" guide. Time-sensitive "as of writing it flags X" examples were removed so the doc does not go stale as the consumer repos are reconciled. Documentation only; no code or test change. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi --- docs/check-kolla-drift.md | 298 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 298 insertions(+) create mode 100644 docs/check-kolla-drift.md diff --git a/docs/check-kolla-drift.md b/docs/check-kolla-drift.md new file mode 100644 index 0000000..796c539 --- /dev/null +++ b/docs/check-kolla-drift.md @@ -0,0 +1,298 @@ +# check-kolla-drift + +A small detector for accidental drift in the OSISM kolla container-image +toolchain. It compares the OSISM consumer repos (`defaults`, `release`, +`generics`, the container-image repos) against upstream `openstack/kolla` and +`openstack/kolla-ansible`, and flags divergence *when it is introduced* — before +it surfaces at deploy or gate time. + +Each check is a plugin; the framework handles config, local/remote source reads, +the allowlist, and output. The checks follow a service's lifecycle — +**enabled → built → version-pinned → deployed** — and the report renders them in +that order. + +## Run it locally + + python3 src/check-kolla-drift.py # all enabled plugins + python3 src/check-kolla-drift.py --plugin kolla_inventory + python3 src/check-kolla-drift.py --base-dir ~/src/osism # read local checkouts + python3 src/check-kolla-drift.py --format json # JSONL for tooling + +Exit codes: **0** = no actionable drift, **1** = drift found (or a stale +allowlist entry), **2** = input/config error. By default every repo is read from +GitHub, so the detector runs anywhere including CI; see "Input resolution". + +## Input resolution: remote by default, `--base-dir` for local + +By default every repo is read from **GitHub** (`remote.branch`, default `main`; +per-repo owner/branch via `sources:`). No local checkout is needed. + +To read local checkouts, pass `--base-dir DIR` (repeatable). For each repo a +plugin needs, the dirs are searched **in order** and the first one containing +`` (the hyphenated repo name, e.g. `container-image-kolla-ansible`) +wins: + + python3 src/check-kolla-drift.py --base-dir ~/src/osism --base-dir ~/src/openstack + +A repo not found under any `--base-dir` is a **hard error** (so a +misconfiguration can't silently compare the wrong thing). Pass +`--remote-fallback` to fetch not-found repos from GitHub instead — this is the CI +shape: OSISM repos resolve to the checked-out change locally, upstream falls back +to remote. `--remote-only` ignores `--base-dir` and fetches everything remotely. + +**What gets read from where is always logged** (to stderr, unless `--quiet`), +before any comparison: + + Resolving sources (1 base dir(s)): + defaults local /home/me/src/osism/defaults [working tree, as-is] + kolla_ansible remote openstack/kolla-ansible @ stable/2025.2 ... [remote] + +Local OSISM-consumer repos are read from the **working tree as-is** (the change +under test). Upstream `kolla`/`kolla-ansible` are **pinned** (`sources:`): if a +`--base-dir` holds them as a **git clone**, they are read from git objects at the +pinned/release refs (no checkout, offline, no GitHub rate limit; a missing ref is +a loud error, and a non-`origin` remote such as `gerrit` is searched too). A +pinned repo whose `--base-dir` dir is not a git clone falls back to remote (with +`--remote-fallback`) or is a hard error. With no local clone they are read +remotely at their refs. + +### Per-repo source overrides (`sources:`) + +By default every repo is read from `default_owner` (osism) at `remote.branch`. A +`sources:` entry overrides the owner and/or ref for one repo: + + sources: + kolla: {owner: openstack, branch: stable/2025.2} + +A repo with a set `branch` is **pinned**: it is always read remotely at that ref +(or from git objects in a local clone), so the result is deterministic regardless +of any local checkout's current branch. + +## The checks + +A kolla service moves through four **stages** in OSISM, and each check guards a +stage or the transition into it: + +1. **enabled** — OSISM turns the service on with an `enable_X` flag in + `osism/defaults`. +2. **built** — OSISM builds its container image (the service is in the release + build set). +3. **version-pinned** — the built image's tag is pinned through the kolla-ansible + versions template and the producer's SBOM map. +4. **deployed** — the service has ansible inventory groups to deploy into. + +The seven plugins run in that order, and the report renders them the same way. +Each opens with the stage it guards. Each is independent and can be run alone +with `--plugin `; every finding can be suppressed with an allowlist entry +(see "Allowlist"). Service names are compared as **key spaces**, normalising +`-`↔`_`; the checks never derive keys from output-variable or image names. + +### Plugin: kolla_enablement_orphan + +**Enabled — an enable flag must name a service upstream still defines.** +Flags an OSISM enable flag (`osism/defaults` `all/099-kolla.yml` `enable_X`) whose service `X` is +**absent from upstream kolla-ansible's enable-defaults at every supported +release** — the service was removed or renamed upstream, so the flag is stale. +Orphan means absent from the **union** across the release range; a service still +defined in any in-range release is not an orphan. Per-release upstream keys come +from the **top-level** enable-defaults only, whose location moves between +releases (the monolithic `ansible/group_vars/all.yml` at 2024.1/2024.2/2025.1, or +the split `ansible/group_vars/all/*.yml` dir at 2025.2+); `roles/*/defaults/`, +tasks, tests, and releasenotes are never read, so a reference-only mention does +not count as a definition. + +The plugin ships `SCOPE = "explicit"`: it considers every `enable_X` flag with a +literal value, including dead `enable_X: "no"` cleanup flags for retired services, +which it surfaces so they can be removed. The OSISM-invented `enable_common` and +`enable_kolla_operations` have no upstream counterpart by design and are kept +allowlisted. (Only top-level `enable_` defaults are matched, never +component-prefixed role defaults; if an OSISM flag ever needs the latter the check +fails closed and loud — a false-positive orphan, never a missed one — and the fix +is to extend `upstream_enable_keys` to union role-default keys.) + + python3 src/check-kolla-drift.py --plugin kolla_enablement_orphan + +- **Reads:** `osism/defaults` `all/099-kolla.yml`; `openstack/kolla-ansible` + enable-defaults per resolved release ref. +- **Fix:** remove the stale `enable_` from `osism/defaults`, or migrate it + to the upstream replacement; if it is an OSISM invention, add an allowlist entry + with a reason. + +### Plugin: kolla_orphan_config + +**Enabled — companion config must not outlive its service.** For each service +`kolla_enablement_orphan` reports as orphaned (minus the allowlisted OSISM +inventions), this flags every dead companion and image-definition var +(`_*`) across `osism/defaults` `all/*.yml`, grouped by file. The +`enable_` flag itself is left to `kolla_enablement_orphan`; this catches the +`_port`, `_tag`, `_*_image` and similar vars that would +otherwise dangle after the flag is removed. + + python3 src/check-kolla-drift.py --plugin kolla_orphan_config + +- **Reads:** `osism/defaults` `all/*.yml` (orphan set derived via + `kolla_enablement_orphan`). +- **Fix:** remove these vars from the listed `osism/defaults` file, or allowlist + any intentionally kept (an OSISM invention with no upstream service). + +### Plugin: kolla_secrets_orphan + +**Enabled — a secret must not outlive its service.** Per release, compares the top-level keys of the OSISM +kolla secrets template (`cfg-cookiecutter` +`environments/kolla/secrets.yml.`) against upstream kolla-ansible's +authoritative `etc/kolla/passwords.yml` at that release's resolved ref. A key in +the OSISM template with no upstream counterpart is an orphaned secret (the service +that consumed it was removed upstream). The comparison is per release, not +unioned. Keys are extracted with a line regex rather than a YAML load, because the +template values are jinja/cookiecutter placeholders that do not parse as YAML; a +release with no OSISM template is skipped, not an error. + + python3 src/check-kolla-drift.py --plugin kolla_secrets_orphan + +- **Reads:** `cfg-cookiecutter` `environments/kolla/secrets.yml.`; + `openstack/kolla-ansible` `etc/kolla/passwords.yml` per resolved release ref. +- **Fix:** remove the orphaned secret from the cfg-cookiecutter template (and + regenerate environments), or allowlist it if it is an OSISM-invented secret. + +### Plugin: kolla_enablement_build + +**Enabled → built — an enabled service must be in the build set.** For each supported release R, an OSISM-enabled kolla service +(`osism/defaults` `all/099-kolla.yml` `enable_X: "yes"`) that is **buildable +upstream** at R (a `docker/X` dir exists in `openstack/kolla` at R's ref) must +appear in OSISM's **build set** for R — `osism/release` +`latest/openstack-R.yml` under `infrastructure_projects` or `openstack_projects`. +An enabled, buildable service absent from the build set is flagged: nothing builds +its image. The upstream `docker/` universe is the scope filter, so feature flags +with no image (`enable_neutron_qos`, …) are naturally out of scope — no alias +table. + +This check is **range-aware**: the supported releases derive from the +`release/latest/openstack-*.yml` file set (override with the `releases:` config +key). Each release resolves to a real upstream ref via a probe — +`stable/` → `unmaintained/` → `-eol` → `-eom`, first that exists — +because OSISM keeps building releases upstream OpenStack has moved past EOL. +Override a specific ref with `release_refs: {kolla: {"2024.2": "2024.2-eol"}}`. A +release that resolves to no ref is a hard error, not a silent skip. + + python3 src/check-kolla-drift.py --plugin kolla_enablement_build + +- **Reads:** `osism/defaults` `all/099-kolla.yml`; `osism/release` + `latest/openstack-.yml`; `openstack/kolla` `docker/` per resolved ref. +- **Fix:** add the service to `infrastructure_projects` or `openstack_projects` + in the release file, or allowlist it if it is intentionally not built. + +### Plugin: kolla_version_chain_upstream + +**Built → version-pinned — a built service must have a version pin.** Lists the top-level service +directories of `openstack/kolla` `docker/` (pinned `stable/2025.2`) and flags any +whose normalised name has **no** `versions['']` line in the kolla-ansible +template — an upstream-built service with no version pin wired. The comparison is +one-way (upstream → template) and does not fold in the producer's keys, so a +service present in the producer but missing a template line still flags here. + + python3 src/check-kolla-drift.py --plugin kolla_version_chain_upstream + +- **Reads:** `openstack/kolla` `docker/`; + `container-image-kolla-ansible` `files/src/templates/versions.yml.j2`. +- **Fix:** add a `versions['']` line to the template to pin the image (and + wire the producer); if intentionally unpinned, add an allowlist entry with a + reason. Build-base layers (`base`, `openstack_base`) and naming/variant or + supporting images with no distinct version key are carried in the shipped + allowlist. + +### Plugin: kolla_version_chain_inner + +**Version-pinned — a version pin must resolve to a value.** The producer +(`container-images-kolla`) emits an SBOM mapping each tracked service to a +concrete version via `SBOM_IMAGE_TO_VERSION` in `src/tag-images-with-the-version.py`. +The consumer (`container-image-kolla-ansible`) renders one line per service in +`files/src/templates/versions.yml.j2`: + + kolla__version: "{{ versions['']|default(openstack_version) }}" + +A `versions['']` key referenced in the template but **absent** from the SBOM +map is never produced, so the line silently falls back to the coarse +`openstack_version` — an inert pin, with no error. This plugin flags exactly those +keys, and **classifies** each: if OSISM actually deploys the service (enabled in +`099-kolla.yml` and buildable in `openstack/kolla` `docker/`) the right fix is to +**wire the SBOM key**; otherwise the template line is dead and should be +**removed**. The report renders the two buckets as separate blocks, each pointing +at the repo to edit. + + python3 src/check-kolla-drift.py --plugin kolla_version_chain_inner + +- **Reads:** `container-image-kolla-ansible` `files/src/templates/versions.yml.j2`; + `container-images-kolla` `src/tag-images-with-the-version.py`; + `osism/defaults` `all/099-kolla.yml`; `openstack/kolla` `docker/`. +- **Fix:** wire the key into `SBOM_IMAGE_TO_VERSION` if OSISM deploys it, else + remove the dead template line; allowlist keys meant to default. + +### Plugin: kolla_inventory + +**Deployed — a deployed service must have inventory groups.** Compares the ansible group names (INI section headers) of upstream +`openstack/kolla-ansible` `ansible/inventory/multinode` (pinned `stable/2025.2`) +against the union of group names in the OSISM inventory files +`generics/inventory/50-kolla` and `51-kolla`. A group present upstream but absent +locally is flagged (one-way, upstream → local) — the failure mode behind +generics#599 (neutron 2025.2 groups) and #601 (ironic-dnsmasq). Each flagged +group's upstream **members** (child groups or hosts) ride in the `expected` field, +so a red line tells a maintainer what to add. Subsumes the retired +`check-kolla-inventory.py`. + + python3 src/check-kolla-drift.py --plugin kolla_inventory + +- **Reads:** `openstack/kolla-ansible` `ansible/inventory/multinode`; + `generics/inventory/50-kolla` and `51-kolla`. +- **Fix:** add the group and its members to `50/51-kolla`; if the service is + intentionally not deployed, add an allowlist entry with a reason (often + `match: prefix` — see below). Base-infra groups the OSISM overlay assumes exist + in the environment inventory are carried in the shipped allowlist. + +## Allowlist + +`src/kolla-drift-allowlist.yml` holds structurally-intentional exceptions — not a +baseline of everything currently drifted. Each entry needs `plugin`, `image`, and +a non-empty `reason`; optional `alias` and `found_src` narrow the match. + +By default an entry matches a drift's `image` **exactly**. An entry may set +`match: prefix` to also match sub-groups, using the ansible group-name separators +`-` and `:` as boundaries: + + - {plugin: kolla_inventory, image: cyborg, match: prefix, reason: not deployed} + +matches `cyborg`, `cyborg:children`, and `cyborg-agent:children`, but **not** +`cyborgx`. `image` must be non-empty in either mode. + +**Stale entries are a hard error.** An allowlist entry (exact or prefix) that +matches no real drift is reported as a **stale allowlist** entry and makes the run +exit non-zero — a dead exception is a bug, not a silent no-op. Detection is scoped +to the plugins that actually ran (a `--plugin` run only judges that plugin's +entries) and is skipped under `--no-allowlist`. Remove an entry once its drift is +fixed. + +## Adding a plugin + +A plugin is a module under `src/osism_drift/drift/` exposing: + + NAME: str + DESCRIPTION: str + INPUT_FILES: list[tuple[str, str]] # (repo_key, repo_relative_path) — for --help + SUMMARY: str # "{n} ..." lead line for the report (must contain {n}) + REMEDIATION: str # the Fix: line for the report + def run(config, allowlist, verbose: bool = False) -> list[DriftEntry] + +`run()` reads inputs via `source.read(repo_key, rel_path, config)` / +`read_optional` / `list_dir` (GitHub by default, or a local checkout discovered +under a `--base-dir`; the `repo_key` underscores convert to hyphens in the +dir/URL). Build a `DriftEntry` (`osism_drift.model`) per finding and pass each +through the allowlist in one step: + + entry = allowlist.apply(entry) # returns it unchanged, or as an allowlisted copy + +Register the module in `src/osism_drift/drift/__init__.py` (`PLUGINS = [...]`, in +lifecycle order) **and** add a `plugins.: {enabled: true}` stanza to +`src/kolla-drift-config.yml` (a plugin absent from config is skipped; a registry +test guards that every registered plugin is enabled). A new repo needs no config — +it is fetched remotely by default, or discovered by name under a `--base-dir`. +Mirror the test pattern in `tests/kolla_drift/`: synthetic fixtures laid out as +`fixtures//...` plus a `Config(..., base_dirs=(str(FIXT),))`. From 6c0a6d1496b077c5b694a1e2f4bcfee8f2d96175 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Mon, 29 Jun 2026 20:55:02 +0200 Subject: [PATCH 12/12] Run kolla drift unit tests in CI The kolla-drift-test tox environment runs the pytest unit suite for the drift-detection code (tests/kolla_drift/, 146 tests, hermetic fixture-based, no network). It was defined in tox.ini but referenced by no Zuul job, so the unit tests never ran in CI. Add a generics-tox-kolla-drift-test job and wire it into the check pipeline so the unit tests gate every PR, plus periodic-daily for parity with the other jobs. The existing generics-tox-kolla-drift job (the live drift detector) stays periodic-only, which is correct: it inspects live upstream state and is not suitable as a PR gate. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi --- .zuul.yaml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.zuul.yaml b/.zuul.yaml index d036f7b..c127423 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -41,12 +41,19 @@ vars: tox_envlist: kolla-drift +- job: + name: generics-tox-kolla-drift-test + parent: tox + vars: + tox_envlist: kolla-drift-test + - project: merge-mode: squash-merge default-branch: main check: jobs: - generics-tox-check + - generics-tox-kolla-drift-test - generics-tox-seed - generics-tox-test-latest - generics-tox-test-stable @@ -58,8 +65,9 @@ periodic-daily: jobs: - generics-tox-check - - generics-tox-seed - generics-tox-kolla-drift + - generics-tox-kolla-drift-test + - generics-tox-seed - generics-tox-test-latest - generics-tox-test-stable - generics-tox-test-stable-legacy