From 5af7ea9b38fbf1382c201cf0721b3de553974af1 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Mon, 12 Apr 2021 21:03:20 +0200 Subject: [PATCH] * Added check of old config in upgrade scenarios. * Added to clixon-config.yang `CLICON_XMLDB_UPGRADE_CHECKOLD`, set to false to keep less strict check --- CHANGELOG.md | 8 ++++--- apps/backend/backend_commit.c | 19 ++++++++++++--- lib/src/clixon_datastore_read.c | 29 ++++++++++++++++------- lib/src/clixon_yang_module.c | 7 ++---- test/test_cli.sh | 4 ++-- test/test_datastore_repair.sh | 3 ++- test/test_nacm_default.sh | 1 + test/test_restconf_basic_auth.sh | 24 +------------------ test/test_restconf_ssl_certs.sh | 1 + test/test_upgrade_auto.sh | 1 + test/test_upgrade_failsafe.sh | 1 + test/test_upgrade_interfaces.sh | 1 + test/test_upgrade_module.sh | 1 + test/test_upgrade_quit.sh | 2 ++ test/test_upgrade_simple.sh | 1 + yang/clixon/clixon-config@2021-03-08.yang | 15 ++++++++++-- 16 files changed, 71 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8c222a8..65d0b97a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,10 +33,11 @@ Expected: April ### New features -* Add multiple yang support also for obsolete versions - * This means that files and datastores supporting modstate also looks for deleted or updated yang modules. +* Add multiple yang support also for old/previous versions + * This means that files and datastores supporting modstate also look for deleted or updated yang modules. * A stricter binding which gives error if loading outdated YANG file does not exist. - + * Keep old behavior: dont check old config file: set `CLICON_XMLDB_UPGRADE_CHECKOLD` to false. + ### API changes on existing protocol/config features * Native RESTCONF mode @@ -55,6 +56,7 @@ Expected: April * New clixon-config@2020-03-08.yang revision * Added: `CLICON_NETCONF_HELLO_OPTIONAL` * Added: `CLICON_CLI_AUTOCLI_EXCLUDE` + * Added: `CLICON_XMLDB_UPGRADE_CHECKOLD` ### C/CLI-API changes on existing features diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index 08734eb6..13d1b893 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -192,6 +192,7 @@ startup_common(clicon_handle h, cxobj *xt = NULL; cxobj *x; cxobj *xret = NULL; + cxobj *xerr = NULL; /* If CLICON_XMLDB_MODSTATE is enabled, then get the db XML with * potentially non-matching module-state in msdiff @@ -203,9 +204,19 @@ startup_common(clicon_handle h, /* Get the startup datastore WITHOUT binding to YANG, sorting and default setting. * It is done below, later in this function */ - if ((ret = xmldb_get0(h, db, YB_NONE, NULL, "/", 0, &xt, msdiff, NULL)) < 0) - goto done; - /* ret should not be 0 */ + if (clicon_option_bool(h, "CLICON_XMLDB_UPGRADE_CHECKOLD")){ + if ((ret = xmldb_get0(h, db, YB_MODULE, NULL, "/", 0, &xt, msdiff, &xerr)) < 0) + goto done; + if (ret == 0){ /* ret should not be 0 */ + if (clicon_xml2cbuf(cbret, xerr, 0, 0, -1) < 0) + goto done; + goto fail; + } + } + else { + if (xmldb_get0(h, db, YB_NONE, NULL, "/", 0, &xt, msdiff, &xerr) < 0) + goto done; + } if ((yspec = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_YANG, 0, "Yang spec not set"); goto done; @@ -315,6 +326,8 @@ startup_common(clicon_handle h, ok: retval = 1; done: + if (xerr) + xml_free(xerr); if (xret) xml_free(xret); if (xt) diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c index 93ec02a7..7fbfdd13 100644 --- a/lib/src/clixon_datastore_read.c +++ b/lib/src/clixon_datastore_read.c @@ -475,7 +475,12 @@ xmldb_readfile(clicon_handle h, clicon_err(OE_UNIX, errno, "open(%s)", dbfile); goto done; } - /* ret == 0 should not happen with YB_NONE. Binding is done later */ + /* Read whole datastore file on the form: + * + * modstate* # this is analyzed, stripped and returned as msdiff in text_read_modstate + * config* + * + * ret == 0 should not happen with YB_NONE. Binding is done later */ if (strcmp(format, "json")==0){ if (clixon_json_parse_file(fp, YB_NONE, yspec, &x0, xerr) < 0) goto done; @@ -502,17 +507,17 @@ xmldb_readfile(clicon_handle h, xml_flag_set(x0, XML_FLAG_TOP); if (xml_child_nr(x0) == 0 && de) de->de_empty = 1; - - /* Datastore files may contain module-state defining - * which modules are used in the file. - * Strip module-state and return msdiff - */ + /* Check if we support modstate */ if (clicon_option_bool(h, "CLICON_XMLDB_MODSTATE")) if ((msdiff = modstate_diff_new()) == NULL) goto done; if ((x = xml_find_type(x0, NULL, "modules-state", CX_ELMNT)) != NULL) if ((xmodfile = xml_dup(x)) == NULL) goto done; + /* Datastore files may contain module-state defining + * which modules are used in the file. + * Strip module-state, analyze it with CHANGE/ADD/RM and return msdiff + */ if (text_read_modstate(h, yspec, x0, msdiff) < 0) goto done; if (yb == YB_MODULE){ @@ -534,6 +539,7 @@ xmldb_readfile(clicon_handle h, /* Extract revision */ if ((rev = xml_find_body(xmsd, "revision")) == NULL) continue; + /* Add old/deleted yangs not present in the loaded/running yangspec. */ if ((ymod = yang_find_module_by_namespace_revision(yspec, ns, rev)) == NULL){ /* Append it */ if (yang_spec_parse_module(h, name, rev, yspec) < 0){ @@ -575,13 +581,15 @@ xmldb_readfile(clicon_handle h, goto done; } } - } + } /* if msdiff */ /* xml looks like: ... actually YB_MODULE_NEXT */ if ((ret = xml_bind_yang(x0, YB_MODULE, yspec1?yspec1:yspec, xerr)) < 0) goto done; if (ret == 0) goto fail; + if (xml_sort_recurse(x0) < 0) + goto done; } if (xp){ *xp = x0; @@ -1026,11 +1034,16 @@ xmldb_get(clicon_handle h, * @note Use of 1 for OK * @code * cxobj *xt; - * if (xmldb_get0(h, "running", YB_MODULE, nsc, "/interface[name="eth"]", 0, &xt, NULL, NULL) < 0) + * cxobj *xerr = NULL; + * if (xmldb_get0(h, "running", YB_MODULE, nsc, "/interface[name="eth"]", 0, &xt, NULL, &xerr) < 0) * err; + * if (ret == 0){ # Not if YB_NONE + * # Error handling + * } * ... * xmldb_get0_clear(h, xt); # Clear tree from default values and flags * xmldb_get0_free(h, &xt); # Free tree + * xml_free(xerr); * @endcode * @see xml_nsctx_node to get a XML namespace context from XML tree * @see xmldb_get for a copy version (old-style) diff --git a/lib/src/clixon_yang_module.c b/lib/src/clixon_yang_module.c index 0e256fda..03e9695c 100644 --- a/lib/src/clixon_yang_module.c +++ b/lib/src/clixon_yang_module.c @@ -376,11 +376,8 @@ yang_modules_state_get(clicon_handle h, /*! For single module state with namespace, get revisions and send upgrade callbacks * @param[in] h Clicon handle * @param[in] xt Top-level XML tree to be updated (includes other ns as well) - * @param[in] xd XML module state diff (for one yang module) - * @param[in] xvec Help vector where to store XML child nodes (??) - * @param[in] xlen Length of xvec - * @param[in] ns0 Namespace of module state we are looking for - * @param[in] op add,del, or mod + * @param[in] xmod XML module state diff (for one yang module) + * @param[in] ns Namespace of module state we are looking for * @param[out] cbret Netconf error message if invalid * @retval 1 OK * @retval 0 Validation failed (cbret set) diff --git a/test/test_cli.sh b/test/test_cli.sh index 281d38d1..df69ad13 100755 --- a/test/test_cli.sh +++ b/test/test_cli.sh @@ -116,11 +116,11 @@ new "cli check load" expectpart "$($clixon_cli -1 -f $cfg -l o show conf cli)" 0 "interfaces interface eth/0/0 ipv4 enabled true" new "cli debug set" -expectpart "$($clixon_cli -1 -f $cfg -l o debug level 1)" 0 "^$" +expectpart "$($clixon_cli -1 -f $cfg -l o debug cli 1)" 0 "^$" # How to test this? new "cli debug reset" -expectpart "$($clixon_cli -1 -f $cfg -l o debug level 0)" 0 "^$" +expectpart "$($clixon_cli -1 -f $cfg -l o debug cli 0)" 0 "^$" new "cli rpc" # We dont know which message-id the cli app uses diff --git a/test/test_datastore_repair.sh b/test/test_datastore_repair.sh index 0ca1f1a9..dc537e87 100755 --- a/test/test_datastore_repair.sh +++ b/test/test_datastore_repair.sh @@ -2,7 +2,7 @@ # Test of the general-purpose (raw) upgrade mechanism. # Input is a startup db without mod-state info. # It has wrong namespace bindings and needs to remove some nodes -# Output is a valid config woith correct namespaces and removed nods +# Output is a valid config with correct namespaces and removed nods # The code for this is in the main example backend plugin. # Magic line must be first in script (see README.md) @@ -26,6 +26,7 @@ cat < $cfg /usr/local/var/$APPNAME/$APPNAME.pidfile $dir true + false /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/cli $APPNAME diff --git a/test/test_nacm_default.sh b/test/test_nacm_default.sh index 69fbbc92..ee515a08 100755 --- a/test/test_nacm_default.sh +++ b/test/test_nacm_default.sh @@ -31,6 +31,7 @@ cat < $cfg /usr/local/lib/$APPNAME/backend /usr/local/var/$APPNAME/$APPNAME.pidfile $dir + true internal true $format diff --git a/test/test_restconf_basic_auth.sh b/test/test_restconf_basic_auth.sh index 42f5b1cc..3ab2ef80 100755 --- a/test/test_restconf_basic_auth.sh +++ b/test/test_restconf_basic_auth.sh @@ -46,29 +46,7 @@ cat < $cfg /usr/local/var/$APPNAME/$APPNAME.sock /usr/local/var/$APPNAME/$APPNAME.pidfile $dir - true - internal - $anonymous - -EOF - -# Start with common config, then append fcgi/native specific config -cat < $cfg - - $cfg - ietf-netconf:startup - /usr/local/share/clixon - $IETFRFC - $fyang - /usr/local/lib/$APPNAME/clispec - /usr/local/lib/$APPNAME/backend - example_backend.so$ - /usr/local/lib/$APPNAME/restconf - /usr/local/lib/$APPNAME/cli - $APPNAME - /usr/local/var/$APPNAME/$APPNAME.sock - /usr/local/var/$APPNAME/$APPNAME.pidfile - $dir + true true internal $anonymous diff --git a/test/test_restconf_ssl_certs.sh b/test/test_restconf_ssl_certs.sh index 55f26163..2dd748eb 100755 --- a/test/test_restconf_ssl_certs.sh +++ b/test/test_restconf_ssl_certs.sh @@ -173,6 +173,7 @@ cat < $cfg /usr/local/var/$APPNAME/$APPNAME.sock /usr/local/var/$APPNAME/$APPNAME.pidfile $dir + true true internal diff --git a/test/test_upgrade_auto.sh b/test/test_upgrade_auto.sh index cef59fb9..b27dc795 100755 --- a/test/test_upgrade_auto.sh +++ b/test/test_upgrade_auto.sh @@ -183,6 +183,7 @@ cat < $cfg true true $changelog + false /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/cli $APPNAME diff --git a/test/test_upgrade_failsafe.sh b/test/test_upgrade_failsafe.sh index b2b5f04d..5fe6441f 100755 --- a/test/test_upgrade_failsafe.sh +++ b/test/test_upgrade_failsafe.sh @@ -105,6 +105,7 @@ cat < $cfg /usr/local/var/$APPNAME/$APPNAME.pidfile $dir true + false /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/cli $APPNAME diff --git a/test/test_upgrade_interfaces.sh b/test/test_upgrade_interfaces.sh index cb12aae9..2c3ca4df 100755 --- a/test/test_upgrade_interfaces.sh +++ b/test/test_upgrade_interfaces.sh @@ -243,6 +243,7 @@ cat < $cfg $dir true false + false /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/cli $APPNAME diff --git a/test/test_upgrade_module.sh b/test/test_upgrade_module.sh index 66fe4802..cc5bb13c 100755 --- a/test/test_upgrade_module.sh +++ b/test/test_upgrade_module.sh @@ -65,6 +65,7 @@ cat < $cfg /usr/local/var/$APPNAME/$APPNAME.pidfile $dir true + false /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/cli $APPNAME diff --git a/test/test_upgrade_quit.sh b/test/test_upgrade_quit.sh index 3e6c5c7e..e674680a 100755 --- a/test/test_upgrade_quit.sh +++ b/test/test_upgrade_quit.sh @@ -242,6 +242,7 @@ cat < $cfg $cfg ietf-netconf:startup /usr/local/share/clixon + $dir interfaces:if-mib $dir /usr/local/var/$APPNAME/$APPNAME.sock @@ -251,6 +252,7 @@ cat < $cfg true false false + true /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/cli $APPNAME diff --git a/test/test_upgrade_simple.sh b/test/test_upgrade_simple.sh index 3b549933..af39bb91 100755 --- a/test/test_upgrade_simple.sh +++ b/test/test_upgrade_simple.sh @@ -25,6 +25,7 @@ cat < $cfg /usr/local/var/hello.pidfile $dir true + false init false diff --git a/yang/clixon/clixon-config@2021-03-08.yang b/yang/clixon/clixon-config@2021-03-08.yang index 5f30e9d1..eb217e2e 100644 --- a/yang/clixon/clixon-config@2021-03-08.yang +++ b/yang/clixon/clixon-config@2021-03-08.yang @@ -46,8 +46,9 @@ module clixon-config { revision 2021-03-08 { description "Added option: - CLICON_NETCONF_HELLO_OPTIONAL; - CLICON_CLI_AUTOCLI_EXCLUDE"; + CLICON_NETCONF_HELLO_OPTIONAL + CLICON_CLI_AUTOCLI_EXCLUDE + CLICON_XMLDB_UPGRADE_CHECKOLD"; } revision 2020-12-30 { description @@ -779,6 +780,16 @@ module clixon-config { yang modules match. See also CLICON_MODULE_LIBRARY_RFC7895"; } + leaf CLICON_XMLDB_UPGRADE_CHECKOLD { + type boolean; + default true; + description + "Controls behavior of check of startup in upgrade scenarios. + If set, yang bind and check datastore syntax against the old Yang. + The old yang must be accessible via YANG_DIR. + Will fail startup if old yang not found or if old config does not match. + If not set, no yang check of old config is made until it is upgraded to new yang."; + } leaf CLICON_XML_CHANGELOG { type boolean; default false;