Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/drift-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ plugins:
kolla_image_orphan: {enabled: true}
kolla_secrets_orphan: {enabled: true}
kolla_source_ref_phase: {enabled: true}
kolla_version_gate_orphan: {enabled: true}
2 changes: 2 additions & 0 deletions src/osism_drift/drift/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
kolla_source_ref_phase,
kolla_version_chain_inner,
kolla_version_chain_upstream,
kolla_version_gate_orphan,
release_vs_manager,
role_shadows,
role_unpinned,
Expand All @@ -26,6 +27,7 @@
kolla_groupvars_missing,
kolla_mirror_verbatim,
kolla_orphan_config,
kolla_version_gate_orphan,
kolla_image_orphan,
kolla_secrets_orphan,
kolla_enablement_build,
Expand Down
136 changes: 136 additions & 0 deletions src/osism_drift/drift/kolla_version_gate_orphan.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
"""kolla_version_gate_orphan: osism/defaults still names a retired release.

Retirement removes a release from the supported range, but osism/defaults keeps
two kinds of reference to it that nothing fails on afterwards:

- a **version gate** -- `openstack_version in ['A', 'B']` -- selecting a value
for particular releases. Once a listed release is gone, that branch can never
be taken again.
- a **per-release file** -- `all/010-<release>.yml` -- a backward-compat layer
extending the 001 mirror for an older supported release. Its own header says
to delete it when that release leaves the range.

Both are dead rather than broken, so nothing prompts the cleanup: a `victoria`
gate sat in the image catalogue for years unnoticed. This compares every
reference against the supported range (the release/latest/openstack-*.yml file
set) and reports the ones that name a release outside it.

The mirror image of kolla_source_ref_phase: that plugin catches a reference that
should have moved *forward* when a release changed phase, this one a reference
that should have been *dropped* when a release was retired.

Findings are advisory. A gate on a release that can no longer be deployed may
still be load-bearing for an upgrade *from* it, so "outside the supported range"
means "confirm this is still needed", not "this is a defect". Deliberate keeps
belong in the allowlist.

Scope is deliberately osism/defaults only. The same redis/valkey cutover is
gated in shell `case` statements in testbed, metalbox and
container-image-kolla-ansible, but a `case` parser would have to handle `;;&`,
`;&`, nested case, arms sharing a line, quoting and heredocs -- and a subtle
mis-parse there yields a silent false negative, the failure this check exists to
prevent. Those copies are covered by the retirement checklist in the guide
instead, which names each file. Same call as kolla_source_ref_phase made for the
hardcoded requirements ref in container-images-kolla scripts/002-generate.sh.
"""

from osism_drift import enablement, source
from osism_drift.model import DriftEntry

NAME = "kolla_version_gate_orphan"
DESCRIPTION = (
"Flag osism/defaults version gates and per-release compat files that name an "
"OpenStack release outside the supported range, so their content is dead."
)
INPUT_FILES = [
("defaults", "all/*.yml"),
("release", "latest/openstack-*.yml (the supported release range)"),
]
SUMMARY = (
"{n} version gates naming a release outside the supported range, so the "
"branch they select can no longer be taken:"
)
REMEDIATION = (
"drop the retired release from the gate; if that leaves the condition "
"matching nothing, remove the conditional and keep the value that remains. "
"Allowlist a gate deliberately kept for upgrades from that release."
)
# Per-entry overrides for the file findings, so they render as their own block:
# deleting a whole file is a different action from editing an expression.
FILE_SUMMARY = (
"{n} per-release compat files for a release outside the supported range, "
"which their own header says to delete at this point:"
)
FILE_REMEDIATION = (
"delete the file. It exists only to extend the 001 mirror for an older "
"release while that release is supported; once it leaves the range every key "
"in the file is dead. Allowlist it if it is deliberately kept."
)

EXPECTED_SRC = "osism/release latest/openstack-*.yml (supported releases)"
_DEFAULTS_DIR = "all"


def stale_releases(named, supported) -> list:
"""The `named` releases that are not in `supported`, order preserved."""
return [r for r in named if r not in supported]


