feat(fingerprint): cluster identity projection from snapshot measurements#844
Merged
Conversation
Adds pkg/fingerprint, a structured cluster identity (service, accelerator, OS, k8s version, node count) derived from the existing collector outputs and emitted alongside Snapshot.Measurements. Each dimension records its source signal so an evidence bundle can prove the recipe was tested on hardware matching its declared criteria, per ADR-007. - pkg/fingerprint: types, FromMeasurements, Match (per-dimension matched/mismatched/unknown diff), GPU SKU normalization - pkg/snapshotter: populate Snapshot.Fingerprint after collectors finish so missing signals surface as zero-value dimensions - pkg/recipe: Criteria.ToFingerprintInput adapter; switch OS aliases to oskind constants - docs/contributor/data.md: schema, detection sources, match semantics, worked example
Collapses matchString/matchUnknownDim/matchNodes into a single matchDim that takes a fingerprintCaptured flag for the "deliberately not captured" vs "signal absent" distinction. Nodes remap 0 to "" to reuse the same wildcard path. No behavior change; all existing match tests pass unchanged.
Reads topology.kubernetes.io/region from the node-topology label subtype. Multi-region clusters surface as empty rather than picking one arbitrarily.
…rder match diffs Lift snapshot→criteria responsibility from pkg/recipe into the fingerprint package via Fingerprint.ToCriteria, dropping the CriteriaInput indirection that previously bridged the import cycle. Match now takes *recipe.Criteria directly. Add GPUNodeCount sourced from nvidia.com/gpu.product topology labels and reconcile per-node smi accelerator against cluster-wide labels — heterogeneous clusters surface as multi-gpu via a new Dimension.Note field rather than misreporting whichever SKU the snapshotter landed on. Same Note path records multi-region when region labels disagree. Change MatchResult.PerDimension from map to ordered slice with a typed DimensionName enum and MatchResult.Find lookup so serialization is byte-stable. Drop OS Version when the ID is unrecognized, and flag Snapshot.Fingerprint as advisory — trust-bearing consumers must recompute from Measurements.
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Reframe the fingerprint description in data.md and the package godoc as the cluster-identity dimensions used to bind a snapshot to a recipe (not just a mirror of recipe criteria), and list the full current set: service, accelerator, OS, K8s version, region, total node count, and GPU node count.
NodeCount.Value == 0 is the "not captured" sentinel (e.g. when the topology summary subtype is missing). The match diff was rendering it as a literal "0" in FingerprintProvides while the recipe side was already wildcarded to "" — the asymmetry made the diff misleading. Remap both sides to "" so isAny in matchDim treats them as wildcards consistently.
Exercise the topology-label backfill path used when smi did not run (agent landed on a non-GPU node) but the GPU operator labels nodes: an unrecognized product string must yield empty rather than fabricating a SKU, matching the smi-side ParseGPUSKU behavior.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Add a callout warning that the fingerprint embedded in the snapshot YAML is a human-readability convenience, not a trust-bearing claim — the snapshot file is not signed at this layer, so trust-bearing consumers (evidence bundler NVIDIA#754, verifier NVIDIA#753, downstream policy gates) must recompute via fingerprint.FromMeasurements before acting on it.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Add a subsection explaining the multi-gpu / multi-region notes the fingerprint emits when nodes disagree on accelerator or region, and update the schema paragraph to cover the optional note field alongside value and source. Distinguishes "nodes disagreed" from "signal not captured" so readers know how the verifier will render each case.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
mchmarny
reviewed
May 11, 2026
mchmarny
left a comment
mchmarny
left a comment
Member
There was a problem hiding this comment.
Clean implementation — the Fingerprint type with three-way DimensionMatch semantics and the Note-for-heterogeneity pattern are exactly the right shapes for an audit-trail data type. The advisory godoc on Snapshot.Fingerprint and the cycle inversion (consolidating two snapshot→criteria projections through one path) are both solid moves.
One medium nit: populateFromGPU and the topology-backfill path in reconcileAccelerator silently drop unrecognized GPU SKUs with no audit signal — would be more consistent with the multi-region/multi-gpu treatment to surface a note: unknown-sku so registry staleness shows up in the snapshot rather than masquerading as "no GPU." The remaining items are nits (label-value parsing duplicated three times, second linear scan in reconcileAccelerator, criteriaAnyValue const duplicated from pkg/recipe).
The k8s/GPU subtype/key renames in pkg/cli/recipe_test.go are a quiet bug fix — the deleted ExtractCriteriaFromSnapshot was reading keys that don't exist in real collector output. Worth a line in the PR description so reviewers don't read it as a pure refactor.
CI: most checks green, tests / E2E, analyze, and the GPU H100 jobs still in-flight at review time. mergeable_state: behind — needs a rebase before merge.
Not blocking on any of this.
The fingerprint package's matchDim needs the same "any" wildcard literal that pkg/recipe's parsers already use, and was carrying its own duplicate const to avoid an import. Export CriteriaAnyValue with godoc explaining its relationship to the typed *Any enums, and drop the duplicate in pkg/fingerprint/match.go in favor of recipe.CriteriaAnyValue.
…lookup Two small refactors with no behavior change: * Pre-find the NodeTopology measurement once in FromMeasurements and hand it to reconcileAccelerator, replacing reconcile's internal scan-and-nil-check loop. The two passes were redundant because populateFromTopology already iterates the measurement. * Factor the topology collector's "<value>|<node1,node2,...>" label-value parsing into a parseLabelEncoding helper. Three call sites (reconcileAccelerator, countGPUNodes, extractRegion) open-coded the same strings.Index split with slightly different branching; folding them into one function makes the encoding contract obvious in one place.
…istry
When nvidia-smi or the GPU operator's topology label reports a
product string ParseGPUSKU does not recognize, the fingerprint
was silently dropping the Accelerator dimension — indistinguishable
from "no GPU detected" to downstream consumers. A reviewer
flagged this as a likely registry-staleness signal that should
surface, not hide.
Emit a new "unknown-sku" note on Accelerator in both paths:
* populateFromGPU sets it from sourceAcceleratorSMI when smi
reports a product outside the registry.
* reconcileAccelerator sets it from sourceTopologyGPU when the
topology label is present but unrecognized, unless smi
already marked it (avoids overwriting an identical signal
from a less-specific source). When topology resolves to a
known SKU after smi marked unknown-sku, topology wins and
clears the note (covered by the new
SMIUnknownPlusTopologyRecognized test).
The raw model string stays in the underlying GPU measurement for
forensics; only the normalized fingerprint surface gains the
audit hint. docs/contributor/data.md picks up the third note
alongside the existing multi-region / multi-gpu cases.
mchmarny
approved these changes
May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a new
pkg/fingerprintpackage that projects snapshot measurementsinto a typed cluster-identity record (service, accelerator, OS, region,
k8s version, node counts) and offers a three-way per-dimension match
against
recipe.Criteria. Surface it as a top-levelfingerprint:block on
aicr snapshotoutput and unify the existing snapshot →criteria extraction through the same projection layer.
Motivation / Context
ADR-007 (verifiable recipe test evidence) needs the bundler and
verifier to confirm that the cluster a recipe was tested on actually
matched the recipe's declared
criteria— without re-runningvalidation. Today the snapshot YAML carries only raw measurements; a
reviewer can't tell at a glance "is this an H100 EKS cluster" without
hand-parsing every collector's output. This PR is the foundational
piece that #753 (
aicr evidence verify) and #754 (aicr validate --emit-attestation) build on.A second motivation surfaced during implementation:
pkg/recipealready had
ExtractCriteriaFromSnapshotdoing a near-identicalprojection with its own GPU/OS normalization. The two were drifting.
This PR consolidates both paths through
fingerprint.FromMeasurementsFingerprint.ToCriteriaand inverts therecipe → snapshotterimport edge so the cycle the original adapter dodged is no longer
needed.
Fixes: #752
Related: #750, #753, #754, #739, ADR-007 (
docs/design/007-recipe-evidence.md)Type of Change
recipe.ExtractCriteriaFromSnapshotconsolidated intofingerprintComponent(s) Affected
cmd/aicr,pkg/cli) —aicr recipe --snapshotreads from the new pathpkg/recipe) — dropsExtractCriteriaFromSnapshot; cycle inversionpkg/collector,pkg/snapshotter) —Snapshot.Fingerprintfield, populated post-collectionpkg/fingerprintdocs/contributor/data.mdImplementation Notes
Dimensions captured
criteriaparticipant?servicek8s.node.provider(parsedproviderID)acceleratorgpu.smi.gpu.modelreconciled againstnvidia.com/gpu.producttopology labelsos.value+os.version/etc/os-releaseIDandVERSION_IDvalueonly (criteria has no OS version)k8sVersionk8s.server.version(leadingvstripped)regiontopology.kubernetes.io/regionaggregated labelnodeCountnodeTopology.summary.node-count(all nodes)nodes)gpuNodeCountnvidia.com/gpu.productMatch semantics
Fingerprint.Match(*recipe.Criteria)returnsMatchResult{Matched bool, PerDimension []DimensionDiff}with three-way per-dimension outcomes(
matched | mismatched | unknown).unknowncovers two cases: thefingerprint deliberately doesn't capture this dimension (intent,
platform — recipe-author choices, not cluster facts) and the
fingerprint failed to detect it (e.g., no GPU collector output). The
overall
Matchedflag stays true unless a dimension ismismatched;unknowns surface for human review without flipping the overall outcome.
PerDimensionis an ordered slice with typedDimensionNamekeys anda
Find()helper — deterministic iteration and serialization, nomagic-string map keys.
Heterogeneity handling
Two cases where naïve detection would lie:
disambiguated
topology.kubernetes.io/region.<value>keys whennodes have differing region labels.
extractRegionreturns emptywith
Note: "multi-region"rather than picking arbitrarily.smicollector only inspectsthe snapshotter's local node, so an 8-node cluster split between
H100 and L40 would otherwise be claimed as homogeneous in whichever
SKU the agent landed on. A reconciliation pass cross-references the
nvidia.com/gpu.producttopology labels; disagreeing labels →Note: "multi-gpu"with empty Value.Cycle inversion
Before this PR:
pkg/recipeimportedpkg/snapshotterforExtractCriteriaFromSnapshot, forcing the newpkg/fingerprinttouse a flat-string
CriteriaInputadapter to avoid closing a cycle(
pkg/snapshotter → pkg/fingerprint → pkg/recipe → pkg/snapshotter).After this PR:
Fingerprint.ToCriteria()owns the snapshot→criteriaprojection,
Fingerprint.Matchtakes*recipe.Criteriadirectly, theCriteriaInput/ToFingerprintInputadapter pair is removed, andpkg/recipe/snapshot.gois deleted. The import direction is nowpkg/recipe → pkg/fingerprintonly.Quiet correctness fix in
pkg/cli/recipe_test.goThe test-data update in
pkg/cli/recipe_test.gois not a pure rename.The deleted
recipe.ExtractCriteriaFromSnapshotwas readingsubtype="server" key="service"andsubtype="device" key="model",but the k8s collector emits
subtype="node" key="provider"(
pkg/collector/k8s/node.go:56) and the GPU collector emitssubtype="smi" key="gpu.model"(pkg/collector/gpu/gpu.go:220). Theold function was reading keys that don't exist in real collector
output — the tests passed only because the synthetic fixtures had
been fabricated to match the broken reads. Consolidating through
fingerprint.FromMeasurementsquietly fixes that bug; thepkg/cli/recipe_test.godata update brings the fixtures back in linewith what real collectors produce. Treat this PR as a correctness
improvement on top of the refactor, not a pure refactor.
Snapshot.Fingerprintis advisory, not authoritativeA field comment on
Snapshot.Fingerprintexplicitly states it is aconvenience for humans reading the YAML, not a trust-bearing claim.
The snapshot file isn't signed at this layer — an attacker controlling
it could swap the embedded fingerprint without touching the
measurements. ADR-007 consumers (bundler at #754, verifier at #753)
MUST recompute via
fingerprint.FromMeasurements(snap.Measurements).Persona-review pass
After the initial implementation, a five-persona review (security,
downstream-consumer, cluster-operator, recipe-contributor DX, Go API
design) surfaced eight follow-up items, all addressed in commits 3-4:
match:state, not bool collapsedata.mdSnapshot.Fingerprintmarked advisory + recompute expectationPerDimensionordered slice with typedDimensionNamekeysExtractCriteriaFromSnapshotintofingerprint.ToCriteriaDimension.NotefieldGPUNodeCountseparate fromNodeCountOS.Versionon a recognizedOS.ValuePer-signal provenance (
signals[],confidenceper dimension) isADR-007 V2 — additive, no schema break.
Agent-mode parity
Local and agent-mode share the same
measure()function and the sameSerializer.Serializepath; the fingerprint is computed inside theagent pod and emitted via the existing ConfigMap writer. The
tests/chainsaw/snapshot/deploy-agentassertion file doesn't yet pinthe fingerprint block; it could be a small follow-up.
Testing
unset GITLAB_TOKEN make qualifyLocal coverage
pkg/fingerprint: 98.0% (new package)pkg/snapshotter: 51.1% → 51.3% (+0.2%)pkg/recipe: 90.5% → 90.5% (unchanged;snapshot.go's 472 test lines deleted along with the function)pkg/cli: unchanged (test data updated to canonical collector subtype/key shapes)What was exercised locally
go build ./...— cleango test ./pkg/fingerprint/... ./pkg/snapshotter/... ./pkg/recipe/... ./pkg/cli/...— all passgolangci-lint -c .golangci.yaml runon the affected packages — 0 issuestools/check-agents-sync,check-docs-sidebar,check-docs-filenames,check-docs-mdx— all passgo vet— cleanWhat CI will exercise that I couldn't locally
make testwith-race(sandbox lacks cgo/gcc; non-race pass clean)make e2e,make scan(grype), lychee link checktests/chainsaw/snapshot/deploy-agentagainst a real Kind clusterRisk Assessment
Rollout notes:
new snapshots silently ignore the unknown
fingerprint:field(Go
yaml.v3default behavior).emits snapshots without the fingerprint block; new controllers
recompute via
fingerprint.FromMeasurementsrather than trust theembedded value.
recipe.ExtractCriteriaFromSnapshotremoved — internal-onlyfunction; the one external caller in
pkg/cli/query.gois migratedin the same commit.
pkg/recipe.Criteria.ToFingerprintInput()andpkg/fingerprint.CriteriaInputremoved — internal-only adaptertypes added then removed in the same PR; no external surface change.
Checklist
make testwith-race) — non-race pass locally;-racedeferred to CI (no gcc in sandbox)make lint)git commit -S)