feat(yandex-cloud): add metric, log, instance, balancer tools and a generic API reader - #4990
feat(yandex-cloud): add metric, log, instance, balancer tools and a generic API reader#4990nowhere-in-space wants to merge 16 commits into
Conversation
…neric API reader The Yandex Cloud integration could authenticate and verify a folder but could not read anything from it. This adds the first tool families and the generic reader that covers the rest of the API. Curated tools hard-code their own paths: - monitoring: query metric series, list metric names and labels - compute: list instances, read serial console output for diagnosis - network: report unhealthy network and application balancer targets Everything else is reached through execute_yc_operation, which resolves a path via find_yc_api against an index generated from Yandex's own protobuf definitions. Only get: bindings are extracted, so the index doubles as the allowlist that keeps the reader GET-only; build_api_index.py regenerates it and records the cloudapi commit it came from. The client regains post(): Monitoring's metric read takes a request body, which is the one read in this set that a GET cannot express. Tools are registered for discovery, classified for Sentry coverage, and held to a seventeen-schema ceiling so later families stay inside the per-turn budget.
Cloud Logging is the one Yandex Cloud read with no REST endpoint: the entry reader is gRPC-only. The stubs therefore ship as an optional extra, following the same shape as the kafka and azure_sql extras - imported inside the function that needs them, with a failure that explains the install rather than reading as a broken integration. Listing log groups is plain REST on the management host and works without the extra. Only reading entries needs it. The reader host is separate from the management one, entry reads are limited to five per second, and retention is 31 days, so the client throttles, honours Retry-After, caps filter expressions, and flags a window that reaches past retention. The install hint is checked against the extras this project declares, so a message naming something unavailable fails the suite rather than reaching a user.
…states path Two defects found while auditing the port against the plugin it came from. The endpoint index was in no distribution artifact. `package-data` lists only `**/SKILL.md` under integrations, and the release manifest globs the same, so `api_index.json` was absent from both the wheel and the frozen binary. The loader swallowed the resulting OSError and returned an empty index, which the tool reports as "no endpoints" - so `find_yc_api`, the entry point the workflow guidance routes to, answered as if Yandex exposed nothing. It now ships, the wheel validator covers it, and the loader logs which file it could not read. The network balancer tool called `:targetStates`. The proto-derived index in this repo spells the binding `:getTargetStates`, as does the application balancer call ten lines below it, so target health came back empty against a real cloud. The test stubbed the misspelling and therefore pinned the bug. Also, guidance and tests that were carrying their own small lies: SKILL.md pointed the model at tools from families this tree does not ship yet; `list_yc_metrics` was the only tool here with no execution coverage; and the schema text that stops the model writing PromQL had nothing holding it in place.
…ng it is whole Two reads answered as if they had seen everything. `get_yc_lb_health` took the first page of each balancer type and dropped the rest without a word. "No unhealthy targets" is the one answer that must never be a guess, so the tool now reports `complete: false` and points at the `type` filter and the generic reader for the remainder. `list_yc_instances` matches a name fragment locally, because Yandex's own filter compares names for equality and has no substring form. That makes the match local to the page just fetched, so an instance on a later page reads as absent. It now says so when a filtered read has more pages behind it. Also records why Monitoring asks for `gapFilling: NULL`: PREVIOUS would carry the last value forward, and a service that stopped reporting would come back as a flat healthy line.
Reading an instance's operation history returned a bare 404, as did every other
collection nested under a named resource: cluster hosts, node groups, disk and
balancer operations. A large part of the API answered as if the resource did not
exist.
The executor decided to send folderId from "is this a collection", but a folder
scopes only the collections directly under the version. A nested one is already
scoped by the resource named in its path, and Yandex rejects the extra parameter
the same silent way it rejects one on a single-resource read.
Verified against the live API: /compute/v1/instances/{id}/operations returns the
history with no folderId and 404s with it, while pageSize is accepted either
way - so only the folder is withheld and paging still applies.
Greptile code reviewThis repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md. Run a review — add a PR comment with: Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5. Optional: automate with the greploop skill. |
|
@greptile review |
Greptile SummaryThis PR adds read-only Yandex Cloud investigation tools, a generated generic API endpoint index, and the packaging and registry support needed to expose them.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the previously reported areas. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| integrations/yc_network/tools/yc_lb_tool/init.py | Implements network and application load-balancer health collection, including graph traversal, pagination, completeness reporting, and relationship-aware target aggregation; the previously reported defects are addressed. |
| integrations/yc_logging/tools/yc_logs_tool/init.py | Adds Cloud Logging tools and now provides empty-result guidance consistent with available tools and the actual since and until schema. |
| integrations/yandex_cloud/rest_client.py | Adds POST support for read APIs with request bodies and consistently wraps raw responses with normalized pagination metadata. |
| integrations/yandex_cloud/api_index.py | Loads and searches the packaged read-endpoint index used by the generic Yandex Cloud reader. |
| integrations/yandex_cloud/tools/yc_operation_tool/init.py | Executes allowlisted generic Yandex Cloud read operations while rejecting state-changing action paths. |
| platform/packaging/release_manifest.py | Includes the generated Yandex Cloud API index in packaged and frozen distributions. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Agent[OpenSRE investigation agent] --> Curated{Curated tool available?}
Curated -->|Yes| Tools[Metrics, logs, compute, or balancer tool]
Curated -->|No| Lookup[find_yc_api]
Lookup --> Index[Packaged read-only API index]
Index --> Execute[execute_yc_operation]
Tools --> Client[Yandex Cloud clients]
Execute --> Client
Client --> YC[Yandex Cloud APIs]
YC --> Evidence[Read-only investigation evidence]
Reviews (11): Last reviewed commit: "fix(yandex-cloud): key target health by ..." | Re-trigger Greptile
CodeQL flagged three new alerts, all in yc_logging. The high-severity one is a false positive in a test - a `host in url` assertion reads to CodeQL as incomplete URL sanitization even though it only checks which host the tool called. Made it exact: parse the URL and compare the hostname, which is both CodeQL-clean and a stronger assertion. The client carried a `logger` that nothing used - removed it and its now-unused import. And the throttle's last-read timestamp was a bare module global whose reassignment reads as write-but-never-used, because the value is consumed on the next call; moved it onto a small dict so the read and write are unambiguous. No behaviour change - the rate limiting is identical.
|
@greptile review |
…dead tool reference Two review findings, both real. get_yc_lb_health summarised unhealthy targets from network balancers only. The application balancer stored its raw target states under a different key that the aggregation never read, so a failing application backend left unhealthy_targets empty - the one answer that must not be wrong. Both kinds return the same getTargetStates shape, so a shared normaliser now feeds both into the summary; the raw application response stays for the detail the flat view drops. The empty-Cloud-Logging guidance still told the agent to read managed-database logs with read_yc_db_logs, a tool this PR does not ship. Reworded to say those logs are not readable yet and to fall back to metrics and cluster state.
|
@greptile review |
…balancer does not serve
The application balancer reused the network balancer's :getTargetStates action,
but that verb does not exist for it: its target states live under a nested path
keyed by backend group and target group
(/apploadbalancer/v1/loadBalancers/{id}/targetStates/{backend_group_id}/{target_group_id}),
which needs the balancer's backend-group graph walked first. The old call 404s
against a real cloud, so application target health was never real.
Rather than ship a request the API rejects or fabricate health from it, the tool
now lists application balancers with their status and carries a pointer to the
real nested path, reachable through execute_yc_operation. Network balancers keep
full per-target health, which does follow the :getTargetStates contract. The
output docs say which is which so unhealthy_targets is not read as covering both.
|
@greptile review |
…h the backend graph
The application balancer keeps target states behind a nested path that the
network balancer's :getTargetStates action does not reach:
/apploadbalancer/v1/loadBalancers/{id}/targetStates/{backend_group}/{target_group}.
Getting there means walking the balancer's graph - listener to HTTP router to
route to backend group to target group - and only then reading targetStates,
where health is reported per zone (a target is unhealthy only when every zone
fails its active health check).
get_yc_lb_health now walks that graph and normalises application targets into
the same shape as network ones, so a failing application backend reaches the
same unhealthy_targets summary. Single-resource reads on that path pass
page_size=None, since Yandex answers a target-state read carrying a stray
pageSize with a bare 404. Shapes captured from a live application balancer.
|
@greptile review |
…get health The backend-group walk read only http routes, so an application balancer that routes over gRPC never reached its backend group, and its unhealthy targets were left out of unhealthy_targets. A route names its backend group under its own protocol key, so both http and grpc are now inspected. The backend group itself already handled http, grpc and stream backends.
|
@greptile review |
|
@codex review |
… index
A get: binding is not on its own proof that a method only reads.
OperationService.Cancel is bound to GET /operations/{operation_id}:cancel, and
cancelling a running operation is a mutation - so the index whose entire purpose
is to carry no write path carried exactly one, and the generic reader would have
executed it.
The read-only test missed it because it checked the verbs Create, Update,
Delete, Start and Stop, and Cancel is none of those.
Closed in three layers: the generator now drops mutating RPC names whatever
their HTTP binding, the shipped index has the entry removed (937 -> 936
endpoints, all 68 services intact), and the client refuses state-changing action
suffixes as a backstop. The suffix guard runs in the tool rather than only in
the client, because the synthetic-backend path never reaches the client and a
guarantee that depends on which branch a call took is not a guarantee. Read-
shaped actions the tools rely on - :serialPortOutput, :getTargetStates, :byValue
- still pass, and the tests now check verbs by prefix so CancelOperation is
caught as well as Cancel.
The generic reader's description and the client's unknown-service error both told the model to call list_yc_services. No such tool ships - the discovery tool is find_yc_api - so an agent that followed either was handed a dead next step at exactly the moment it was already lost. SKILL.md carried the same wrong name and was fixed earlier; these two survived because a documentation sweep does not reach string literals in code. A test now asserts every tool name mentioned in the reader's description is one the registry actually has.
|
@greptile review |
The application walk crosses three reads - virtual hosts, backend group, target states - and a failure in any of them produced an empty target list that the result presented as complete. An unreadable graph is indistinguishable from a balancer with no unhealthy targets, and "nothing is wrong" is the one answer that must never be a guess. A failed virtual-host read is now reported instead of being treated as an empty collection, and the per-balancer target_states_error is promoted into the same complete: false signal that page truncation already sets, naming the balancers whose health could not be read. Tests cover a failure at each of the three reads, plus a fully readable graph that must not be flagged.
|
@greptile review |
…uments correctly Two more places where a confident answer was not backed by what was read. The application walk read only the first page of a router's virtual hosts, so a backend group reachable only from a later page contributed no targets while the result still claimed to be complete - the same silent truncation the balancer list had, one level deeper in the graph. Every page is followed now, with a page cap that is reported rather than passed off as a full read. The empty-log advice told the agent to retry a past incident with from_time and to_time. read_yc_logs takes since and until, so following that advice would have been rejected or silently answered for the default recent window. A test now checks the advice only names arguments the schema actually has.
|
@greptile review |
…ain HTTP A listener is http, tls or stream, and the tls one nests a default handler plus any number of SNI handlers, each of which is an httpHandler or a streamHandler. The walk only understood http.handler and tls.defaultHandler.httpHandler, so a TLS SNI route, a stream listener or a TLS stream handler contributed no targets while the result still reported complete health. Stream handlers matter twice over: they name their backend group on the listener instead of going through an HTTP router, so walking routers alone could never reach them however many router shapes were covered. Shapes taken from the API reference rather than guessed. Tests cover all five: http.handler, tls.defaultHandler.httpHandler, tls.sniHandlers[].handler. httpHandler, stream.handler, and the two streamHandler forms.
|
@greptile review |
One backend can serve several target groups and be healthy in one while failing in another. Collapsing the walk's results by IP alone let the healthy reading arrive first and discard the failing one, so unhealthy_targets came back empty for a backend that was actively failing a route. Records are now keyed by (backend group, target group, address) and carry both ids, which is also what tells an operator which route is affected rather than just which host.
|
@greptile review |
Part of #4605
Describe the changes you have made in this PR -
Second PR of the Yandex Cloud series. #4947 landed the credential and REST client layer but shipped no tools, so a connected folder could be verified and then not read. This adds the first tool families and the generic reader that covers everything else. Most of these tools are ported from my working plugin, opensre-yc-plugin, where they have been running against real Yandex Cloud infrastructure.
Curated tools (they hard-code their own paths and resolve the host through the endpoint registry added in #4947):
integrations/yc_monitoringquery_yc_metrics,list_yc_metricsintegrations/yc_loggingread_yc_logs,list_yc_log_groupsintegrations/yc_computelist_yc_instances,get_yc_instance_diagnosticsintegrations/yc_networkget_yc_lb_healthGeneric reader in
integrations/yandex_cloud/tools:find_yc_apiresolves a path by plain words ("security groups", "certificates"), andexecute_yc_operationreads it. That is what makes services without a dedicated tool reachable - container registry, DNS, KMS, Lockbox, YDB and the rest - without a tool per service.api_index.json. Yandex generates its REST API from the protos in yandex-cloud/cloudapi, and every method carries its REST binding as agoogle.api.httpoption.build_api_index.pyextracts only theget:bindings, which is why the index is also the allowlist: the client refuses anything but GET, and a write path is never in the file to begin with. The file records the cloudapi commit and build date, so "is this stale" is answerable without rebuilding and diffing.Coverage. The index carries 937 read endpoints across 68 services, and every service in it resolves to a reachable host (asserted in the tests). Everything SRE-shaped is in there - compute, vpc, all managed databases, managed kubernetes, both load balancers, serverless, container registry, dns, iam, kms, lockbox, certificates, ydb, storage, backup, quota-manager, resource-manager, data transfer, audit trails, cdn. What the index does not carry is what Yandex exposes with no GET binding at all: model inference (
ai-*), data-plane endpoints that read a payload rather than a resource (kms-crypto,lockbox-payload,*-data), and the two reads that take a request body - Monitoring's metric read and Cloud Logging's entry reader, which are exactly what the curated tools above cover. So a read either has a dedicated tool or is onefind_yc_apicall away; nothing readable is unreachable.post()returns to the client. It was removed in #4947 as unused, which was correct at the time. Monitoring's metric read takes a request body and is the one read here that a GET cannot express; it is the only caller.Read-only throughout. When an investigation concludes something needs changing, the tools report the exact
yccommand for an operator rather than attempting it.One optional dependency. Cloud Logging is the only Yandex Cloud read with no REST endpoint - the entry reader is gRPC-only - so Yandex's generated stubs ship as the
yandex_cloud_logsextra, in the same shape as the existingkafka,postgresqlandazure_sqlextras: imported inside the function that needs them, with a failure that explains the install. Listing log groups is plain REST and works without it. A test asserts the install hint names an extra this project actually declares, so a message pointing at something uninstallable fails the suite rather than reaching a user.A folder does not scope a nested collection. The executor decided to send
folderIdfrom "is this a collection", but a folder scopes only the collections directly under the version./compute/v1/instances/{id}/operationsis already scoped by the instance in its path, and Yandex rejects the extra parameter with the same bare 404 it gives a single-resource read - so operation histories, cluster hosts, node groups and disk operations all answered as if the resource did not exist. Confirmed against the live API (no folderId returns the history, folderId 404s, pageSize is fine either way) and fixed.The commits after the first two fix defects found auditing this port, both of which would have shipped silently.
api_index.jsonwas in neither the wheel nor the frozen binary -package-dataand the release manifest both glob only**/SKILL.md- and the loader swallowed the resulting error, sofind_yc_apianswered "no endpoints" on any packaged install. It is now packaged, covered by the wheel validator, and the loader says which file it could not read. Separately, the balancer tool called:targetStateswhere this repo's own proto-derived index (and the application-balancer call ten lines below) spell it:getTargetStates; target health came back empty against a real cloud, and the test had stubbed the misspelling, pinning the bug.A folder with more balancers or instances than one page holds now says so instead of implying the list is whole:
get_yc_lb_healthreportscomplete: false, and a name-filteredlist_yc_instanceswith further pages warns that the match was local to the page. "No unhealthy target" must never be a guess.Read-only hardening. A get: binding is not on its own proof that a method only reads: OperationService.Cancel is bound to GET /operations/{id}:cancel, so the index whose purpose is to carry no write path carried exactly one, and find_yc_api surfaced it for a query like "stuck operation". Closed in three layers — the generator drops mutating RPC names whatever their binding, the shipped index has the entry removed (937 → 936 endpoints, all 68 services intact), and the reader refuses state-changing action suffixes. Read-shaped actions (:serialPortOutput, :getTargetStates, :byValue) still pass.
Not in this PR: reading audit events. The audittrails service itself is in the index, so trails are listable, but the events land in a sink rather than an API, so surfacing them needs its own design discussion.
Demo/Screenshot for feature changes and bug fixes -
pr2demopro.mp4
Explain your implementation approach:
The problem is coverage against tool budget. Yandex Cloud exposes more than 900 read endpoints across nearly 70 services, and only 32 tool schemas reach the model per turn, shared with every other configured integration. A tool per service is therefore impossible, and picking a dozen services by hand means the agent reports "cannot read that" for everything else.
So the split is deliberate: a small set of curated tools where the shape of the answer matters - a metric series has to be summarised, an instance list has to say which ones are stopped, a balancer has to name the unhealthy targets - and one generic reader for the long tail, where returning the raw resource is the right answer.
On the optional dependency: vendoring the generated stubs or hand-writing a protobuf client was possible but means carrying generated code that Yandex already publishes. Making it a hard dependency would put grpcio in every install for a read most users never make. The extra keeps both out of the way.
Alternatives I considered. Generating the endpoint list at build time was rejected because it adds a network dependency on cloudapi to CI; the file is 230 KB, comparable to snapshots already in the tree. Trimming the index to only the services with curated tools defeats its purpose, which is precisely the long tail. Enforcing read-only with a regex on operation names, as the AWS integration does with boto3, was not available here because there is no shipped catalogue to match against - extracting
get:bindings from the protos gives the same guarantee from the authoritative source.Key components:
api_index.pyloads and searches the index, ranking collection endpoints above single-resource reads because an agent without an id yet needs the list;build_api_index.pyregenerates it, including an alias table for the six services where the registry hyphenates and the protos do not, which had silently dropped 34 endpoints; each family's tools convert the raw payload into the narrowing answer described above.Edge cases covered by tests: a path containing
..or a scheme is rejected, an unknown service is refused before any request, folder scope is applied only to endpoints that accept it, the folder is discovered from instance metadata when it is not configured, an unreadable serial console does not fail the diagnostics call, and an unknown aggregation is rejected without a call going out.Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Checklist before requesting a review