def _entry(image, found, found_src, supported, summary, remediation):
"""One advisory finding: `image` still names `found`, which is unsupported."""
return DriftEntry(
plugin=NAME,
image=image,
alias=image,
expected=", ".join(sorted(supported)),
found=found,
expected_src=EXPECTED_SRC,
found_src=found_src,
summary=summary,
remediation=remediation,
severity="advisory",
)


def run(config, allowlist, verbose: bool = False) -> list[DriftEntry]:
"""Return advisory drifts for defaults references to unsupported releases."""
supported = set(enablement.release_range(config))
drifts = []
for filename in sorted(source.list_dir("defaults", _DEFAULTS_DIR, config)):
if not filename.endswith(".yml"):
continue

target = enablement.per_release_file_target(filename)
if target is not None and target not in supported:
drifts.append(
allowlist.apply(
_entry(
image=filename,
found=target,
found_src=f"osism/defaults {_DEFAULTS_DIR}/",
supported=supported,
summary=FILE_SUMMARY,
remediation=FILE_REMEDIATION,
)
)
)

body = source.read("defaults", f"{_DEFAULTS_DIR}/{filename}", config)
gates = enablement.parse_version_gates(body)
for var, named in sorted(gates.items()):
dead = stale_releases(named, supported)
if not dead:
continue
drifts.append(
allowlist.apply(
_entry(
image=var,
found=", ".join(dead),
found_src=f"osism/defaults {_DEFAULTS_DIR}/{filename}",
supported=supported,
summary=SUMMARY,
remediation=REMEDIATION,
)
)
)
return drifts
76 changes: 75 additions & 1 deletion src/osism_drift/enablement.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
build keys use hyphens) so every cross-space comparison is on one form.
"""

import re

import yaml

from osism_drift import secrets_map, source
Expand All @@ -16,6 +18,78 @@ def canon(name: str) -> str:
return name.replace("-", "_")


# The one idiom osism/defaults uses to make a value depend on the release. The
# `not in` form is matched too: whether a named release is still supported does
# not depend on the sense of the test.
_VERSION_GATE = re.compile(r"openstack_version\s+(?:not\s+)?in\s*\[([^\]]*)\]")
_GATE_LITERAL = re.compile(r"""['"]\s*([^'"]+?)\s*['"]""")


def _value_strings(value):
"""Yield every string inside `value`, walking mappings and lists.

A gate can sit anywhere in a var's value, not only in a scalar: the defaults
carry nested structures (container-dimension maps, option lists).
"""
if isinstance(value, str):
yield value
elif isinstance(value, dict):
for item in value.values():
yield from _value_strings(item)
elif isinstance(value, list):
for item in value:
yield from _value_strings(item)


def parse_version_gates(body: bytes) -> dict:
"""{var: [release, ...]} for every openstack_version gate in a defaults file.

Jinja lives only in values, which safe_load returns as strings, so the gate
is matched textually inside them rather than evaluated. Only the literals
inside the bracket list are returned; everything else in the expression --
other conditions, filters, nested parentheses -- is ignored on purpose,
keeping the parse to one stable construct instead of trying to understand
jinja. A var whose value holds no gate is absent from the result.
"""
data = yaml.safe_load(body) or {}
if not isinstance(data, dict):
return {}
gates = {}
for key, value in data.items():
if not isinstance(key, str):
continue
releases = []
for text in _value_strings(value):
for listed in _VERSION_GATE.findall(text):
releases.extend(_GATE_LITERAL.findall(listed))
if releases:
gates[key] = list(dict.fromkeys(releases))
return gates


_PER_RELEASE_PREFIX = "010-"


def per_release_file(release: str) -> str:
"""The all/ path holding backward-compat keys for `release` (parent spec D8).

One definition of the convention, so the check that fills these files
(groupvars_home) and the check that retires them cannot disagree about the
name.
"""
return f"all/{_PER_RELEASE_PREFIX}{release}.yml"


def per_release_file_target(filename: str) -> str | None:
"""The release an all/ `filename` carries compat keys for, else None.

