diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c6c40ec..7fd27e05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -v# Clixon Changelog +# Clixon Changelog * [5.4.0](#540) Expected: November * [5.3.0](#530) 27 September 2021 @@ -89,6 +89,10 @@ Developers may need to change their code ### Minor features +* Added sorting of YANG statements + * Some openconfig specs seem to have use/when before a "config" which it depends on. This leads to XML encoding being in the "wrong order. + * When parsing, clixon now sorts container/list statements so that sub-statements with WHEN are put last. + * See [Statements given in "load set" are order dependent](https://github.com/clicon/clixon/issues/287) * Added: [Recursive search CLIXON_YANG_DIR](https://github.com/clicon/clixon/issues/284) * Plugin context check before and after all callbacks. * Check blocked signals and signal handlers @@ -96,6 +100,7 @@ Developers may need to change their code * Any changes to context are logged at loglevel WARNING * [OpenConfig path compression](https://github.com/clicon/clixon/pull/276) * C API: Added set/get pointer API to clixon_data: + * Added json/cli support for cli save/load * clicon_ptr_get(), clicon_ptr_set(), * Restconf YANG PATCH according to RFC 8072 * Changed YANG PATCH enabling: @@ -105,6 +110,8 @@ Developers may need to change their code ### Corrected Bugs +* [Statements given in "load set" are order dependent](https://github.com/clicon/clixon/issues/287) + * Modify ordering of XML encoding to put sub-elements with YANG WHEN statements last * [RPC get-conf method returned some content not specified by select filter](https://github.com/clicon/clixon/issues/281) * Bug introduced when upgrading of list pagination * [type leafref in type union ineffective](https://github.com/clicon/clixon/issues/277) diff --git a/apps/cli/cli_auto.c b/apps/cli/cli_auto.c index 5e4e4f6d..8ca1eafe 100644 --- a/apps/cli/cli_auto.c +++ b/apps/cli/cli_auto.c @@ -142,7 +142,7 @@ tleaf(cxobj *x) return (xml_child_nr_notype(xc, CX_ATTR) == 0); } -/*! Print an XML tree structure to an output stream and encode chars "<>&" +/*! Print an XML tree structure from an auto-cli env to stdout and encode chars "<>&" * * @param[in] xn clicon xml tree * @param[in] level how many spaces to insert before each line @@ -322,6 +322,7 @@ cli_xml2txt(cxobj *xn, * @param[in] prepend Print this text in front of all commands. * @param[in] gt option to steer cli syntax * @param[in] fn Callback to make print function + * @see xml2cli */ int cli_xml2cli(cxobj *xn, diff --git a/apps/cli/cli_common.c b/apps/cli/cli_common.c index 24ed0d3c..76aad286 100644 --- a/apps/cli/cli_common.c +++ b/apps/cli/cli_common.c @@ -772,8 +772,11 @@ compare_dbs(clicon_handle h, * Utility function used by cligen spec file * @param[in] h CLICON handle * @param[in] cvv Vector of variables (where is found) - * @param[in] argv A string: " (merge|replace)" + * @param[in] argv A string: " []" * is name of a variable occuring in "cvv" containing filename + * : merge or replace + * "text"|"xml"|"json"|"cli"|"netconf" (see format_enum) + * * @note that "filename" is local on client filesystem not backend. * @note file is assumed to have a dummy top-tag, eg * @code @@ -798,14 +801,21 @@ load_config_file(clicon_handle h, cxobj *xt = NULL; cxobj *x; cbuf *cbxml; + char *formatstr; + enum format_enum format = FORMAT_XML; - if (cvec_len(argv) != 2){ - if (cvec_len(argv)==1) - clicon_err(OE_PLUGIN, EINVAL, "Got single argument:\"%s\". Expected \",\"", cv_string_get(cvec_i(argv,0))); - else - clicon_err(OE_PLUGIN, EINVAL, "Got %d arguments. Expected: ,", cvec_len(argv)); + if (cvec_len(argv) < 2 || cvec_len(argv) > 4){ + clicon_err(OE_PLUGIN, EINVAL, "Received %d arguments. Expected: ,[,]", + cvec_len(argv)); goto done; } + if (cvec_len(argv) > 2){ + formatstr = cv_string_get(cvec_i(argv, 2)); + if ((int)(format = format_str2int(formatstr)) < 0){ + clicon_err(OE_PLUGIN, 0, "Not valid format: %s", formatstr); + goto done; + } + } varstr = cv_string_get(cvec_i(argv, 0)); opstr = cv_string_get(cvec_i(argv, 1)); if (strcmp(opstr, "merge") == 0) @@ -830,8 +840,20 @@ load_config_file(clicon_handle h, clicon_err(OE_UNIX, errno, "open(%s)", filename); goto done; } - if (clixon_xml_parse_file(fp, YB_NONE, NULL, &xt, NULL) < 0) + switch (format){ + case FORMAT_XML: + if (clixon_xml_parse_file(fp, YB_NONE, NULL, &xt, NULL) < 0) + goto done; + break; + case FORMAT_JSON: + if (clixon_json_parse_file(fp, YB_NONE, NULL, &xt, NULL) < 0) + goto done; + break; + default: + clicon_err(OE_PLUGIN, 0, "format: %s not implemented", formatstr); goto done; + break; + } if (xt == NULL) goto done; if ((cbxml = cbuf_new()) == NULL) @@ -858,13 +880,15 @@ load_config_file(clicon_handle h, return ret; } -/*! Copy database to local file +/*! Copy database to local file as XMLn + * * Utility function used by cligen spec file * @param[in] h CLICON handle * @param[in] cvv variable vector (containing ) - * @param[in] argv a string: " " + * @param[in] argv a string: " []" * is running, candidate, or startup * is name of cligen variable in the "cvv" vector containing file name + * "text"|"xml"|"json"|"cli"|"netconf" (see format_enum) * Note that "filename" is local on client filesystem not backend. * The function can run without a local database * @note The file is saved with dummy top-tag: clicon: @@ -878,33 +902,41 @@ save_config_file(clicon_handle h, cvec *cvv, cvec *argv) { - int retval = -1; - char *filename = NULL; - cg_var *cv; - char *dbstr; - char *varstr; - cxobj *xt = NULL; - cxobj *xerr; - FILE *f = NULL; - - if (cvec_len(argv) != 2){ - if (cvec_len(argv)==1) - clicon_err(OE_PLUGIN, EINVAL, "Got single argument:\"%s\". Expected \",\"", - cv_string_get(cvec_i(argv,0))); - else - clicon_err(OE_PLUGIN, EINVAL, " Got %d arguments. Expected: ,", - cvec_len(argv)); + int retval = -1; + char *filename = NULL; + cg_var *cv; + char *dbstr; + char *varstr; + cxobj *xt = NULL; + cxobj *xc; + cxobj *xerr; + FILE *f = NULL; + enum genmodel_type gt; + char *formatstr; + enum format_enum format = FORMAT_XML; + char *prefix = "set "; /* XXX hardcoded */ + int pretty = 1; /* XXX hardcoded */ + if (cvec_len(argv) < 2 || cvec_len(argv) > 4){ + clicon_err(OE_PLUGIN, EINVAL, "Received %d arguments. Expected: ,[,]", + cvec_len(argv)); goto done; } + if (cvec_len(argv) > 2){ + formatstr = cv_string_get(cvec_i(argv, 2)); + if ((int)(format = format_str2int(formatstr)) < 0){ + clicon_err(OE_PLUGIN, 0, "Not valid format: %s", formatstr); + goto done; + } + } dbstr = cv_string_get(cvec_i(argv, 0)); - varstr = cv_string_get(cvec_i(argv, 1)); if (strcmp(dbstr, "running") != 0 && strcmp(dbstr, "candidate") != 0 && strcmp(dbstr, "startup") != 0) { clicon_err(OE_PLUGIN, 0, "No such db name: %s", dbstr); goto done; } + varstr = cv_string_get(cvec_i(argv, 1)); if ((cv = cvec_find(cvv, varstr)) == NULL){ clicon_err(OE_PLUGIN, 0, "No such var name: %s", varstr); goto done; @@ -929,8 +961,33 @@ save_config_file(clicon_handle h, clicon_err(OE_CFG, errno, "Creating file %s", filename); goto done; } - if (clicon_xml2file(f, xt, 0, 1) < 0) - goto done; + switch (format){ + case FORMAT_XML: + if (clicon_xml2file(f, xt, 0, pretty) < 0) + goto done; + break; + case FORMAT_JSON: + if (xml2json(f, xt, pretty) < 0) + goto done; + break; + case FORMAT_TEXT: + cli_xml2txt(xt, cligen_output, 0); /* tree-formed text */ + break; + case FORMAT_CLI: + if ((gt = clicon_cli_genmodel_type(h)) == GT_ERR) + goto done; + if (xml2cli(f, xt, prefix, gt) < 0) + goto done; + break; + case FORMAT_NETCONF: + fprintf(f, "", + NETCONF_BASE_NAMESPACE); + fprintf(f, "\n"); + if (clicon_xml2file(f, xt, 0, pretty) < 0) + goto done; + fprintf(f, "]]>]]>\n"); + break; + } /* switch */ retval = 0; /* Fall through */ done: diff --git a/apps/cli/cli_show.c b/apps/cli/cli_show.c index 07ba04ec..be1e551e 100644 --- a/apps/cli/cli_show.c +++ b/apps/cli/cli_show.c @@ -442,7 +442,7 @@ cli_show_config1(clicon_handle h, char *prefix = NULL; if (cvec_len(argv) < 3 || cvec_len(argv) > 5){ - clicon_err(OE_PLUGIN, EINVAL, "Got %d arguments. Expected: ,,[,, []]", cvec_len(argv)); + clicon_err(OE_PLUGIN, EINVAL, "Received %d arguments. Expected: ,,[,, []]", cvec_len(argv)); goto done; } diff --git a/example/main/example_cli.cli b/example/main/example_cli.cli index f0a118ce..8afa850b 100644 --- a/example/main/example_cli.cli +++ b/example/main/example_cli.cli @@ -95,10 +95,20 @@ show("Show a particular state of the system"){ } } -save("Save candidate configuration to XML file") ("Filename (local filename)"), save_config_file("candidate","filename"); +save("Save candidate configuration to XML file") ("Filename (local filename)"), save_config_file("candidate","filename", "xml");{ + cli("Save configuration as CLI commands"), save_config_file("candidate","filename", "cli"); + xml("Save configuration as XML"), save_config_file("candidate","filename", "xml"); + json("Save configuration as JSON"), save_config_file("candidate","filename", "json"); +} load("Load configuration from XML file") ("Filename (local filename)"),load_config_file("filename", "replace");{ - replace("Replace candidate with file contents"), load_config_file("filename", "replace"); - merge("Merge file with existent candidate"), load_config_file("filename", "merge"); + replace("Replace candidate with file contents"), load_config_file("filename", "replace");{ + xml("Replace candidate with file containing XML"), load_config_file("","filename", "replace", "xml"); + json("Replace candidate with file containing JSON"), load_config_file("","filename", "replace", "json"); + } + merge("Merge file with existent candidate"), load_config_file("filename", "merge");{ + xml("Merge candidate with file containing XML"), load_config_file("","filename", "merge", "xml"); + json("Merge candidate with file containing JSON"), load_config_file("","filename", "merge", "json"); + } } example("This is a comment") ("Just a random number"), mycallback("myarg"); rpc("example rpc") ("routing instance"), example_client_rpc(""); diff --git a/include/clixon_custom.h b/include/clixon_custom.h index b73d0004..fcf0582d 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -128,3 +128,14 @@ * Consider removing this option after 5.4 */ #undef JSON_CDATA_STRIP + +/*! + * RFC 7950 generally does not specify an XML/JSON encoding order of sub-elements of list or + * containers. See sections 7.5.7 and 7.8.5 + * There are some exceptions, eg rpc/action input/output or list key elements + * Clixon by default encodes them in yang order. + * Set this option if you want sub-elements with WHEN sub-statements last + * See https://github.com/clicon/clixon/issues/287 + * Consider enabling this option permanently after 5.4 + */ +#define YANG_ORDERING_WHEN_LAST diff --git a/lib/clixon/clixon_yang.h b/lib/clixon/clixon_yang.h index 96084036..8df96b14 100644 --- a/lib/clixon/clixon_yang.h +++ b/lib/clixon/clixon_yang.h @@ -271,5 +271,6 @@ int yang_type_cache_set(yang_stmt *ys, yang_stmt *resolved, int options, cvec *patterns, uint8_t fraction); yang_stmt *yang_anydata_add(yang_stmt *yp, char *name); int yang_extension_value(yang_stmt *ys, char *name, char *ns, int *exist, char **value); +int yang_sort_subelements(yang_stmt *ys); #endif /* _CLIXON_YANG_H_ */ diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index 1d9ce117..09d76402 100644 --- a/lib/src/clixon_datastore_write.c +++ b/lib/src/clixon_datastore_write.c @@ -207,6 +207,12 @@ check_body_namespace(cxobj *x0, * * Check if there is a when condition. First try it on the new request (x1), then on the * existing (x0). + * This is according to RFC 7950 8.3.2 NETCONF Processing + * During this processing [of edit-config] : + * o Modification requests for nodes tagged with "when", and the "when" + * condition evaluates to "false". In this case, the server MUST + * reply with an "unknown-element" in the . + * This is somewhat strange since this is different from checking "when" at validation * @param[in] x0p Parent of x0 * @param[in] x1 XML tree which modifies base * @param[in] y0 Yang spec corresponding to xml-node x0. NULL if x0 is NULL diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index c9a8b0a0..5056517a 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -766,7 +766,6 @@ ys_dup(yang_stmt *old) * * @param[in] yorig Existing yang statement * @param[in] yfrom New empty (but created) yang statement - * @retval 0 OK * @retval -1 Error * @code @@ -1346,7 +1345,7 @@ order1(yang_stmt *yp, else { if (!yang_datanode(ys)) continue; - if (ys==y) + if (ys == y) return 1; (*index)++; } @@ -1373,6 +1372,7 @@ yang_order(yang_stmt *y) if (y == NULL) return -1; + /* Some special handling if yp is choice (or case) * if so, the real parent (from an xml point of view) is the parents * parent. @@ -3631,6 +3631,53 @@ yang_extension_value(yang_stmt *ys, return retval; } +#ifdef YANG_ORDERING_WHEN_LAST +/* Sort substatements with when:s last */ +static int +yang_sort_subelements_fn(const void* arg1, + const void* arg2) +{ + yang_stmt *ys1 = *(yang_stmt **)arg1; + yang_stmt *ys2 = *(yang_stmt **)arg2; + int w1; /* ys1 has when substatement */ + int w2; + + w1 = yang_find(ys1, Y_WHEN, NULL) != NULL; + w2 = yang_find(ys2, Y_WHEN, NULL) != NULL; + if (w1 == w2) + return 0; + if (w1) + return 1; + if (w2) + return -1; + return 0; +} +#endif + +/*! Experimental code for sorting YANG children + * + * RFC 7950 7.5.7 and 7.8.5 says that containers and list sub elements are encoded in any order. + * with some exceptions, eg rpc/action input/output or list key elements + * + * Clixon by default encodes them in yang order. But this function can change that. + * For example, openconfig-network-instances.yang requires subelements with "when" statements last. + */ +int +yang_sort_subelements(yang_stmt *ys) +{ + int retval = -1; + +#ifdef YANG_ORDERING_WHEN_LAST + if ((yang_keyword_get(ys) == Y_CONTAINER || + yang_keyword_get(ys) == Y_LIST)){ + qsort(ys->ys_stmt, ys->ys_len, sizeof(ys), yang_sort_subelements_fn); + } +#endif + retval = 0; + // done: + return retval; +} + #ifdef XML_EXPLICIT_INDEX /*! Mark element as search_index in list * @retval 0 OK diff --git a/lib/src/clixon_yang_parse.y b/lib/src/clixon_yang_parse.y index 454f868f..97084d75 100644 --- a/lib/src/clixon_yang_parse.y +++ b/lib/src/clixon_yang_parse.y @@ -249,12 +249,14 @@ yang_parse_exit(clixon_yang_yacc *yy) int ystack_pop(clixon_yang_yacc *yy) { - struct ys_stack *ystack; + struct ys_stack *ystack; if ((ystack = yy->yy_stack) == NULL){ clicon_err(OE_YANG, 0, "ystack is NULL"); return -1; } + if (yang_sort_subelements(ystack->ys_node) < 0) + return -1; yy->yy_stack = ystack->ys_next; free(ystack); return 0; diff --git a/test/test_cli.sh b/test/test_cli.sh index 9933db6b..b86ac290 100755 --- a/test/test_cli.sh +++ b/test/test_cli.sh @@ -110,13 +110,13 @@ new "cli commit" expectpart "$($clixon_cli -1 -f $cfg -l o commit)" 0 "^$" new "cli save" -expectpart "$($clixon_cli -1 -f $cfg -l o save /tmp/foo)" 0 "^$" +expectpart "$($clixon_cli -1 -f $cfg -l o save $dir/foo)" 0 "^$" new "cli delete all" expectpart "$($clixon_cli -1 -f $cfg -l o delete all)" 0 "^$" new "cli load" -expectpart "$($clixon_cli -1 -f $cfg -l o load /tmp/foo)" 0 "^$" +expectpart "$($clixon_cli -1 -f $cfg -l o load $dir/foo)" 0 "^$" new "cli check load" expectpart "$($clixon_cli -1 -f $cfg -l o show conf cli)" 0 "interfaces interface eth/0/0 ipv4 enabled true"