experiments/clixon
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fixed: [Deletion of leaf in YANG choice removes leaf in a different choice/case](https://github.com/clicon/clixon/issues/542)

Browse source
This commit is contained in:
Olof hagsand 2024-08-20 10:04:46 +02:00
parent f0eadc6e32
commit 7cbc0a8dc3
3 changed files with 67 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -51,6 +51,7 @@ Developers may need to change their code
### Corrected Busg ### Corrected Busg
* Fixed: [Deletion of leaf in YANG choice removes leaf in a different choice/case](https://github.com/clicon/clixon/issues/542)
* Fixed: Deviated types were resolved in target context, not lexically in deviation context * Fixed: Deviated types were resolved in target context, not lexically in deviation context
* Fixed: Signal handling of recv message * Fixed: Signal handling of recv message
Revert to signal handling in 6.5 that was changed in the netconf uniform handling in 7.0 Revert to signal handling in 6.5 that was changed in the netconf uniform handling in 7.0

69
lib/src/clixon_datastore_write.c
View file

@ -109,6 +109,7 @@ struct xmldb_multi_write_arg {
* @param[in] x XML node (where to look for attribute) * @param[in] x XML node (where to look for attribute)
* @param[in] name Attribute name * @param[in] name Attribute name
* @param[in] ns (Expected) Namespace of attribute * @param[in] ns (Expected) Namespace of attribute
* @param[in] peek If 0 dont remove attribute
* @param[out] cbret Error message (if retval=0) * @param[out] cbret Error message (if retval=0)
* @param[out] valp Malloced value (if retval=1) * @param[out] valp Malloced value (if retval=1)
* @retval 1 OK * @retval 1 OK
@ -120,6 +121,7 @@ static int
attr_ns_value(cxobj *x, attr_ns_value(cxobj *x,
char *name, char *name,
char *ns, char *ns,
int peek,
cbuf *cbret, cbuf *cbret,
char **valp) char **valp)
{ {
@ -144,6 +146,7 @@ attr_ns_value(cxobj *x,
clixon_err(OE_UNIX, errno, "malloc"); clixon_err(OE_UNIX, errno, "malloc");
goto done; goto done;
} }
if (!peek)
xml_purge(xa); xml_purge(xa);
} }
} }
@ -373,14 +376,16 @@ choice_is_other(yang_stmt *y0c,
return 0; return 0;
} }
/*! Check if choice nodes and implicitly remove all other cases. /*! Check if choice nodes and if add implicitly remove all other cases, or fail if delete
* *
* Special case is if yc parent (yp) is choice/case * Special case is if yc parent (yp) is choice/case
* then find x0 child with same yc even though it does not match lexically * then find x0 child with same yc even though it does not match lexically
* However this will give another y0c != yc * However this will give another y0c != yc
* @param[in] x0 Base tree node * @param[in] x0 Base tree node
* @param[in] y1c Yang spec of tree child. If null revert to linear search. * @param[in] x1c Tree child
* @retval 0 OK * @param[in] y1c Yang spec of x1c
* @retval 1 OK
* @retval 0 Fail with cbret set
* @retval -1 Error * @retval -1 Error
* *
* From RFC 7950 Sec 7.9 * From RFC 7950 Sec 7.9
@ -391,8 +396,11 @@ choice_is_other(yang_stmt *y0c,
* other cases inside the choice. * other cases inside the choice.
*/ */
static int static int
choice_delete_other(cxobj *x0, choice_other_match(cxobj *x0,
yang_stmt *y1c) cxobj *x1c,
yang_stmt *y1c,
enum operation_type op,
cbuf *cbret)
{ {
int retval = -1; int retval = -1;
cxobj *x0c; cxobj *x0c;
@ -402,7 +410,16 @@ choice_delete_other(cxobj *x0,
yang_stmt *y0choice; yang_stmt *y0choice;
yang_stmt *y1case = NULL; yang_stmt *y1case = NULL;
yang_stmt *y1choice = NULL; yang_stmt *y1choice = NULL;
char *opstr = NULL;
int ret;
if ((ret = attr_ns_value(x1c, "operation", NETCONF_BASE_NAMESPACE, 1, cbret, &opstr)) < 0)
goto done;
if (ret == 0)
goto fail;
if (opstr != NULL)
if (xml_operation(opstr, &op) < 0)
goto done;
if (yang_choice_case_get(y1c, &y1case, &y1choice) == 0) if (yang_choice_case_get(y1c, &y1case, &y1choice) == 0)
goto ok; goto ok;
x0prev = NULL; x0prev = NULL;
@ -419,19 +436,37 @@ choice_delete_other(cxobj *x0,
} }
/* Check if x0/y0 is part of other choice/case than y1 recursively , if so purge */ /* Check if x0/y0 is part of other choice/case than y1 recursively , if so purge */
if (choice_is_other(y0c, y0case, y0choice, y1c, y1case, y1choice) == 1){ if (choice_is_other(y0c, y0case, y0choice, y1c, y1case, y1choice) == 1){
switch (op){
case OP_MERGE:
case OP_REPLACE:
case OP_CREATE:
if (xml_purge(x0c) < 0) if (xml_purge(x0c) < 0)
goto done; goto done;
x0c = x0prev; x0c = x0prev;
continue; continue;
break;
case OP_DELETE:
if (netconf_data_missing(cbret, "Data does not exist; cannot delete resource") < 0)
goto done;
goto fail;
break;
case OP_REMOVE:
case OP_NONE:
break;
}
} }
x0prev = x0c; x0prev = x0c;
} }
ok: ok:
retval = 0; retval = 1;
done: done:
return retval; return retval;
fail:
retval = 0;
goto done;
} }
/*! Modify a base tree x0 with x1 with yang spec y according to operation op /*! Modify a base tree x0 with x1 with yang spec y according to operation op
* *
* @param[in] h Clixon handle * @param[in] h Clixon handle
@ -506,7 +541,7 @@ text_modify(clixon_handle h,
if (ret == 0) if (ret == 0)
goto fail; goto fail;
/* Check for operations embedded in tree according to netconf */ /* Check for operations embedded in tree according to netconf */
if ((ret = attr_ns_value(x1, "operation", NETCONF_BASE_NAMESPACE, if ((ret = attr_ns_value(x1, "operation", NETCONF_BASE_NAMESPACE, 0,
cbret, &opstr)) < 0) cbret, &opstr)) < 0)
goto done; goto done;
if (ret == 0) if (ret == 0)
@ -514,7 +549,7 @@ text_modify(clixon_handle h,
if (opstr != NULL) if (opstr != NULL)
if (xml_operation(opstr, &op) < 0) if (xml_operation(opstr, &op) < 0)
goto done; goto done;
if ((ret = attr_ns_value(x1, "objectcreate", NULL, cbret, &createstr)) < 0) if ((ret = attr_ns_value(x1, "objectcreate", NULL, 0, cbret, &createstr)) < 0)
goto done; goto done;
if (ret == 0) if (ret == 0)
goto fail; goto fail;
@ -554,7 +589,7 @@ text_modify(clixon_handle h,
if (yang_keyword_get(y0) == Y_LEAF_LIST && if (yang_keyword_get(y0) == Y_LEAF_LIST &&
yang_find(y0, Y_ORDERED_BY, "user") != NULL){ yang_find(y0, Y_ORDERED_BY, "user") != NULL){
if ((ret = attr_ns_value(x1, if ((ret = attr_ns_value(x1,
"insert", YANG_XML_NAMESPACE, "insert", YANG_XML_NAMESPACE, 0,
cbret, &instr)) < 0) cbret, &instr)) < 0)
goto done; goto done;
if (ret == 0) if (ret == 0)
@ -563,7 +598,7 @@ text_modify(clixon_handle h,
xml_attr_insert2val(instr, &insert) < 0) xml_attr_insert2val(instr, &insert) < 0)
goto done; goto done;
if ((ret = attr_ns_value(x1, if ((ret = attr_ns_value(x1,
"value", YANG_XML_NAMESPACE, "value", YANG_XML_NAMESPACE, 0,
cbret, &valstr)) < 0) cbret, &valstr)) < 0)
goto done; goto done;
/* if insert/before, value attribute must be there */ /* if insert/before, value attribute must be there */
@ -752,7 +787,7 @@ text_modify(clixon_handle h,
if (yang_keyword_get(y0) == Y_LIST && if (yang_keyword_get(y0) == Y_LIST &&
yang_find(y0, Y_ORDERED_BY, "user") != NULL){ yang_find(y0, Y_ORDERED_BY, "user") != NULL){
if ((ret = attr_ns_value(x1, if ((ret = attr_ns_value(x1,
"insert", YANG_XML_NAMESPACE, "insert", YANG_XML_NAMESPACE, 0,
cbret, &instr)) < 0) cbret, &instr)) < 0)
goto done; goto done;
if (ret == 0) if (ret == 0)
@ -761,7 +796,7 @@ text_modify(clixon_handle h,
xml_attr_insert2val(instr, &insert) < 0) xml_attr_insert2val(instr, &insert) < 0)
goto done; goto done;
if ((ret = attr_ns_value(x1, if ((ret = attr_ns_value(x1,
"key", YANG_XML_NAMESPACE, "key", YANG_XML_NAMESPACE, 0,
cbret, &keystr)) < 0) cbret, &keystr)) < 0)
goto done; goto done;
@ -903,9 +938,11 @@ text_modify(clixon_handle h,
x1cname, yang_find_mynamespace(y0)); x1cname, yang_find_mynamespace(y0));
goto done; goto done;
} }
/* Check if existing choice/case should be deleted */ /* Check if existing choice/case, if so may be error or delete */
if (choice_delete_other(x0, yc) < 0) if ((ret = choice_other_match(x0, x1c, yc, op, cbret)) < 0)
goto done; goto done;
if (ret == 0)
goto fail;
/* See if there is a corresponding node in the base tree */ /* See if there is a corresponding node in the base tree */
x0c = NULL; x0c = NULL;
if (match_base_child(x0, x1c, yc, &x0c) < 0) if (match_base_child(x0, x1c, yc, &x0c) < 0)
@ -1052,7 +1089,7 @@ text_modify_top(clixon_handle h,
/* Check for operations embedded in tree according to netconf */ /* Check for operations embedded in tree according to netconf */
if ((ret = attr_ns_value(x1t, if ((ret = attr_ns_value(x1t,
"operation", NETCONF_BASE_NAMESPACE, "operation", NETCONF_BASE_NAMESPACE, 0,
cbret, &opstr)) < 0) cbret, &opstr)) < 0)
goto done; goto done;
if (ret == 0) if (ret == 0)
@ -1060,7 +1097,7 @@ text_modify_top(clixon_handle h,
if (opstr != NULL) if (opstr != NULL)
if (xml_operation(opstr, &op) < 0) if (xml_operation(opstr, &op) < 0)
goto done; goto done;
if ((ret = attr_ns_value(x1t, "objectcreate", NULL, cbret, &createstr)) < 0) if ((ret = attr_ns_value(x1t, "objectcreate", NULL, 0, cbret, &createstr)) < 0)
goto done; goto done;
if (ret == 0) if (ret == 0)
goto fail; goto fail;

11
test/test_choice_recursive.sh → test/test_choice_hierarchy.sh
View file

@ -135,16 +135,23 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS>
new "show config, only A2x" new "show config, only A2x"
expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><A2x>bbb</A2x></c>$" expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><A2x>bbb</A2x></c>$"
new "delete 1st A stmt"
expectpart "$($clixon_cli -1 -f $cfg -l o delete c A1x aaa)" 0 "^$"
new "show config, A2x remains"
expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><A2x>bbb</A2x></c>$"
new "cli set 3rd A stmt" new "cli set 3rd A stmt"
expectpart "$($clixon_cli -1 -f $cfg -l o set c Ay ccc)" 0 "^$" expectpart "$($clixon_cli -1 -f $cfg -l o set c Ay ccc)" 0 "^$"
new "show config: A2x + Ay" new "show config: both A2x + Ay"
expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><A2x>bbb</A2x><Ay>ccc</Ay></c>$" expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><A2x>bbb</A2x><Ay>ccc</Ay></c>$"
new "cli set 1st B stmt" new "cli set 1st B stmt"
expectpart "$($clixon_cli -1 -f $cfg -l o set c B1x ddd)" 0 "^$" expectpart "$($clixon_cli -1 -f $cfg -l o set c B1x ddd)" 0 "^$"
# Ay + B1x Ay should not be there
new "show config: B1x" new "show config: only B1x"
expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><B1x>ddd</B1x></c>$" expectpart "$($clixon_cli -1 -f $cfg -l o show config)" 0 "^<c xmlns=\"urn:example:config\"><B1x>ddd</B1x></c>$"
new "cli set 3rd A stmt" new "cli set 3rd A stmt"