System-only config: Source of truth

Candidate, fix system-only in cache when locked or modified, re-read otherwise
Remove system-only from cache after commit
This commit is contained in:
Olof hagsand 2024-10-31 09:22:27 +01:00
parent cfa4803e0f
commit 313a2caadd
16 changed files with 189 additions and 78 deletions

View file

@ -20,11 +20,11 @@ Expected: January 2025
* New: [feature request: support xpath functions for strings](https://github.com/clicon/clixon/issues/556)
* Added: re-match, substring, string, string-length, translate, substring-before, substring-after, starts-with
* Added support for system-only-config data
* A mechanism to not store sensitive data in the datastore, instead use application callbacks to store the data in system state.
* Store sensitive data in the "system" instead of in datastores
* New `CLICON_XMLDB_SYSTEM_ONLY_CONFIG` configuration option
* New `system-only-config` extension
* New `ca_system_only` backend callback for reading system-only data
* New `clixon-config@2024-08-01.yang` revision
* New `clixon-config@2024-11-01.yang` revision
* Added: `CLICON_XMLDB_SYSTEM_ONLY_CONFIG`
### C/CLI-API changes on existing features

View file

@ -187,15 +187,15 @@ release_all_dbs(clixon_handle h,
char **keys = NULL;
size_t klen;
int i;
uint32_t iddb;
db_elmnt *de;
if (clicon_option_bool(h, "CLICON_AUTOLOCK") &&
(iddb = xmldb_islocked(h, "candidate")) == id){
if (xmldb_islocked(h, "candidate") == id){
if (clicon_option_bool(h, "CLICON_AUTOLOCK")){
if (xmldb_copy(h, "running", "candidate") < 0)
goto done;
xmldb_modified_set(h, "candidate", 0); /* reset dirty bit */
}
}
/* get all db:s */
if (clicon_hash_keys(clicon_db_elmnt(h), &keys, &klen) < 0)
goto done;
@ -204,8 +204,6 @@ release_all_dbs(clixon_handle h,
if ((de = clicon_db_elmnt_get(h, keys[i])) != NULL &&
de->de_id == id){
de->de_id = 0; /* unlock */
clicon_db_elmnt_set(h, keys[i], de);
if (clixon_plugin_lockdb_all(h, keys[i], 0, id) < 0)
goto done;
@ -477,13 +475,14 @@ do_lock(clixon_handle h,
/* 2) The target configuration is <candidate>, it has already been modified, and
* these changes have not been committed or rolled back.
*/
if (strcmp(db, "candidate") == 0 &&
xmldb_modified_get(h, db)){
if (strcmp(db, "candidate") == 0) {
if (xmldb_exists(h, db) && xmldb_modified_get(h, db)){
if (netconf_lock_denied(cbret, "<session-id>0</session-id>",
"Operation failed, candidate has already been modified and the changes have not been committed or rolled back (RFC 6241 7.5)") < 0)
goto done;
goto failed;
}
}
/* 3) The target configuration is <running>, and another NETCONF
* session has an ongoing confirmed commit
*/
@ -500,6 +499,13 @@ do_lock(clixon_handle h,
}
if (xmldb_lock(h, db, id) < 0)
goto done;
if (strcmp(db, "candidate") == 0) {
/* Add system-only config to candidate cache */
if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
if (system_only_data_add(h, "candidate") < 0)
goto done;
}
}
/* user callback */
if (clixon_plugin_lockdb_all(h, db, 1, id) < 0)
goto done;
@ -586,6 +592,13 @@ from_client_edit_config(clixon_handle h,
if (ret == 0)
goto ok;
}
if (strcmp(target, "candidate") == 0) {
/* Add system-only config to candidate cache */
if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
if (system_only_data_add(h, "candidate") < 0)
goto done;
}
}
if (xml_nsctx_node(xn, &nsc) < 0)
goto done;
/* Get prefix of netconf base namespace in the incoming message */
@ -733,6 +746,7 @@ from_client_edit_config(clixon_handle h,
goto done;
}
cprintf(cbret, "<rpc-reply xmlns=\"%s\"><ok", NETCONF_BASE_NAMESPACE);
// Set in text_modify
if (clicon_data_get(h, "objectexisted", &val) == 0)
cprintf(cbret, " %s:objectexisted=\"%s\" xmlns:%s=\"%s\"",
CLIXON_LIB_PREFIX, val,
@ -820,7 +834,21 @@ from_client_copy_config(clixon_handle h,
goto done;
goto ok;
}
if (strcmp(target, "candidate") == 0){
xmldb_modified_set(h, target, 1); /* mark as dirty */
/* Add system-only config to candidate */
if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
if (system_only_data_add(h, "candidate") < 0)
goto done;
}
}
/* Remove system-only-config data from destination cache */
if (strcmp(source, "candidate") == 0){
if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
xmldb_clear(h, target);
}
}
cprintf(cbret, "<rpc-reply xmlns=\"%s\"><ok/></rpc-reply>", NETCONF_BASE_NAMESPACE);
ok:
retval = 0;
@ -974,6 +1002,10 @@ from_client_lock(clixon_handle h,
* @param[in] regarg User argument given at rpc_callback_register()
* @retval 0 OK
* @retval -1 Error
*
* RFC 6241, 8.3.5.2: To facilitate easy recovery, any outstanding changes are discarded when the
* lock is released, whether explicitly with the <unlock> operation or implicitly from session
* failure. (XXX This is not implemented)
*/
static int
from_client_unlock(clixon_handle h,

View file

@ -730,9 +730,10 @@ candidate_commit(clixon_handle h,
*/
if (xmldb_copy(h, db, "running") < 0)
goto done;
/* Remove system-only-config data from destination */
/* Remove system-only-config data from destination cache */
if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
xmldb_clear(h, "running");
xmldb_clear(h, db);
}
xmldb_modified_set(h, db, 0); /* reset dirty bit */
/* Here pointers to old (source) tree are obsolete */
@ -1126,3 +1127,40 @@ load_failsafe(clixon_handle h,
cbuf_free(cbret);
return retval;
}
/*! Add system-only data to db
*
* @param[in] h Clixon handle
* @param[in] db Datastore
* @retval 0 OK
* @retval -1 Error
*/
int
system_only_data_add(clixon_handle h,
char *db)
{
int retval = -1;
cxobj *x;
if ((x = xmldb_cache_get(h, db)) != NULL){
if (xmldb_system_only_config(h, "/", NULL, &x) < 0)
goto done;
}
else{ // XXX extra complexity
if ((x = xml_new(DATASTORE_TOP_SYMBOL, NULL, CX_ELMNT)) == NULL)
goto done;
xml_flag_set(x, XML_FLAG_TOP);
if (xmldb_system_only_config(h, "/", NULL, &x) < 0)
goto done;
if (xml_child_nr(x)){
db_elmnt *de;
if ((de = clicon_db_elmnt_get(h, db)) != NULL)
de->de_xml = x;
}
else
xml_free(x);
}
retval = 0;
done:
return retval;
}

View file

@ -1030,16 +1030,9 @@ main(int argc,
/* Initiate the shared candidate. */
if (xmldb_copy(h, "running", "candidate") < 0)
goto done;
/* Add system-only config to candidate */
if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
cxobj *x;
if ((x = xmldb_cache_get(h, "candidate")) != NULL)
if (xmldb_system_only_config(h, "/", NULL, &x) < 0)
goto done;
}
if (xmldb_modified_set(h, "candidate", 0) <0)
goto done;
/* Set startup status */
if (clicon_startup_status_set(h, status) < 0)
goto done;

View file

@ -73,11 +73,11 @@ int startup_commit(clixon_handle h, char *db, cbuf *cbret);
int candidate_validate(clixon_handle h, char *db, cbuf *cbret);
int candidate_commit(clixon_handle h, cxobj *xe, char *db, uint32_t myid,
validate_level vlev, cbuf *cbret);
int from_client_commit(clixon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
int from_client_discard_changes(clixon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
int from_client_validate(clixon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
int from_client_restart_one(clixon_handle h, clixon_plugin_t *cp, cbuf *cbret);
int load_failsafe(clixon_handle h, char *phase);
int system_only_data_add(clixon_handle h, char *db);
#endif /* _CLIXON_BACKEND_COMMIT_H_ */

View file

@ -676,7 +676,7 @@ example_statefile(clixon_handle h,
* @retval 0 OK
* @retval -1 Error
*
* System-only config data as defined by _ is not written to datastore.
* System-only config data is not written to datastore.
* Instead, in this ocmmit action, it is written to file _state_file
* @see main_system_only_commit callback for reading data
* @note Only single system-only config data supported

View file

@ -428,6 +428,7 @@ xmldb_unlock(clixon_handle h,
de->de_id = 0;
memset(&de->de_tv, 0, sizeof(struct timeval));
clicon_db_elmnt_set(h, db, de);
}
return 0;
}
@ -562,6 +563,9 @@ xmldb_clear(clixon_handle h,
xml_free(xt);
de->de_xml = NULL;
}
de->de_modified = 0;
de->de_id = 0;
memset(&de->de_tv, 0, sizeof(struct timeval));
}
return 0;
}
@ -572,8 +576,8 @@ xmldb_clear(clixon_handle h,
* @param[in] db Database
* @retval 0 OK
* @retval -1 Error
* @note Datastore is not actually deleted so that a backend after dropping priviliges can re-create it
* @note Datastores / dirs are not actually deleted so that a backend after dropping priviliges
* can re-create them
*/
int
xmldb_delete(clixon_handle h,
@ -612,17 +616,11 @@ xmldb_delete(clixon_handle h,
for (i = 0; i < ndp; i++){
cbuf_reset(cb);
cprintf(cb, "%s/%s", subdir, dp[i].d_name);
if (unlink(cbuf_get(cb)) < 0){
clixon_err(OE_DB, errno, "unlink(%s)", cbuf_get(cb));
if (truncate(cbuf_get(cb), 0) < 0){
clixon_err(OE_DB, errno, "truncate %s", filename);
goto done;
}
}
if (rmdir(subdir) < 0){
#if 0 /* Ignore this for now, there are some cornercases where this is problamatic, see confirmed-commit */
clixon_err(OE_DB, errno, "rmdir(%s)", subdir);
goto done;
#endif
}
}
}
retval = 0;

View file

@ -886,8 +886,11 @@ xmldb_get_cache(clixon_handle h,
if (xml_apply(x1t, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)(XML_FLAG_MARK|XML_FLAG_CHANGE)) < 0)
goto done;
}
if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG") &&
(strcmp(db, "candidate") != 0)) {
/* Unless a modified/locked candidate, add system-only-config data */
if (strcmp(db, "candidate") != 0 ||
(xmldb_modified_get(h, db) == 0 &&
xmldb_islocked(h, db) == 0)){
if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG"))
if (xmldb_system_only_config(h, xpath?xpath:"/", nsc, &x1t) < 0)
goto done;
}

View file

@ -1239,7 +1239,15 @@ text_modify_top(clixon_handle h,
/* Special case top-level replace */
else if (op == OP_REPLACE || op == OP_DELETE){
if (createstr != NULL){
if (xml_child_nr_type(x0t, CX_ELMNT)) /* base tree not empty */
x0c = NULL;
/* Specialization of xml_default_nopresence for skiptop and mode=0 */
while ((x0c = xml_child_each(x0t, x0c, CX_ELMNT)) != NULL) {
if ((ret = xml_default_nopresence(x0c, 0, 0)) < 0)
goto done;
if (ret == 0)
break;
}
if (x0c != NULL)
clicon_data_set(h, "objectexisted", "true");
else
clicon_data_set(h, "objectexisted", "false");
@ -1421,6 +1429,12 @@ xmldb_put(clixon_handle h,
goto done;
if (ret == 0)
goto fail;
/* Add default global values (see also xmldb_populate) */
if (xml_global_defaults(h, x0, nsc, "/", yspec, 0) < 0)
goto done;
/* Add default recursive values */
if (xml_default_recurse(x0, 0, 0) < 0)
goto done;
}
if (strcmp(xml_name(x0), DATASTORE_TOP_SYMBOL) !=0 ||
xml_flag(x0, XML_FLAG_TOP) == 0){
@ -1461,7 +1475,7 @@ xmldb_put(clixon_handle h,
*/
if (xml_default_nopresence(x0, 3, XML_FLAG_ADD|XML_FLAG_DEL) < 0)
goto done;
/* Complete defaults in incoming x1
/* Complete defaults
*/
if (xml_global_defaults(h, x0, nsc, "/", yspec, 0) < 0)
goto done;

View file

@ -133,6 +133,7 @@ EOF
new "check datastore using netconf"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><candidate/></source><filter type=\"xpath\" select=\"/ex:table/ex:parameter[ex:name='x']\" xmlns:ex=\"urn:example:clixon\" /></get-config></rpc>" "" "<rpc-reply $DEFAULTNS><data>$XML</data></rpc-reply>"
sudo chmod a+r $dir/candidate_db
new "check datastore direct access"
expectpart "$($clixon_util_datastore -d candidate -b $dir -y $fyang -Y ${YANG_INSTALLDIR} -Y $dir get /)" 0 "$XML"

View file

@ -276,6 +276,56 @@ check_db running true xml
new "Get mydata from running"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><running/></source></get-config></rpc>" "<rpc-reply $DEFAULTNS><data><store xmlns=\"urn:example:std\"><keys><key><name>a</name><system-only-data>mydata</system-only-data><normal-data>otherdata</normal-data></key></keys></store></data></rpc-reply>"
new "Get mydata from candidate"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><running/></source></get-config></rpc>" "<rpc-reply $DEFAULTNS><data><store xmlns=\"urn:example:std\"><keys><key><name>a</name><system-only-data>mydata</system-only-data><normal-data>otherdata</normal-data></key></keys></store></data></rpc-reply>"
new "Source-of-truth: modify system-only"
sudo chmod 666 $dir/system-only.xml
cat <<EOF > $dir/system-only.xml
<store xmlns="urn:example:std">
<keys>
<key>
<name>a</name>
<system-only-data>CHANGED</system-only-data>
</key>
</keys>
</store>
EOF
new "Get mydata from candidate again"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>" "<rpc-reply $DEFAULTNS><data><store xmlns=\"urn:example:std\"><keys><key><name>a</name><system-only-data>CHANGED</system-only-data><normal-data>otherdata</normal-data></key></keys></store></data></rpc-reply>"
new "Restore original"
cp $dir/y_db $dir/system-only.xml
new "Get mydata from candidate again"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><running/></source></get-config></rpc>" "<rpc-reply $DEFAULTNS><data><store xmlns=\"urn:example:std\"><keys><key><name>a</name><system-only-data>mydata</system-only-data><normal-data>otherdata</normal-data></key></keys></store></data></rpc-reply>"
new "Source-of-truth: modify system-only, then edit"
cat <<EOF > $dir/system-only.xml
<store xmlns="urn:example:std">
<keys>
<key>
<name>a</name>
<system-only-data>CHANGED</system-only-data>
</key>
</keys>
</store>
EOF
new "Add normal data"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><store xmlns=\"urn:example:std\"><keys><key><name>a</name><normal-data>otherdata2</normal-data></key></keys></store></config></edit-config></rpc>" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
new "Get mydata from candidate expect CHANGED"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>" "<rpc-reply $DEFAULTNS><data><store xmlns=\"urn:example:std\"><keys><key><name>a</name><system-only-data>CHANGED</system-only-data><normal-data>otherdata2</normal-data></key></keys></store></data></rpc-reply>"
new "Restore original"
cp $dir/y_db $dir/system-only.xml
new "Discard"
new "netconf discard-changes"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><discard-changes/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
new "Remove mydata"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><store xmlns=\"urn:example:std\"><keys><key><name>a</name><system-only-data nc:operation=\"delete\" xmlns:nc=\"${BASENS}\">mydata</system-only-data></key></keys></store></config><default-operation>none</default-operation></edit-config></rpc>" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
@ -419,7 +469,7 @@ fi
new "wait restconf"
wait_restconf
new "Add mydata"
new "Add system-only data"
expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"clixon-standard:store":{"keys":{"key":[{"name":"a","system-only-data":"mydata"}]}}}' $RCPROTO://localhost/restconf/data)" 0 "HTTP/$HVER 201"
new "Add normal data"

View file

@ -97,30 +97,9 @@ wait_backend
# clixon_cli < $dir/cli1 &
# echo "show devices" > $dir/cli1
if false; then
mkfifo $dir/cli1
# cat > $dir/cli1 &
$clixon_cli -f $cfg < $dir/cli1 &
new "cli 1st edit async"
echo "set table parameter x value a" > $dir/cli1
jobs -l %
PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}'))
# echo "PIDS:$PIDS"
new "cli 2nd edit expect fail"
expectpart "$($clixon_cli -1f $cfg set table parameter y value b 2>&1)" 255 "lock-denied" "lock is already held"
kill ${PIDS[0]} # kill the while loop above to close STDIN on 1st
wait
new "cli 3rd edit expect ok"
expectpart "$($clixon_cli -1f $cfg set table parameter z value c)" 0 "^$"
else
new "cli 1st edit async"
sleep 60 | expectpart "$($clixon_cli -f $cfg set table parameter x value a)" 0 "" &
if [ $valgrindtest -eq 1 ]; then
sleep 1
fi
sleep 1
PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}'))
new "cli 2nd edit expect fail"
@ -152,7 +131,7 @@ PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}'))
new "cli edit 2nd expected ok"
expectpart "$($clixon_cli -1f $cfg set table parameter x value a)" 0 "^$"
fi
if [ $BE -ne 0 ]; then
new "Kill backend"
# Check if premature kill

View file

@ -377,9 +377,7 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<xx:rpc xmlns:xx
new "asynchronous lock running"
sleep 60 | cat <(echo "$HELLONO11<rpc $DEFAULTNS><lock><target><running/></target></lock></rpc>]]>]]>") -| $clixon_netconf -qf $cfg >> /dev/null &
if [ $valgrindtest -eq 1 ]; then
sleep 1
fi
sleep 1
PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}'))
new "try commit should fail"
@ -415,7 +413,7 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS>
new "netconf lock"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><lock><target><candidate/></target></lock></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
new "copy startup to candidate"
new "copy candidate to startup"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><copy-config><target><startup/></target><source><candidate/></source></copy-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
new "copy startup to candidate using prefix xx"

View file

@ -110,7 +110,8 @@ function testrun(){
echo -n " /proc/$pid/statm: "
cat /proc/$pid/statm|awk '{print $1*4/1000 "M"}'
fi
for db in running candidate startup; do
dbs="running candidate startup";
for db in $dbs; do
echo "$db"
resdb0=$(echo "$res" | $clixon_util_xpath -p "/rpc-reply/datastores/datastore[name=\"$db\"]")
resdb=${resdb0#"nodeset:0:"}

View file

@ -173,7 +173,7 @@ new "restconf GET null datastore"
expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example:cont1)" 0 "HTTP/$HVER 404" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"invalid-value","error-severity":"error","error-message":"Instance does not exist"}}}'
new "restconf PUT initial datastore"
expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" -d '{"ietf-restconf:data":{"example:cont1":{"interface":{"name":"local0","type":"regular"}}}}' $RCPROTO://localhost/restconf/data)" 0 "HTTP/$HVER 204" # 201 Created (new resource) -> 204 No Content (existing modified)
expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" -d '{"ietf-restconf:data":{"example:cont1":{"interface":{"name":"local0","type":"regular"}}}}' $RCPROTO://localhost/restconf/data)" 0 "HTTP/$HVER 201" # 201 Created (new resource) -> 204 No Content (existing modified)
new "restconf GET datastore"
expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example:cont1)" 0 "HTTP/$HVER 200" '{"example:cont1":{"interface":\[{"name":"local0","type":"regular"}\]}}'

View file

@ -60,7 +60,6 @@ module clixon-config {
"Added options:
CLICON_YANG_DOMAIN_DIR
CLICON_YANG_USE_ORIGINAL
CLICON_XMLDB_SYSTEM_ONLY_CONFIG (tentative)
Released in Clixon 7.2";
}
revision 2024-04-01 {
@ -1206,8 +1205,13 @@ module clixon-config {
default true;
description
"If set, some fields in the configuration tree are not stored to datastore.
Instead, the application must provide a mechanism to save the system-only-config
Instead, the application provides a mechanism to save the system-only-config
in the system via commit/system-only-config callbacks.
Specifically, system-only data is read from the system except in the following case:
datastore is candidate, and either locked or modified
In that case, the system-only config is stored in the cache (not in file) and
not read from the system.
The system-only data is still not stored in the datastore however.
See also extension system-only-config in clixon-lib.yang";
}
leaf CLICON_XML_CHANGELOG {