From 523dabfef5e4b173c0dfdf010856b22e3732d1b8 Mon Sep 17 00:00:00 2001 From: bradleymoon-nexthop Date: Thu, 30 Jul 2026 10:30:11 -0700 Subject: [PATCH 1/4] Moved create-only to YANG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: bradleymoon-nexthop Signed-off-by: Bradley Moon  --- generic_config_updater/patch_sorter.py | 94 +++++++++++- .../patch_sorter_test.py | 136 +++++++++++++++++- 2 files changed, 226 insertions(+), 4 deletions(-) diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index b6198153b2c..ee9ebd86923 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -8,6 +8,48 @@ from .gu_common import OperationWrapper, OperationType, GenericConfigUpdaterError, \ JsonChange, PathAddressing, genericUpdaterLogging +<<<<<<< HEAD +======= +# Floor of create-only patterns used during the transition to YANG annotations. +# Exit condition: delete this list (and the union below) once every branch this +# consumer ships to carries annotated models that discover at least these +# entries via sonic_yang.get_create_only_fields(). Until then, discovery is +# unioned with this floor so YANG can only add patterns, never silently drop +# protection. test_discover_create_only_fields__yang_set_covers_fallback_floor +# fails if a non-empty discovered set ever narrows below this floor. +_CREATE_ONLY_FIELDS_FALLBACK = [ + ["PORT", "*", "lanes"], + ["LOOPBACK_INTERFACE", "*", "vrf_name"], + ["BGP_NEIGHBOR", "*", "holdtime"], + ["BGP_NEIGHBOR", "*", "keepalive"], + ["BGP_NEIGHBOR", "*", "name"], + ["BGP_NEIGHBOR", "*", "asn"], + ["BGP_NEIGHBOR", "*", "local_addr"], + ["BGP_NEIGHBOR", "*", "nhopself"], + ["BGP_NEIGHBOR", "*", "rrclient"], + ["BGP_PEER_RANGE", "*", "*"], + ["BGP_SENTINELS", "*", "*"], + ["BGP_MONITORS", "*", "holdtime"], + ["BGP_MONITORS", "*", "keepalive"], + ["BGP_MONITORS", "*", "name"], + ["BGP_MONITORS", "*", "asn"], + ["BGP_MONITORS", "*", "local_addr"], + ["BGP_MONITORS", "*", "nhopself"], + ["BGP_MONITORS", "*", "rrclient"], + ["MIRROR_SESSION", "*", "*"], + ["SCHEDULER", "*", "type"], + ["SCHEDULER", "*", "weight"], + ["SCHEDULER", "*", "meter_type"], + ["SCHEDULER", "*", "cir"], + ["SCHEDULER", "*", "cbs"], + ["SCHEDULER", "*", "pir"], + ["SCHEDULER", "*", "pbs"], +] + +GCU_FIELD_OP_CONF_FILE = os.path.join(os.path.dirname(os.path.realpath(__file__)), + "gcu_field_operation_validators.conf.json") + +>>>>>>> 905604f7 (NOS-12728: Moved create-only to YANG (#801)) class Diff: """ A class that contains the diff info between current and target configs. @@ -712,8 +754,8 @@ class CreateOnlyFilter: A filtering class for create-only fields. """ def __init__(self, path_addressing): - # TODO: create-only fields are hard-coded for now, it should be moved to YANG model self.path_addressing = path_addressing +<<<<<<< HEAD self.patterns = [ ["PORT", "*", "lanes"], ["LOOPBACK_INTERFACE", "*", "vrf_name"], @@ -735,6 +777,56 @@ def __init__(self, path_addressing): ["BGP_MONITORS", "*", "rrclient"], ["MIRROR_SESSION", "*", "*"], ] +======= + self.logger = genericUpdaterLogging.get_logger(title="Patch Sorter - CreateOnlyFilter") + self.patterns = self._discover_create_only_fields() + + def _discover_create_only_fields(self): + """ + Prefer YANG-derived create-only patterns from sonic_yang, unioned with + _CREATE_ONLY_FIELDS_FALLBACK so an incomplete annotation set cannot + silently narrow protection. When discovery is unavailable or empty + (unannotated models, missing sonic_yang API, PathAddressing without + config_wrapper), use the fallback alone. + """ + try: + sy = self.path_addressing._create_sonic_yang_with_loaded_models() + except AttributeError: + # PathAddressing built without a config_wrapper (gu_common.py). + sy = None + except Exception as ex: + self.logger.log_warning( + "Failed to load sonic_yang for create-only discovery, " + f"using fallback. Error: {ex}") + sy = None + + yang_patterns = [] + if sy is not None and hasattr(sy, "get_create_only_fields"): + yang_patterns = sy.get_create_only_fields() or [] + + fallback = [list(p) for p in _CREATE_ONLY_FIELDS_FALLBACK] + if not yang_patterns: + return fallback + + # De-duplicate YANG results while preserving order, then union the + # fallback floor so YANG can only add patterns. + deduped = [] + seen = set() + for pattern in yang_patterns: + key = tuple(pattern) + if key not in seen: + seen.add(key) + deduped.append(list(pattern)) + + missing = [p for p in fallback if tuple(p) not in seen] + if missing: + self.logger.log_warning( + f"YANG create-only discovery omitted {len(missing)} fallback " + f"pattern(s), unioning them back in: {missing}") + for pattern in missing: + deduped.append(list(pattern)) + return deduped +>>>>>>> 905604f7 (NOS-12728: Moved create-only to YANG (#801)) def get_filter(self): return JsonPointerFilter(self.patterns, diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index 501b38ca272..ff6989d6e7c 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -963,9 +963,114 @@ def test_validate__passes_quiet_true_to_config_wrapper(self): self.any_simulated_config) +class TestCreateOnlyFilter(unittest.TestCase): + def test_discover_create_only_fields__fallback_floor_includes_port_lanes(self): + """ + Shipping images without get_create_only_fields (or with empty discovery) + must still protect PORT/lanes via the fallback floor. + """ + create_only_filter = ps.CreateOnlyFilter(PathAddressing(ConfigWrapper())) + patterns = create_only_filter.patterns + + self.assertTrue(patterns, "create-only pattern list must not be empty") + self.assertIn(["PORT", "*", "lanes"], patterns) + # Union floor: runtime patterns always cover the historical set. + for pattern in ps._CREATE_ONLY_FIELDS_FALLBACK: + self.assertIn(pattern, patterns) + + def test_discover_create_only_fields__yang_set_covers_fallback_floor(self): + """ + When discovery is available and non-empty, the YANG-derived set must + cover the fallback floor. Skips on images that still lack the accessor + or annotated models (those take the fallback-only path above). + """ + path_addressing = PathAddressing(ConfigWrapper()) + try: + sy = path_addressing._create_sonic_yang_with_loaded_models() + except Exception: + self.skipTest("sonic_yang models unavailable in this environment") + + if not hasattr(sy, "get_create_only_fields"): + self.skipTest("sonic_yang.get_create_only_fields not installed yet") + + discovered = sy.get_create_only_fields() or [] + if not discovered: + self.skipTest("YANG models not yet annotated; fallback-only path") + + discovered_set = {tuple(p) for p in discovered} + fallback_set = {tuple(p) for p in ps._CREATE_ONLY_FIELDS_FALLBACK} + missing = sorted(fallback_set - discovered_set) + self.assertFalse( + missing, + f"YANG create-only set narrowed below fallback floor: {missing}", + ) + + def test_discover_create_only_fields__no_accessor_uses_fallback(self): + """Images without get_create_only_fields take the fallback path.""" + path_addressing = PathAddressing(ConfigWrapper()) + # MagicMock() always has every attribute; spec=[] removes the accessor. + mock_sy = MagicMock(spec=[]) + with mock.patch.object(path_addressing, "_create_sonic_yang_with_loaded_models", + return_value=mock_sy): + create_only_filter = ps.CreateOnlyFilter(path_addressing) + + self.assertFalse(hasattr(mock_sy, "get_create_only_fields")) + self.assertEqual(create_only_filter.patterns, ps._CREATE_ONLY_FIELDS_FALLBACK) + + def test_discover_create_only_fields__missing_config_wrapper_uses_fallback(self): + """AttributeError from PathAddressing(None) must fall back, not raise.""" + create_only_filter = ps.CreateOnlyFilter(PathAddressing(None)) + self.assertEqual(create_only_filter.patterns, ps._CREATE_ONLY_FIELDS_FALLBACK) + + def test_discover_create_only_fields__empty_yang_uses_fallback(self): + path_addressing = PathAddressing(ConfigWrapper()) + mock_sy = MagicMock() + mock_sy.get_create_only_fields.return_value = [] + with mock.patch.object(path_addressing, "_create_sonic_yang_with_loaded_models", + return_value=mock_sy): + create_only_filter = ps.CreateOnlyFilter(path_addressing) + + self.assertEqual(create_only_filter.patterns, ps._CREATE_ONLY_FIELDS_FALLBACK) + self.assertIn(["PORT", "*", "lanes"], create_only_filter.patterns) + + def test_discover_create_only_fields__partial_yang_unions_fallback(self): + path_addressing = PathAddressing(ConfigWrapper()) + mock_sy = MagicMock() + mock_sy.get_create_only_fields.return_value = [["PORT", "*", "lanes"]] + with mock.patch.object(path_addressing, "_create_sonic_yang_with_loaded_models", + return_value=mock_sy): + create_only_filter = ps.CreateOnlyFilter(path_addressing) + + patterns = create_only_filter.patterns + self.assertIn(["PORT", "*", "lanes"], patterns) + # Incomplete discovery must not drop the rest of the floor. + self.assertIn(["SCHEDULER", "*", "type"], patterns) + for pattern in ps._CREATE_ONLY_FIELDS_FALLBACK: + self.assertIn(pattern, patterns) + + def test_discover_create_only_fields__deduplicates_yang_patterns(self): + path_addressing = PathAddressing(ConfigWrapper()) + mock_sy = MagicMock() + mock_sy.get_create_only_fields.return_value = [ + ["BGP_NEIGHBOR", "*", "asn"], + ["BGP_NEIGHBOR", "*", "asn"], + ["BGP_NEIGHBOR", "*", "name"], + ] + with mock.patch.object(path_addressing, "_create_sonic_yang_with_loaded_models", + return_value=mock_sy): + create_only_filter = ps.CreateOnlyFilter(path_addressing) + + patterns = create_only_filter.patterns + self.assertEqual(patterns.count(["BGP_NEIGHBOR", "*", "asn"]), 1) + self.assertIn(["BGP_NEIGHBOR", "*", "name"], patterns) + # Deduped YANG entries are unioned with the fallback floor. + for pattern in ps._CREATE_ONLY_FIELDS_FALLBACK: + self.assertIn(pattern, patterns) + + class TestCreateOnlyMoveValidator(unittest.TestCase): def setUp(self): - self.validator = ps.CreateOnlyMoveValidator(ps.PathAddressing()) + self.validator = ps.CreateOnlyMoveValidator(PathAddressing(ConfigWrapper())) self.any_diff = ps.Diff({}, {}) def test_validate__no_create_only_field__success(self): @@ -1093,7 +1198,14 @@ def test_validate__parent_added_without_create_only_field_but_target_have_the_fi } self.verify_parent_adding(added_parent_value, False) - def test_hard_coded_create_only_paths(self): + def test_get_create_only_paths__covers_fallback_annotated_tables(self): + """ + Path expansion for every table in the historical create-only floor. + Exercises whichever pattern source CreateOnlyFilter selected for this + environment (YANG discovery when annotated models + accessor are + present; otherwise the hard-coded fallback). Includes SCHEDULER so all + 26 fallback patterns are asserted at least once. + """ config = { "PORT": { "Ethernet0":{"lanes":"65"}, @@ -1164,7 +1276,18 @@ def test_hard_coded_create_only_paths(self): "ttl": "32", "type": "ERSPAN" } - } + }, + "SCHEDULER": { + "scheduler0": { + "type": "DWRR", + "weight": "10", + "meter_type": "bytes", + "cir": "1000", + "cbs": "2000", + "pir": "3000", + "pbs": "4000", + } + }, } expected = [ "/PORT/Ethernet0/lanes", @@ -1200,6 +1323,13 @@ def test_hard_coded_create_only_paths(self): "/MIRROR_SESSION/mirror_session_dscp/src_ip", "/MIRROR_SESSION/mirror_session_dscp/ttl", "/MIRROR_SESSION/mirror_session_dscp/type", + "/SCHEDULER/scheduler0/type", + "/SCHEDULER/scheduler0/weight", + "/SCHEDULER/scheduler0/meter_type", + "/SCHEDULER/scheduler0/cir", + "/SCHEDULER/scheduler0/cbs", + "/SCHEDULER/scheduler0/pir", + "/SCHEDULER/scheduler0/pbs", ] actual = self.validator._get_create_only_paths(config) From 84f72adf56858273dfa3ff8de9d76cbd35fc050e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bradley=20Moon=C2=A0?= Date: Thu, 30 Jul 2026 17:46:02 +0000 Subject: [PATCH 2/4] Resolve leftover merge conflict markers in CreateOnlyFilter Keep YANG create-only discovery with the fallback floor, and drop the unused private-only GCU_FIELD_OP_CONF_FILE constant from the public port. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bradley Moon  --- generic_config_updater/patch_sorter.py | 30 -------------------------- 1 file changed, 30 deletions(-) diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index ee9ebd86923..1a6b341c09c 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -8,8 +8,6 @@ from .gu_common import OperationWrapper, OperationType, GenericConfigUpdaterError, \ JsonChange, PathAddressing, genericUpdaterLogging -<<<<<<< HEAD -======= # Floor of create-only patterns used during the transition to YANG annotations. # Exit condition: delete this list (and the union below) once every branch this # consumer ships to carries annotated models that discover at least these @@ -46,10 +44,6 @@ ["SCHEDULER", "*", "pbs"], ] -GCU_FIELD_OP_CONF_FILE = os.path.join(os.path.dirname(os.path.realpath(__file__)), - "gcu_field_operation_validators.conf.json") - ->>>>>>> 905604f7 (NOS-12728: Moved create-only to YANG (#801)) class Diff: """ A class that contains the diff info between current and target configs. @@ -755,29 +749,6 @@ class CreateOnlyFilter: """ def __init__(self, path_addressing): self.path_addressing = path_addressing -<<<<<<< HEAD - self.patterns = [ - ["PORT", "*", "lanes"], - ["LOOPBACK_INTERFACE", "*", "vrf_name"], - ["BGP_NEIGHBOR", "*", "holdtime"], - ["BGP_NEIGHBOR", "*", "keepalive"], - ["BGP_NEIGHBOR", "*", "name"], - ["BGP_NEIGHBOR", "*", "asn"], - ["BGP_NEIGHBOR", "*", "local_addr"], - ["BGP_NEIGHBOR", "*", "nhopself"], - ["BGP_NEIGHBOR", "*", "rrclient"], - ["BGP_PEER_RANGE", "*", "*"], - ["BGP_SENTINELS", "*", "*"], - ["BGP_MONITORS", "*", "holdtime"], - ["BGP_MONITORS", "*", "keepalive"], - ["BGP_MONITORS", "*", "name"], - ["BGP_MONITORS", "*", "asn"], - ["BGP_MONITORS", "*", "local_addr"], - ["BGP_MONITORS", "*", "nhopself"], - ["BGP_MONITORS", "*", "rrclient"], - ["MIRROR_SESSION", "*", "*"], - ] -======= self.logger = genericUpdaterLogging.get_logger(title="Patch Sorter - CreateOnlyFilter") self.patterns = self._discover_create_only_fields() @@ -826,7 +797,6 @@ def _discover_create_only_fields(self): for pattern in missing: deduped.append(list(pattern)) return deduped ->>>>>>> 905604f7 (NOS-12728: Moved create-only to YANG (#801)) def get_filter(self): return JsonPointerFilter(self.patterns, From 4d2e683b4167976a50b87b3379ca0031989a83c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bradley=20Moon=C2=A0?= Date: Thu, 30 Jul 2026 17:50:52 +0000 Subject: [PATCH 3/4] Fix flake8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bradley Moon  --- tests/generic_config_updater/patch_sorter_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index ff6989d6e7c..ab9afc183e5 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -3,6 +3,7 @@ import jsonpatch import sys import unittest +from unittest import mock from unittest.mock import MagicMock, Mock import generic_config_updater.patch_sorter as ps from .gutest_helpers import Files, create_side_effect_dict, create_side_effect_jsonmovegroup_dict, \ From 99c7b88cbf0e0721f1097b8d7605c93ed7d74690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bradley=20Moon=C2=A0?= Date: Thu, 30 Jul 2026 17:57:09 +0000 Subject: [PATCH 4/4] rerun build python3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bradley Moon