From 80bb361c707aa2d0f4e669a88d0a7319b7466113 Mon Sep 17 00:00:00 2001 From: Elior Erez Date: Fri, 14 Aug 2026 12:48:05 -0400 Subject: [PATCH 1/3] OSAC-4050: Universal YAML/JSON structural coverage, k8s manifests as the rich layer graphify/extractors/k8s_manifest.py: recognizes real Kubernetes resources (apiVersion+kind+metadata, value-validated) and extracts owner-reference edges (both standard metadata.ownerReferences and this repo's own osac.openshift.io/owner-reference annotation convention, plus osac.openshift.io/tenant scoping -- confirmed against osac/.claude/rules/architecture-patterns.md and the operator's own Go source), ConfigMap/Secret references, *Ref/*Refs CRD cross-references (confirmed against real usage in osac-operator's CRD samples), and label-selector matches via a shared label-hub node (confirmed exact against a real Service/Deployment pair in osac-operator/config/console-proxy/). Direction change confirmed explicitly mid-implementation: rather than stay k8s-only, ANY YAML or JSON with no recognized schema now also gets a generic structural walk (graphify/extractors/yaml_generic.py, graphify/extractors/json_generic.py) instead of staying invisible -- layered under the k8s-specific extractor via graphify/extractors/yaml_dispatch.py, matching how extract_json already layers config-JSON extraction over a (previously empty, now generic) fallback for data JSON. detect.py now routes ALL .yaml/.yml to FileType.CODE unconditionally, matching .json's existing precedent. Go-templated Helm chart YAML (confirmed empirically to break the tree-sitter-yaml grammar outright, producing a root ERROR node rather than a partial tree) is detected per-document and skipped with a one-line warning, never crashed on or walked for garbage structure. Real, measured corpus impact against the actual osac repo (before/after on the same commit): 96,879 -> 396,492 nodes, query wall-clock 26.2s -- both well past the established ceiling (170K nodes / 10s). Root-caused: 70% of the new nodes come from one vendored third-party tree (osac-aap/vendor/ansible_collections) that contributed nothing under the old YAML-is-a-document behavior. Estimated ~118K nodes with that tree excluded, comfortably under ceiling -- the fix is a .graphifyignore change in the osac repo, not a defect in this extractor; shipping this PR now on that basis, per explicit decision. Full test suite: 4390 passed, 0 failures (one pre-existing, unrelated flaky test confirmed passing in isolation). --- graphify/detect.py | 11 +- graphify/extract.py | 13 + graphify/extractors/_yaml_cst.py | 153 +++++++++ graphify/extractors/json_config.py | 27 +- graphify/extractors/json_generic.py | 119 +++++++ graphify/extractors/k8s_manifest.py | 453 +++++++++++++++++++++++++++ graphify/extractors/yaml_dispatch.py | 119 +++++++ graphify/extractors/yaml_generic.py | 120 +++++++ pyproject.toml | 15 +- tests/test_extract.py | 27 +- tests/test_k8s_manifest.py | 355 +++++++++++++++++++++ tests/test_manifest_ingest.py | 9 +- uv.lock | 26 +- 13 files changed, 1420 insertions(+), 27 deletions(-) create mode 100644 graphify/extractors/_yaml_cst.py create mode 100644 graphify/extractors/json_generic.py create mode 100644 graphify/extractors/k8s_manifest.py create mode 100644 graphify/extractors/yaml_dispatch.py create mode 100644 graphify/extractors/yaml_generic.py create mode 100644 tests/test_k8s_manifest.py diff --git a/graphify/detect.py b/graphify/detect.py index ab8e6b0111..c8a4b7b6fb 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -41,8 +41,15 @@ class FileType(str, Enum): _MTIME_COARSE_S = 2.0 _MTIME_SUBSECOND_S = 0.05 -CODE_EXTENSIONS = {'.py', '.ts', '.tsx', '.mts', '.cts', '.js', '.jsx', '.mjs', '.cjs', '.ejs', '.ets', '.go', '.rs', '.java', '.groovy', '.gradle', '.cpp', '.cc', '.cxx', '.c', '.h', '.hpp', '.cu', '.cuh', '.metal', '.rb', '.rake', '.swift', '.kt', '.kts', '.cs', '.scala', '.php', '.lua', '.luau', '.toc', '.zig', '.ps1', '.psm1', '.psd1', '.ex', '.exs', '.m', '.mm', '.jl', '.vue', '.svelte', '.astro', '.dart', '.v', '.sv', '.svh', '.sql', '.r', '.f', '.F', '.f90', '.F90', '.f95', '.F95', '.f03', '.F03', '.f08', '.F08', '.pas', '.pp', '.dpr', '.dpk', '.lpr', '.inc', '.dfm', '.lfm', '.lpk', '.sh', '.bash', '.json', '.tf', '.tfvars', '.hcl', '.dm', '.dme', '.dmi', '.dmm', '.dmf', '.sln', '.slnx', '.csproj', '.fsproj', '.vbproj', '.xaml', '.razor', '.cshtml', '.cls', '.trigger'} -DOC_EXTENSIONS = {'.md', '.mdx', '.qmd', '.skill', '.txt', '.rst', '.html', '.yaml', '.yml'} +# .yaml/.yml unconditionally CODE (OSAC-4050) -- matches .json's existing +# precedent exactly (extract_yaml/extract_json each layer a rich, +# recognized-schema extractor over a generic structural fallback, so +# nothing YAML/JSON-shaped is ever invisible to the graph; the user +# confirmed this universal-coverage direction explicitly, overriding the +# ticket's original k8s-only scope, having been told plainly that +# unrecognized content gets low-value/noisy nodes this way). +CODE_EXTENSIONS = {'.py', '.ts', '.tsx', '.mts', '.cts', '.js', '.jsx', '.mjs', '.cjs', '.ejs', '.ets', '.go', '.rs', '.java', '.groovy', '.gradle', '.cpp', '.cc', '.cxx', '.c', '.h', '.hpp', '.cu', '.cuh', '.metal', '.rb', '.rake', '.swift', '.kt', '.kts', '.cs', '.scala', '.php', '.lua', '.luau', '.toc', '.zig', '.ps1', '.psm1', '.psd1', '.ex', '.exs', '.m', '.mm', '.jl', '.vue', '.svelte', '.astro', '.dart', '.v', '.sv', '.svh', '.sql', '.r', '.f', '.F', '.f90', '.F90', '.f95', '.F95', '.f03', '.F03', '.f08', '.F08', '.pas', '.pp', '.dpr', '.dpk', '.lpr', '.inc', '.dfm', '.lfm', '.lpk', '.sh', '.bash', '.json', '.yaml', '.yml', '.tf', '.tfvars', '.hcl', '.dm', '.dme', '.dmi', '.dmm', '.dmf', '.sln', '.slnx', '.csproj', '.fsproj', '.vbproj', '.xaml', '.razor', '.cshtml', '.cls', '.trigger'} +DOC_EXTENSIONS = {'.md', '.mdx', '.qmd', '.skill', '.txt', '.rst', '.html'} PAPER_EXTENSIONS = {'.pdf'} IMAGE_EXTENSIONS = {'.png', '.jpg', '.jpeg', '.gif', '.webp', '.svg'} OFFICE_EXTENSIONS = {'.docx', '.xlsx'} diff --git a/graphify/extract.py b/graphify/extract.py index 6e1fca8541..810d9275c3 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -54,6 +54,9 @@ from graphify.extractors.sql import extract_sql # noqa: F401 from graphify.extractors.terraform import extract_terraform # noqa: F401 from graphify.extractors.verilog import extract_verilog # noqa: F401 +from graphify.extractors.k8s_manifest import extract_k8s_manifest # noqa: F401 +from graphify.extractors.yaml_generic import extract_generic_structure as extract_yaml_generic # noqa: F401 +from graphify.extractors.yaml_dispatch import extract_yaml # noqa: F401 from graphify.extractors.zig import extract_zig # noqa: F401 from graphify.security import sanitize_metadata from graphify.paths import disambiguate_ambiguous_candidates @@ -4843,6 +4846,14 @@ def add_existing_edge(edge: dict) -> None: ".sh": extract_bash, ".bash": extract_bash, ".json": extract_json, + # NOTE: OSAC-4049 (unmerged as of this writing) also dispatches + # .yaml/.yml, to extract_github_actions -- when both PRs merge these two + # entries will conflict and need combining into one dispatcher that tries + # each shape in turn (GH Actions, then k8s manifest, then the generic + # structural fallback below), not simply picking one. Flagged in this + # PR's description. + ".yaml": extract_yaml, + ".yml": extract_yaml, ".tf": extract_terraform, ".tfvars": extract_terraform, ".hcl": extract_terraform, @@ -4870,6 +4881,8 @@ def add_existing_edge(edge: dict) -> None: # extract() to tell the user which extra restores the language. _EXTRA_FOR_EXTENSION = { ".sql": "sql", + ".yaml": "yaml", + ".yml": "yaml", ".tf": "terraform", ".tfvars": "terraform", ".hcl": "terraform", diff --git a/graphify/extractors/_yaml_cst.py b/graphify/extractors/_yaml_cst.py new file mode 100644 index 0000000000..4afcbc820a --- /dev/null +++ b/graphify/extractors/_yaml_cst.py @@ -0,0 +1,153 @@ +"""Shared tree-sitter-yaml CST traversal helpers. + +Used by both ``graphify/extractors/k8s_manifest.py`` and +``graphify/extractors/yaml_generic.py`` (OSAC-4050). Originally written +once inline in ``k8s_manifest.py``, adapted with attribution from the +tree-sitter-yaml traversal in the unmerged Graphify-Labs/graphify PR #2541; +factored out here once a second real consumer (the generic structural +walker) needed the exact same helpers within the same branch. Note: +``graphify/extractors/github_actions.py`` (OSAC-4049, unmerged as of this +writing) still carries its own independent copy of an earlier version of +these same helpers, since its branch predates this module and should not be +made to depend on this one landing first -- worth deduping further once +both have merged. +""" +from __future__ import annotations + +_MAPPING_TYPES = frozenset({"block_mapping", "flow_mapping"}) +_SEQUENCE_TYPES = frozenset({"block_sequence", "flow_sequence"}) + + +def descend(node, wanted: frozenset[str]): + """Return the first descendant of *node* whose type is in *wanted*. + + YAML wraps every value in `block_node`/`flow_node` before the actual + collection, and a document adds another layer, so callers would otherwise + repeat the same two-or-three-step unwrap everywhere. + """ + if node is None: + return None + if node.type in wanted: + return node + for child in node.children: + if not child.is_named: + continue + if child.type in ("block_node", "flow_node", "document"): + found = descend(child, wanted) + if found is not None: + return found + elif child.type in wanted: + return child + return None + + +def mapping(node): + return descend(node, _MAPPING_TYPES) + + +def scalar_text(node) -> str: + """Text of the scalar at *node*, with one layer of quotes stripped.""" + if node is None: + return "" + text = node.text.decode("utf-8", errors="replace").strip() + if len(text) >= 2 and text[0] == text[-1] and text[0] in ("'", '"'): + text = text[1:-1] + return text.strip() + + +def pairs(node): + """Yield `(key, value_node, line)` for each pair of the mapping at *node*. + + *node* may be the mapping itself or any wrapper around it. Pairs whose key + is not a plain scalar are skipped rather than stringified, so they never + mint a garbage node. + """ + m = mapping(node) + if m is None: + return + for pair in m.children: + if pair.type not in ("block_mapping_pair", "flow_pair"): + continue + key_node = pair.child_by_field_name("key") + if key_node is None: + continue + key = scalar_text(key_node) + if not key: + continue + yield key, pair.child_by_field_name("value"), key_node.start_point[0] + 1 + + +def item_value(item): + """The value inside a `block_sequence_item`, without the `- ` marker.""" + if item.type != "block_sequence_item": + return item + for child in item.children: + if child.is_named: + return child + return item + + +def string_items(node) -> list[tuple[str, int]]: + """Scalars reachable from *node* as `(text, line)` -- a bare scalar, a + sequence (block or flow), or (defensively) a mapping's keys.""" + if node is None: + return [] + seq = descend(node, _SEQUENCE_TYPES) + if seq is not None: + items = [] + for item in seq.children: + if item.type not in ("block_sequence_item", "flow_node"): + continue + text = scalar_text(item_value(item)) + if text and "\n" not in text and ":" not in text: + items.append((text, item.start_point[0] + 1)) + return items + m = mapping(node) + if m is not None: + return [(key, line) for key, _value, line in pairs(m)] + text = scalar_text(node) + return [(text, node.start_point[0] + 1)] if text else [] + + +def sequence_items(node): + """Yield the item nodes of the sequence at *node*.""" + seq = descend(node, _SEQUENCE_TYPES) + if seq is None: + return + for item in seq.children: + if item.type in ("block_sequence_item", "flow_node"): + yield item + + +def all_documents(root): + """Yield the raw top-level node of every document in the file (whatever + its type -- mapping, sequence, scalar, or ERROR; callers decide what to + do with each). + + Root type is `stream` with one `document` child per resource for a + multi-document file (confirmed against a real file in this repo, + ``osac-operator/config/manager/manager.yaml``, which holds a Namespace + and a Deployment separated by `---`), vs a bare `document` (or, for a + malformed/templated file, an `ERROR` node) for a single-document file. + """ + if root.type == "stream": + docs = [c for c in root.children if c.type == "document"] + elif root.type == "document": + docs = [root] + else: + docs = [root] + for doc in docs: + yield doc + + +def all_top_level_mappings(root): + """Yield the top-level MAPPING of every document in the file (documents + that aren't mapping-shaped, or are malformed, are silently skipped -- + for callers that only care about mapping-shaped resources, e.g. a k8s + manifest). See ``all_documents`` for the version that yields every + document regardless of shape. + """ + for doc in all_documents(root): + m = mapping(doc) + if m is not None: + yield m diff --git a/graphify/extractors/json_config.py b/graphify/extractors/json_config.py index 6a9b641a9d..af1da290b1 100644 --- a/graphify/extractors/json_config.py +++ b/graphify/extractors/json_config.py @@ -202,15 +202,20 @@ def walk_object(obj_node, parent_nid: str, parent_key: str | None, doc = root if doc.type == "document" and doc.child_count > 0: doc = doc.children[0] - if doc.type == "object": - # Only AST-extract recognized config/manifest JSON. Data JSON (fixtures, - # datasets, GeoJSON, API dumps) is skipped so it doesn't explode into - # orphan key-nodes (#1224); it's left to the LLM semantic pass. - if not _is_config_json(path, doc, source): - return {"nodes": [], "edges": [], "skipped": "data json (not a config/manifest)"} + if doc.type == "object" and _is_config_json(path, doc, source): walk_object(doc, file_nid, None, 0, [0]) - else: - # Top-level array or scalar => data JSON, never a config/manifest. - return {"nodes": [], "edges": [], "skipped": "data json (non-object root)"} - - return {"nodes": nodes, "edges": edges} + return {"nodes": nodes, "edges": edges} + + # Data JSON (fixtures, datasets, GeoJSON, API dumps, or any top-level + # array/scalar) doesn't get the rich config-specific dependency/extends/ + # $ref treatment above (#1224 -- that produced hundreds of orphan + # key-nodes when applied indiscriminately). Per OSAC-4050's explicit, + # confirmed direction change, it no longer disappears from the graph + # either: fall back to a genuinely generic structural walk (no domain + # semantics), same design as graphify/extractors/yaml_generic.py. + from graphify.extractors.json_generic import extract_generic_structure as _generic_json + generic_nodes, generic_edges, truncated = _generic_json(doc, source, str_path, file_nid) + result: dict = {"nodes": nodes + generic_nodes, "edges": edges + generic_edges} + if truncated: + result["truncated"] = f"generic structural walk capped at node limit for {path.name}" + return result diff --git a/graphify/extractors/json_generic.py b/graphify/extractors/json_generic.py new file mode 100644 index 0000000000..3f1ffadce6 --- /dev/null +++ b/graphify/extractors/json_generic.py @@ -0,0 +1,119 @@ +"""Generic structural JSON extractor. + +OSAC-4050 -- the same universal-coverage direction change applied to JSON: +``extract_json()`` (graphify/extractors/json_config.py) already recognizes +config/manifest JSON (package.json, tsconfig.json, ...) via +``_is_config_json`` and gives it rich dependency/extends/$ref edges: data +JSON that doesn't match previously returned an empty result outright +(#1224 -- AST-walking arbitrary data JSON produced hundreds of orphan +key-nodes). Per the user's explicit, confirmed decision, that data JSON now +gets a genuinely generic structural walk instead (no domain semantics, one +node per key/list item) rather than staying invisible -- mirroring +``graphify/extractors/yaml_generic.py``'s design exactly, just walking +tree-sitter-JSON's simpler node vocabulary (``object``/``pair``/``array``) +instead of YAML's block/flow-wrapped one. +""" +from __future__ import annotations + +from graphify.extractors.base import _make_id, _read_text + +MAX_NODES_PER_DOCUMENT = 2000 + +_MAX_LABEL_LEN = 80 + + +def _truncate_label(text: str) -> str: + text = text.strip() + if len(text) <= _MAX_LABEL_LEN: + return text + return text[: _MAX_LABEL_LEN - 1] + "…" + + +def _key_text(pair_node, source: bytes) -> str: + key_node = pair_node.child_by_field_name("key") + if key_node is None: + return "" + if key_node.type == "string": + content = key_node.child_by_field_name("string_content") + if content: + return _read_text(content, source) + return _read_text(key_node, source).strip('"\'') + return _read_text(key_node, source) + + +def _scalar_text(node, source: bytes) -> str: + if node.type == "string": + content = node.child_by_field_name("string_content") + if content: + return _read_text(content, source) + return _read_text(node, source).strip('"\'') + return _read_text(node, source) + + +def extract_generic_structure(root_value, source: bytes, str_path: str, file_nid: str) -> tuple[list[dict], list[dict], bool]: + """Walk a JSON document's root value (object, array, or scalar) and emit + structural nodes/edges with no domain semantics -- one node per object + key, one node per array item, no dependency/extends/$ref interpretation + (that stays exclusive to recognized config/manifest JSON in + ``extract_json``). + + Returns (nodes, edges, truncated) -- see yaml_generic.py's + extract_generic_structure for the identical truncation contract. + """ + nodes: list[dict] = [] + edges: list[dict] = [] + seen_ids: set[str] = set() + truncated = False + + def _mint(parts: tuple[str, ...], label: str, line: int) -> str: + nid = _make_id(str_path, *parts) + if nid not in seen_ids: + seen_ids.add(nid) + nodes.append({"id": nid, "label": _truncate_label(label), "file_type": "code", + "source_file": str_path, "source_location": f"L{line}"}) + return nid + + def _add_contains(parent_nid: str, child_nid: str, line: int) -> None: + edges.append({"source": parent_nid, "target": child_nid, "relation": "contains", + "confidence": "EXTRACTED", "source_file": str_path, + "source_location": f"L{line}", "weight": 1.0}) + + def _walk(node, parent_nid: str, parts: tuple[str, ...]) -> None: + nonlocal truncated + if truncated or node is None: + return + if node.type == "object": + for child in node.children: + if child.type != "pair": + continue + if len(nodes) >= MAX_NODES_PER_DOCUMENT: + truncated = True + return + key = _key_text(child, source) + if not key: + continue + value = child.child_by_field_name("value") + line = child.start_point[0] + 1 + child_parts = parts + (key,) + child_nid = _mint(child_parts, key, line) + _add_contains(parent_nid, child_nid, line) + _walk(value, child_nid, child_parts) + return + if node.type == "array": + items = [c for c in node.children if c.is_named] + for i, item in enumerate(items): + if len(nodes) >= MAX_NODES_PER_DOCUMENT: + truncated = True + return + text = _scalar_text(item, source) if item.type in ("string", "number", "true", "false", "null") else "" + label = text if text else f"[{i}]" + line = item.start_point[0] + 1 + child_parts = parts + (f"[{i}]",) + child_nid = _mint(child_parts, label, line) + _add_contains(parent_nid, child_nid, line) + _walk(item, child_nid, child_parts) + return + # Scalar leaf: nothing further to mint. + + _walk(root_value, file_nid, ("doc0",)) + return nodes, edges, truncated diff --git a/graphify/extractors/k8s_manifest.py b/graphify/extractors/k8s_manifest.py new file mode 100644 index 0000000000..184ec11ce8 --- /dev/null +++ b/graphify/extractors/k8s_manifest.py @@ -0,0 +1,453 @@ +"""Kubernetes manifest extractor. + +Recognizes files shaped like a real Kubernetes resource (apiVersion + kind + +metadata present and value-validated, not just key names) -- OSAC-4050, +originally the real follow-up from the generic-YAML discussion on +https://github.com/eliorerz/graphify/pull/2#issuecomment-5294807827: k8s +manifests have a known-enough schema with genuinely useful relationships, +which is exactly why a scoped AST traversal is worthwhile here the same way +it was for GitHub Actions (OSAC-4049). + +This module is now the RICH, PRIORITY layer of a two-layer design (the +user explicitly broadened this ticket's scope mid-implementation to +universal YAML/JSON coverage): recognized k8s manifests get these +relationships; anything else, including Helm ``values.yaml`` (still no +fixed schema of its own, still can't get THIS treatment specifically), gets +a generic structural fallback instead of being skipped entirely -- see +``graphify/extractors/yaml_dispatch.py`` for the combined entry point that +layers the two, and ``graphify/extractors/yaml_generic.py`` for the +fallback itself. + +The tree-sitter-yaml CST traversal is shared with +``graphify/extractors/yaml_generic.py`` via ``graphify/extractors/_yaml_cst.py``. +That helper module's own origin is ``graphify/extractors/github_actions.py`` +(OSAC-4049) -- both ultimately adapted, with attribution, from the +tree-sitter-yaml traversal in the unmerged Graphify-Labs/graphify PR #2541. +``github_actions.py`` still carries its own independent copy rather than +importing ``_yaml_cst`` (its branch predates this module and should not +depend on this one landing first) -- worth deduping further once both have +merged. + +Relationships extracted (edges are all ``EXTRACTED`` -- every one is a +literal field read directly from the manifest, not inferred): + +- ``metadata.ownerReferences`` (standard k8s): owner -> owned, relation + ``owns``. +- This repo's own annotation-based convention + (``osac.openshift.io/owner-reference`` = the parent's ID, + ``osac.openshift.io/tenant`` = tenant scoping) -- see + ``osac/.claude/rules/architecture-patterns.md``. Confirmed directly + against the operator's own Go source (``subnet_type.pb.go`` et al.) that + the annotation's value is the parent's ID, not its name, so the minted + stub for it is keyed by that raw ID string; it will only collapse onto a + real resource node if something else in the corpus exposes that same ID + as an alias (a known, documented limitation, not a bug -- the same class + of "stub that may never resolve to a local definition" GitHub Actions' + ``actions/checkout@v4`` stubs already accept). +- ConfigMap / Secret references (``configMapKeyRef``/``secretKeyRef``, + ``configMapRef``/``secretRef``, volume ``configMap``/``secret``), found via + a generic recursive walk of ``spec`` rather than hardcoded exact paths + (these appear at varying depths: ``containers[].env[].valueFrom.*``, + ``containers[].envFrom[].*``, ``volumes[].*``) -- relation ``uses``. +- CRD cross-references via the ``*Ref``/``*Refs`` field-naming convention + (e.g. ``subnetRef``, ``securityGroupRefs``) -- a real, live convention + confirmed directly against this repo's own CRD samples + (``osac-operator/config/samples/osac_v1alpha1_computeinstance.yaml``), not + invented for this extractor. The referenced kind is inferred by stripping + the ``Ref``/``Refs`` suffix and capitalizing (``subnetRef`` -> ``Subnet``). + Deliberately NOT attempted: a bare field naming a resource with no + ``Ref``/``Refs`` suffix (e.g. ``Subnet.spec.virtualNetwork``, which this + repo's own samples set to a UUID) -- indistinguishable from an arbitrary + opaque config value without hardcoding per-CRD-kind field knowledge, which + would make this extractor fragile and high-maintenance for one more field + per CRD change. A known, deliberate scope limit, not an oversight. +- Label-selector matches (Service -> pods/Deployments via ``spec.selector``) + -- approximated via a shared "label hub" stub node per distinct + ``key=value`` pair: the selecting resource gets a ``selects`` edge to the + hub, the labeled resource (``metadata.labels`` or + ``spec.template.metadata.labels``) gets a ``has_label`` edge to the same + hub, so they connect through it without needing a bespoke cross-file + matching pass (reuses the exact same stub-collapsing mechanism GitHub + Actions relies on for shared actions). This is a genuine approximation + for a selector with MORE THAN ONE key=value pair: Kubernetes requires ALL + of a selector's pairs to match (AND semantics), but this hub-per-pair + design would still show a connection through any ONE shared pair even if + the others don't match. Confirmed against a REAL single-key example in + this repo (``osac-operator/config/console-proxy/service.yaml``'s + ``selector: {app: osac-console-proxy}`` matching + ``deployment.yaml``'s pod template labels exactly) where this is exact, + not approximate. Real Helm-templated Services in this repo (e.g. + ``charts/operator/templates/metrics-service.yaml``) have their selector + as a template ``include``, not literal YAML, so they parse to a tree-sitter + ERROR node and are excluded automatically (see ``is_k8s_manifest_shape`` + and the module-level test coverage) -- confirmed directly, not assumed. + +Not modelled BY THIS MODULE specifically: Helm ``values.yaml`` (no fixed +schema at all -- confirmed in the PR #2 discussion this cannot get THIS +rich, schema-aware treatment) and any YAML that doesn't validate as +apiVersion+kind+metadata-shaped. This function (``extract_k8s_resources``) +and the standalone ``extract_k8s_manifest`` wrapper both return an empty +result for such input -- it's ``yaml_dispatch.py``'s job to route it to the +generic fallback instead, not this module's. +""" +from __future__ import annotations + +import re +from pathlib import Path + +from graphify.extractors.base import _make_id +from graphify.extractors._yaml_cst import ( + all_top_level_mappings as _all_top_level_mappings, + item_value as _item_value, + mapping as _mapping, + pairs as _pairs, + scalar_text as _scalar_text, + sequence_items as _sequence_items, + string_items as _string_items, +) + +# --------------------------------------------------------------------------- +# Real, value-validated shape check (used inside extract_k8s_manifest, which +# has already parsed the file with tree-sitter-yaml). Was paired with an +# is_k8s_manifest_path() cheap pre-filter for a classify_file() carve-out in +# an earlier version of this ticket; superseded once the direction changed +# to universal YAML/JSON coverage (all .yaml/.yml is unconditionally CODE +# now, matching .json's existing precedent -- see graphify/detect.py). +# --------------------------------------------------------------------------- + +# apiVersion is either bare ("v1") or "/" +# ("apps/v1", "osac.openshift.io/v1alpha1", "apiextensions.k8s.io/v1"). +_API_VERSION_RE = re.compile(r"^([a-zA-Z0-9.\-]+/)?v[0-9]+((alpha|beta)[0-9]*)?$") + + +def is_k8s_manifest_shape(top) -> bool: + """True if *top* (a document's top-level mapping) is a real, + value-validated k8s resource: apiVersion looks like a real k8s + apiVersion string, kind looks like a real PascalCase type name, and + metadata is present and is itself a mapping. + + Key presence alone is not enough (the ticket's own explicit warning, + confirmed to matter in practice): a coincidental `kind: foo` in some + unrelated config must not pass. All three checks must hold together. + """ + if top is None: + return False + pairs = {key: value for key, value, _line in _pairs(top)} + if not ({"apiVersion", "kind", "metadata"} <= pairs.keys()): + return False + api_version = _scalar_text(pairs["apiVersion"]) + kind = _scalar_text(pairs["kind"]) + if not _API_VERSION_RE.match(api_version): + return False + if not kind or not kind[0].isupper() or not kind.isalnum(): + return False + return _mapping(pairs["metadata"]) is not None + + +# --------------------------------------------------------------------------- +# Relationship extraction. +# --------------------------------------------------------------------------- + +_OWNER_ANNOTATION = "osac.openshift.io/owner-reference" +_TENANT_ANNOTATION = "osac.openshift.io/tenant" + +# Field-name conventions for ConfigMap/Secret references, found at varying +# depths under spec (containers[].env[].valueFrom.*, containers[].envFrom[].*, +# volumes[].*) -- walked generically rather than hardcoding each exact path. +_CONFIGMAP_REF_KEYS = frozenset({"configMapKeyRef", "configMapRef", "configMap"}) +_SECRET_REF_KEYS = frozenset({"secretKeyRef", "secretRef", "secret"}) +# volumes' `secret:` block names the Secret via `secretName`, not `name`. +_REF_NAME_FIELDS = ("name", "secretName") + + +def _resource_id(kind: str, namespace: str, name: str) -> str: + # namespace is folded in when present; make_id already drops empty + # parts, so an unnamespaced resource just falls back to (kind, name) -- + # a documented simplification (this repo's own samples rarely set + # namespace explicitly), not a false-collision risk in the common case. + return _make_id(kind, namespace, name) + + +def _walk_configmap_secret_refs(node, owner_nid, namespace, add_edge, ref_stub): + mapping = _mapping(node) + if mapping is not None: + for key, value, line in _pairs(mapping): + kind = "ConfigMap" if key in _CONFIGMAP_REF_KEYS else "Secret" if key in _SECRET_REF_KEYS else None + if kind: + sub = _mapping(value) + if sub is not None: + sub_pairs = {k: v for k, v, _l in _pairs(sub)} + ref_name = "" + for field in _REF_NAME_FIELDS: + if field in sub_pairs: + ref_name = _scalar_text(sub_pairs[field]) + if ref_name: + break + if ref_name: + tgt = ref_stub(_resource_id(kind, namespace, ref_name), f"{kind}/{ref_name}") + add_edge(owner_nid, tgt, "uses", line) + continue + _walk_configmap_secret_refs(value, owner_nid, namespace, add_edge, ref_stub) + return + for item in _sequence_items(node): + _walk_configmap_secret_refs(_item_value(item), owner_nid, namespace, add_edge, ref_stub) + + +def _infer_ref_kind(field_key: str, suffix: str) -> str: + base = field_key[: -len(suffix)] + return base[0].upper() + base[1:] if base else "" + + +def _walk_ref_convention(node, owner_nid, namespace, add_edge, ref_stub): + mapping = _mapping(node) + if mapping is not None: + for key, value, line in _pairs(mapping): + if key in _CONFIGMAP_REF_KEYS or key in _SECRET_REF_KEYS: + continue # handled by _walk_configmap_secret_refs; do not double-classify as a generic *Ref + if key.endswith("Refs") and len(key) > len("Refs"): + kind = _infer_ref_kind(key, "Refs") + for item_text, item_line in _string_items(value): + tgt = ref_stub(_resource_id(kind, namespace, item_text), f"{kind}/{item_text}") + add_edge(owner_nid, tgt, "references", item_line) + elif key.endswith("Ref") and len(key) > len("Ref"): + kind = _infer_ref_kind(key, "Ref") + ref_text = _scalar_text(value) + if ref_text: + tgt = ref_stub(_resource_id(kind, namespace, ref_text), f"{kind}/{ref_text}") + add_edge(owner_nid, tgt, "references", line) + else: + _walk_ref_convention(value, owner_nid, namespace, add_edge, ref_stub) + return + for item in _sequence_items(node): + _walk_ref_convention(_item_value(item), owner_nid, namespace, add_edge, ref_stub) + + +def _emit_label_hub_edges(labels_node, owner_nid, relation, add_edge, ref_stub): + mapping = _mapping(labels_node) + if mapping is None: + return + for key, value, line in _pairs(mapping): + value_text = _scalar_text(value) + if value_text: + hub = ref_stub(_make_id("label", key, value_text), f"{key}={value_text}") + add_edge(owner_nid, hub, relation, line) + + +def _handle_selectors_and_labels(spec_pairs, meta_pairs, owner_nid, add_edge, ref_stub): + if "selector" in spec_pairs: + selector_value, _l = spec_pairs["selector"] + selector_mapping = _mapping(selector_value) + if selector_mapping is not None: + for key, value, line in _pairs(selector_mapping): + if key == "matchLabels": + ml = _mapping(value) + if ml is not None: + for lk, lv, lline in _pairs(ml): + lv_text = _scalar_text(lv) + if lv_text: + hub = ref_stub(_make_id("label", lk, lv_text), f"{lk}={lv_text}") + add_edge(owner_nid, hub, "selects", lline) + continue + if key == "matchExpressions": + # Operator-based selectors (In/NotIn/Exists/DoesNotExist) + # have no single value to hub on -- skip rather than guess. + continue + value_text = _scalar_text(value) + if value_text: + hub = ref_stub(_make_id("label", key, value_text), f"{key}={value_text}") + add_edge(owner_nid, hub, "selects", line) + + if "labels" in meta_pairs: + _emit_label_hub_edges(meta_pairs["labels"][0], owner_nid, "has_label", add_edge, ref_stub) + + if "template" in spec_pairs: + template_mapping = _mapping(spec_pairs["template"][0]) + if template_mapping is not None: + template_pairs = {k: (v, l) for k, v, l in _pairs(template_mapping)} + if "metadata" in template_pairs: + tmpl_meta = _mapping(template_pairs["metadata"][0]) + if tmpl_meta is not None: + tmpl_meta_pairs = {k: (v, l) for k, v, l in _pairs(tmpl_meta)} + if "labels" in tmpl_meta_pairs: + _emit_label_hub_edges(tmpl_meta_pairs["labels"][0], owner_nid, "has_label", add_edge, ref_stub) + + +def extract_k8s_resources(resource_tops: list, str_path: str, file_nid: str) -> tuple[list[dict], list[dict]]: + """Extract resource nodes and ownership/reference/selector edges for a + list of already-shape-validated top-level mappings (one per k8s + resource -- callers filter with is_k8s_manifest_shape() first). + + Split out from extract_k8s_manifest() so a combined dispatcher handling + a mixed file (some documents k8s-shaped, some not) can call this for + just the matching subset while owning the file node itself centrally -- + see graphify/extractors/yaml_dispatch.py. + + Nodes: one per resource (keyed globally by (kind, namespace, name), not + file-scoped -- unlike a GitHub Actions job, the same resource can + legitimately be defined in one file and referenced from many others). + Sourceless stub nodes (type=module, same hub-collapsing exemption + GitHub Actions' shared-action stubs use, #1327) stand in for anything + referenced but not locally defined in the same file, so cross-file + ownership/reference edges collapse onto the real definition when it + exists elsewhere in the same extraction batch, and survive as a + portable node when it doesn't. + + See the module docstring for the full list of relationships modelled + and their known, deliberate limitations. Returns (nodes, edges) -- does + NOT include the file node/contains-edge-from-file, which is the + caller's responsibility (shared across k8s and generic-structural + resources in the same file). + """ + nodes: list[dict] = [] + edges: list[dict] = [] + seen_ids: set[str] = {file_nid} + seen_edges: set[tuple[str, str, str]] = set() + local_nids: dict[tuple[str, str, str], str] = {} + + def _ref_stub(nid: str, label: str) -> str: + if nid not in seen_ids: + seen_ids.add(nid) + nodes.append({"id": nid, "label": label, "file_type": "code", + "source_file": "", "source_location": "", + "origin_file": str_path, "type": "module"}) + return nid + + def _add_edge(src: str, tgt: str, relation: str, line: int) -> None: + if not src or not tgt or src == tgt: + return + key = (src, tgt, relation) + if key in seen_edges: + return + seen_edges.add(key) + edges.append({"source": src, "target": tgt, "relation": relation, + "confidence": "EXTRACTED", "source_file": str_path, + "source_location": f"L{line}", "weight": 1.0}) + + # Parse each resource's top-level fields once, for the two-pass process + # below (definitions first, so a same-file forward reference -- e.g. the + # Namespace and Deployment in manager.yaml -- binds locally). + parsed = [] + for top in resource_tops: + pairs = {key: (value, line) for key, value, line in _pairs(top)} + kind = _scalar_text(pairs["kind"][0]) + metadata = _mapping(pairs["metadata"][0]) + meta_pairs = {key: (value, line) for key, value, line in _pairs(metadata)} if metadata is not None else {} + name = _scalar_text(meta_pairs["name"][0]) if "name" in meta_pairs else "" + if not name: + continue + namespace = _scalar_text(meta_pairs["namespace"][0]) if "namespace" in meta_pairs else "" + spec_entry = pairs.get("spec") + parsed.append({ + "kind": kind, "name": name, "namespace": namespace, + "meta_pairs": meta_pairs, + "spec": spec_entry[0] if spec_entry else None, + "line": pairs["kind"][1], + }) + + if not parsed: + return nodes, edges + + for r in parsed: + nid = _resource_id(r["kind"], r["namespace"], r["name"]) + if nid not in seen_ids: + seen_ids.add(nid) + nodes.append({"id": nid, "label": f"{r['kind']}/{r['name']}", "file_type": "code", + "source_file": str_path, "source_location": f"L{r['line']}"}) + edges.append({"source": file_nid, "target": nid, "relation": "contains", + "confidence": "EXTRACTED", "source_file": str_path, + "source_location": f"L{r['line']}", "weight": 1.0}) + local_nids[(r["kind"], r["namespace"], r["name"])] = nid + r["nid"] = nid + + for r in parsed: + owner_nid = r["nid"] + namespace = r["namespace"] + meta_pairs = r["meta_pairs"] + + # -- standard metadata.ownerReferences -- + if "ownerReferences" in meta_pairs: + owner_refs_node, _l = meta_pairs["ownerReferences"] + for item in _sequence_items(owner_refs_node): + ref_mapping = _mapping(_item_value(item)) + if ref_mapping is None: + continue + ref_pairs = {k: v for k, v, _ln in _pairs(ref_mapping)} + ref_kind = _scalar_text(ref_pairs.get("kind")) + ref_name = _scalar_text(ref_pairs.get("name")) + if not ref_kind or not ref_name: + continue + parent_nid = local_nids.get((ref_kind, namespace, ref_name)) or _ref_stub( + _resource_id(ref_kind, namespace, ref_name), f"{ref_kind}/{ref_name}") + _add_edge(parent_nid, owner_nid, "owns", r["line"]) + + # -- custom annotation-based owner-reference + tenant scoping -- + if "annotations" in meta_pairs: + ann_mapping = _mapping(meta_pairs["annotations"][0]) + if ann_mapping is not None: + for key, value, line in _pairs(ann_mapping): + if key == _OWNER_ANNOTATION: + owner_ref_value = _scalar_text(value) + if owner_ref_value: + parent_nid = _ref_stub(_make_id("owner-ref", owner_ref_value), owner_ref_value) + _add_edge(parent_nid, owner_nid, "owns", line) + elif key == _TENANT_ANNOTATION: + tenant_value = _scalar_text(value) + if tenant_value: + tenant_nid = _ref_stub(_make_id("tenant", tenant_value), tenant_value) + _add_edge(owner_nid, tenant_nid, "scoped_to", line) + + if r["spec"] is not None: + _walk_configmap_secret_refs(r["spec"], owner_nid, namespace, _add_edge, _ref_stub) + _walk_ref_convention(r["spec"], owner_nid, namespace, _add_edge, _ref_stub) + + spec_pairs = {} + if r["spec"] is not None: + spec_mapping = _mapping(r["spec"]) + if spec_mapping is not None: + spec_pairs = {k: (v, l) for k, v, l in _pairs(spec_mapping)} + _handle_selectors_and_labels(spec_pairs, meta_pairs, owner_nid, _add_edge, _ref_stub) + + return nodes, edges + + +def extract_k8s_manifest(path: Path) -> dict: + """Standalone entry point: parse *path* and extract k8s resources from + it directly (used for direct testing/backward-compat; the combined + dispatcher in yaml_dispatch.py calls extract_k8s_resources() directly + on a file it has already parsed, to avoid re-parsing). + + Any YAML that doesn't validate as a real k8s resource (data YAML, an + unrelated config that happens to share a key name, or a + Helm-templated file whose Go template syntax breaks the YAML grammar) + returns an empty result and is left to the semantic pass. + """ + _YAML_MAX_BYTES = 1_048_576 # 1 MiB -- manifests are small; this rejects junk + + try: + import tree_sitter_yaml as tsyaml + from tree_sitter import Language, Parser + except ImportError: + return {"nodes": [], "edges": [], "error": "tree_sitter_yaml not installed. Run: pip install tree-sitter-yaml"} + + try: + with path.open("rb") as fh: + source = fh.read(_YAML_MAX_BYTES + 1) + if len(source) > _YAML_MAX_BYTES: + return {"nodes": [], "edges": [], "error": "yaml file too large to index"} + language = Language(tsyaml.language()) + parser = Parser(language) + tree = parser.parse(source) + root = tree.root_node + except Exception as e: + return {"nodes": [], "edges": [], "error": str(e)} + + resource_tops = [top for top in _all_top_level_mappings(root) if is_k8s_manifest_shape(top)] + if not resource_tops: + return {"nodes": [], "edges": []} + + str_path = str(path) + file_nid = _make_id(str_path) + resource_nodes, resource_edges = extract_k8s_resources(resource_tops, str_path, file_nid) + nodes = [{"id": file_nid, "label": path.name, "file_type": "code", + "source_file": str_path, "source_location": None}] + resource_nodes + return {"nodes": nodes, "edges": resource_edges} diff --git a/graphify/extractors/yaml_dispatch.py b/graphify/extractors/yaml_dispatch.py new file mode 100644 index 0000000000..b24376c3b9 --- /dev/null +++ b/graphify/extractors/yaml_dispatch.py @@ -0,0 +1,119 @@ +"""Combined YAML dispatcher -- the actual `.yaml`/`.yml` entry point. + +OSAC-4050. Layered per document (a single file can bundle multiple +`---`-separated documents, confirmed against a real file in this repo -- +see graphify/extractors/_yaml_cst.py's ``all_documents`` docstring): + +1. If a document parses cleanly and looks like a real k8s resource + (``is_k8s_manifest_shape``), extract its rich ownership/reference/ + selector relationships (``graphify.extractors.k8s_manifest``). +2. Otherwise, if it parses cleanly, fall back to a generic structural walk + with no domain semantics (``graphify.extractors.yaml_generic``) -- + universal coverage, per the user's explicit, confirmed direction change + partway through this ticket's implementation: even YAML with no + recognized schema should get SOME representation in the graph, rather + than staying invisible the way pre-OSAC-4050 graphify left ALL YAML. +3. If a document doesn't parse cleanly at all (`node.has_error` -- e.g. a + Helm chart's Go-templated `{{ include ... }}` syntax, confirmed + empirically during this ticket's investigation to break the + tree-sitter-yaml grammar outright, producing an ERROR node rather than a + best-effort partial tree), it is skipped with a one-line warning naming + the file -- never crashed on, never walked for "structure" that would + really just be gibberish extracted from a broken parse. +""" +from __future__ import annotations + +import sys +from pathlib import Path + +from graphify.extractors._yaml_cst import all_documents, mapping as _mapping +from graphify.extractors.base import _make_id +from graphify.extractors.k8s_manifest import extract_k8s_resources, is_k8s_manifest_shape +from graphify.extractors.yaml_generic import extract_generic_structure + +_YAML_MAX_BYTES = 1_048_576 # 1 MiB -- matches every other extractor's cap in this fork + + +def extract_yaml(path: Path) -> dict: + """Extract structure from a .yaml/.yml file: rich k8s relationships for + recognized manifests, generic structural nodes for everything else that + parses cleanly, a skip-with-warning for anything that doesn't. + """ + try: + import tree_sitter_yaml as tsyaml + from tree_sitter import Language, Parser + except ImportError: + return {"nodes": [], "edges": [], "error": "tree_sitter_yaml not installed. Run: pip install tree-sitter-yaml"} + + try: + with path.open("rb") as fh: + source = fh.read(_YAML_MAX_BYTES + 1) + if len(source) > _YAML_MAX_BYTES: + return {"nodes": [], "edges": [], "error": "yaml file too large to index"} + language = Language(tsyaml.language()) + parser = Parser(language) + tree = parser.parse(source) + root = tree.root_node + except Exception as e: + return {"nodes": [], "edges": [], "error": str(e)} + + str_path = str(path) + file_nid = _make_id(str_path) + all_nodes: list[dict] = [] + all_edges: list[dict] = [] + file_node_added = False + skipped_docs = 0 + truncated_docs = 0 + + def _ensure_file_node() -> None: + nonlocal file_node_added + if not file_node_added: + all_nodes.append({"id": file_nid, "label": path.name, "file_type": "code", + "source_file": str_path, "source_location": None}) + file_node_added = True + + for doc_index, doc in enumerate(all_documents(root)): + if doc.has_error: + skipped_docs += 1 + continue + m = _mapping(doc) + if m is not None and is_k8s_manifest_shape(m): + _ensure_file_node() + k8s_nodes, k8s_edges = extract_k8s_resources([m], str_path, file_nid) + all_nodes.extend(k8s_nodes) + all_edges.extend(k8s_edges) + continue + # Not k8s-shaped (or not even a mapping at the top level) but parses + # cleanly -- generic structural fallback. Walk from the raw doc + # value (not `m`, which is None for a non-mapping document). + generic_nodes, generic_edges, truncated = extract_generic_structure(doc, str_path, file_nid, doc_index) + if generic_nodes: + _ensure_file_node() + all_nodes.extend(generic_nodes) + all_edges.extend(generic_edges) + if truncated: + truncated_docs += 1 + + if skipped_docs: + # One-line warning naming the file, printed immediately rather than + # folded into extract.py's end-of-run aggregate warnings (#1666/ + # #1745 are keyed on "produced zero nodes"/"dependency missing", + # neither of which fits "this specific document didn't parse") -- + # the ticket's explicit ask was a clear warning naming the file, not + # a silent skip. + suffix = "s" if skipped_docs > 1 else "" + print( + f" warning: {path.name}: {skipped_docs} document{suffix} did not " + f"parse as valid YAML (commonly Go/Helm template syntax breaking " + f"the grammar) -- skipped, not extracted.", + file=sys.stderr, + ) + if truncated_docs: + print( + f" warning: {path.name}: generic structural walk hit its node " + f"cap in {truncated_docs} document(s) -- truncated, not " + f"exhaustive.", + file=sys.stderr, + ) + + return {"nodes": all_nodes, "edges": all_edges} diff --git a/graphify/extractors/yaml_generic.py b/graphify/extractors/yaml_generic.py new file mode 100644 index 0000000000..e44ef1af04 --- /dev/null +++ b/graphify/extractors/yaml_generic.py @@ -0,0 +1,120 @@ +"""Generic structural YAML extractor. + +OSAC-4050 -- a direction change from that ticket's original k8s-only scope, +confirmed explicitly by the user mid-implementation: universal coverage for +ANY YAML that doesn't match a known, recognized schema (like a k8s +manifest), rather than leaving it invisible to the graph. Emits raw +structural nodes (one per mapping key, one per sequence item) with NO +domain semantics -- accepted, on the user's explicit instruction, to be +lower-value/noisier than a schema-aware extractor (the same failure mode +already confirmed empirically for unrecognized JSON, #1224/OSAC-4050 +investigation) in exchange for nothing being silently invisible. + +Malformed/templated YAML (a Helm chart's `{{ include ... }}` etc., which +breaks the tree-sitter-yaml grammar outright -- confirmed empirically +against a real chart in this fork's OSAC-4050 investigation, producing a +root ERROR node, not a best-effort partial tree) is NOT this module's +concern: the caller (graphify/extractors/yaml_dispatch.py) checks +`node.has_error` per document before ever calling into this walker, and +warns + skips instead. This module only ever sees already-known-clean +document values. +""" +from __future__ import annotations + +from graphify.extractors.base import _make_id +from graphify.extractors._yaml_cst import ( + item_value as _item_value, + mapping as _mapping, + pairs as _pairs, + scalar_text as _scalar_text, + sequence_items as _sequence_items, +) + +# Safety valve: a single pathological file (deeply nested, huge sequences) +# must not single-handedly blow up a corpus-wide extraction. This is a +# ceiling, not a target -- logged explicitly when hit (no silent caps), so +# it's visible rather than read as "covered everything" when it didn't. See +# OSAC-4050's empirical corpus-scale measurement for whether real files ever +# approach it. +MAX_NODES_PER_DOCUMENT = 2000 + +_MAX_LABEL_LEN = 80 + + +def _truncate_label(text: str) -> str: + text = text.strip() + if len(text) <= _MAX_LABEL_LEN: + return text + return text[: _MAX_LABEL_LEN - 1] + "…" + + +def extract_generic_structure(doc_value, str_path: str, file_nid: str, doc_index: int) -> tuple[list[dict], list[dict], bool]: + """Walk one YAML document's top-level value (mapping, sequence, or + scalar -- a generic document isn't required to be a mapping the way a + k8s resource is) and emit structural nodes/edges with no domain + semantics. + + IDs are hierarchical and file+document+path scoped + (`_make_id(str_path, "doc{N}", key1, key2, ...)`) -- unlike a k8s + resource's globally-scoped (kind, namespace, name) id, a raw structural + position has no meaningful identity outside its own file, so there is + nothing to collapse across files here. + + Returns (nodes, edges, truncated) -- `truncated` is True if + MAX_NODES_PER_DOCUMENT was hit, so the caller can log it once per file + rather than the walker doing so per node. + """ + nodes: list[dict] = [] + edges: list[dict] = [] + seen_ids: set[str] = set() + truncated = False + + def _mint(parts: tuple[str, ...], label: str, line: int) -> str: + nid = _make_id(str_path, *parts) + if nid not in seen_ids: + seen_ids.add(nid) + nodes.append({"id": nid, "label": _truncate_label(label), "file_type": "code", + "source_file": str_path, "source_location": f"L{line}"}) + return nid + + def _add_contains(parent_nid: str, child_nid: str, line: int) -> None: + edges.append({"source": parent_nid, "target": child_nid, "relation": "contains", + "confidence": "EXTRACTED", "source_file": str_path, + "source_location": f"L{line}", "weight": 1.0}) + + def _walk(node, parent_nid: str, parts: tuple[str, ...]) -> None: + nonlocal truncated + if truncated or node is None: + return + mapping = _mapping(node) + if mapping is not None: + for key, value, line in _pairs(mapping): + if len(nodes) >= MAX_NODES_PER_DOCUMENT: + truncated = True + return + child_parts = parts + (key,) + child_nid = _mint(child_parts, key, line) + _add_contains(parent_nid, child_nid, line) + _walk(value, child_nid, child_parts) + return + seq_items = list(_sequence_items(node)) + if seq_items: + for i, item in enumerate(seq_items): + if len(nodes) >= MAX_NODES_PER_DOCUMENT: + truncated = True + return + item_value = _item_value(item) + text = _scalar_text(item_value) + label = text if text else f"[{i}]" + line = item.start_point[0] + 1 + child_parts = parts + (f"[{i}]",) + child_nid = _mint(child_parts, label, line) + _add_contains(parent_nid, child_nid, line) + _walk(item_value, child_nid, child_parts) + return + # Scalar leaf: nothing further to mint -- the key/item node the + # caller already minted for this position represents it. + + doc_parts = (f"doc{doc_index}",) + _walk(doc_value, file_nid, doc_parts) + return nodes, edges, truncated diff --git a/pyproject.toml b/pyproject.toml index de54be6e94..4dc2abbc66 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -74,7 +74,20 @@ anthropic = ["anthropic"] gemini = ["openai", "tiktoken"] openai = ["openai", "tiktoken"] chinese = ["jieba"] +# NOTE: OSAC-4049 (unmerged as of this writing) also adds this exact same +# extra for extract_github_actions() -- when both PRs merge these entries +# are identical and trivially resolve, unlike the _DISPATCH collision noted +# in extract.py. sql = ["tree-sitter-sql"] +# extract_yaml() (graphify/extractors/yaml_dispatch.py) models k8s manifest +# ownership/reference/selector structure when recognized, and a generic +# structural fallback for every other .yaml/.yml otherwise (OSAC-4050 -- +# deliberately universal coverage, not just recognized schemas). ALL YAML is +# now routed to FileType.CODE (graphify/detect.py) unconditionally, matching +# how .json already works -- so without this extra, every .yaml/.yml in the +# corpus hits the #1745 missing-dependency warning; there is no more +# semantic-pass fallback for YAML to silently degrade to. +yaml = ["tree-sitter-yaml"] # extract_pascal() uses tree-sitter-pascal for AST-quality extraction (more # accurate calls/inherits edges) and falls back to a regex extractor when it is # absent (#781), so this stays optional. Unlike tree-sitter-dm below, it ships @@ -85,7 +98,7 @@ pascal = ["tree-sitter-pascal"] # avoids breaking the default `uv tool install graphifyy` for everyone (#1104). dm = ["tree-sitter-dm"] terraform = ["tree-sitter-hcl"] -all = ["mcp>=1,<3", "starlette>=1.3.1,<2", "neo4j", "falkordb", "pypdf>=6.12.0", "markdownify", "watchdog", "graspologic; python_version < '3.13'", "python-docx", "openpyxl", "faster-whisper; python_version >= '3.11'", "yt-dlp>=2026.6.9", "matplotlib", "numpy>=2.0; python_version >= '3.13'", "openai", "tiktoken", "boto3", "anthropic", "tree-sitter-sql", "jieba", "tree-sitter-dm", "tree-sitter-hcl", "tree-sitter-pascal"] +all = ["mcp>=1,<3", "starlette>=1.3.1,<2", "neo4j", "falkordb", "pypdf>=6.12.0", "markdownify", "watchdog", "graspologic; python_version < '3.13'", "python-docx", "openpyxl", "faster-whisper; python_version >= '3.11'", "yt-dlp>=2026.6.9", "matplotlib", "numpy>=2.0; python_version >= '3.13'", "openai", "tiktoken", "boto3", "anthropic", "tree-sitter-sql", "tree-sitter-yaml", "jieba", "tree-sitter-dm", "tree-sitter-hcl", "tree-sitter-pascal"] [project.scripts] graphify = "graphify.__main__:main" diff --git a/tests/test_extract.py b/tests/test_extract.py index cb9d715991..00e2409009 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -2840,8 +2840,12 @@ def test_extract_json_no_self_loops(): # Data JSON must not explode into orphan key-nodes (#1224) # --------------------------------------------------------------------------- -def test_extract_json_data_file_skipped(tmp_path): - """A data-shaped .json (eval fixture / dataset) must NOT emit per-key nodes.""" +def test_extract_json_data_file_gets_generic_structure(tmp_path): + """A data-shaped .json (eval fixture / dataset) does not get the rich + config-specific dependency/extends/$ref treatment, but (OSAC-4050, + universal-coverage direction) is no longer invisible either -- it falls + back to a generic structural walk (one node per key/list item, no + domain semantics).""" data = tmp_path / "cases.json" data.write_text(json.dumps({ "generation": {"target": "gpt-4", "cases_file": "c.json", "num_cases": 12}, @@ -2849,18 +2853,23 @@ def test_extract_json_data_file_skipped(tmp_path): "suite": [{"name": "x"}, {"name": "y"}], })) result = extract_json(data) - assert result["nodes"] == [] - assert result["edges"] == [] - assert "skipped" in result + labels = {n["label"] for n in result["nodes"]} + assert "generation" in labels + assert "prompt_inputs_spec" in labels + assert "suite" in labels + # None of the rich config-only edge kinds (imports/extends/references) + # -- those stay exclusive to recognized config/manifest JSON. + assert not any(e["relation"] in ("imports", "extends", "references") for e in result["edges"]) -def test_extract_json_top_level_array_skipped(tmp_path): - """A JSON file whose root is an array is data, never a config/manifest.""" +def test_extract_json_top_level_array_gets_generic_structure(tmp_path): + """A JSON file whose root is an array is data, never a config/manifest, + but still gets generic structural nodes (OSAC-4050).""" data = tmp_path / "records.json" data.write_text(json.dumps([{"id": 1}, {"id": 2}])) result = extract_json(data) - assert result["nodes"] == [] - assert result["edges"] == [] + labels = {n["label"] for n in result["nodes"]} + assert "id" in labels def test_extract_json_config_by_filename_still_extracted(tmp_path): diff --git a/tests/test_k8s_manifest.py b/tests/test_k8s_manifest.py new file mode 100644 index 0000000000..340661f23b --- /dev/null +++ b/tests/test_k8s_manifest.py @@ -0,0 +1,355 @@ +"""Tests for the Kubernetes manifest extractor +(graphify/extractors/k8s_manifest.py) and the combined YAML dispatcher +(graphify/extractors/yaml_dispatch.py) that layers it over the generic +structural fallback (graphify/extractors/yaml_generic.py) -- OSAC-4050. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from graphify.build import build_from_json +from graphify.detect import FileType, classify_file +from graphify.extract import extract, extract_k8s_manifest, extract_yaml + + +def _write(tmp_path: Path, name: str, body: str) -> Path: + p = tmp_path / name + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(body, encoding="utf-8") + return p + + +def _labels(r) -> list[str]: + return [n["label"] for n in r["nodes"]] + + +def _rel_pairs(r, relation: str) -> set[tuple[str, str]]: + lab = {n["id"]: n["label"] for n in r["nodes"]} + return { + (lab.get(e["source"], e["source"]), lab.get(e["target"], e["target"])) + for e in r["edges"] + if e["relation"] == relation + } + + +@pytest.fixture(autouse=True) +def _require_grammar(): + pytest.importorskip("tree_sitter_yaml") + + +# ── ownerReferences (standard k8s) ────────────────────────────────────────── + +OWNER_REF_CHILD = """\ +apiVersion: v1 +kind: Pod +metadata: + name: worker-pod + ownerReferences: + - apiVersion: apps/v1 + kind: ReplicaSet + name: worker-rs +""" + + +def test_owner_references_become_owns_edges(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "pod.yaml", OWNER_REF_CHILD)) + assert r.get("error") is None + assert ("ReplicaSet/worker-rs", "Pod/worker-pod") in _rel_pairs(r, "owns") + + +def test_owner_reference_resolves_to_real_definition_across_files(tmp_path): + parent = _write(tmp_path, "rs.yaml", "apiVersion: apps/v1\nkind: ReplicaSet\nmetadata:\n name: worker-rs\n") + child = _write(tmp_path, "pod.yaml", OWNER_REF_CHILD) + r = extract([parent.resolve(), child.resolve()], root=tmp_path) + rs_ids = {n["id"] for n in r["nodes"] if n["label"] == "ReplicaSet/worker-rs"} + assert len(rs_ids) == 1, f"expected one ReplicaSet node, got {rs_ids}" + assert rs_ids.pop() in {e["source"] for e in r["edges"] if e["relation"] == "owns"} + + +# ── custom annotation-based owner-reference + tenant (architecture-patterns.md) ── + +ANNOTATED_CHILD = """\ +apiVersion: osac.openshift.io/v1alpha1 +kind: Subnet +metadata: + name: my-subnet + annotations: + osac.openshift.io/owner-reference: 00000000-0000-0000-0000-000000000000 + osac.openshift.io/tenant: my-tenant +""" + + +def test_custom_owner_reference_annotation_becomes_owns_edge(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "subnet.yaml", ANNOTATED_CHILD)) + owns = _rel_pairs(r, "owns") + assert ("00000000-0000-0000-0000-000000000000", "Subnet/my-subnet") in owns + + +def test_tenant_annotation_becomes_scoped_to_edge(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "subnet.yaml", ANNOTATED_CHILD)) + assert ("Subnet/my-subnet", "my-tenant") in _rel_pairs(r, "scoped_to") + + +# ── ConfigMap / Secret references ─────────────────────────────────────────── + +WORKLOAD_WITH_REFS = """\ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: api +spec: + template: + spec: + containers: + - name: app + envFrom: + - configMapRef: + name: app-config + - secretRef: + name: app-secrets + env: + - name: DB_PASSWORD + valueFrom: + secretKeyRef: + name: db-secret + key: password + volumeMounts: + - name: cfg + mountPath: /etc/cfg + volumes: + - name: cfg + configMap: + name: shared-config +""" + + +def test_configmap_ref_from_envfrom(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "deploy.yaml", WORKLOAD_WITH_REFS)) + uses = _rel_pairs(r, "uses") + assert ("Deployment/api", "ConfigMap/app-config") in uses + + +def test_secret_ref_from_envfrom(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "deploy.yaml", WORKLOAD_WITH_REFS)) + assert ("Deployment/api", "Secret/app-secrets") in _rel_pairs(r, "uses") + + +def test_secret_key_ref_from_env_valuefrom(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "deploy.yaml", WORKLOAD_WITH_REFS)) + assert ("Deployment/api", "Secret/db-secret") in _rel_pairs(r, "uses") + + +def test_configmap_ref_from_volume(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "deploy.yaml", WORKLOAD_WITH_REFS)) + assert ("Deployment/api", "ConfigMap/shared-config") in _rel_pairs(r, "uses") + + +def test_configmap_secret_ref_resolves_across_files(tmp_path): + cm = _write(tmp_path, "cm.yaml", "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: app-config\n") + dep = _write(tmp_path, "deploy.yaml", WORKLOAD_WITH_REFS) + r = extract([cm.resolve(), dep.resolve()], root=tmp_path) + cm_ids = {n["id"] for n in r["nodes"] if n["label"] == "ConfigMap/app-config"} + assert len(cm_ids) == 1 + + +# ── *Ref / *Refs CRD cross-reference convention ───────────────────────────── + +COMPUTE_INSTANCE = """\ +apiVersion: osac.openshift.io/v1alpha1 +kind: ComputeInstance +metadata: + name: computeinstance-sample +spec: + networkAttachments: + - subnetRef: my-subnet + securityGroupRefs: + - web-sg + - monitoring-sg +""" + + +def test_singular_ref_convention(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "ci.yaml", COMPUTE_INSTANCE)) + refs = _rel_pairs(r, "references") + assert ("ComputeInstance/computeinstance-sample", "Subnet/my-subnet") in refs + + +def test_plural_refs_convention(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "ci.yaml", COMPUTE_INSTANCE)) + refs = _rel_pairs(r, "references") + assert ("ComputeInstance/computeinstance-sample", "SecurityGroup/web-sg") in refs + assert ("ComputeInstance/computeinstance-sample", "SecurityGroup/monitoring-sg") in refs + + +def test_bare_field_without_ref_suffix_is_not_treated_as_a_reference(tmp_path): + # Subnet.spec.virtualNetwork (this repo's own real sample) is a UUID with + # no Ref/Refs suffix -- deliberately not modelled (module docstring). + body = ("apiVersion: osac.openshift.io/v1alpha1\nkind: Subnet\nmetadata:\n" + " name: subnet-sample\nspec:\n virtualNetwork: 00000000-0000-0000-0000-000000000000\n") + r = extract_k8s_manifest(_write(tmp_path, "subnet.yaml", body)) + assert _rel_pairs(r, "references") == set() + + +# ── label-selector matching ────────────────────────────────────────────────── + +SERVICE = "apiVersion: v1\nkind: Service\nmetadata:\n name: osac-console-proxy\nspec:\n selector:\n app: osac-console-proxy\n" +DEPLOYMENT = ( + "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: osac-console-proxy\nspec:\n" + " template:\n metadata:\n labels:\n app: osac-console-proxy\n" +) + + +def test_service_selector_and_deployment_labels_share_a_hub(tmp_path): + """Real example from this repo: osac-operator/config/console-proxy/ + service.yaml's selector matches deployment.yaml's pod template labels + exactly (single key=value pair -- the exact case, not the approximate + multi-key one).""" + svc = _write(tmp_path, "service.yaml", SERVICE) + dep = _write(tmp_path, "deployment.yaml", DEPLOYMENT) + r = extract([svc.resolve(), dep.resolve()], root=tmp_path) + + hub_ids = {n["id"] for n in r["nodes"] if n["label"] == "app=osac-console-proxy"} + assert len(hub_ids) == 1, f"expected one shared label hub, got {hub_ids}" + hub_id = hub_ids.pop() + + selects_sources = {e["source"] for e in r["edges"] if e["relation"] == "selects" and e["target"] == hub_id} + has_label_sources = {e["source"] for e in r["edges"] if e["relation"] == "has_label" and e["target"] == hub_id} + assert len(selects_sources) == 1 + assert len(has_label_sources) == 1 + + G = build_from_json({"nodes": r["nodes"], "edges": r["edges"]}) + assert G.has_node(hub_id) + + +def test_metadata_labels_also_emit_has_label(tmp_path): + body = "apiVersion: v1\nkind: Pod\nmetadata:\n name: worker\n labels:\n tier: backend\n" + r = extract_k8s_manifest(_write(tmp_path, "pod.yaml", body)) + assert ("Pod/worker", "tier=backend") in _rel_pairs(r, "has_label") + + +def test_match_expressions_are_skipped_not_guessed(tmp_path): + body = ( + "apiVersion: apps/v1\nkind: NetworkPolicy\nmetadata:\n name: np\nspec:\n" + " selector:\n matchExpressions:\n - key: tier\n operator: In\n values: [backend]\n" + ) + r = extract_k8s_manifest(_write(tmp_path, "np.yaml", body)) + assert _rel_pairs(r, "selects") == set() + + +# ── shape validation: real vs coincidental key names ──────────────────────── + +def test_key_presence_alone_is_not_enough(tmp_path): + # Has apiVersion/kind/metadata as keys, but kind isn't a real k8s-style + # PascalCase type and apiVersion isn't a real k8s apiVersion string. + body = "apiVersion: yes\nkind: not-a-real-kind\nmetadata:\n name: x\n" + r = extract_k8s_manifest(_write(tmp_path, "coincidence.yaml", body)) + assert r["nodes"] == [] + + +def test_data_yaml_returns_empty(tmp_path): + body = "openapi: 3.0.0\npaths:\n /users:\n get:\n summary: list users\n" + r = extract_k8s_manifest(_write(tmp_path, "openapi.yaml", body)) + assert r["nodes"] == [] + assert r["edges"] == [] + + +def test_docker_compose_is_out_of_scope(tmp_path): + body = "services:\n api:\n image: api:latest\n" + r = extract_k8s_manifest(_write(tmp_path, "docker-compose.yml", body)) + assert r["nodes"] == [] + + +# ── multi-document files ──────────────────────────────────────────────────── + +def test_multi_document_file_extracts_all_resources(tmp_path): + """Real shape from osac-operator/config/manager/manager.yaml: a + Namespace and a Deployment bundled in one file via `---`.""" + body = ( + "apiVersion: v1\nkind: Namespace\nmetadata:\n name: osac\n" + "---\n" + "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: osac-controller-manager\n" + " namespace: osac\n ownerReferences:\n - apiVersion: v1\n kind: Namespace\n name: osac\n" + ) + r = extract_k8s_manifest(_write(tmp_path, "manager.yaml", body)) + labels = set(_labels(r)) + assert "Namespace/osac" in labels + assert "Deployment/osac-controller-manager" in labels + assert ("Namespace/osac", "Deployment/osac-controller-manager") in _rel_pairs(r, "owns") + + +def test_no_dangling_edge_endpoints(tmp_path): + r = extract_k8s_manifest(_write(tmp_path, "deploy.yaml", WORKLOAD_WITH_REFS)) + node_ids = {n["id"] for n in r["nodes"]} + for e in r["edges"]: + assert e["source"] in node_ids + assert e["target"] in node_ids + + +# ── combined dispatcher: layering + templated-YAML safety ────────────────── + +def test_dispatcher_routes_k8s_shaped_yaml_to_rich_extraction(tmp_path): + r = extract_yaml(_write(tmp_path, "service.yaml", SERVICE)) + assert "selects" in {e["relation"] for e in r["edges"]} + + +def test_dispatcher_falls_back_to_generic_structure_for_non_k8s_yaml(tmp_path): + body = "replicaCount: 3\nimage:\n repository: myapp\n" + r = extract_yaml(_write(tmp_path, "values.yaml", body)) + labels = set(_labels(r)) + assert "replicaCount" in labels + assert "image" in labels + assert "repository" in labels + # No k8s-specific relations should appear for a plain values file. + assert not ({"owns", "uses", "selects", "has_label", "references"} & {e["relation"] for e in r["edges"]}) + + +def test_dispatcher_skips_templated_yaml_without_crashing(tmp_path, capsys): + """Real Go-templated shape from osac-operator/charts/operator/templates/ + metrics-service.yaml -- confirmed empirically (during this ticket's + investigation) to produce a tree-sitter ERROR node, not a best-effort + partial parse.""" + body = ( + "apiVersion: v1\nkind: Service\nmetadata:\n" + ' name: {{ include "osac-operator.fullname" . }}-metrics\n' + "spec:\n selector:\n {{- include \"osac-operator.selectorLabels\" . | nindent 4 }}\n" + ) + p = _write(tmp_path, "metrics-service.yaml", body) + r = extract_yaml(p) + assert r == {"nodes": [], "edges": []} + captured = capsys.readouterr() + assert "metrics-service.yaml" in captured.err + assert "did not parse" in captured.err + + +def test_dispatcher_handles_mixed_multi_doc_file(tmp_path): + """One k8s-shaped document and one plain document in the same file -- + each should be routed to the correct layer independently.""" + body = ( + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm\ndata:\n key: value\n" + "---\n" + "plainKey: plainValue\n" + ) + r = extract_yaml(_write(tmp_path, "mixed.yaml", body)) + labels = set(_labels(r)) + assert "ConfigMap/cm" in labels + assert "plainKey" in labels + + +# ── classify_file(): universal YAML/JSON coverage ─────────────────────────── + +def test_all_yaml_classified_as_code(): + # OSAC-4050: matches .json's existing precedent -- every .yaml/.yml is + # CODE unconditionally now, regardless of shape (k8s-shaped or not). + assert classify_file(Path("charts/myapp/values.yaml")) == FileType.CODE + assert classify_file(Path("k8s/deployment.yaml")) == FileType.CODE + assert classify_file(Path("openapi.yaml")) == FileType.CODE + assert classify_file(Path("docker-compose.yml")) == FileType.CODE + assert classify_file(Path(".github/actions/setup/action.yml")) == FileType.CODE + + +def test_all_json_still_classified_as_code(): + # Unaffected by this ticket -- .json was already unconditionally CODE. + assert classify_file(Path("data.json")) == FileType.CODE + assert classify_file(Path("package.json")) == FileType.CODE diff --git a/tests/test_manifest_ingest.py b/tests/test_manifest_ingest.py index 9f01675c4f..ca04bc0d7d 100644 --- a/tests/test_manifest_ingest.py +++ b/tests/test_manifest_ingest.py @@ -24,8 +24,13 @@ def test_manifests_classify_as_code_not_document(tmp_path): p = _write(tmp_path / name, "x") assert is_package_manifest_path(p) assert classify_file(p) is FileType.CODE, name - # a generic yaml stays a document - assert classify_file(_write(tmp_path / "config.yaml", "a: 1")) is FileType.DOCUMENT + # OSAC-4050: .yaml/.yml is unconditionally CODE now too (matching + # .json's existing precedent), so this no longer distinguishes a + # manifest from "a generic yaml" the way it used to -- kept as a + # regression guard that the manifest carve-out still fires (it's + # checked via is_package_manifest_path above, independent of the + # extension-based classification a plain config.yaml also gets). + assert classify_file(_write(tmp_path / "config.yaml", "a: 1")) is FileType.CODE # ── per-format parsing ─────────────────────────────────────────────────────── diff --git a/uv.lock b/uv.lock index b551d6f75c..883538dccb 100644 --- a/uv.lock +++ b/uv.lock @@ -1090,7 +1090,7 @@ wheels = [ [[package]] name = "graphifyy" -version = "0.9.41" +version = "0.9.42" source = { editable = "." } dependencies = [ { name = "networkx", version = "3.4.2", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.11'" }, @@ -1149,6 +1149,7 @@ all = [ { name = "tree-sitter-hcl" }, { name = "tree-sitter-pascal" }, { name = "tree-sitter-sql" }, + { name = "tree-sitter-yaml" }, { name = "watchdog" }, { name = "yt-dlp" }, ] @@ -1226,6 +1227,9 @@ video = [ watch = [ { name = "watchdog" }, ] +yaml = [ + { name = "tree-sitter-yaml" }, +] [package.dev-dependencies] dev = [ @@ -1325,13 +1329,15 @@ requires-dist = [ { name = "tree-sitter-swift", specifier = ">=0.7,<0.9" }, { name = "tree-sitter-typescript", specifier = ">=0.23,<0.25" }, { name = "tree-sitter-verilog", specifier = ">=1.0,<2.0" }, + { name = "tree-sitter-yaml", marker = "extra == 'all'" }, + { name = "tree-sitter-yaml", marker = "extra == 'yaml'" }, { name = "tree-sitter-zig", specifier = ">=1.0,<2.0" }, { name = "watchdog", marker = "extra == 'all'" }, { name = "watchdog", marker = "extra == 'watch'" }, { name = "yt-dlp", marker = "extra == 'all'", specifier = ">=2026.6.9" }, { name = "yt-dlp", marker = "extra == 'video'", specifier = ">=2026.6.9" }, ] -provides-extras = ["mcp", "neo4j", "falkordb", "pdf", "watch", "svg", "leiden", "office", "google", "postgres", "video", "kimi", "ollama", "bedrock", "anthropic", "gemini", "openai", "chinese", "sql", "pascal", "dm", "terraform", "all"] +provides-extras = ["mcp", "neo4j", "falkordb", "pdf", "watch", "svg", "leiden", "office", "google", "postgres", "video", "kimi", "ollama", "bedrock", "anthropic", "gemini", "openai", "chinese", "sql", "yaml", "pascal", "dm", "terraform", "all"] [package.metadata.requires-dev] dev = [ @@ -4908,6 +4914,22 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/0a/a3/229851168ec3997f1ced60b93edbeb294a0c2b3af2d71143469371c05851/tree_sitter_verilog-1.0.3-cp39-abi3-win_arm64.whl", hash = "sha256:11576eaa43f89266ab8869fb8d2fb1c22c8da74aa8dc82e67259d6560635c68f", size = 749282, upload-time = "2024-11-10T23:35:30.602Z" }, ] +[[package]] +name = "tree-sitter-yaml" +version = "0.7.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/57/b6/941d356ac70c90b9d2927375259e3a4204f38f7499ec6e7e8a95b9664689/tree_sitter_yaml-0.7.2.tar.gz", hash = "sha256:756db4c09c9d9e97c81699e8f941cb8ce4e51104927f6090eefe638ee567d32c", size = 84882, upload-time = "2025-10-07T14:40:36.071Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/38/29/c0b8dbff302c49ff4284666ffb6f2f21145006843bb4c3a9a85d0ec0b7ae/tree_sitter_yaml-0.7.2-cp310-abi3-macosx_10_9_x86_64.whl", hash = "sha256:7e269ddcfcab8edb14fbb1f1d34eed1e1e26888f78f94eedfe7cc98c60f8bc9f", size = 43898, upload-time = "2025-10-07T14:40:29.486Z" }, + { url = "https://files.pythonhosted.org/packages/18/0d/15a5add06b3932b5e4ce5f5e8e179197097decfe82a0ef000952c8b98216/tree_sitter_yaml-0.7.2-cp310-abi3-macosx_11_0_arm64.whl", hash = "sha256:0807b7966e23ddf7dddc4545216e28b5a58cdadedcecca86b8d8c74271a07870", size = 44691, upload-time = "2025-10-07T14:40:30.369Z" }, + { url = "https://files.pythonhosted.org/packages/72/92/c4b896c90d08deb8308fadbad2210fdcc4c66c44ab4292eac4e80acb4b61/tree_sitter_yaml-0.7.2-cp310-abi3-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:f1a5c60c98b6c4c037aae023569f020d0c489fad8dc26fdfd5510363c9c29a41", size = 91430, upload-time = "2025-10-07T14:40:31.16Z" }, + { url = "https://files.pythonhosted.org/packages/89/59/61f1fed31eb6d46ff080b8c0d53658cf29e10263f41ef5fe34768908037a/tree_sitter_yaml-0.7.2-cp310-abi3-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:88636d19d0654fd24f4f242eaaafa90f6f5ebdba8a62e4b32d251ed156c51a2a", size = 92428, upload-time = "2025-10-07T14:40:31.954Z" }, + { url = "https://files.pythonhosted.org/packages/e3/62/a33a04d19b7f9a0ded780b9c9fcc6279e37c5d00b89b00425bb807a22cc2/tree_sitter_yaml-0.7.2-cp310-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:1d2e8f0bb14aa4537320952d0f9607eef3021d5aada8383c34ebeece17db1e06", size = 90580, upload-time = "2025-10-07T14:40:33.037Z" }, + { url = "https://files.pythonhosted.org/packages/6c/e7/9525defa7b30792623f56b1fba9bbba361752348875b165b8975b87398fd/tree_sitter_yaml-0.7.2-cp310-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:74ca712c50fc9d7dbc68cb36b4a7811d6e67a5466b5a789f19bf8dd6084ef752", size = 90455, upload-time = "2025-10-07T14:40:33.778Z" }, + { url = "https://files.pythonhosted.org/packages/4a/d6/8d1e1ace03db3b02e64e91daf21d1347941d1bbecc606a5473a1a605250d/tree_sitter_yaml-0.7.2-cp310-abi3-win_amd64.whl", hash = "sha256:7587b5ca00fc4f9a548eff649697a3b395370b2304b399ceefa2087d8a6c9186", size = 45514, upload-time = "2025-10-07T14:40:34.562Z" }, + { url = "https://files.pythonhosted.org/packages/d8/c7/dcf3ea1c4f5da9b10353b9af4455d756c92d728a8f58f03c480d3ef0ead5/tree_sitter_yaml-0.7.2-cp310-abi3-win_arm64.whl", hash = "sha256:f63c227b18e7ce7587bce124578f0bbf1f890ac63d3e3cd027417574273642c4", size = 44065, upload-time = "2025-10-07T14:40:35.337Z" }, +] + [[package]] name = "tree-sitter-zig" version = "1.1.2" From 0a48d809729ea591cffc81dcd29c46845d400d46 Mon Sep 17 00:00:00 2001 From: Elior Erez Date: Fri, 14 Aug 2026 13:46:09 -0400 Subject: [PATCH 2/3] OSAC-4050: Fix owner-reference namespace scoping and templated-YAML detection Two real bugs found in review, both verified empirically before and after the fix: 1. Namespace-scoping bug in ownerReference resolution, reproduced against this PR's own manager.yaml-style example (Namespace owning a Deployment). The owner lookup was keyed by the CHILD's own namespace, but a cluster-scoped owner (Namespace, ClusterRole, a CRD, ...) is indexed with no namespace at all -- guaranteed miss, minting a duplicate stub instead of linking to the real node. Fixed with _resolve_owner(), which tries cluster scope for well-known cluster-scoped kinds and falls back to trying both the child's namespace and cluster scope otherwise. This was compounded by a second, real bug in yaml_dispatch.py: it called extract_k8s_resources() once PER DOCUMENT instead of once per file, defeating the two-pass same-file design k8s_manifest.py's own comments already claimed -- each document got its own empty local_nids scope, so a same-file forward reference (the Namespace declared before the Deployment that owns it) was never visible when resolving the owner. Fixed by classifying all documents first, then extracting every k8s-shaped document in the file together in one call. 2. The templated-Helm-chart safety net was unreliable in both directions, not "always warns and skips" as originally claimed. Verified empirically: a bare inline template value like "replicas: {{ .Values.x }}" (the single most common Helm templating idiom) parses with has_error=False on the specific document node checked -- the template markers look like valid, if bogus, nested flow-mapping syntax to the grammar, not a parse error, so it was silently extracted as a clean resource. Concatenated template blocks like "image: {{ .Values.x }}:{{ .Values.y }}" go the other way: has_error=True on the parse tree's stream root but NOT on the specific document node checked, so it silently produced zero output with no warning. Fixed by also checking the document's raw text for a literal template-open marker, independent of has_error -- confirmed this catches both directions. New tests assert on real node-ID uniqueness (a set of ids, and the built graph via build_from_json) for the cluster-scoped-owner scenario, not label-based _rel_pairs/_labels helpers, which are blind to two distinct node dicts sharing the same label -- exactly the shape the first bug produced. Plus direct regression tests for both new templated-YAML repro patterns. Full suite: 4395 passed, 0 failures. --- graphify/extractors/k8s_manifest.py | 44 ++++++++++++- graphify/extractors/yaml_dispatch.py | 91 ++++++++++++++++++-------- tests/test_k8s_manifest.py | 95 +++++++++++++++++++++++++++- 3 files changed, 200 insertions(+), 30 deletions(-) diff --git a/graphify/extractors/k8s_manifest.py b/graphify/extractors/k8s_manifest.py index 184ec11ce8..65c1e63b84 100644 --- a/graphify/extractors/k8s_manifest.py +++ b/graphify/extractors/k8s_manifest.py @@ -168,6 +168,47 @@ def _resource_id(kind: str, namespace: str, name: str) -> str: return _make_id(kind, namespace, name) +# Well-known built-in cluster-scoped kinds -- these never carry a namespace +# regardless of what namespace the resource referencing them (as an owner) +# happens to live in. Real bug this fixes: a namespaced child (e.g. a +# Deployment in namespace "osac") owned by a cluster-scoped resource (e.g. +# the Namespace "osac" itself) previously had its owner looked up keyed by +# the CHILD's namespace ("osac"), while the owner was indexed with an empty +# namespace (Namespace resources have no metadata.namespace of their own) -- +# a guaranteed miss, minting a duplicate stub instead of resolving to the +# real node. Not exhaustive (a cluster-scoped CRD not in this list still +# resolves correctly via the "" fallback in _resolve_owner below, as long +# as it's genuinely indexed with no namespace -- this list only lets the +# common built-ins resolve WITHOUT relying on that fallback ordering). +_CLUSTER_SCOPED_KINDS = frozenset({ + "Namespace", "Node", "PersistentVolume", "StorageClass", + "ClusterRole", "ClusterRoleBinding", "CustomResourceDefinition", + "APIService", "ValidatingWebhookConfiguration", + "MutatingWebhookConfiguration", "PriorityClass", "RuntimeClass", + "VolumeAttachment", "CSIDriver", "CSINode", +}) + + +def _resolve_owner(ref_kind: str, ref_name: str, child_namespace: str, + local_nids: dict, ref_stub) -> str: + """Resolve an ownerReference to the real local definition if one + exists in this batch, trying both the child's own namespace and + cluster scope (empty namespace) rather than assuming the owner always + shares the child's namespace -- a namespaced child's owner is USUALLY + in the same namespace (the only legal case for two namespaced + resources), but a cluster-scoped owner (Namespace, ClusterRole, a CRD + declared cluster-scoped, ...) never has one at all. + """ + candidate_namespaces = ([""] if ref_kind in _CLUSTER_SCOPED_KINDS + else [child_namespace, ""]) + for ns in candidate_namespaces: + nid = local_nids.get((ref_kind, ns, ref_name)) + if nid is not None: + return nid + guess_ns = "" if ref_kind in _CLUSTER_SCOPED_KINDS else child_namespace + return ref_stub(_resource_id(ref_kind, guess_ns, ref_name), f"{ref_kind}/{ref_name}") + + def _walk_configmap_secret_refs(node, owner_nid, namespace, add_edge, ref_stub): mapping = _mapping(node) if mapping is not None: @@ -376,8 +417,7 @@ def _add_edge(src: str, tgt: str, relation: str, line: int) -> None: ref_name = _scalar_text(ref_pairs.get("name")) if not ref_kind or not ref_name: continue - parent_nid = local_nids.get((ref_kind, namespace, ref_name)) or _ref_stub( - _resource_id(ref_kind, namespace, ref_name), f"{ref_kind}/{ref_name}") + parent_nid = _resolve_owner(ref_kind, ref_name, namespace, local_nids, _ref_stub) _add_edge(parent_nid, owner_nid, "owns", r["line"]) # -- custom annotation-based owner-reference + tenant scoping -- diff --git a/graphify/extractors/yaml_dispatch.py b/graphify/extractors/yaml_dispatch.py index b24376c3b9..6fee0525e0 100644 --- a/graphify/extractors/yaml_dispatch.py +++ b/graphify/extractors/yaml_dispatch.py @@ -1,25 +1,42 @@ """Combined YAML dispatcher -- the actual `.yaml`/`.yml` entry point. -OSAC-4050. Layered per document (a single file can bundle multiple -`---`-separated documents, confirmed against a real file in this repo -- -see graphify/extractors/_yaml_cst.py's ``all_documents`` docstring): - -1. If a document parses cleanly and looks like a real k8s resource - (``is_k8s_manifest_shape``), extract its rich ownership/reference/ - selector relationships (``graphify.extractors.k8s_manifest``). -2. Otherwise, if it parses cleanly, fall back to a generic structural walk +OSAC-4050. Two phases (a single file can bundle multiple `---`-separated +documents, confirmed against a real file in this repo -- see +graphify/extractors/_yaml_cst.py's ``all_documents`` docstring): + +1. Classify every document in the file: unparseable-for-our-purposes + (skip + warn), k8s-manifest-shaped, or generic. +2. Extract ALL k8s-shaped documents in ONE call to + ``extract_k8s_resources`` (not one call per document -- a real, + reviewer-caught bug in an earlier version of this file: calling it + per-document defeats the two-pass same-file design + ``graphify.extractors.k8s_manifest`` itself relies on for resolving a + same-file forward/cross-document reference, e.g. a Deployment owned by + a Namespace declared earlier in the same multi-document file -- each + document got its own empty ``local_nids`` scope, so the real Namespace + definition was never visible when resolving the Deployment's owner + reference, minting a duplicate stub instead of linking to it). Every + other (non-k8s-shaped) document falls back to a generic structural walk with no domain semantics (``graphify.extractors.yaml_generic``) -- universal coverage, per the user's explicit, confirmed direction change partway through this ticket's implementation: even YAML with no recognized schema should get SOME representation in the graph, rather than staying invisible the way pre-OSAC-4050 graphify left ALL YAML. -3. If a document doesn't parse cleanly at all (`node.has_error` -- e.g. a - Helm chart's Go-templated `{{ include ... }}` syntax, confirmed - empirically during this ticket's investigation to break the - tree-sitter-yaml grammar outright, producing an ERROR node rather than a - best-effort partial tree), it is skipped with a one-line warning naming - the file -- never crashed on, never walked for "structure" that would - really just be gibberish extracted from a broken parse. + +A document is treated as unparseable-for-our-purposes if EITHER +`node.has_error` is set OR its raw text contains a `{{` marker. Both +checks are necessary -- confirmed empirically (another reviewer-caught +gap) that neither alone is reliable for real Helm template syntax: +`replicas: {{ .Values.replicaCount }}` (the single most common Helm +templating idiom) parses with `has_error=False` on the specific document +node this dispatcher checks even though it's obviously not real YAML content +(`{{` opens what tree-sitter-yaml treats as valid, if bogus, nested flow- +mapping syntax) -- silently extracted as "clean" without the `{{` check. +Conversely, `image: {{ .Values.x }}:{{ .Values.y }}` (concatenated +template blocks) sets `has_error=True` on the STREAM root but NOT on the +specific per-document node checked -- silently produces zero output +without the `has_error` check, since the per-document check alone missed +an error that exists elsewhere in the same parse tree. """ from __future__ import annotations @@ -33,6 +50,16 @@ _YAML_MAX_BYTES = 1_048_576 # 1 MiB -- matches every other extractor's cap in this fork +# Go/Helm template marker. See module docstring: has_error alone (checked +# per-document) misses real cases in both directions, so any document whose +# raw text contains this is treated as unparseable-for-our-purposes +# regardless of what has_error says. +_TEMPLATE_MARKER = b"{{" + + +def _is_unparseable(doc) -> bool: + return doc.has_error or _TEMPLATE_MARKER in doc.text + def extract_yaml(path: Path) -> dict: """Extract structure from a .yaml/.yml file: rich k8s relationships for @@ -72,20 +99,30 @@ def _ensure_file_node() -> None: "source_file": str_path, "source_location": None}) file_node_added = True + # Phase 1: classify every document before extracting anything, so all + # k8s-shaped documents in this file can be extracted together in ONE + # call (see module docstring for why per-document calls are a bug, not + # just a style choice). + k8s_shaped_tops: list = [] + generic_docs: list[tuple[int, object]] = [] for doc_index, doc in enumerate(all_documents(root)): - if doc.has_error: + if _is_unparseable(doc): skipped_docs += 1 continue m = _mapping(doc) if m is not None and is_k8s_manifest_shape(m): - _ensure_file_node() - k8s_nodes, k8s_edges = extract_k8s_resources([m], str_path, file_nid) - all_nodes.extend(k8s_nodes) - all_edges.extend(k8s_edges) - continue - # Not k8s-shaped (or not even a mapping at the top level) but parses - # cleanly -- generic structural fallback. Walk from the raw doc - # value (not `m`, which is None for a non-mapping document). + k8s_shaped_tops.append(m) + else: + generic_docs.append((doc_index, doc)) + + # Phase 2: extract. + if k8s_shaped_tops: + _ensure_file_node() + k8s_nodes, k8s_edges = extract_k8s_resources(k8s_shaped_tops, str_path, file_nid) + all_nodes.extend(k8s_nodes) + all_edges.extend(k8s_edges) + + for doc_index, doc in generic_docs: generic_nodes, generic_edges, truncated = extract_generic_structure(doc, str_path, file_nid, doc_index) if generic_nodes: _ensure_file_node() @@ -103,9 +140,9 @@ def _ensure_file_node() -> None: # a silent skip. suffix = "s" if skipped_docs > 1 else "" print( - f" warning: {path.name}: {skipped_docs} document{suffix} did not " - f"parse as valid YAML (commonly Go/Helm template syntax breaking " - f"the grammar) -- skipped, not extracted.", + f" warning: {path.name}: {skipped_docs} document{suffix} not " + f"treated as real YAML (parse error, or Go/Helm template syntax " + f"detected) -- skipped, not extracted.", file=sys.stderr, ) if truncated_docs: diff --git a/tests/test_k8s_manifest.py b/tests/test_k8s_manifest.py index 340661f23b..010c5f2661 100644 --- a/tests/test_k8s_manifest.py +++ b/tests/test_k8s_manifest.py @@ -287,6 +287,69 @@ def test_no_dangling_edge_endpoints(tmp_path): assert e["target"] in node_ids +def test_cluster_scoped_owner_resolves_to_one_real_node_not_a_duplicate_stub(tmp_path): + """Regression test for a real reviewer-caught bug: a namespaced child + (Deployment, namespace "osac") owned by a cluster-scoped resource + (Namespace "osac" itself, which has no metadata.namespace of its own) + previously had its owner lookup keyed by the CHILD's namespace, missing + the real Namespace node (indexed with an empty namespace) and minting a + duplicate stub instead. + + Deliberately asserts on real node ID uniqueness/identity, not just + label pairs via _rel_pairs/_labels -- those helpers are blind to two + distinct node dicts that happen to share the same label, which is + exactly the shape this bug produced. + """ + body = ( + "apiVersion: v1\nkind: Namespace\nmetadata:\n name: osac\n" + "---\n" + "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: osac-controller-manager\n" + " namespace: osac\n ownerReferences:\n - apiVersion: v1\n kind: Namespace\n name: osac\n" + ) + r = extract_yaml(_write(tmp_path, "manager.yaml", body)) + + namespace_nodes = [n for n in r["nodes"] if n["label"] == "Namespace/osac"] + assert len(namespace_nodes) == 1, f"expected exactly one Namespace/osac node, got {namespace_nodes}" + real_namespace_id = namespace_nodes[0]["id"] + # The real definition has a source_file/source_location; a stub does not. + assert namespace_nodes[0]["source_file"], "the surviving node must be the real definition, not a sourceless stub" + + owns_edges = [e for e in r["edges"] if e["relation"] == "owns"] + assert len(owns_edges) == 1 + assert owns_edges[0]["source"] == real_namespace_id, ( + f"owns edge must bind to the real Namespace node ({real_namespace_id}), " + f"not a duplicate stub (got {owns_edges[0]['source']!r})" + ) + + +def test_cluster_scoped_owner_across_separate_files_still_resolves(tmp_path): + """Same scenario as above, but the Namespace and Deployment are two + separate files fed through extract() together -- confirms the fix + holds for genuine cross-file resolution too, not just same-file + same-batch resolution.""" + ns = _write(tmp_path, "namespace.yaml", "apiVersion: v1\nkind: Namespace\nmetadata:\n name: osac\n") + dep = _write(tmp_path, "deployment.yaml", ( + "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: osac-controller-manager\n" + " namespace: osac\n ownerReferences:\n - apiVersion: v1\n kind: Namespace\n name: osac\n" + )) + r = extract([ns.resolve(), dep.resolve()], root=tmp_path) + # Two independent per-file extractions can each mint their own raw dict + # for the same id (one real, one stub) -- true collapsing across files + # happens when the graph itself is built (same precedent as + # test_shared_action_merges_across_workflows in test_github_actions.py), + # so assert on the unique ID set and the built graph, not the raw list. + namespace_ids = {n["id"] for n in r["nodes"] if n["label"] == "Namespace/osac"} + assert len(namespace_ids) == 1, f"expected one shared Namespace/osac id, got {namespace_ids}" + namespace_id = namespace_ids.pop() + + G = build_from_json({"nodes": r["nodes"], "edges": r["edges"]}) + assert G.has_node(namespace_id) + + owns_edges = [e for e in r["edges"] if e["relation"] == "owns"] + assert len(owns_edges) == 1 + assert owns_edges[0]["source"] == namespace_id + + # ── combined dispatcher: layering + templated-YAML safety ────────────────── def test_dispatcher_routes_k8s_shaped_yaml_to_rich_extraction(tmp_path): @@ -320,7 +383,7 @@ def test_dispatcher_skips_templated_yaml_without_crashing(tmp_path, capsys): assert r == {"nodes": [], "edges": []} captured = capsys.readouterr() assert "metrics-service.yaml" in captured.err - assert "did not parse" in captured.err + assert "not treated as real YAML" in captured.err def test_dispatcher_handles_mixed_multi_doc_file(tmp_path): @@ -337,6 +400,36 @@ def test_dispatcher_handles_mixed_multi_doc_file(tmp_path): assert "plainKey" in labels +def test_dispatcher_skips_bare_inline_template_value(tmp_path, capsys): + """Regression test for a real reviewer-caught gap: `replicas: {{ .Values.x }}` + (arguably the single most common Helm templating idiom -- more common + than the `{{ include ... }}` pattern in test_dispatcher_skips_templated_yaml_without_crashing) + parses with has_error=False on the specific document node the dispatcher + checks (confirmed empirically) -- `{{` looks like valid, if bogus, + nested flow-mapping syntax to the YAML grammar, not a parse error. Only + the additional raw-text `{{` marker check catches this; has_error alone + does not.""" + body = "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: x\nspec:\n replicas: {{ .Values.replicaCount }}\n" + p = _write(tmp_path, "deployment.yaml", body) + r = extract_yaml(p) + assert r == {"nodes": [], "edges": []} + assert "not treated as real YAML" in capsys.readouterr().err + + +def test_dispatcher_skips_concatenated_template_blocks(tmp_path, capsys): + """Regression test for the opposite direction of the same reviewer-caught + gap: `image: {{ .Values.x }}:{{ .Values.y }}` sets has_error=True on the + STREAM root but NOT on the specific per-document node the dispatcher + checks (confirmed empirically) -- has_error alone misses an error that + exists elsewhere in the same parse tree. Only the additional raw-text + `{{` marker check catches this.""" + body = "apiVersion: v1\nkind: Pod\nmetadata:\n name: x\nspec:\n containers:\n - image: {{ .Values.x }}:{{ .Values.y }}\n" + p = _write(tmp_path, "pod.yaml", body) + r = extract_yaml(p) + assert r == {"nodes": [], "edges": []} + assert "not treated as real YAML" in capsys.readouterr().err + + # ── classify_file(): universal YAML/JSON coverage ─────────────────────────── def test_all_yaml_classified_as_code(): From 6232da4a0e99373a4dde349319c08f2000668ed6 Mon Sep 17 00:00:00 2001 From: Elior Erez Date: Fri, 14 Aug 2026 14:10:35 -0400 Subject: [PATCH 3/3] OSAC-4050: Fix 5 more CodeRabbit findings -- cap counting, API groups, template-gate bypass, scalar roots, test scoping All verified against actual current code before fixing; all 5 were real: 1. yaml_generic.py / json_generic.py: MAX_NODES_PER_DOCUMENT was checked against len(nodes), which is deduplicated via seen_ids. Confirmed real: make_id(path, "a.b", "c") normalizes to the same id as make_id(path, "a", "b.c") -- a mapping key containing "." collapses two genuinely different structural positions. This let the walk visit unboundedly more positions than the cap intends while len(nodes) stayed under it, defeating the safety valve for exactly the pathological, deeply-nested files it exists to bound. Fixed with a separate `visited` counter incremented per position considered, independent of dedup. 2. k8s_manifest.py: resource identity was (kind, namespace, name) with no API group, so two different API groups defining the same kind/namespace/name (a legitimate real k8s scenario -- apiVersion exists precisely to allow this, e.g. NetworkPolicy historically existed in both extensions/v1beta1 and networking.k8s.io/v1) would silently collide into one node -- the same class of bug as the namespace-scoping fix from the previous round. Fixed: _api_group() extracts the group from apiVersion (deliberately excluding version, since the same group+kind+namespace+ name accessed via a different version is the same logical resource, not a distinct one), threaded through _resource_id, local_nids keys, and ownerReferences resolution (which always carries its own apiVersion). ConfigMap/Secret refs use the core group (always correct, no ambiguity). *Ref/*Refs convention references (which carry no apiVersion of their own) default to the referencing resource's own group -- a documented, reasonable approximation, not a perfect resolution. 3. extract_k8s_manifest() -- confirmed via grep it is NOT wired into _DISPATCH (unreachable from the real extraction pipeline) but IS a real, re-exported, directly-callable entry point (used throughout this test file, importable via graphify.extract) that lacked the has_error/template-marker gate yaml_dispatch.py has. A templated file routed through this function directly would have been silently mis-extracted. Fixed by moving the shared gate into _yaml_cst.is_unparseable() and filtering in all_top_level_mappings() itself (which both extract_k8s_manifest and yaml_dispatch already route through), rather than duplicating the check in a second place. 4. yaml_generic.py / json_generic.py: a document that's a bare scalar at the top level (e.g. a file containing just `true` or `"hello"`, no mapping or sequence at all) produced zero nodes -- not even a file node, since the caller only creates one when the walk returns at least one node. Directly contradicts the "nothing invisible" goal universal coverage exists for. Fixed: mint a doc-root node for the scalar, connected to file_nid. 5. tests/test_k8s_manifest.py: the module's autouse _require_grammar fixture (skips if tree_sitter_yaml isn't installed) applied to two classify_file()-only tests that never parse YAML content and don't need the grammar at all -- reducing real coverage when the optional [yaml] extra is absent. Moved to a new, fixture-free tests/test_k8s_manifest_classify.py; confirmed passing with the extra uninstalled. 9 new/moved regression tests. Full suite: 4398 passed, 0 failures. ruff check: clean. --- graphify/extractors/_yaml_cst.py | 40 +++++++++++-- graphify/extractors/json_generic.py | 29 +++++++-- graphify/extractors/k8s_manifest.py | 89 +++++++++++++++++++++------- graphify/extractors/yaml_dispatch.py | 34 +++-------- graphify/extractors/yaml_generic.py | 35 +++++++++-- tests/test_k8s_manifest.py | 73 +++++++++++++++++------ tests/test_k8s_manifest_classify.py | 31 ++++++++++ 7 files changed, 250 insertions(+), 81 deletions(-) create mode 100644 tests/test_k8s_manifest_classify.py diff --git a/graphify/extractors/_yaml_cst.py b/graphify/extractors/_yaml_cst.py index 4afcbc820a..25abd266c3 100644 --- a/graphify/extractors/_yaml_cst.py +++ b/graphify/extractors/_yaml_cst.py @@ -140,14 +140,44 @@ def all_documents(root): yield doc +# Go/Helm template marker. `has_error` alone is not a reliable signal for +# real Go-template contamination -- confirmed empirically (OSAC-4050 review): +# a bare inline template value like `replicas: {{ .Values.x }}` parses with +# `has_error=False` on the specific document node (the template markers look +# like valid, if bogus, nested flow-mapping syntax to the YAML grammar, not +# a parse error), while a concatenated pattern like +# `image: {{ .Values.x }}:{{ .Values.y }}` sets `has_error=True` on the +# STREAM root but not on the specific document node -- has_error alone +# misses real cases in both directions. Any document whose raw text +# contains this marker is treated as unparseable-for-our-purposes +# regardless of what has_error reports. +_TEMPLATE_MARKER = b"{{" + + +def is_unparseable(doc) -> bool: + """True if *doc* should not be trusted as real YAML content -- either + tree-sitter itself flagged a parse error, or its raw text contains a + Go/Helm template marker that has_error alone does not reliably catch + (see comment above). Shared by graphify.extractors.k8s_manifest (both + the extract_k8s_resources/yaml_dispatch path and the standalone + extract_k8s_manifest entry point -- the latter previously lacked this + check entirely, a real bypass of the safety net for anyone calling it + directly) and graphify.extractors.yaml_dispatch. + """ + return doc.has_error or _TEMPLATE_MARKER in doc.text + + def all_top_level_mappings(root): - """Yield the top-level MAPPING of every document in the file (documents - that aren't mapping-shaped, or are malformed, are silently skipped -- - for callers that only care about mapping-shaped resources, e.g. a k8s - manifest). See ``all_documents`` for the version that yields every - document regardless of shape. + """Yield the top-level MAPPING of every document in the file that + parses cleanly (see ``is_unparseable``) and is mapping-shaped -- + documents that aren't mapping-shaped, or are malformed/templated, are + silently skipped. For callers that only care about mapping-shaped + resources, e.g. a k8s manifest. See ``all_documents`` for the version + that yields every document regardless of shape or parseability. """ for doc in all_documents(root): + if is_unparseable(doc): + continue m = mapping(doc) if m is not None: yield m diff --git a/graphify/extractors/json_generic.py b/graphify/extractors/json_generic.py index 3f1ffadce6..c4e36130da 100644 --- a/graphify/extractors/json_generic.py +++ b/graphify/extractors/json_generic.py @@ -64,6 +64,11 @@ def extract_generic_structure(root_value, source: bytes, str_path: str, file_nid edges: list[dict] = [] seen_ids: set[str] = set() truncated = False + # Counts every structural POSITION visited, independent of how many + # distinct node dicts that produced -- see yaml_generic.py's identical + # `visited` counter for why len(nodes) alone (deduplicated via + # seen_ids) can undercount and let the walk run past the intended cap. + visited = 0 def _mint(parts: tuple[str, ...], label: str, line: int) -> str: nid = _make_id(str_path, *parts) @@ -79,19 +84,20 @@ def _add_contains(parent_nid: str, child_nid: str, line: int) -> None: "source_location": f"L{line}", "weight": 1.0}) def _walk(node, parent_nid: str, parts: tuple[str, ...]) -> None: - nonlocal truncated + nonlocal truncated, visited if truncated or node is None: return if node.type == "object": for child in node.children: if child.type != "pair": continue - if len(nodes) >= MAX_NODES_PER_DOCUMENT: + if visited >= MAX_NODES_PER_DOCUMENT: truncated = True return key = _key_text(child, source) if not key: continue + visited += 1 value = child.child_by_field_name("value") line = child.start_point[0] + 1 child_parts = parts + (key,) @@ -102,9 +108,10 @@ def _walk(node, parent_nid: str, parts: tuple[str, ...]) -> None: if node.type == "array": items = [c for c in node.children if c.is_named] for i, item in enumerate(items): - if len(nodes) >= MAX_NODES_PER_DOCUMENT: + if visited >= MAX_NODES_PER_DOCUMENT: truncated = True return + visited += 1 text = _scalar_text(item, source) if item.type in ("string", "number", "true", "false", "null") else "" label = text if text else f"[{i}]" line = item.start_point[0] + 1 @@ -115,5 +122,19 @@ def _walk(node, parent_nid: str, parts: tuple[str, ...]) -> None: return # Scalar leaf: nothing further to mint. - _walk(root_value, file_nid, ("doc0",)) + doc_parts = ("doc0",) + if root_value is not None and root_value.type not in ("object", "array"): + # Degenerate but real case (same as yaml_generic.py's identical + # fix): the whole document is a bare scalar (e.g. a JSON file + # containing just `true` or `"hello"`). Without this, such a file + # would be completely invisible -- even its own file node, since + # the caller only creates one when this function returns at least + # one node. + text = _scalar_text(root_value, source) if root_value.type in ("string", "number", "true", "false", "null") else "" + if text: + line = root_value.start_point[0] + 1 + root_nid = _mint(doc_parts, text, line) + _add_contains(file_nid, root_nid, line) + else: + _walk(root_value, file_nid, doc_parts) return nodes, edges, truncated diff --git a/graphify/extractors/k8s_manifest.py b/graphify/extractors/k8s_manifest.py index 65c1e63b84..f518d77d86 100644 --- a/graphify/extractors/k8s_manifest.py +++ b/graphify/extractors/k8s_manifest.py @@ -160,12 +160,34 @@ def is_k8s_manifest_shape(top) -> bool: _REF_NAME_FIELDS = ("name", "secretName") -def _resource_id(kind: str, namespace: str, name: str) -> str: - # namespace is folded in when present; make_id already drops empty - # parts, so an unnamespaced resource just falls back to (kind, name) -- - # a documented simplification (this repo's own samples rarely set - # namespace explicitly), not a false-collision risk in the common case. - return _make_id(kind, namespace, name) +def _api_group(api_version: str) -> str: + """Extract the API group from an apiVersion string: "group/version" -> + group; a bare "version" (the core group -- e.g. Pod/Service/ConfigMap/ + Namespace's apiVersion is "v1" with no group at all) -> "". + + Deliberately excludes the VERSION from resource identity: the same + group+kind+namespace+name accessed via a different version of that + group (e.g. a CRD moving from v1alpha1 to v1beta1) is the same logical + resource -- that's exactly what conversion webhooks exist to preserve + -- so including version would incorrectly split one real resource into + per-version duplicates. GROUP is what distinguishes genuinely different + resources that happen to share a kind name (the actual bug this + function fixes, e.g. two different API groups both defining a + "NetworkPolicy" or a "Subnet"). + """ + return api_version.split("/", 1)[0] if "/" in api_version else "" + + +def _resource_id(group: str, kind: str, namespace: str, name: str) -> str: + # group/namespace are folded in when present; make_id already drops + # empty parts, so a core-group/unnamespaced resource just falls back to + # (kind, name) -- a documented simplification (this repo's own samples + # rarely set namespace explicitly), not a false-collision risk in the + # common case. group specifically fixes a real bug: without it, two + # different API groups defining the same kind/namespace/name (a + # legitimate, if uncommon, real k8s scenario -- apiVersion exists + # precisely to let this happen) would silently collide into one node. + return _make_id(group, kind, namespace, name) # Well-known built-in cluster-scoped kinds -- these never carry a namespace @@ -189,7 +211,7 @@ def _resource_id(kind: str, namespace: str, name: str) -> str: }) -def _resolve_owner(ref_kind: str, ref_name: str, child_namespace: str, +def _resolve_owner(ref_group: str, ref_kind: str, ref_name: str, child_namespace: str, local_nids: dict, ref_stub) -> str: """Resolve an ownerReference to the real local definition if one exists in this batch, trying both the child's own namespace and @@ -198,18 +220,27 @@ def _resolve_owner(ref_kind: str, ref_name: str, child_namespace: str, in the same namespace (the only legal case for two namespaced resources), but a cluster-scoped owner (Namespace, ClusterRole, a CRD declared cluster-scoped, ...) never has one at all. + + ref_group (from the ownerReference's own apiVersion field, which, + unlike a *Ref/*Refs convention reference, IS always present) keys the + lookup precisely -- two different API groups defining the same kind + must not collide. """ candidate_namespaces = ([""] if ref_kind in _CLUSTER_SCOPED_KINDS else [child_namespace, ""]) for ns in candidate_namespaces: - nid = local_nids.get((ref_kind, ns, ref_name)) + nid = local_nids.get((ref_group, ref_kind, ns, ref_name)) if nid is not None: return nid guess_ns = "" if ref_kind in _CLUSTER_SCOPED_KINDS else child_namespace - return ref_stub(_resource_id(ref_kind, guess_ns, ref_name), f"{ref_kind}/{ref_name}") + return ref_stub(_resource_id(ref_group, ref_kind, guess_ns, ref_name), f"{ref_kind}/{ref_name}") def _walk_configmap_secret_refs(node, owner_nid, namespace, add_edge, ref_stub): + # ConfigMap/Secret are always built-in core-API-group ("v1", group "") + # kinds -- no apiVersion is available at the reference site to derive a + # group from, but there's also no ambiguity to resolve: unlike a CRD + # kind, "ConfigMap"/"Secret" only ever exist in the core group. mapping = _mapping(node) if mapping is not None: for key, value, line in _pairs(mapping): @@ -225,7 +256,7 @@ def _walk_configmap_secret_refs(node, owner_nid, namespace, add_edge, ref_stub): if ref_name: break if ref_name: - tgt = ref_stub(_resource_id(kind, namespace, ref_name), f"{kind}/{ref_name}") + tgt = ref_stub(_resource_id("", kind, namespace, ref_name), f"{kind}/{ref_name}") add_edge(owner_nid, tgt, "uses", line) continue _walk_configmap_secret_refs(value, owner_nid, namespace, add_edge, ref_stub) @@ -239,7 +270,14 @@ def _infer_ref_kind(field_key: str, suffix: str) -> str: return base[0].upper() + base[1:] if base else "" -def _walk_ref_convention(node, owner_nid, namespace, add_edge, ref_stub): +def _walk_ref_convention(node, owner_nid, owner_group, namespace, add_edge, ref_stub): + # A *Ref/*Refs field's raw string value carries no apiVersion of its + # own (unlike ownerReferences, which is a structured object that always + # includes one) -- there's an inherent ambiguity here that can't be + # perfectly resolved. Default to the REFERENCING resource's own group: + # in practice, cross-references via this convention are between CRDs of + # the same group (e.g. this repo's own ComputeInstance -> Subnet, both + # osac.openshift.io) far more often than across groups. mapping = _mapping(node) if mapping is not None: for key, value, line in _pairs(mapping): @@ -248,19 +286,19 @@ def _walk_ref_convention(node, owner_nid, namespace, add_edge, ref_stub): if key.endswith("Refs") and len(key) > len("Refs"): kind = _infer_ref_kind(key, "Refs") for item_text, item_line in _string_items(value): - tgt = ref_stub(_resource_id(kind, namespace, item_text), f"{kind}/{item_text}") + tgt = ref_stub(_resource_id(owner_group, kind, namespace, item_text), f"{kind}/{item_text}") add_edge(owner_nid, tgt, "references", item_line) elif key.endswith("Ref") and len(key) > len("Ref"): kind = _infer_ref_kind(key, "Ref") ref_text = _scalar_text(value) if ref_text: - tgt = ref_stub(_resource_id(kind, namespace, ref_text), f"{kind}/{ref_text}") + tgt = ref_stub(_resource_id(owner_group, kind, namespace, ref_text), f"{kind}/{ref_text}") add_edge(owner_nid, tgt, "references", line) else: - _walk_ref_convention(value, owner_nid, namespace, add_edge, ref_stub) + _walk_ref_convention(value, owner_nid, owner_group, namespace, add_edge, ref_stub) return for item in _sequence_items(node): - _walk_ref_convention(_item_value(item), owner_nid, namespace, add_edge, ref_stub) + _walk_ref_convention(_item_value(item), owner_nid, owner_group, namespace, add_edge, ref_stub) def _emit_label_hub_edges(labels_node, owner_nid, relation, add_edge, ref_stub): @@ -323,8 +361,11 @@ def extract_k8s_resources(resource_tops: list, str_path: str, file_nid: str) -> just the matching subset while owning the file node itself centrally -- see graphify/extractors/yaml_dispatch.py. - Nodes: one per resource (keyed globally by (kind, namespace, name), not - file-scoped -- unlike a GitHub Actions job, the same resource can + Nodes: one per resource (keyed globally by (group, kind, namespace, + name) -- group is the apiVersion's group, e.g. "apps" for "apps/v1" or + "" for the core "v1" group, so two different API groups defining the + same kind/namespace/name never collide), not file-scoped -- unlike a + GitHub Actions job, the same resource can legitimately be defined in one file and referenced from many others). Sourceless stub nodes (type=module, same hub-collapsing exemption GitHub Actions' shared-action stubs use, #1327) stand in for anything @@ -343,7 +384,7 @@ def extract_k8s_resources(resource_tops: list, str_path: str, file_nid: str) -> edges: list[dict] = [] seen_ids: set[str] = {file_nid} seen_edges: set[tuple[str, str, str]] = set() - local_nids: dict[tuple[str, str, str], str] = {} + local_nids: dict[tuple[str, str, str, str], str] = {} def _ref_stub(nid: str, label: str) -> str: if nid not in seen_ids: @@ -370,6 +411,7 @@ def _add_edge(src: str, tgt: str, relation: str, line: int) -> None: parsed = [] for top in resource_tops: pairs = {key: (value, line) for key, value, line in _pairs(top)} + group = _api_group(_scalar_text(pairs["apiVersion"][0])) kind = _scalar_text(pairs["kind"][0]) metadata = _mapping(pairs["metadata"][0]) meta_pairs = {key: (value, line) for key, value, line in _pairs(metadata)} if metadata is not None else {} @@ -379,7 +421,7 @@ def _add_edge(src: str, tgt: str, relation: str, line: int) -> None: namespace = _scalar_text(meta_pairs["namespace"][0]) if "namespace" in meta_pairs else "" spec_entry = pairs.get("spec") parsed.append({ - "kind": kind, "name": name, "namespace": namespace, + "group": group, "kind": kind, "name": name, "namespace": namespace, "meta_pairs": meta_pairs, "spec": spec_entry[0] if spec_entry else None, "line": pairs["kind"][1], @@ -389,7 +431,7 @@ def _add_edge(src: str, tgt: str, relation: str, line: int) -> None: return nodes, edges for r in parsed: - nid = _resource_id(r["kind"], r["namespace"], r["name"]) + nid = _resource_id(r["group"], r["kind"], r["namespace"], r["name"]) if nid not in seen_ids: seen_ids.add(nid) nodes.append({"id": nid, "label": f"{r['kind']}/{r['name']}", "file_type": "code", @@ -397,7 +439,7 @@ def _add_edge(src: str, tgt: str, relation: str, line: int) -> None: edges.append({"source": file_nid, "target": nid, "relation": "contains", "confidence": "EXTRACTED", "source_file": str_path, "source_location": f"L{r['line']}", "weight": 1.0}) - local_nids[(r["kind"], r["namespace"], r["name"])] = nid + local_nids[(r["group"], r["kind"], r["namespace"], r["name"])] = nid r["nid"] = nid for r in parsed: @@ -413,11 +455,12 @@ def _add_edge(src: str, tgt: str, relation: str, line: int) -> None: if ref_mapping is None: continue ref_pairs = {k: v for k, v, _ln in _pairs(ref_mapping)} + ref_group = _api_group(_scalar_text(ref_pairs.get("apiVersion"))) ref_kind = _scalar_text(ref_pairs.get("kind")) ref_name = _scalar_text(ref_pairs.get("name")) if not ref_kind or not ref_name: continue - parent_nid = _resolve_owner(ref_kind, ref_name, namespace, local_nids, _ref_stub) + parent_nid = _resolve_owner(ref_group, ref_kind, ref_name, namespace, local_nids, _ref_stub) _add_edge(parent_nid, owner_nid, "owns", r["line"]) # -- custom annotation-based owner-reference + tenant scoping -- @@ -438,7 +481,7 @@ def _add_edge(src: str, tgt: str, relation: str, line: int) -> None: if r["spec"] is not None: _walk_configmap_secret_refs(r["spec"], owner_nid, namespace, _add_edge, _ref_stub) - _walk_ref_convention(r["spec"], owner_nid, namespace, _add_edge, _ref_stub) + _walk_ref_convention(r["spec"], owner_nid, r["group"], namespace, _add_edge, _ref_stub) spec_pairs = {} if r["spec"] is not None: diff --git a/graphify/extractors/yaml_dispatch.py b/graphify/extractors/yaml_dispatch.py index 6fee0525e0..5935a1a008 100644 --- a/graphify/extractors/yaml_dispatch.py +++ b/graphify/extractors/yaml_dispatch.py @@ -23,43 +23,25 @@ recognized schema should get SOME representation in the graph, rather than staying invisible the way pre-OSAC-4050 graphify left ALL YAML. -A document is treated as unparseable-for-our-purposes if EITHER -`node.has_error` is set OR its raw text contains a `{{` marker. Both -checks are necessary -- confirmed empirically (another reviewer-caught -gap) that neither alone is reliable for real Helm template syntax: -`replicas: {{ .Values.replicaCount }}` (the single most common Helm -templating idiom) parses with `has_error=False` on the specific document -node this dispatcher checks even though it's obviously not real YAML content -(`{{` opens what tree-sitter-yaml treats as valid, if bogus, nested flow- -mapping syntax) -- silently extracted as "clean" without the `{{` check. -Conversely, `image: {{ .Values.x }}:{{ .Values.y }}` (concatenated -template blocks) sets `has_error=True` on the STREAM root but NOT on the -specific per-document node checked -- silently produces zero output -without the `has_error` check, since the per-document check alone missed -an error that exists elsewhere in the same parse tree. +"Unparseable-for-our-purposes" is decided by +`graphify.extractors._yaml_cst.is_unparseable` -- shared with +`k8s_manifest.all_top_level_mappings` (used by the standalone +`extract_k8s_manifest` entry point) so both paths apply the identical +gate; see that function's docstring for why `has_error` alone is not +reliable in either direction for real Helm template syntax. """ from __future__ import annotations import sys from pathlib import Path -from graphify.extractors._yaml_cst import all_documents, mapping as _mapping +from graphify.extractors._yaml_cst import all_documents, is_unparseable, mapping as _mapping from graphify.extractors.base import _make_id from graphify.extractors.k8s_manifest import extract_k8s_resources, is_k8s_manifest_shape from graphify.extractors.yaml_generic import extract_generic_structure _YAML_MAX_BYTES = 1_048_576 # 1 MiB -- matches every other extractor's cap in this fork -# Go/Helm template marker. See module docstring: has_error alone (checked -# per-document) misses real cases in both directions, so any document whose -# raw text contains this is treated as unparseable-for-our-purposes -# regardless of what has_error says. -_TEMPLATE_MARKER = b"{{" - - -def _is_unparseable(doc) -> bool: - return doc.has_error or _TEMPLATE_MARKER in doc.text - def extract_yaml(path: Path) -> dict: """Extract structure from a .yaml/.yml file: rich k8s relationships for @@ -106,7 +88,7 @@ def _ensure_file_node() -> None: k8s_shaped_tops: list = [] generic_docs: list[tuple[int, object]] = [] for doc_index, doc in enumerate(all_documents(root)): - if _is_unparseable(doc): + if is_unparseable(doc): skipped_docs += 1 continue m = _mapping(doc) diff --git a/graphify/extractors/yaml_generic.py b/graphify/extractors/yaml_generic.py index e44ef1af04..9f416242ce 100644 --- a/graphify/extractors/yaml_generic.py +++ b/graphify/extractors/yaml_generic.py @@ -68,6 +68,15 @@ def extract_generic_structure(doc_value, str_path: str, file_nid: str, doc_index edges: list[dict] = [] seen_ids: set[str] = set() truncated = False + # Counts every structural POSITION visited, independent of how many + # distinct node dicts that produced. len(nodes) alone undercounts when + # two different positions normalize to the same id (confirmed real: + # make_id(path, "a.b", "c") == make_id(path, "a", "b.c") -- a mapping + # key containing "." collapses with a differently-shaped path), which + # let the walk visit unboundedly more positions than the cap intends + # while len(nodes) stayed under it -- exactly defeating the point of a + # safety valve for a pathological, deeply-nested file. + visited = 0 def _mint(parts: tuple[str, ...], label: str, line: int) -> str: nid = _make_id(str_path, *parts) @@ -83,15 +92,16 @@ def _add_contains(parent_nid: str, child_nid: str, line: int) -> None: "source_location": f"L{line}", "weight": 1.0}) def _walk(node, parent_nid: str, parts: tuple[str, ...]) -> None: - nonlocal truncated + nonlocal truncated, visited if truncated or node is None: return mapping = _mapping(node) if mapping is not None: for key, value, line in _pairs(mapping): - if len(nodes) >= MAX_NODES_PER_DOCUMENT: + if visited >= MAX_NODES_PER_DOCUMENT: truncated = True return + visited += 1 child_parts = parts + (key,) child_nid = _mint(child_parts, key, line) _add_contains(parent_nid, child_nid, line) @@ -100,9 +110,10 @@ def _walk(node, parent_nid: str, parts: tuple[str, ...]) -> None: seq_items = list(_sequence_items(node)) if seq_items: for i, item in enumerate(seq_items): - if len(nodes) >= MAX_NODES_PER_DOCUMENT: + if visited >= MAX_NODES_PER_DOCUMENT: truncated = True return + visited += 1 item_value = _item_value(item) text = _scalar_text(item_value) label = text if text else f"[{i}]" @@ -116,5 +127,21 @@ def _walk(node, parent_nid: str, parts: tuple[str, ...]) -> None: # caller already minted for this position represents it. doc_parts = (f"doc{doc_index}",) - _walk(doc_value, file_nid, doc_parts) + mapping_root = _mapping(doc_value) + seq_root = list(_sequence_items(doc_value)) if mapping_root is None and doc_value is not None else [] + if doc_value is not None and mapping_root is None and not seq_root: + # Degenerate but real case: the WHOLE document is a bare scalar + # (e.g. a file containing just `true` or `"hello"`, no mapping or + # sequence at all). Without this, such a file -- and even its own + # file node, since the caller only creates one when this function + # returns at least one node -- would be completely invisible, + # directly contradicting the "nothing invisible" goal universal + # coverage exists for. + text = _scalar_text(doc_value) + if text: + line = doc_value.start_point[0] + 1 + root_nid = _mint(doc_parts, text, line) + _add_contains(file_nid, root_nid, line) + else: + _walk(doc_value, file_nid, doc_parts) return nodes, edges, truncated diff --git a/tests/test_k8s_manifest.py b/tests/test_k8s_manifest.py index 010c5f2661..78e2bbdbbe 100644 --- a/tests/test_k8s_manifest.py +++ b/tests/test_k8s_manifest.py @@ -10,7 +10,6 @@ import pytest from graphify.build import build_from_json -from graphify.detect import FileType, classify_file from graphify.extract import extract, extract_k8s_manifest, extract_yaml @@ -68,6 +67,47 @@ def test_owner_reference_resolves_to_real_definition_across_files(tmp_path): assert rs_ids.pop() in {e["source"] for e in r["edges"] if e["relation"] == "owns"} +def test_different_api_groups_same_kind_namespace_name_do_not_collide(tmp_path): + """Regression test for a real reviewer-caught bug: resource identity + was (kind, namespace, name) with no API group, so two genuinely + different resources from different groups sharing a kind/namespace/name + (a legitimate real k8s scenario -- apiVersion exists precisely to allow + this, e.g. NetworkPolicy historically existed in both extensions/v1beta1 + and networking.k8s.io/v1) would silently merge into one node.""" + body = ( + "apiVersion: apps/v1\nkind: NetworkPolicy\nmetadata:\n name: np\n namespace: osac\n" + "---\n" + "apiVersion: networking.k8s.io/v1\nkind: NetworkPolicy\nmetadata:\n name: np\n namespace: osac\n" + ) + r = extract_k8s_manifest(_write(tmp_path, "networkpolicies.yaml", body)) + np_nodes = [n for n in r["nodes"] if n["label"] == "NetworkPolicy/np"] + assert len(np_nodes) == 2, f"expected two distinct NetworkPolicy/np nodes (different groups), got {np_nodes}" + assert len({n["id"] for n in np_nodes}) == 2, "the two resources must have distinct ids" + + +def test_owner_reference_apiversion_group_disambiguates_cross_group_owner(tmp_path): + """An ownerReference names its owner's group via its own apiVersion + field -- confirms that field is actually used to resolve to the + correctly-grouped owner, not just any resource sharing the kind/name.""" + body = ( + "apiVersion: apps/v1\nkind: Foo\nmetadata:\n name: shared-name\n" + "---\n" + "apiVersion: osac.openshift.io/v1alpha1\nkind: Foo\nmetadata:\n name: shared-name\n" + "---\n" + "apiVersion: v1\nkind: Bar\nmetadata:\n name: child\n" + " ownerReferences:\n - apiVersion: osac.openshift.io/v1alpha1\n kind: Foo\n name: shared-name\n" + ) + r = extract_k8s_manifest(_write(tmp_path, "mixed-groups.yaml", body)) + foo_nodes = {n["id"]: n for n in r["nodes"] if n["label"] == "Foo/shared-name"} + assert len(foo_nodes) == 2 + owns_edges = [e for e in r["edges"] if e["relation"] == "owns"] + assert len(owns_edges) == 1 + owner_id = owns_edges[0]["source"] + assert foo_nodes[owner_id]["source_file"], "must resolve to a real definition, not a fresh stub" + # The resolved owner must be the osac.openshift.io one, not the apps one. + assert "osac" in owner_id or "openshift" in owner_id, f"resolved to the wrong group's Foo: {owner_id}" + + # ── custom annotation-based owner-reference + tenant (architecture-patterns.md) ── ANNOTATED_CHILD = """\ @@ -261,6 +301,19 @@ def test_docker_compose_is_out_of_scope(tmp_path): assert r["nodes"] == [] +def test_standalone_entry_point_also_rejects_templated_yaml(tmp_path): + """Regression test for a real reviewer-caught bypass: extract_k8s_manifest() + is a separate, directly-callable, re-exported entry point (not wired + into the real _DISPATCH pipeline, but used directly by most of this + test file and importable via graphify.extract) that previously lacked + the has_error/template-marker gate yaml_dispatch.py has -- a templated + file routed through THIS function instead of extract_yaml() would have + been silently mis-extracted as a valid resource.""" + body = "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: x\nspec:\n replicas: {{ .Values.replicaCount }}\n" + r = extract_k8s_manifest(_write(tmp_path, "deployment.yaml", body)) + assert r == {"nodes": [], "edges": []} + + # ── multi-document files ──────────────────────────────────────────────────── def test_multi_document_file_extracts_all_resources(tmp_path): @@ -428,21 +481,3 @@ def test_dispatcher_skips_concatenated_template_blocks(tmp_path, capsys): r = extract_yaml(p) assert r == {"nodes": [], "edges": []} assert "not treated as real YAML" in capsys.readouterr().err - - -# ── classify_file(): universal YAML/JSON coverage ─────────────────────────── - -def test_all_yaml_classified_as_code(): - # OSAC-4050: matches .json's existing precedent -- every .yaml/.yml is - # CODE unconditionally now, regardless of shape (k8s-shaped or not). - assert classify_file(Path("charts/myapp/values.yaml")) == FileType.CODE - assert classify_file(Path("k8s/deployment.yaml")) == FileType.CODE - assert classify_file(Path("openapi.yaml")) == FileType.CODE - assert classify_file(Path("docker-compose.yml")) == FileType.CODE - assert classify_file(Path(".github/actions/setup/action.yml")) == FileType.CODE - - -def test_all_json_still_classified_as_code(): - # Unaffected by this ticket -- .json was already unconditionally CODE. - assert classify_file(Path("data.json")) == FileType.CODE - assert classify_file(Path("package.json")) == FileType.CODE diff --git a/tests/test_k8s_manifest_classify.py b/tests/test_k8s_manifest_classify.py new file mode 100644 index 0000000000..9f961ec35e --- /dev/null +++ b/tests/test_k8s_manifest_classify.py @@ -0,0 +1,31 @@ +"""classify_file() coverage for OSAC-4050's universal YAML/JSON direction +change (graphify/detect.py). + +Deliberately separate from tests/test_k8s_manifest.py: these tests only +call classify_file(), never parse YAML/JSON content, so they don't need +tree_sitter_yaml installed -- but that module's `_require_grammar` autouse +fixture would skip them anyway if they lived there (a real reviewer-caught +gap: reduced test coverage when the optional [yaml] extra is absent, for +tests that never needed it in the first place). +""" +from __future__ import annotations + +from pathlib import Path + +from graphify.detect import FileType, classify_file + + +def test_all_yaml_classified_as_code(): + # OSAC-4050: matches .json's existing precedent -- every .yaml/.yml is + # CODE unconditionally now, regardless of shape (k8s-shaped or not). + assert classify_file(Path("charts/myapp/values.yaml")) == FileType.CODE + assert classify_file(Path("k8s/deployment.yaml")) == FileType.CODE + assert classify_file(Path("openapi.yaml")) == FileType.CODE + assert classify_file(Path("docker-compose.yml")) == FileType.CODE + assert classify_file(Path(".github/actions/setup/action.yml")) == FileType.CODE + + +def test_all_json_still_classified_as_code(): + # Unaffected by this ticket -- .json was already unconditionally CODE. + assert classify_file(Path("data.json")) == FileType.CODE + assert classify_file(Path("package.json")) == FileType.CODE