* [Adding feature to top level container doesn't work](https://github.com/clicon/clixon/issues/322)

* Instead of removing YANG which is disabled by `if-feature`, replace it with an yang `anydata` node.
  * This means XML specified by such YANG is ignored, and it is not an error to access it
  * Note the similarity with `CLICON_YANG_UNKNOWN_ANYDATA`

CHANGELOG.md

@@ -64,6 +64,10 @@ Users may have to change how they access the system
 
 ### Minor features
 
+* [Adding feature to top level container doesn't work](https://github.com/clicon/clixon/issues/322)
+  * Instead of removing YANG which is disabled by `if-feature`, replace it with an yang `anydata` node.
+  * This means XML specified by such YANG is ignored, and it is not an error to access it
+  * Note the similarity with `CLICON_YANG_UNKNOWN_ANYDATA`
 * [provide support for load config of cli format along with json and xml format as save config is supported for all 3 formats](https://github.com/clicon/clixon/issues/320)
 * [prevent clixon-restconf@2021-05-20.yang module from loading](https://github.com/clicon/clixon/issues/318)
   * Instead of always loading it, load it to datastore YANGs only if `CLICON_BACKEND_RESTCONF_PROCESS` is `true`

lib/src/clixon_yang.c

@@ -93,7 +93,7 @@ static int yang_search_index_extension(clicon_handle h, yang_stmt *yext, yang_st
 /*
  * Local variables
  */
-/* Mapping between yang keyword string <--> clicon constants 
+/* Mapping between yang keyword string <--> clixon constants 
  * Here is also the place where doc on some types store variables (cv)
  */
 static const map_str2int ykmap[] = {
@@ -653,7 +653,7 @@ ys_free1(yang_stmt *ys,
 	free(ys->ys_filename);
     if (self){
 	free(ys);
-	_stats_yang_nr++;
+	_stats_yang_nr--;
     }
     return 0;
 }
@@ -723,6 +723,27 @@ ys_prune_self(yang_stmt *ys)
     return retval;
 }
 
+/*! Free all yang children tree recursively 
+ * @param[in]  ys   Yang node to its children recursively
+ */
+static int
+ys_freechildren(yang_stmt *ys)
+{
+    int i;
+    yang_stmt *yc;
+    
+    for (i=0; i<ys->ys_len; i++){
+	if ((yc = ys->ys_stmt[i]) != NULL)
+	    ys_free(yc);
+    }
+    ys->ys_len = 0;
+    if (ys->ys_stmt){
+	free(ys->ys_stmt);
+	ys->ys_stmt = NULL;
+    }
+    return 0;
+}
+
 /*! Free a yang statement tree recursively 
  * @param[in]  ys   Yang node to remove and all its children recursively
  * @note does not remove yang node from tree
@@ -731,13 +752,7 @@ ys_prune_self(yang_stmt *ys)
 int 
 ys_free(yang_stmt *ys)
 {
-    int i;
+    ys_freechildren(ys);
-    yang_stmt *yc;
-
-    for (i=0; i<ys->ys_len; i++){
-	if ((yc = ys->ys_stmt[i]) != NULL)
-	    ys_free(yc);
-    }
     ys_free1(ys, 1);
     return 0;
 }
@@ -2925,7 +2940,7 @@ yang_features(clicon_handle h,
     int        ret;
 
     i = 0;
-    while (i<yt->ys_len){ /* Note, children may be removed */
+    while (i<yt->ys_len){
 	ys = yt->ys_stmt[i];
 	if (ys->ys_keyword == Y_IF_FEATURE){
 	    if ((ret = yang_if_feature(h, ys)) < 0)
@@ -2933,25 +2948,34 @@ yang_features(clicon_handle h,
 	    if (ret == 0)
 		goto disabled;
 	}
-	else
+	else if (ys->ys_keyword == Y_FEATURE){
-	    if (ys->ys_keyword == Y_FEATURE){
 		if (ys_populate_feature(h, ys) < 0)
 		    goto done;
-	    } else switch (yang_features(h, ys)){
+	}
-	    case -1: /* error */
+	else
-		goto done;
+	    switch (yang_features(h, ys)){
-		break;
+		case -1: /* error */
-	    case 0: /* disabled: remove ys */
+		    goto done;
-		for (j=i+1; j<yt->ys_len; j++)
+		    break;
-		    yt->ys_stmt[j-1] = yt->ys_stmt[j];
+		case 0: /* disabled: remove ys */
-		yt->ys_len--;
+		    /* Change datanodes YANG to ANYDATA, other nodes are removed
-		yt->ys_stmt[yt->ys_len] = NULL;
+		     */
-		ys_free(ys);
+		    if (yang_datanode(ys) && yang_config_ancestor(ys)){
-		continue; /* Don't increment i */
+			ys->ys_keyword = Y_ANYDATA;
-		break;
+			ys_freechildren(ys);
-	    default: /* ok */
+			ys->ys_len = 0;
-		break;
+			break;
-	    }
+		    }
+		    for (j=i+1; j<yt->ys_len; j++)
+			yt->ys_stmt[j-1] = yt->ys_stmt[j];
+		    yt->ys_len--;
+		    yt->ys_stmt[yt->ys_len] = NULL;
+		    ys_free(ys);
+		    continue; /* Don't increment i */
+		    break;
+		default: /* ok */
+		    break;
+		}
 	i++;
     }
     retval = 1;
@@ -3723,7 +3747,7 @@ yang_anydata_add(yang_stmt *yp,
 	goto done;
     }
     yang_argument_set(ys, name);
-    if (yn_insert(yp, ys) < 0){ /* Insert into hierarchy */
+    if (yp && yn_insert(yp, ys) < 0){ /* Insert into hierarchy */
 	ys = NULL;
 	goto done;
     }

lib/src/clixon_yang_parse_lib.c

@@ -264,6 +264,8 @@ yang_augment_node(clicon_handle h,
     /* The target node MUST be either a container, list, choice, case, input, output, or notification node.
      * which means it is slightly different than a schema-nodeid ? */
     targetkey = yang_keyword_get(ytarget);
+    if (targetkey == Y_ANYDATA)
+	goto ok;
 
     /* Find when statement, if present */
     if ((ywhen = yang_find(ys, Y_WHEN, NULL)) != NULL){
@@ -615,6 +617,9 @@ yang_expand_uses_node(yang_stmt *yn,
 	/* Not found, try next */
 	if (yrt == NULL) 		
 	    continue;
+	/* Refine ANYDATA does not make sense */
+	if (yang_keyword_get(yrt) == Y_ANYDATA || yang_keyword_get(yrt) == Y_ANYXML)
+	    continue;
 	/* Do the actual refinement */
 	if (ys_do_refine(yr, yrt) < 0)
 	    goto done;

test/test_feature.sh

@@ -123,21 +123,31 @@ EOF
 # Run netconf feature test
 # 1: syntax node
 # 2: disabled or enabled
+# NOTE, this was before failures when disabled, but after https://github.com/clicon/clixon/issues/322 that
+# disabled nodes should be "ignored". Instead now if disabled a random node is inserted under the disabled node
+# which should not work 
 function testrun()
 {
     node=$1
     enabled=$2
     
+    new "netconf set $node"
+    expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><$node xmlns=\"urn:example:clixon\">foo</$node></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
+    new "netconf validate $node"
+    expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
     if $enabled; then
-	new "netconf set $node"
+	new "netconf set extra element under $node (expect fail)"
-	expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><$node xmlns=\"urn:example:clixon\">foo</$node></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+	expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><$node xmlns=\"urn:example:clixon\"><kallekaka/></$node></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><rpc-error><error-type>application</error-type><error-tag>unknown-element</error-tag><error-info><bad-element>kallekaka</bad-element></error-info><error-severity>error</error-severity><error-message>Failed to find YANG spec of XML node: kallekaka with parent: $node in namespace: urn:example:clixon</error-message></rpc-error></rpc-reply>"
+    else
+	new "netconf set extra element under $node"
+	expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><$node xmlns=\"urn:example:clixon\"><kallekaka/></$node></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
 
 	new "netconf validate $node"
 	expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
-    else
-	new "netconf set $node, expect fail"
-	expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><$node xmlns=\"urn:example:clixon\">foo</$node></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><rpc-error><error-type>application</error-type><error-tag>unknown-element</error-tag><error-info><bad-element>$node</bad-element></error-info><error-severity>error</error-severity><error-message>Failed to find YANG spec of XML node: $node with parent: config in namespace: urn:example:clixon</error-message></rpc-error></rpc-reply>"
     fi
+
     new "netconf discard-changes"
     expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><discard-changes/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
 }

test/test_feature_startup.sh

@@ -0,0 +1,101 @@
+#!/usr/bin/env bash
+# Yang features. if-feature.
+# The test has a example module with FEATURES A and B, where A is enabled and
+# Ignore dont throw error, see https://github.com/clicon/clixon/issues/322
+
+# Magic line must be first in script (see README.md)
+s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
+
+APPNAME=example
+
+cfg=$dir/conf_yang.xml
+fyang=$dir/test.yang
+
+# Note ietf-routing@2018-03-13 assumed
+cat <<EOF > $cfg
+<clixon-config xmlns="http://clicon.org/config">
+  <CLICON_FEATURE>ietf-netconf:startup</CLICON_FEATURE>
+  <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
+  <CLICON_YANG_DIR>${YANG_INSTALLDIR}</CLICON_YANG_DIR>
+  <CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>
+  <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
+  <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
+  <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
+  <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
+  <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
+  <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
+</clixon-config>
+EOF
+
+cat <<EOF > $fyang
+module example{
+   yang-version 1.1;
+   namespace "urn:example:clixon";
+   prefix ex;
+
+   feature A{
+      description "This test feature is enabled";
+   }
+
+    grouping mytop {
+        container mycontainer {
+            if-feature A;
+            leaf myleaf {
+                type string;
+           }
+       }
+    }
+    uses mytop;
+}
+EOF
+
+cat<<EOF > $dir/startup_db
+<${DATASTORE_TOP}>
+     <mycontainer xmlns="urn:example:clixon">
+      <myleaf>boo</myleaf>
+   </mycontainer>
+</${DATASTORE_TOP}>
+EOF
+
+#
+testrun()
+{
+    opt=$1
+
+    new "kill old backend"
+    sudo clixon_backend -zf $cfg
+    if [ $? -ne 0 ]; then
+	err
+    fi
+
+    new "start backend -s startup -f $cfg"
+    start_backend -s startup ${opt} -f $cfg
+
+    new "wait backend"
+    wait_backend
+}
+
+
+new "test params: -f $cfg"
+
+new "enable feature A"
+testrun "-o CLICON_FEATURE=example:A"
+
+new "enable feature B, expect fail"
+testrun "-o CLICON_FEATURE=example:B"
+
+if [ $BE -ne 0 ]; then
+    new "kill old backend"
+    sudo clixon_backend -zf $cfg
+    if [ $? -ne 0 ]; then
+	err
+    fi
+    # kill backend
+    stop_backend -f $cfg
+fi
+
+unset ret
+
+endtest
+
+rm -rf $dir

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

@@ -475,7 +475,9 @@ module clixon-config {
                  use with care.
                  The primary issue is that the unknown->anydata handling is not restricted to
                  only loading from startup but may occur in other circumstances as well. This
-                 means that sanity checks of erroneous XML/JSON may not be properly signalled.";
+                 means that sanity checks of erroneous XML/JSON may not be properly signalled.
+                 Note this is similar to what happens to YANG nodes that are disabled by a false
+                 if-feature statement.";
 	}
 	leaf CLICON_BACKEND_DIR {
 	    type string;