Rewrote parsing of extra config-files to work recursively over structured data

Fixed that modified config variables were not properly overwritten in XML
Added `pretty` parameter to clicon_options_dump
Added testcases for recursive and structure extra config files
This commit is contained in:
Olof hagsand 2023-08-23 11:02:18 +02:00
parent 124afb8788
commit a7ef2c4f12
11 changed files with 135 additions and 65 deletions

View file

@ -60,6 +60,8 @@ Developers may need to change their code
### Corrected Bugs ### Corrected Bugs
* Fixed several issues with extra-config files, including overwriting of structured sub-configs
* including `<restconf>`and m̀ <autoconf>`
* [YANG error when poking on EOS configuration](https://github.com/clicon/clixon-controller/issues/26) * [YANG error when poking on EOS configuration](https://github.com/clicon/clixon-controller/issues/26)
* [CLICON_CONFIGDIR with external subsystems causes endless looping](https://github.com/clicon/clixon/issues/439) * [CLICON_CONFIGDIR with external subsystems causes endless looping](https://github.com/clicon/clixon/issues/439)
* Fixed: ["show configuration devices" and "show configuration devices | display cli" differs](https://github.com/clicon/clixon-controller/issues/24) * Fixed: ["show configuration devices" and "show configuration devices | display cli" differs](https://github.com/clicon/clixon-controller/issues/24)

View file

@ -1014,7 +1014,7 @@ main(int argc,
goto done; goto done;
/* Explicit dump of config (also debug dump below). */ /* Explicit dump of config (also debug dump below). */
if (config_dump){ if (config_dump){
if (clicon_option_dump1(h, stdout, config_dump_format) < 0) if (clicon_option_dump1(h, stdout, config_dump_format, 1) < 0)
goto done; goto done;
} }

View file

@ -862,7 +862,7 @@ main(int argc,
* (there is also debug dump below). * (there is also debug dump below).
*/ */
if (config_dump){ if (config_dump){
if (clicon_option_dump1(h, stdout, config_dump_format) < 0) if (clicon_option_dump1(h, stdout, config_dump_format, 1) < 0)
goto done; goto done;
} }
/* Debug dump of config options */ /* Debug dump of config options */

View file

@ -1138,6 +1138,8 @@ cli_show_auto_mode(clicon_handle h,
} }
/*! Show clixon configuration options as loaded /*! Show clixon configuration options as loaded
*
'* @see clicon_option_dump clicon_option_dump1
*/ */
int int
cli_show_options(clicon_handle h, cli_show_options(clicon_handle h,

View file

@ -879,7 +879,7 @@ main(int argc,
/* Explicit dump of config (also debug dump below). */ /* Explicit dump of config (also debug dump below). */
if (config_dump){ if (config_dump){
if (clicon_option_dump1(h, stdout, config_dump_format) < 0) if (clicon_option_dump1(h, stdout, config_dump_format, 1) < 0)
goto done; goto done;
goto ok; goto ok;
} }

View file

@ -539,7 +539,7 @@ main(int argc,
/* Explicit dump of config (also debug dump below). */ /* Explicit dump of config (also debug dump below). */
if (config_dump){ if (config_dump){
if (clicon_option_dump1(h, stdout, config_dump_format) < 0) if (clicon_option_dump1(h, stdout, config_dump_format, 1) < 0)
goto done; goto done;
goto ok; goto ok;
} }

View file

@ -1297,7 +1297,7 @@ main(int argc,
/* Explicit dump of config (also debug dump below). */ /* Explicit dump of config (also debug dump below). */
if (config_dump){ if (config_dump){
if (clicon_option_dump1(h, stdout, config_dump_format) < 0) if (clicon_option_dump1(h, stdout, config_dump_format, 1) < 0)
goto done; goto done;
goto ok; goto ok;
} }

View file

@ -523,7 +523,7 @@ main(int argc,
goto done; goto done;
if (config_dump){ if (config_dump){
if (clicon_option_dump1(h, stdout, config_dump_format) < 0) if (clicon_option_dump1(h, stdout, config_dump_format, 1) < 0)
goto done; goto done;
goto ok; goto ok;
} }

View file

@ -109,7 +109,7 @@ enum regexp_mode{
int clicon_option_dump(clicon_handle h, int dblevel); int clicon_option_dump(clicon_handle h, int dblevel);
/*! Dump config options on stdio using output format */ /*! Dump config options on stdio using output format */
int clicon_option_dump1(clicon_handle h, FILE *f, int format); int clicon_option_dump1(clicon_handle h, FILE *f, int format, int pretty);
/* Add a clicon options overriding file setting */ /* Add a clicon options overriding file setting */
int clicon_option_add(clicon_handle h, const char *name, char *value); int clicon_option_add(clicon_handle h, const char *name, char *value);

View file

@ -71,6 +71,7 @@
#include "clixon_log.h" #include "clixon_log.h"
#include "clixon_yang.h" #include "clixon_yang.h"
#include "clixon_xml.h" #include "clixon_xml.h"
#include "clixon_xml_sort.h"
#include "clixon_json.h" #include "clixon_json.h"
#include "clixon_text_syntax.h" #include "clixon_text_syntax.h"
#include "clixon_proto.h" #include "clixon_proto.h"
@ -141,6 +142,7 @@ static const map_str2int yang_regexp_map[] = {
* @note CLICON_FEATURE, CLICON_YANG_DIR and CLICON_SNMP_MIB are treated specially since they are lists * @note CLICON_FEATURE, CLICON_YANG_DIR and CLICON_SNMP_MIB are treated specially since they are lists
* @note sub-config structs not shown: eg autocli/restconf * @note sub-config structs not shown: eg autocli/restconf
* @see clicon_option_dump1 different formats * @see clicon_option_dump1 different formats
* @see cli_show_options
*/ */
int int
clicon_option_dump(clicon_handle h, clicon_option_dump(clicon_handle h,
@ -201,6 +203,7 @@ clicon_option_dump(clicon_handle h,
* @param[in] h Clicon handle * @param[in] h Clicon handle
* @param[in] f Output file * @param[in] f Output file
* @param[in] format Dump output format * @param[in] format Dump output format
* @param[in] pretty Set if pretty-print xml and json
* @retval 0 OK * @retval 0 OK
* @retval -1 Error * @retval -1 Error
* @see clicon_option_dump for debug log * @see clicon_option_dump for debug log
@ -208,7 +211,8 @@ clicon_option_dump(clicon_handle h,
int int
clicon_option_dump1(clicon_handle h, clicon_option_dump1(clicon_handle h,
FILE *f, FILE *f,
int format) int format,
int pretty)
{ {
int retval = -1; int retval = -1;
cxobj *xc; cxobj *xc;
@ -216,11 +220,11 @@ clicon_option_dump1(clicon_handle h,
xc = clicon_conf_xml(h); xc = clicon_conf_xml(h);
switch (format){ switch (format){
case FORMAT_XML: case FORMAT_XML:
if (clixon_xml2file(f, xc, 0, 1, "", cligen_output, 0, 0) < 0) if (clixon_xml2file(f, xc, 0, pretty, "", cligen_output, 0, 0) < 0)
goto done; goto done;
break; break;
case FORMAT_JSON: case FORMAT_JSON:
if (clixon_json2file(f, xc, 1, cligen_output, 0, 0) < 0) if (clixon_json2file(f, xc, pretty, cligen_output, 0, 0) < 0)
goto done; goto done;
break; break;
case FORMAT_TEXT: case FORMAT_TEXT:
@ -304,6 +308,68 @@ parse_configfile_one(const char *filename,
return retval; return retval;
} }
/*! Merge XML sub config file into main config-file
*/
static int
merge_control_xml(clicon_handle h,
cxobj *xt,
cxobj *xe)
{
int retval = -1;
char *name;
char *body;
cxobj *xec;
cxobj *xtc;
cxobj *x;
/* One could have used xml_merge, but there are several special conditions */
xec = NULL;
while ((xec = xml_child_each(xe, xec, CX_ELMNT)) != NULL){
if ((name = xml_name(xec)) == NULL)
continue;
if ((body = xml_body(xec)) == NULL){
if ((xtc = xml_find_type(xt, NULL, name, CX_ELMNT)) != NULL){
if (merge_control_xml(h, xtc, xec) < 0)
goto done;
}
else{
/* Copy and add to master */
if ((x = xml_dup(xec)) == NULL)
goto done;
if (xml_addsub(xt, x) < 0)
goto done;
}
continue;
}
/* Ignored from file due to bootstrapping */
if (strcmp(name,"CLICON_CONFIGFILE")==0) {
continue;
}
/* List options for configure options that are lists or leaf-lists: append to main */
if (strcmp(name,"CLICON_FEATURE")==0 ||
strcmp(name,"CLICON_YANG_DIR")==0 ||
strcmp(name,"CLICON_SNMP_MIB")==0){
if ((x = xml_dup(xec)) == NULL)
goto done;
if (xml_addsub(xt, x) < 0)
goto done;
continue;
}
/* Overwrite: remove existing in master if any */
if ((x = xml_find_type(xt, NULL, name, CX_ELMNT)) != NULL)
xml_purge(x);
/* Copy and add to master */
if ((x = xml_dup(xec)) == NULL)
goto done;
if (xml_addsub(xt, x) < 0)
goto done;
}
retval = 0;
done:
return retval;
}
/*! Read filename and set values to global options registry. XML variant. /*! Read filename and set values to global options registry. XML variant.
* *
* @param[in] h Clixon handle * @param[in] h Clixon handle
@ -340,7 +406,6 @@ parse_configfile(clicon_handle h,
char filename1[MAXPATHLEN]; char filename1[MAXPATHLEN];
char *extraconfdir = NULL; char *extraconfdir = NULL;
cxobj *xe = NULL; cxobj *xe = NULL;
cxobj *xec;
DIR *dirp; DIR *dirp;
if (filename == NULL || !strlen(filename)){ if (filename == NULL || !strlen(filename)){
@ -380,35 +445,9 @@ parse_configfile(clicon_handle h,
snprintf(filename1, sizeof(filename1), "%s/%s", extraconfdir, dp[i].d_name); snprintf(filename1, sizeof(filename1), "%s/%s", extraconfdir, dp[i].d_name);
if (parse_configfile_one(filename1, yspec, &xe) < 0) if (parse_configfile_one(filename1, yspec, &xe) < 0)
goto done; goto done;
/* Drain objects from extrafile and replace/append to main */ /* Merge objects from extrafile into main, xml_merge cannot be used due to special cases */
while ((xec = xml_child_i_type(xe, 0, CX_ELMNT)) != NULL) { if (merge_control_xml(h, xt, xe) < 0)
name = xml_name(xec);
body = xml_body(xec);
/* Ignore non-leafs */
if (name == NULL || body == NULL) {
xml_purge(xec);
continue;
}
/* Ignored from file due to bootstrapping */
if (strcmp(name,"CLICON_CONFIGFILE")==0) {
xml_purge(xec);
continue;
}
/* List options for configure options that are lists or leaf-lists: append to main */
if (strcmp(name,"CLICON_FEATURE")==0 ||
strcmp(name,"CLICON_YANG_DIR")==0 ||
strcmp(name,"CLICON_SNMP_MIB")==0){
if (xml_addsub(xt, xec) < 0)
goto done; goto done;
continue;
}
/* Remove existing in master if any */
if ((x = xml_find_type(xt, NULL, name, CX_ELMNT)) != NULL)
xml_purge(x);
/* Append to master (removed from xe) */
if (xml_addsub(xt, xec) < 0)
goto done;
}
if (xe) if (xe)
xml_free(xe); xml_free(xe);
xe = NULL; xe = NULL;
@ -439,6 +478,9 @@ parse_configfile(clicon_handle h,
clicon_err(OE_CFG, 0, "Config file validation: %s", cbuf_get(cbret)); clicon_err(OE_CFG, 0, "Config file validation: %s", cbuf_get(cbret));
goto done; goto done;
} }
/* Add top-level hash options.
* Hashed options are historical and could be replaced with xml, see eg clicon_option_str
*/
x = NULL; x = NULL;
while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) { while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) {
name = xml_name(x); name = xml_name(x);
@ -464,6 +506,7 @@ parse_configfile(clicon_handle h,
strlen(body)+1) == NULL) strlen(body)+1) == NULL)
goto done; goto done;
} }
xml_sort(xt);
retval = 0; retval = 0;
*xconfig = xt; *xconfig = xt;
xt = NULL; xt = NULL;
@ -500,24 +543,35 @@ clicon_option_add(clicon_handle h,
{ {
int retval = -1; int retval = -1;
clicon_hash_t *copt = clicon_options(h); clicon_hash_t *copt = clicon_options(h);
cxobj *x; cxobj *xconfig;
cxobj *xopt;
if (strcmp(name, "CLICON_FEATURE")==0 || if ((xconfig = clicon_conf_xml(h)) == NULL){
strcmp(name, "CLICON_YANG_DIR")==0 ||
strcmp(name, "CLICON_SNMP_MIB")==0){
if ((x = clicon_conf_xml(h)) == NULL){
clicon_err(OE_UNIX, ENOENT, "option %s not found (clicon_conf_xml_set has not been called?)", name); clicon_err(OE_UNIX, ENOENT, "option %s not found (clicon_conf_xml_set has not been called?)", name);
goto done; goto done;
} }
if (clixon_xml_parse_va(YB_NONE, NULL, &x, NULL, "<%s>%s</%s>", if (strcmp(name, "CLICON_FEATURE")==0 ||
strcmp(name, "CLICON_YANG_DIR")==0 ||
strcmp(name, "CLICON_SNMP_MIB")==0){
if (clixon_xml_parse_va(YB_NONE, NULL, &xconfig, NULL, "<%s>%s</%s>",
name, value, name) < 0) name, value, name) < 0)
goto done; goto done;
} }
else{
/* Add/change hash */
if (clicon_hash_add(copt, if (clicon_hash_add(copt,
name, name,
value, value,
strlen(value)+1) == NULL) strlen(value)+1) == NULL)
goto done; goto done;
/* Add/change in clicon_conf_xml */
if ((xopt = xpath_first(xconfig, 0, "%s", name)) != NULL)
xml_purge(xopt);
if (clixon_xml_parse_va(YB_NONE, NULL, &xconfig, NULL, "<%s>%s</%s>",
name, value, name) < 0)
goto done;
}
xml_sort(xconfig);
retval = 0; retval = 0;
done: done:
return retval; return retval;
@ -624,6 +678,7 @@ clicon_options_main(clicon_handle h)
goto done; goto done;
yspec = NULL; yspec = NULL;
/* Set clixon_conf pointer to handle */ /* Set clixon_conf pointer to handle */
xml_sort(xconfig);
if (clicon_conf_xml_set(h, xconfig) < 0) if (clicon_conf_xml_set(h, xconfig) < 0)
goto done; goto done;

View file

@ -10,6 +10,7 @@
# Two options are used for testing: # Two options are used for testing:
# CLICON_MODULE_SET_ID is a single var (replaced) # CLICON_MODULE_SET_ID is a single var (replaced)
# CLICON_FEATURE is a list var (append) # CLICON_FEATURE is a list var (append)
# Check subconfigs, ie /restconf/server-cert-path used since it does not have default
# #
# Magic line must be first in script (see README.md) # Magic line must be first in script (see README.md)
s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
@ -29,14 +30,15 @@ cat <<EOF > $cfg
<CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE> <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR> <CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
<CLICON_YANG_DIR>${YANG_INSTALLDIR}</CLICON_YANG_DIR> <CLICON_YANG_DIR>${YANG_INSTALLDIR}</CLICON_YANG_DIR>
<CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR> <CLICON_CLISPEC_DIR>$dir</CLICON_CLISPEC_DIR>
<CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
<CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
<CLICON_MODULE_SET_ID>1</CLICON_MODULE_SET_ID> <CLICON_MODULE_SET_ID>1</CLICON_MODULE_SET_ID>
<CLICON_FEATURE>test1</CLICON_FEATURE> <CLICON_FEATURE>test1</CLICON_FEATURE>
</clixon-config> </clixon-config>
EOF EOF
# dummy
touch $dir/spec.cli
new "Start without configdir as baseline" new "Start without configdir as baseline"
cat <<EOF > $cfile1 cat <<EOF > $cfile1
<clixon-config xmlns="http://clicon.org/config"> <clixon-config xmlns="http://clicon.org/config">
@ -45,7 +47,7 @@ cat <<EOF > $cfile1
</clixon-config> </clixon-config>
EOF EOF
expectpart "$($clixon_cli -1 -f $cfg show options)" 0 'CLICON_MODULE_SET_ID: "1"' 'CLICON_FEATURE: "test1"' --not-- 'CLICON_FEATURE: "test2"' expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "<CLICON_MODULE_SET_ID>1</CLICON_MODULE_SET_ID>" "<CLICON_FEATURE>test1</CLICON_FEATURE>" --not-- "<CLICON_FEATURE>test2</CLICON_FEATURE>"
new "Start with wrong configdir" new "Start with wrong configdir"
cat <<EOF > $cfg cat <<EOF > $cfg
@ -63,7 +65,7 @@ cat <<EOF > $cfg
</clixon-config> </clixon-config>
EOF EOF
expectpart "$($clixon_cli -1 -f $cfg -l o show options)" 255 "UNIX error: CLICON_CONFIGDIR:" "opendir: No such file or directory" expectpart "$($clixon_cli -1 -f $cfg -l o -C xml)" 255 "UNIX error: CLICON_CONFIGDIR:" "opendir: No such file or directory"
new "Start with wrong configdir -E override" new "Start with wrong configdir -E override"
rm -f $cfile1 rm -f $cfile1
@ -82,7 +84,7 @@ cat <<EOF > $cfg
</clixon-config> </clixon-config>
EOF EOF
expectpart "$($clixon_cli -1 -f $cfg -E $cdir show options)" 0 'CLICON_MODULE_SET_ID: "1"' 'CLICON_FEATURE: "test1"' --not-- 'CLICON_FEATURE: "test2"' expectpart "$($clixon_cli -1 -f $cfg -E $cdir -C xml)" 0 "<CLICON_MODULE_SET_ID>1</CLICON_MODULE_SET_ID>" "<CLICON_FEATURE>test1</CLICON_FEATURE>" --not-- "<CLICON_FEATURE>test2</CLICON_FEATURE>"
new "Start with empty configdir" new "Start with empty configdir"
cat <<EOF > $cfg cat <<EOF > $cfg
@ -97,23 +99,32 @@ cat <<EOF > $cfg
<CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE> <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
<CLICON_MODULE_SET_ID>1</CLICON_MODULE_SET_ID> <CLICON_MODULE_SET_ID>1</CLICON_MODULE_SET_ID>
<CLICON_FEATURE>test1</CLICON_FEATURE> <CLICON_FEATURE>test1</CLICON_FEATURE>
<restconf>
<server-cert-path>foo</server-cert-path>
</restconf>
</clixon-config> </clixon-config>
EOF EOF
expectpart "$($clixon_cli -1 -f $cfg -l o show options)" 0 'CLICON_MODULE_SET_ID: "1"' 'CLICON_FEATURE: "test1"' --not-- 'CLICON_FEATURE: "test2"' expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "<CLICON_MODULE_SET_ID>1</CLICON_MODULE_SET_ID>" "<CLICON_FEATURE>test1</CLICON_FEATURE>" --not-- "<CLICON_FEATURE>test2</CLICON_FEATURE>"
new "Check subconfig"
expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "<restconf>" "<server-cert-path>foo</server-cert-path>"
new "Start with 1 extra configfile" new "Start with 1 extra configfile"
cat <<EOF > $cfile1 cat <<EOF > $cfile1
<clixon-config xmlns="http://clicon.org/config"> <clixon-config xmlns="http://clicon.org/config">
<CLICON_MODULE_SET_ID>2</CLICON_MODULE_SET_ID> <CLICON_MODULE_SET_ID>2</CLICON_MODULE_SET_ID>
<CLICON_FEATURE>test2</CLICON_FEATURE> <CLICON_FEATURE>test2</CLICON_FEATURE>
<autocli> <restconf>
<module-default>false</module-default> <server-cert-path>bar</server-cert-path>
</autocli> </restconf>
</clixon-config> </clixon-config>
EOF EOF
expectpart "$($clixon_cli -1 -f $cfg -l o show options)" 0 'CLICON_MODULE_SET_ID: "2"' 'CLICON_FEATURE: "test1"' 'CLICON_FEATURE: "test2"' expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "<CLICON_MODULE_SET_ID>2</CLICON_MODULE_SET_ID>" "<CLICON_FEATURE>test1</CLICON_FEATURE>" "<CLICON_FEATURE>test2</CLICON_FEATURE>"
new "Check subconfig override"
expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "<restconf>" "<server-cert-path>bar</server-cert-path>"
new "Start with 2 extra configfiles" new "Start with 2 extra configfiles"
cat <<EOF > $cfile2 cat <<EOF > $cfile2
@ -123,10 +134,10 @@ cat <<EOF > $cfile2
</clixon-config> </clixon-config>
EOF EOF
expectpart "$($clixon_cli -1 -f $cfg -l o show options)" 0 'CLICON_MODULE_SET_ID: "3"' 'CLICON_FEATURE: "test1"' 'CLICON_FEATURE: "test2"' 'CLICON_FEATURE: "test3"' expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "<CLICON_MODULE_SET_ID>3</CLICON_MODULE_SET_ID>" "<CLICON_FEATURE>test1</CLICON_FEATURE>" "<CLICON_FEATURE>test2</CLICON_FEATURE>" "<CLICON_FEATURE>test3</CLICON_FEATURE>"
new "Start with 2 extra configfiles + command-line" new "Start with 2 extra configfiles + command-line -C xml"
expectpart "$($clixon_cli -1 -f $cfg -o CLICON_MODULE_SET_ID=4 -o CLICON_FEATURE=test4 -l o show options)" 0 'CLICON_MODULE_SET_ID: "4"' 'CLICON_FEATURE: "test1"' 'CLICON_FEATURE: "test2"' 'CLICON_FEATURE: "test3"' 'CLICON_FEATURE: "test4"' expectpart "$($clixon_cli -1 -f $cfg -o CLICON_MODULE_SET_ID=4 -o CLICON_FEATURE=test4 -C xml)" 0 "<CLICON_MODULE_SET_ID>4</CLICON_MODULE_SET_ID>" "<CLICON_FEATURE>test1</CLICON_FEATURE>" "<CLICON_FEATURE>test2</CLICON_FEATURE>" "<CLICON_FEATURE>test3</CLICON_FEATURE>" "<CLICON_FEATURE>test4</CLICON_FEATURE>"
rm -rf $dir rm -rf $dir