From 0e81e8137bc837ce167d2139fdd13b6136621fbb Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Mon, 21 Aug 2023 10:54:48 +0200 Subject: [PATCH] [YANG error when poking on EOS configuration](https://github.com/clicon/clixon-controller/issues/26) --- CHANGELOG.md | 1 + apps/cli/cli_generate.c | 4 +- example/main/clixon-example@2020-12-01.yang | 233 -------------------- lib/src/clixon_xpath_yang.c | 14 +- test/test_autocli_grouping.sh | 216 +++++++++++------- test/test_netconf_monitoring.sh | 2 +- 6 files changed, 151 insertions(+), 319 deletions(-) delete mode 100644 example/main/clixon-example@2020-12-01.yang diff --git a/CHANGELOG.md b/CHANGELOG.md index d71c0c94..a7ecdfcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Developers may need to change their code ### Corrected Bugs +* [YANG error when poking on EOS configuration](https://github.com/clicon/clixon-controller/issues/26) * [CLICON_CONFIGDIR with external subsystems causes endless looping](https://github.com/clicon/clixon/issues/439) * Fixed: ["show configuration devices" and "show configuration devices | display cli" differs](https://github.com/clicon/clixon-controller/issues/24) * Fixed: [Configuring Juniper PTX produces CLI errors](https://github.com/clicon/clixon-controller/issues/19) diff --git a/apps/cli/cli_generate.c b/apps/cli/cli_generate.c index 8dd6ffa2..1518ac13 100644 --- a/apps/cli/cli_generate.c +++ b/apps/cli/cli_generate.c @@ -724,7 +724,7 @@ yang2cli_var(clicon_handle h, } cprintf(cb, ")"); } - else if (strcmp(restype,"leafref")==0){ + else if (strcmp(restype, "leafref")==0){ yang_stmt *ypath; char *path_arg; yang_stmt *yref = NULL; @@ -1202,7 +1202,7 @@ yang2cli_stmt(clicon_handle h, int retval = -1; yang_stmt *yc; int treeref_state = 0; - int grouping_treeref; + int grouping_treeref = 0; if (ys == NULL){ clicon_err(OE_YANG, EINVAL, "No yang spec"); diff --git a/example/main/clixon-example@2020-12-01.yang b/example/main/clixon-example@2020-12-01.yang deleted file mode 100644 index 71980e39..00000000 --- a/example/main/clixon-example@2020-12-01.yang +++ /dev/null @@ -1,233 +0,0 @@ -module clixon-example { - yang-version 1.1; - namespace "urn:example:clixon"; - prefix ex; - import ietf-interfaces { - /* is in yang/optional which means clixon must be installed using --opt-yang-installdir */ - prefix if; - } - import ietf-ip { - prefix ip; - } - import iana-if-type { - prefix ianaift; - } - import ietf-datastores { - prefix ds; - } - import clixon-autocli{ - prefix autocli; - } - description - "Clixon example used as a part of the Clixon test suite. - It can be used as a basis for making new Clixon applications. - Note, may change without updating revision, just for testing current master. - "; - revision 2020-12-01 { - description "Added table/parameter/value as the primary data example"; - } - revision 2020-03-11 { - description "Added container around translation list. Released in Clixon 4.4.0"; - } - revision 2019-11-05 { - description "Augment interface. Released in Clixon 4.3.0"; - } - revision 2019-07-23 { - description "Extension e4. Released in Clixon 4.1.0"; - } - revision 2019-01-13 { - description "Released in Clixon 3.9"; - } - /* Example interface type for tests, local callbacks, etc */ - identity eth { - base if:interface-type; - } - identity loopback { - base if:interface-type; - } - /* Generic config data */ - container table{ - list parameter{ - key name; - leaf name{ - type string; - } - leaf value{ - type string; - } - leaf hidden{ - type string; - autocli:hide; - } - leaf stat{ - description "Inline state data for example application"; - config false; - type int32; - } - } - } - /* State data (not config) for the example application*/ - container state { - config false; - description "state data for the example application (must be here for example get operation)"; - leaf-list op { - type string; - } - } - augment "/if:interfaces/if:interface" { - container my-status { - config false; - description "For testing augment+state"; - leaf int { - type int32; - } - leaf str { - type string; - } - } - } - /* yang extension implemented by the example backend code. */ - extension e4 { - description - "The first child of the ex:e4 (unknown) statement is inserted into - the module as a regular data statement. This means that 'uses bar;' - in the ex:e4 statement below is a valid data node"; - argument arg; - } - grouping bar { - leaf bar{ - type string; - } - } - ex:e4 arg1{ - uses bar; - } - /* Example notification as used in RFC 5277 and RFC 8040 */ - notification event { - description "Example notification event."; - leaf event-class { - type string; - description "Event class identifier."; - } - container reportingEntity { - description "Event specific information."; - leaf card { - type string; - description "Line card identifier."; - } - } - leaf severity { - type string; - description "Event severity description."; - } - } - rpc client-rpc { - description "Example local client-side RPC that is processed by the - the netconf/restconf and not sent to the backend. - This is a clixon implementation detail: some rpc:s - are better processed by the client for API or perf reasons"; - input { - leaf x { - type string; - } - } - output { - leaf x { - type string; - } - } - } - rpc empty { - description "Smallest possible RPC with no input or output sections"; - } - rpc optional { - description "Small RPC with optional input and output"; - input { - leaf x { - type string; - } - } - output { - leaf x { - type string; - } - } - } - rpc example { - description "Some example input/output for testing RFC7950 7.14. - RPC simply echoes the input for debugging."; - input { - leaf x { - description - "If a leaf in the input tree has a 'mandatory' statement with - the value 'true', the leaf MUST be present in an RPC invocation."; - type string; - mandatory true; - } - leaf y { - description - "If a leaf in the input tree has a 'mandatory' statement with the - value 'true', the leaf MUST be present in an RPC invocation."; - type string; - default "42"; - } - leaf-list z { - description - "If a leaf-list in the input tree has one or more default - values, the server MUST use these values (XXX not supported)"; - type string; - } - leaf w { - description - "If any node has a 'when' statement that would evaluate to - 'false',then this node MUST NOT be present in the input tree. - (XXX not supported)"; - type string; - } - list u0 { - description "list without key"; - leaf uk{ - type string; - } - } - list u1 { - description "list with key"; - key uk; - leaf uk{ - type string; - } - leaf val{ - type string; - } - } - } - output { - leaf x { - type string; - } - leaf y { - type string; - } - leaf z { - type string; - } - leaf w { - type string; - } - list u0 { - leaf uk{ - type string; - } - } - list u1 { - key uk; - leaf uk{ - type string; - } - leaf val{ - type string; - } - } - } - } -} diff --git a/lib/src/clixon_xpath_yang.c b/lib/src/clixon_xpath_yang.c index 4afb83f9..28ac909e 100644 --- a/lib/src/clixon_xpath_yang.c +++ b/lib/src/clixon_xpath_yang.c @@ -109,9 +109,9 @@ xy_dup(xp_yang_ctx *xy0) * Always evaluates to true since there are no instances */ static int -xp_yang_op_eq(xp_yang_ctx *xy1, - xp_yang_ctx *xy2, - xp_yang_ctx **xyr) +xp_yang_op_eq(xp_yang_ctx *xy1, + xp_yang_ctx *xy2, + xp_yang_ctx **xyr) { int retval = -1; xp_yang_ctx *xy = NULL; @@ -119,11 +119,11 @@ xp_yang_op_eq(xp_yang_ctx *xy1, if ((xy = xy_dup(xy1)) == NULL) goto done; if (xy1 == NULL || xy2 == NULL || xy1->xy_node == NULL || xy2->xy_node == NULL){ - clicon_err(OE_YANG, EINVAL, "Invalid path-arg (Error in xy1 or xy2) "); - goto done; + xy->xy_bool = 0; } + else + xy->xy_bool = 1; xy->xy_type = XT_BOOL; - xy->xy_bool = 1; xy->xy_node = NULL; *xyr = xy; retval = 0; @@ -251,7 +251,7 @@ xp_yang_eval_predicate(xp_yang_ctx *xy, if (xp_yang_eval(xy0, xptree->xs_c1, &xy1) < 0) goto done; /* Check xrc: if "true" then xyr=xy0? */ - if (xy1->xy_type == XT_BOOL && xy1->xy_bool) + if (xy1 && xy1->xy_type == XT_BOOL && xy1->xy_bool) ; else xy0->xy_node = NULL; diff --git a/test/test_autocli_grouping.sh b/test/test_autocli_grouping.sh index 293ce141..311ca617 100755 --- a/test/test_autocli_grouping.sh +++ b/test/test_autocli_grouping.sh @@ -7,16 +7,19 @@ s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi APPNAME=example cfg=$dir/conf_yang.xml +cfd=$dir/conf_yang.d +if [ ! -d $cfd ]; then + mkdir $cfd +fi fyang=$dir/example.yang fyang2=$dir/example-external.yang fyang3=$dir/example-external3.yang -# Whether grouping treeref is enabled -grouping_treeref=true - +# XXX try -E? cat < $cfg $cfg + $cfd ietf-netconf:startup ${YANG_INSTALLDIR} $dir @@ -29,16 +32,6 @@ cat < $cfg /usr/local/var/$APPNAME/$APPNAME.pidfile $dir false - - false - kw-nokey - ${grouping_treeref} - - include ${APPNAME} - enable - ${APPNAME}* - - EOF @@ -54,6 +47,7 @@ set @datamodel, cli_auto_set(); merge @datamodel, cli_auto_merge(); create @datamodel, cli_auto_create(); commit("Commit the changes"), cli_commit(); +validate("Validate changes"), cli_validate(); delete("Delete a configuration item") { @datamodel, cli_auto_del(); all("Delete whole candidate configuration"), delete_all("candidate"); @@ -83,14 +77,28 @@ module example { type string; } list index1{ - key i; - leaf i{ - type string; - } - leaf iv{ - description "a value"; - type string; - } + key i; + leaf i{ + type string; + } + leaf iv{ + description "a value"; + type string; + } + /* See https://github.com/clicon/clixon-controller/issues/26 + * reference that goes beyond the scope of this grouping + */ + leaf-list scope { + type leafref { + path "../../value0"; + } + } +/* leaf-list inscope { + type leafref { + path "../iv"; + } + } +*/ } } container table{ @@ -145,65 +153,121 @@ module example-external3 { } EOF -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 +# Args: +# 1: grouping_treeref +function testrun() +{ + # Whether grouping treeref is enabled + grouping_treeref=$1 + echo "grouping_treeref=$1" + # cat < $cfd/autocli.xml # XXX + cat < $cfg + + $cfg + $cfd + ietf-netconf:startup + ${YANG_INSTALLDIR} + $dir + $dir + /usr/local/lib/$APPNAME/backend + $dir + /usr/local/lib/$APPNAME/cli + $APPNAME + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + $dir + false + + false + kw-nokey + ${grouping_treeref} + + include ${APPNAME} + enable + ${APPNAME}* + + + +EOF + + 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 "start backend -s init -f $cfg" - start_backend -s init -f $cfg -fi -new "wait backend" -wait_backend + new "wait backend" + wait_backend -# Test of generated clispecs. With autocli grouping_treeref, there should be treerefs for -# groupings. Without they should not be present -# The testcase assumes enabled -if ${grouping_treeref}; then - new "verify grouping is enabled" - expectpart "$($clixon_cli -f $cfg -G -1 2>&1)" 0 "@grouping-urn:example:clixon-pg1" "@grouping-urn:example:external-pg2" "@grouping-urn:example:external-pg3" -else - new "verify grouping is disabled" - expectpart "$($clixon_cli -f $cfg -G -1 2>&1)" 0 --not-- "@grouping-urn:example:clixon-pg1" "@grouping-urn:example:external-pg2" "@grouping-urn:example:external-pg3" -fi - -new "set top-level grouping" -expectpart "$($clixon_cli -f $cfg -1 set value1 39)" 0 "" - -new "set inline grouping value" -expectpart "$($clixon_cli -f $cfg -1 set table parameter x value0 40)" 0 "" - -new "set grouping in same module" -expectpart "$($clixon_cli -f $cfg -1 set table parameter x value1 41)" 0 "" - -new "set list grouping in same module" -expectpart "$($clixon_cli -f $cfg -1 set table parameter x index1 a iv foo)" 0 "" - -new "set grouping in other module" -expectpart "$($clixon_cli -f $cfg -1 set table parameter x value2 42)" 0 "" - -new "set grouping in other+other module" -expectpart "$($clixon_cli -f $cfg -1 set table parameter x c2 value3 43)" 0 "" - -new "commit" -expectpart "$($clixon_cli -f $cfg -1 commit)" 0 "" - -new "show config" -expectpart "$($clixon_cli -f $cfg -1 show config)" 0 "x4041afoo4243
" "39" - -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" + # Test of generated clispecs. With autocli grouping_treeref, there should be treerefs for + # groupings. Without they should not be present + # The testcase assumes enabled + if ${grouping_treeref}; then + new "verify grouping is enabled" + expectpart "$($clixon_cli -f $cfg -G -1 2>&1)" 0 "@grouping-urn:example:clixon-pg1" "@grouping-urn:example:external-pg2" "@grouping-urn:example:external-pg3" + else + new "verify grouping is disabled" + expectpart "$($clixon_cli -f $cfg -G -1 2>&1)" 0 --not-- "@grouping-urn:example:clixon-pg1" "@grouping-urn:example:external-pg2" "@grouping-urn:example:external-pg3" fi - # kill backend - stop_backend -f $cfg -fi + + new "set top-level grouping" + expectpart "$($clixon_cli -f $cfg -1 set value1 39)" 0 "" + + new "set inline grouping value" + expectpart "$($clixon_cli -f $cfg -1 set table parameter x value0 40)" 0 "" + + new "set grouping in same module" + expectpart "$($clixon_cli -f $cfg -1 set table parameter x value1 41)" 0 "" + + new "set list grouping in same module" + expectpart "$($clixon_cli -f $cfg -1 set table parameter x index1 a iv foo)" 0 "" + + new "set grouping in other module" + expectpart "$($clixon_cli -f $cfg -1 set table parameter x value2 42)" 0 "" + + new "set grouping in other+other module" + expectpart "$($clixon_cli -f $cfg -1 set table parameter x c2 value3 43)" 0 "" + + new "commit" + expectpart "$($clixon_cli -f $cfg -1 commit)" 0 "" + + new "show config" + expectpart "$($clixon_cli -f $cfg -1 show config)" 0 "x4041afoo4243
" "39" + + new "set leafref lack origin" + expectpart "$($clixon_cli -f $cfg -1 set table parameter x index1 a scope 43)" 0 "" + + new "validate expect fail" + expectpart "$($clixon_cli -f $cfg -1 validate 2>&1)" 255 "bad-element Leafref validation failed: No leaf 43 matching path" + + new "set leafref expect fail" + expectpart "$($clixon_cli -f $cfg -1 set table parameter x value0 43)" 0 "" + + new "validate ok" + expectpart "$($clixon_cli -f $cfg -1 validate)" 0 "^$" --not-- "bad-element Leafref validation failed: No leaf 43 matching path" + + 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 "autocli grouping=true" +testrun true + +new "autocli grouping=false" +testrun false rm -rf $dir diff --git a/test/test_netconf_monitoring.sh b/test/test_netconf_monitoring.sh index 63b64a60..58919f4c 100755 --- a/test/test_netconf_monitoring.sh +++ b/test/test_netconf_monitoring.sh @@ -147,6 +147,7 @@ fi # for some reason valgrind tests fail below? if [ ${valgrindtest} -eq 0 ]; then # Error dont cleanup mem OK +# XXX there may be inconsistent YANGs in this dir YANGDIR=$YANG_INSTALLDIR if [ $BE -ne 0 ]; then @@ -157,7 +158,6 @@ new "wait backend" wait_backend new "Loop over all yangs in $YANGDIR" - for f in ${YANGDIR}/*.yang; do b=$(basename $f) id=$(echo "$b" | sed 's/.yang//' | sed 's/@.*//')