From 7c79094e979a6f1ecdef0449717adda85d4d43af Mon Sep 17 00:00:00 2001 From: Leonardo Rodoni Date: Tue, 25 Aug 2026 14:17:21 +0200 Subject: [PATCH] fix(netconf-proto): sort modules before content-id hash 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. --- crates/netconf-proto/src/yanglib.rs | 153 ++++++++++++++++++++++++++-- 1 file changed, 147 insertions(+), 6 deletions(-) diff --git a/crates/netconf-proto/src/yanglib.rs b/crates/netconf-proto/src/yanglib.rs index f65d7dcc..acfc2e7a 100644 --- a/crates/netconf-proto/src/yanglib.rs +++ b/crates/netconf-proto/src/yanglib.rs @@ -1,3 +1,19 @@ +// Copyright (C) 2026-present The NetCalyx Authors. +// Copyright (C) 2025-present The NetGauze Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. +// See the License for the specific language governing permissions and +// limitations under the License. + use crate::xml_utils::{ParsingError, XmlDeserialize, XmlParser, XmlSerialize, XmlWriter}; use crate::yangparser::{YangDependencies, extract_yang_dependencies}; use crate::{YANG_DATASTORES_NS_STR, YANG_LIBRARY_AUGMENTED_BY_NS, YANG_LIBRARY_NS}; @@ -2091,21 +2107,47 @@ impl ModuleSetBuilder { } /// Produce a YANG library that contains only one module set + /// + /// The `content-id` is a SHA-256 fingerprint computed over the module + /// schemas. To make it a stable, canonical fingerprint of the *content* + /// (independent of the order modules/features/submodules were + /// discovered or inserted, e.g. due to BFS dependency traversal order or + /// device-reported ordering), everything that is hashed is first sorted + /// deterministically (by name/revision). Otherwise, semantically + /// identical module sets could produce different content-ids just + /// because of insertion order, causing spurious cache duplication. pub fn build_yang_lib(self) -> (YangLibrary, HashMap, Box>) { let default_name: Box = "ALL".into(); let mut content_id = sha2::Sha256::new(); - for module in self.module_set.modules().values() { - for feature in module.features() { + + let mut modules: Vec<_> = self.module_set.modules().values().collect(); + modules.sort_unstable_by(|a, b| a.name().cmp(b.name())); + for module in modules { + let mut features: Vec<&Box> = module.features().iter().collect(); + features.sort_unstable(); + for feature in features { content_id.update(feature.as_ref()); } - for submodule in module.submodules() { + let mut submodules: Vec<_> = module.submodules().iter().collect(); + submodules.sort_unstable_by(|a, b| a.name().cmp(b.name())); + for submodule in submodules { content_id.update(self.yang_schemas.get(submodule.name()).unwrap().as_ref()); } content_id.update(self.yang_schemas.get(module.name()).unwrap().as_ref()); } - for import_only_versions in self.module_set.import_only_modules().values() { - for module in import_only_versions.values() { - for (_, submodule) in module.submodules() { + + let mut import_only_names: Vec<_> = self.module_set.import_only_modules().keys().collect(); + import_only_names.sort_unstable(); + for name in import_only_names { + let import_only_versions = &self.module_set.import_only_modules()[name]; + let mut modules: Vec<_> = import_only_versions.values().collect(); + modules.sort_unstable_by(|a, b| { + a.name().cmp(b.name()).then(a.revision().cmp(&b.revision())) + }); + for module in modules { + let mut submodules: Vec<_> = module.submodules().iter().collect(); + submodules.sort_unstable_by(|a, b| a.0.cmp(b.0)); + for (_, submodule) in submodules { content_id.update(self.yang_schemas.get(submodule.name()).unwrap().as_ref()); } content_id.update(self.yang_schemas.get(module.name()).unwrap().as_ref()); @@ -2821,4 +2863,103 @@ mod tests { None ); } + + /// The `content-id` produced by `ModuleSetBuilder::build_yang_lib` must be + /// a canonical fingerprint of the module set content: it should not + /// depend on the order modules (and their features/submodules) were + /// inserted into the builder. Otherwise, semantically identical module + /// sets discovered/traversed in a different order (e.g. BFS dependency + /// resolution visiting modules in a different sequence) would produce + /// different content-ids, causing spurious cache duplication. + #[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()), + "urn:example:module-a".into(), + Box::new(["feature-2".into(), "feature-1".into()]), + Box::new([]), + Box::new([Submodule::new( + "module-a-sub".into(), + Some("2020-01-01".into()), + Box::new([]), + )]), + Box::new([]), + Box::new([]), + ); + let module_b = Module::new( + "module-b".into(), + Some("2020-02-02".into()), + "urn:example:module-b".into(), + Box::new([]), + Box::new([]), + Box::new([]), + Box::new([]), + Box::new([]), + ); + + let mut builder1 = ModuleSetBuilder::new("ALL".into()); + builder1 + .add_module( + module_a.clone(), + "module-a-schema".into(), + &PermissiveVersionChecker, + ) + .unwrap(); + builder1 + .add_submodule_for_module( + "module-a", + Submodule::new( + "module-a-sub".into(), + Some("2020-01-01".into()), + Box::new([]), + ), + "module-a-sub-schema".into(), + &PermissiveVersionChecker, + ) + .unwrap(); + builder1 + .add_module( + module_b.clone(), + "module-b-schema".into(), + &PermissiveVersionChecker, + ) + .unwrap(); + let (yang_lib1, _) = builder1.build_yang_lib(); + + // Insert the same modules in reverse order, and with module_a's + // features reordered, to also check feature-order independence. + let mut module_a2 = module_a; + module_a2.feature = Box::new(["feature-1".into(), "feature-2".into()]); + let mut builder2 = ModuleSetBuilder::new("ALL".into()); + builder2 + .add_module( + module_b, + "module-b-schema".into(), + &PermissiveVersionChecker, + ) + .unwrap(); + builder2 + .add_module( + module_a2, + "module-a-schema".into(), + &PermissiveVersionChecker, + ) + .unwrap(); + builder2 + .add_submodule_for_module( + "module-a", + Submodule::new( + "module-a-sub".into(), + Some("2020-01-01".into()), + Box::new([]), + ), + "module-a-sub-schema".into(), + &PermissiveVersionChecker, + ) + .unwrap(); + let (yang_lib2, _) = builder2.build_yang_lib(); + + assert_eq!(yang_lib1.content_id(), yang_lib2.content_id()); + } }