Skip to content

fix: normalize identityref values in JSON importer and leafref validation#444

Open
steiler wants to merge 4 commits into
mainfrom
fix/identityref-leafref-normalization
Open

fix: normalize identityref values in JSON importer and leafref validation#444
steiler wants to merge 4 commits into
mainfrom
fix/identityref-leafref-normalization

Conversation

@steiler
Copy link
Copy Markdown
Collaborator

@steiler steiler commented Jun 1, 2026

Summary

  • JSON importer `GetKeyValue`: routes through `GetTVValue`/`tv.ToString()` so identityref keys are stored as bare identity names (e.g. `BGP`) rather than a raw proto struct dump, making JSON_IETF-encoded configs produce the same tree keys as plain JSON.
  • `resolveLeafrefKeyPath`: switches `tv.GetStringVal()` → `tv.ToString()` so `current()`-relative key predicates resolve correctly when the key type is `identityref` (`GetStringVal` returns `""` for `IdentityrefVal`).
  • `Navigate` in `yangParserEntryAdapter`: calls `p.StripPathElemPrefixPath()` before `NavigateSdcpbPath`, consistent with `BreadthSearch`, so xpath evaluation against identityref-keyed list entries succeeds when path elements carry a module prefix.

Together with the companion schema-server PR these four fixes fully resolve the Arista `routing-policy` silent validation failures first reported in #349.

Companion PR

schema-server (compile-time must-statement literal normalisation):
sdcio/schema-server#241

…tion

GetKeyValue in the JSON importer now routes through GetTVValue/ToString
so identityref keys are emitted as module-qualified strings (e.g.
"sdcio-model-identity:ETHERNET") rather than a raw proto-struct dump.

resolveLeafrefKeyPath switches from tv.GetStringVal() to tv.ToString()
for the same reason — GetStringVal returns an empty string for
TypedValue_IdentityRef, causing leafref lookups to silently fail.

Navigate in yangParserEntryAdapter now calls StripPathElemPrefixPath
before NavigateSdcpbPath so xpath evaluation against identityref-keyed
list entries succeeds when path elements carry a module prefix.

Adds regression tests: TestLeafref_IdentityrefKey, TestMust_IdentityrefKey
(validation), and JSON-importer round-trip test with identityref list key.
Extends sdcio_model_identity.yang with matching fixtures.

Co-authored-by: Cursor <cursoragent@cursor.com>
steiler and others added 3 commits June 2, 2026 14:25
Periodic GET sync revert raced with concurrent intent delete transactions:
LoadAllButRunningIntents read stale cache (deleted intent still present),
then applyIntent pushed deleted config back to device, undoing the deletion.

Acquiring dmutex in performRevert forces it to wait for any active
transaction to fully complete (device write + IntentDelete) before
snapshotting the intent store.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ErrTransactionOngoing was falling through translateInternalToGrpcError
and being wrapped by gRPC as codes.Unknown. config-server treats
codes.Unknown as non-recoverable, causing a cascade of permanent failures
instead of a backoff-and-retry when a transaction collision occurs.
Map it to codes.Aborted alongside ErrDatastoreLocked so config-server
correctly retries.

Additionally, split the single dmutex acquisition in performRevert into
two narrow critical sections: one covering LoadAllButRunningIntents (the
cache snapshot that must be atomic with IntentDelete) and one covering
applyIntent (the gNMI device write). FinishInsertionPhase, GetDeletes,
and ToProtoUpdates operate only on the local tree copy and no longer
hold the mutex, reducing contention on concurrent transactions.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 57.14286% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/datastore/sync.go 50.00% 2 Missing ⚠️
pkg/server/transaction.go 0.00% 2 Missing ⚠️
pkg/tree/importer/json/json_tree_importer.go 66.66% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@steiler steiler linked an issue Jun 3, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Must-statements

1 participant