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..25abd266c3 --- /dev/null +++ b/graphify/extractors/_yaml_cst.py @@ -0,0 +1,183 @@ +"""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 + + +# 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 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_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..c4e36130da --- /dev/null +++ b/graphify/extractors/json_generic.py @@ -0,0 +1,140 @@ +"""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 + # 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) + 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, visited + if truncated or node is None: + return + if node.type == "object": + for child in node.children: + if child.type != "pair": + continue + 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,) + 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 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 + 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. + + 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 new file mode 100644 index 0000000000..f518d77d86 --- /dev/null +++ b/graphify/extractors/k8s_manifest.py @@ -0,0 +1,536 @@ +"""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 _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 +# 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_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 + 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. + + 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_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_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): + 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, 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): + 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(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(owner_group, kind, namespace, ref_text), f"{kind}/{ref_text}") + add_edge(owner_nid, tgt, "references", line) + else: + _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, owner_group, 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 (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 + 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], 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)} + 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 {} + 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({ + "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], + }) + + if not parsed: + return nodes, edges + + for r in parsed: + 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", + "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["group"], 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_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_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 -- + 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, r["group"], 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..5935a1a008 --- /dev/null +++ b/graphify/extractors/yaml_dispatch.py @@ -0,0 +1,138 @@ +"""Combined YAML dispatcher -- the actual `.yaml`/`.yml` entry point. + +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. + +"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, 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 + + +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 + + # 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 is_unparseable(doc): + skipped_docs += 1 + continue + m = _mapping(doc) + if m is not None and is_k8s_manifest_shape(m): + 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() + 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} not " + f"treated as real YAML (parse error, or Go/Helm template syntax " + f"detected) -- 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..9f416242ce --- /dev/null +++ b/graphify/extractors/yaml_generic.py @@ -0,0 +1,147 @@ +"""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 + # 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) + 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, visited + if truncated or node is None: + return + mapping = _mapping(node) + if mapping is not None: + for key, value, line in _pairs(mapping): + 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) + _walk(value, child_nid, child_parts) + return + seq_items = list(_sequence_items(node)) + if seq_items: + for i, item in enumerate(seq_items): + 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}]" + 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}",) + 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/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..78e2bbdbbe --- /dev/null +++ b/tests/test_k8s_manifest.py @@ -0,0 +1,483 @@ +"""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.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"} + + +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 = """\ +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"] == [] + + +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): + """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 + + +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): + 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 "not treated as real YAML" 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 + + +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 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 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"