feat(yang-push): normalize target xpath filters and add failed xpath resolution check with libyang - #45
Open
rodonile wants to merge 9 commits into
Open
Conversation
rodonile
changed the title
rodonile
force-pushed
the
xpath-normalization
branch
4 times, most recently
from
September 4, 2026 09:03
4289526 to
40a040f
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new literal-QName detection/rewriting path currently treats leading/trailing whitespace inside quoted literals as valid QNames due to trimming, which can incorrectly retain/resolve prefixes and rewrite predicate literal values.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends the YANG-Push pipeline to canonicalize datastore-xpath-filter targets into an RFC 8641 / libyang-aligned “module-name-qualified, prefix-on-change” form across both NETCONF/XML subscription fetches and JSON SubscriptionStarted/SubscriptionModified notifications, and adds a diagnostic to warn when reported targets don’t resolve or aren’t canonical against the loaded schema.
Changes:
- Introduces a new
crates/netconf-proto/src/xpath.rsmodule consolidating XPath string parsing/utility helpers and updates XML namespace extraction to use it. - Adds
DatastoreXPathFilter::normalize_pathand wires normalization into the NETCONF subscription fetcher and JSON notification ingestion path. - Adds a schema-resolution diagnostic (
check_xpath_target_resolves) plus unit tests validating both normalization and diagnostics behavior.
File summaries
| File | Description |
|---|---|
| crates/yang-push/src/validation/mod.rs | Normalize JSON target xpath filters on ingestion and add a schema-resolution / canonical-form diagnostic with tests. |
| crates/yang-push/src/cache/fetcher.rs | Normalize NETCONF-fetched subscription target xpath filters using YANG library module resolution. |
| crates/netconf-proto/src/yang_push/tests.rs | Adds/expands unit tests for xpath prefix detection and DatastoreXPathFilter::normalize_path. |
| crates/netconf-proto/src/yang_push/filters.rs | Implements DatastoreXPathFilter::normalize_path and refines required-prefix extraction semantics. |
| crates/netconf-proto/src/xpath.rs | New shared XPath 1.0-subset string utilities and accompanying tests. |
| crates/netconf-proto/src/xml_utils.rs | Switches xpath-prefix discovery to the new crate::xpath helpers. |
| crates/netconf-proto/src/lib.rs | Exposes the new xpath module. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
rodonile
force-pushed
the
xpath-normalization
branch
from
September 4, 2026 11:17
40a040f to
8b62630
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core XPath normalization and schema-resolution behavior across ingestion and fetch paths, so it warrants a final domain-focused review despite the added tests.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
rodonile
force-pushed
the
xpath-normalization
branch
from
September 4, 2026 11:31
8b62630 to
08810b8
Compare
rodonile
marked this pull request as ready for review
September 4, 2026 11:32
Move find_xpath_prefixes out of xml_utils.rs (a broad, unrelated XML-parsing grab-bag) into a new xpath.rs module, the shared home for XPath 1.0 subset text utilities. Register the module in lib.rs and update its two call sites. Pure refactor, no behavior change. Sets up xpath.rs as the target for the normalize_path engine added next.
Add DatastoreXPathFilter::normalize_path: converts an xpath to RFC 8641's canonical, module-name-qualified, prefix-on-change form (matches libyang's SchemaPathFormat::DATA), whether the source path uses declared xmlns prefixes or bare module-name prefixes. Backed by three new pure helpers in xpath.rs: split_location_path, parse_node_test, is_ncname. Bails to None (caller keeps the original path) for unsupported XPath 1.0 constructs or an unresolvable declared prefix. Not wired into any caller yet.
Apply DatastoreXPathFilter::normalize_path to the datastore xpath filter fetched via get_yang_push_subscription_by_id, so the cached target is always in canonical module-name-qualified form regardless of how the device encoded prefixes (declared xmlns vs. bare module name). Falls back to the original path (with a warning) when the path can't be confidently normalized.
JSON-encoded SubscriptionStarted/Modified notifications carry their datastore-xpath-filter as a plain string with no xmlns table. Normalize it the same way as the NETCONF/XML path, in build_subscription_info, via normalize_json_target_xpath. Needs no schema access: passing an empty namespace table makes normalize_path treat every prefix as already-resolved. Reduces noise in the canonical-form diagnostic and avoids spurious subscription-changed refetches from pure prefix-style variance.
Add check_xpath_target_resolves: after a schema loads, evaluate the subscription's datastore-xpath-filter against the libyang context and warn if it does not resolve to a schema node, or resolves but isn't in libyang's canonical (SchemaPathFormat:: DATA) form. Diagnostic only, never mutates the target. Move xpath_diff and strip_xpath_predicates into netconf-proto's xpath module so they're reusable and directly testable.
find_xpath_prefixes previously skipped all string-literal content when extracting xpath prefixes, so a module referenced only inside a predicate's identityref-shaped literal (e.g. hw-hwt in 'hw-hwt:ethernetCsmacd-xcvr-link') was never returned. This made the fetcher silently drop that module from the schema fetch list, and the parser drop the matching xmlns binding on deserialize. find_xpath_prefixes now also reports a literal's prefix when its entire content is shaped like one QName. normalize_path's predicate rewriter mirrors this: such a literal is now resolved and rewritten to its full module name too, keeping the canonical xpath self-contained once the fetcher wipes the namespace map.
normalize_path treated every empty path segment as valid, letting unsupported `//` and trailing `/` paths through instead of bailing. strip_xpath_predicates never checked bracket/quote balance, silently truncating malformed input instead of returning it unchanged.
find_xpath_prefixes now splits structural (node-name) prefixes from ones seen only inside a whole-QName-shaped literal. path_prefixes() drops an undeclared literal-shaped prefix instead of treating it as a required module, so an incidental value like an interface name 'ge:0' no longer fails the fetch when no such module exists.
parse_node_test trims its input, so a predicate literal padded with whitespace (e.g. ' a:b') was treated as a whole QName and its prefix wrongly recorded/resolved. Both find_xpath_prefixes and resolve_literal_qname now require literal == literal.trim() before treating it as a QName.
rodonile
force-pushed
the
xpath-normalization
branch
from
September 4, 2026 12:32
08810b8 to
500ed4f
Compare
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.
This PR depends on #43.
Summary
Normalizes
datastore-xpath-filtertargets — from both NETCONF/XMLsubscription fetches and JSON-encoded
SubscriptionStarted/SubscriptionModifiednotifications — to RFC 8641's module-name-qualified,prefix-on-change form, and adds a diagnostic that warns when a
target xpath doesn't resolve against the loaded schema or isn't in libyang's
canonical form.
Motivation
Different publishers/transports encode the same xpath target
differently: some use declared
xmlnsprefixes, some use baremodule-name prefixes, some repeat the prefix on every step instead
of only on module change. This caused issues to non-YANG-aware
post processing engines that are relying on xpath matching
for filtering messages.
Changes
new
crates/netconf-proto/src/xpath.rsmodule.DatastoreXPathFilter::normalize_path, the canonicalizationengine (bails safely to the original path on anything unsupported).
declared prefixes via the router's YANG library) and JSON-encoded
SubscriptionStarted/Modifiedtargets.check_xpath_target_resolves, a diagnostic that warns when atarget xpath doesn't resolve against the loaded schema or isn't in
libyang's canonical form.