* Fixed: [when condition error under augment in restconf #227](https://github.com/clicon/clixon/issues/227)

* As part of this fix added custom constant XML_PARENT_CANDIDATE

CHANGELOG.md

@@ -81,6 +81,7 @@ Users may have to change how they access the system
 	
 ### Corrected Bugs
 
+* Fixed: [when condition error under augment in restconf #227](https://github.com/clicon/clixon/issues/227)
 * Fixed: [Using YANG union with decimal64 and string leads to regexp match fail #226](https://github.com/clicon/clixon/issues/226)
 * Fixed: [xpath function count did not work properly #224](https://github.com/clicon/clixon/issues/224)
 * Fixed: RESTCONF Native: Failed binding of socket in network namespace caused process zombie

include/clixon_custom.h

@@ -83,13 +83,32 @@
  */
 #define STATE_ORDERED_BY_SYSTEM
 
-/* Top-symbol in clixon datastores
+/*! Top-symbol in clixon datastores
  * This is traditionally same as NETCONF_INPUT_CONFIG ("config") but can be different
  * If you change this, you need to change test shell variable in lib.sh: DATASTORE_TOP
- * Consider making this an option or configure option 
+ * Consider making this an option (but this has bootstrap problems) or configure option 
  */
 #define DATASTORE_TOP_SYMBOL "config"
 
-/* Name of default netns for clixon-restconf.yang socket/namespace field
+/*! Name of default netns for clixon-restconf.yang socket/namespace field
+ * Restconf allows opening sockets in different network namespaces. This is teh name of 
+ * "host"/"default" namespace. Unsure what to really label this but seems like there is differing
+ * consensus on how to label it.
+ * Either find that proper label, or move it to a option
  */
 #define RESTCONF_NETNS_DEFAULT "default"
+
+/*! Set a temporary parent for use in special case "when" xpath calls
+ * Problem is when changing an existing (candidate) in-memory datastore that yang "when" conditionals
+ * should be changed in clixon_datastore_write.c:text_modify().
+ * Problem is that the tree is in an intermediate state so that a when condition may not see the
+ * full context.
+ * More specifically, new nodes (x0) are created without hooking them into the existing parent (x0p)
+ * and thus an xpath on the form ".."/PARENT may not be evaluated as they should. x0 is eventually 
+ * added to its parent but then it is more difficult to check trhe when condition.
+ * This fix add the parent x0p as a "candidate" so that the xpath-eval function can use it as
+ * an alernative if it exists.
+ * Note although this solves many usecases involving parents and absolute paths, itstill does not
+ * solve all usecases, such as absolute usecases where the added node is looked for
+ */
+#define XML_PARENT_CANDIDATE

lib/clixon/clixon_xml.h

@@ -186,6 +186,10 @@ int       nscache_replace(cxobj *x, cvec *ns);
 int       xmlns_set(cxobj *x, char *prefix, char *ns);
 cxobj    *xml_parent(cxobj *xn);
 int       xml_parent_set(cxobj *xn, cxobj *parent);
+#ifdef XML_PARENT_CANDIDATE
+cxobj    *xml_parent_candidate(cxobj *xn);
+int       xml_parent_candidate_set(cxobj *xn, cxobj *parent);
+#endif /* XML_PARENT_CANDIDATE */
 
 uint16_t  xml_flag(cxobj *xn, uint16_t flag);
 int       xml_flag_set(cxobj *xn, uint16_t flag);

lib/src/clixon_datastore_write.c

@@ -205,7 +205,7 @@ check_body_namespace(cxobj     *x0,
 
 /*! Check yang when condition between a new xml x1 and old x0
  *
- * check if there is a when condition. First try it on the new request (x1), then on the
+ * Check if there is a when condition. First try it on the new request (x1), then on the
  * existing (x0). 
  * @param[in]  x0p      Parent of x0
  * @param[in]  x1       XML tree which modifies base
@@ -226,9 +226,9 @@ check_when_condition(cxobj              *x0p,
     char      *xpath = NULL;
     cvec      *nsc = NULL;
     int        nr;
-    cxobj     *x1p;
     yang_stmt *y = NULL;
     cbuf      *cberr = NULL;
+    cxobj     *x1p;
 
     if ((y = y0) != NULL ||
 	(y = (yang_stmt*)xml_spec(x1)) != NULL){
@@ -272,7 +272,7 @@ check_when_condition(cxobj              *x0p,
  * @param[in]  h        Clicon handle
  * @param[in]  x0       Base xml tree (can be NULL in add scenarios)
  * @param[in]  x0p      Parent of x0
- * @param[in]  x0t
+ * @param[in]  x0t      Top level of existing tree, eg needed for NACM rules
  * @param[in]  x1       XML tree which modifies base
  * @param[in]  x1t      Request root node (nacm needs this)
  * @param[in]  y0       Yang spec corresponding to xml-node x0. NULL if x0 is NULL
@@ -662,7 +662,9 @@ text_modify(clicon_handle       h,
 		if ((x0 = xml_new(x1name, NULL, CX_ELMNT)) == NULL)
 		    goto done;
 		xml_spec_set(x0, y0);
-
+#ifdef XML_PARENT_CANDIDATE
+		xml_parent_candidate_set(x0, x0p);
+#endif
 		changed++;
 		/* Get namespace from x1
 		 * Check if namespace exists in x0 parent
@@ -730,8 +732,8 @@ text_modify(clicon_handle       h,
 	    x1c = NULL;
 	    i = 0;
 	    while ((x1c = xml_child_each(x1, x1c, CX_ELMNT)) != NULL) {
-		x1cname = xml_name(x1c);
 		x0c = x0vec[i++];
+		x1cname = xml_name(x1c);
 		yc = yang_find_datanode(y0, x1cname);
 		if ((ret = text_modify(h, x0c, x0, x0t, x1c, x1t,
 				       yc, op,
@@ -742,6 +744,9 @@ text_modify(clicon_handle       h,
 		    goto fail;
 	    }
 	    if (changed){
+#ifdef XML_PARENT_CANDIDATE
+		xml_parent_candidate_set(x0, NULL);
+#endif
 		if (xml_insert(x0p, x0, insert, keystr, nscx1) < 0)
 		    goto done;
 	    }
@@ -786,7 +791,7 @@ text_modify(clicon_handle       h,
 /*! Modify a top-level base tree x0 with modification tree x1
  * @param[in]  h        Clicon handle
  * @param[in]  x0       Base xml tree (can be NULL in add scenarios)
- * @param[in]  x0t
+ * @param[in]  x0t      Top level of existing tree, eg needed for NACM rules
  * @param[in]  x1       XML tree which modifies base
  * @param[in]  x1t      Request root node (nacm needs this)
  * @param[in]  yspec    Top-level yang spec (if y is NULL)

lib/src/clixon_xml.c

@@ -166,6 +166,9 @@ struct xml{
     char             *x_prefix;     /* namespace localname N, called prefix */
     uint16_t          x_flags;      /* Flags according to XML_FLAG_* */
     struct xml       *x_up;         /* parent node in hierarchy if any */
+#ifdef XML_PARENT_CANDIDATE
+    struct xml       *x_up_candidate; /* Candidate parent node for special cases (when+xpath) */
+#endif
     int              _x_vector_i;   /* internal use: xml_child_each */
     int              _x_i;          /* internal use for sorting: 
 				       see xml_enumerate and xml_cmp */
@@ -196,6 +199,9 @@ struct xmlbody{
     char             *xb_prefix;     /* namespace localname N, called prefix */
     uint16_t          xb_flags;      /* Flags according to XML_FLAG_* */
     struct xml       *xb_up;         /* parent node in hierarchy if any */
+#ifdef XML_PARENT_CANDIDATE
+    struct xml       *xb_up_candidate; /* Candidate parent node for special cases (when+xpath) */
+#endif
     int              _xb_vector_i;   /* internal use: xml_child_each */
     int              _xb_i;          /* internal use for sorting: 
 				       see xml_enumerate and xml_cmp */
@@ -251,6 +257,7 @@ xml_stats_one(cxobj    *x,
 {
     size_t sz = 0;
 
+
     if (x->x_name)
 	sz += strlen(x->x_name) + 1;
     if (x->x_prefix)
@@ -587,6 +594,34 @@ xml_parent_set(cxobj *xn,
     return 0;
 }
 
+#ifdef XML_PARENT_CANDIDATE
+/*! Get candidate parent of xnode
+ * @param[in]  xn    xml node
+ * @retval     parent xml node
+ */
+cxobj*
+xml_parent_candidate(cxobj *xn)
+{
+    if (xn == NULL) {
+	return NULL;
+    }
+    return xn->x_up_candidate;
+}
+
+/*! Set candidate parent of xml node
+ * @param[in]  xn      xml node
+ * @param[in]  parent  pointer to candidate parent xml node
+ * @retval     0       OK
+ */
+int
+xml_parent_candidate_set(cxobj *xn, 
+			 cxobj *parent)
+{
+    xn->x_up_candidate = parent;
+    return 0;
+}
+#endif /* XML_PARENT_CANDIDATE */
+
 /*! Get xml node flags, used for internal algorithms
  * @param[in]  xn    xml node
  * @retval     flag  Flag value(s), see XML_FLAG_MARK et al
@@ -1420,8 +1455,7 @@ xml_child_rm(cxobj *xp,
  * @param[in]   xc     xml child node to be removed
  * @retval      0      OK
  * @retval      -1     Error
- * @note you should not remove xchild in loop (unless yoy keep track of xprev)
+ * @note you should not remove xchild in loop (unless you keep track of xprev)
- *
  * @see xml_child_rm  Remove a child of a node
  */
 int

lib/src/clixon_xpath_eval.c

@@ -392,7 +392,12 @@ xp_eval_step(xp_ctx     *xc0,
 	xc->xc_nodeset = NULL;
 	for (i=0; i<veclen; i++){
 	    x = vec[i];
-	    if ((xp = xml_parent(x)) != NULL)
+	    if ((xp = xml_parent(x)) != NULL
+#ifdef XML_PARENT_CANDIDATE
+		/* Also check "candidate" parent for special when use-case */
+		|| (xp = xml_parent_candidate(x)) != NULL
+#endif /* XML_PARENT_CANDIDATE */
+		)
 		if (cxvec_append(xp, &xc->xc_nodeset, &xc->xc_size) < 0)
 		    goto done;
 	}
@@ -979,8 +984,13 @@ xp_eval(xp_ctx     *xc,
     case XP_ABSPATH:
 	/* Set context node to top node, and nodeset to that node only */
 	x = xc->xc_node;
+#ifdef XML_PARENT_CANDIDATE
+	while (xml_parent(x) != NULL || xml_parent_candidate(x) != NULL)
+	    x = xml_parent(x)?xml_parent(x):xml_parent_candidate(x);
+#else
 	while (xml_parent(x) != NULL)
 	    x = xml_parent(x);
+#endif
 	xc->xc_node = x;
 	xc->xc_nodeset[0] = x;
 	xc->xc_size=1;

test/test_yang_when.sh

@@ -4,7 +4,6 @@
 # 1. set/get/validate matching and non-matching when statement
 # 2 RFC 7950 7.21.5: If a key leaf is defined in a grouping that is used in a list, the
 #  "uses" statement MUST NOT have a "when" statement.
-# XXX:Originally for cli tests but not specific to cli (clean when done?)
 
 # Magic line must be first in script (see README.md)
 s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
@@ -69,7 +68,7 @@ commit("Commit the changes"), cli_commit();
 quit("Quit"), cli_quit();
 EOF
 
-
+# Start backend, add entry with matching name / non-matrching and check get/validation
 function testrun()
 {
     new "test params: -f $cfg"
@@ -81,8 +80,6 @@ function testrun()
 	fi
 	new "start backend -s init -f $cfg"
 	start_backend -s init -f $cfg
-
-
     fi
     new "wait backend"
     wait_backend
@@ -129,6 +126,7 @@ function testrun()
     new "netconf validate default set"
     expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"
 
+    if [ $BE -ne 0 ]; then
 	new "Kill backend"
 	# Check if premature kill
 	pid=$(pgrep -u root -f clixon_backend)
@@ -137,6 +135,7 @@ function testrun()
 	fi
 	# kill backend
 	stop_backend -f $cfg
+    fi
 }
 
 cat <<EOF > $fyang
@@ -198,9 +197,6 @@ EOF
 new "2. augment/uses/when test edit,default (same but with augment)"
 testrun
 
-new "3. RFC 7950 7.21.5: If a key leaf is defined in a grouping that is used in a list"
-# the "uses" statement MUST NOT have a "when" statement.
-
 cat <<EOF > $fyang
 module example {
   namespace "urn:example:clixon";
@@ -221,14 +217,91 @@ module example {
   }
 }
 EOF
+new "3. RFC 7950 7.21.5: If a key leaf is defined in a grouping that is used in a list"
+# the "uses" statement MUST NOT have a "when" statement.
 
 if [ ${valgrindtest} -eq 0 ]; then # Error dont cleanup mem OK
     new "start backend -s init -f $cfg, expect yang when fail"
     expectpart "$(sudo $clixon_backend -F -D $DBG -s init -f $cfg 2>&1)" 255 "Yang error: Key leaf 'name' defined in grouping 'value-top' is used in a 'uses' statement, This is not allowed according to RFC 7950 Sec 7.21.5"
 fi
 
+cat <<EOF > $fyang
+module example {
+  namespace "urn:example:clixon";
+  prefix ex;
+
+  leaf allow {
+     type boolean;
+     default false;
+  }
+  grouping value-top {
+    leaf value{
+       type string;
+       default foo;
+    }
+  }
+  container table{
+    list parameter{
+      key name;
+      leaf name{
+        type string;
+      }
+      uses value-top {
+        when "../../ex:allow = 'true'";
+      }
+    }
+  }
+}
+EOF
+
+
+new "3. Check a top-level value as allowed value"
+
+new "test params: -f $cfg"
+if [ $BE -ne 0 ]; then
+    new "kill old backend"
+    sudo clixon_backend -z -f $cfg
+    if [ $? -ne 0 ]; then
+	err
+    fi
+    new "start backend -s init -f $cfg"
+    start_backend -s init -f $cfg
+fi
+
+new "wait backend"
+wait_backend
+
+new "netconf set allow true"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><allow xmlns=\"urn:example:clixon\">true</allow></config><default-operation>merge</default-operation></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"
+
+new "netconf commit"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><commit/></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"
+
+new "netconf set value expect OK"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><table xmlns=\"urn:example:clixon\"><parameter><name>kalle</name><value>42</value></parameter></table></config><default-operation>merge</default-operation></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"
+
+new "netconf discard-changes"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><discard-changes/></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"
+
+new "netconf set allow false"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><allow xmlns=\"urn:example:clixon\">false</allow></config><default-operation>merge</default-operation></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"
+
+new "netconf commit"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><commit/></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"
+
+new "netconf set value expect fail"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><table xmlns=\"urn:example:clixon\"><parameter><name>kalle</name><value>42</value></parameter></table></config><default-operation>merge</default-operation></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><rpc-error><error-type>application</error-type><error-tag>unknown-element</error-tag><error-info><bad-element>value</bad-element></error-info><error-severity>error</error-severity><error-message>Node 'value' tagged with 'when' condition '../../ex:allow = 'true'' in module 'example' evaluates to false in edit-config operation (see RFC 7950 Sec 8.3.2)</error-message></rpc-error></rpc-reply>]]>]]>$"
+
+if [ $BE -ne 0 ]; then
+    new "Kill backend"
+    # Check if premature kill
+    pid=$(pgrep -u root -f clixon_backend)
+    if [ -z "$pid" ]; then
+	err "backend already dead"
+    fi
     # kill backend
     stop_backend -f $cfg
+fi
 
 rm -rf $dir
 

yang/clixon/clixon-config@2021-05-20.yang

@@ -484,7 +484,7 @@ module clixon-config {
 	    type string;
 	    default "/usr/local/sbin";
 	    description
-		"Path to clixon-restconf daemon binary as used by backend if started internally
+		"Path to dir of clixon-restconf daemon binary as used by backend if started internally
                  Discussion: Somewhat problematic to have it as run time option. It may think it
                  should be known at configure or install time, but for example the main docker
                  installation moves the binaries, and this may be true elsewehere too.
@@ -520,7 +520,8 @@ module clixon-config {
 	leaf CLICON_RESTCONF_USER {
 	    type string;
 	    description 
-		"User name for restconf. 
+		"Run clixon_daemon as this user
+                 When drop privileges is used, the daemon will drop privileges to this user.
                  In pre-5.2 code this was configured as compile-time constant WWWUSER with
                  default value www-data
                  See also CLICON_PRIVILEGES setting";