(Work in progress) Restconf error handling for get and edit operations

CHANGELOG.md

@@ -4,7 +4,13 @@
 
 
 ### Major changes:
+* (Work in progress) Restconf error handling for get and edit operations
+
 ### Minor changes:
+* Add username to rpc calls to prepare for authorization for backend:
+  clicon_rpc_config_get(h, db, xpath, xt) --> clicon_rpc_config_get(h, db, xpath, username, xt)
+  clicon_rpc_get(h, xpath, xt) --> clicon_rpc_get(h, xpath, username, xt)
+
 * Experimental: Added CLICON_TRANSACTION_MOD configurqation option. If set,
   modifications in validation and commit callbacks are written back
   into the datastore.
@@ -31,7 +37,7 @@ enables saved files to be used as datastore without any editing. Thanks Matt.
 ## 3.5.0 (12 February 2018)
 
 ### Major changes:
-* Major Restconf feature update to compy to RFC 8040. Thanks Stephen Jones for getting right.
+* Major Restconf feature update to comply to RFC 8040. Thanks Stephen Jones for getting right.
   * GET: Always return object referenced (and nothing else). ie, GET /restconf/data/X returns X. 
   * GET Added support for the following resources: Well-known, top-level resource, and yang library version,
   * GET Single element JSON lists use {list:[element]}, not {list:element}.

apps/cli/cli_generate.c

@@ -238,7 +238,6 @@ yang2cli_var_sub(clicon_handle h,
 		goto done;
 	    }
 	}
