Confirm-commit RESTCONF support

CHANGELOG.md

@@ -52,6 +52,9 @@ Expected: End of 2022
   * Added support for relevant arguments to CLI commit
     * See [example_cli.cli](https://github.com/clicon/clixon/blob/master/example/main/example_cli.cli)
   * See [Netconf Confirmed Commit Capability](https://github.com/clicon/clixon/issues/255)
+  * Known issues
+    * Backend privileges drop
+    * Lock check, see RFC 6241 7.5
 
 ### API changes on existing protocol/config features
 

apps/backend/backend_client.c

@@ -455,19 +455,34 @@ from_client_edit_config(clicon_handle h,
 	autocommit = 1;
     /* If autocommit option is set or requested by client */
     if (clicon_autocommit(h) || autocommit) {
-        // TODO: if this is from a restconf client ...
+        /* if this is from a restconf client ...
-        //      and, if there is an existing ephemeral commit, set is_valid_confirming_commit=1 such that
+	 *      and, if there is an existing ephemeral commit, set is_valid_confirming_commit=1 such that
-        //          candidate_commit will apply the configuration per RFC 8040 1.4:
+	 *          candidate_commit will apply the configuration per RFC 8040 1.4:
-        //              If a confirmed commit procedure is
+	 *              If a confirmed commit procedure is
-        //              in progress by any NETCONF client, then any new commit will act as
+	 *              in progress by any NETCONF client, then any new commit will act as
-        //              the confirming commit.
+	 *              the confirming commit.
-        //      and, if there is an existing persistent commit, netconf_operation_failed with "in-use", so
+	 *      and, if there is an existing persistent commit, netconf_operation_failed with "in-use", so
-        //          that the restconf server will return "409 Conflict" per RFC 8040 1.4:
+	 *          that the restconf server will return "409 Conflict" per RFC 8040 1.4:
-        //              If the NETCONF server is expecting a
+	 *              If the NETCONF server is expecting a
-        //              "persist-id" parameter to complete the confirmed commit procedure,
+	 *              "persist-id" parameter to complete the confirmed commit procedure,
-        //              then the RESTCONF edit operation MUST fail with a "409 Conflict"
+	 *              then the RESTCONF edit operation MUST fail with a "409 Conflict"
-        //              status-line.  The error-tag "in-use" is used in this case.
+	 *              status-line.  The error-tag "in-use" is used in this case.
-
+	 */
+	if (if_feature(yspec, "ietf-netconf", "confirmed-commit")) {
+	    switch (confirmed_commit.state){
+	    case INACTIVE:
+		break;
+	    case PERSISTENT:
+		if (netconf_in_use(cbret, "application", "Persistent commit is ongoing")< 0)
+		    goto done;
+		goto ok;
+		break;
+	    case EPHEMERAL:
+	    case ROLLBACK:
+		cancel_confirmed_commit(h);
+		break;
+	    }
+	}
         if ((ret = candidate_commit(h, "candidate", cbret)) < 0){ /* Assume validation fail, nofatal */
 	    if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
 		goto done;

apps/backend/backend_commit.c

@@ -648,7 +648,7 @@ candidate_validate(clicon_handle h,
  * @retval      -1      No Rollback event was found
  */
 int
-cancel_rollback_event()
+cancel_rollback_event(void)
 {
     int retval;
 
@@ -723,6 +723,28 @@ schedule_rollback_event(clicon_handle h,
     return retval;
 }
 
+/*! Cancel a confirming commit by removing rollback, and free state
+ * @param[in]  h
+ * @param[out] cbret
+ * @retval     0      OK
+ */
+int
+cancel_confirmed_commit(clicon_handle h)
+{
+    cancel_rollback_event();
+
+    if (confirmed_commit.state == PERSISTENT && confirmed_commit.persist_id != NULL) {
+	free(confirmed_commit.persist_id);
+	confirmed_commit.persist_id = NULL;
+    }
+
+    confirmed_commit.state = INACTIVE;
+
+    if (xmldb_delete(h, "rollback") < 0)
+	clicon_err(OE_DB, 0, "Error deleting the rollback configuration");
+    return 0;
+}
+
 /*! Handle the second phase of confirmed-commit processing.
  *
  * In the first phase, the proper action was taken in the case of a valid confirming-commit, but no subsequent
@@ -1211,20 +1233,8 @@ from_client_commit(clicon_handle h,
         /* If <confirmed/> is *not* present, this will conclude the confirmed-commit, so cancel the rollback. */
         if (xml_find_type(confirmed_commit.xe, NULL, "confirmed", CX_ELMNT) == NULL
                 && is_valid_confirming_commit) {
-            cancel_rollback_event();
+	    cancel_confirmed_commit(h);
-
+	    cprintf(cbret, "<rpc-reply xmlns=\"%s\"><ok/></rpc-reply>", NETCONF_BASE_NAMESPACE);
-            if (confirmed_commit.state == PERSISTENT && confirmed_commit.persist_id != NULL) {
-                free(confirmed_commit.persist_id);
-                confirmed_commit.persist_id = NULL;
-            }
-
-            confirmed_commit.state = INACTIVE;
-
-            if (xmldb_delete(h, "rollback") < 0)
-                clicon_err(OE_DB, 0, "Error deleting the rollback configuration");
-
-            cprintf(cbret, "<rpc-reply xmlns=\"%s\"><ok/></rpc-reply>", NETCONF_BASE_NAMESPACE);
-
             goto ok;
         }
     }

apps/backend/clixon_backend_commit.h

@@ -65,13 +65,14 @@ struct confirmed_commit {
     void *arg;                          // the clicon_handle that will be passed to rollback_fn()
 };
 
-extern struct confirmed_commit confirmed_commit;
+extern struct confirmed_commit confirmed_commit; // XXX global
 
 /*
  * Prototypes
  */
 int do_rollback(clicon_handle h, uint8_t *errs);
-int cancel_rollback_event();
+int cancel_rollback_event(void);
+int cancel_confirmed_commit(clicon_handle h);
 
 int startup_validate(clicon_handle h, char *db, cxobj **xtr, cbuf *cbret);
 int startup_commit(clicon_handle h, char *db, cbuf *cbret);

test/test_confirmed_commit.sh

@@ -1,10 +1,8 @@
 #!/usr/bin/env bash
 # Netconf confirm commit capability
-# See RFC 6241 Section 8.4
+# See RFC 6241 Section 8.4 and RFC 8040 Section 1.4
-# Test uses privileges drop
 # TODO:
 # - privileges drop
-# - restconf
 # - lock check
 
 # Magic line must be first in script (see README.md)
@@ -24,19 +22,12 @@ RESTCONFIG=$(restconf_config none false)
 
 cat <<EOF > $cfg
 <clixon-config xmlns="http://clicon.org/config">
-  <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
   <CLICON_FEATURE>ietf-netconf:confirmed-commit</CLICON_FEATURE>
   <CLICON_FEATURE>clixon-restconf:allow-auth-none</CLICON_FEATURE> <!-- Use auth-type=none -->
-  <CLICON_MODULE_SET_ID>42</CLICON_MODULE_SET_ID>
+  <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
   <CLICON_YANG_DIR>${YANG_INSTALLDIR}</CLICON_YANG_DIR>
-  <CLICON_YANG_DIR>$IETFRFC</CLICON_YANG_DIR>
   <CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
-  <CLICON_BACKEND_DIR>/usr/local/lib/$APPNAME/backend</CLICON_BACKEND_DIR>
-  <CLICON_BACKEND_REGEXP>example_backend.so$</CLICON_BACKEND_REGEXP>
-  <CLICON_NETCONF_DIR>/usr/local/lib/$APPNAME/netconf</CLICON_NETCONF_DIR>
-  <CLICON_NETCONF_MESSAGE_ID_OPTIONAL>false</CLICON_NETCONF_MESSAGE_ID_OPTIONAL>
-  <CLICON_RESTCONF_DIR>/usr/local/lib/$APPNAME/restconf</CLICON_RESTCONF_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
   <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
   <CLICON_SOCK>$dir/$APPNAME.sock</CLICON_SOCK>
@@ -90,6 +81,7 @@ function rpc() {
 }
 
 function commit() {
+  new "commit $1"
   if [[ "$1" == "" ]]
   then
     rpc "<commit/>" "<ok/>"
@@ -101,6 +93,7 @@ function commit() {
 function edit_config() {
   TARGET="$1"
   CONFIG="$2"
+  new "edit-config $1 $2"
   rpc "<edit-config><target><$TARGET/></target><config>$CONFIG</config></edit-config>" "<ok/>"
 }
 
@@ -111,6 +104,7 @@ function assert_config_equals() {
   rpc "<get-config><source><$TARGET/></source></get-config>" "$(data "$EXPECTED")"
 }
 
+# delete all
 function reset() {
   rpc "<edit-config><target><candidate/></target><default-operation>none</default-operation><config operation=\"delete\"/></edit-config>" "<ok/>"
   commit
@@ -123,8 +117,10 @@ RUNNING_PATH="/usr/local/var/$APPNAME/running_db"
 ROLLBACK_PATH="/usr/local/var/$APPNAME/rollback_db"
 FAILSAFE_PATH="/usr/local/var/$APPNAME/failsafe_db"
 
+
 CONFIGB="<table xmlns=\"urn:example:clixon\"><parameter><name>eth0</name></parameter></table>"
 CONFIGC="<table xmlns=\"urn:example:clixon\"><parameter><name>eth1</name></parameter></table>"
+CONFIGCONLY="<parameter xmlns=\"urn:example:clixon\"><name>eth1</name></parameter>" # restcpnf
 CONFIGBPLUSC="<table xmlns=\"urn:example:clixon\"><parameter><name>eth0</name></parameter><parameter><name>eth1</name></parameter></table>"
 FAILSAFE_CFG="<table xmlns=\"urn:example:clixon\"><parameter><name>eth99</name></parameter></table>"
 
@@ -144,8 +140,6 @@ fi
 new "wait backend"
 wait_backend
 
-
-
 new "Hello check confirm-commit capability"
 expecteof "$clixon_netconf -f $cfg" 0 "<?xml version=\"1.0\" encoding=\"UTF-8\"?><hello $DEFAULTONLY><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>" "<capability>urn:ietf:params:netconf:capability:confirmed-commit:1.1</capability>" '^$'
 
@@ -225,7 +219,6 @@ commit "<persist-id/>"
 assert_config_equals "running" "$CONFIGB"
 
 ################################################################################
-
 # TODO reconsider logic around presence/absence of rollback_db as a signal as dropping permissions may impact ability
 # to unlink and/or create that file. see clixon_datastore.c#xmldb_delete() and backend_startup.c#startup_mode_startup()
 
@@ -247,14 +240,16 @@ stop_backend -f $cfg
 start_backend -s init -f $cfg
 
 ################################################################################
-
 new "backend loads failsafe at startup if rollback present but cannot be loaded"
+
+if [ ${valgrindtest} -eq 2 ]; then # backend valgrind
+    sleep 3
+fi
 reset
 
 sudo tee "$FAILSAFE_PATH" > /dev/null << EOF                    # create a failsafe database
 <config>$FAILSAFE_CFG</config>
 EOF
-
 edit_config "candidate" "$CONFIGC"
 commit "<persist>foobar</persist><confirmed/>"
 assert_config_equals "running" "$CONFIGC"
@@ -382,7 +377,6 @@ assert_config_equals "running" "$CONFIGBPLUSC"
 sleep 5
 assert_config_equals "running" ""
 
-
 # TODO test restconf receives "409 conflict" when there is a persistent confirmed-commit active
 # TODO test restconf causes confirming-commit for ephemeral confirmed-commit
 if [ $RC -ne 0 ]; then
@@ -396,6 +390,42 @@ fi
 new "wait restconf"
 wait_restconf
 
+new "restconf as confirmed commit"
+reset
+edit_config "candidate" "$CONFIGB"
+assert_config_equals "candidate" "$CONFIGB"
+# use HELLONO11 which uses older EOM framing
+sleep 60 |  cat <(echo "$HELLONO11<rpc $DEFAULTNS><commit><confirmed/><confirm-timeout>60</confirm-timeout></commit></rpc>]]>]]><rpc $DEFAULTNS><commit></commit></rpc>]]>]]>") -| $clixon_netconf -qf $cfg  >> /dev/null &
+PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}'))
+assert_config_equals "running" "$CONFIGB"                       # assert config twice to prove it surives disconnect
+
+new "restconf POST"
+expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+xml" -d "$CONFIGCONLY" $RCPROTO://localhost/restconf/data/clixon-example:table)" 0 "HTTP/$HVER 201" "location:"
+
+assert_config_equals "running" "$CONFIGBPLUSC"
+
+new "soft kill"
+kill ${PIDS[0]}                   # kill the while loop above to close STDIN on 1st
+
+assert_config_equals "running" "$CONFIGBPLUSC"
+
+kill -9 ${PIDS[0]} 2> /dev/null   # kill the while loop above to close STDIN on 1st
+
+assert_config_equals "running" "$CONFIGBPLUSC"
+
+################################################################################
+
+new "restconf persistid expect fail"
+reset
+edit_config "candidate" "$CONFIGB"
+commit "<confirmed/><persist>a</persist>"
+assert_config_equals "running" "$CONFIGB"
+
+new "restconf POST"
+expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+xml" -d "$CONFIGCONLY" $RCPROTO://localhost/restconf/data/clixon-example:table)" 0 # "HTTP/$HVER 409"
+
+assert_config_equals "running" "$CONFIGB"
+
 if [ $RC -ne 0 ]; then
     new "Kill restconf daemon"
     stop_restconf 

yang/clixon/clixon-config@2022-03-21.yang

@@ -862,6 +862,8 @@ module clixon-config {
 	    description
 		"Set if all configuration changes are committed automatically 
                  on every edit change. Explicit commit commands unnecessary
+                 If confirm-commit, follow RESTCONF semantics: commit ephemeral but fail on
+                 persistent confirming commit.
                  (consider boolean)";
 	}
 	leaf CLICON_XMLDB_DIR {