Skip to content

fix(netconf-proto): make YANG library content-id calculation independent on the module order - #44

Open
rodonile wants to merge 1 commit into
network-analytics:mainfrom
rodonile:fix-order-sha256
Open

fix(netconf-proto): make YANG library content-id calculation independent on the module order#44
rodonile wants to merge 1 commit into
network-analytics:mainfrom
rodonile:fix-order-sha256

Conversation

@rodonile

Copy link
Copy Markdown
Member

Problem

ModuleSetBuilder::build_yang_lib() computes a SHA-256 content-id
by hashing modules, features, and submodules in the order they
happen to be stored in an IndexMap (insertion order). That order
is driven by the BFS dependency traversal in
NetConfSshClient::load_from_modules(), seeded from the
subscription's xpath-filter namespace resolution.

Since content-id is used verbatim as the on-disk cache directory
name (kafka-yang-producer-cache/<content-id>/...), two fetches
that resolve the same set of modules but in a different order
produce two different SHA-256 digests, and thus two separate cache
directories for what is really the same schema. This was observed
on a subscription where the order of modules in yang-lib.xml
resulted being different, due to the root modules list also being
ordered differently when extracted from subscription-started
message wrt. from get-subscriptions rpc.

Fix

Sort everything that feeds the hash before updating it:

  • modules by name
  • features lexically
  • submodules by name
  • import-only modules by name, then revision

This makes content-id a canonical fingerprint of the module set
content, independent of discovery/insertion order.

@rodonile rodonile self-assigned this Aug 25, 2026
@rodonile
rodonile requested a lite review from Copilot August 25, 2026 12:23
@rodonile
rodonile enabled auto-merge (rebase) August 25, 2026 12:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes ModuleSetBuilder::build_yang_lib() compute a stable content-id (SHA-256) that depends only on the set content rather than the module discovery/insertion order, preventing duplicate on-disk cache directories for semantically identical schemas.

Changes:

  • Sorts modules, features, submodules, and import-only modules deterministically before feeding them into the SHA-256 hasher.
  • Adds a regression test asserting content-id is independent of module insertion order.
Suppressed comments (1)

crates/netconf-proto/src/yanglib.rs:2100

  • Doc comment says the hash inputs are sorted "by name/revision", but the implementation sorts modules/submodules by name only (modules are unique by name) and features lexically. Rewording avoids implying revision is part of the general sort key.
                })

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/netconf-proto/src/yanglib.rs
The SHA-256 content-id fingerprint in build_yang_lib() hashed
modules, features, and submodules in IndexMap insertion order,
which depends on BFS dependency traversal order. Semantically
identical module sets could therefore produce different
content-ids, causing duplicate yang-push cache directories for
the same subscription.

Sort modules by name, features lexically, submodules by name,
and import-only modules by name+revision before hashing, so the
content-id is a canonical, order-independent fingerprint.

Add a regression test asserting content-id is stable across
insertion order.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment on lines +2874 to +2878
#[test]
fn test_build_yang_lib_content_id_is_insertion_order_independent() {
let module_a = Module::new(
"module-a".into(),
Some("2020-01-01".into()),
Comment on lines +2126 to 2130
let mut features: Vec<&Box<str>> = module.features().iter().collect();
features.sort_unstable();
for feature in features {
content_id.update(feature.as_ref());
}
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.

2 participants