From 344786a97129a11ea3c688b009f645fdc4701ab1 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 11 Apr 2024 12:02:08 +0200 Subject: [PATCH] Fixed: [Duplicate config files in configdir causes merge problems -> set ? = NULL](https://github.com/clicon/clixon/issues/510) Made file-dir match more exact, eg .cli not ?cli Added -1 as any arg to xml_rm_children() --- CHANGELOG.md | 4 ++ apps/cli/cli_plugin.c | 2 +- example/main/example_pipe.cli | 2 +- lib/clixon/clixon_file.h | 1 - lib/src/clixon_file.c | 2 +- lib/src/clixon_options.c | 45 +++++++++++++++-------- lib/src/clixon_plugin.c | 2 +- lib/src/clixon_xml.c | 2 +- lib/src/clixon_yang_parse_lib.c | 2 +- test/test_config_dir.sh | 34 +++++++++++++++-- test/test_config_dump.sh | 4 +- yang/clixon/clixon-config@2024-04-01.yang | 4 +- 12 files changed, 76 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f359ccad..73a90885 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ Expected: June 2024 * New `clixon-lib@2024-04-01.yang` revision - Added: Default format +### Corrected Bugs + +* Fixed: [Duplicate config files in configdir causes merge problems -> set ? = NULL](https://github.com/clicon/clixon/issues/510) + ## 7.0.1 3 April 2024 diff --git a/apps/cli/cli_plugin.c b/apps/cli/cli_plugin.c index 93139822..eec5e28a 100644 --- a/apps/cli/cli_plugin.c +++ b/apps/cli/cli_plugin.c @@ -413,7 +413,7 @@ clispec_load(clixon_handle h) /* Load all clispec .cli files in directory */ if (clispec_dir){ /* Get directory list of files */ - if ((ndp = clicon_file_dirent(clispec_dir, &dp, "(.cli)$", S_IFREG)) < 0) + if ((ndp = clicon_file_dirent(clispec_dir, &dp, "\\.cli$", S_IFREG)) < 0) goto done; /* Load the syntax parse trees into cli_syntax stx structure */ for (i = 0; i < ndp; i++) { diff --git a/example/main/example_pipe.cli b/example/main/example_pipe.cli index 3b85329d..26f127c0 100644 --- a/example/main/example_pipe.cli +++ b/example/main/example_pipe.cli @@ -43,5 +43,5 @@ CLICON_MODE="|example_pipe"; # Must start with | json("JSON"), pipe_showas_fn("json"); text("Text curly braces"), pipe_showas_fn("text"); } - save("Save configuration to file") ("Filename (local filename)"), pipe_save_file("filename"); + save("Save to file") ("Local filename"), pipe_save_file("filename"); } diff --git a/lib/clixon/clixon_file.h b/lib/clixon/clixon_file.h index 0da646a6..1c7a6fa3 100644 --- a/lib/clixon/clixon_file.h +++ b/lib/clixon/clixon_file.h @@ -39,7 +39,6 @@ #ifndef _CLIXON_FILE_H_ #define _CLIXON_FILE_H_ - int clicon_file_dirent(const char *dir, struct dirent **ent, const char *regexp, mode_t type); int clicon_files_recursive(const char *dir, const char *regexp, cvec *cvv); diff --git a/lib/src/clixon_file.c b/lib/src/clixon_file.c index 462b86cc..8c9257d7 100644 --- a/lib/src/clixon_file.c +++ b/lib/src/clixon_file.c @@ -177,7 +177,7 @@ clicon_files_recursive(const char *dir, * @code * char *dir = "/root/fs"; * struct dirent *dp; - * if ((ndp = clicon_file_dirent(dir, &dp, "(.so)$", S_IFREG)) < 0) + * if ((ndp = clicon_file_dirent(dir, &dp, "\\.so$", S_IFREG)) < 0) * return -1; * for (i = 0; i < ndp; i++) * do something with dp[i].d_name; diff --git a/lib/src/clixon_options.c b/lib/src/clixon_options.c index 4e8d8e6f..fcdd33da 100644 --- a/lib/src/clixon_options.c +++ b/lib/src/clixon_options.c @@ -295,6 +295,12 @@ parse_configfile_one(clixon_handle h, } /*! Merge XML sub config file into main config-file + * + * @param[in] h Clixon handle + * @param[in] xt Existing target XML + * @param[in] xe Source XML (merge this to xt) + * @retval 0 OK + * @retval -1 Error */ static int merge_control_xml(clixon_handle h, @@ -307,6 +313,7 @@ merge_control_xml(clixon_handle h, cxobj *xec; cxobj *xtc; cxobj *x; + // int ret; /* One could have used xml_merge, but there are several special conditions */ xec = NULL; @@ -315,7 +322,9 @@ merge_control_xml(clixon_handle h, 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) + if (xml_rm_children(xtc, -1) < 0) + goto done; + if (xml_copy(xec, xtc) < 0) goto done; } else{ @@ -332,25 +341,31 @@ merge_control_xml(clixon_handle h, 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 (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; + /* Replace existing */ + xtc = xml_find_type(xt, NULL, name, CX_ELMNT); + if (xtc != NULL){ + if (xml_rm_children(xtc, -1) < 0) + goto done; + if (xml_copy(xec, xtc) < 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; + } } - retval = 0; done: return retval; @@ -423,7 +438,7 @@ parse_configfile(clixon_handle h, goto done; } closedir(dirp); - if((ndp = clicon_file_dirent(extraconfdir, &dp, NULL, S_IFREG)) < 0) /* Read dir */ + if((ndp = clicon_file_dirent(extraconfdir, &dp, "\\.xml$", S_IFREG)) < 0) /* Read dir */ goto done; /* Loop through files */ for (i = 0; i < ndp; i++){ @@ -489,7 +504,7 @@ parse_configfile(clixon_handle h, strlen(body)+1) == NULL) goto done; } - xml_sort(xt); + xml_sort_recurse(xt); retval = 0; *xconfig = xt; xt = NULL; diff --git a/lib/src/clixon_plugin.c b/lib/src/clixon_plugin.c index 4213baaa..a7101234 100644 --- a/lib/src/clixon_plugin.c +++ b/lib/src/clixon_plugin.c @@ -424,7 +424,7 @@ clixon_plugins_load(clixon_handle h, goto done; } /* Get plugin objects names from plugin directory */ - if((ndp = clicon_file_dirent(dir, &dp, regexp?regexp:"(.so)$", S_IFREG)) < 0) + if((ndp = clicon_file_dirent(dir, &dp, regexp?regexp:"\\.so$", S_IFREG)) < 0) goto done; /* Load all plugins */ for (i = 0; i < ndp; i++) { diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 0c46bccf..3f9121b2 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -1575,7 +1575,7 @@ xml_rm_children(cxobj *xp, return 0; for (i=0; i $cfg @@ -39,6 +42,8 @@ EOF # dummy touch $dir/spec.cli +new "test params: -f $cfg" + new "Start without configdir as baseline" cat < $cfile1 @@ -139,6 +144,29 @@ expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "34" "test1" "test2" "test3" "test4" +# Ensure two sub-dirs (eg ) only last is present +cat < $cfile3 + + + nisse + + +EOF +new "Last replaces first" +expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "" "nisse" --not-- "bar" + +# Ensure file without .xml suffix is not read +cat < $cfile4 + + + laban + + +EOF + +new "File without .xml is not read" +expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "" "nisse" --not-- "laban" + rm -rf $dir new "endtest" diff --git a/test/test_config_dump.sh b/test/test_config_dump.sh index f8f4086a..b2581af1 100755 --- a/test/test_config_dump.sh +++ b/test/test_config_dump.sh @@ -35,7 +35,7 @@ cat < $cfg EOF -cat < $cfdir/extra +cat < $cfdir/extra.xml extradir @@ -55,6 +55,7 @@ module example { } EOF +new "test params: -f $cfg" if [ $BE -ne 0 ]; then # kill old backend (if any) new "kill old backend" @@ -64,7 +65,6 @@ if [ $BE -ne 0 ]; then fi fi - # Extra cmdline opts, first is overwritten, second appended CMDOPTS='-o CLICON_MODULE_SET=42 -o CLICON_FEATURE="cmdline"' diff --git a/yang/clixon/clixon-config@2024-04-01.yang b/yang/clixon/clixon-config@2024-04-01.yang index 1056faaf..50efacd1 100644 --- a/yang/clixon/clixon-config@2024-04-01.yang +++ b/yang/clixon/clixon-config@2024-04-01.yang @@ -469,7 +469,9 @@ module clixon-config { AFTER the main config file (CLICON_CONFIGFILE) in the following way: - leaf values are overwritten - leaf-list values are appended - The files in this directory will be loaded alphabetically. + The files in this directory are loaded alphabetically. + Only files ending with .xml are read + Sub-structures, eg are replaced with the latest (alphabetically) If the dir is given but does not exist will result in an error. You can override file setting with -E command-line option. Note that due to bootstraping this value is only meaningful in the main config file";