From 8ae647c32b32e5210e30c426e6af47f8d8113a65 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 7 Oct 2020 20:57:49 +0200 Subject: [PATCH] * More explanatory validation error messages for when and augments error messages. * Example: error-message: `Mandatory variable` -> `Mandatory variable of edit-config in module ietf-netconf`. --- CHANGELOG.md | 3 +- apps/backend/backend_commit.c | 31 +++++--- lib/src/clixon_validate.c | 16 +++- test/test_augment_trans.sh | 138 ++++++++++++++++++++++++++++++++++ test/test_cli.sh | 2 +- test/test_restconf2.sh | 2 +- test/test_rpc.sh | 8 +- test/test_type.sh | 4 +- test/test_xpath_functions.sh | 6 +- 9 files changed, 184 insertions(+), 26 deletions(-) create mode 100755 test/test_augment_trans.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index a5615a7f..f1866403 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,8 @@ Users may have to change how they access the system * New clixon-config@2020-10-01.yang revision * Added option for configuration directory: `CLICON_CONFIGDIR` * Not implemented XPath functions will cause a backend exit on startup, instead of being ignored. - +* More explanatory validation error messages for when and augments error messages. + * Example: error-message: `Mandatory variable` -> `Mandatory variable of edit-config in module ietf-netconf`. ### Minor changes * Added stricter check on schema-node identifier checking, such as for augments. diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index cc1f625e..05590aec 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -93,12 +93,14 @@ generic_validate(clicon_handle h, transaction_data_t *td, cxobj **xret) { - int retval = -1; - cxobj *x1; - cxobj *x2; - yang_stmt *ys; - int i; - int ret; + int retval = -1; + cxobj *x1; + cxobj *x2; + yang_stmt *ys; + int i; + int ret; + cbuf *cb = NULL; + yang_stmt *yp; /* All entries */ if ((ret = xml_yang_validate_all_top(h, td->td_target, xret)) < 0) @@ -120,10 +122,17 @@ generic_validate(clicon_handle h, x1 = td->td_dvec[i]; ys = xml_spec(x1); if (ys && yang_mandatory(ys) && yang_config(ys)==1){ - yang_stmt *yp =yang_parent_get(ys); - if (yp== NULL || - (yang_keyword_get(yp)!=Y_MODULE && yang_keyword_get(yp)!=Y_SUBMODULE)){ - if (netconf_missing_element_xml(xret, "protocol", xml_name(x1), "May not remove mandatory variable") < 0) + yp = yang_parent_get(ys); + if (yp == NULL || + (yang_keyword_get(yp) != Y_MODULE && yang_keyword_get(yp) != Y_SUBMODULE)){ + if ((cb = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, errno, "cbuf_new"); + goto done; + } + cprintf(cb, "Mandatory variable of %s in module %s", + xml_parent(x1)?xml_name(xml_parent(x1)):"", + yang_argument_get(ys_module(ys))); + if (netconf_missing_element_xml(xret, "protocol", xml_name(x1), cbuf_get(cb)) < 0) goto done; goto fail; } @@ -140,6 +149,8 @@ generic_validate(clicon_handle h, // ok: retval = 1; done: + if (cb) + cbuf_free(cb); return retval; fail: retval = 0; diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c index 8fda734d..3861041d 100644 --- a/lib/src/clixon_validate.c +++ b/lib/src/clixon_validate.c @@ -458,6 +458,7 @@ check_mandatory(cxobj *xt, cvec *cvk = NULL; /* vector of index keys */ cg_var *cvi; char *keyname; + cbuf *cb = NULL; yc = NULL; while ((yc = yn_each(yt, yc)) != NULL) { @@ -493,8 +494,13 @@ check_mandatory(cxobj *xt, break; /* got it */ } if (x == NULL){ - if (netconf_missing_element_xml(xret, "application", yang_argument_get(yc), "Mandatory variable") < 0) + if ((cb = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, errno, "cbuf_new"); goto done; + } + cprintf(cb, "Mandatory variable of %s in module %s", xml_name(xt), yang_argument_get(ys_module(yc))); + if (netconf_missing_element_xml(xret, "application", yang_argument_get(yc), cbuf_get(cb)) < 0) + goto done; goto fail; } break; @@ -521,6 +527,8 @@ check_mandatory(cxobj *xt, } retval = 1; done: + if (cb) + cbuf_free(cb); return retval; fail: retval = 0; @@ -1151,8 +1159,7 @@ xml_yang_validate_all(clicon_handle h, /* WHEN xpath needs namespace context */ if (xml_nsctx_yang(ys, &nsc) < 0) goto done; - if ((nr = xpath_vec_bool(xt, nsc, - "%s", xpath)) < 0) + if ((nr = xpath_vec_bool(xt, nsc, "%s", xpath)) < 0) goto done; if (nsc){ xml_nsctx_free(nsc); @@ -1182,7 +1189,8 @@ xml_yang_validate_all(clicon_handle h, clicon_err(OE_UNIX, errno, "cbuf_new"); goto done; } - cprintf(cb, "Failed augmented WHEN condition of %s in module %s", + cprintf(cb, "Failed augmented WHEN condition %s of node %s in module %s", + xpath, xml_name(xt), yang_argument_get(ys_module(ys))); if (netconf_operation_failed_xml(xret, "application", diff --git a/test/test_augment_trans.sh b/test/test_augment_trans.sh new file mode 100755 index 00000000..c700b14b --- /dev/null +++ b/test/test_augment_trans.sh @@ -0,0 +1,138 @@ +#!/usr/bin/env bash +# Test augmenting in two steps and an augment accessing both +# ie aug2->aug1->base +# where the augment arg in aug2 is: "base:../aug1:.. +# This occurs in several openconfig/yangmodels models, but does not work in clixon 4.7 +# + +# 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/example-lib.yang # base +fyang1=$dir/example-augment1.yang # first augment +fyang2=$dir/example-augment2.yang # second augment + +cat < $cfg + + $cfg + a:test + $dir + /usr/local/share/clixon + $fyang2 + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/cli + $APPNAME + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + /usr/local/lib/$APPNAME/backend + false + /usr/local/var/$APPNAME + false + +EOF + +# This is the lib function with the base container +cat < $fyang +module example-lib { + yang-version 1.1; + namespace "urn:example:lib"; + revision "2019-03-04"; + prefix lib; + container base-config { + } + /* No prefix */ + augment "/base-config" { + description "no prefix"; + list parameter{ + key name; + leaf name{ + type string; + } + } + } +} +EOF + +# This is the first augment1 +cat < $fyang1 +module example-augment1 { + yang-version 1.1; + namespace "urn:example:augment1"; + prefix aug1; + revision "2020-09-25"; + import example-lib { + prefix lib; + } + /* Augments config */ + augment "/lib:base-config/lib:parameter" { + container aug1{ + description "Local augmented optional"; + } + } +} +EOF + +# This is the main module with second augment +cat < $fyang2 +module example-augment2 { + yang-version 1.1; + namespace "urn:example:augment2"; + prefix aug2; + revision "2020-09-25"; + import example-lib { + prefix lib; + } + import example-augment1 { + prefix aug1; + } + /* Augments config */ + augment "/lib:base-config/lib:parameter/aug1:aug1" { +/* when 'lib:name="foobar" and aug:aug1="foobar"'; */ + leaf aug2{ + description "Local augmented optional"; + 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_backend -s init -f $cfg +fi +new "waiting" +wait_backend + + +new "get-config empty" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + +if false; then +new "Add config" +expecteof "$clixon_netconf -qf $cfg" 0 "<>]]>]]>" "^]]>]]>" + +new "netconf commit" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + +fi + +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 + +rm -rf $dir diff --git a/test/test_cli.sh b/test/test_cli.sh index 687a06f2..0ee51e2a 100755 --- a/test/test_cli.sh +++ b/test/test_cli.sh @@ -74,7 +74,7 @@ new "cli configure using encoded chars name <&" expectfn "$clixon_cli -1 -f $cfg set interfaces interface fddi&< type ianaift:ethernetCsmacd" 0 "" new "cli failed validate" -expectpart "$($clixon_cli -1 -f $cfg -l o validate)" 255 "Validate failed. Edit and try again or discard changes: application missing-element Mandatory variable type" +expectpart "$($clixon_cli -1 -f $cfg -l o validate)" 255 "Validate failed. Edit and try again or discard changes: application missing-element Mandatory variable of interface in module ietf-interfaces type" new "cli configure ip addr" expectpart "$($clixon_cli -1 -f $cfg set interfaces interface eth/0/0 ipv4 address 1.2.3.4 prefix-length 24)" 0 "^$" diff --git a/test/test_restconf2.sh b/test/test_restconf2.sh index 769e6fa9..ece54ac1 100755 --- a/test/test_restconf2.sh +++ b/test/test_restconf2.sh @@ -120,7 +120,7 @@ new "restconf GET if-type" expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example:cont1/interface=local0/type)" 0 "HTTP/1.1 200 OK" '{"example:type":"regular"}' new "restconf POST interface without mandatory type" -expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/example:cont1 -d '{"example:interface":{"name":"TEST"}}')" 0 "HTTP/1.1 400 Bad Request" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"missing-element","error-info":{"bad-element":"type"},"error-severity":"error","error-message":"Mandatory variable"}}}' +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/example:cont1 -d '{"example:interface":{"name":"TEST"}}')" 0 "HTTP/1.1 400 Bad Request" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"missing-element","error-info":{"bad-element":"type"},"error-severity":"error","error-message":"Mandatory variable of interface in module example"}}}' new "restconf POST interface without mandatory key" expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/example:cont1 -d '{"example:interface":{"type":"regular"}}')" 0 "HTTP/1.1 400 Bad Request" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"missing-element","error-info":{"bad-element":"name"},"error-severity":"error","error-message":"Mandatory key"}}}' diff --git a/test/test_rpc.sh b/test/test_rpc.sh index 4cbff92a..7ccf6339 100755 --- a/test/test_rpc.sh +++ b/test/test_rpc.sh @@ -122,7 +122,7 @@ if [ -z "$match" ]; then fi new "restconf omit mandatory" -expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"clixon-example:input":null}' $RCPROTO://localhost/restconf/operations/clixon-example:example)" 0 'HTTP/1.1 400 Bad Request' '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"missing-element","error-info":{"bad-element":"x"},"error-severity":"error","error-message":"Mandatory variable"}}} ' +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"clixon-example:input":null}' $RCPROTO://localhost/restconf/operations/clixon-example:example)" 0 'HTTP/1.1 400 Bad Request' '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"missing-element","error-info":{"bad-element":"x"},"error-severity":"error","error-message":"Mandatory variable of example in module clixon-example"}}} ' new "restconf add extra w/o yang: should fail" if ! $YANG_UNKNOWN_ANYDATA ; then @@ -133,10 +133,10 @@ new "restconf wrong method" expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"clixon-example:input":{"x":"0"}}' $RCPROTO://localhost/restconf/operations/clixon-example:wrong)" 0 'HTTP/1.1 400 Bad Request' '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"missing-element","error-info":{"bad-element":"wrong"},"error-severity":"error","error-message":"RPC not defined"}}} ' new "restconf example missing input" -expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"clixon-example:input":null}' $RCPROTO://localhost/restconf/operations/ietf-netconf:edit-config)" 0 'HTTP/1.1 400 Bad Request' '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"missing-element","error-info":{"bad-element":"target"},"error-severity":"error","error-message":"Mandatory variable"}}} ' +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"clixon-example:input":null}' $RCPROTO://localhost/restconf/operations/ietf-netconf:edit-config)" 0 'HTTP/1.1 400 Bad Request' '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"missing-element","error-info":{"bad-element":"target"},"error-severity":"error","error-message":"Mandatory variable of edit-config in module ietf-netconf"}}} ' new "netconf kill-session missing session-id mandatory" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^applicationmissing-elementsession-iderrorMandatory variable]]>]]>$" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^applicationmissing-elementsession-iderrorMandatory variable of kill-session in module ietf-netconf]]>]]>$" new "netconf edit-config ok" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" @@ -150,7 +150,7 @@ new "netconf edit-config empty target: should fail" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^applicationdata-missingmissing-choiceconfig-targeterror]]>]]>$" new "netconf edit-config missing target: should fail" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^applicationmissing-elementtargeterrorMandatory variable]]>]]>$" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^applicationmissing-elementtargeterrorMandatory variable of edit-config in module ietf-netconf]]>]]>$" new "netconf edit-config missing config: should fail" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^applicationdata-missingmissing-choiceedit-contenterror]]>]]>$" diff --git a/test/test_type.sh b/test/test_type.sh index 056a52c6..8f70b5e5 100755 --- a/test/test_type.sh +++ b/test/test_type.sh @@ -605,7 +605,7 @@ EOF expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>" new "netconf validate should fail" - expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "applicationmissing-elementmanerrorMandatory variable]]>]]>" + expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "applicationmissing-elementmanerrorMandatory variable of manc in module example]]>]]>" new "netconf set container with mandatory leaf" expecteof "$clixon_netconf -qf $cfg" 0 "foo]]>]]>" "^]]>]]>" @@ -620,7 +620,7 @@ EOF expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" new "netconf validate should fail" - expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^protocolmissing-elementmanerrorMay not remove mandatory variable]]>]]>$" + expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^protocolmissing-elementmanerrorMandatory variable of manc in module example]]>]]>$" new "netconf discard-changes" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" diff --git a/test/test_xpath_functions.sh b/test/test_xpath_functions.sh index f7d3bd95..230d0f81 100755 --- a/test/test_xpath_functions.sh +++ b/test/test_xpath_functions.sh @@ -140,13 +140,13 @@ new "Change type to atm" expecteof "$clixon_netconf -qf $cfg -D $DBG" 0 "e0atm]]>]]>" "^]]>]]>" new "netconf validate not OK (mtu not allowed)" -expecteof "$clixon_netconf -qf $cfg" 0 "^]]>]]>" "^applicationoperation-failederrorFailed augmented WHEN condition of mtu in module example]]>]]>$" +expecteof "$clixon_netconf -qf $cfg" 0 "^]]>]]>" "^applicationoperation-failederrorFailed augmented WHEN condition derived-from(type, \"ex:ethernet\") of node mtu in module example]]>]]>$" new "Change type to ethernet (self)" expecteof "$clixon_netconf -qf $cfg -D $DBG" 0 "e0ethernet]]>]]>" "^]]>]]>" new "netconf validate not OK (mtu not allowed on self)" -expecteof "$clixon_netconf -qf $cfg" 0 "^]]>]]>" "^applicationoperation-failederrorFailed augmented WHEN condition of mtu in module example]]>]]>$" +expecteof "$clixon_netconf -qf $cfg" 0 "^]]>]]>" "^applicationoperation-failederrorFailed augmented WHEN condition derived-from(type, \"ex:ethernet\") of node mtu in module example]]>]]>$" new "netconf discard-changes" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" @@ -162,7 +162,7 @@ new "Change type to atm" expecteof "$clixon_netconf -qf $cfg -D $DBG" 0 "e0atm]]>]]>" "^]]>]]>" new "netconf validate not OK (crc not allowed)" -expecteof "$clixon_netconf -qf $cfg" 0 "^]]>]]>" "^applicationoperation-failederrorFailed augmented WHEN condition of crc in module example]]>]]>$" +expecteof "$clixon_netconf -qf $cfg" 0 "^]]>]]>" "^applicationoperation-failederrorFailed augmented WHEN condition derived-from-or-self(type, \"ex:ethernet\") of node crc in module example]]>]]>$" new "Change type to ethernet (self)" expecteof "$clixon_netconf -qf $cfg -D $DBG" 0 "e0ethernet]]>]]>" "^]]>]]>"