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

Added decriptive error message when plugins produce invalid state XML.

Browse source
This commit is contained in:
Olof hagsand 2020-05-10 17:10:07 +02:00
parent 266e5581e4
commit bf04131e54
11 changed files with 156 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -57,6 +57,8 @@ Expected: May 2020
### Minor changes
* Added decriptive error message when plugins produce invalid state XML.
* Example: `<error-tag>operation-failed</error-tag><error-info><bad-element>mystate</bad-element></error-info><error-message>No such yang module. Internal error, state callback returned invalid XML: example_backend</error-message>`
* Added option `CLICON_YANG_UNKNOWN_ANYDATA` to treat unknown XML (wrt YANG) as anydata.
* This is a way to loosen sanity checks if you need to accept eg unsynchronized YANG and XML
* Compile-time option: `USE_CLIGEN44` for running clixon-45 with cligen-44.

19
apps/backend/backend_client.c
View file

@ -284,7 +284,7 @@ clixon_stats_get_db(clicon_handle h,
if (xml_stats(xt, &nr, &sz) < 0)
goto done;
cprintf(cb, "<datastore><name>%s</name><nr>%" PRIu64 "</nr>"
"<size>%" PRIu64 "</size></datastore>",
"<size>%zu</size></datastore>",
dbname, nr, sz);
}
retval = 0;
@ -977,8 +977,6 @@ from_client_get(clicon_handle h,
yang_stmt *yspec;
int i;
cxobj *xerr = NULL;
cxobj *xr;
cxobj *xb;
int ret;
clicon_debug(1, "%s", __FUNCTION__);
@ -1064,21 +1062,16 @@ from_client_get(clicon_handle h,
*/
if ((ret = xml_yang_validate_all_top(h, xret, &xerr)) < 0)
goto done;
if (ret > 0 && (ret = xml_yang_validate_add(h, xret, &xerr)) < 0)
if (ret > 0 &&
(ret = xml_yang_validate_add(h, xret, &xerr)) < 0)
goto done;
if (ret == 0){
if (debug)
clicon_log_xml(LOG_DEBUG, xret, "VALIDATE_STATE");
if ((xr = xpath_first(xerr, NULL, "//error-tag")) != NULL &&
(xb = xml_body_get(xr))){
if (xml_value_set(xb, "operation-failed") < 0)
if (clixon_netconf_internal_error(xerr,
". Internal error, state callback returned invalid XML",
NULL) < 0)
goto done;
}
if ((xr = xpath_first(xerr, NULL, "//error-message")) != NULL &&
(xb = xml_body_get(xr))){
if (xml_value_append(xb, " Internal error, state callback returned invalid XML") < 0)
goto done;
}
if (clicon_xml2cbuf(cbret, xerr, 0, 0, -1) < 0)
goto done;
goto ok;

15
apps/backend/backend_plugin.c
View file

@ -230,6 +230,7 @@ clixon_plugin_statedata_all(clicon_handle h,
cxobj *x = NULL;
clixon_plugin *cp = NULL;
cbuf *cberr = NULL;
cxobj *xerr = NULL;
clicon_debug(1, "%s", __FUNCTION__);
while ((cp = clixon_plugin_each(h, cp)) != NULL) {
@ -249,8 +250,18 @@ clixon_plugin_statedata_all(clicon_handle h,
clicon_log_xml(LOG_DEBUG, x, "%s STATE:", __FUNCTION__);
#endif
/* XXX: ret == 0 invalid yang binding should be handled as internal error */
if (xml_bind_yang(x, YB_MODULE, yspec, NULL) < 0)
if ((ret = xml_bind_yang(x, YB_MODULE, yspec, &xerr)) < 0)
goto done;
if (ret == 0){
if (clixon_netconf_internal_error(xerr,
". Internal error, state callback returned invalid XML: ",
cp->cp_name) < 0)
goto done;
xml_free(*xret);
*xret = xerr;
xerr = NULL;
goto fail;
}
if (xml_sort_recurse(x) < 0)
goto done;
if (xml_default_recurse(x) < 0)
@ -266,6 +277,8 @@ clixon_plugin_statedata_all(clicon_handle h,
} /* while plugin */
retval = 1;
done:
if (xerr)
xml_free(xerr);
if (cberr)
cbuf_free(cberr);
if (x)

12
example/main/example_backend.c
View file

@ -364,13 +364,23 @@ example_statedata(clicon_handle h,
#endif
}
else{
cxobj *x1;
if ((fd = open(_state_file, O_RDONLY)) < 0){
clicon_err(OE_UNIX, errno, "open(%s)", _state_file);
goto done;
}
if (clixon_xml_parse_file(fd, YB_MODULE, yspec, NULL, &xstate, NULL) < 0)
if (clixon_xml_parse_file(fd, YB_MODULE, yspec, NULL, &xt, NULL) < 0)
goto done;
close(fd);
if ((x1 = xpath_first(xt, nsc, "%s", xpath)) != NULL){
xml_flag_set(x1, XML_FLAG_MARK);
/* Remove everything that is not marked */
if (xml_tree_prune_flagged_sub(xt, XML_FLAG_MARK, 1, NULL) < 0)
goto done;
xml_flag_reset(x1, XML_FLAG_MARK);
if (xml_copy(xt, xstate) < 0)
goto done;
}
}
}
else {

2
lib/clixon/clixon_netconf_lib.h
View file

@ -107,7 +107,7 @@ const netconf_content netconf_content_str2int(char *str);
const char *netconf_content_int2str(netconf_content nr);
int netconf_hello_server(clicon_handle h, cbuf *cb, uint32_t session_id);
int netconf_hello_req(clicon_handle h, cbuf *cb);
int clixon_netconf_error_fn(const char *fn, const int line, cxobj *xerr, const char *fmt, const char *arg);
int clixon_netconf_internal_error(cxobj *xerr, char *msg, char *arg);
#endif /* _CLIXON_NETCONF_LIB_H */

39
lib/src/clixon_netconf_lib.c
View file

@ -1546,3 +1546,42 @@ clixon_netconf_error_fn(const char *fn,
cbuf_free(cb);
return retval;
}
/*! Add internal error info to existing netconf error message by rewriting
*
* If a eg sanity check detects errors in internal messages passing, such as from a plugin or
* in backend communication, an error is generated. However, it does not make sense to send this
* error message as-is to the requestor. Instead this function transforms the error to a more
* generic "operation-failed" error and adds info in its error message to say it is an internal error.
* If a requestor receives such an error, it will be clear that the error is internal.
* @param[in] xerr Netconf error xml tree on the form: <rpc-error>
* @param[in] msg Error message
* @param[in] arg Extra error message (consider stdarg?)
* @retval 0 OK
* @retval -1 Error
*/
int
clixon_netconf_internal_error(cxobj *xerr,
char *msg,
char *arg)
{
int retval = -1;
cxobj *xr;
cxobj *xb;
if ((xr = xpath_first(xerr, NULL, "//error-tag")) != NULL &&
(xb = xml_body_get(xr))){
if (xml_value_set(xb, "operation-failed") < 0)
goto done;
}
if ((xr = xpath_first(xerr, NULL, "//error-message")) != NULL &&
(xb = xml_body_get(xr))){
if (xml_value_append(xb, msg) < 0)
goto done;
if (arg &&xml_value_append(xb, arg) < 0)
goto done;
}
retval = 0;
done:
return retval;
}

9
lib/src/clixon_proto_client.c
View file

@ -374,6 +374,10 @@ clicon_rpc_get_config(clicon_handle h,
if ((ret = xml_bind_yang(xd, YB_MODULE, yspec, &xerr)) < 0)
goto done;
if (ret == 0){
if (clixon_netconf_internal_error(xerr,
". Internal error, backend returned invalid XML.",
NULL) < 0)
goto done;
if ((xd = xpath_first(xerr, NULL, "rpc-error")) == NULL){
clicon_err(OE_XML, ENOENT, "Expected rpc-error tag but none found(internal)");
goto done;
@ -730,7 +734,10 @@ clicon_rpc_get(clicon_handle h,
if ((ret = xml_bind_yang(xd, YB_MODULE, yspec, &xerr)) < 0)
goto done;
if (ret == 0){
assert(xerr != NULL);
if (clixon_netconf_internal_error(xerr,
". Internal error, backend returned invalid XML.",
NULL) < 0)
goto done;
if ((xd = xpath_first(xerr, NULL, "rpc-error")) == NULL){
clicon_err(OE_XML, ENOENT, "Expected rpc-error tag but none found(internal)");
goto done;

23
test/lib.sh
View file

@ -345,20 +345,23 @@ expecteq(){
# Evaluate and return
# like expecteq but partial match is OK
# Example: expectpart $(fn arg) 0 "my return"
# Example: expectpart $(fn arg) 0 "my return" -- "foo"
# - evaluated expression
# - expected command return value (0 if OK)
# - expected stdout outcome*
# - the token "--not--"
# - not expected stdout outcome*
# @note need to escape \[\]
expectpart(){
r=$?
ret=$1
retval=$2
expect=$3
# echo "r:$r"
# echo "ret:\"$ret\""
# echo "retval:$retval"
# echo "expect:$expect"
# echo "expect:\"$expect\""
if [ $r != $retval ]; then
echo -e "\e[31m\nError ($r != $retval) in Test$testnr [$testname]:"
echo -e "\e[0m:"
@ -367,16 +370,27 @@ expectpart(){
if [ -z "$ret" -a -z "$expect" ]; then
return
fi
# Loop over all variable args expect strings
# Loop over all variable args expect strings (skip first two args)
# note that "expect" var is never actually used
# Then test positive for strings, if the token --neg-- is detected, then test negative for the rest
positive=true;
let i=0;
for exp in "$@"; do
if [ $i -gt 1 ]; then
if [ "$exp" == "--not--" ]; then
positive=false;
elif [ $i -gt 1 ]; then
# echo "echo \"$ret\" | grep --null -o \"$exp"\"
match=$(echo "$ret" | grep --null -o "$exp") # XXX -EZo: -E cant handle {}
r=$?
if $positive; then
if [ $r != 0 ]; then
err "$exp" "$ret"
fi
else
if [ $r == 0 ]; then
err "not $exp" "$ret"
fi
fi
fi
let i++;
done
@ -385,7 +399,6 @@ expectpart(){
# fi
}
# Pipe stdin to command
# Arguments:
# - Command

4
test/test_leafref_state.sh
View file

@ -167,10 +167,10 @@ expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><commit/></rpc>]]>]]>" "^<rpc-reply
# Leafref wrong
new "netconf get / config+state should fail"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><get content="all"><filter type="xpath" select="/"/></get></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>operation-failed</error-tag><error-info><bad-element>x</bad-element></error-info><error-severity>error</error-severity><error-message>Leafref validation failed: No leaf x matching path /ex:sender-config/ex:name Internal error, state callback returned invalid XML</error-message></rpc-error></rpc-reply>]]>]]>$'
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><get content="all"><filter type="xpath" select="/"/></get></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>operation-failed</error-tag><error-info><bad-element>x</bad-element></error-info><error-severity>error</error-severity><error-message>Leafref validation failed: No leaf x matching path /ex:sender-config/ex:name. Internal error, state callback returned invalid XML</error-message></rpc-error></rpc-reply>]]>]]>$'
new "netconf get / state-only should fail"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><get content="nonconfig"><filter type="xpath" select="/"/></get></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>operation-failed</error-tag><error-info><bad-element>x</bad-element></error-info><error-severity>error</error-severity><error-message>Leafref validation failed: No leaf x matching path /ex:sender-config/ex:name Internal error, state callback returned invalid XML</error-message></rpc-error></rpc-reply>]]>]]>$'
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><get content="nonconfig"><filter type="xpath" select="/"/></get></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>operation-failed</error-tag><error-info><bad-element>x</bad-element></error-info><error-severity>error</error-severity><error-message>Leafref validation failed: No leaf x matching path /ex:sender-config/ex:name. Internal error, state callback returned invalid XML</error-message></rpc-error></rpc-reply>]]>]]>$'
new "netconf get / config-only ok"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><get content="config"><filter type="xpath" select="/"/></get></rpc>]]>]]>' '^<rpc-reply><data><sender-config xmlns="urn:example:example"><name>y</name></sender-config></data></rpc-reply>]]>]]>$'

