diff --git a/CHANGELOG.md b/CHANGELOG.md index b71952cc..bae8d056 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,8 @@ Developers may need to change their code ### Corrected Bugs +* [type leafref in type union ineffective](https://github.com/clicon/clixon/issues/277) + * Leafrefs and identityrefs in unions were not validated correctly * [cl:autocli-op hide has no effect in yang submodule](https://github.com/clicon/clixon/issues/282) * [Doxygen - Typo in Input #275](https://github.com/clicon/clixon/issues/275) diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c index b7a7bd3c..743b213e 100644 --- a/lib/src/clixon_validate.c +++ b/lib/src/clixon_validate.c @@ -1151,6 +1151,59 @@ xml_yang_validate_list_key_only(cxobj *xt, goto done; } +static int +xml_yang_validate_leaf_union(clicon_handle h, + cxobj *xt, + yang_stmt *yt, + yang_stmt *yrestype, + cxobj **xret) +{ + int retval = -1; + int ret; + yang_stmt *yc = NULL; + cxobj *xret1 = NULL; + + /* Enough that one is valid, eg returns 1,otherwise fail */ + while ((yc = yn_each(yrestype, yc)) != NULL){ + if (yang_keyword_get(yc) != Y_TYPE) + continue; + ret = 1; /* If not leafref/identityref it is valid on this level */ + if (strcmp(yang_argument_get(yc), "leafref") == 0){ + if ((ret = validate_leafref(xt, yt, yc, &xret1)) < 0) + goto done; + } + else if (strcmp(yang_argument_get(yc), "identityref") == 0){ + if ((ret = validate_identityref(xt, yt, yc, &xret1)) < 0) + goto done; + } + else if (strcmp("union", yang_argument_get(yc)) == 0){ + if ((ret = xml_yang_validate_leaf_union(h, xt, yt, yc, &xret1)) < 0) + goto done; + } + if (ret == 1) + break; + if (ret == 0 && xret1 != NULL) { + /* If validation failed, save reason, reset error and continue, + * save latest reason if nothing validates. + */ + if (*xret) + xml_free(*xret); + *xret = xret1; + xret1 = NULL; + } + } + if (yc == NULL) + goto fail; + retval = 1; + done: + if (xret1) + xml_free(xret1); + return retval; + fail: + retval = 0; + goto done; +} + /*! Validate a single XML node with yang specification for all (not only added) entries * 1. Check leafrefs. Eg you delete a leaf and a leafref references it. * @param[in] xt XML node to be validated @@ -1177,7 +1230,7 @@ xml_yang_validate_all(clicon_handle h, cxobj **xret) { int retval = -1; - yang_stmt *ys; /* yang node */ + yang_stmt *yt; /* yang node associated with xt */ yang_stmt *yc; /* yang child */ yang_stmt *ye; /* yang must error-message */ char *xpath; @@ -1192,8 +1245,7 @@ xml_yang_validate_all(clicon_handle h, /* if not given by argument (overide) use default link and !Node has a config sub-statement and it is false */ - ys=xml_spec(xt); - if (ys==NULL){ + if ((yt = xml_spec(xt)) == NULL){ if (clicon_option_bool(h, "CLICON_YANG_UNKNOWN_ANYDATA") == 1) { clicon_log(LOG_WARNING, "%s: %d: No YANG spec for %s, validation skipped", @@ -1215,9 +1267,9 @@ xml_yang_validate_all(clicon_handle h, goto done; goto fail; } - if (yang_config(ys) != 0){ + if (yang_config(yt) != 0){ /* Node-specific validation */ - switch (yang_keyword_get(ys)){ + switch (yang_keyword_get(yt)){ case Y_ANYXML: case Y_ANYDATA: goto ok; @@ -1229,16 +1281,22 @@ xml_yang_validate_all(clicon_handle h, current xml tree */ /* Get base type yc */ - if (yang_type_get(ys, NULL, &yc, NULL, NULL, NULL, NULL, NULL) < 0) + if (yang_type_get(yt, NULL, &yc, NULL, NULL, NULL, NULL, NULL) < 0) goto done; if (strcmp(yang_argument_get(yc), "leafref") == 0){ - if ((ret = validate_leafref(xt, ys, yc, xret)) < 0) + if ((ret = validate_leafref(xt, yt, yc, xret)) < 0) goto done; if (ret == 0) goto fail; } else if (strcmp(yang_argument_get(yc), "identityref") == 0){ - if ((ret = validate_identityref(xt, ys, yc, xret)) < 0) + if ((ret = validate_identityref(xt, yt, yc, xret)) < 0) + goto done; + if (ret == 0) + goto fail; + } + else if (strcmp("union", yang_argument_get(yc)) == 0){ + if ((ret = xml_yang_validate_leaf_union(h, xt, yt, yc, xret)) < 0) goto done; if (ret == 0) goto fail; @@ -1250,7 +1308,7 @@ xml_yang_validate_all(clicon_handle h, /* must sub-node RFC 7950 Sec 7.5.3. Can be several. * XXX. use yang path instead? */ yc = NULL; - while ((yc = yn_each(ys, yc)) != NULL) { + while ((yc = yn_each(yt, yc)) != NULL) { if (yang_keyword_get(yc) != Y_MUST) continue; xpath = yang_argument_get(yc); /* "must" has xpath argument */ @@ -1269,7 +1327,7 @@ xml_yang_validate_all(clicon_handle h, goto done; } cprintf(cb, "Failed MUST xpath '%s' of '%s' in module %s", - xpath, xml_name(xt), yang_argument_get(ys_module(ys))); + xpath, xml_name(xt), yang_argument_get(ys_module(yt))); if (xret && netconf_operation_failed_xml(xret, "application", ye?yang_argument_get(ye):cbuf_get(cb)) < 0) goto done; @@ -1280,7 +1338,7 @@ xml_yang_validate_all(clicon_handle h, nsc = NULL; } } - if (yang_check_when_xpath(xt, xml_parent(xt), ys, &hit, &nr, &xpath) < 0) + if (yang_check_when_xpath(xt, xml_parent(xt), yt, &hit, &nr, &xpath) < 0) goto done; if (hit && nr == 0){ if ((cb = cbuf_new()) == NULL){ @@ -1289,7 +1347,7 @@ xml_yang_validate_all(clicon_handle h, } cprintf(cb, "Failed WHEN condition of %s in module %s (WHEN xpath is %s)", xml_name(xt), - yang_argument_get(ys_module(ys)), + yang_argument_get(ys_module(yt)), xpath); if (xret && netconf_operation_failed_xml(xret, "application", cbuf_get(cb)) < 0) @@ -1305,7 +1363,7 @@ xml_yang_validate_all(clicon_handle h, goto fail; } /* Check unique and min-max after choice test for example*/ - if (yang_config(ys) != 0){ + if (yang_config(yt) != 0){ /* Checks if next level contains any unique list constraints */ if ((ret = check_list_unique_minmax(xt, xret)) < 0) goto done; diff --git a/test/test_leafref_union.sh b/test/test_leafref_union.sh new file mode 100755 index 00000000..94371afc --- /dev/null +++ b/test/test_leafref_union.sh @@ -0,0 +1,186 @@ +#!/usr/bin/env bash +# Yang leafref + union test (+ identitytref) +# See eg https://github.com/clicon/clixon/issues/277 + +# 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/leafref.yang + +cat < $cfg + + $cfg + $dir + /usr/local/share/clixon + $IETFRFC + $fyang + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/cli + $APPNAME + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + /usr/local/var/$APPNAME + +EOF + +cat < $fyang +module example{ + yang-version 1.1; + namespace "urn:example:clixon"; + prefix ex; + identity crypto { + description + "Base identity from which all crypto algorithms + are derived."; + } + identity des { + base "crypto"; + description "DES crypto algorithm."; + } + identity des3 { + base "crypto"; + description "Triple DES crypto algorithm."; + } + identity airplane; + identity boeing { + base "airplane"; + } + identity saab { + base "airplane"; + } + container c { + leaf x { + type string; + } + leaf y { + type string; + } + leaf z { + description "leafref union"; + type union { + type leafref { + path "../x"; + require-instance true; + } + type leafref { + path "../y"; + require-instance true; + } + } + } + leaf w { + description "idref union"; + type union { + type identityref { + base crypto; + } + type identityref { + base airplane; + } + } + } + leaf u { + description "Union mix of idref and leafref"; + type union { + type identityref { + base crypto; + } + type leafref { + path "../x"; + require-instance true; + } + } + } + } +} +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" + start_backend -s init -f $cfg +fi + +new "wait backend" +wait_backend + +new "netconf set x,y leafs" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO4299merge ]]>]]>" "^]]>]]>$" + +new "netconf commit x,y" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +new "netconf set union 42" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO42none]]>]]>" "^]]>]]>$" + +new "netconf validate ok" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +new "netconf check config" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^429942]]>]]>$" + +new "netconf set union 66" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO66none]]>]]>" "^]]>]]>$" + +new "netconf validate not ok" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationbad-element66errorLeafref validation failed: No leaf 66 matching path ../y" + +new "netconf discard-changes" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +new "netconf set union id saab" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLOex:saabnone]]>]]>" "^]]>]]>$" +new "netconf validate ok" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +new "netconf set union idref zzz" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLOex:zzznone]]>]]>" "^]]>]]>$" + +new "netconf validate idref not ok" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorIdentityref validation failed" + +new "netconf discard-changes" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +new "netconf set union id des" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLOex:desnone]]>]]>" "^]]>]]>$" + +new "netconf validate ok" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +new "netconf set union x=42" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO42none]]>]]>" "^]]>]]>$" + +new "netconf validate ok" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$" + +new "netconf set union 99" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO99none]]>]]>" "^]]>]]>$" + +new "netconf validate not ok" +expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationbad-element99errorLeafref validation failed" + +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 + +rm -rf $dir + +new "endtest" +endtest