Add cisco:ctr:device:past_ids to the identity-assertion vocabulary (XDR-48383) - #485
Add cisco:ctr:device:past_ids to the identity-assertion vocabulary (XDR-48383)#485yogsototh wants to merge 3 commits into
Conversation
…DR-48383)
Device Insights merges duplicate posture endpoints into a single device and
previously lost the merged-away PostureEndpointID, so a query by the old id
could not resolve the device (XDR-48383). Providers can now record the ids a
device was previously known by.
An Assertion :value is a single string, so a multi-valued assertion is
expressed by repeating the entry once per value. That rule was previously
undocumented for the nine existing plural entries (cisco:ctr:user:emails,
cisco:ctr:ad:user_resolved_identities, ...); it is now stated in the
AssertionType :description so it reaches the generated documentation.
Note that AssertionType is an open vocabulary (:open? true), which compiles to
plain s/Str -- an unlisted assertion name already validated. This addition
therefore changes no validation behavior; it publishes the field so consumers
can discover it.
- doc/structures/{bundle,casebook}.md regenerated with `lein doc`
- Requested by Zachary Terlizzese for Device ID Tracking in Assets
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@zterlizz this is the Two points that may affect how your side uses it: The schema cannot hold an array. :assertions [{:name "cisco:ctr:device:past_ids" :value "00-00-00-01"}
{:name "cisco:ctr:device:past_ids" :value "00-00-00-02"}]This matches the nine plural entries already in the vocabulary ( One consumer-side caveat worth flagging: if anything downstream collapses assertions with You are likely not blocked today. On the push-access point -- you mentioned you could not push to CODEOWNERS review still applies here, so this needs a maintainer approval before it can merge. |
zterlizz
left a comment
There was a problem hiding this comment.
This is exactly what I was hoping for. Thank you, Yann!
msprunck
left a comment
There was a problem hiding this comment.
The vocabulary entry itself is correct and uncontroversial. One blocking issue: the multiplicity convention documented alongside it describes a shape the producer does not emit. Details inline.
I verified the producer implementation and its integration tests directly; since that source is not public I have kept the specifics out of this review and posted them to XDR-48383 instead. Happy to walk through them internally.
Where the convention lives
Not inlineable (outside the diff), but worth fixing in the same pass: the multiplicity rule is a property of value and of the list, yet it is attached to AssertionType, i.e. the type of the sibling name key. It renders under ## Property name (bundle.md:11120), above the 52-item allowed-values list. A consumer reading ## Property assertions (bundle.md:10913, still "Any known context about the identity attributes.") or the Assertion object table (bundle.md:11108-11110, both Description cells empty) is told nothing. Consider moving it to the :assertions :description at identity_assertion.cljc:97-98, or adding one to :value at L82.
Corrections for the PR description
- "nine plural entries" is 11 — missing
cisco:ctr:user:phone_numbersandcisco:ctr:common:ir_attributes. The undercount favours your argument. - C6's conclusion is right, the reason isn't.
doc/json's example value is not vocabulary-independent: flanders'enumsets:default (-> values sort first)and->jsondoes not null defaults.doc/jsonescaped churn only becausecisco:ctr:ad:host_domain_namestill sorts first. A future addition sorting earlier (cisco:ctr:aa:*) will rewritedoc/json/bundle.jsonandcasebook.json. Worth stating so the next contributor regenerates. - Your dismissal of alternative (c) is stronger than you argued: flanders map types compile to closed maps, so an optional
:valuesentry is rejected asdisallowed-keyby any consumer validating against an older CTIM jar. Forward-incompatible, not merely a contract change. - C4 reports a command CI doesn't run.
.github/workflows/build.yml:57gates onlein do clean, compile :all, doo node node once— there is nolein teststep. That gate also passes: 36 tests, 147 assertions.
Verified
Independently reproduced: C1 (:open? true makes flanders emit bare s/Str, :values discarded; {:name "totally:made:up"} validated before this change and still does — no validation change in either direction), C2's mechanics, C3, C5 (regenerated docs byte-identical: 678209/678209 chars bundle.md, 696079/696079 casebook.md), C6's conclusion, C7, and C4's 139 tests / 510 assertions. bundle.md and casebook.md are the complete set of generated artifacts carrying this vocabulary; both are updated and nothing is stale.
Non-blocking follow-ups
doc/README.md:45,72link tostructures/identity_assertion.mdandjson/identity_assertion.json, which have never been generated —src/ctim/document.cljhas no IdentityAssertion row. You declared this out of scope and it is genuinely separable, but it means this field's only published home is inside two ~680KB files. A linked issue would beat an unlinked note.- Nothing ties example assertion names to the
assertionset — a singularpast_idtypo would pass all 139 JVM tests, all 36 cljs tests andlein docsilently. - A number of assertion names that appear in practice are absent from this set, and the plural entries are not represented uniformly across every XDR surface. Combined with the encoding issue below, "published contract" is currently aspirational — further argument for describing observed behaviour rather than prescribing new behaviour here.
- Pre-existing:
lein releaseis already broken at HEAD.resources/ctim/version.txtis1.3.30-SNAPSHOT, committeddoc/jsonsays1.3.30, and:release-tasksrunsdocthenvcs assert-committed, so the 21-file churn will trip it regardless of this PR.
Address msprunck's review of PR #485. The prior description prescribed a repeat-per-entry convention (one Assertion per value) that no producer emits. The Device Insights producer emits a SINGLE entry whose value is a JSON-encoded array of strings, for every plural-named entry (cisco:ctr:user:emails, :groups, :phone_numbers, cisco:ctr:device:past_ids, etc.). The acceptance test JSON-decodes that value. - identity_assertion.cljc: rewrite the AssertionType :description to document the JSON-encoded-array-in-a-single-string shape the producer actually emits; name AssetProperty (which carries this vocabulary in practice) alongside IdentityAssertion; drop the public-repo-inappropriate "posture endpoint" jargon in favor of "after duplicate device records were merged into one". - examples/identity_assertions.cljc: maximal example now shows the current-id/past-ids pairing the ticket is about -- a cisco:ctr:device:id entry plus a single past_ids entry carrying a JSON-encoded array (["00-00-00-08","00-00-00-09"]); past ids no longer collide with the current id shared across the two published fixtures. - doc/structures/bundle.md, casebook.md: regenerated with lein doc. Tests: 139 JVM tests / 510 assertions and 36 cljs tests / 147 assertions, 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addressing @msprunck's reviewCommit 775b6af addresses all findings. Thanks for verifying the producer directly -- the central finding was correct and changed the design of this PR.
PR description corrected too: the multiplicity section now documents the JSON-encoded-array shape (not repeat-per-entry); "nine plural entries" corrected to 11 (added Verification: 139 JVM tests / 510 assertions and 36 ClojureScript tests / 147 assertions, 0 failures. |
msprunck
left a comment
There was a problem hiding this comment.
Both blocking items from my previous review are fixed and verified on 775b6af. Approving.
Verified
identity_assertion.cljc:70-78— description now documents the encoding the producer actually emits, drops the producer-specific jargon, spells the field`value`, and names AssetProperty alongside IdentityAssertion.identity_assertions.cljc:28-29— fixture is now a single entry with a JSON-encoded array, plus acisco:ctr:device:identry, so the example demonstrates the current-id / past-ids pairing and no longer collides withidentity-assertion-minimal.- Docs regenerated:
bundle.md:11120,casebook.md:5393. - PR description updated on all four corrections (11 plural entries, the forward-incompatibility of widening
:value/ adding:values, the rewritten rejected-alternatives list, and the encoding rationale).
Re-ran the checks myself: lein with-profile -user test → 139 tests, 510 assertions, 0 failures, 0 errors. The command CI actually gates on, lein do clean, compile :all, doo node node once → 36 tests, 147 assertions, 0 failures, 0 errors. lein doc leaves doc/structures/ byte-identical to the committed files, with only the pre-existing 21-file schema_version SNAPSHOT churn appearing — i.e. nothing is missing from this PR.
Retracting one of my earlier comments
My inline comment on the :description asking you to scope the rule per-name, and warning that these names are not represented uniformly across every XDR surface, is withdrawn. Confirmed internally that Device Insights is the only producer and applies this encoding uniformly, so the universal wording is sound as written — no change needed there. Sorry for the detour.
Non-blocking leftovers, none of which should hold this up
- The multiplicity rule still lives on
AssertionType, i.e. the type of the siblingnamekey.:assertions:description(L100-101) still reads only "Any known context about the identity attributes." and:value(L82) has none, so a consumer who navigates to either surface does not see the convention. Worth a follow-up, not a blocker. asset_properties.cljcis unchanged. The new text names AssetProperty, but an AssetProperties reader gets no cross-reference back to this vocabulary beyond the existing#L11permalink.- Still outstanding from before:
doc/README.md:45,72dead links (no IdentityAssertion row insrc/ctim/document.clj); nothing ties example assertion names to theassertionset, so a singularpast_idtypo would pass every gate silently; a number of names emitted in practice are absent from the set; andlein releaseis already broken at HEAD (resources/ctim/version.txtis1.3.30-SNAPSHOT, committeddoc/jsonsays1.3.30, and:release-tasksrunsdocthenvcs assert-committed).
One observation for a separate cleanup: if there is a single producer, the four cisco:ctr:ad:*_resolved_* entries and cisco:ctr:common:ir_attributes appear to be dead vocabulary that nothing emits.
|
Thanks @msprunck for re-verifying on I've resolved the six inline threads now that each is answered. The non-blocking leftovers are noted as out-of-scope follow-ups for this single-entry vocabulary addition and can be tracked separately:
|
The multiplicity convention (plural-named entries carry a JSON-encoded
array of strings in the single `value` field) was documented only on the
`AssertionType` enum, i.e. the type of the sibling `:name` key. A consumer
reading the `:value` field saw no description at all.
Add a short cross-reference on the `Assertion` `:value` entry pointing to
AssertionType. Doc-only: no schema-shape or validation change (`:value`
stays `f/any-str`). Regenerated doc/structures/{bundle,casebook}.md; the
pre-existing SNAPSHOT schema_version churn in doc/json is excluded, as in
the rest of this PR.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @msprunck -- appreciate the re-verification and the retraction on the per-name scoping (agreed: single producer, uniform encoding, so the universal wording holds). Disposition of the non-blocking leftovers: Addressed here (
I left Deferred -- out of scope for this PR (pre-existing, not introduced here)
Happy to file follow-up tickets for any of the deferred items if you'd like them tracked. |
JIRA https://cisco-sbg.atlassian.net/browse/XDR-48383
Summary
Adds
cisco:ctr:device:past_idsto the IdentityAssertion open vocabulary, soproviders can record the device ids a device was previously known by.
Device Insights merges duplicate device records into a single device. Before
XDR-48383 the merged-away id was lost, so "query asset by id" against the old id
failed even though the device still existed. This field publishes the past ids
for that lookup.
Requested by @zterlizz for Device ID Tracking in Assets.
Changes
src/ctim/schemas/identity_assertion.cljc-- addscisco:ctr:device:past_idsto the
assertionset; rewrites theAssertionType:descriptionto documentthe multi-value encoding the producer actually emits (see below).
src/ctim/examples/identity_assertions.cljc--identity-assertion-maximalgains a
cisco:ctr:device:identry plus acisco:ctr:device:past_idsentrycarrying a JSON-encoded array, demonstrating the current-id / past-ids pairing.
doc/structures/bundle.md,doc/structures/casebook.md-- regenerated withlein doc.Two things worth a reviewer's attention
1. This changes no validation behavior.
AssertionTypeis:open? true,which flanders compiles to plain
s/Str-- the vocabulary set is erased atcompile time. An unlisted assertion name already validated before this change
(
s/checkon{:name "cisco:ctr:device:past_ids" :value "x"}returnednilagainst
origin/master). The value of this PR is discoverability and apublished contract, not enforcement.
2. Multi-valued assertions are carried as a JSON-encoded array inside the
single
:valuestring.Assertionis{:name Str, :value Str}--:valueis a single string. The Device Insights producer encodes any non-string value
as a JSON-encoded array of strings in that one
:value, and this is its blanketrule for every plural-named entry (
cisco:ctr:user:emails,:groups,:phone_numbers,cisco:ctr:device:past_ids); the acceptance test JSON-decodesthe value. The description now documents that shape:
:assertions [{:name "cisco:ctr:device:id" :value "00-00-00-01"} {:name "cisco:ctr:device:past_ids" :value "[\"00-00-00-08\",\"00-00-00-09\"]"}]This matches the 11 plural-named entries already in the vocabulary
(
cisco:ctr:user:emails,:phone_numbers,:roles,:groups,:entitlements,cisco:ctr:device:administrators, the fourcisco:ctr:ad:*_resolved_*, andcisco:ctr:common:ir_attributes), all of whichcarry
:value Str. Because:valueiss/Str, a nested array is invisible toschema validation -- consumers must JSON-decode it. A consumer that collapses
assertions with
(into {} (map (juxt :name :value)))keeps the array stringintact under one key, which is the intended shape.
Alternatives considered and rejected:
past_idname would be the vocabulary's only singular-repeatentry, contradicting 11 plural siblings.
documented -- is not what any producer emits (see @msprunck's review thread,
which verified the producer directly). Documenting it would have marked the
only producer non-conformant and prescribed a shape no consumer receives.
Assertion :valueto accept arrays, or adding an optional:valuesentry, is forward-incompatible, not merely a contract change: flanders map
types compile to closed maps, so any consumer validating against an older CTIM
jar rejects the extra/retyped key as a disallowed-key error.
Testing
lein with-profile -user test-- 139 tests, 510 assertions, 0 failures..github/workflows/build.yml:57runslein do clean, compile :all, doo node node once; there is nolein teststep in CI):lein with-profile -user do clean, doo node node once-- 36 tests, 147assertions, 0 failures.
lein docregeneratesdoc/structures/bundle.mdandcasebook.mdwith thenew description.
doc/json:lein docon a-SNAPSHOTversion also rewritesschema_versionin 21doc/json/*.jsonfiles (1.3.30->1.3.30-SNAPSHOT);that churn is pre-existing and unrelated, so it is excluded here. This
description edit does not rewrite
doc/jsonon its own, but the earlier claimthat
doc/json's example value is "vocabulary-independent" is not accurate:the flanders enum's
:defaultis(-> values sort first)and->jsondoesnot null defaults, so
doc/json/bundle.jsonandcasebook.jsonescape churnhere only because
cisco:ctr:ad:host_domain_namestill sorts first. A futurevocabulary addition that sorts earlier (e.g. a
cisco:ctr:aa:*name) WILLrewrite those files -- the next contributor adding such a name must regenerate.
QA
:assertionsinclude acisco:ctr:device:past_idsentry whose:valueis a JSON-encoded array ofids (e.g.
"[\"00-00-00-08\",\"00-00-00-09\"]"); confirm it round-trips andthe value is preserved verbatim for consumers to JSON-decode.
cisco:ctr:device:past_idsappears in the AssertionType vocabularyin the generated docs (
doc/structures/bundle.md,casebook.md).(e.g.
cisco:ctr:device:id) still validates unchanged.Out of scope (pre-existing, filed separately if wanted)
doc/README.md:45,72link tostructures/identity_assertion.mdandjson/identity_assertion.json, which have never been generated -- there are noIdentityAssertion rows in
src/ctim/document.clj. Dead since the entity wasadded in 2019 (e5f4e62). Not touched here to keep this diff minimal.
Rejected findings
:assertions:description(L100-101) (msprunck, 2026-08-12 approval) -- Declined. The convention now lives on the exact field it governs (Assertion:value, added in7cd2fb4) and onAssertionType.:assertionsdescribes the collection at a higher level; duplicating the encoding rule a third time on the parent collection adds drift risk without giving a decoding consumer a new surface to read. The:valuecross-reference from the same review was addressed.🤖 Generated with Claude Code