-
 	else{ /* Cligen does not have 'max' keyword in range so need to find actual
 		 max value of type if yang range expression is 0..max 
 	      */

apps/restconf/restconf_methods.c

@@ -144,47 +144,62 @@ api_data_options(clicon_handle h,
  * @param[in]  h      Clixon handle
  * @param[in]  r      Fastcgi request handle
  * @param[in]  xerr   XML error message from backend
+ * @param[in]  pretty Set to 1 for pretty-printed xml/json output
+ * @param[in]  use_xml Set to 0 for JSON and 1 for XML
  */
 static int
-api_data_get_err(clicon_handle h,
+api_return_err(clicon_handle h,
 	       FCGX_Request *r,
-		 cxobj     *xerr)
+	       cxobj        *xerr,
+	       int           pretty,
+	       int           use_xml)
 {
     int        retval = -1;
-    cbuf      *cbj = NULL;
+    cbuf      *cb = NULL;
     cxobj     *xtag;
     int        code;	
     const char *reason_phrase;
 
-    if ((cbj = cbuf_new()) == NULL)
+    clicon_debug(1, "%s", __FUNCTION__);
+    if ((cb = cbuf_new()) == NULL)
 	goto done;
-    if ((xtag = xpath_first(xerr, "/error-tag")) == NULL){
+    if ((xtag = xpath_first(xerr, "error-tag")) == NULL){
 	notfound(r); /* bad reply? */
 	goto ok;
     }
     code = restconf_err2code(xml_body(xtag));
     if ((reason_phrase = restconf_code2reason(code)) == NULL)
 	reason_phrase="";
-    clicon_debug(1, "%s code:%d reason phrase:%s", 
-		 __FUNCTION__, code, reason_phrase);
-
     if (xml_name_set(xerr, "error") < 0)
 	goto done;
-    if (xml2json_cbuf(cbj, xerr, 1) < 0)
+    if (use_xml){
+	if (clicon_xml2cbuf(cb, xerr, 2, pretty) < 0)
+	    goto done;
+    }
+    else
+	if (xml2json_cbuf(cb, xerr, pretty) < 0)
 	    goto done;
     FCGX_FPrintF(r->out, "Status: %d %s\r\n", code, reason_phrase);
-    FCGX_FPrintF(r->out, "Content-Type: application/yang-data+json\r\n\r\n");
+    FCGX_FPrintF(r->out, "Content-Type: application/yang-data+%s\r\n\r\n",
-    FCGX_FPrintF(r->out, "\r\n");
+		 use_xml?"xml":"json");
-    FCGX_FPrintF(r->out, "{\r\n");
+    if (use_xml){
-    FCGX_FPrintF(r->out, "  \"ietf-restconf:errors\" : {\r\n");
+	FCGX_FPrintF(r->out, "    <errors xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\">%s", cbuf_get(cb), pretty?"\r\n":"");
-    FCGX_FPrintF(r->out, "    %s", cbuf_get(cbj));
+	FCGX_FPrintF(r->out, "%s", cbuf_get(cb));
-    FCGX_FPrintF(r->out, "  }\r\n");
+	FCGX_FPrintF(r->out, "    </errors>\r\n");
+    }
+    else{
+	FCGX_FPrintF(r->out, "{%s", pretty?"\r\n":"");
+	FCGX_FPrintF(r->out, "  \"ietf-restconf:errors\" : {%s", pretty?"\r\n":"");
+	FCGX_FPrintF(r->out, "    %s", cbuf_get(cb));
+	FCGX_FPrintF(r->out, "  }%s", pretty?"\r\n":"");
 	FCGX_FPrintF(r->out, "}\r\n");
+    }
  ok:
     retval = 0;
  done:
-    if (cbj)
+    clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);
-        cbuf_free(cbj);
+    if (cb)
+        cbuf_free(cb);
     return retval;
 }
 
@@ -269,9 +284,9 @@ api_data_get2(clicon_handle h,
 	cbuf_free(cb);
     }
 #endif
-    /* Check if error return */
+    /* Check if error return XXX this needs more work */
     if ((xerr = xpath_first(xret, "/rpc-error")) != NULL){
-	if (api_data_get_err(h, r, xerr) < 0)
+	if (api_return_err(h, r, xerr, pretty, use_xml) < 0)
 	    goto done;
 	goto ok;
     }
@@ -425,6 +440,7 @@ api_data_post(clicon_handle h,
 {
     int        retval = -1;
     enum operation_type op = OP_CREATE;
+    int        pretty;
     int        i;
     cxobj     *xdata = NULL;
     cbuf      *cbx = NULL;
@@ -437,10 +453,18 @@ api_data_post(clicon_handle h,
     cxobj     *xu;
     char      *media_content_type;
     int        parse_xml = 0; /* By default expect and parse JSON */
+    cxobj     *xret = NULL;
+    cxobj     *xerr;
+    char      *media_accept;
+    int        use_xml = 0; /* By default use JSON */
 	
     clicon_debug(1, "%s api_path:\"%s\" json:\"%s\"",
 		 __FUNCTION__, 
 		 api_path, data);
+    pretty = clicon_option_bool(h, "CLICON_RESTCONF_PRETTY");
+    media_accept = FCGX_GetParam("HTTP_ACCEPT", r->envp);
+    if (strcmp(media_accept, "application/yang-data+xml")==0)
+	use_xml++;
     media_content_type = FCGX_GetParam("HTTP_CONTENT_TYPE", r->envp);
     if (media_content_type &&
 	strcmp(media_content_type, "application/yang-data+xml")==0)
@@ -499,14 +523,19 @@ api_data_post(clicon_handle h,
     /* Create text buffer for transfer to backend */
     if ((cbx = cbuf_new()) == NULL)
 	goto done;
+    cprintf(cbx, "<rpc><edit-config><target><candidate /></target>");
+    cprintf(cbx, "<default-operation>none</default-operation>");
     if (clicon_xml2cbuf(cbx, xtop, 0, 0) < 0)
 	goto done;
+
+    cprintf(cbx, "</edit-config></rpc>");
     clicon_debug(1, "%s xml: %s api_path:%s",__FUNCTION__, cbuf_get(cbx), api_path);
-    if (clicon_rpc_edit_config(h, "candidate", 
+    if (clicon_rpc_netconf(h, cbuf_get(cbx), &xret, NULL) < 0)
-			       OP_NONE,
+	goto done;
-			       cbuf_get(cbx)) < 0){
+    if ((xerr = xpath_first(xret, "//rpc-error")) != NULL){
-	conflict(r);
+	if (api_return_err(h, r, xerr, pretty, use_xml) < 0)
-	goto ok;
+	    goto done;
+	goto done;
     }
     /* Assume this is validation failed since commit includes validate */
     if (clicon_rpc_commit(h) < 0){
@@ -522,6 +551,8 @@ api_data_post(clicon_handle h,
     retval = 0;
  done:
     clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);
+    if (xret)
+	xml_free(xret);
     if (xtop)
 	xml_free(xtop);
     if (xdata)

lib/src/clixon_proto_client.c

@@ -200,7 +200,7 @@ clicon_rpc_netconf_xml(clicon_handle  h,
     return retval;
 }
 
-/*! Generate clicon error function call from Netconf error message
+/*! Generate and log clicon error function call from Netconf error message
  * @param[in]  xerr    Netconf error message on the level: <rpc-reply><rpc-error>
  */
 int
@@ -308,7 +308,7 @@ clicon_rpc_get_config(clicon_handle       h,
  * @param[in] op         Operation on database item: OP_MERGE, OP_REPLACE
  * @param[in] xml        XML string. Ex: <config><a>..</a><b>...</b></config>
  * @retval    0          OK
- * @retval   -1          Error
+ * @retval   -1          Error and logged to syslog
  * @note xml arg need to have <config> as top element
  * @code
  * if (clicon_rpc_edit_config(h, "running", OP_MERGE, 
@@ -361,6 +361,8 @@ clicon_rpc_edit_config(clicon_handle       h,
  * @param[in] h        CLICON handle
  * @param[in] db1      src database, eg "running"
  * @param[in] db2      dst database, eg "startup"
+ * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  * @code
  * if (clicon_rpc_copy_config(h, "running", "startup") < 0)
  *    err;
@@ -396,6 +398,8 @@ clicon_rpc_copy_config(clicon_handle h,
 /*! Send a request to backend to delete a config database
  * @param[in] h        CLICON handle
  * @param[in] db       database, eg "running"
+ * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  * @code
  * if (clicon_rpc_delete_config(h, "startup") < 0)
  *    err;
@@ -430,6 +434,8 @@ clicon_rpc_delete_config(clicon_handle h,
 /*! Lock a database
  * @param[in] h        CLICON handle
  * @param[in] db       database, eg "running"
+ * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  */
 int
 clicon_rpc_lock(clicon_handle h, 
@@ -460,6 +466,8 @@ clicon_rpc_lock(clicon_handle h,
 /*! Unlock a database
  * @param[in] h        CLICON handle
  * @param[in] db       database, eg "running"
+ * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  */
 int
 clicon_rpc_unlock(clicon_handle h, 
@@ -557,6 +565,8 @@ clicon_rpc_get(clicon_handle       h,
 
 /*! Close a (user) session
  * @param[in] h        CLICON handle
+ * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  */
 int
 clicon_rpc_close_session(clicon_handle h)
@@ -586,6 +596,8 @@ clicon_rpc_close_session(clicon_handle h)
 /*! Kill other user sessions
  * @param[in] h           CLICON handle
  * @param[in] session_id  Session id of other user session
+ * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  */
 int
 clicon_rpc_kill_session(clicon_handle h,
@@ -617,6 +629,7 @@ clicon_rpc_kill_session(clicon_handle h,
  * @param[in] h        CLICON handle
  * @param[in] db       Name of database
  * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  */
 int
 clicon_rpc_validate(clicon_handle h, 
@@ -647,6 +660,7 @@ clicon_rpc_validate(clicon_handle h,
 /*! Commit changes send a commit request to backend daemon
  * @param[in] h          CLICON handle
  * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  */
 int
 clicon_rpc_commit(clicon_handle h)
@@ -676,6 +690,7 @@ clicon_rpc_commit(clicon_handle h)
 /*! Discard all changes in candidate / revert to running
  * @param[in] h        CLICON handle
  * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  */
 int
 clicon_rpc_discard_changes(clicon_handle h)
@@ -707,6 +722,9 @@ clicon_rpc_discard_changes(clicon_handle h)
  * @param{in]   stream   name of notificatio/log stream (CLICON is predefined)
  * @param{in]   filter   message filter, eg xpath for xml notifications
  * @param[out]  s0       socket returned where notification mesages will appear
+ * @retval      0        OK
+ * @retval      -1       Error and logged to syslog
+
  * @note When using netconf create-subsrciption,status and format is not supported
  */
 int
@@ -744,6 +762,8 @@ clicon_rpc_create_subscription(clicon_handle    h,
 /*! Send a debug request to backend server
  * @param[in] h        CLICON handle
  * @param[in] level    Debug level
+ * @retval    0        OK
+ * @retval   -1        Error and logged to syslog
  */
 int
 clicon_rpc_debug(clicon_handle h, 

test/clixon

@@ -1,73 +0,0 @@
-#!/bin/sh
-# Top-level cron scripts. Add this to (for example) /etc/cron.daily
-
-err(){
-  testname=$1
-  errcode=$2
-  echo "Error in [$testname]"
-  logger "CLIXON: Error in [$testname]"
-  exit $errcode
-}
-
-# cd to working dir
-cd /var/tmp
-if [ $# -ne 0 ]; then
-    err "usage: $0" 0
-fi
-rm -rf cligen
-rm -rf clixon
-git clone https://github.com/olofhagsand/cligen.git
-if [ $? -ne 0 ]; then
-    err "git clone cligen" 1
-fi
-cd cligen
-CFLAGS=-Werror ./configure
-if [ $? -ne 0 ]; then
-    err "configure" 2
-fi
-make
-if [ $? -ne 0 ]; then
-    err "make" 3
-fi
-cd ..
-git clone https://github.com/clicon/clixon.git
-if [ $? -ne 0 ]; then
-    err "git clone clixon" 1
-fi
-cd clixon
-CFLAGS=-Werror ./configure --with-cligen=../cligen
-if [ $? -ne 0 ]; then
-    err "configure" 2
-fi
-make
-if [ $? -ne 0 ]; then
-    err "make" 3
-fi
-sudo make install
-if [ $? -ne 0 ]; then
-    err "make install" 4
-fi
-sudo make install-include
-if [ $? -ne 0 ]; then
-    err "make install include" 5
-   exit 1
-fi
-cd example
-make
-if [ $? -ne 0 ]; then
-    err "make example" 6
-fi
-sudo make install
-if [ $? -ne 0 ]; then
-    err "make install example" 7
-fi
-cd ../test
-#./all.sh
-(cd /home/olof/src/clixon/test; ./all.sh)
-errcode=$?
-if [ $errcode -ne 0 ]; then
-    err "test" $errcode
-fi
-cd ../..
-rm -rf clixon cligen
-logger "CLIXON: tests OK"

test/test_restconf.sh

@@ -128,7 +128,6 @@ new "restconf get empty config + state json"
 expectfn "curl -sSG http://localhost/restconf/data" "{\"data\": $state}"
 
 new "restconf get empty config + state xml"
-# Cant get shell macros to work, inline matching from lib.sh
 ret=$(curl -s -H "Accept: application/yang-data+xml" -G http://localhost/restconf/data)
 expect="<data><interfaces-state><interface><name>eth0</name><type>eth</type><if-index>42</if-index></interface></interfaces-state></data>"
 match=`echo $ret | grep -EZo "$expect"`
@@ -161,8 +160,19 @@ if [ -z "$match" ]; then
     err "$expect" "$ret"
 fi
 
+new "restconf GET datastore"
+expectfn "curl -s -X GET http://localhost/restconf/data" "data"
+
 new "restconf Add subtree to datastore using POST"
-expectfn 'curl -s -X POST -d {"interfaces":{"interface":{"name":"eth/0/0","type":"eth","enabled":true}}} http://localhost/restconf/data' ""
+ret=$(curl -s -i -X POST -H "Accept: application/yang-data+json" -d '{"interfaces":{"interface":{"name":"eth/0/0","type":"eth","enabled":true}}}' http://localhost/restconf/data)
+expect="HTTP/1.1 200 OK"
+match=`echo $ret | grep -EZo "$expect"`
+if [ -z "$match" ]; then
+    err "$expect" "$ret"
+fi
+
+new "restconf Re-add subtree which should give error"
+expectfn 'curl -s -i -X POST -d {"interfaces":{"interface":{"name":"eth/0/0","type":"eth","enabled":true}}} http://localhost/restconf/data' '{"error-tag": "operation-failed"'
 
 new "restconf Check interfaces eth/0/0 added"
 expectfn "curl -s -G http://localhost/restconf/data" '{"interfaces": {"interface": \[{"name": "eth/0/0","type": "eth","enabled": true}\]},"interfaces-state": {"interface": \[{"name": "eth0","type": "eth","if-index": 42}\]}}
@@ -182,7 +192,7 @@ expectfn "curl -s -G http://localhost/restconf/data" '{"interfaces": {"interface
 $'
 
 new "restconf Re-post eth/0/0 which should generate error"
-expectfn 'curl -s -X POST -d {"interface":{"name":"eth/0/0","type":"eth","enabled":true}} http://localhost/restconf/data/interfaces' "Data resource already exists"
+expectfn 'curl -s -X POST -d {"interface":{"name":"eth/0/0","type":"eth","enabled":true}} http://localhost/restconf/data/interfaces' 'Object to create already exists'
 
 new "Add leaf description using POST"
 expectfn 'curl -s -X POST -d {"description":"The-first-interface"} http://localhost/restconf/data/interfaces/interface=eth%2f0%2f0' ""

test/test_restconf2.sh

@@ -81,10 +81,10 @@ new "restconf POST interface"
 expectfn 'curl -s -X POST -d {"interface":{"name":"TEST","type":"eth0"}} http://localhost/restconf/data/cont1' ""
 
 new "restconf POST again"
-expectfn 'curl -s -X POST -d {"interface":{"name":"TEST","type":"eth0"}} http://localhost/restconf/data/cont1' "Data resource already exis"
+expectfn 'curl -s -X POST -d {"interface":{"name":"TEST","type":"eth0"}} http://localhost/restconf/data/cont1' "Object to create already exists"
 
 new "restconf POST from top"
-expectfn 'curl -s -X POST -d {"cont1":{"interface":{"name":"TEST","type":"eth0"}}} http://localhost/restconf/data' "Data resource already exists"
+expectfn 'curl -s -X POST -d {"cont1":{"interface":{"name":"TEST","type":"eth0"}}} http://localhost/restconf/data' "Object to create already exists"
 
 new "restconf DELETE"
 expectfn 'curl -s -X DELETE http://localhost/restconf/data/cont1' ""

test/test_type.sh

@@ -4,8 +4,10 @@
 
 # include err() and new() functions and creates $dir
 . ./lib.sh
-fyang=$dir/type.yang
+
 cfg=$dir/conf_yang.xml
+fyang=$dir/type.yang
+
 
 cat <<EOF > $cfg
 <config>
@@ -70,6 +72,57 @@ module example{
          enum down;
       }
    }
+   leaf length1 {
+       type string {
+       length "1";
+      }
+   }
+/*   leaf length2 {
+       type string {
+         length "max";
+      }
+   }
+   leaf length3 {
+       type string {
+         length "min";
+      }
+   }*/
+   leaf length4 {
+       type string {
+         length "4..4000";
+      }
+   }
+/*   leaf length5 {
+       type string {
+         length "min..max";
+      }
+   }*/
+   leaf num1 {
+       type int32 {
+       range "1";
+      }
+   }
+/*   leaf num2 {
+       type int32 {
+         range "min";
+      }
+   }
+   leaf num3 {
+       type int32 {
+         range "max";
+      }
+   }
+*/
+   leaf num4 {
+       type int32 {
+         range "4..4000";
+      }
+   }
+/*   leaf num5 {
+       type int32 {
+         range "min..max";
+      }
+   }*/
 }
 EOF
 

yang/clixon-config@2018-02-12.yang

@@ -304,7 +304,7 @@ module clixon-config {
 	    type boolean;
 	    default false;
 	    description "If set, modifications in validation and commit 
-                         callbacks will be saved into running";
+                         callbacks are written back into the datastore";
 	}
     }
 }