diff --git a/CHANGELOG.md b/CHANGELOG.md index 00c1b25c..0c9c0ee7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,8 +39,7 @@ Expected: Early March 2020 [search](https://clixon-docs.readthedocs.io/en/latest/xml.html#searching-in-xml) ### API changes on existing features (you may need to change your code) -* C-API change: `xml_new()` changed from: - * `xml_new(name, xp, ys)` to `xml_new(name, prefix, xp, type)` +* C-API change: `xml_new()` changed from `xml_new(name, xp, ys)` to `xml_new(name, prefix, xp, type)` * If you have used, `ys`, add `xml_spec_set(x, ys)` after the statement * If you have `xml_type_set(x, TYPE)` or `xml_prefix_set(x, PREFIX)` immediately after the statement, you can remove those and set them directly as: `xml_new(name, PREFIX, xp, TYPE)` * NACM datanode write rules have been changed from looking at datastore being chekend (eg running/candidate/startup) to *only* look at running. @@ -102,6 +101,8 @@ Expected: Early March 2020 ### Corrected Bugs +* Fixed: Datastore read on startup got fixed default values. +* Fixed: Default values only worked on leafs, not typedefs. * Fixed: NACM datanode write problem: read/write/exec default rules did not work. * Fixed [Makefile syntax error *** mixed implicit and normal rules #104](https://github.com/clicon/clixon/issues/104). Make operator `|=` seems not to work on GNU make version < 4. * Yang specs with recursive grouping/use statement is now fixed: instead of stack overflow, you get an error message and an exit diff --git a/lib/clixon/clixon_xml.h b/lib/clixon/clixon_xml.h index eb202d41..eb62e69b 100644 --- a/lib/clixon/clixon_xml.h +++ b/lib/clixon/clixon_xml.h @@ -115,7 +115,7 @@ typedef struct clixon_xml_vec clixon_xvec; /* struct defined in clicon_xvec.c */ #define XML_FLAG_DEL 0x04 /* Node is deleted (commits) or parent deleted rec */ #define XML_FLAG_CHANGE 0x08 /* Node is changed (commits) or child changed rec */ #define XML_FLAG_NONE 0x10 /* Node is added as NONE */ -#define XML_FLAG_DEFAULT 0x20 /* Added as default value @see xml_default*/ +#define XML_FLAG_DEFAULT 0x20 /* Added as default value @see xml_default */ /* * Prototypes diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c index 85435e95..22dcfbb3 100644 --- a/lib/src/clixon_datastore_read.c +++ b/lib/src/clixon_datastore_read.c @@ -754,8 +754,6 @@ xmldb_get0_clear(clicon_handle h, { int retval = -1; - if (clicon_datastore_cache(h) != DATASTORE_CACHE_ZEROCOPY) - goto ok; if (x == NULL) goto ok; /* clear XML tree of defaults */ diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 4bed7c75..85059046 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -784,7 +784,7 @@ xml_tree_prune_flagged(cxobj *xt, x = NULL; xprev = NULL; while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) { - if (xml_flag(x, flag) == test?flag:0){ /* Pass test means purge */ + if (xml_flag(x, flag) == (test?flag:0)){ /* Pass test means purge */ if (xml_purge(x) < 0) goto done; x = xprev; diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index d8cb5865..ab2949cc 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -2,7 +2,8 @@ * ***** BEGIN LICENSE BLOCK ***** - Copyright (C) 2009-2020 Olof Hagsand + Copyright (C) 2009-2019 Olof Hagsand + Copyright (C) 2020 Olof Hagsand and Rubicon Communications, LLC This file is part of CLIXON. @@ -1314,6 +1315,7 @@ ys_populate_leaf(clicon_handle h, char *origtype=NULL; /* original type */ uint8_t fraction_digits; int options = 0x0; + yang_stmt *ytypedef; /* where type is define */ yparent = ys->ys_parent; /* Find parent: list/container */ /* 1. Find type specification and set cv type accordingly */ @@ -1335,8 +1337,12 @@ ys_populate_leaf(clicon_handle h, clicon_err(OE_YANG, errno, "cv_new_set"); goto done; } + /* get parent of where type is defined, can be original object */ + ytypedef = yrestype?yang_parent_get(yrestype):ys; + /* 3. Check if default value. Here we parse the string in the default-stmt * and add it to the leafs cv. + * 3a) First check local default */ if ((ydef = yang_find(ys, Y_DEFAULT, NULL)) != NULL){ if ((cvret = cv_parse1(ydef->ys_argument, cv, &reason)) < 0){ /* error */ @@ -1349,6 +1355,19 @@ ys_populate_leaf(clicon_handle h, goto done; } } + /* 2. then check typedef default */ + else if (ytypedef != ys && + (ydef = yang_find(ytypedef, Y_DEFAULT, NULL)) != NULL) { + if ((cvret = cv_parse1(ydef->ys_argument, cv, &reason)) < 0){ /* error */ + clicon_err(OE_YANG, errno, "parsing cv"); + goto done; + } + if (cvret == 0){ /* parsing failed */ + clicon_err(OE_YANG, errno, "Parsing CV: %s", reason); + free(reason); + goto done; + } + } else{ /* 3b. If not default value, indicate empty cv. */ cv_flag_set(cv, V_UNSET); /* no value (no default) */ diff --git a/test/test_with_default.sh b/test/test_with_default.sh index c6ba8c33..9fb1adea 100755 --- a/test/test_with_default.sh +++ b/test/test_with_default.sh @@ -6,8 +6,9 @@ # (o report-all-tagged) # o trim # o explicit -# Clixon supports explicit, bit the testcases define the other cases as well +# Clixon supports explicit, but the testcases define the other cases as well # in case others will be supported +# XXX I dont think this is correct. Or at least it is not complete. # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -78,14 +79,10 @@ if [ $BE -ne 0 ]; then wait_backend fi -# This is the base XML with thre values in the server: noset, default, other +# This is the base XML with three values in the server: notset, default, other XML='default42notsetother99' -# report-all: doe not consider any data node to be default data, -# even schema default data. (noset is set to 42) -REPORT_ALL='default42notsetother99' - -# report-all: doe not consider any data node to be default data, +# report-all: does not consider any data node to be default data, # even schema default data. (noset is set to 42) REPORT_ALL='default42notset42other99' @@ -96,7 +93,7 @@ TRIM='defaultnotset$XML]]>]]>" "^]]>]]>$" @@ -104,7 +101,6 @@ expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>' "^$EXPLICIT]]>]]>$" - if [ $BE -eq 0 ]; then exit # BE fi diff --git a/test/test_yang_default.sh b/test/test_yang_default.sh new file mode 100755 index 00000000..bf585303 --- /dev/null +++ b/test/test_yang_default.sh @@ -0,0 +1,127 @@ +#!/usr/bin/env bash +# Default handling and datastores. +# There was a bug with loading startup and writing it back to candidate/running +# where default mode in startup being "explicit" was transformed to "report-all" +# according to RFC 6243. +# Ie, all default values were populated. +# This test goes through (all) testcases where clixon writes data back to a datastore, and +# ensures default values are not present in the datastore. +# XXX two errors: +# 1. Running is changed +# XXX 2. type default not set + +# 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 +fyang=$dir/example-default.yang + +cat < $cfg + + $cfg + ietf-netconf:startup + 42 + /usr/local/share/clixon + $IETFRFC + $fyang + /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 + $dir + false + +EOF + +cat < $fyang + module example-default { + namespace "urn:example:default"; + prefix "ex"; + typedef def { + description "Just a base type with a default value of 42"; + type int32; + default 42; + } + container a{ + list b { + key c; + leaf c{ + type string; + } + leaf d1 { + description "direct default"; + type string; + default "foo"; + } + leaf d2 { /* <-- ys */ + description "default in type"; + type def; + } + } + } + } +EOF + +testrun(){ + + # Initial data (default y not given) + XML='0' + + db=startup + if [ $db = startup ]; then + sudo echo "$XML" > $dir/startup_db + fi + if [ $BE -ne 0 ]; then # Bring your own backend + new "kill old backend" + sudo clixon_backend -zf $cfg + if [ $? -ne 0 ]; then + err + fi + new "start backend -s $db -f $cfg" + start_backend -s $db -f $cfg + fi + + new "waiting" + wait_backend + + # permission kludges + sudo chmod 666 $dir/running_db + sudo chmod 666 $dir/startup_db + + new "Checking startup unchanged" + ret=$(diff $dir/startup_db <(echo "$XML")) + if [ $? -ne 0 ]; then + err "$XML" "$ret" + fi + + new "Checking running unchanged" + ret=$(diff $dir/running_db <(echo -n "$XML")) + if [ $? -ne 0 ]; then + err "$XML" "$ret" + fi + + new "check running defaults" + expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' '^0foo42]]>]]>$' + + if [ $BE -ne 0 ]; then # Bring your own backend + 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 +} # testrun + +testrun + +rm -rf $dir