* Enhanced Clixon if-feature handling:

* If-feature now supports and/or lists, such as: `if-feature "a and b"` and `if-feature "a or b or c"`. However, full if-feature-expr including `not` and nested boolean experessions is still not supported.
  * Sanity check: if an `if-feature` statement exists, a corresponding `feature` statement must exists that declares that feature.

CHANGELOG.md

@@ -83,6 +83,9 @@ Expected: July 2020
   
 ### Minor changes
 
+* Enhanced Clixon if-feature handling:
+  * If-feature now supports and/or lists, such as: `if-feature "a and b"` and `if-feature "a or b or c"`. However, full if-feature-expr including `not` and nested boolean experessions is still not supported.
+  * Sanity check: if an `if-feature` statement exists, a corresponding `feature` statement must exists that declares that feature.
 * Optimized get config xpath of large lists, such as `a[x=1000]` in a list of 100000s `a:s`.
 * Added docker support for three restconf modes: nginx/fcgi(default); evhtp ; and none.
 * Added [Vagrant tests](test/vagrant/README.md)

lib/src/clixon_yang.c

@@ -2064,46 +2064,122 @@ ys_populate2(yang_stmt    *ys,
     return retval;
 }
 
-/*! Find feature and if-feature nodes, check features and remove disabled nodes
+/*! Handle complexity of if-feature node
- * @param[in] h   CLICON handle
+ * @param[in] h   Clixon handle
- * @param[in] yt  Yang statement
+ * @param[in] ys  Yang if-feature statement
  * @retval   -1   Error
  * @retval    0   Feature not enabled: remove yt
  * @retval    1   OK
- * @note On return 1 the over-lying function need to remove yt from its parent
+ * @note if-feature syntax is restricted to single, and, or, syntax, such as "a or b"
- * @note cannot use yang_apply here since child-list is modified (destructive) 
+ * but RFC7950 allows for nested expr/term/factor syntax.
  */
