diff --git a/CHANGELOG.md b/CHANGELOG.md
index c3d768ad..d27b9226 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -51,6 +51,8 @@ Users may have to change how they access the system
### Corrected Bugs
+* Fixed: [OpenConfig BGP afi-safi and when condition issues #249](https://github.com/clicon/clixon/issues/249)
+ * YANG when was not properly implemented for default values
* Fixed: SEGV in clixon_netconf_lib functions from internal errors including validation.
* Check xerr argument both before and after call on netconf lib functions
* Fixed: RFC 8040 yang-data extension allows non-key lists
diff --git a/lib/clixon/clixon_xml_map.h b/lib/clixon/clixon_xml_map.h
index 8f4a288f..feba656e 100644
--- a/lib/clixon/clixon_xml_map.h
+++ b/lib/clixon/clixon_xml_map.h
@@ -76,5 +76,6 @@ int assign_namespace_body(cxobj *x0, cxobj *x1);
int xml_merge(cxobj *x0, cxobj *x1, yang_stmt *yspec, char **reason);
int yang_enum_int_value(cxobj *node, int32_t *val);
int xml_copy_marked(cxobj *x0, cxobj *x1);
+int yang_when_xpath(cxobj *xn, cxobj *xp, yang_stmt *yn, int *hit, int *nrp, char **xpathp);
#endif /* _CLIXON_XML_MAP_H_ */
diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c
index 802330c8..48070400 100644
--- a/lib/src/clixon_validate.c
+++ b/lib/src/clixon_validate.c
@@ -114,6 +114,7 @@ validate_leafref(cxobj *xt,
cvec *nsc = NULL;
cbuf *cberr = NULL;
char *path;
+ yang_stmt *ymod;
if ((leafrefbody = xml_body(xt)) == NULL)
goto ok;
@@ -140,7 +141,8 @@ validate_leafref(cxobj *xt,
clicon_err(OE_UNIX, errno, "cbuf_new");
goto done;
}
- cprintf(cberr, "Leafref validation failed: No leaf %s matching path %s", leafrefbody, path);
+ ymod = ys_module(ys);
+ cprintf(cberr, "Leafref validation failed: No leaf %s matching path %s in module %s", leafrefbody, path, yang_argument_get(ymod));
if (xret && netconf_bad_element_xml(xret, "application", leafrefbody, cbuf_get(cberr)) < 0)
goto done;
goto fail;
@@ -1136,6 +1138,7 @@ xml_yang_validate_all(clicon_handle h,
char *ns = NULL;
cbuf *cb = NULL;
cvec *nsc = NULL;
+ int hit = 0;
/* if not given by argument (overide) use default link
and !Node has a config sub-statement and it is false */
@@ -1227,53 +1230,21 @@ xml_yang_validate_all(clicon_handle h,
nsc = NULL;
}
}
- /* First variant of when, actual "when" sub-node RFC 7950 Sec 7.21.5. Can only be one. */
- if ((yc = yang_find(ys, Y_WHEN, NULL)) != NULL){
- xpath = yang_argument_get(yc); /* "when" has xpath argument */
- /* WHEN xpath needs namespace context */
- if (xml_nsctx_yang(ys, &nsc) < 0)
+ if (yang_when_xpath(xt, xml_parent(xt), ys, &hit, &nr, &xpath) < 0)
+ goto done;
+ if (hit && nr == 0){
+ if ((cb = cbuf_new()) == NULL){
+ clicon_err(OE_UNIX, errno, "cbuf_new");
goto done;
- if ((nr = xpath_vec_bool(xt, nsc, "%s", xpath)) < 0)
+ }
+ cprintf(cb, "Failed WHEN condition of %s in module %s (WHEN xpath is %s)",
+ xml_name(xt),
+ yang_argument_get(ys_module(ys)),
+ xpath);
+ if (xret && netconf_operation_failed_xml(xret, "application",
+ cbuf_get(cb)) < 0)
goto done;
- if (nsc){
- xml_nsctx_free(nsc);
- nsc = NULL;
- }
- if (nr == 0){
- if ((cb = cbuf_new()) == NULL){
- clicon_err(OE_UNIX, errno, "cbuf_new");
- goto done;
- }
- cprintf(cb, "Failed WHEN condition of %s in module %s",
- xml_name(xt),
- yang_argument_get(ys_module(ys)));
- if (xret && netconf_operation_failed_xml(xret, "application",
- cbuf_get(cb)) < 0)
- goto done;
- goto fail;
- }
- }
- /* Second variants of WHEN:
- * Augmented and uses when using special info in node
- */
- if ((xpath = yang_when_xpath_get(ys)) != NULL){
- if ((nr = xpath_vec_bool(xml_parent(xt), yang_when_nsc_get(ys),
- "%s", xpath)) < 0)
- goto done;
- if (nr == 0){
- if ((cb = cbuf_new()) == NULL){
- clicon_err(OE_UNIX, errno, "cbuf_new");
- goto done;
- }
- cprintf(cb, "Failed augmented 'when' condition '%s' of node '%s' in module '%s'",
- xpath,
- xml_name(xt),
- yang_argument_get(ys_module(ys)));
- if (xret && netconf_operation_failed_xml(xret, "application",
- cbuf_get(cb)) < 0)
- goto done;
- goto fail;
- }
+ goto fail;
}
}
x = NULL;
diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c
index 0b933992..5c77c22d 100644
--- a/lib/src/clixon_xml_map.c
+++ b/lib/src/clixon_xml_map.c
@@ -1139,10 +1139,11 @@ xml_default1(yang_stmt *yt,
int retval = -1;
yang_stmt *yc;
cxobj *xc;
- int top=0; /* Top symbol (set default namespace) */
+ int top = 0; /* Top symbol (set default namespace) */
int create = 0;
char *xpath;
- int nr;
+ int nr = 0;
+ int hit = 0;
if (xt == NULL){ /* No xml */
clicon_err(OE_XML, EINVAL, "No XML argument");
@@ -1181,12 +1182,10 @@ xml_default1(yang_stmt *yt,
case Y_CONTAINER:
if (yang_find(yc, Y_PRESENCE, NULL) == NULL){
/* Check when statement from uses or augment */
- if ((xpath = yang_when_xpath_get(yc)) != NULL){
- if ((nr = xpath_vec_bool(xt, yang_when_nsc_get(yc), "%s", xpath)) < 0)
- goto done;
- if (nr == 0)
- break; /* Do not create default if xpath fails */
- }
+ if (yang_when_xpath(NULL, xt, yc, &hit, &nr, &xpath) < 0)
+ goto done;
+ if (hit && nr == 0)
+ break; /* Do not create default if xpath fails */
/* If this is non-presence, (and it does not exist in xt) call
* recursively and create nodes if any default value exist first.
* Then continue and populate?
@@ -2257,3 +2256,70 @@ xml_copy_marked(cxobj *x0,
return retval;
}
+/*! Check when condition
+ *
+ * @param[in] h Clixon handle
+ * @param[in] xn XML node, can be NULL, in which case it is added as dummy under xp
+ * @param[in] xp XML parent
+ * @param[in] ys Yang node
+ * First variants of WHEN: Augmented and uses when using special info in node
+ * Second variant of when, actual "when" sub-node RFC 7950 Sec 7.21.5. Can only be one.
+ */
+int
+yang_when_xpath(cxobj *xn,
+ cxobj *xp,
+ yang_stmt *yn,
+ int *hit,
+ int *nrp,
+ char **xpathp)
+{
+ int retval = 1;
+ yang_stmt *yc;
+ char *xpath = NULL;
+ cxobj *x = NULL;
+ int nr = 0;
+ cvec *nsc = NULL;
+ int xmalloc = 0; /* ugly help variable to clean temporary object */
+ int nscmalloc = 0; /* ugly help variable to remove */
+
+ /* First variant */
+ if ((xpath = yang_when_xpath_get(yn)) != NULL){
+ x = xp;
+ nsc = yang_when_nsc_get(yn);
+ *hit = 1;
+ }
+ /* Second variant */
+ else if ((yc = yang_find(yn, Y_WHEN, NULL)) != NULL){
+ xpath = yang_argument_get(yc); /* "when" has xpath argument */
+ /* Create dummy */
+ if (xn == NULL){
+ if ((x = xml_new(yang_argument_get(yn), xp, CX_ELMNT)) == NULL)
+ goto done;
+ xml_spec_set(x, yn);
+ xmalloc++;
+ }
+ else
+ x = xn;
+ if (xml_nsctx_yang(yn, &nsc) < 0)
+ goto done;
+ nscmalloc++;
+ *hit = 1;
+ }
+ else
+ *hit = 0;
+ if (x && xpath){
+ if ((nr = xpath_vec_bool(x, nsc, "%s", xpath)) < 0)
+ goto done;
+ }
+ if (nrp)
+ *nrp = nr;
+ if (xpathp)
+ *xpathp = xpath;
+ retval = 0;
+ done:
+ if (xmalloc)
+ xml_purge(x);
+ if (nsc && nscmalloc)
+ xml_nsctx_free(nsc);
+ return retval;
+}
diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c
index 7c1af5a1..c90fe4ae 100644
--- a/lib/src/clixon_yang.c
+++ b/lib/src/clixon_yang.c
@@ -340,11 +340,13 @@ yang_flag_reset(yang_stmt *ys,
* @param[in] ys Yang statement
* @retval xpath xpath should evaluate to true at validation
* @retval NULL Not set
+ * Note xpath context is PARENT which is different from when actual when child which is
+ * child itself
*/
char*
yang_when_xpath_get(yang_stmt *ys)
{
- return ys->ys_when_xpath;
+ return ys->ys_when_xpath;
}
/*! Set yang xpath and namespace context for "when"-associated augment
diff --git a/test/test_leafref.sh b/test/test_leafref.sh
index a7a414c3..b425f61e 100755
--- a/test/test_leafref.sh
+++ b/test/test_leafref.sh
@@ -140,7 +140,7 @@ new "leafref add non-existing ref"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLOeth310.0.4.6]]>]]>" "^]]>]]>$"
new "leafref validate"
-expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationbad-elementeth3errorLeafref validation failed: No leaf eth3 matching path /if:interfaces/if:interface/if:name]]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationbad-elementeth3errorLeafref validation failed: No leaf eth3 matching path /if:interfaces/if:interface/if:name in module example]]>]]>$"
#new "leafref wrong ref"
#expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLOeth310.0.4.6]]>]]>" "^]]>]]>$"
@@ -170,7 +170,7 @@ new "leafref delete leaf"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLOeth0]]>]]>" "^"
new "leafref validate (should fail)"
-expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationbad-elementeth0errorLeafref validation failed: No leaf eth0 matching path /if:interfaces/if:interface/if:name]]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationbad-elementeth0errorLeafref validation failed: No leaf eth0 matching path /if:interfaces/if:interface/if:name in module example]]>]]>$"
new "leafref discard-changes"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$"
diff --git a/test/test_leafref_augment.sh b/test/test_leafref_augment.sh
index 4c4c046a..c6472be9 100755
--- a/test/test_leafref_augment.sh
+++ b/test/test_leafref_augment.sh
@@ -209,7 +209,7 @@ new "leafref augment+leafref config wrong ref"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO$XML]]>]]>" "^]]>]]>$"
new "leafref augment+leafref validate wrong ref"
-expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationbad-elementxxxerrorLeafref validation failed: No leaf xxx matching path /ex:sender/ex:name]]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationbad-elementxxxerrorLeafref validation failed: No leaf xxx matching path /ex:sender/ex:name in module augment]]>]]>$"
new "netconf discard-changes"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$"
diff --git a/test/test_leafref_state.sh b/test/test_leafref_state.sh
index 9ed123ad..feb4032e 100755
--- a/test/test_leafref_state.sh
+++ b/test/test_leafref_state.sh
@@ -168,10 +168,10 @@ expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failedxerrorLeafref validation failed: No leaf x matching path /ex:sender-config/ex:name. Internal error, state callback returned invalid XML]]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failedxerrorLeafref validation failed: No leaf x matching path /ex:sender-config/ex:name in module leafref. Internal error, state callback returned invalid XML]]>]]>$"
new "netconf get / state-only should fail"
-expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failedxerrorLeafref validation failed: No leaf x matching path /ex:sender-config/ex:name. Internal error, state callback returned invalid XML]]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failedxerrorLeafref validation failed: No leaf x matching path /ex:sender-config/ex:name in module leafref. Internal error, state callback returned invalid XML]]>]]>$"
new "netconf get / config-only ok"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^y]]>]]>$"
diff --git a/test/test_when_must.sh b/test/test_when_must.sh
index 12c94da3..83bc9a6a 100755
--- a/test/test_when_must.sh
+++ b/test/test_when_must.sh
@@ -117,7 +117,7 @@ new "when get config"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^directr2staticr1]]>]]>$"
new "when: validate fail"
-expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed WHEN condition of static-routes in module example]]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed WHEN condition of static-routes in module example (WHEN xpath is ../type='static')]]>]]>$"
new "when: discard-changes"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$"
diff --git a/test/test_xpath_functions.sh b/test/test_xpath_functions.sh
index f996ad1a..3b2cfbee 100755
--- a/test/test_xpath_functions.sh
+++ b/test/test_xpath_functions.sh
@@ -124,7 +124,7 @@ new "Set site to fie which invalidates the when contains"
expecteof "$clixon_netconf -qf $cfg -D $DBG" 0 "$DEFAULTHELLOfie]]>]]>" "^]]>]]>"
new "netconf validate not OK"
-expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed WHEN condition of site in module example]]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed WHEN condition of site in module example (WHEN xpath is contains(../../class,'foo') or contains(../../class,'bar'))]]>]]>$"
new "netconf discard-changes"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$"
@@ -140,13 +140,13 @@ new "Change type to atm"
expecteof "$clixon_netconf -qf $cfg -D $DBG" 0 "$DEFAULTHELLOe0atm]]>]]>" "^]]>]]>"
new "netconf validate not OK (mtu not allowed)"
-expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed augmented 'when' condition 'derived-from(type, \"ex:ethernet\")' of node 'mtu' in module 'example']]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed WHEN condition of mtu in module example (WHEN xpath is derived-from(type, \"ex:ethernet\"))]]>]]>$"
new "Change type to ethernet (self)"
expecteof "$clixon_netconf -qf $cfg -D $DBG" 0 "$DEFAULTHELLOe0ethernet]]>]]>" "^]]>]]>"
new "netconf validate not OK (mtu not allowed on self)"
-expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed augmented 'when' condition 'derived-from(type, \"ex:ethernet\")' of node 'mtu' in module 'example']]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed WHEN condition of mtu in module example (WHEN xpath is derived-from(type, \"ex:ethernet\"))]]>]]>$"
new "netconf discard-changes"
expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^]]>]]>$"
@@ -162,7 +162,7 @@ new "Change type to atm"
expecteof "$clixon_netconf -qf $cfg -D $DBG" 0 "$DEFAULTHELLOe0atm]]>]]>" "^]]>]]>"
new "netconf validate not OK (crc not allowed)"
-expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed augmented 'when' condition 'derived-from-or-self(type, \"ex:ethernet\")' of node 'crc' in module 'example']]>]]>$"
+expecteof "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO]]>]]>" "^applicationoperation-failederrorFailed WHEN condition of crc in module example (WHEN xpath is derived-from-or-self(type, \"ex:ethernet\"))]]>]]>$"
new "Change type to ethernet (self)"
expecteof "$clixon_netconf -qf $cfg -D $DBG" 0 "$DEFAULTHELLOe0ethernet]]>]]>" "^]]>]]>"