Inverse of per_release_file, over a bare filename as list_dir yields it.
"""
if not filename.startswith(_PER_RELEASE_PREFIX) or not filename.endswith(".yml"):
return None
return filename[len(_PER_RELEASE_PREFIX) : -len(".yml")] or None


def parse_enable_flags(body: bytes) -> dict:
"""{service_id: raw_value} for every enable_<id> in an OSISM vars file."""
data = yaml.safe_load(body) or {}
Expand Down Expand Up @@ -340,7 +414,7 @@ def groupvars_home(key, newest, newest_keys, dropped_map):
return ("all/001-kolla-defaults.yml", f"upstream defines it at {newest}")
L = dropped_map.get(key)
if L:
return (f"all/010-{L}.yml", f"upstream dropped by {newest}; last in {L}")
return (per_release_file(L), f"upstream dropped by {newest}; last in {L}")
return None


Expand Down
75 changes: 75 additions & 0 deletions tests/kolla_drift/test_enablement_version_gates.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
from osism_drift import enablement


def test_parses_a_gate_into_its_listed_releases():
body = b"""
enable_redis: "{{ 'yes' if openstack_version in ['2024.1', '2024.2'] else 'no' }}"
"""
assert enablement.parse_version_gates(body) == {
"enable_redis": ["2024.1", "2024.2"]
}


def test_parses_the_not_in_form_the_same_way():
"""Whether a named release is still supported does not depend on the sense."""
body = b"enable_x: \"{{ 'a' if openstack_version not in ['2024.1'] else 'b' }}\"\n"
assert enablement.parse_version_gates(body) == {"enable_x": ["2024.1"]}


def test_parses_a_named_release_literal():
body = b"mariadb_image: \"{{ 'mariadb' if openstack_version in ['victoria'] }}\"\n"
assert enablement.parse_version_gates(body) == {"mariadb_image": ["victoria"]}


def test_ignores_the_rest_of_the_expression():
"""Only the bracket list is read; other conditions and filters are not parsed."""
body = (
b"horizon_listen_port: \"{{ '8080' if (enable_haproxy | bool and "
b"openstack_version not in ['2024.1', '2025.1']) else horizon_port }}\"\n"
)
assert enablement.parse_version_gates(body) == {
"horizon_listen_port": ["2024.1", "2025.1"]
}


def test_finds_a_gate_nested_inside_a_structured_value():
"""Values are not always scalars; a gate can sit inside a map or a list."""
body = b"""
dims:
ulimits:
nofile: "{{ 1024 if openstack_version in ['2024.2'] else 2048 }}"
opts:
- "{{ 'x' if openstack_version in ['victoria'] else 'y' }}"
"""
assert enablement.parse_version_gates(body) == {
"dims": ["2024.2"],
"opts": ["victoria"],
}


def test_var_without_a_gate_is_absent():
body = b'plain: "{{ docker_image_url }}nova-api"\nother: 5\n'
assert enablement.parse_version_gates(body) == {}


def test_deduplicates_repeated_literals_preserving_order():
body = (
b"v: \"{{ 'a' if openstack_version in ['2025.1', '2024.1'] else "
b"('b' if openstack_version in ['2024.1'] else 'c') }}\"\n"
)
assert enablement.parse_version_gates(body) == {"v": ["2025.1", "2024.1"]}


def test_empty_and_non_mapping_documents_yield_nothing():
assert enablement.parse_version_gates(b"") == {}
assert enablement.parse_version_gates(b"- a\n- b\n") == {}


def test_per_release_file_round_trips():
assert enablement.per_release_file("2024.2") == "all/010-2024.2.yml"
assert enablement.per_release_file_target("010-2024.2.yml") == "2024.2"


def test_per_release_file_target_rejects_other_files():
for name in ("001-kolla-defaults.yml", "099-kolla.yml", "010-.yml", "010-2024.2"):
assert enablement.per_release_file_target(name) is None, name
Loading