diff --git a/charts/runtime-agent/templates/controller-rbac.yaml b/charts/runtime-agent/templates/controller-rbac.yaml index 7ea345b..37235b2 100644 --- a/charts/runtime-agent/templates/controller-rbac.yaml +++ b/charts/runtime-agent/templates/controller-rbac.yaml @@ -52,6 +52,9 @@ rules: - apiGroups: [""] resources: [limitranges, resourcequotas, persistentvolumeclaims] verbs: [get, list, watch] + - apiGroups: [""] + resources: [services] + verbs: [get, list, watch] - apiGroups: [policy] resources: [poddisruptionbudgets] verbs: [get, list, watch] diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 1096de1..5f43cdc 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -363,7 +363,7 @@ func run(ctx context.Context, logger *slog.Logger, clientset kubernetes.Interfac // One capture instant for both payloads of a flush: they describe the // same cluster state and are joined against each other downstream. capturedAt := time.Now() - records := metadata.Aggregate(podWatcher.Pods()) + records := metadata.Aggregate(podWatcher.Pods(), podWatcher.Rollouts()) if err := spool.WriteWorkloadMetadata(capturedAt, records); err != nil { logger.Error("spooling workload metadata", "error", err) } diff --git a/docs/adr/0017-build-facts-keyed-by-digest.md b/docs/adr/0017-build-facts-keyed-by-digest.md index c7faced..0a4a32c 100644 --- a/docs/adr/0017-build-facts-keyed-by-digest.md +++ b/docs/adr/0017-build-facts-keyed-by-digest.md @@ -3,7 +3,7 @@ Date: 2026-08-21 Status: Accepted Amends: 0012 §1, §3 -Amended by: 0018, 0019, 0027 +Amended by: 0018, 0019, 0027, 0048 Amends [ADR 0012](0012-payload-registry-and-provenance.md): §1's registry gains a kind, and §3's "it is true as of the flush that wrote it" becomes a field diff --git a/docs/adr/0032-cluster-policy.md b/docs/adr/0032-cluster-policy.md index e47fef7..1a2152a 100644 --- a/docs/adr/0032-cluster-policy.md +++ b/docs/adr/0032-cluster-policy.md @@ -4,7 +4,7 @@ Date: 2026-08-24 Status: Accepted Amends: 0031 §7 -Amended by: 0033 +Amended by: 0033, 0048 Adds the payload kinds `workload_policy` and `cluster_policy`. Adds six resources to the ClusterRole — the first time the agent's read access has widened since it diff --git a/docs/adr/0046-the-cache-is-the-source.md b/docs/adr/0046-the-cache-is-the-source.md index 07b1ff8..d426804 100644 --- a/docs/adr/0046-the-cache-is-the-source.md +++ b/docs/adr/0046-the-cache-is-the-source.md @@ -6,7 +6,7 @@ Status: Accepted Amends: 0031 §7 -Amended by: 0047 +Amended by: 0047, 0048 Moves the drop of `env`, `args` and `command` from the payload seam to the informer's transform, so the values are never held rather than never shipped. diff --git a/docs/adr/0048-findings-name-the-fields-they-need.md b/docs/adr/0048-findings-name-the-fields-they-need.md new file mode 100644 index 0000000..eebb6c8 --- /dev/null +++ b/docs/adr/0048-findings-name-the-fields-they-need.md @@ -0,0 +1,175 @@ +# 0048. Four first-hour findings name fields the agent did not collect + +Date: 2026-08-28 + +Status: Accepted + +Amends: 0017 §2, 0032 §1–2, 0046 §2 + +Adds probe schedules and the workload's update strategy to `workload_metadata`, +Services to `workload_policy`, and a version to each module in `go_build`. Adds +one resource to the ClusterRole (`services`). Extends the informer transform of +ADR 0046 to the probe handlers, which were being cached whole. + +## Context + +Every decision here so far has answered the same question one fact at a time: +*is this fact worth collecting?* That question has no stopping rule. Asked the +other way — *what sentence do we want to be able to say, and what does it need?* +— it terminates, and it produces a different list. + +Five findings are meant to be true from a single read of a cluster, with no +window closed and no profile captured: what a node drain takes down, what is +already burning, what is throttled against its own limit, one number for the +whole cluster, and Go hygiene. Checking each against what the agent ships found +four fields missing, and none of them was missing by decision: + +- **Nothing says a liveness probe exists.** A liveness probe is the only part of + a spec that restarts a *healthy* container on a schedule, and the two failure + modes — one more aggressive than the readiness probe beside it, or identical to + it and therefore not an independent check — are visible in the numbers alone. +- **Nothing says how a workload replaces its own replicas.** `workload_policy` + reports the budget that bounds an eviction; no eviction is involved when a + Deployment rolls itself, and `maxUnavailable: 100%` with `minReadySeconds: 0` + takes a workload down without a budget being consulted. +- **Nothing says traffic is routed to a workload.** One replica is a batch size + or an outage depending on whether anything sends it requests, and the agent + reads only the pod's declared `containerPorts` — an announcement, not a route. +- **`go_build` carries module paths without versions**, by ADR 0017 §2. So "one + library at four versions across the fleet" cannot be said, and neither can any + finding that compares a dependency version against another. + +Reading the probe fields turned up something the third item did not predict. +ADR 0046 removes `env`, `envFrom`, `args` and `command` at the transform, on the +grounds that a container's free text must not be *held*, not merely not shipped. +A probe's `exec` handler is a command, and its `httpGet` handler carries +`httpHeaders` — where an `Authorization` value is written by hand. Both have been +sitting in the controller's cache since the first informer, and so have the +`preStop` and `postStart` hooks, which nothing reads at all. + +## Decision + +**1. Probe schedules are collected; probe handlers are emptied at the +transform.** `workload_metadata` records carry, per container, the liveness, +readiness and startup probes as `initialDelaySeconds`, `periodSeconds`, +`timeoutSeconds`, `failureThreshold`, `successThreshold` and the handler's +*kind* — `exec`, `httpGet`, `tcpSocket`, `grpc`. Everything the handler names is +cleared in `dropUncollectedFields` before the object enters the cache: the exec +command, the HTTP path, host and headers, the TCP host, the gRPC service. The +lifecycle hooks are removed whole. + +The kind is kept and the target is not, because the finding asks whether two +probes are the same check, not what either calls. The numbers are the API +server's defaulted ones, which are the numbers the kubelet will use. + +**2. The workload's update strategy ships in a `workload` scope block.** Type, +`maxUnavailable`, `maxSurge`, a StatefulSet's `partition`, and +`minReadySeconds`, read from the Deployment, StatefulSet or DaemonSet the pods +belong to. + +Absence has to answer two different questions, and one flag separates them. A +Job, a CronJob, a bare ReplicaSet and an owner-less pod replace no replicas, so +they carry no rollout and that absence is the fact. A pod managed by a custom +resource — an Argo Rollout, a Knative Revision, an in-house operator — is the +opposite case: it almost certainly *does* roll, and the agent holds no RBAC to +read how (`docs/security.md` §11). Reporting nothing there would state "this +does not roll", which is worse than silence. Those records carry +`rollout: {unread: true}` and nothing else — ADR 0013's rule that the agent +reports what it observed rather than what it concluded, applied to a structural +payload. This is the case a first-hour report must not get wrong quietly: Argo +Rollouts is common, and the finding is about exactly the workloads it manages. + +It is a fact of the workload, and the record's key is narrower than that, so it +is nested and not flattened (ADR 0014) — a `workload` object beside the existing +`pod` one. `maxUnavailable` and `maxSurge` are kept as written, because a count +and a percentage are not interchangeable when the replica count moves; that is +the treatment a budget's own declaration already gets. + +It is read when the payload is built and not when a pod was described. Editing +`maxUnavailable` rolls nothing, so a value captured at describe time would stay +wrong until something unrelated happened to the pods. + +**3. `go_build` modules carry versions, which makes it a dependency inventory.** +This reverses ADR 0017 §2 and is worth stating as the reversal it is. That ADR +declined versions on the grounds that a path with a version is a +vulnerability-scanning feed and that adopting one is "a new decision, not a field +added quietly". This is that decision, made for one reason: the fleet-wide +findings that motivate the whole profiling arc — one internal module hot in many +services, a dependency's cost measured version against version — are arithmetic +over versions and cannot be approximated without them. + +What is given up is real. `security.md` said `go_build` "carries module paths +only, so it is not and cannot be used as a vulnerability-scanning feed", and that +sentence is now false. The payload is a software bill of materials for every Go +build in the cluster, from which anyone holding it can derive known +vulnerabilities. Two things bound it rather than excuse it: the payload is +already scoped to admitted workloads and already carries the main module, the Go +version and the build's VCS revision, so this widens a disclosure rather than +opening one; and the agent still ships no vulnerability finding of its own — +reachability is the gate here (ADR 0038), and a module graph is not one. + +A dependency redirected by a `replace` directive is reported under the path and +version the build *required*, flagged `replaced`. The replacement's own path is +not read: a local replacement's path is a directory on the build machine, which +is the class of string the build-settings allow-list exists to keep out +(ADR 0019). + +**4. Services are added to `workload_policy`, and `services` to the +ClusterRole.** Name, type, headless, and the internal and external traffic +policies. The selector is resolved through admitted pods, exactly as a budget's +is, so a Service pointing only at excluded pods attaches to nothing and is never +named. A Service with no selector — an ExternalName, or one whose EndpointSlices +are written by hand — attaches to nothing either. + +The cache is a degrading source (ADR 0033): withholding the grant costs the +`services` list and names itself in `unavailable_sources`. + +Addresses are not read. A ClusterIP is cluster-internal and a load balancer's is +the customer's public address, and no finding here needs either; `clusterIP` is +read only for the one comparison against `None`. Ports are not read: the +container's own declared ports already ship, and the mapping between them is a +question no first-hour finding asks. `trafficDistribution` and EndpointSlices are +not read — they answer a zone-routing question that belongs to a later finding, +and collecting ahead of one is the habit this ADR exists to break. + +## Consequences + +**Easier.** Each of the five first-hour findings can now name a payload field for +every clause it states. The probe change also closes a hole ADR 0046 was written +to close and missed: a heap dump of the controller no longer contains a +customer's probe commands or the bearer token in a probe header. + +**Harder / given up.** The `go_build` promise is weaker and the document has to +say so plainly, which §3 does. The payload also roughly doubles: ADR 0017 +measured 200–1500 modules for a typical service, and a version string is about +as long as the path it follows. `go_build` is write-once per image digest, so +this is one larger file per distinct build rather than a per-flush cost, and the +spool's bound (ADR 0042) is what it lands against. + +`security.md`'s file ceiling moves 920 → 930 (ADR 0044). It is paid for by three +things the document did not owe before — a grant, a class of collected field, and +the disclosure in §10.4 — and the section ceiling does not move: §8 came out of +this change four lines smaller than it went in, because the room was found by +deleting ADR rationale that had drifted back into it. + +The ClusterRole gains a resource, so the "first widening" of ADR 0032 is now the +second. `services` is one of the least sensitive objects in the API — it holds no +customer data beyond names and routing — but the grant is cluster-wide in a +cluster-wide installation, like every other row. + +No CRD is read, and the closed ClusterRole that §9 of `docs/security.md` leans +on gains no vendor resource. Reading Argo Rollouts properly is a larger decision +than a field — a first custom-resource grant, an informer for a kind that may not +exist in the cluster, and a strategy whose canary and blue-green shapes are not +this struct — so it is left to its own ADR. What this one owes is that the gap is +legible from the payload rather than inferred from a list of kinds the backend +would have to keep in step with the agent. + +**Not changed.** No new payload kind, no registry row, no change to any natural +key, no verb that writes, no new privilege on the node, and nothing new is read +from a pod's `status`. Probe handlers, lifecycle hooks, `env`, `envFrom`, `args` +and `command` all remain uncollected — the list of what the transform removes got +longer, not shorter. + +This ADR records decisions implemented in the same pull request, per the process +in [`README.md`](README.md). diff --git a/docs/adr/README.md b/docs/adr/README.md index 89613e0..8587557 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -105,3 +105,4 @@ line to paste. - [0045. A kubelet read is bounded in time and in size, and one node's silence costs one node](0045-a-kubelet-read-is-bounded.md) - [0046. The informer cache is the source, so the fields the agent promises not to collect are removed there](0046-the-cache-is-the-source.md) - [0047. Five environment variables are named, kept and shipped; every other one is still dropped](0047-runtime-knobs-are-named-and-kept.md) +- [0048. Four first-hour findings name fields the agent did not collect](0048-findings-name-the-fields-they-need.md) diff --git a/docs/backend-requirements.md b/docs/backend-requirements.md index 74a75f3..91750e8 100644 --- a/docs/backend-requirements.md +++ b/docs/backend-requirements.md @@ -276,6 +276,15 @@ was taken away, which is the normal state. `EvictionByEvictionAPI`, `Evicted`) plus `other`. The backend MUST tolerate `other` and MUST NOT assume the set never grows. +**`workload_metadata.workload` and `.pod` are two different scopes, and neither +is per record** (ADR 0014). A pod's containers each produce a record repeating +both blocks, so **the backend MUST NOT sum either across one workload's +records**. `rollout` is absent for a kind that replaces no replicas — a Job, a +CronJob, a bare ReplicaSet, an owner-less pod — and MUST NOT be rendered as one +that replaces everything at once. A `rollout` carrying only `unread: true` is a +workload managed by a custom resource the agent holds no RBAC for, such as an +Argo Rollout: **unobserved, not absent**, and MUST NOT be read as no strategy. + **`workload_metadata.pod.unscheduled` explains the replica shortfall, it does not restate it.** The gap between `replicas` and the sum of `nodes` has always shown how many replicas are unplaced; `unscheduled` counts those by the @@ -324,9 +333,11 @@ immutable given its key and the agent sends it once per distinct build. The backend MUST upsert it by image digest and MUST NOT expect it on every flush or treat a redelivery (after an agent restart) as a change. It carries no `captured_at` for the same reason. Join it to workloads through -`go_inventory.records[].image_digest`. It contains module *paths* only, never -versions — it is not a dependency-version inventory and MUST NOT be presented as -one. +`go_inventory.records[].image_digest`. Each entry of `modules` is an object with +`path` and the `version` the toolchain recorded; one flagged `replaced` was +redirected by a `replace` directive, so its version names what the build +*required* rather than what is linked, and MUST NOT be presented as the running +version ([ADR 0048](adr/0048-findings-name-the-fields-they-need.md)). **`go_build.settings` is a bounded allow-list, and its keys are optional** ([ADR 0019](adr/0019-build-settings-by-allow-list.md)). The agent keeps only the diff --git a/docs/prose_test.go b/docs/prose_test.go index d2e6550..89f801e 100644 --- a/docs/prose_test.go +++ b/docs/prose_test.go @@ -26,13 +26,14 @@ const maxCommentRun = 8 // back at ADR 0024's 750: five kinds have been added since, each a table row and // a paragraph. `backend-requirements.md` restates decisions on purpose — its // reader implements ingest and reads neither our ADRs nor `security.md` — so -// what is bounded there is growth, not duplication. +// what is bounded there is growth, not duplication. The file ceiling moved +// 920 → 930 for obligations the document did not have before (ADR 0048). var ceilings = []struct { file string lines int section int }{ - {"security.md", 920, 290}, + {"security.md", 930, 290}, {"backend-requirements.md", 460, 320}, } diff --git a/docs/security.md b/docs/security.md index 11d8b89..add4cf2 100644 --- a/docs/security.md +++ b/docs/security.md @@ -140,6 +140,7 @@ bound to nothing and its token is not mounted — so it appears in no row here. | `replicasets`, `deployments`, `statefulsets`, `daemonsets`, `jobs`, `cronjobs` | get/list/watch | Resolve the `ownerReferences` chain (Pod → ReplicaSet → Deployment) so findings are aggregated per workload, not per pod; read each workload's own annotations to honor an opt-out written there ([ADR 0028](adr/0028-workload-level-opt-out.md)); and read finished Jobs' timings and outcomes for `job_runs` ([ADR 0029](adr/0029-job-runs.md)) | | `nodes` | get/list/watch | `allocatable`/`capacity` for node idle computation; labels `node.kubernetes.io/instance-type` and capacity-type (spot/on-demand) for the cost model; `status.nodeInfo.kernelVersion` to report whether nodes meet the kernel floor for the eBPF profile (`CAP_BPF` requires kernel 5.8+, see [§7](#7-node-privileges)) | | `namespaces` | get/list/watch | Evaluate namespace allow/deny filters and the opt-out annotation | +| `services` | get/list/watch | Report that traffic is routed to a workload at all ([ADR 0048](adr/0048-findings-name-the-fields-they-need.md)): one replica is a batch size or an outage depending on whether anything sends it requests, and a declared `containerPort` is an announcement rather than a route. The selector is resolved against pods that passed your filters and never reported as such. Addresses are not read — a ClusterIP is compared against `None` to tell a headless Service apart, and nothing else | | `poddisruptionbudgets`, `horizontalpodautoscalers` | get/list/watch | Report what bounds a workload from outside its own spec ([ADR 0032](adr/0032-cluster-policy.md)): a budget can forbid eviction outright, so a node covered by one cannot be drained at all; an autoscaler targeting CPU *utilization* targets a percentage of the request, so the request cannot be read safely without it. A budget's selector is resolved against pods that passed your filters and never reported as such | | `limitranges`, `resourcequotas` | get/list/watch | Distinguish a request a team chose from one the namespace supplied on their behalf, and report the ceiling a namespace sits under and how much of it is spent (ADR 0032). Read only for namespaces that pass your filters | | `persistentvolumeclaims`, `storageclasses` | get/list/watch | A bound claim on a zonal volume pins its pod to that zone, which no field of the pod spec states (ADR 0032). A StorageClass's `parameters` are **not** read — that is the one field here carrying provider configuration (endpoints, key identifiers); what is read is how the class behaves: provisioner, reclaim policy, `volumeBindingMode`, expansion | @@ -156,8 +157,8 @@ does run as root is the node role, in [§7.1](#71-the-go-binary-scanner-the-current-node-role). **Declining a policy grant degrades one payload, not the agent.** The caches -behind `poddisruptionbudgets`, `horizontalpodautoscalers`, `limitranges`, -`resourcequotas`, `persistentvolumeclaims`, `priorityclasses` and +behind `services`, `poddisruptionbudgets`, `horizontalpodautoscalers`, +`limitranges`, `resourcequotas`, `persistentvolumeclaims`, `priorityclasses` and `storageclasses` gate nothing else, so withholding one costs the corresponding facts and nothing more. The payload then names the source it could not read in `unavailable_sources`, so the absence is legible rather than indistinguishable @@ -470,16 +471,14 @@ classes with different sensitivity and different default policies: | Data class | Examples | Sensitivity | Default policy | |---|---|---|---| | Resource metrics | CPU usage, working set, throttling ratios, requests/limits, node allocatable | Low (numbers) | Collected for everything visible, minus the infrastructure deny-list and your filters | -| Workload metadata | Workload names, namespaces, image digests, Go version, module path, node zone and instance type | Medium | Same as metrics | +| Workload metadata | Workload names, namespaces, image digests, Go version, module paths and versions, node zone and instance type | Medium | Same as metrics | | Object history (journal) | Pod names, container names, restart counts, termination reasons and exit codes | Medium | Same as metrics. This is the one class that names individual **pods** — see below | | Profiles (stack traces) | Function names, call graphs | **High** — function names reveal the structure of your code | **Explicit allow-list only.** A stack trace never leaves the cluster unless its module path is on the allow-list you configured | ### Every payload the agent produces -This is the complete list — there is no other channel and no other shape. It is -generated from the same registry the code writes payloads through -(`internal/sink/registry.go`), and a test fails if a kind ships without -appearing here (ADR 0022). +This is the complete list — there is no other channel and no other shape, and a +test fails if a kind ships without appearing here (ADR 0022). | Payload | What it carries | Names pods? | |---|---|---| @@ -491,12 +490,12 @@ appearing here (ADR 0022). | `pod_disruptions` | Per hour: the pods the *cluster* removed — preempted, evicted under node pressure, drained, or removed via the eviction API — with the node and the instant | **yes** | | `job_runs` | Per hour: each Job that finished — when it started and finished, whether it succeeded, the failure reason, its pod success/failure counts, and its declared `parallelism`/`completions`/`backoffLimit` | **yes** | | `deployment_revisions` | Per flush: each ReplicaSet of a collected Deployment — its revision number, when it was created, its desired/current/ready replica counts, and each container's image reference | **yes** | -| `workload_metadata` | Declared shape per (namespace, workload, container, image digest): image, requests, limits, ports, the five runtime knobs of the field-minimization list below, QoS, replica counts by phase, by node, and by unscheduled reason, and the pod's reduced placement constraints (ADR 0031) | no | -| `workload_policy` | Per collected workload, what bounds it from outside its own spec: disruption budgets covering it, autoscalers driving it, and the volume claims its pods mount (ADR 0032); plus `unavailable_sources`, naming any of those the agent could not read — never granted, or no longer being fed (ADR 0033, ADR 0035) | not directly — but a claim is named, and a StatefulSet's claim is `