-int
+static int
-yang_features(clicon_handle h,
+yang_if_feature(clicon_handle h,
-	      yang_stmt    *yt)
+		yang_stmt    *ys)
 {
     int         retval = -1;
+    char      **vec = NULL;
+    int         nvec;
+    char       *f;
     int         i;
-    int        j;
-    yang_stmt *ys = NULL;
     char       *prefix = NULL;
     char       *feature = NULL;
     yang_stmt  *ymod;  /* module yang node */
     yang_stmt  *yfeat; /* feature yang node */
+    int         opand = -1; /* -1:not set, 0:or, 1:and */
+    int         enabled = 0;
     
-    i = 0;
+    if ((vec = clicon_strsep(ys->ys_argument, " \t", &nvec)) == NULL)
-    while (i<yt->ys_len){ /* Note, children may be removed */
+	goto done;
-	ys = yt->ys_stmt[i];
+    if (nvec%2 == 0){ /* Must be odd: eg a / "a or b" etc */
-	if (ys->ys_keyword == Y_IF_FEATURE){
+	clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument);
-	    if (nodeid_split(ys->ys_argument, &prefix, &feature) < 0)
+	goto done;
+    }
+    /* Two steps: first detect operators
+     * Step 1: support "a" or "a or b or c" or "a and b and c "
+     */
+    for (i=0; i<nvec; i++){
+	if (i%2==0) /* only keep odd i:s 1,3,... */
+	    continue;
+	f = vec[i]; 
+	/* odd i: operator "and" or "or" */
+	if (strcmp(f, "or") == 0){
+	    switch (opand){
+	    case -1:
+		if (i != 1){
+		    clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument);
+		    goto done;
+		}
+		opand = 0;
+		break;
+	    case 0:
+		break;
+	    case 1:
+		clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument);
+		goto done;
+		break;
+	    }
+	}
+	else if (strcmp(f, "and") == 0){
+	    switch (opand){
+	    case -1:
+		if (i != 1){
+		    clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument);
+		    goto done;
+		}
+		opand = 1;
+		break;
+	    case 0:
+		clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument);
+		goto done;
+		break;
+	    case 1:
+		break;
+	    }
+	}
+	else{
+	    clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument);
+	    goto done;
+	}
+    } /* for step 1 */
+    if (opand == -1) /* Uninitialized means single operand */
+	opand = 1;
+    if (opand) /* if AND,  start as enabled, if OR start as disabled */
+	enabled = 1;
+    else
+	enabled = 0;
+    /* Step 2: Boolean operations on operands */
+    for (i=0; i<nvec; i++){
+	if (i%2==1) /* only keep even i:s 0,2,... */
+	    continue;
+	f = vec[i]; 
+	if (nodeid_split(f, &prefix, &feature) < 0)
 	    goto done;
 	/* Specifically need to handle? strcmp(prefix, myprefix)) */
 	if (prefix == NULL)
 	    ymod = ys_module(ys);
 	else
-		ymod = yang_find_module_by_prefix(yt, prefix);
+	    ymod = yang_find_module_by_prefix(ys, prefix);
-
 	/* Check if feature exists, and is set, otherwise remove */
-	    if ((yfeat = yang_find(ymod, Y_FEATURE, feature)) == NULL ||
+	if ((yfeat = yang_find(ymod, Y_FEATURE, feature)) == NULL){
-		yfeat->ys_cv == NULL || !cv_bool_get(yfeat->ys_cv)){
+	    clicon_err(OE_YANG, EINVAL, "Yang module %s has IF_FEATURE %s, but no such FEATURE statement exists",
-		retval = 0; /* feature not enabled */
+		       ymod?yang_argument_get(ymod):"none",
+		       feature);
 	    goto done;
 	}
+	/* Check if this feature is enabled or not 
+	 * Continue loop to catch unbound features and make verdict at end
+	 */
+	if (yfeat->ys_cv == NULL || !cv_bool_get(yfeat->ys_cv)){    /* disabled */
+	    /* if AND then this is permanently disabled */
+	    if (opand && enabled)
+		enabled = 0;
+	}
+	else{                                                       /* enabled */
+	    /* if OR then this is permanently enabled */
+	    if (!opand && !enabled)
+		enabled = 1;
+	}
 	if (prefix){
 	    free(prefix);
 	    prefix = NULL;
@@ -2113,6 +2189,51 @@ yang_features(clicon_handle h,
 	    feature = NULL;
 	}
     }
+    if (!enabled)
+	goto disabled;
+    retval = 1;
+ done:
+    if (vec)
+	free(vec); 
+    if (prefix)
+	free(prefix);
+    if (feature)
+	free(feature);
+    return retval;
+ disabled: 
+    retval = 0;  /* feature not enabled */
+    goto done;
+}
+
+/*! Find feature and if-feature nodes, check features and remove disabled nodes
+ * @param[in] h   Clixon handle
+ * @param[in] yt  Yang statement
+ * @retval   -1   Error
+ * @retval    0   Feature not enabled: remove yt
+ * @retval    1   OK
+ * @note On return 1 the over-lying function need to remove yt from its parent
+ * @note cannot use yang_apply here since child-list is modified (destructive) 
+ * @note if-feature syntax is restricted to single, and, or, syntax, such as "a or b"
+ */
+int
+yang_features(clicon_handle h,
+	      yang_stmt    *yt)
+{
+    int        retval = -1;
+    int        i;
+    int        j;
+    yang_stmt *ys = NULL;
+    int        ret;
+
+    i = 0;
+    while (i<yt->ys_len){ /* Note, children may be removed */
+	ys = yt->ys_stmt[i];
+	if (ys->ys_keyword == Y_IF_FEATURE){
+	    if ((ret = yang_if_feature(h, ys)) < 0)
+		goto done;
+	    if (ret == 0)
+		goto disabled;
+	}
 	else
 	    if (ys->ys_keyword == Y_FEATURE){
 		if (ys_populate_feature(h, ys) < 0)
@@ -2136,11 +2257,10 @@ yang_features(clicon_handle h,
     }
     retval = 1;
  done:
-    if (prefix)
-	free(prefix);
-    if (feature)
-	free(feature);
     return retval;
+ disabled: 
+    retval = 0;  /* feature not enabled */
+    goto done;
 }
 
 /*! Apply a function call recursively on all yang-stmt s recursively

test/test_feature.sh

@@ -29,6 +29,7 @@ fyang=$dir/test.yang
 cat <<EOF > $cfg
 <clixon-config xmlns="http://clicon.org/config">
   <CLICON_FEATURE>$APPNAME:A</CLICON_FEATURE>
+  <CLICON_FEATURE>$APPNAME:A1</CLICON_FEATURE>
   <CLICON_FEATURE>ietf-routing:router-id</CLICON_FEATURE>
   <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
   <CLICON_YANG_DIR>/usr/local/share/clixon</CLICON_YANG_DIR>
@@ -55,9 +56,15 @@ module example{
    feature A{
       description "This test feature is enabled";
    }
+   feature A1{
+      description "This test feature is enabled (extra for multiple)";
+   }
    feature B{
       description "This test feature is disabled";
    }
+   feature B1{
+      description "This test feature is disabled (extra for multiple)";
+   }
    leaf x{
      if-feature A;
      type "string";
@@ -69,9 +76,71 @@ module example{
    leaf z{
      type "string";
    }
+   leaf m1{
+     if-feature "A and A1";
+     description "Enabled";
+     type "string";
+   }
+   leaf m2{
+     if-feature "A or A1";
+     description "Enabled";
+     type "string";
+   }
+   leaf m3{
+     if-feature "A and B";
+     description "Not enabled";
+     type "string";
+   }
+   leaf m4{
+     if-feature "A or B";
+     description "Enabled";
+     type "string";
+   }
+   leaf m5{
+     if-feature "B and B1";
+     description "Not enabled";
+     type "string";
+   }
+   leaf m6{
+     if-feature "B or B1";
+     description "Not enabled";
+     type "string";
+   }
+   leaf m7{
+     if-feature "A or A1 or B or B1";
+     description "Enabled";
+     type "string";
+   }
+   leaf m8{
+     if-feature "A and A1 and B and B1";
+     description "Not enabled";
+     type "string";
+   }
 }
 EOF
 
+# Run netconf feature test
+# 1: syntax node
+# 2: disabled or enabled
+testrun()
+{
+    node=$1
+    enabled=$2
+    
+    if $enabled; then
+	new "netconf set $node"
+	expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><edit-config><target><candidate/></target><config><$node xmlns=\"urn:example:clixon\">foo</$node></config></edit-config></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
+
+	new "netconf validate $node"
+	expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
+    else
+	new "netconf set $node, expect fail"
+	expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><edit-config><target><candidate/></target><config><$node xmlns=\"urn:example:clixon\">foo</$node></config></edit-config></rpc>]]>]]>" "^<rpc-reply><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 "$clixon_netconf -qf $cfg" 0 "<rpc><discard-changes/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
+}
+
 new "test params: -f $cfg"
 if [ $BE -ne 0 ]; then
     new "kill old backend"
@@ -102,14 +171,20 @@ expectfn "$clixon_cli -1f $cfg -l o set routing ribs rib default-rib false" 255
 new "netconf discard-changes"
 expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><discard-changes/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
 
-new "netconf enabled feature"
+# Single if-feature
-expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><x xmlns="urn:example:clixon">foo</x></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
+testrun x true
 
-new "netconf validate enabled feature"
+testrun y false
-expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
 
-new "netconf disabled feature"
+# Multiple if-feature
-expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><y xmlns="urn:example:clixon">foo</y></config></edit-config></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>unknown-element</error-tag><error-info><bad-element>y</bad-element></error-info><error-severity>error</error-severity><error-message>Failed to find YANG spec of XML node: y with parent: config in namespace: urn:example:clixon</error-message></rpc-error></rpc-reply>]]>]]>$'
+testrun m1 true
+testrun m2 true
+testrun m3 false
+testrun m4 true
+testrun m5 false
+testrun m6 false
+testrun m7 true
+testrun m8 false
 
 # This test has been broken up into all different modules instead of one large
 # reply since the modules change so often
@@ -128,13 +203,13 @@ if [ -z "$match" ]; then
 fi
 
 new "netconf module A"
-expect="<module><name>example</name><revision/><namespace>urn:example:clixon</namespace><feature>A</feature><conformance-type>implement</conformance-type></module>"
+expect="<module><name>example</name><revision/><namespace>urn:example:clixon</namespace><feature>A</feature><feature>A1</feature><conformance-type>implement</conformance-type></module>"
 match=`echo "$ret" | grep --null -Go "$expect"`
 if [ -z "$match" ]; then
       err "$expect" "$ret"
 fi
 
-if false ; then # clixon "config" bug
+if false ; then # clixon "config" is a meta-config and not visisble in regular features
 new "netconf module clixon-config"
 expect="<module><name>clixon-config</name><revision>2018-09-30</revision><namespace/></module>"
 match=`echo "$ret" | grep --null -Go "$expect"`
@@ -184,17 +259,46 @@ if [ -z "$match" ]; then
       err "$expect" "$ret"
 fi
 
-if [ $BE -eq 0 ]; then
+if [ $BE -ne 0 ]; then
-    exit # BE
+    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
 
-new "Kill backend"
+#------------------------
-# Check if premature kill
+# Negative test, if if-feature but no feature, signal error
-pid=$(pgrep -u root -f clixon_backend)
+cat <<EOF > $fyang
-if [ -z "$pid" ]; then
+module example{
-    err "backend already dead"
+   yang-version 1.1;
+   namespace "urn:example:clixon";
+   prefix ex;
+   feature A{
+      description "This feature exists";
+   }
+   leaf x{
+     if-feature "A or B";
+     type "string";
+   }
+}
+EOF
+
+new "test params: -f $cfg"
+if [ $BE -ne 0 ]; then
+    new "kill old backend"
+    sudo clixon_backend -zf $cfg
+    if [ $? -ne 0 ]; then
+	err
+    fi
+
+    new "start backend -s init -f $cfg: feature missing expected fail"
+    expectpart "$(sudo $clixon_backend -F1s init -f $cfg -l o)" 255 " Yang module example has IF_FEATURE B, but no such FEATURE statement exists"
+
+    stop_backend -f $cfg
 fi
-# kill backend
-stop_backend -f $cfg
 
 rm -rf $dir