diff --git a/CHANGELOG.md b/CHANGELOG.md
index 43f98a8b..e9d23ee7 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c
index 94866a9b..95cfdda3 100644
--- a/apps/backend/backend_client.c
+++ b/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 ...
- // 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:
- // If a confirmed commit procedure is
- // in progress by any NETCONF client, then any new commit will act as
- // the confirming commit.
- // 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:
- // If the NETCONF server is expecting a
- // "persist-id" parameter to complete the confirmed commit procedure,
- // then the RESTCONF edit operation MUST fail with a "409 Conflict"
- // status-line. The error-tag "in-use" is used in this case.
-
+ /* if this is from a restconf client ...
+ * 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:
+ * If a confirmed commit procedure is
+ * in progress by any NETCONF client, then any new commit will act as
+ * the confirming commit.
+ * 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:
+ * If the NETCONF server is expecting a
+ * "persist-id" parameter to complete the confirmed commit procedure,
+ * then the RESTCONF edit operation MUST fail with a "409 Conflict"
+ * 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;
diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c
index 8e689620..58af5c3c 100644
--- a/apps/backend/backend_commit.c
+++ b/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 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();
-
- 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, "", NETCONF_BASE_NAMESPACE);
-
+ cancel_confirmed_commit(h);
+ cprintf(cbret, "", NETCONF_BASE_NAMESPACE);
goto ok;
}
}
diff --git a/apps/backend/clixon_backend_commit.h b/apps/backend/clixon_backend_commit.h
index 62785eef..46a00a64 100644
--- a/apps/backend/clixon_backend_commit.h
+++ b/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);
diff --git a/test/test_confirmed_commit.sh b/test/test_confirmed_commit.sh
index 9d109328..a805ce27 100755
--- a/test/test_confirmed_commit.sh
+++ b/test/test_confirmed_commit.sh
@@ -1,10 +1,8 @@
#!/usr/bin/env bash
# Netconf confirm commit capability
-# See RFC 6241 Section 8.4
-# Test uses privileges drop
+# See RFC 6241 Section 8.4 and RFC 8040 Section 1.4
# 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 < $cfg
- $cfgietf-netconf:confirmed-commitclixon-restconf:allow-auth-none
- 42
+ $cfg${YANG_INSTALLDIR}
- $IETFRFC$fyang/usr/local/lib/$APPNAME/clispec
- /usr/local/lib/$APPNAME/backend
- example_backend.so$
- /usr/local/lib/$APPNAME/netconf
- false
- /usr/local/lib/$APPNAME/restconf/usr/local/lib/$APPNAME/cli$APPNAME$dir/$APPNAME.sock
@@ -90,6 +81,7 @@ function rpc() {
}
function commit() {
+ new "commit $1"
if [[ "$1" == "" ]]
then
rpc "" ""
@@ -101,6 +93,7 @@ function commit() {
function edit_config() {
TARGET="$1"
CONFIG="$2"
+ new "edit-config $1 $2"
rpc "<$TARGET/>$CONFIG" ""
}
@@ -111,6 +104,7 @@ function assert_config_equals() {
rpc "<$TARGET/>" "$(data "$EXPECTED")"
}
+# delete all
function reset() {
rpc "none" ""
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="
eth0
"
CONFIGC="
eth1
"
+CONFIGCONLY="eth1" # restcpnf
CONFIGBPLUSC="
eth0eth1
"
FAILSAFE_CFG="
eth99
"
@@ -144,8 +140,6 @@ fi
new "wait backend"
wait_backend
-
-
new "Hello check confirm-commit capability"
expecteof "$clixon_netconf -f $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" "urn:ietf:params:netconf:capability:confirmed-commit:1.1" '^$'
@@ -225,7 +219,6 @@ commit ""
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
$FAILSAFE_CFG
EOF
-
edit_config "candidate" "$CONFIGC"
commit "foobar"
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 "$HELLONO1160]]>]]>]]>]]>") -| $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 "a"
+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
diff --git a/yang/clixon/clixon-config@2022-03-21.yang b/yang/clixon/clixon-config@2022-03-21.yang
index 00441a8b..693b3be1 100644
--- a/yang/clixon/clixon-config@2022-03-21.yang
+++ b/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 {