diff --git a/ncdiff/docs/changelog/undistributed/np_containers.rst b/ncdiff/docs/changelog/undistributed/np_containers.rst
new file mode 100644
index 00000000..4dc496eb
--- /dev/null
+++ b/ncdiff/docs/changelog/undistributed/np_containers.rst
@@ -0,0 +1,5 @@
+--------------------------------------------------------------------------------
+ Fixes
+--------------------------------------------------------------------------------
+* yang.ncdiff
+ * Modified ConfigDelta to avoid 'create' and 'delete' operations on non-presence containers.
diff --git a/ncdiff/src/yang/ncdiff/netconf.py b/ncdiff/src/yang/ncdiff/netconf.py
index dbb811c8..60b81fda 100755
--- a/ncdiff/src/yang/ncdiff/netconf.py
+++ b/ncdiff/src/yang/ncdiff/netconf.py
@@ -352,7 +352,6 @@ def get_config_replace(self, node_self, node_other):
for child_other in in_o_not_in_s:
child_self = etree.Element(child_other.tag,
- {operation_tag: self.preferred_delete},
nsmap=child_other.nsmap)
siblings = list(node_self.iterchildren(tag=child_other.tag))
if siblings:
@@ -361,14 +360,24 @@ def get_config_replace(self, node_self, node_other):
node_self.append(child_self)
s_node = self.device.get_schema_node(child_other)
if s_node.get('type') == 'leaf-list':
+ child_self.set(operation_tag, self.preferred_delete)
self._merge_text(child_other, child_self)
elif s_node.get('type') == 'list':
+ child_self.set(operation_tag, self.preferred_delete)
keys = self._get_list_keys(s_node)
for key in keys:
key_node = child_other.find(key)
e = etree.SubElement(
child_self, key, nsmap=key_node.nsmap)
e.text = key_node.text
+ elif (
+ s_node.get('type') == 'container' and
+ s_node.get('presence') != 'true' and
+ self.preferred_delete == 'delete'
+ ):
+ self.set_delete_operation(child_self, child_other)
+ else:
+ child_self.set(operation_tag, self.preferred_delete)
for child_self, child_other in in_s_and_in_o:
child_self.set(operation_tag, 'replace')
@@ -1066,7 +1075,6 @@ def node_sub(self, node_self, node_other, depth=0):
choice_nodes = {}
for child_self in in_s_not_in_o:
child_other = etree.Element(child_self.tag,
- {operation_tag: self.preferred_delete},
nsmap=child_self.nsmap)
if self.diff_type == 'replace':
child_self.set(operation_tag, 'replace')
@@ -1084,8 +1092,10 @@ def node_sub(self, node_self, node_other, depth=0):
if s_node.get('ordered-by') == 'user' and \
s_node.tag not in ordered_by_user:
ordered_by_user[s_node.tag] = 'leaf-list'
+ child_other.set(operation_tag, self.preferred_delete)
self._merge_text(child_self, child_other)
elif s_node.get('type') == 'list':
+ child_other.set(operation_tag, self.preferred_delete)
keys = self._get_list_keys(s_node)
if s_node.get('ordered-by') == 'user' and \
s_node.tag not in ordered_by_user:
@@ -1095,12 +1105,19 @@ def node_sub(self, node_self, node_other, depth=0):
e = etree.SubElement(
child_other, key, nsmap=key_node.nsmap)
e.text = key_node.text
+ elif (
+ s_node.get('type') == 'container' and
+ s_node.get('presence') != 'true' and
+ self.preferred_delete == 'delete'
+ ):
+ self.set_delete_operation(child_other, child_self)
+ else:
+ child_other.set(operation_tag, self.preferred_delete)
if s_node.getparent().get('type') == 'case':
# key: choice node, value: case node
choice_nodes[s_node.getparent().getparent()] = s_node.getparent()
for child_other in in_o_not_in_s:
child_self = etree.Element(child_other.tag,
- {operation_tag: self.preferred_delete},
nsmap=child_other.nsmap)
if self.preferred_create == 'replace':
child_other.set(operation_tag, self.preferred_create)
@@ -1126,8 +1143,10 @@ def node_sub(self, node_self, node_other, depth=0):
if s_node.get('ordered-by') == 'user' and \
s_node.tag not in ordered_by_user:
ordered_by_user[s_node.tag] = 'leaf-list'
+ child_self.set(operation_tag, self.preferred_delete)
self._merge_text(child_other, child_self)
elif s_node.get('type') == 'list':
+ child_self.set(operation_tag, self.preferred_delete)
keys = self._get_list_keys(s_node)
if s_node.get('ordered-by') == 'user' and \
s_node.tag not in ordered_by_user:
@@ -1136,6 +1155,14 @@ def node_sub(self, node_self, node_other, depth=0):
key_node = child_other.find(key)
e = etree.SubElement(child_self, key, nsmap=key_node.nsmap)
e.text = key_node.text
+ elif (
+ s_node.get('type') == 'container' and
+ s_node.get('presence') != 'true' and
+ self.preferred_delete == 'delete'
+ ):
+ self.set_delete_operation(child_self, child_other)
+ else:
+ child_self.set(operation_tag, self.preferred_delete)
for child_self, child_other in in_s_and_in_o:
s_node = self.device.get_schema_node(child_self)
if s_node.get('type') == 'leaf':
@@ -1246,16 +1273,74 @@ def set_create_operation(self, node):
'''
schema_node = self.device.get_schema_node(node)
+
+ # Create operation on non-presence containers is not allowed as per
+ # ConfD implementation although the expected behavior is ambiguous in
+ # RFC7950. More discussion can be found in the Tail-F ticket PS-47089.
if (
schema_node.get('type') == 'container' and
- schema_node.get('presence') != 'true' and
- len(self.device.default_in_use(schema_node)) > 0
+ schema_node.get('presence') != 'true'
):
+ default_xpaths = [self.device.get_xpath(n)
+ for n in self.device.default_in_use(schema_node)]
for child in node:
- self.set_create_operation(child)
+ child_xpath = self.device.get_xpath(
+ self.device.get_schema_node(child))
+ if child_xpath not in default_xpaths:
+ self.set_create_operation(child)
else:
node.set(operation_tag, 'create')
+ def set_delete_operation(self, node, reference_node):
+ '''set_delete_operation
+ Low-level api: Set the `operation` attribute of a node to `delete` when
+ it is not already set. This method is used when the preferred_delete is
+ `delete`.
+ Parameters
+ ----------
+ node : `Element`
+ A config node in a config tree. `delete` operation will be set on
+ descendants of this node.
+ reference_node : `Element`
+ A config node in a config tree as a reference when building the
+ input parameter `node`. This node has descendants that `node` does
+ not have.
+ Returns
+ -------
+ None
+ There is no return of this method.
+ '''
+
+ # Delete operation on non-presence containers is allowed even there is
+ # nothing inside the non-presence container as per ConfD implementation
+ # although the expected behavior is ambiguous in RFC7950. More
+ # discussion can be found in the Tail-F ticket PS-47089. In negative
+ # testing case, if we want the delete operation to be rejected when
+ # there is nothing inside the non-presence container, we have to put
+ # delete operations on the descendants of the non-presence container,
+ # which are not non-presence containers.
+ for reference_child in reference_node:
+ child = etree.SubElement(node, reference_child.tag,
+ nsmap=reference_child.nsmap)
+ schema_node = self.device.get_schema_node(reference_child)
+ if schema_node.get('type') == 'leaf-list':
+ child.set(operation_tag, 'delete')
+ self._merge_text(reference_child, child)
+ elif schema_node.get('type') == 'list':
+ child.set(operation_tag, 'delete')
+ keys = self._get_list_keys(schema_node)
+ for key in keys:
+ key_node = reference_child.find(key)
+ e = etree.SubElement(child, key, nsmap=key_node.nsmap)
+ e.text = key_node.text
+ elif (
+ schema_node.get('type') == 'container' and
+ schema_node.get('presence') != 'true'
+ ):
+ self.set_delete_operation(child, reference_child)
+ else:
+ child.set(operation_tag, 'delete')
+
@staticmethod
def _url_to_prefix(node, id):
'''_url_to_prefix
diff --git a/ncdiff/src/yang/ncdiff/runningconfig.py b/ncdiff/src/yang/ncdiff/runningconfig.py
index ef953bad..0423dbee 100755
--- a/ncdiff/src/yang/ncdiff/runningconfig.py
+++ b/ncdiff/src/yang/ncdiff/runningconfig.py
@@ -105,6 +105,7 @@
(re.compile(r'^ *ospfv3 neighbor '), 1),
(re.compile(r'^ *ospfv3 \d+ ipv(\d) neighbor'), 1),
(re.compile(r'^ *ospfv3 \d+ neighbor '), 1),
+ (re.compile(r'^ *member vni '), 1),
(re.compile(r'^ *ipv6 route '), 0),
(re.compile(r'^ *ip route '), 0),
]
@@ -181,6 +182,7 @@
('exit-address-family', ' exit-address-family'),
]
+
class ListDiff(object):
'''ListDiff
diff --git a/ncdiff/src/yang/ncdiff/tests/test_ncdiff.py b/ncdiff/src/yang/ncdiff/tests/test_ncdiff.py
index 33c1aa05..f8ad7c73 100755
--- a/ncdiff/src/yang/ncdiff/tests/test_ncdiff.py
+++ b/ncdiff/src/yang/ncdiff/tests/test_ncdiff.py
@@ -484,7 +484,7 @@ def test_delta_1(self):
GigabitEthernet1/0/1
-
+
@@ -494,6 +494,7 @@ def test_delta_1(self):
delta1 = config2 - config1
delta2 = config1 - config2
self.assertEqual(str(delta1).strip(), expected_delta1.strip())
+ delta2.preferred_delete = "remove"
self.assertEqual(str(delta2).strip(), expected_delta2.strip())
def test_delta_2(self):
@@ -1497,6 +1498,172 @@ def test_delta_12(self):
f"but got the delta {delta.nc} instead.",
)
+ def test_delta_13(self):
+ xml1 = """
+
+
+
+
+ """
+ xml2 = """
+
+
+
+
+ true
+
+
+
+
+ """
+ config1 = Config(self.d, xml1)
+ config2 = Config(self.d, xml2)
+ delta1 = config2 - config1
+ delta1.preferred_create = "create"
+ verification = [
+ # Create operation at tracking or logging, which are non-presence
+ # containers, is not allowed as per confd implementation although
+ # the expected behavior is ambiguous in RFC7950. More discussion
+ # can be found in the Tail-F ticket PS-47089.
+ (delta1, "/nc:config/jon:tracking/jon:logging/jon:local"),
+ ]
+ for delta, xpath in verification:
+ nodes = delta.nc.xpath(
+ xpath,
+ namespaces=delta.ns)
+ self.assertEqual(
+ len(nodes),
+ 1,
+ f"Expected to find xpath '{xpath}' in delta "
+ f"but the delta is {delta.nc}",
+ )
+ for node in nodes:
+ self.assertEqual(
+ node.get(operation_tag),
+ "create",
+ f"Expected 'create' operation at {xpath} "
+ f"but got the delta {delta.nc} instead.",
+ )
+
+ def test_delta_14(self):
+ xml1 = """
+
+
+
+
+ Calgary
+
+
+
+
+ """
+ xml2 = """
+
+
+
+
+ Calgary
+
+
+
+ ON
+
+
+
+
+
+ """
+ config1 = Config(self.d, xml1)
+ config2 = Config(self.d, xml2)
+ delta = config2 - config1
+ delta.preferred_create = "create"
+ verification = [
+ # Create operation at "other-info" is not allowed as it is a NP
+ # container.
+ "/nc:config/jon:location/jon:other-info",
+
+ # Create operation at "geo-facts" is not allowed as it is a NP
+ # container.
+ "/nc:config/jon:location/jon:other-info/jon:geo-facts",
+
+ # Create operation at "code" is not allowed as it has default in
+ # use.
+ "/nc:config/jon:location/jon:other-info/jon:geo-facts/jon:code",
+ ]
+ for xpath in verification:
+ nodes = delta.nc.xpath(
+ xpath,
+ namespaces=delta.ns)
+ self.assertEqual(
+ len(nodes),
+ 1,
+ f"Expected to find xpath '{xpath}' in delta "
+ f"but the delta is {delta}",
+ )
+ for node in nodes:
+ self.assertIsNone(
+ node.get(operation_tag),
+ f"Expected there is no 'create' operation at {xpath} "
+ f"but got the delta {delta} instead.",
+ )
+
+ def test_delta_15(self):
+ xml1 = """
+
+
+
+
+ Calgary
+
+
+
+
+ """
+ xml2 = """
+
+
+
+
+ Calgary
+
+
+
+ ON
+
+
+
+
+
+ """
+ config1 = Config(self.d, xml1)
+ config2 = Config(self.d, xml2)
+ delta = config1 - config2
+ verification = [
+ # Delete operation at "other-info" is not ideal as it is a NP
+ # container.
+ "/nc:config/jon:location/jon:other-info",
+
+ # Delete operation at "geo-facts" is not ideal as it is a NP
+ # container.
+ "/nc:config/jon:location/jon:other-info/jon:geo-facts",
+ ]
+ for xpath in verification:
+ nodes = delta.nc.xpath(
+ xpath,
+ namespaces=delta.ns)
+ self.assertEqual(
+ len(nodes),
+ 1,
+ f"Expected to find xpath '{xpath}' in delta "
+ f"but the delta is {delta}",
+ )
+ for node in nodes:
+ self.assertIsNone(
+ node.get(operation_tag),
+ f"Expected there is no 'delete' operation at {xpath} "
+ f"but got the delta {delta} instead.",
+ )
+
def test_delta_replace_1(self):
config_xml1 = """
@@ -1695,9 +1862,9 @@ def test_delta_2(self):
10
-
+
-
+
@@ -1708,6 +1875,7 @@ def test_delta_2(self):
delta1 = config2 - config1
delta2 = -delta1
self.assertEqual(str(delta1).strip(), expected_delta1.strip())
+ delta2.preferred_delete = "remove"
self.assertEqual(str(delta2).strip(), expected_delta2.strip())
def test_delta_3(self):
@@ -2247,7 +2415,7 @@ def test_delta_7(self):
5
-
+
@@ -2281,6 +2449,7 @@ def test_delta_7(self):
delta6 = config2 - config3
self.assertEqual(str(delta1).strip(), expected_delta1.strip())
self.assertEqual(str(delta2).strip(), expected_delta2.strip())
+ delta3.preferred_delete = "remove"
self.assertEqual(str(delta3).strip(), expected_delta3.strip())
self.assertEqual(str(delta4).strip(), expected_delta2.strip())
self.assertEqual(str(delta5).strip(), expected_delta4.strip())
diff --git a/ncdiff/src/yang/ncdiff/tests/yang/jon.yang b/ncdiff/src/yang/ncdiff/tests/yang/jon.yang
index 2c7654e9..a05bea44 100644
--- a/ncdiff/src/yang/ncdiff/tests/yang/jon.yang
+++ b/ncdiff/src/yang/ncdiff/tests/yang/jon.yang
@@ -69,7 +69,7 @@ module jon {
leaf-list store {
ordered-by user;
- default "Target";
+ type string;
}
container location {