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

Fixed: [If services add duplicate entries, controller does not detect this](https://github.com/clicon/clixon-controller/issues/107)

Browse source 
Rename function `xml_yang_minmax_recurse()` -> `xml_yang_validate_minmax()`
This commit is contained in:
Olof hagsand 2024-03-05 15:39:26 +01:00
parent a1badc312e
commit 6b8f7754b8
9 changed files with 382 additions and 38 deletions
Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -57,13 +57,13 @@ Users may have to change how they access the system
### C/CLI-API changes on existing features
Developers may need to change their code
* Rename function `xml_yang_minmax_recurse()` -> `xml_yang_validate_minmax()`
* Modified msg functions for clearer NETCONF 1.0 vs 1.1 API:
* `clicon_rpc1` --> `clixon_rpc10`
* `clicon_msg_send1` --> `clixon_msg_send10`
* `clicon_msg_rcv` and `clicon_msg_decode` --> `clixon_msg_rcv11`
* Rewrite by calling `clixon_msg_rcv11` and explicit xml parsing
* `clicon_msg_rcv1` --> `clixon_msg_rcv10`
* Added `yspec` parameter to `api_path_fmt2api_path()`:
* `api_path_fmt2api_path(af, c, a, c)` --> `api_path_fmt2api_path(af, c, yspec, a, c)`
* Added flags parameter to default functions:
@ -109,6 +109,7 @@ Developers may need to change their code
### Corrected Bugs
* Fixed: [If services add duplicate entries, controller does not detect this](https://github.com/clicon/clixon-controller/issues/107)
* Fixed: [Problems with diff of YANG lists ordered-by user](https://github.com/clicon/clixon/issues/496)
* Fixed: [show compare does not show correct diff while load merge xml](https://github.com/clicon/clixon-controller/issues/101)
* Fixed: [commit goes 2 times](https://github.com/clicon/clixon/issues/488)

6
apps/backend/backend_client.c
View file

@ -555,10 +555,12 @@ from_client_edit_config(clixon_handle h,
}
/* Limited validation of incoming payload
*/
if ((ret = xml_yang_minmax_recurse(xc, 1, &xret)) < 0)
if ((ret = xml_yang_validate_minmax(xc, 1, &xret)) < 0)
goto done;
if (ret == 1 && (ret = xml_yang_validate_unique_recurse(xc, &xret)) < 0)
goto done;
/* xmldb_put (difflist handling) requires list keys */
if (ret==1 && (ret = xml_yang_validate_list_key_only(xc, &xret)) < 0)
if (ret == 1 && (ret = xml_yang_validate_list_key_only(xc, &xret)) < 0)
goto done;
if (ret == 0){
if (clixon_xml2cbuf(cbret, xret, 0, 0, NULL, -1, 0) < 0)

5
lib/clixon/clixon_validate_minmax.h
View file

@ -43,6 +43,9 @@
/*
* Prototypes
*/
int xml_yang_minmax_recurse(cxobj *xt, int recurse, cxobj **xret);
int xml_yang_validate_minmax(cxobj *xt, int presence, cxobj **xret);
int xml_yang_validate_minmax_recurse(cxobj *xt, cxobj **xret);
int xml_yang_validate_unique(cxobj *xt, cxobj **xret);
int xml_yang_validate_unique_recurse(cxobj *xt, cxobj **xret);
#endif /* _CLIXON_VALIDATE_MINMAX_H_ */

2
lib/src/clixon_netconf_lib.c
View file

@ -1474,7 +1474,7 @@ netconf_data_not_unique_xml(cxobj **xret,
/* error-info: <non-unique> Contains an instance identifier that points to a leaf
* that invalidates the "unique" constraint. This element is present once for each
* non-unique leaf. */
if (cvec_len(cvk)){
if (cvk && cvec_len(cvk)){
if ((xinfo = xml_new("error-info", xerr, CX_ELMNT)) == NULL)
goto done;
if (xml2xpath(x, NULL, 0, 0, &path) < 0)

3
lib/src/clixon_plugin.c
View file

@ -1288,6 +1288,9 @@ rpc_callback_call(clixon_handle h,
nr++;
if (clixon_resource_check(h, &wh, rc->rc_name, __FUNCTION__) < 0)
goto done;
/* Ensure only one reply: first wins */
if (cbuf_len(cbret) > 0)
break;
}
rc = NEXTQ(rpc_callback_t *, rc);
} while (rc != ms->ms_rpc_callbacks);

4
lib/src/clixon_validate.c
View file

@ -1383,7 +1383,7 @@ xml_yang_validate_all(clixon_handle h,
/* Check unique and min-max after choice test for example*/
if (yang_config(yt) != 0){
/* Checks if next level contains any unique list constraints */
if ((ret = xml_yang_minmax_recurse(xt, 1, xret)) < 0)
if ((ret = xml_yang_validate_minmax(xt, 1, xret)) < 0)
goto done;
if (ret == 0)
goto fail;
@ -1422,7 +1422,7 @@ xml_yang_validate_all_top(clixon_handle h,
if ((ret = xml_yang_validate_all(h, x, xret)) < 1)
return ret;
}
if ((ret = xml_yang_minmax_recurse(xt, 0, xret)) < 1)
if ((ret = xml_yang_validate_minmax(xt, 0, xret)) < 1)
return ret;
return 1;
}

261
lib/src/clixon_validate_minmax.c
View file

@ -85,12 +85,10 @@
* @param[in] i1 The new entry is placed at vec[i1]
* @param[in] vlen Length of entry
* @param[in] sorted Sorted by system, ie sorted by key, otherwise no assumption
* @retval 0 OK, entry is unique
* @retval -1 Duplicate detected
* @note This is currently quadratic complexity. It could be improved by inserting new element sorted and binary search.
* @retval 1 Validation OK
* @retval 0 Validation failed (cbret set)
* @retval -1 Error
* @note This is currently quadratic complexity. It could be improved by inserting new element sorted and binary search.
*/
static int
unique_search_xpath(cxobj *x,
@ -385,7 +383,7 @@ check_unique_list(cxobj *x,
goto done;
}
/*! Given a list, check if any min/max-elemants constraints apply
/*! Given a list or leaf-list, check if any min/max-elemants constraints apply
*
* @param[in] xp Parent of the xml list there are too few/many (for error)
* @param[in] y Yang spec of the failing list
@ -395,7 +393,7 @@ check_unique_list(cxobj *x,
* @retval 0 Validation failed (cbret set)
* @retval -1 Error
* @see RFC7950 7.7.5
* @note No recurse for non-presence container is made, see eg xml_yang_minmax_recurse
* @note No recurse for non-presence container is made, see eg xml_yang_minmax
*/
static int
check_minmax(cxobj *xp,
@ -480,8 +478,18 @@ check_empty_list_minmax(cxobj *xt,
goto done;
}
/*! Check duplicates/unique in list
*
* @param[in] x XML LIST node
* @param[in] xt XML parent
* @param[in] y YANG of x
* @param[out] xret Error as XML if ret = 0
* @retval 1 Validation OK
* @retval 0 Validation failed (xret set)
* @retval -1 Error
*/
static int
xml_yang_minmax_newlist(cxobj *x,
xml_yang_minmax_new_list(cxobj *x,
cxobj *xt,
yang_stmt *y,
cxobj **xret)
@ -523,6 +531,55 @@ xml_yang_minmax_newlist(cxobj *x,
goto done;
}
/*! Check duplicates in leaf-list
*
* @param[in] x0 XML LIST node
* @param[in] xt XML parent
* @param[in] y0 YANG of x0
* @param[out] xret Error as XML if ret = 0
* @retval 1 Validation OK
* @retval 0 Validation failed (xret set)
* @retval -1 Error
* @note works for both ordered-by user and system. But worst case quadratic
*/
static int
xml_yang_minmax_new_leaf_list(cxobj *x0,
cxobj *xt,
yang_stmt *y0,
cxobj **xret)
{
int retval = -1;
cxobj *xi;
cxobj *xj;
char *bi;
char *bj;
xi = x0;
do {
if ((bi = xml_body(xi)) == NULL)
continue;
xj = xi;
while ((xj = xml_child_each(xt, xj, CX_ELMNT)) != NULL &&
xml_spec(xj) == y0) {
if ((bj = xml_body(xj)) == NULL)
continue;
if (bi && bj && strcmp(bi, bj) == 0){
if (xret && netconf_data_not_unique_xml(xret, xi, NULL) < 0)
goto done;
goto fail;
}
}
}
while ((xi = xml_child_each(xt, xi, CX_ELMNT)) != NULL &&
xml_spec(xi) == y0);
retval = 1;
done:
return retval;
fail:
retval = 0;
goto done;
}
/*! Perform gap analysis in a child-vector interval [ye,y]
*
* Gap analysis here meaning if there is a list x with min-element constraint but there are no
@ -581,7 +638,7 @@ xml_yang_minmax_gap_analysis(cxobj *xt,
goto done;
}
/*! Recursive minmax check
/*! YANG Minmax check, no recursion
*
* Assume xt:s children are sorted and yang populated.
* The function does two different things of the children of an XML node:
@ -637,7 +694,7 @@ xml_yang_minmax_gap_analysis(cxobj *xt,
* 7 null nolist neq gap analysis; nopresence-check;
* 8 list nolist neq gap analysis; yprev: check-minmax; nopresence-check;
* @param[in] xt XML parent (may have lists w unique constraints as child)
* @param[in] recurse Set if called in a recursive loop (will recurse anyway),
* @param[in] presence Set if called in a recursive loop (the caller will recurse anyway),
* otherwise non-presence containers will be traversed
* @param[out] xret Error XML tree. Free with xml_free after use
* @retval 1 Validation OK
@ -646,10 +703,11 @@ xml_yang_minmax_gap_analysis(cxobj *xt,
* Note that many checks, ie all except gap analysis, may be unnecessary if this fn
* is called in a recursive environment, since the recursion being made here will
* be made in that environment anyway and thus leading to double checks.
* @see xml_yang_validate_unique Sunset of unique tests
*/
int
xml_yang_minmax_recurse(cxobj *xt,
int recurse,
xml_yang_validate_minmax(cxobj *xt,
int presence,
cxobj **xret)
{
int retval = -1;
@ -686,10 +744,18 @@ xml_yang_minmax_recurse(cxobj *xt,
}
nr=1;
/* new list check */
if (ret &&
keyw == Y_LIST)
if ((ret = xml_yang_minmax_newlist(x, xt, y, xret)) < 0)
if (ret){
if (keyw == Y_LIST){
if ((ret = xml_yang_minmax_new_list(x, xt, y, xret)) < 0)
goto done;
}
#ifdef NOTYET /* XXX This is enforced in xml_yang_validate_unique instead */
else if (keyw == Y_LEAF_LIST){
if ((ret = xml_yang_minmax_new_leaf_list(x, xt, y, xret)) < 0)
goto done;
}
#endif
}
if (ret == 0)
goto fail;
yprev = y;
@ -716,11 +782,11 @@ xml_yang_minmax_recurse(cxobj *xt,
}
if (ret == 0)
goto fail;
if (recurse && keyw == Y_CONTAINER &&
if (presence && keyw == Y_CONTAINER &&
yang_find(y, Y_PRESENCE, NULL) == NULL){
yang_stmt *yc = NULL;
while ((yc = yn_each(y, yc)) != NULL) {
if ((ret = xml_yang_minmax_recurse(x, recurse, xret)) < 0)
if ((ret = xml_yang_validate_minmax(x, presence, xret)) < 0)
goto done;
if (ret == 0)
goto fail;
@ -765,3 +831,168 @@ xml_yang_minmax_recurse(cxobj *xt,
retval = 0;
goto done;
}
/*! Recursive minmax check
*
* Callback function type for xml_apply
* @param[in] x XML node
* @param[in] arg General-purpose argument
* @retval 2 Locally abort this subtree, continue with others
* @retval 1 Abort, dont continue with others, return 1 to end user
* @retval 0 OK, continue
* @retval -1 Error, aborted at first error encounter, return -1 to end user
*/
static int
xml_yang_minmax_apply(cxobj *x,
void *arg)
{
int ret;
cxobj **xret = (cxobj **)arg;
if ((ret = xml_yang_validate_minmax(x, 1, xret)) < 0)
return -1;
if (ret == 0){ /* Validation failed (xret set) */
return 1; /* Abort dont continue */
}
return 0;
}
/*! Recursive YANG Minmax check
*
* @param[in] xt XML parent (may have lists w unique constraints as child)
* @param[out] xret Error XML tree. Free with xml_free after use
* @retval 1 Validation OK
* @retval 0 Validation failed (xret set)
* @retval -1 Error
*/
int
xml_yang_validate_minmax_recurse(cxobj *xt,
cxobj **xret)
{
int retval = -1;
int ret;
if ((ret = xml_apply0(xt, CX_ELMNT, xml_yang_minmax_apply, xret)) < 0)
goto done;
if (ret == 1)
goto fail;
retval = 1;
done:
return retval;
fail:
retval = 0;
goto done;
}
/*! YANG unique check, no recursion
*
* Assume xt:s children are sorted and yang populated.
* @param[in] xt XML parent (may have lists w unique constraints as child)
* @param[out] xret Error XML tree. Free with xml_free after use
* @retval 1 Validation OK
* @retval 0 Validation failed (xret set)
* @retval -1 Error
* @see xml_yang_validate_minmax which include these unique tests
*/
int
xml_yang_validate_unique(cxobj *xt,
cxobj **xret)
{
int retval = -1;
cxobj *x = NULL;
yang_stmt *y;
yang_stmt *yprev = NULL;
enum rfc_6020 keyw;
int nr = 0;
int ret;
while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL){
if ((y = xml_spec(x)) == NULL)
continue;
keyw = yang_keyword_get(y);
if (keyw == Y_LIST || keyw == Y_LEAF_LIST){
/* equal: just continue*/
if (y == yprev){
nr++;
continue;
}
nr=1;
/* new list check */
switch (keyw){
case Y_LIST:
if ((ret = xml_yang_minmax_new_list(x, xt, y, xret)) < 0)
goto done;
if (ret == 0)
goto fail;
break;
case Y_LEAF_LIST:
if ((ret = xml_yang_minmax_new_leaf_list(x, xt, y, xret)) < 0)
goto done;
if (ret == 0)
goto fail;
break;
default:
break;
}
yprev = y;
}
}
retval = 1;
done:
return retval;
fail:
retval = 0;
goto done;
}
/*! Recursive unique check
*
* Callback function type for xml_apply
* @param[in] x XML node
* @param[in] arg General-purpose argument
* @retval 2 Locally abort this subtree, continue with others
* @retval 1 Abort, dont continue with others, return 1 to end user
* @retval 0 OK, continue
* @retval -1 Error, aborted at first error encounter, return -1 to end user
*/
static int
xml_yang_unique_apply(cxobj *x,
void *arg)
{
int ret;
cxobj **xret = (cxobj **)arg;
if ((ret = xml_yang_validate_unique(x, xret)) < 0)
return -1;
if (ret == 0){ /* Validation failed (xret set) */
return 1; /* Abort dont continue */
}
return 0;
}
/*! Recursive YANG unique check
*
* @param[in] xt XML parent (may have lists w unique constraints as child)
* @param[out] xret Error XML tree. Free with xml_free after use
* @retval 1 Validation OK
* @retval 0 Validation failed (xret set)
* @retval -1 Error
*/
int
xml_yang_validate_unique_recurse(cxobj *xt,
cxobj **xret)
{
int retval = -1;
int ret;
if ((ret = xml_apply0(xt, CX_ELMNT, xml_yang_unique_apply, xret)) < 0)
goto done;
if (ret == 1)
goto fail;
retval = 1;
done:
return retval;
fail:
retval = 0;
goto done;
}

104
test/test_netconf_duplicate.sh Executable file
View file

@ -0,0 +1,104 @@
#!/usr/bin/env bash
# Detect duplicates in incoming edit-configs (not top-level)
# Both list and leaf-list
# See https://github.com/clicon/clixon-controller/issues/107
# Magic line must be first in script (see README.md)
s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
APPNAME=example
cfg=$dir/conf_yang.xml
fyang=$dir/unique.yang
cat <<EOF > $cfg
<clixon-config xmlns="http://clicon.org/config">
<CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
<CLICON_YANG_DIR>${YANG_INSTALLDIR}</CLICON_YANG_DIR>
<CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>
<CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
<CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
<CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
<CLICON_SOCK>/usr/local/var/run/$APPNAME.sock</CLICON_SOCK>
<CLICON_BACKEND_PIDFILE>/usr/local/var/run/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
</clixon-config>
EOF
# Example (the list server part) from RFC7950 Sec 7.8.3.1 w changed types
cat <<EOF > $fyang
module unique{
yang-version 1.1;
namespace "urn:example:clixon";
prefix un;
container c{
presence "trigger"; // force presence container to trigger error
list server {
description "";
key "name";
leaf name {
type string;
}
leaf value {
type string;
}
}
leaf-list b{
type string;
}
}
}
EOF
new "test params: -f $cfg"
if [ $BE -ne 0 ]; then
new "kill old backend"
sudo clixon_backend -zf $cfg
if [ $? -ne 0 ]; then
err
fi
new "start backend -s init -f $cfg"
# start new backend
start_backend -s init -f $cfg
fi
new "wait backend"
wait_backend
if false; then
new "Add list with duplicate"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><default-operation>replace</default-operation><config><c xmlns=\"urn:example:clixon\">
<server>
<name>one</name>
<value>foo</value>
</server>
<server>
<name>one</name>
<value>foo</value>
</server>
</c></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><rpc-error><error-type>application</error-type><error-tag>operation-failed</error-tag><error-app-tag>data-not-unique</error-app-tag><error-severity>error</error-severity><error-info><non-unique xmlns=\"urn:ietf:params:xml:ns:yang:1\">/rpc/edit-config/config/c/server[name=\"one\"]/name</non-unique></error-info></rpc-error></rpc-reply>"
fi
new "Add leaf-list with duplicate"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><default-operation>replace</default-operation><config><c xmlns=\"urn:example:clixon\">
<b>one</b>
<b>two</b>
<b>one</b>
</c></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><rpc-error><error-type>application</error-type><error-tag>operation-failed</error-tag><error-app-tag>data-not-unique</error-app-tag><error-severity>error</error-severity></rpc-error></rpc-reply>"
if [ $BE -ne 0 ]; then
new "Kill backend"
# Check if premature kill
pid=$(pgrep -u root -f clixon_backend)
if [ -z "$pid" ]; then
err "backend already dead"
fi
# kill backend
stop_backend -f $cfg
fi
rm -rf $dir
new "endtest"
endtest

2
test/test_unique.sh
View file

@ -94,7 +94,7 @@ fi
new "wait backend"
wait_backend
# RFC test two-field caes
# RFC test two-field case
new "Add not valid example"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><default-operation>replace</default-operation><config><c xmlns=\"urn:example:clixon\"><server>
<name>smtp</name>