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

@@ -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
 

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++) {

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:string>("Filename (local filename)"), pipe_save_file("filename");
+   save("Save to file") <filename:string>("Local filename"), pipe_save_file("filename");
 }

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);

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;

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 ||
+        if (strcmp(name,"CLICON_FEATURE") == 0 ||
-            strcmp(name,"CLICON_YANG_DIR")==0 ||
+            strcmp(name,"CLICON_YANG_DIR") == 0 ||
-            strcmp(name,"CLICON_SNMP_MIB")==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 */
+        /* Replace existing  */
-        if ((x = xml_find_type(xt, NULL, name, CX_ELMNT)) != NULL)
+        xtc = xml_find_type(xt, NULL, name, CX_ELMNT);
-            xml_purge(x);
+        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;

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++) {

lib/src/clixon_xml.c

@@ -1575,7 +1575,7 @@ xml_rm_children(cxobj          *xp,
         return 0;
     for (i=0; i<xml_child_nr(xp);){
         xc = xml_child_i(xp, i);
-        if (xml_type(xc) != type){
+        if (type != -1 && xml_type(xc) != type){
             i++;
             continue;
         }

lib/src/clixon_yang_parse_lib.c

@@ -1682,7 +1682,7 @@ yang_spec_load_dir(clixon_handle h,
      * a@2000-01-01.yang, 
      * a@2111-11-11.yang
      */
-    if((ndp = clicon_file_dirent(dir, &dp, "(.yang)$", S_IFREG)) < 0)
+    if((ndp = clicon_file_dirent(dir, &dp, "\\.yang$", S_IFREG)) < 0)
         goto done;
     if (ndp == 0)
         goto ok;

test/test_config_dir.sh

@@ -19,10 +19,13 @@ APPNAME=example
 
 cfg=$dir/conf_yang.xml
 cdir=$dir/conf.d
-cfile1=$cdir/00a.xml
+cfile1=$cdir/01a.xml
-cfile2=$cdir/01a.xml
+cfile2=$cdir/02a.xml
+cfile3=$cdir/03a.xml
+cfile4=$cdir/04a.xxxml
 
-test -d $cdir || mkdir $cdir
+rm -rf $cdir
+mkdir $cdir
 
 cat <<EOF > $cfg
 <clixon-config xmlns="http://clicon.org/config">
@@ -39,6 +42,8 @@ EOF
 # dummy
 touch  $dir/spec.cli
 
+new "test params: -f $cfg"
+
 new "Start without configdir as baseline"
 cat <<EOF > $cfile1
 <clixon-config xmlns="http://clicon.org/config">
@@ -139,6 +144,29 @@ expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "<CLICON_MODULE_SET_ID>3</CLICON
 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 -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>" 
 
+# Ensure two sub-dirs (eg <restconf>) only last is present
+cat <<EOF > $cfile3
+<clixon-config xmlns="http://clicon.org/config">
+  <restconf>
+     <server-cert-path>nisse</server-cert-path>
+  </restconf>
+</clixon-config>
+EOF
+new "Last <restconf> replaces first"
+expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "<restconf>" "<server-cert-path>nisse</server-cert-path>" --not-- "<server-cert-path>bar</server-cert-path>"
+
+# Ensure file without .xml suffix is not read
+cat <<EOF > $cfile4
+<clixon-config xmlns="http://clicon.org/config">
+  <restconf>
+     <server-cert-path>laban</server-cert-path>
+  </restconf>
+</clixon-config>
+EOF
+
+new "File without .xml is not read"
+expectpart "$($clixon_cli -1 -f $cfg -C xml)" 0 "<restconf>" "<server-cert-path>nisse</server-cert-path>" --not-- "<server-cert-path>laban</server-cert-path>"
+
 rm -rf $dir
 
 new "endtest"

test/test_config_dump.sh

@@ -35,7 +35,7 @@ cat <<EOF > $cfg
 </clixon-config>
 EOF
 
-cat <<EOF > $cfdir/extra
+cat <<EOF > $cfdir/extra.xml
 <clixon-config xmlns="http://clicon.org/config">
   <CLICON_FEATURE>extradir</CLICON_FEATURE>
 </clixon-config>
@@ -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"'
 

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 <autocli> 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 <dir> command-line option.
                 Note that due to bootstraping this value is only meaningful in the main config file";