Skip to content

Commit eb2d916

Browse files
authored
[GCU] Prohibit removal of PFC_WD POLL_INTERVAL field (#2777)
What I did -Add new infrastructure to GCU for validation of moves that YANG models are unable to capture -Prohibit deletion of PFC_WD POLL_INTERVAL field How I did it Ensure JSON Diff does not have POLL_INTERVAL deletion If diff reflects request for POLL_INTERVAL deletion, raise an Exception and do not allow patch application to go through How to verify it Attempt to delete POLL_INTERVAL field using GCU
1 parent fe224f0 commit eb2d916

File tree

3 files changed

+38
-0
lines changed

3 files changed

+38
-0
lines changed

generic_config_updater/generic_updater.py

+4
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ def apply(self, patch):
4747
# Generate target config
4848
self.logger.log_notice("Simulating the target full config after applying the patch.")
4949
target_config = self.patch_wrapper.simulate_patch(patch, old_config)
50+
51+
# Validate all JsonPatch operations on specified fields
52+
self.logger.log_notice("Validating all JsonPatch operations are permitted on the specified fields")
53+
self.config_wrapper.validate_field_operation(old_config, target_config)
5054

5155
# Validate target config does not have empty tables since they do not show up in ConfigDb
5256
self.logger.log_notice("Validating target config does not have empty tables, " \

generic_config_updater/gu_common.py

+22
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616
class GenericConfigUpdaterError(Exception):
1717
pass
1818

19+
class IllegalPatchOperationError(ValueError):
20+
pass
21+
22+
class EmptyTableError(ValueError):
23+
pass
24+
1925
class JsonChange:
2026
"""
2127
A class that describes a partial change to a JSON object.
@@ -135,6 +141,22 @@ def validate_config_db_config(self, config_db_as_json):
135141

136142
return True, None
137143

144+
def validate_field_operation(self, old_config, target_config):
145+
"""
146+
Some fields in ConfigDB are restricted and may not allow third-party addition, replacement, or removal.
147+
Because YANG only validates state and not transitions, this method helps to JsonPatch operations/transitions for the specified fields.
148+
"""
149+
patch = jsonpatch.JsonPatch.from_diff(old_config, target_config)
150+
151+
# illegal_operations_to_fields_map['remove'] yields a list of fields for which `remove` is an illegal operation
152+
illegal_operations_to_fields_map = {'add':[],
153+
'replace': [],
154+
'remove': ['/PFC_WD/GLOBAL/POLL_INTERVAL', '/PFC_WD/GLOBAL']}
155+
for operation, field_list in illegal_operations_to_fields_map.items():
156+
for field in field_list:
157+
if any(op['op'] == operation and field == op['path'] for op in patch):
158+
raise IllegalPatchOperationError("Given patch operation is invalid. Operation: {} is illegal on field: {}".format(operation, field))
159+
138160
def validate_lanes(self, config_db):
139161
if "PORT" not in config_db:
140162
return True, None

tests/generic_config_updater/gu_common_test.py

+12
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,18 @@ def setUp(self):
6969
self.config_wrapper_mock = gu_common.ConfigWrapper()
7070
self.config_wrapper_mock.get_config_db_as_json=MagicMock(return_value=Files.CONFIG_DB_AS_JSON)
7171

72+
def test_validate_field_operation_legal(self):
73+
old_config = {"PFC_WD": {"GLOBAL": {"POLL_INTERVAL": "60"}}}
74+
target_config = {"PFC_WD": {"GLOBAL": {"POLL_INTERVAL": "40"}}}
75+
config_wrapper = gu_common.ConfigWrapper()
76+
config_wrapper.validate_field_operation(old_config, target_config)
77+
78+
def test_validate_field_operation_illegal(self):
79+
old_config = {"PFC_WD": {"GLOBAL": {"POLL_INTERVAL": 60}}}
80+
target_config = {"PFC_WD": {"GLOBAL": {}}}
81+
config_wrapper = gu_common.ConfigWrapper()
82+
self.assertRaises(gu_common.IllegalPatchOperationError, config_wrapper.validate_field_operation, old_config, target_config)
83+
7284
def test_ctor__default_values_set(self):
7385
config_wrapper = gu_common.ConfigWrapper()
7486

0 commit comments

Comments
 (0)