1
test/test_perf_state.sh
View file

@ -83,6 +83,7 @@ module $APPNAME{
}
EOF
# Mixed config + state XML
new "generate state file with $perfnr list entries"
echo -n "<interfaces xmlns=\"urn:example:clixon\"><a><name>foo</name><b>" > $fstate
for (( i=0; i<$perfnr; i++ )); do

49
test/test_restconf_err.sh
View file

@ -15,6 +15,9 @@
# Note this is different from an api-path that is invalid from a yang point
# of view, this is interpreted as 400 Bad Request invalid-value/unknown-element
# XXX: complete non-existent yang with unknown-element for all PUT/POST/GET api-paths
#
# Also generate an invalid state XML. This should generate an "Internal" error and the name of the
# plugin should be visible in the error message.
# Magic line must be first in script (see README.md)
s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
@ -25,6 +28,7 @@ cfg=$dir/conf.xml
fyang=$dir/example.yang
fyang2=$dir/augment.yang
fxml=$dir/initial.xml
fstate=$dir/state.xml
# <CLICON_YANG_MODULE_MAIN>example</CLICON_YANG_MODULE_MAIN>
cat <<EOF > $cfg
@ -36,6 +40,8 @@ cat <<EOF > $cfg
<CLICON_RESTCONF_PRETTY>false</CLICON_RESTCONF_PRETTY>
<CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
<CLICON_BACKEND_PIDFILE>$dir/restconf.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_BACKEND_DIR>/usr/local/lib/$APPNAME/backend</CLICON_BACKEND_DIR>
<CLICON_BACKEND_REGEXP>example_backend.so$</CLICON_BACKEND_REGEXP>
<CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
<CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
<CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
@ -103,7 +109,30 @@ module example{
}
}
}
container mystate{
config false;
description "Just for generating a invalid XML";
list parameter{
key name;
leaf name{
type string;
}
leaf value{
type string;
}
}
}
}
EOF
# State file with error: wrong namespace
cat <<EOF > $fstate
<mystate xmlns="urn:example:foobar">
<parameter>
<name>x</name>
<value>x</value>
</parameter>
</mystate>
EOF
# Initial tree
@ -121,13 +150,14 @@ if [ $BE -ne 0 ]; then
err
fi
sudo pkill -f clixon_backend # to be sure
new "start backend -s init -f $cfg"
start_backend -s init -f $cfg
new "start backend -s init -f $cfg -- -sS $fstate"
start_backend -s init -f $cfg -- -sS $fstate
fi
new "waiting"
wait_backend
if [ $RC -ne 0 ]; then
new "kill old restconf daemon"
sudo pkill -u $wwwuser -f clixon_restconf
@ -136,13 +166,13 @@ start_restconf -f $cfg
new "waiting"
wait_restconf
fi
new "restconf POST initial tree"
expecteq "$(curl -s -X POST -H 'Content-Type: application/yang-data+xml' -d "$XML" http://localhost/restconf/data)" 0 ''
expectpart "$(curl -si -X POST -H 'Content-Type: application/yang-data+xml' -d "$XML" http://localhost/restconf/data)" 0 'HTTP/1.1 201 Created'
new "restconf GET initial datastore"
expecteq "$(curl -s -X GET -H 'Accept: application/yang-data+xml' http://localhost/restconf/data/example:a=0)" 0 "$XML
"
expectpart "$(curl -si -X GET -H 'Accept: application/yang-data+xml' http://localhost/restconf/data/example:a=0)" 0 'HTTP/1.1 200 OK' "$XML"
new "restconf GET non-qualified list"
expectpart "$(curl -si -X GET http://localhost/restconf/data/example:a)" 0 'HTTP/1.1 400 Bad Request' "{\"ietf-restconf:errors\":{\"error\":{\"error-type\":\"rpc\",\"error-tag\":\"malformed-message\",\"error-severity\":\"error\",\"error-message\":\"malformed key =example:a, expected '=restval'\"}}}"
@ -165,7 +195,7 @@ expectpart "$(curl -is -X POST -H 'Content-Type: application/yang-data+xml' -d "
# Test for multi-module path where an augment stretches across modules
new "restconf POST augment multi-namespace path"
expecteq "$(curl -s -X POST -H 'Content-Type: application/yang-data+xml' -d '<route-config xmlns="urn:example:aug"><dynamic><ospf xmlns="urn:example:clixon"><reference-bandwidth>23</reference-bandwidth></ospf></dynamic></route-config>' http://localhost/restconf/data)" 0 ''
expectpart "$(curl -si -X POST -H 'Content-Type: application/yang-data+xml' -d '<route-config xmlns="urn:example:aug"><dynamic><ospf xmlns="urn:example:clixon"><reference-bandwidth>23</reference-bandwidth></ospf></dynamic></route-config>' http://localhost/restconf/data)" 0 'HTTP/1.1 201 Created'
new "restconf GET augment multi-namespace top"
expectpart "$(curl -si -X GET http://localhost/restconf/data/augment:route-config)" 0 'HTTP/1.1 200 OK' '{"augment:route-config":{"dynamic":{"example:ospf":{"reference-bandwidth":23}}}}'
@ -183,8 +213,15 @@ expectpart "$(curl -si -X GET http://localhost/restconf/data/augment:route-confi
new "restconf GET augment multi-namespace, no 2nd module in api-path, fail"
expectpart "$(curl -si -X GET http://localhost/restconf/data/augment:route-config/dynamic/ospf)" 0 'HTTP/1.1 404 Not Found' '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"invalid-value","error-severity":"error","error-message":"Instance does not exist"}}}'
#----------------------------------------------
# Also generate an invalid state XML. This should generate an "Internal" error and the name of the
new "restconf GET failed state"
expectpart "$(curl -si -X GET -H 'Accept: application/yang-data+xml' http://localhost/restconf/data?content=nonconfig)" 0 '412 Precondition Failed' '<errors xmlns="urn:ietf:params:xml:ns:yang:ietf-restconf"><error><error-type>application</error-type><error-tag>operation-failed</error-tag><error-info><bad-element>mystate</bad-element></error-info><error-severity>error</error-severity><error-message>No such yang module. Internal error, state callback returned invalid XML: example_backend</error-message></error></errors>'
if [ $RC -ne 0 ]; then
new "Kill restconf daemon"
stop_restconf
fi
if [ $BE -eq 0 ]; then
exit # BE