From 5b39418e92f870e768c0ae572a0382dbce6ba877 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Tue, 25 May 2021 11:07:41 +0200 Subject: [PATCH] * [Need an option to disable restconf mandatory action of overwriting startup_db #230](https://github.com/clicon/clixon/issues/230) * Disable RFC 8040 mandatory copy of running to startup after commit * Rename CLICON_RESTCONF_INSTALL_DIR -> CLICON_RESTCONF_INSTALLDIR --- CHANGELOG.md | 7 +++++-- apps/backend/backend_plugin_restconf.c | 4 ++-- apps/restconf/restconf_methods.c | 10 ++++++++-- apps/restconf/restconf_methods_post.c | 5 ++++- test/test_restconf_internal.sh | 2 +- test/test_restconf_internal_usecases.sh | 10 +++++----- test/test_restconf_startup.sh | 22 +++++++++++++--------- yang/clixon/clixon-config@2021-05-20.yang | 18 +++++++++++++++--- 8 files changed, 53 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9162f6ee..269ff08b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,14 +50,15 @@ Users may have to change how they access the system * Changed config and install options for Restconf * clixon_restconf daemon is installed in /usr/local/sbin (as clixon_backend), instead of /www-data * `configure --with-wwwdir=` remains but only applies to fcgi socket and log - * New option `CLICON_RESTCONF_INSTALL_DIR` is set to where clixon_restconf is installed, with default `/usr/local/sbin/` + * New option `CLICON_RESTCONF_INSTALLDIR` is set to where clixon_restconf is installed, with default `/usr/local/sbin/` * Restconf drop privileges user is defined by `CLICON_RESTCONF_USER` * `configure --with-wwwuser=` is removed * clixon_restconf drop of privileges is defined by `CLICON_RESTCONF_PRIVILEGES` option * New clixon-config@2020-05-20.yang revision * Added: `CLICON_RESTCONF_USER` * Added: `CLICON_RESTCONF_PRIVILEGES` - * Added: `CLICON_RESTCONF_INSTALL_DIR` + * Added: `CLICON_RESTCONF_INSTALLDIR` + * Added: `CLICON_RESTCONF_STARTUP_DONTUPDATE` * New clixon-restconf@2020-05-20.yang revision * Added: restconf `log-destination` * RESTCONF error replies have changed @@ -71,6 +72,8 @@ Users may have to change how they access the system ### Minor features +* [Need an option to disable restconf mandatory action of overwriting startup_db #230](https://github.com/clicon/clixon/issues/230) + * Disable RFC 8040 mandatory copy of running to startup after commit * Add default network namespace constant: `RESTCONF_NETNS_DEFAULT` with default value "default". * CLI: Two new hide variables added (thanks: shmuelnatan) * hide-database : specifies that a command is not visible in database. This can be useful for setting passwords and not exposing them to users. diff --git a/apps/backend/backend_plugin_restconf.c b/apps/backend/backend_plugin_restconf.c index f808e4ca..9b064256 100644 --- a/apps/backend/backend_plugin_restconf.c +++ b/apps/backend/backend_plugin_restconf.c @@ -202,12 +202,12 @@ restconf_pseudo_process_control(clicon_handle h) clicon_err(OE_UNIX, errno, "cbuf_new"); goto done; } - /* CLICON_RESTCONF_INSTALL_DIR is where we think clixon_restconf is installed + /* CLICON_RESTCONF_INSTALLDIR is where we think clixon_restconf is installed * Problem is where to define it? Now in config file, but maybe it should be in configure? * Tried Makefile but didnt work on Docker since it was moved around. * Use PATH? */ - cprintf(cb, "%s/clixon_restconf", clicon_option_str(h, "CLICON_RESTCONF_INSTALL_DIR")); + cprintf(cb, "%s/clixon_restconf", clicon_option_str(h, "CLICON_RESTCONF_INSTALLDIR")); argv[i++] = cbuf_get(cb); argv[i++] = "-f"; argv[i++] = clicon_option_str(h, "CLICON_CONFIGFILE"); diff --git a/apps/restconf/restconf_methods.c b/apps/restconf/restconf_methods.c index adea6960..a4c1f51a 100644 --- a/apps/restconf/restconf_methods.c +++ b/apps/restconf/restconf_methods.c @@ -522,8 +522,11 @@ api_data_write(clicon_handle h, * configuration datastore, after the "running" datastore has been altered * as a consequence of a RESTCONF edit operation. */ - if ((IETF_DS_NONE == ds) && if_feature(yspec, "ietf-netconf", "startup")) + if ((IETF_DS_NONE == ds) && + if_feature(yspec, "ietf-netconf", "startup") && + !clicon_option_bool(h, "CLICON_RESTCONF_STARTUP_DONTUPDATE")){ cprintf(cbx, " copystartup=\"true\""); + } cprintf(cbx, " autocommit=\"true\""); cprintf(cbx, ">"); cprintf(cbx, "none"); @@ -760,8 +763,11 @@ api_data_delete(clicon_handle h, * configuration datastore, after the "running" datastore has been altered * as a consequence of a RESTCONF edit operation. */ - if ((IETF_DS_NONE == ds) && if_feature(yspec, "ietf-netconf", "startup")) + if ((IETF_DS_NONE == ds) && + if_feature(yspec, "ietf-netconf", "startup") && + !clicon_option_bool(h, "CLICON_RESTCONF_STARTUP_DONTUPDATE")){ cprintf(cbx, " copystartup=\"true\""); + } cprintf(cbx, " autocommit=\"true\""); cprintf(cbx, ">"); cprintf(cbx, "none"); diff --git a/apps/restconf/restconf_methods_post.c b/apps/restconf/restconf_methods_post.c index 248e0d11..eddc210c 100644 --- a/apps/restconf/restconf_methods_post.c +++ b/apps/restconf/restconf_methods_post.c @@ -346,8 +346,11 @@ api_data_post(clicon_handle h, * configuration datastore, after the "running" datastore has been altered * as a consequence of a RESTCONF edit operation. */ - if ((IETF_DS_NONE == ds) && if_feature(yspec, "ietf-netconf", "startup")) + if ((IETF_DS_NONE == ds) && + if_feature(yspec, "ietf-netconf", "startup") && + !clicon_option_bool(h, "CLICON_RESTCONF_STARTUP_DONTUPDATE")){ cprintf(cbx, " copystartup=\"true\""); + } cprintf(cbx, " autocommit=\"true\""); cprintf(cbx, ">"); cprintf(cbx, "none"); diff --git a/test/test_restconf_internal.sh b/test/test_restconf_internal.sh index 9a16dfc2..8c9724c4 100755 --- a/test/test_restconf_internal.sh +++ b/test/test_restconf_internal.sh @@ -54,7 +54,7 @@ cat < $cfg /usr/local/lib/$APPNAME/backend example_backend.so$ /usr/local/lib/$APPNAME/restconf - $RESTCONFDIR + $RESTCONFDIR /usr/local/lib/$APPNAME/cli $APPNAME /usr/local/var/$APPNAME/$APPNAME.sock diff --git a/test/test_restconf_internal_usecases.sh b/test/test_restconf_internal_usecases.sh index c0afa16e..ff7b15b4 100755 --- a/test/test_restconf_internal_usecases.sh +++ b/test/test_restconf_internal_usecases.sh @@ -68,7 +68,7 @@ cat < $cfg /usr/local/lib/$APPNAME/backend example_backend.so$ /usr/local/lib/$APPNAME/restconf - $RESTCONFDIR + $RESTCONFDIR /usr/local/lib/$APPNAME/cli $APPNAME /usr/local/var/$APPNAME/$APPNAME.sock @@ -194,15 +194,15 @@ ps aux|grep clixon_ # XXX new "Check $pid1 exists" # Here backend dies / is killed -#while sudo kill -0 $pid1 2> /dev/null; do -if sudo kill -0 $pid1; then # XXX +# if sudo kill -0 $pid1; then # XXX +while sudo kill -0 $pid1 2> /dev/null; do new "kill $pid1 externally" sudo kill $pid1 sleep 1 # There is a race condition here when restconf is killed while waiting for reply from backend echo "pid1:$pid1" # XXX ps aux|grep clixon_ # XXX -fi -#done +done +# fi echo "pid1:$pid1" # XXX ps aux|grep clixon_ # XXX diff --git a/test/test_restconf_startup.sh b/test/test_restconf_startup.sh index 7c273a4f..91a39b0b 100755 --- a/test/test_restconf_startup.sh +++ b/test/test_restconf_startup.sh @@ -66,16 +66,18 @@ function testrun(){ start_backend -s init -f $cfg -y $fyang $option fi - new "waiting" + new "wait backend" wait_backend - new "kill old restconf daemon" - stop_restconf_pre + if [ $RC -ne 0 ]; then + new "kill old restconf daemon" + stop_restconf_pre - new "start restconf daemon" - start_restconf -f $cfg -y $fyang $option + new "start restconf daemon" + start_restconf -f $cfg -y $fyang $option + fi - new "waiting" + new "wait restconf" wait_restconf new "restconf put 42" @@ -90,9 +92,11 @@ function testrun(){ new "restconf delete 42" expectpart "$(curl $CURLOPTS -X DELETE $RCPROTO://localhost/restconf/data/example:x/y=42)" 0 "HTTP/1.1 204 No Content" - new "Kill restconf daemon" - stop_restconf - + if [ $RC -ne 0 ]; then + new "Kill restconf daemon" + stop_restconf + fi + if [ $BE -ne 0 ]; then new "Kill backend" # Check if premature kill diff --git a/yang/clixon/clixon-config@2021-05-20.yang b/yang/clixon/clixon-config@2021-05-20.yang index 71dd9ac0..750272fe 100644 --- a/yang/clixon/clixon-config@2021-05-20.yang +++ b/yang/clixon/clixon-config@2021-05-20.yang @@ -48,7 +48,8 @@ module clixon-config { "Added option: CLICON_RESTCONF_USER CLICON_RESTCONF_PRIVILEGES - CLICON_RESTCONF_INSTALL_DIR + CLICON_RESTCONF_INSTALLDIR + CLICON_RESTCONF_STARTUP_DONTUPDATE Released in Clixon 5.2"; } revision 2021-03-08 { @@ -479,7 +480,7 @@ module clixon-config { Note: Obsolete, use fcgi-socket in clixon-restconf.yang instead"; status obsolete; } - leaf CLICON_RESTCONF_INSTALL_DIR { + leaf CLICON_RESTCONF_INSTALLDIR { type string; default "/usr/local/sbin"; description @@ -489,7 +490,18 @@ module clixon-config { installation moves the binaries, and this may be true elsewehere too. Maybe one could locate it via PATHs search"; } - + leaf CLICON_RESTCONF_STARTUP_DONTUPDATE { + type boolean; + default false; + description + "According to RFC 8040 Sec 1.4: + If the NETCONF server supports :startup, the RESTCONF server MUST automatically + update the [...] startup configuration [...] as a consequence of a RESTCONF + edit operation. + Setting this option disables this behaviour, ie the startup configuration is NOT + automatically updated. + If this option is false, the startup is autoamtically updated following the RFC"; + } leaf CLICON_RESTCONF_PRETTY { type boolean; default true;