From 9f5176adf54b93cd4c4aa63d4c31a1471bcc07d7 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Fri, 9 Apr 2021 12:34:41 +0200 Subject: [PATCH] Fix: Restconf service did not close when restconf config was removed --- apps/backend/backend_plugin_restconf.c | 8 +- test/lib.sh | 4 +- test/test_netconf_hello.sh | 112 ++++++++++++++++++++ test/test_restconf_rpc2.sh | 136 ++++++++++++++++++++++--- 4 files changed, 241 insertions(+), 19 deletions(-) create mode 100755 test/test_netconf_hello.sh diff --git a/apps/backend/backend_plugin_restconf.c b/apps/backend/backend_plugin_restconf.c index f7d448b8..536e6018 100644 --- a/apps/backend/backend_plugin_restconf.c +++ b/apps/backend/backend_plugin_restconf.c @@ -258,16 +258,14 @@ restconf_pseudo_process_commit(clicon_handle h, /* Toggle start/stop if enable flag changed */ if ((cx = xpath_first(xtarget, NULL, "/restconf/enable")) != NULL && xml_flag(cx, XML_FLAG_CHANGE|XML_FLAG_ADD)){ - - if (clixon_process_operation(h, RESTCONF_PROCESS, enabled?PROC_OP_START:PROC_OP_STOP, 0) < 0) goto done; } else if (enabled){ /* If something changed and running, restart process */ - if (transaction_dlen(xtarget) != 0 || - transaction_alen(xtarget) != 0 || - transaction_clen(xtarget) != 0){ + if (transaction_dlen(td) != 0 || + transaction_alen(td) != 0 || + transaction_clen(td) != 0){ if ((cx = xpath_first(xtarget, NULL, "/restconf")) != NULL && xml_flag(cx, XML_FLAG_CHANGE|XML_FLAG_ADD)){ /* A restart can terminate a restconf connection (cut the tree limb you are sitting on) diff --git a/test/lib.sh b/test/lib.sh index 26722c3c..50f45ced 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -72,8 +72,10 @@ testname= # eg logging to a file: RCLOG="-l f/www-data/restconf.log" : ${RCLOG:=} +BASENS='urn:ietf:params:xml:ns:netconf:base:1.0' + # Default netconf namespace statement, typically as placed on top-level see test_xml.sh +# Note clixon sends rpc-error back on primitive errors prior to knowing it is an rpc. +# This means that some errors for hello messages get an rpc-error back which is somewhat +# fringe to the standard. + +# 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 +tmp=$dir/tmp.x + +# Use yang in example + +cat < $cfg + + $cfg + ietf-netconf:startup + 42 + /usr/local/share/clixon + $IETFRFC + clixon-example + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/backend + example_backend.so$ + /usr/local/lib/$APPNAME/netconf + /usr/local/lib/$APPNAME/restconf + /usr/local/lib/$APPNAME/cli + $APPNAME + $dir/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + /usr/local/var/$APPNAME + true + +EOF + +new "test params: -f $cfg -- -s" +# Bring your own backend +if [ $BE -ne 0 ]; then + # kill old backend (if any) + new "kill old backend" + sudo clixon_backend -zf $cfg + if [ $? -ne 0 ]; then + err + fi + new "start backend -s init -f $cfg -- -s" + start_backend -s init -f $cfg -- -s + + new "waiting" + wait_backend +fi + +# Hello +new "Netconf snd hello with xmldecl" +expecteof "$clixon_netconf -qf $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" '^$' '^$' + +new "Netconf snd hello without xmldecl" +expecteof "$clixon_netconf -qf $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" '^$' '^$' + +new "Netconf snd hello with RFC 4741 base capability" +expecteof "$clixon_netconf -qf $cfg" 0 "urn:ietf:params:netconf:base:1.0]]>]]>" '^$' '^$' + +new "Netconf snd hello with wrong base capability" +expecteof "$clixon_netconf -qf $cfg" 0 "urn:ietf:params:netconf:base:1.2]]>]]>" '^$' 'Server received hello without netconf base capability urn:ietf:params:netconf:base:1.1' + +# This is a case where hello error gets an rpc-error back. Not strictly correct. +new "Netconf hello with session-id is wrong" +expecteof "$clixon_netconf -qf $cfg" 0 "42urn:ietf:params:netconf:base:1.2]]>]]>" '^protocolunknown-elementsession-iderrorUnrecognized hello/capabilities element]]>]]>$' '^$' + +# When comparing protocol version capability URIs, only the base part is used, in the event any +# parameters are encoded at the end of the URI string. +new "Netconf snd hello with base capability with extra arguments" +expecteof "$clixon_netconf -qf $cfg" 0 "urn:ietf:params:netconf:base:1.1?arg=val]]>]]>" '^$' '^$' + +# This is hello - shouldnt really get rpc back? +new "Netconf snd hello with wrong prefix" +expecteof "$clixon_netconf -qf $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" '^$' 'XML error: No appropriate namespace associated with prefix:xx: Bad address' + +new "Netconf snd hello with prefix" +expecteof "$clixon_netconf -qf $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" '^$' '^$' + +new "netconf snd + rcv hello" +expecteof "$clixon_netconf -f $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" "^urn:ietf:params:netconf:base:1.1urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:capability:yang-library:1.0?revision=2019-01-04&module-set-id=42urn:ietf:params:netconf:capability:candidate:1.0urn:ietf:params:netconf:capability:validate:1.1urn:ietf:params:netconf:capability:startup:1.0urn:ietf:params:netconf:capability:xpath:1.0urn:ietf:params:netconf:capability:notification:1.0[0-9]*]]>]]>$" '^$' + +new "Netconf snd hello with extra element" +expecteof "$clixon_netconf -qf $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" '^protocolunknown-elementextra-elementerrorUnrecognized hello/capabilities element]]>]]>$' '^$' + +new "Netconf send rpc without hello error" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "rpcoperation-failederrorClient must send an hello element before any RPC]]>]]>" '^$' + +new "Netconf send rpc without hello w CLICON_NETCONF_HELLO_OPTIONAL" +expecteof "$clixon_netconf -qf $cfg -o CLICON_NETCONF_HELLO_OPTIONAL=true" 0 "]]>]]>" "]]>]]>" '^$' + +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 diff --git a/test/test_restconf_rpc2.sh b/test/test_restconf_rpc2.sh index eceddbaf..4e399f9b 100755 --- a/test/test_restconf_rpc2.sh +++ b/test/test_restconf_rpc2.sh @@ -4,14 +4,18 @@ # In comparison test_restconf_rpc.sh: # - uses externally started restconf, here started by backend # - generic tests, here specific -# The first usecases is: +# The first usecases is: empty status message # 1. Start a minimal restconf # 2. Kill it externally (or it exits) # 3. Start a server # 4. Query status (Error message is returned) -# The second usecase is +# The second usecase is: zombie process on exit # 1. Start server with bad address # 2. Zombie process appears +# The third usecase is: restconf not removed +# 1. Start server +# 2. Remove server +# 3. Check status (Error: still up) # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -23,6 +27,7 @@ startupdb=$dir/startup_db # Restconf debug RESTCONFDBG=0 +RCPROTO=http # no ssl here cat < $cfg @@ -58,7 +63,6 @@ module example { } EOF - function testrpc() { operation=$1 @@ -77,7 +81,7 @@ $DEFAULTHELLO EOF ) - >&2 echo "ret:$ret" # debug +# >&2 echo "ret:$ret" # debug expect1="[0-9]*" match=$(echo "$ret" | grep --null -Go "$expect1") @@ -87,7 +91,7 @@ EOF else pid=$(echo "$match" | awk -F'[<>]' '{print $3}') fi - >&2 echo "pid:$pid" # debug +# >&2 echo "pid:$pid" # debug if [ -z "$pid" ]; then err "Running process" "$ret" @@ -109,6 +113,10 @@ EOF sleep $DEMSLEEP } +# FIRST usecase + +new "1. Empty status message" + new "kill old restconf" stop_restconf_pre @@ -146,8 +154,9 @@ new "netconf commit" expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" # Get pid2 +new "get pid" pid2=$(testrpc status 1) -echo "pid:$pid2" +echo "pid2:$pid2" new "get status 2" ret=$($clixon_netconf -qf $cfg< /dev/null RESTCONFIG2=$(cat < @@ -201,7 +218,8 @@ if [ $BE -ne 0 ]; then stop_backend -f $cfg fi -sleep $DEMSLEEP # Lots of processes need to die before next test +# SECOND usecase +new "2. zombie process on exit" new "kill old restconf" stop_restconf_pre @@ -221,16 +239,13 @@ new "wait backend" wait_backend new "get status 1" -testrpc status 0 +testrpc status 0 > /dev/null RESTCONFIG1=$(cat < true $RESTCONFDBG none - $srvcert - $srvkey - $cakey false default
221.0.0.1
80false @@ -245,11 +260,105 @@ expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO /dev/null + +RESTCONFIG1=$(cat < + true + $RESTCONFDBG + none + false + default
0.0.0.0
80false + +EOF +) + +new "Create server" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO$RESTCONFIG1]]>]]>" "^]]>]]>$" + +new "commit create" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +# pid +pid1=$(testrpc status 1) + +sleep $DEMSLEEP + +new "Get restconf config 1" +expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+xml' $RCPROTO://localhost/restconf/data/clixon-restconf:restconf)" 0 "HTTP/1.1 200 OK" "truenone0falsedefault
0.0.0.0
80false" + +# remove it +new "Delete server" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLOnonedefault
0.0.0.0
80]]>]]>" "^]]>]]>$" + +new "commit delete" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +new "Get restconf config 2" +expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+xml' $RCPROTO://localhost/restconf/data/clixon-restconf:restconf)" 0 "HTTP/1.1 200 OK" "truenone0false" + +pid2=$(testrpc status 1) + +if [ $pid1 -ne $pid2 ]; then + err "Same pid:$pid1" "$pid2" +fi + +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 + new "endtest" endtest @@ -257,6 +366,7 @@ endtest unset RESTCONFIG1 unset RESTCONFIG2 unset RESTCONFDBG +unset RCPROTO rm -rf $dir