* Fixed: [RESTConf GET for a specific list instance retrieves data from other submodules that have same list name and key value #244](https://github.com/clicon/clixon/issues/244)

* Fixed: Double clixon error messages in yang2cli code
* Sanity checks for submodule belongs-to

CHANGELOG.md

@@ -40,6 +40,10 @@ Developers may need to change their code
 * Native Restconf is now default, not fcgi/nginx
   * That is, to configure with fcgi, you need to explicitly configure: `--with-restconf=fcgi`
 
+### Corrected Bugs
+
+* Fixed: [RESTConf GET for a specific list instance retrieves data from other submodules that have same list name and key value #244](https://github.com/clicon/clixon/issues/244)
+
 ## 5.2.0
 1 July 2021
 

apps/cli/cli_generate.c

@@ -626,8 +626,8 @@ yang2cli_var(clicon_handle h,
 	    completionp = clicon_cli_genmodel_completion(h);
 	if (completionp)
 	    cprintf(cb, "(");
-	if ((retval = yang2cli_var_sub(h, ys, yrestype, helptext, cvtype, 
+	if (yang2cli_var_sub(h, ys, yrestype, helptext, cvtype, 
-				       options, cvv, patterns, fraction_digits, cb)) < 0)
+			     options, cvv, patterns, fraction_digits, cb) < 0)
 	    goto done;
 	if (completionp){
 	    result = cli_expand_var_generate(h, ys, cvtype, 

lib/src/clixon_path.c

@@ -731,6 +731,7 @@ api_path2xpath_cvv(cvec       *api_path,
 		cvk = yang_cvec_get(y); /* Use Y_LIST cache, see ys_populate_list() */
 		cvi = NULL;
 		/* Iterate over individual yang keys  */
+		if (i != offset)
 		    cprintf(xpath, "/");
 		if (xprefix)
 		    cprintf(xpath, "%s:", xprefix);
@@ -752,6 +753,7 @@ api_path2xpath_cvv(cvec       *api_path,
 		}
 		break;
 	    case Y_LEAF_LIST: /* XXX: LOOP? */
+		if (i != offset)
 		    cprintf(xpath, "/");
 		if (xprefix)
 		    cprintf(xpath, "%s:", xprefix);
@@ -1129,6 +1131,9 @@ api_path2xml_vec(char      **vec,
 }
 
 /*! Create xml tree from api-path
+ *
+ * Create an XML tree from "scratch" using api-path, ie no other input than the api-path itself,
+ * ie not from an existing datastore for example.
  * @param[in]     api_path   (Absolute) API-path as defined in RFC 8040
  * @param[in]     yspec      Yang spec
  * @param[in,out] xtop       Incoming XML tree

lib/src/clixon_yang.c

@@ -1414,6 +1414,7 @@ ys_real_module(yang_stmt  *ys,
 {
     int        retval = -1;
     yang_stmt *ym = NULL;
+    yang_stmt *ysubm;
     yang_stmt *yb;
     char      *name;
     yang_stmt *yspec;
@@ -1433,9 +1434,13 @@ ys_real_module(yang_stmt  *ys,
 		clicon_err(OE_YANG, ENOENT, "Belongs-to statement of submodule %s has no argument", yang_argument_get(ym)); /* shouldnt happen */
 		goto done;
 	    }
-	    if ((ym = yang_find_module_by_name(yspec, name)) == NULL)
+	    if ((ysubm = yang_find_module_by_name(yspec, name)) == NULL){
+		clicon_err(OE_YANG, ENOENT, "submodule %s references non-existent module %s in its belongs-to statement",
+			   yang_argument_get(ym), name);
 		goto done;
 	    }
+	    ym = ysubm;
+	}
     }
     *ymod = ym;
     retval = 0;

lib/src/clixon_yang_parse_lib.c

@@ -1032,6 +1032,10 @@ yang_parse_module(clicon_handle h,
 	ymod = NULL;
 	goto done;
     }
+    /* Sanity check that requested module name matches loaded module
+     * If this does not match, the filename and containing module do not match
+     * RFC 7950 Sec 5.2 
+     */
     if ((yrev = yang_find(ymod, Y_REVISION, NULL)) != NULL)
 	revm = cv_uint32_get(yang_cv_get(yrev));
     if (filename2revision(filename, NULL, &revf) < 0)
@@ -1067,11 +1071,15 @@ yang_parse_recurse(clicon_handle h,
     int           retval = -1;
     yang_stmt    *yi = NULL; /* import */
     yang_stmt    *yrev;
+    yang_stmt    *ybelongto;
+    yang_stmt    *yrealmod;
     char         *submodule;
     char         *subrevision;
     yang_stmt    *subymod;
     enum rfc_6020 keyw;
 
+    if (ys_real_module(ymod, &yrealmod) < 0)
+	goto done;
     /* go through all import (modules) and include(submodules) of ysp */
     while ((yi = yn_each(ymod, yi)) != NULL){
 	keyw = yang_keyword_get(yi);
@@ -1091,6 +1099,21 @@ yang_parse_recurse(clicon_handle h,
 	    /* recursive call */
 	    if ((subymod = yang_parse_module(h, submodule, subrevision, ysp)) == NULL)
 		goto done;
+	    /* Sanity check: if submodule, its belongs-to statement shall point to the module */
+	    if (keyw == Y_INCLUDE){
+		ybelongto = yang_find(subymod, Y_BELONGS_TO, NULL);
+		if (ybelongto == NULL){
+		    clicon_err(OE_YANG, ENOENT, "Sub-module \"%s\" does not have a belongs-to statement", submodule); /* shouldnt happen */
+		    goto done;
+		}
+		if (strcmp(yang_argument_get(ybelongto), yang_argument_get(yrealmod)) != 0){
+		    clicon_err(OE_YANG, ENOENT, "Sub-module \"%s\" references module \"%s\" in its belongs-to statement but should reference %s",
+			       submodule,
+			       yang_argument_get(ybelongto),
+			       yang_argument_get(yrealmod));
+		    goto done;
+		}
+	    }
 	    /* Go through its sub-modules recursively */
 	    if (yang_parse_recurse(h, subymod, ysp) < 0){
 		ymod = NULL;

test/test_openconfig.sh

@@ -97,7 +97,7 @@ for f in $files; do
     fi
 done
 
-new "Openconfig test: $clixon_cli -1f $cfg -y $f show version ($m modules)"
+new "Openconfig test: $clixon_cli -1f $cfg show version ($m modules)"
 for f in $files; do
     if [ -n "$(head -1 $f|grep '^module')" ]; then
 	new "$clixon_cli -D $DBG  -1f $cfg -y $f show version"

test/test_openconfig_interfaces.sh

@@ -1,5 +1,6 @@
 #!/usr/bin/env bash
 # Run a system around openconfig interface, ie: openconfig-if-ethernet
+# Note first variant uses ietf-interfaces, maybe remove this?
 
 # Magic line must be first in script (see README.md)
 s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
@@ -22,13 +23,11 @@ cat <<EOF > $cfg
 <clixon-config xmlns="http://clicon.org/config">
   <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
   <CLICON_FEATURE>ietf-netconf:startup</CLICON_FEATURE>
-  <CLICON_YANG_DIR>$OPENCONFIG/third_party/ietf/</CLICON_YANG_DIR>
   <CLICON_YANG_DIR>/usr/local/share/clixon</CLICON_YANG_DIR>
   <CLICON_YANG_DIR>$OCDIR</CLICON_YANG_DIR>
-  <CLICON_YANG_DIR>$OCDIR</CLICON_YANG_DIR>
   <CLICON_YANG_DIR>$OCDIR/interfaces</CLICON_YANG_DIR>
   <CLICON_YANG_DIR>$OCDIR/types</CLICON_YANG_DIR>
-  <CLICON_YANG_DIR>$OCDIR/wifi</CLICON_YANG_DIR>
+  <CLICON_YANG_DIR>$OCDIR/vlan</CLICON_YANG_DIR>
   <CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>	
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
@@ -42,6 +41,7 @@ cat <<EOF > $cfg
 </clixon-config>
 EOF
 
+# First using ietf-interfaces (not openconfig-interfaces)
 # Example yang
 cat <<EOF > $fyang
 module clixon-example{
@@ -99,7 +99,7 @@ fi
 new "wait backend"
 wait_backend
 
-new "$clixon_cli -D $DBG -1f $cfg -y $f show version"
+new "$clixon_cli -D $DBG -1f $cfg show version"
 expectpart "$($clixon_cli -D $DBG -1f $cfg show version)" 0 "${CLIXON_VERSION}"
 
 new "$clixon_netconf -qf $cfg"
@@ -126,6 +126,76 @@ if [ $BE -ne 0 ]; then
     stop_backend -f $cfg
 fi
 
+# Second using openconfig-interfaces instead
+# Example yang
+cat <<EOF > $fyang
+module clixon-example{
+  yang-version 1.1;
+  namespace "urn:example:example";
+  prefix ex;
+
+  import openconfig-vlan {
+    prefix oc-vlan;
+  }
+  import openconfig-if-ethernet {
+    prefix oc-eth;
+  }
+}
+EOF
+
+# Example system
+cat <<EOF > $dir/startup_db
+<config>
+   <interfaces xmlns="http://openconfig.net/yang/interfaces">
+      <interface>
+         <name>eth1</name>
+         <config>
+            <name>eth1</name>
+            <type>ianaift:ethernetCsmacd</type>
+            <mtu>9206</mtu>
+            <enabled>true</enabled>
+            <oc-vlan:tpid xmlns:oc-vlan="http://openconfig.net/yang/vlan">oc-vlan-types:TPID_0X8100</oc-vlan:tpid>
+         </config>
+         <oc-eth:ethernet xmlns:oc-eth="http://openconfig.net/yang/interfaces/ethernet">
+            <oc-eth:config>
+               <oc-eth:mac-address>2c:53:4a:09:59:73</oc-eth:mac-address>
+            </oc-eth:config>
+         </oc-eth:ethernet>
+      </interface>
+   </interfaces>
+</config>
+EOF
+
+if [ $BE -ne 0 ]; then
+    new "kill old backend"
+    sudo clixon_backend -zf $cfg
+    if [ $? -ne 0 ]; then
+	err
+    fi
+    sudo pkill -f clixon_backend # to be sure
+    
+    new "start backend -s startup -f $cfg"
+    start_backend -s startup -f $cfg
+fi
+
+new "wait backend"
+wait_backend
+
+new "$clixon_cli -D $DBG -1f $cfg show version"
+expectpart "$($clixon_cli -D $DBG -1f $cfg show version)" 0 "${CLIXON_VERSION}"
+
+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"