diff --git a/CHANGELOG.md b/CHANGELOG.md index 280aa77b..dde41a15 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/apps/cli/cli_generate.c b/apps/cli/cli_generate.c index 7712ccfb..701288cc 100644 --- a/apps/cli/cli_generate.c +++ b/apps/cli/cli_generate.c @@ -626,18 +626,18 @@ 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, - options, cvv, patterns, fraction_digits, cb)) < 0) + if (yang2cli_var_sub(h, ys, yrestype, helptext, cvtype, + options, cvv, patterns, fraction_digits, cb) < 0) goto done; if (completionp){ result = cli_expand_var_generate(h, ys, cvtype, options, fraction_digits, cb); if (result < 0) - goto done; + goto done; if (result == 0) - yang2cli_helptext(cb, helptext); - cprintf(cb, ")"); + yang2cli_helptext(cb, helptext); + cprintf(cb, ")"); } } retval = 0; diff --git a/lib/src/clixon_path.c b/lib/src/clixon_path.c index 38d62300..b7127939 100644 --- a/lib/src/clixon_path.c +++ b/lib/src/clixon_path.c @@ -731,7 +731,8 @@ 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 */ - cprintf(xpath, "/"); + if (i != offset) + cprintf(xpath, "/"); if (xprefix) cprintf(xpath, "%s:", xprefix); cprintf(xpath, "%s", name); @@ -752,7 +753,8 @@ api_path2xpath_cvv(cvec *api_path, } break; case Y_LEAF_LIST: /* XXX: LOOP? */ - cprintf(xpath, "/"); + if (i != offset) + cprintf(xpath, "/"); if (xprefix) cprintf(xpath, "%s:", xprefix); cprintf(xpath, "%s", name); @@ -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 diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 69f37a45..57f5bece 100644 --- a/lib/src/clixon_yang.c +++ b/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,8 +1434,12 @@ 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; diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index 629860e4..18c21fc4 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/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) @@ -1064,14 +1068,18 @@ yang_parse_recurse(clicon_handle h, yang_stmt *ymod, yang_stmt *ysp) { - int retval = -1; - yang_stmt *yi = NULL; /* import */ - yang_stmt *yrev; - char *submodule; - char *subrevision; - yang_stmt *subymod; + 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; diff --git a/test/test_openconfig.sh b/test/test_openconfig.sh index f974ff96..f7c23e2c 100755 --- a/test/test_openconfig.sh +++ b/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" diff --git a/test/test_openconfig_interfaces.sh b/test/test_openconfig_interfaces.sh index 7f24fffc..1065f5cb 100755 --- a/test/test_openconfig_interfaces.sh +++ b/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 < $cfg $cfg ietf-netconf:startup - $OPENCONFIG/third_party/ietf/ /usr/local/share/clixon $OCDIR - $OCDIR $OCDIR/interfaces $OCDIR/types - $OCDIR/wifi + $OCDIR/vlan $fyang /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/cli @@ -42,6 +41,7 @@ cat < $cfg EOF +# First using ietf-interfaces (not openconfig-interfaces) # Example yang cat < $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 < $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 < $dir/startup_db + + + + eth1 + + eth1 + ianaift:ethernetCsmacd + 9206 + true + oc-vlan-types:TPID_0X8100 + + + + 2c:53:4a:09:59:73 + + + + + +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"