From f7b640810bf3da81f86a5d713900de2b71d65253 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Mon, 27 Jul 2020 12:06:36 +0200 Subject: [PATCH] * 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 | 3 + lib/src/clixon_yang.c | 180 +++++++++++++++++++++++++++++++++++------- test/test_feature.sh | 138 ++++++++++++++++++++++++++++---- 3 files changed, 274 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fed34fcc..3962fdcf 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index b10559a3..f80d186b 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -2064,14 +2064,156 @@ ys_populate2(yang_stmt *ys, return retval; } +/*! Handle complexity of if-feature node + * @param[in] h Clixon handle + * @param[in] ys Yang if-feature statement + * @retval -1 Error + * @retval 0 Feature not enabled: remove yt + * @retval 1 OK + * @note if-feature syntax is restricted to single, and, or, syntax, such as "a or b" + * but RFC7950 allows for nested expr/term/factor syntax. + */ +static int +yang_if_feature(clicon_handle h, + yang_stmt *ys) +{ + int retval = -1; + char **vec = NULL; + int nvec; + char *f; + int i; + 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; + + if ((vec = clicon_strsep(ys->ys_argument, " \t", &nvec)) == NULL) + goto done; + if (nvec%2 == 0){ /* Must be odd: eg a / "a or b" etc */ + clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument); + 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; iys_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; iys_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; + } + if (feature){ + free(feature); + 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 CLICON handle + * @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, @@ -2081,37 +2223,16 @@ yang_features(clicon_handle h, 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 ret; i = 0; while (iys_len){ /* Note, children may be removed */ ys = yt->ys_stmt[i]; if (ys->ys_keyword == Y_IF_FEATURE){ - if (nodeid_split(ys->ys_argument, &prefix, &feature) < 0) + if ((ret = yang_if_feature(h, ys)) < 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); - - /* Check if feature exists, and is set, otherwise remove */ - if ((yfeat = yang_find(ymod, Y_FEATURE, feature)) == NULL || - yfeat->ys_cv == NULL || !cv_bool_get(yfeat->ys_cv)){ - retval = 0; /* feature not enabled */ - goto done; - } - if (prefix){ - free(prefix); - prefix = NULL; - } - if (feature){ - free(feature); - feature = NULL; - } + if (ret == 0) + goto disabled; } else if (ys->ys_keyword == Y_FEATURE){ @@ -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 diff --git a/test/test_feature.sh b/test/test_feature.sh index de822e30..3d38b404 100755 --- a/test/test_feature.sh +++ b/test/test_feature.sh @@ -29,6 +29,7 @@ fyang=$dir/test.yang cat < $cfg $APPNAME:A + $APPNAME:A1 ietf-routing:router-id $cfg /usr/local/share/clixon @@ -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 "<$node xmlns=\"urn:example:clixon\">foo]]>]]>" "^]]>]]>$" + + new "netconf validate $node" + expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + else + new "netconf set $node, expect fail" + expecteof "$clixon_netconf -qf $cfg" 0 "<$node xmlns=\"urn:example:clixon\">foo]]>]]>" "^applicationunknown-element$nodeerrorFailed to find YANG spec of XML node: $node with parent: config in namespace: urn:example:clixon]]>]]>$" + fi + new "netconf discard-changes" + expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" +} + 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 "]]>]]>" "^]]>]]>$" -new "netconf enabled feature" -expecteof "$clixon_netconf -qf $cfg" 0 'foo]]>]]>' "^]]>]]>$" +# Single if-feature +testrun x true -new "netconf validate enabled feature" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" +testrun y false -new "netconf disabled feature" -expecteof "$clixon_netconf -qf $cfg" 0 'foo]]>]]>' '^applicationunknown-elementyerrorFailed to find YANG spec of XML node: y with parent: config in namespace: urn:example:clixon]]>]]>$' +# Multiple if-feature +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="exampleurn:example:clixonAimplement" +expect="exampleurn:example:clixonAA1implement" 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="clixon-config2018-09-30" match=`echo "$ret" | grep --null -Go "$expect"` @@ -184,17 +259,46 @@ if [ -z "$match" ]; then err "$expect" "$ret" fi -if [ $BE -eq 0 ]; then - exit # BE +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 -new "Kill backend" -# Check if premature kill -pid=$(pgrep -u root -f clixon_backend) -if [ -z "$pid" ]; then - err "backend already dead" +#------------------------ +# Negative test, if if-feature but no feature, signal error +cat < $fyang +module example{ + 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