Validation of mandatory choice and recursive mandatory containers.

This commit is contained in:
Olof hagsand 2019-01-05 11:08:16 +01:00
parent 058a14579f
commit 5b6af82e29
14 changed files with 186 additions and 94 deletions

View file

@ -3,7 +3,7 @@
## 3.9.0 (Preliminary Target: Mid-January 2019)
### Planned new features
* [Roadmap](ROADMAP.md) (Uncommitted and unprioritized)
* [Roadmap](ROADMAP.md)
### Major New features
* Correct XML namespace handling
@ -56,11 +56,6 @@
```
* To keep previous non-strict namespace handling (backwards compatible), set CLICON_XML_NS_STRICT to false.
* See https://github.com/clicon/clixon/issues/49
* NACM extension (RFC8341)
* NACM module support (RFC8341 A1+A2)
* Recovery user "_nacm_recovery" added.
* Example use is restconf PUT when NACM edit-config is permitted, then automatic commit and discard are permitted using recovery user.
* Example user changed adm1 to andy to comply with RFC8341 example
* Yang code upgrade (RFC7950)
* YANG parser cardinality checked (https://github.com/clicon/clixon/issues/48)
* See https://github.com/clicon/clixon/issues/84
@ -70,13 +65,18 @@
* Openconfig yang specs parsed: https://github.com/openconfig/public
* Improved "unknown" handling
* More precise Yang validation and better error messages
* Example: adding bad-, missing-, or unknown-element error messages, etc instead of operation-failed, bad-element instead of "yang node not found", etc.
*
* Example: adding bad-, missing-, or unknown-element error messages, instead of operation-failed.
* Validation of mandatory choice and recursive mandatory containers
* Yang load file configure options changed
* `CLICON_YANG_DIR` is changed from a single directory to a path of directories
* Note CLIXON_DATADIR (=/usr/local/share/clixon) need to be in the list
* CLICON_YANG_MAIN_FILE Provides a filename with a single module filename.
* CLICON_YANG_MAIN_DIR Provides a directory where all yang modules should be loaded.
* NACM extension (RFC8341)
* NACM module support (RFC8341 A1+A2)
* Recovery user "_nacm_recovery" added.
* Example use is restconf PUT when NACM edit-config is permitted, then automatic commit and discard are permitted using recovery user.
* Example user changed adm1 to andy to comply with RFC8341 example
### API changes on existing features (you may need to change your code)
* Stricter YANG choice validation leads to enforcement of structures like: `choice c{ mandatory true; leaf x` statements. `x` was not previously enforced.
@ -95,7 +95,6 @@
* For backward compatibility, define CLICON_CLI_MODEL_TREENAME_PATCH in clixon_custom.h
### Minor changes
* Yang choice functionality improved and stricter validation for CLI generation, mandatory flags, etc.
* Added new clixon-lib yang module for internal netconf protocol. Currently only extends the standard with a debug RPC.
* Added three-valued return values for several validate functions where -1 is fatal error, 0 is validation failed and 1 is validation OK.
* This includes: `xmldb_put`, `xml_yang_validate_all`, `xml_yang_validate_add`, `xml_yang_validate_rpc`, `api_path2xml`, `api_path2xpath`

View file

@ -430,8 +430,9 @@ from_client_edit_config(clicon_handle h,
goto done;
}
if ((target = netconf_db_find(xn, "target")) == NULL){
clicon_err(OE_XML, 0, "db not found");
goto done;
if (netconf_missing_element(cbret, "protocol", "target", NULL) < 0)
goto done;
goto ok;
}
if ((cbx = cbuf_new()) == NULL){
clicon_err(OE_XML, errno, "cbuf_new");

View file

@ -117,8 +117,8 @@ generic_validate(yang_spec *yspec,
for (i=0; i<td->td_dlen; i++){
x1 = td->td_dvec[i];
ys = xml_spec(x1);
if (ys && yang_mandatory(ys)){
if (netconf_missing_element(cbret, "protocol", xml_name(x1), "Removed mandatory variable") < 0)
if (ys && yang_mandatory(ys) && yang_config(ys)==0){
if (netconf_missing_element(cbret, "protocol", xml_name(x1), "Missing mandatory variable") < 0)
goto done;
goto fail;
}

View file

@ -97,8 +97,8 @@ netconf_input_packet(clicon_handle h,
cxobj *xa;
cxobj *xa2;
clicon_debug(1, "RECV");
clicon_debug(2, "%s: RCV: \"%s\"", __FUNCTION__, cbuf_get(cb));
clicon_debug(1, "%s", __FUNCTION__);
clicon_debug(2, "%s: \"%s\"", __FUNCTION__, cbuf_get(cb));
if ((cbret = cbuf_new()) == NULL){
clicon_err(LOG_ERR, errno, "cbuf_new");
goto done;

View file

@ -2,10 +2,10 @@
* [Installation](#installation)
* [Streams](#streams)
* [Nchan Streams](#nchan-streams)
* [Nchan Streams](#nchan)
* [Debugging](#debugging)
## 1. Installation
## Installation
The examples are based on Nginx. Other reverse proxies should work but are not verified.
@ -76,7 +76,7 @@ Example of writing a new interfaces specification:
curl -sX PUT http://localhost/restconf/data -d '{"ietf-interfaces:interfaces":{"interface":{"name":"eth1","type":"ex:eth","enabled":true}}}'
```
## 2. Streams
## Streams
Clixon have two experimental restconf event stream implementations following
RFC8040 Section 6 using SSE. One native and one using Nginx
@ -125,7 +125,7 @@ You can also specify start and stop time. Start-time enables replay of existing
See (stream tests)[../test/test_streams.sh] for more examples.
## 3. Nchan
## Nchan
As an alternative streams implementation, Nginx/Nchan can be used.
Nginx uses pub/sub channels and can be configured in a variety of
@ -180,7 +180,7 @@ curl -H "Accept: text/event-stream" -H "Last-Event-ID: 1539961709:0" -s -X GET h
See (https://nchan.io/#eventsource) on more info on how to access an SSE sub endpoint.
## 4. Debugging
## Debugging
Start the restconf fastcgi program with debug flag:
```

View file

@ -96,7 +96,7 @@ fib_route_rpc(clicon_handle h,
/* User supplied variable in CLI command */
instance = cvec_find(cvv, "instance"); /* get a cligen variable from vector */
/* Create XML for fib-route netconf RPC */
if (xml_parse_va(&xtop, NULL, "<rpc xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\" username=\"%s\"><fib-route xmlns=\"urn:ietf:params:xml:ns:yang:ietf-routing\"><routing-instance-name>%s</routing-instance-name></fib-route></rpc>",
if (xml_parse_va(&xtop, NULL, "<rpc xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\" username=\"%s\"><fib-route xmlns=\"urn:ietf:params:xml:ns:yang:ietf-routing\"><routing-instance-name>%s</routing-instance-name><destination-address><address-family>ipv4</address-family></destination-address></fib-route></rpc>",
clicon_username_get(h),
cv_string_get(instance)) < 0)
goto done;

View file

@ -482,6 +482,89 @@ xml_yang_validate_rpc(cxobj *xrpc,
goto done;
}
/*! Check if an xml node lacks mandatory children
* @param[in] xt XML node to be validated
* @param[in] yt xt:s yang statement
* @param[out] cbret Error buffer (set w netconf error if retval == 0)
* @retval 1 Validation OK
* @retval 0 Validation failed (cbret set)
* @retval -1 Error
*/
static int
check_mandatory(cxobj *xt,
yang_stmt *yt,
cbuf *cbret)
{
int retval = -1;
int i;
cxobj *x;
yang_stmt *y;
yang_stmt *yc;
yang_node *yp;
for (i=0; i<yt->ys_len; i++){
yc = yt->ys_stmt[i];
if (!yang_mandatory(yc))
continue;
switch (yc->ys_keyword){
case Y_CONTAINER:
case Y_ANYDATA:
case Y_ANYXML:
case Y_LEAF:
if (yang_config(yc)==0)
break;
/* Find a child with the mandatory yang */
x = NULL;
while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) {
if ((y = xml_spec(x)) != NULL
&& y==yc)
break; /* got it */
}
if (x == NULL){
if (netconf_missing_element(cbret, "application", yc->ys_argument, "Mandatory variable") < 0)
goto done;
goto fail;
}
break;
case Y_CHOICE: /* More complex because of choice/case structure */
x = NULL;
while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) {
if ((y = xml_spec(x)) != NULL &&
(yp = yang_choice(y)) != NULL &&
yp == (yang_node*)yc){
break; /* leave loop with x set */
}
}
if (x == NULL){
/* @see RFC7950: 15.6 Error Message for Data That Violates
* a Mandatory "choice" Statement */
if (cprintf(cbret, "<rpc-reply><rpc-error>"
"<error-type>application</error-type>"
"<error-tag>data-missing</error-tag>"
"<error-app-tag>missing-choice</error-app-tag>"
#ifdef NYI
// "<error-path></error-path>"
#endif
"<error-info><missing-choice>%s</missing-choice></error-info>"
"<error-severity>error</error-severity>"
"</rpc-error></rpc-reply>",
yc->ys_argument) <0)
goto done;
goto fail;
}
break;
default:
break;
} /* switch */
}
retval = 1;
done:
return retval;
fail:
retval = 0;
goto done;
}
/*! Validate a single XML node with yang specification for added entry
* 1. Check if mandatory leafs present as subs.
* 2. Check leaf values, eg int ranges and string regexps.
@ -510,10 +593,6 @@ xml_yang_validate_add(cxobj *xt,
cg_var *cv = NULL;
char *reason = NULL;
yang_stmt *yt; /* yang spec of xt going in */
yang_stmt *yc; /* yang spec of yt child */
yang_stmt *yx; /* yang spec of xt children */
yang_node *yp; /* yang spec of parent of yang spec of xt children */
int i;
char *body;
int ret;
cxobj *x;
@ -521,47 +600,12 @@ xml_yang_validate_add(cxobj *xt,
/* if not given by argument (overide) use default link
and !Node has a config sub-statement and it is false */
if ((yt = xml_spec(xt)) != NULL && yang_config(yt) != 0){
if ((ret = check_mandatory(xt, yt, cbret)) < 0)
goto done;
if (ret == 0)
goto fail;
/* Check leaf values */
switch (yt->ys_keyword){
case Y_RPC:
case Y_INPUT:
case Y_LIST:
/* fall thru */
case Y_CONTAINER:
for (i=0; i<yt->ys_len; i++){
yc = yt->ys_stmt[i];
switch (yc->ys_keyword){
case Y_LEAF:
if (yang_config(yc)==0)
break;
if (yang_mandatory(yc) && xml_find(xt, yc->ys_argument)==NULL){
if (netconf_missing_element(cbret, "application", yc->ys_argument, "Mandatory variable") < 0)
goto done;
goto fail;
}
break;
case Y_CHOICE:
if (yang_mandatory(yc)){
/* If there is one child with this choice as parent */
x = NULL;
while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) {
if ((yx = xml_spec(x)) != NULL &&
(yp = yang_choice(yx)) != NULL &&
yp == (yang_node*)yc){
break; /* leave loop with x set */
}
}
if (x == NULL){ /* No hit */
if (netconf_missing_element(cbret, "application", yc->ys_argument, "Mandatory choice") < 0)
goto done;
goto fail;
}
}
break;
default:
break;
}
}
break;
case Y_LEAF:
/* fall thru */
case Y_LEAF_LIST:

View file

@ -2701,26 +2701,62 @@ ys_parse_sub(yang_stmt *ys,
* Note: one can cache this value in ys_cvec instead of functionally evaluating it.
* @retval 1 yang statement is leaf and it has a mandatory sub-stmt with value true
* @retval 0 The negation of conditions for return value 1.
* @see RFC7950 Sec 3:
* o mandatory node: A mandatory node is one of:
* 1) A leaf, choice, anydata, or anyxml node with a "mandatory"
* statement with the value "true".
* 2) A list or leaf-list node with a "min-elements" statement with a
* value greater than zero.
* 3) A container node without a "presence" statement and that has at
* least one mandatory node as a child.
*/
int
yang_mandatory(yang_stmt *ys)
{
yang_stmt *ym;
char *reason = NULL;
uint8_t min_elements; /* XXX change to 32 (need new cligen version) */
if (ys->ys_keyword != Y_LEAF && ys->ys_keyword != Y_CHOICE)
return 0;
if ((ym = yang_find((yang_node*)ys, Y_MANDATORY, NULL)) != NULL){
if (ym->ys_cv == NULL) /* shouldnt happen */
return 0;
return cv_bool_get(ym->ys_cv);
/* 1) A leaf, choice, anydata, or anyxml node with a "mandatory"
* statement with the value "true". */
if (ys->ys_keyword == Y_LEAF || ys->ys_keyword == Y_CHOICE ||
ys->ys_keyword == Y_ANYDATA || ys->ys_keyword == Y_ANYXML){
if ((ym = yang_find((yang_node*)ys, Y_MANDATORY, NULL)) != NULL){
if (ym->ys_cv != NULL) /* shouldnt happen */
return cv_bool_get(ym->ys_cv);
}
}
/* 2) A list or leaf-list node with a "min-elements" statement with a
* value greater than zero. */
else if (ys->ys_keyword == Y_LIST || ys->ys_keyword == Y_LEAF_LIST){
if ((ym = yang_find((yang_node*)ys, Y_MIN_ELEMENTS, NULL)) != NULL){
/* XXX change to 32 (need new cligen version) */
if (parse_uint8(ym->ys_argument, &min_elements, &reason) != 1){
clicon_err(OE_YANG, EINVAL, "%s", reason?reason:"parse_uint8");
return 0; /* XXX ignore error */
}
return min_elements > 0;
}
}
/* 3) A container node without a "presence" statement and that has at
* least one mandatory node as a child. */
else if (ys->ys_keyword == Y_CONTAINER &&
yang_find((yang_node*)ys, Y_PRESENCE, NULL) == NULL){
yang_stmt *yc;
int i;
for (i=0; i<ys->ys_len; i++){
yc = ys->ys_stmt[i];
if (yang_mandatory(yc))
return 1;
}
}
return 0;
}
/*! Return config state of this node
* config statement is default true.
* Note that a node with config=false may not have a sub
* statement where config=true. And this function does not check the sttaus of a parent.
* @note a node with config=false may not have a sub statement where
* config=true. And this function does not check the status of a parent.
* @retval 0 if node has a config sub-statement and it is false
* @retval 1 node has not config sub-statement or it is true
*/

View file

@ -179,7 +179,7 @@ new "netconf get mandatory empty"
expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 "<rpc><get-config><source><candidate/></source></get-config></rpc>]]>]]>" '^<rpc-reply><data><system xmlns="urn:example:config"><mandatory/></system></data></rpc-reply>]]>]]>$'
new "netconf validate mandatory"
expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>missing-element</error-tag><error-info><bad-element>name</bad-element></error-info><error-severity>error</error-severity><error-message>Mandatory choice</error-message></rpc-error></rpc-reply>]]>]]>$'
expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>data-missing</error-tag><error-app-tag>missing-choice</error-app-tag><error-info><missing-choice>name</missing-choice></error-info><error-severity>error</error-severity></rpc-error></rpc-reply>]]>]]>$'
new "netconf set mandatory udp"
expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 '<rpc><edit-config><target><candidate/></target><config><system xmlns="urn:example:config"><mandatory><udp/></mandatory></system></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"

View file

@ -137,8 +137,8 @@ EOF
new "test params: -f $cfg -y $fyang"
if [ $BE -ne 0 ]; then
new "kill old backend -zf $cfg -y $fyang"
sudo clixon_backend -zf $cfg -y $fyang
new "kill old backend -zf $cfg "
sudo clixon_backend -zf $cfg
if [ $? -ne 0 ]; then
err
fi

View file

@ -21,7 +21,7 @@
# automatically committed to running immediately after each successful
# edit.
# Which means that restconf -X DELETE /data translates to edit-config + commit
# WHICH IS allowed.
# which is allowed.
APPNAME=example
# include err() and new() functions and creates $dir
@ -65,7 +65,6 @@ module $APPNAME{
}
}
EOF
# The groups are slightly modified from RFC8341 A.1
# The rule-list is from A.2
RULES=$(cat <<EOF
@ -228,4 +227,4 @@ if [ $? -ne 0 ]; then
err "kill backend"
fi
#rm -rf $dir # XXX
rm -rf $dir

View file

@ -10,14 +10,12 @@ OPENCONFIG=public
OCDIR=$OPENCONFIG/release/models
# Clone openconfig dir if not there
if false; then
if [ ! -d public ]; then
git clone https://github.com/openconfig/public
else
(cd public;
#git pull
)
fi
#else
# (cd public;
# #git pull
# )
fi
# include err() and new() functions and creates $dir
@ -86,17 +84,33 @@ cat <<EOF > $cfg
EOF
files=$(find $OPENCONFIG -name "*.yang")
# Just cound nr of modules (exclude submodule)
let m=0; # Nr of modules
# Count nr of modules (exclude submodule) Assume "module" or "submodule"
# first word on first line
let ms=0; # Nr of modules
let ss=0; # Nr of smodules
for f in $files; do
if [ -n "$(head -1 $f|grep '^module')" ]; then
let m=0; # Nr of modules
let s=0; # Nr of modules
if [ -n "$(head -15 $f|grep '^[ ]*module')" ]; then
let m++;
let ms++;
elif [ -n "$(head -15 $f|grep '^[ ]*submodule')" ]; then
let s++;
let ss++;
else
echo "No module or submodule found $f"
exit
fi
if [ $m -eq 1 -a $s -eq 1 ]; then
echo "Double match $f"
exit
fi
done
echo "m:$ms s:$ss"
new "Openconfig test: $clixon_cli -1f $cfg -y $f show version ($m modules)"
for f in $files; do
if [ -n "$(head -1 $f|grep '^module')" ]; then
new "cli $f"
new "$clixon_cli -1f $cfg -y $f show version"
expectfn "$clixon_cli -1f $cfg -y $f show version" 0 "3."
fi

View file

@ -293,7 +293,7 @@ new2 "restconf rpc missing input"
expecteq "$(curl -s -X POST -d '{}' http://localhost/restconf/operations/ietf-routing:fib-route)" '{"ietf-restconf:errors" : {"error": {"error-type": "rpc","error-tag": "malformed-message","error-severity": "error","error-message": "restconf RPC does not have input statement"}}} '
new "restconf rpc using POST xml"
ret=$(curl -s -X POST -H "Accept: application/yang-data+xml" -d '{"ietf-routing:input":{"routing-instance-name":"ipv4"}}' http://localhost/restconf/operations/ietf-routing:fib-route)
ret=$(curl -s -X POST -H "Accept: application/yang-data+xml" -d '{"ietf-routing:input":{"routing-instance-name":"ipv4","destination-address":{"address-family":"ipv4"}}}' http://localhost/restconf/operations/ietf-routing:fib-route)
expect='<output xmlns="urn:ietf:params:xml:ns:yang:ietf-routing"><route><address-family>ipv4</address-family><next-hop><next-hop-list>2.3.4.5</next-hop-list></next-hop><source-protocol>static</source-protocol></route></output>'
match=`echo $ret | grep -EZo "$expect"`
if [ -z "$match" ]; then

View file

@ -129,14 +129,13 @@ new "netconf edit-config extra arg should fail"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><target><candidate/></target><extra/><config/></edit-config></rpc>]]>]]>' "^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>unknown-element</error-tag><error-info><bad-element>extra</bad-element></error-info><error-severity>error</error-severity></rpc-error></rpc-reply>]]>]]>$"
new "netconf edit-config empty target should fail"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><target/><config/></edit-config></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>missing-element</error-tag><error-info><bad-element>config-target</bad-element></error-info><error-severity>error</error-severity><error-message>Mandatory choice</error-message></rpc-error></rpc-reply>]]>]]>
$'
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><target/><config/></edit-config></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>data-missing</error-tag><error-app-tag>missing-choice</error-app-tag><error-info><missing-choice>config-target</missing-choice></error-info><error-severity>error</error-severity></rpc-error></rpc-reply>]]>]]>$'
new "netconf edit-config missing target should fail"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><config/></edit-config></rpc>]]>]]>' "^<rpc-reply><rpc-error><error-tag>missing-element</error-tag><error-type>protocol</error-type><error-severity>error</error-severity><error-info><bad-element>target</bad-element></error-info></rpc-error></rpc-reply>]]>]]>$"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><config/></edit-config></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>missing-element</error-tag><error-info><bad-element>target</bad-element></error-info><error-severity>error</error-severity><error-message>Mandatory variable</error-message></rpc-error></rpc-reply>]]>]]>$'
new "netconf edit-config missing config should fail"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><target><candidate/></target></edit-config></rpc>]]>]]>' '<rpc-reply><rpc-error><error-type>application</error-type><error-tag>missing-element</error-tag><error-info><bad-element>edit-content</bad-element></error-info><error-severity>error</error-severity><error-message>Mandatory choice</error-message></rpc-error></rpc-reply>]]>]]>$'
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><target><candidate/></target></edit-config></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>data-missing</error-tag><error-app-tag>missing-choice</error-app-tag><error-info><missing-choice>edit-content</missing-choice></error-info><error-severity>error</error-severity></rpc-error></rpc-reply>]]>]]>$'
new "Kill restconf daemon"
sudo pkill -u www-data -f "/www-data/clixon_restconf"