fix: normalize identityref values in JSON importer and leafref validation#444
Open
steiler wants to merge 4 commits into
Open
fix: normalize identityref values in JSON importer and leafref validation#444steiler wants to merge 4 commits into
steiler wants to merge 4 commits into
Conversation
…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>
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 Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
Open
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
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
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