From ca695ea38609f14f5b45a67d2f82f20ca5e3241e Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Fri, 8 Nov 2024 16:37:31 +0100 Subject: [PATCH] Fixed: [Error with submodules and feature Interaction](https://github.com/clicon/clixon-controller/issues/158) --- CHANGELOG.md | 1 + lib/src/clixon_yang.c | 14 ++-- lib/src/clixon_yang_parse_lib.c | 2 +- lib/src/clixon_yang_sub_parse.c | 26 ++++---- lib/src/clixon_yang_sub_parse.h | 18 ++--- lib/src/clixon_yang_sub_parse.y | 24 +++++-- test/test_feature.sh | 114 ++++++++++++++++++++++++++------ 7 files changed, 143 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 444a2ab3..947a26de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ Developers may need to change their code ### Corrected Bugs +* Fixed: [Error with submodules and feature Interaction](https://github.com/clicon/clixon-controller/issues/158) * Fixed: [Expansion removes the double quote](https://github.com/clicon/clixon/issues/524) * Add escaping in expand_dbvar instead of automatic in cligen expand * Fixed: [string length validation doesn't work for the entry "" in case it has default value specified](https://github.com/clicon/clixon/issues/563) diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index a0be0004..ee1ce23e 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -3613,7 +3613,7 @@ yang_features(clixon_handle h, yang_stmt *ys = NULL; yang_stmt *ymod; const char *mainfile = NULL; - int ret; + int enabled; i = 0; while (iys_len){ @@ -3622,16 +3622,16 @@ yang_features(clixon_handle h, /* Parse the if-feature-expr string using yang sub-parser */ if ((ymod = ys_module(ys)) != NULL) mainfile = yang_filename_get(ymod); - ret = 0; - if (yang_subparse(yang_argument_get(ys), ys, YA_IF_FEATURE, mainfile, 1, &ret, h) < 0) + enabled = 0; + if (yang_subparse(h, yang_argument_get(ys), ys, YA_IF_FEATURE, mainfile, 1, &enabled) < 0) goto done; - clixon_debug(CLIXON_DBG_YANG | CLIXON_DBG_DETAIL, "%s %d", yang_argument_get(ys), ret); - if (ret == 0) + clixon_debug(CLIXON_DBG_YANG | CLIXON_DBG_DETAIL, "%s %d", yang_argument_get(ys), enabled); + if (enabled == 0) goto disabled; } else if (ys->ys_keyword == Y_FEATURE){ - if (ys_populate_feature(h, ys) < 0) - goto done; + if (ys_populate_feature(h, ys) < 0) + goto done; } else switch (yang_features(h, ys)){ diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index c7fdd524..94a4d6f1 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/lib/src/clixon_yang_parse_lib.c @@ -2060,7 +2060,7 @@ ys_parse_sub(yang_stmt *ys, * pass 1 is not yet resolved, only check syntax, actual feature check made in next pass * @see yang_features */ - if (yang_subparse(yang_argument_get(ys), ys, YA_IF_FEATURE, filename, yang_linenum_get(ys), NULL, NULL) < 0) + if (yang_subparse(NULL, yang_argument_get(ys), ys, YA_IF_FEATURE, filename, yang_linenum_get(ys), NULL) < 0) goto done; break; case Y_AUGMENT: /* If parent is module/submodule: absolute-schema-nodeid diff --git a/lib/src/clixon_yang_sub_parse.c b/lib/src/clixon_yang_sub_parse.c index 69b9b6a1..07b32b2f 100644 --- a/lib/src/clixon_yang_sub_parse.c +++ b/lib/src/clixon_yang_sub_parse.c @@ -61,24 +61,24 @@ /*! Invoke yang sub-parser on string * - * @param[in] str yang string - * @param[in] ys Yang statement - * @param[in] accept Sub-parse rule to accept - * @param[in] mainfile Name of main parse file - * @param[in] linenum Line number context in mainfile (assume parse module of ys) - * @param[in] h Clixon handle - * @param[out] enabled 0: Disabled, 1: Enabled (if present) - * @retval 0 OK - * @retval -1 Error + * @param[in] h Clixon handle (can be NULL) + * @param[in] str Yang string + * @param[in] ys Yang statement + * @param[in] accept Sub-parse rule to accept + * @param[in] mainfile Name of main parse file + * @param[in] linenum Line number context in mainfile (assume parse module of ys) + * @param[out] enabled 0: Disabled, 1: Enabled (if present) + * @retval 0 OK + * @retval -1 Error */ int -yang_subparse(char *str, +yang_subparse(clixon_handle h, + char *str, yang_stmt *ys, enum yang_sub_parse_accept accept, const char *mainfile, int linenum, - int *enabled, - clixon_handle h) + int *enabled) { int retval = -1; clixon_yang_sub_parse_yacc ife = {0,}; @@ -90,7 +90,7 @@ yang_subparse(char *str, ife.if_ys = ys; /* Used as trigger to check if enabled */ ife.if_accept = accept; ife.if_mainfile = mainfile; - ife.h = h; + ife.if_h = h; if (clixon_yang_sub_parsel_init(&ife) < 0) goto done; if (clixon_yang_sub_parseparse(&ife) != 0) { /* yacc returns 1 on error */ diff --git a/lib/src/clixon_yang_sub_parse.h b/lib/src/clixon_yang_sub_parse.h index bd3c7e08..a002a19f 100644 --- a/lib/src/clixon_yang_sub_parse.h +++ b/lib/src/clixon_yang_sub_parse.h @@ -49,14 +49,14 @@ enum yang_sub_parse_accept{ /*! XML parser yacc handler struct */ struct clixon_yang_sub_parse_yacc { - char *if_parse_string; /* original (copy of) parse string */ - const char *if_mainfile; /* Original main-file (this is a sib-parser) */ - int if_linenum; /* Number of \n in parsed buffer (in mainfile) */ - void *if_lexbuf; /* Internal parse buffer from lex */ - yang_stmt *if_ys; /* Yang statement, NULL if no check */ - enum yang_sub_parse_accept if_accept; /* Which sub-parse rule to accept */ - int if_enabled; /* Result: 0: feature disabled, 1: enabled */ - clixon_handle h; + char *if_parse_string; /* original (copy of) parse string */ + const char *if_mainfile; /* Original main-file (this is a sib-parser) */ + int if_linenum; /* Number of \n in parsed buffer (in mainfile) */ + void *if_lexbuf; /* Internal parse buffer from lex */ + yang_stmt *if_ys; /* Yang statement, NULL if no check */ + enum yang_sub_parse_accept if_accept; /* Which sub-parse rule to accept */ + int if_enabled; /* Result: 0: feature disabled, 1: enabled */ + clixon_handle if_h; }; typedef struct clixon_yang_sub_parse_yacc clixon_yang_sub_parse_yacc; @@ -74,7 +74,7 @@ int clixon_yang_sub_parsel_linenr(void); int clixon_yang_sub_parselex(void *); int clixon_yang_sub_parseparse(void *); -int yang_subparse(char *str, yang_stmt *ys, enum yang_sub_parse_accept accept, const char *mainfile, int linenum, int *enabled, clixon_handle h); +int yang_subparse(clixon_handle h, char *str, yang_stmt *ys, enum yang_sub_parse_accept accept, const char *mainfile, int linenum, int *enabled); int yang_schema_nodeid_subparse(char *str, enum yang_sub_parse_accept accept, const char *mainfile, int linenum); #endif /* _CLIXON_YANG_SUB_PARSER_H_ */ diff --git a/lib/src/clixon_yang_sub_parse.y b/lib/src/clixon_yang_sub_parse.y index 004dfdbe..f44bb2bb 100644 --- a/lib/src/clixon_yang_sub_parse.y +++ b/lib/src/clixon_yang_sub_parse.y @@ -86,6 +86,8 @@ #include #include +#include // XXX + /* cligen */ #include @@ -128,10 +130,11 @@ clixon_yang_sub_parseerror(void *arg, /*! Check if feature "str" is enabled or not in context of yang node ys * + * @param[in] ife If-feature struct * @param[in] str feature str. - * @param[in] ys If-feature type yang node * @retval 0 OK * @retval -1 Error + * @note ife->if_ys is used as implicit flag to perform actual checks */ static int if_feature_check(clixon_yang_sub_parse_yacc *ife, @@ -150,12 +153,19 @@ if_feature_check(clixon_yang_sub_parse_yacc *ife, if (nodeid_split(str, &prefix, &feature) < 0) goto done; /* Specifically need to handle? strcmp(prefix, myprefix)) */ - if (prefix == NULL) - ymod = ys_module(ys); - else + if (prefix != NULL){ ymod = yang_find_module_by_prefix(ys, prefix); - if (ymod == NULL) + if (ys_real_module(ymod, &ymod) < 0) + goto done; + } + else { + if (ys_real_module(ys, &ymod) < 0) + goto done; + } + if (ymod == NULL){ + clixon_err(OE_YANG, 0, "Module not found: %s", str); goto done; + } /* Check if feature exists, and is set, otherwise remove */ if ((yfeat = yang_find(ymod, Y_FEATURE, feature)) == NULL){ clixon_err(OE_YANG, EINVAL, "Yang module %s has IF_FEATURE %s, but no such FEATURE statement exists", @@ -167,8 +177,8 @@ if_feature_check(clixon_yang_sub_parse_yacc *ife, * Continue loop to catch unbound features and make verdict at end */ cv = yang_cv_get(yfeat); - if (cv == NULL && ife->h) { - ys_populate_feature(ife->h, yfeat); + if (cv == NULL && ife->if_h) { + ys_populate_feature(ife->if_h, yfeat); } cv = yang_cv_get(yfeat); diff --git a/test/test_feature.sh b/test/test_feature.sh index 13b773c8..57eb1dd9 100755 --- a/test/test_feature.sh +++ b/test/test_feature.sh @@ -23,7 +23,8 @@ s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi APPNAME=example cfg=$dir/conf_yang.xml -fyang=$dir/test.yang +fyang=$dir/example.yang +fsub1=$dir/sub1.yang # Note ietf-routing@2018-03-13 assumed cat < $cfg @@ -34,6 +35,7 @@ cat < $cfg $cfg ${YANG_INSTALLDIR} $IETFRFC + ${dir} $fyang /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/cli @@ -45,11 +47,36 @@ cat < $cfg EOF +cat < $fsub1 +submodule sub1{ + yang-version 1.1; + belongs-to example { + prefix ex; + } + feature S1{ + description "submodule feature"; + } + leaf subA1{ + if-feature A; + type "string"; + } + leaf subA2{ + if-feature ex:A; + type "string"; + } + leaf subS1{ + if-feature S1; + type "string"; + } +} +EOF + cat < $fyang module example{ yang-version 1.1; namespace "urn:example:clixon"; prefix ex; + include sub1; import ietf-routing { prefix rt; } @@ -76,18 +103,18 @@ module example{ leaf u { if-feature rt:router-id; type "string"; - } + } leaf z{ type "string"; } leaf m1{ - if-feature "A and + if-feature "A and A1"; description "Enabled"; type "string"; } leaf m2{ - if-feature "A or + if-feature "A or A1"; description "Enabled"; type "string"; @@ -139,6 +166,10 @@ module example{ description "Disabled"; type "string"; } + leaf subS1{ + if-feature S1; + type "string"; + } } EOF @@ -147,12 +178,12 @@ EOF # 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 +# which should not work function testrun() { node=$1 enabled=$2 - + new "netconf set $node" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<$node xmlns=\"urn:example:clixon\">foo" "" "" @@ -227,7 +258,7 @@ testrun m11 false # reply since the modules change so often new "netconf schema resource, RFC 7895" rpc=$(chunked_framing "") -ret=$($clixon_netconf -qf $cfg< $fyang module example{ yang-version 1.1; @@ -324,21 +364,57 @@ module example{ } } 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" + new "Feature B 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 +# Negative test 2, if if-feature but no feature, signal error +cat < $fyang +module example{ + yang-version 1.1; + namespace "urn:example:clixon"; + prefix ex; + include sub1; + leaf x{ + if-feature "S2"; + type "string"; + } +} +EOF +if [ $BE -ne 0 ]; then + new "Feature S2 missing expected fail" + expectpart "$(sudo $clixon_backend -F1s init -f $cfg -l o)" 255 " Yang module example has IF_FEATURE S2, but no such FEATURE statement exists" + + stop_backend -f $cfg +fi + +# Negative test 3, if if-feature but no feature, signal error +cat < $fsub1 +submodule sub1{ + yang-version 1.1; + belongs-to example { + prefix ex; + } + feature S2{ + description "submodule feature"; + } + leaf x{ + if-feature "A"; + type "string"; + } +} +EOF + +if [ $BE -ne 0 ]; then + new "Feature A missing expected fail" + expectpart "$(sudo $clixon_backend -F1s init -f $cfg -l o)" 255 " Yang module example has IF_FEATURE A, but no such FEATURE statement exists" + + stop_backend -f $cfg +fi + rm -rf $dir new "endtest"