From 27bb0a1d2b2bc4ee1c36eaa74fbe3ba4b937622d Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 10 Sep 2020 18:09:17 +0200 Subject: [PATCH] * Fixed: Restconf failed put/post could leave residue in candidate causing errors in next put/post * Added -v option for backend plugins to generate validation error --- CHANGELOG.md | 1 + apps/backend/backend_client.c | 6 ++--- example/main/example_backend.c | 30 ++++++++++++++++++++- example/main/example_backend_nacm.c | 41 +++++++++++++++-------------- test/test_restconf_err.sh | 23 ++++++++++++++-- test/test_transaction.sh | 9 +++---- 6 files changed, 79 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3dd9140..a19fc0b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Developers may need to change their code ### Corrected Bugs +* Fixed: Restconf failed put/post could leave residue in candidate causing errors in next put/post * Fixed: [clixon_netconf does not respond to hello #136](https://github.com/clicon/clixon/issues/136) * The error showed only when CLICON_MODULE_LIBRARY_RFC7895 was disabled. * Fixed: Do not check min/max constraints on state data in config validate code diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 0eb7d18b..f9ea4692 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -669,9 +669,9 @@ from_client_edit_config(clicon_handle h, /* If autocommit option is set or requested by client */ if (clicon_autocommit(h) || autocommit) { if ((ret = candidate_commit(h, "candidate", cbret)) < 0){ /* Assume validation fail, nofatal */ - if (ret < 0) - if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0) - goto done; + if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0) + goto done; + xmldb_copy(h, "running", "candidate"); goto ok; } if (ret == 0){ /* discard */ diff --git a/example/main/example_backend.c b/example/main/example_backend.c index b8e1c4ce..79f07c48 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -42,6 +42,7 @@ * -u enable upgrade function - auto-upgrade testing * -U general-purpose upgrade * -t enable transaction logging (cal syslog for every transaction) + * -v Failing validate and commit if is present (synthetic error) */ #include #include @@ -65,7 +66,7 @@ #include /* Command line options to be passed to getopt(3) */ -#define BACKEND_EXAMPLE_OPTS "rsS:iuUt" +#define BACKEND_EXAMPLE_OPTS "rsS:iuUt:v:" /*! Variable to control if reset code is run. * The reset code inserts "extra XML" which assumes ietf-interfaces is @@ -111,6 +112,13 @@ static int _general_upgrade = 0; */ static int _transaction_log = 0; +/*! Variable to control transaction logging (for debug) + * If set, call syslog for every transaction callback + * Start backend with -- -v + */ +static char *_validate_fail_xpath = NULL; +static int _validate_fail_toggle = 0; /* fail at validate and commit */ + /* forward */ static int example_stream_timer_setup(clicon_handle h); @@ -130,6 +138,14 @@ main_validate(clicon_handle h, { if (_transaction_log) transaction_log(h, td, LOG_NOTICE, __FUNCTION__); + if (_validate_fail_xpath){ + if (_validate_fail_toggle==0 && + xpath_first(transaction_target(td), NULL, "%s", _validate_fail_xpath)){ + _validate_fail_toggle = 1; /* toggle if triggered */ + clicon_err(OE_XML, 0, "User error"); + return -1; /* induce fail */ + } + } return 0; } @@ -157,6 +173,15 @@ main_commit(clicon_handle h, if (_transaction_log) transaction_log(h, td, LOG_NOTICE, __FUNCTION__); + if (_validate_fail_xpath){ + if (_validate_fail_toggle==1 && + xpath_first(transaction_target(td), NULL, "%s", _validate_fail_xpath)){ + _validate_fail_toggle = 0; /* toggle if triggered */ + clicon_err(OE_XML, 0, "User error"); + return -1; /* induce fail */ + } + } + /* Create namespace context for xpath */ if ((nsc = xml_nsctx_init(NULL, "urn:ietf:params:xml:ns:yang:ietf-interfaces")) == NULL) goto done; @@ -1041,6 +1066,9 @@ clixon_plugin_init(clicon_handle h) case 't': /* transaction log */ _transaction_log = 1; break; + case 'v': /* validate fail */ + _validate_fail_xpath = optarg; + break; } /* Example stream initialization: diff --git a/example/main/example_backend_nacm.c b/example/main/example_backend_nacm.c index 3e25bfce..6bc0ac63 100644 --- a/example/main/example_backend_nacm.c +++ b/example/main/example_backend_nacm.c @@ -37,12 +37,9 @@ * features: * - nacm * - transaction test - * The transaction test is test/test_transaction.sh where a user-error is triggered - * by this plugin if started with -- -t . _transaction_xpath is then set - * and triggers a validation error if it matches. The error also toggles between - * validation and commit errors. + * -t enable transaction logging (cal syslog for every transaction) + * -v Failing validate and commit if is present (synthetic error) */ - #include #include #include @@ -67,10 +64,12 @@ */ static int _transaction_log = 0; -/*! Variable for failing validate and commit, if set, fail on validate vs commit +/*! Variable to control transaction logging (for debug) + * If set, call syslog for every transaction callback + * Start backend with -- -v */ -static char * _transaction_xpath = NULL; -static int _transaction_error_toggle = 0; /* fail at validate vs commit */ +static char *_validate_fail_xpath = NULL; +static int _validate_fail_toggle = 0; /* fail at validate and commit */ int nacm_begin(clicon_handle h, @@ -86,11 +85,12 @@ int nacm_validate(clicon_handle h, transaction_data td) { - if (_transaction_log){ + if (_transaction_log) transaction_log(h, td, LOG_NOTICE, __FUNCTION__); - if (_transaction_error_toggle==0 && - xpath_first(transaction_target(td), NULL, "%s", _transaction_xpath)){ - _transaction_error_toggle=1; /* toggle if triggered */ + if (_validate_fail_xpath){ + if (_validate_fail_toggle==0 && + xpath_first(transaction_target(td), NULL, "%s", _validate_fail_xpath)){ + _validate_fail_toggle = 1; /* toggle if triggered */ clicon_err(OE_XML, 0, "User error"); return -1; /* induce fail */ } @@ -113,13 +113,12 @@ int nacm_commit(clicon_handle h, transaction_data td) { - cxobj *target = transaction_target(td); /* wanted XML tree */ - - if (_transaction_log){ + if (_transaction_log) transaction_log(h, td, LOG_NOTICE, __FUNCTION__); - if (_transaction_error_toggle==1 && - xpath_first(target, NULL, "%s", _transaction_xpath)){ - _transaction_error_toggle=0; /* toggle if triggered */ + if (_validate_fail_xpath){ + if (_validate_fail_toggle==1 && + xpath_first(transaction_target(td), NULL, "%s", _validate_fail_xpath)){ + _validate_fail_toggle = 0; /* toggle if triggered */ clicon_err(OE_XML, 0, "User error"); return -1; /* induce fail */ } @@ -234,11 +233,13 @@ clixon_plugin_init(clicon_handle h) goto done; opterr = 0; optind = 1; - while ((c = getopt(argc, argv, "rsut:")) != -1) + while ((c = getopt(argc, argv, "tv:")) != -1) switch (c) { case 't': /* transaction log */ _transaction_log = 1; - _transaction_xpath = optarg; + break; + case 'v': /* validate fail */ + _validate_fail_xpath = optarg; break; } diff --git a/test/test_restconf_err.sh b/test/test_restconf_err.sh index 270491e3..6e64d319 100755 --- a/test/test_restconf_err.sh +++ b/test/test_restconf_err.sh @@ -122,6 +122,17 @@ module example{ } } } + container table{ + list parameter{ + key name; + leaf name{ + type string; + } + leaf value{ + type string; + } + } + } } EOF @@ -150,8 +161,8 @@ if [ $BE -ne 0 ]; then err fi sudo pkill -f clixon_backend # to be sure - new "start backend -s init -f $cfg -- -sS $fstate" - start_backend -s init -f $cfg -- -sS $fstate + new "start backend -s init -f $cfg -- -sS $fstate -v /table/parameter[name=\"4242\"]" + start_backend -s init -f $cfg -- -sS $fstate -v /table/parameter[name="4242"] fi new "waiting" @@ -218,6 +229,14 @@ expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/augment:r new "restconf GET failed state" expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+xml' $RCPROTO://localhost/restconf/data?content=nonconfig)" 0 '412 Precondition Failed' 'applicationoperation-failedmystateerrorFailed to find YANG spec of XML node: mystate with parent: top in namespace: urn:example:foobar. Internal error, state callback returned invalid XML: example_backend' +# Add error XML a[4242] , it should fail on autocommit but may not be discarded, therefore still +# there in candidate when want to add something else +new "Add user-invalid entry (should fail)" +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+xml" $RCPROTO://localhost/restconf/data -d '4242
')" 0 "HTTP/1.1 412 Precondition Failed" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"operation-failed","error-severity":"error","error-message":"User error"}}}' + +new "Add OK entry" +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+xml" $RCPROTO://localhost/restconf/data -d '1
')" 0 "HTTP/1.1 201 Created" + if [ $RC -ne 0 ]; then new "Kill restconf daemon" stop_restconf diff --git a/test/test_transaction.sh b/test/test_transaction.sh index 4da86e4d..92ca78bb 100755 --- a/test/test_transaction.sh +++ b/test/test_transaction.sh @@ -28,7 +28,7 @@ fyang=$dir/trans.yang flog=$dir/backend.log touch $flog -# Used as a trigger for user-validittion errors, eg $errnr is invalid +# Used as a trigger for user-validittion errors, eg $errnr = 42 is invalid errnr=42 cat < $fyang @@ -68,7 +68,6 @@ cat < $cfg $fyang /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/backend - /usr/local/lib/$APPNAME/netconf /usr/local/lib/$APPNAME/restconf /usr/local/lib/$APPNAME/cli @@ -109,7 +108,7 @@ checklog(){ fi } -new "test params: -f $cfg -l f$flog -- -t /x/y[a=$errnr]" # Fail on this +new "test params: -f $cfg -l f$flog -- -t -v /x/y[a=$errnr]" # Fail on this # Bring your own backend if [ $BE -ne 0 ]; then # kill old backend (if any) @@ -118,8 +117,8 @@ if [ $BE -ne 0 ]; then if [ $? -ne 0 ]; then err fi - new "start backend -s init -f $cfg -l f$flog -- -t /x/y[a=$errnr]" - start_backend -s init -f $cfg -l f$flog -- -t /x/y[a=$errnr] # -t means transaction logging + new "start backend -s init -f $cfg -l f$flog -- -t -v /x/y[a=$errnr]" + start_backend -s init -f $cfg -l f$flog -- -t -v /x/y[a=$errnr] # -t means transaction logging new "waiting" wait_backend