* Top-level default leafs assigned.

* Enforcing RFC 7950 Sec 7.6.1 means unassigned top-level leafs (or leafs under non-presence containers) are assigned default values. * NACM default behaviour is read-only (empty configs are dead-lockedd) * This applies if NACM is loaded and `CLICON_NACM_MODE` is `internal` * Fixed: [default values don't show up in datastores #111](#111)
clicon · Aug 6, 2020 · 794d51a · 794d51a
1 parent 65733ff
commit 794d51a
Show file tree

Hide file tree

Showing 30 changed files with 586 additions and 160 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -42,6 +42,34 @@ Expected: July 2020
 
 ### API changes on existing protocol/config features (For users)
 
+* Top-level default leafs assigned.
+  * Enforcing RFC 7950 Sec 7.6.1 means unassigned top-level leafs (or leafs under non-presence containers) are assigned default values.
+  * In this process non-presence containers may be created.
+  * See also [default values don't show up in datastores #111](https://github.com/clicon/clixon/issues/111).
+* NACM default behaviour is read-only (empty configs are dead-lockedd)
+  * This applies if NACM is loaded and `CLICON_NACM_MODE` is `internal`
+  * Due to the previous bult (top-level default leafs)
+  * This means that empty configs or empty NACM configs are not writable (deadlocked).
+  * Workarounds:
+    1. Access the system with the recovery user, see clixon option CLICON_NACM_RECOVERY_USER
+
+  * This means that empty configs or empty NACM configs are not writable (deadlocked).
+  * Workarounds:
+    1. Access the system with the recovery user, see clixon option CLICON_NACM_RECOVERY_USER
+
+  * This means that empty configs or empty NACM configs are not writable (deadlocked).
+  * Workarounds:
+    1. Access the system with the recovery user, see clixon option CLICON_NACM_RECOVERY_USER
+
+  * This means that empty configs or empty NACM configs are not writable (deadlocked).
+  * Workarounds:
+    1. Access the system with the recovery user, see clixon option CLICON_NACM_RECOVERY_USER
+
+  * This means that empty configs or empty NACM configs are not writable (deadlocked).
+  * Workarounds:
+    1. Access the system with the recovery user, see clixon option CLICON_NACM_RECOVERY_USER
+    2. Edit the startup-db with a valid NACM config and restart the system
+    3. Set clixon option CLICON_NACM_DISABLED_ON_EMPTY to true which means that if the config is (completely) empty, you can add a NACM config as a first edit.
 * Netconf lock/unlock behaviour changed to adhere to RFC 6241
   * Changed commit lock error tag from "lock denied" to "in-use".
   * Changed unlock error message from "lock is already held" to #lock not active" or "lock held by other session".
@@ -53,6 +81,7 @@ Expected: July 2020
     * CLICON_SSL_SERVER_CERT
     * CLICON_SSL_SERVER_KEY
     * CLICON_SSL_CA_CERT
+  * Added CLICON_NACM_DISABLED_ON_EMPTY to mitigate read-only "dead-lock" of empty startup configs.
 * Restconf FCGI (eg via nginx) have changed reply message syntax slightly as follows (due to refactoring and common code with evhtp):
     * Bodies in error retuns including html code have been removed
     * Some (extra) CRLF:s have been removed
@@ -99,6 +128,8 @@ Expected: July 2020
 
 ### Corrected Bugs
 
+* Fixed: [default values don't show up in datastores #111](https://github.com/clicon/clixon/issues/111).
+  * See also API changes since this changes NACM behavior for example.
 * Fixed: Don't call upgrade callbacks if no revision defined so there's no way to determine right way 'from' and 'to'
 * Fixed: [lock candidate succeeded even though it is modified #110](https://github.com/clicon/clixon/issues/110)
 * Fixed: [Need to add the possibility to use anchors around patterns #51](https://github.com/clicon/cligen/issues/51):

diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c
@@ -226,7 +226,7 @@ startup_extraxml(clicon_handle        h,
     /* Clear tmp db */
     if (xmldb_db_reset(h, tmp_db) < 0)
 	goto done;
-    /* Application may define extra xml in its reset function*/
+    /* Application may define extra xml in its reset function */
     if (clixon_plugin_reset_all(h, tmp_db) < 0)   
 	goto done;
     /* Extra XML can also be added via file */
@@ -238,13 +238,13 @@ startup_extraxml(clicon_handle        h,
 	    goto fail;
     }
     /* 
-     * Check if tmp db is empty.
+     * Check if tmp db is empty. XXX no this is not possible.
      * It should be empty if extra-xml is null and reset plugins did nothing
      * then skip validation.
      */
     if (xmldb_get(h, tmp_db, NULL, NULL, &xt0) < 0)
 	goto done;
-    if (xt0==NULL || xml_child_nr(xt0)==0) 
+    if (xmldb_empty_get(h, tmp_db))
 	goto ok;
     xt = NULL;
     /* Validate the tmp db and return possibly upgraded xml in xt

diff --git a/lib/clixon/clixon_data.h b/lib/clixon/clixon_data.h
@@ -54,7 +54,8 @@
 typedef struct {
     uint32_t  de_id;       /* session id */
     cxobj    *de_xml;      /* cache */
-    int       de_modified; 
+    int       de_modified; /* Dirty since loaded/copied/committed/etc XXX:nocache? */
+    int       de_empty;    /* Empty on read from file, xmldb_readfile and xmldb_put sets it */
 } db_elmnt;
 
 /*

diff --git a/lib/clixon/clixon_datastore.h b/lib/clixon/clixon_datastore.h
@@ -73,7 +73,7 @@ cxobj *xmldb_cache_get(clicon_handle h, const char *db);
 
 int xmldb_modified_get(clicon_handle h, const char *db);
 int xmldb_modified_set(clicon_handle h, const char *db, int value);
-
+int xmldb_empty_get(clicon_handle h, const char *db);
 int xmldb_dump(clicon_handle h, FILE *f, cxobj *xt);
 
 #endif /* _CLIXON_DATASTORE_H */
diff --git a/lib/clixon/clixon_xml_map.h b/lib/clixon/clixon_xml_map.h
@@ -66,6 +66,9 @@ int xml_tree_prune_flagged(cxobj *xt, int flag, int test);
 int xml_namespace_change(cxobj *x, char *ns, char *prefix);
 int xml_default(cxobj *x);
 int xml_default_recurse(cxobj *xn);
+int xml_default_yspec(yang_stmt *yspec, cxobj *xn);
+int xml_nopresence_default(cxobj *xt);
+int xml_nopresence_default_mark(cxobj *x, void *arg);
 int xml_sanity(cxobj *x, void  *arg);
 int xml_non_config_data(cxobj *xt, void *arg);
 

diff --git a/lib/clixon/clixon_yang_module.h b/lib/clixon/clixon_yang_module.h
@@ -49,12 +49,13 @@
 
 /* Struct containing module state differences between two modules or two 
  * revisions of same module. 
- * This is in state of flux so it needs to be contained and easily changed.
+ * The most significant usecase is one module-state is a loaded datastore and the other
+ * is the one loaded by the server by its YANG files.
  */
 typedef struct {
     int    md_status; /* 0 if no module-state in a datastore, 1 if there is */
     char  *md_set_id; /* server-specific identifier */
-    cxobj *md_diff;  /* yang module state containing revisions and XML_FLAG_ADD|DEL|CHANGE */
+    cxobj *md_diff;   /* yang module state containing revisions and XML_FLAG_ADD|DEL|CHANGE */
 } modstate_diff_t;
 
 /*

diff --git a/lib/src/clixon_data.c b/lib/src/clixon_data.c
@@ -636,12 +636,11 @@ clicon_argv_set(clicon_handle h,
     return retval;
 }
 
-/*! Get xml database element including pid and xml cache
+/*! Get xml database element including id, xml cache, empty on startup and dirty bit
  * @param[in]  h    Clicon handle
  * @param[in]  db   Name of database
  * @retval     de   Database element
  * @retval     NULL None found
- * @note these use db_elmnt hash, not data
  */
 db_elmnt *
 clicon_db_elmnt_get(clicon_handle h,
@@ -655,14 +654,12 @@ clicon_db_elmnt_get(clicon_handle h,
     return NULL;
 }
 
-/*! Set xml database element including pid and xml cache
+/*! Set xml database element including id, xml cache, empty on startup and dirty bit
  * @param[in] h   Clicon handle
  * @param[in] db  Name of database
  * @param[in] de  Database element
  * @retval    0   OK
  * @retval   -1   Error
- * XXX add prefix to db to ensure uniqueness?
- * @note these use db_elmnt hash, not data
 */
 int
 clicon_db_elmnt_set(clicon_handle h, 

diff --git a/lib/src/clixon_datastore.c b/lib/src/clixon_datastore.c
@@ -518,6 +518,26 @@ xmldb_modified_get(clicon_handle h,
     return de->de_modified;
 }
 
+/*! Get empty flag from datastore (the datastore was empty ON LOAD)
+ * @param[in]  h     Clicon handle
+ * @param[in]  db    Database name
+ * @retval    -1     Error (datastore does not exist)
+ * @retval     0     Db was not empty on load
+ * @retval     1     Db was empty on load
+ */
+int
+xmldb_empty_get(clicon_handle h,
+		const char   *db)
+{
+    db_elmnt *de;
+
+    if ((de = clicon_db_elmnt_get(h, db)) == NULL){
+	clicon_err(OE_CFG, EFAULT, "datastore %s does not exist", db);
+	return -1;
+    }
+    return de->de_empty;
+}
+
 /*! Get modified flag from datastore
  * @param[in]  h     Clicon handle
  * @param[in]  db    Database name