Skip to content

Commit

Permalink
* Fixed: [State XML validation error when CLICON_MODULE_LIBRARY_RFC78…
Browse files Browse the repository at this point in the history
…95=true and ietf-yang-libra$

  * Removed obsolete option: `CLICON_MODULE_LIBRARY_RFC7895'
  * Obsolete config options given in the confi file are considered an error
* Added section in CONTRIBUTING relating to optimzation
* Changed reset merge to implicit default values.
  * This avoids a potential overwriting of explicitly set default values in the existing config
* Adapted some code to [Make cligen_* functions const ](clicon/cligen#83)
* Test: fixed test for * Fixed: [datamodel tree generated from basemodel tree is not proper when a list has more than one key and key is of enum type](#417)
  • Loading branch information
olofhagsand committed Feb 12, 2023
1 parent 7868cf0 commit d358387
Show file tree
Hide file tree
Showing 13 changed files with 77 additions and 59 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,12 @@ Expected: beginning of 2023

Users may have to change how they access the system

* New `[email protected]` revision
* Removed obsolete option: `CLICON_MODULE_LIBRARY_RFC7895'
* Obsolete config options given in the configuration file are considered an error
* clixon-lib,yang
* Moved all extended internal NETCONF attributes to the clicon-lib namespace
* These are: content, depth, username, autocommit, copystartup, transport, source-host, objectcreate, objectexisted.
* The internal attributes are documented in https://clixon-docs.readthedocs.io/en/latest/netconf.html
* With-defaults default retrieval mode has changed from `REPORT-ALL` to `EXPLICIT`
* This means that all get operations without `with-defaults` parameter do no longer
Expand Down Expand Up @@ -111,6 +115,7 @@ Developers may need to change their code

### Corrected Bugs

* Fixed: [State XML validation error when CLICON_MODULE_LIBRARY_RFC7895=true and ietf-yang-library@2019-01-04 is loaded](https://github.com/clicon/clixon/issues/408)
* Fixed: [SNMP: snmpwalk is slow and can timeout #404 ](https://github.com/clicon/clixon/issues/404)
* Fixed: [SNMP accepts only u32 & u64 #405](https://github.com/clicon/clixon/issues/405)
* Fixed: [Yang leaves without smiv2:oid directive are not shown well in snmpwalk #398](https://github.com/clicon/clixon/issues/398)
Expand Down
16 changes: 14 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ The clixon project welcomes contributions from the community.

## Licensing

A contribution must follow the [CLIXON
licensing](https://github.com/clicon/clixon/blob/master/LICENSE.md)
A contribution must follow the [CLIXON licensing](https://github.com/clicon/clixon/blob/master/LICENSE.md)
with the dual licensing: either Apache License, Version 2.0 or
GNU General Public License Version 3.

Expand Down Expand Up @@ -138,3 +137,16 @@ include:
- [CI on other platforms](https://github.com/clicon/clixon/tree/master/test/cicd). Other platforms include x86-64, 32-bit i686, and armv71
- [Coverage tests](https://app.codecov.io/gh/clicon/clixon)
- [Fuzzing](https://github.com/clicon/clixon/tree/master/test/fuzz) Fuzzing are run occasionally using AFL

## Optimization

Optimizating Clixon code should be based on an observable improvement
of measurements of cycles or memory usage.

Usually, new clixon code starts out with functional compliance
with appropriate regression tests.

Therafter "non-functional" analysis, including performance tests can
be made. Performance improvements should be based on specific usecase
and actual measurement. The benefit of an optimization should
be larger than a potential increase of complexity.
2 changes: 1 addition & 1 deletion apps/backend/backend_startup.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ db_merge(clicon_handle h,
cxobj *xt = NULL;

/* Get data as xml from db1 */
if (xmldb_get0(h, (char*)db1, YB_MODULE, NULL, NULL, 0, 0, &xt, NULL, NULL) < 0)
if (xmldb_get0(h, (char*)db1, YB_MODULE, NULL, NULL, 1, WITHDEFAULTS_EXPLICIT, &xt, NULL, NULL) < 0)
goto done;
xml_name_set(xt, NETCONF_INPUT_CONFIG);
/* Merge xml into db2. Without commit */
Expand Down
10 changes: 5 additions & 5 deletions apps/cli/cli_generate.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ You can see which CLISPEC it generates via clixon_cli -D 2:
static int
cli_expand_var_generate(clicon_handle h,
yang_stmt *ys,
char *cvtypestr,
const char *cvtypestr,
int options,
uint8_t fraction_digits,
int pre,
Expand Down Expand Up @@ -207,7 +207,7 @@ yang2cli_helptext(cbuf *cb,
static int
yang2cli_var_identityref(yang_stmt *ys,
yang_stmt *ytype,
char *cvtypestr,
const char *cvtypestr,
char *helptext,
cbuf *cb)
{
Expand Down Expand Up @@ -426,7 +426,7 @@ yang2cli_var_sub(clicon_handle h,
yang_stmt *yi = NULL;
int i = 0;
int j;
char *cvtypestr;
const char *cvtypestr;
char *arg;
size_t len;

Expand Down Expand Up @@ -601,7 +601,7 @@ yang2cli_var_leafref(clicon_handle h,
int retval = -1;
char *type;
int completionp;
char *cvtypestr;
const char *cvtypestr;
int ret;
int flag;
int regular_value = 1; /* if strict-expand==0 then regular-value is false */
Expand Down Expand Up @@ -674,7 +674,7 @@ yang2cli_var(clicon_handle h,
cvec *patterns = NULL;
uint8_t fraction_digits = 0;
enum cv_type cvtype;
char *cvtypestr;
const char *cvtypestr;
int options = 0;
int result;
int completionp;
Expand Down
12 changes: 12 additions & 0 deletions lib/src/clixon_options.c
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ parse_configfile(clicon_handle h,
cxobj *xt = NULL;
cxobj *xc = NULL;
cxobj *x = NULL;
yang_stmt *y;
char *name;
char *body;
clicon_hash_t *copt = clicon_options(h);
Expand Down Expand Up @@ -357,6 +358,17 @@ parse_configfile(clicon_handle h,
xe = NULL;
}
}
/* Check obsolete options before default expansion */
x = NULL;
while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) {
if ((y = xml_spec(x)) != NULL){
if ((yang_find(y, Y_STATUS, "obsolete")) != NULL){
clicon_err(OE_CFG, 0, "Clixon option %s is obsolete but given in the config file which is considered an error",
xml_name(x));
goto done;
}
}
}
if (xml_default_recurse(xt, 0) < 0)
goto done;
if ((ret = xml_yang_validate_add(h, xt, &xerr)) < 0)
Expand Down
23 changes: 5 additions & 18 deletions lib/src/clixon_yang_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ yang_modules_revision(clicon_handle h)
* @retval 0 OK
* @retval -1 Error
* This assumes CLICON_YANG_LIBRARY is enabled
* If also CLICON_MODULE_LIBRARY_RFC7895 is set, module-state is built according to RFC7895 instead
* @see RFC8525
*/
int
Expand Down Expand Up @@ -226,15 +225,10 @@ yang_modules_state_build(clicon_handle h,
clicon_err(OE_YANG, 0, "%s yang namespace not found", module);
goto done;
}
if (clicon_option_bool(h, "CLICON_MODULE_LIBRARY_RFC7895")){
cprintf(cb,"<modules-state xmlns=\"%s\">", yang_argument_get(yns));
cprintf(cb,"<module-set-id>%s</module-set-id>", msid);
}
else { /* RFC 8525 */
cprintf(cb,"<yang-library xmlns=\"%s\">", yang_argument_get(yns));
cprintf(cb,"<content-id>%s</content-id>", msid);
cprintf(cb,"<module-set><name>default</name>");
}
/* RFC 8525 */
cprintf(cb,"<yang-library xmlns=\"%s\">", yang_argument_get(yns));
cprintf(cb,"<content-id>%s</content-id>", msid);
cprintf(cb,"<module-set><name>default</name>");
ymod = NULL;
while ((ymod = yn_each(yspec, ymod)) != NULL) {
if (yang_keyword_get(ymod) != Y_MODULE)
Expand Down Expand Up @@ -270,8 +264,6 @@ yang_modules_state_build(clicon_handle h,
break;
}
}
if (clicon_option_bool(h, "CLICON_MODULE_LIBRARY_RFC7895"))
cprintf(cb, "<conformance-type>implement</conformance-type>");
}
yinc = NULL;
while ((yinc = yn_each(ymod, yinc)) != NULL) {
Expand All @@ -288,12 +280,7 @@ yang_modules_state_build(clicon_handle h,
}
cprintf(cb,"</module>");
}
if (clicon_option_bool(h, "CLICON_MODULE_LIBRARY_RFC7895")){
cprintf(cb,"</modules-state>");
}
else{
cprintf(cb,"</module-set></yang-library>");
}
cprintf(cb,"</module-set></yang-library>");
retval = 0;
done:
return retval;
Expand Down
1 change: 0 additions & 1 deletion test/fuzz/backend/runfuzz.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ cat <<EOF > $cfg
<CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
<CLICON_STARTUP_MODE>init</CLICON_STARTUP_MODE>
<CLICON_MODULE_LIBRARY_RFC7895>false</CLICON_MODULE_LIBRARY_RFC7895>
</clixon-config>
EOF

Expand Down
1 change: 0 additions & 1 deletion test/fuzz/cli/runfuzz.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ cat <<EOF > $cfg
<CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
<CLICON_STARTUP_MODE>init</CLICON_STARTUP_MODE>
<CLICON_MODULE_LIBRARY_RFC7895>false</CLICON_MODULE_LIBRARY_RFC7895>
<CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
<CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
<CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
Expand Down
1 change: 0 additions & 1 deletion test/fuzz/http1/runfuzz.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ cat <<EOF > $cfg
<CLICON_BACKEND_PIDFILE>/usr/local/var/hello.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>/usr/local/var/hello</CLICON_XMLDB_DIR>
<CLICON_STARTUP_MODE>init</CLICON_STARTUP_MODE>
<CLICON_MODULE_LIBRARY_RFC7895>false</CLICON_MODULE_LIBRARY_RFC7895>
<CLICON_SOCK_GROUP>clicon</CLICON_SOCK_GROUP>
<CLICON_NETCONF_HELLO_OPTIONAL>true</CLICON_NETCONF_HELLO_OPTIONAL>
<CLICON_RESTCONF_USER>www-data</CLICON_RESTCONF_USER>
Expand Down
1 change: 0 additions & 1 deletion test/fuzz/netconf/runfuzz.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ cat <<EOF > $cfg
<CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
<CLICON_STARTUP_MODE>init</CLICON_STARTUP_MODE>
<CLICON_MODULE_LIBRARY_RFC7895>false</CLICON_MODULE_LIBRARY_RFC7895>
<CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
<CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
<CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
Expand Down
1 change: 0 additions & 1 deletion test/fuzz/restconf/runfuzz.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ cat <<EOF > $cfg
<CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
<CLICON_STARTUP_MODE>init</CLICON_STARTUP_MODE>
<CLICON_MODULE_LIBRARY_RFC7895>false</CLICON_MODULE_LIBRARY_RFC7895>
<restconf><enable>true</enable><auth-type>none</auth-type><socket><namespace>default</namespace><address>0.0.0.0</address><port>80</port><ssl>false</ssl></socket></restconf>
</clixon-config>
EOF
Expand Down
45 changes: 32 additions & 13 deletions test/test_cli_multikey.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,19 @@ module $APPNAME{
}
}
}
list z{
key "a b" ;
leaf a {
type enumeration{
enum v1;
enum v2;
enum v3;
}
}
leaf b {
type string;
}
}
}
}
EOF
Expand All @@ -82,49 +95,55 @@ fi
new "wait backend"
wait_backend

new "set 1 v1"
new "ordered by system"
new "set x 1 v1"
expectpart "$($clixon_cli -1 -f $cfg set ex x a 1 b v1)" 0 ""

new "set 1 v2"
new "set x 1 v2"
expectpart "$($clixon_cli -1 -f $cfg set ex x a 1 b v2)" 0 ""

new "set 1 v3"
new "set x 1 v3"
expectpart "$($clixon_cli -1 -f $cfg set ex x a 1 b v3)" 0 ""

new "set 2 v1"
new "set x 2 v1"
expectpart "$($clixon_cli -1 -f $cfg set ex x a 2 b v1)" 0 ""

new "set 2 v2"
new "set x 2 v2"
expectpart "$($clixon_cli -1 -f $cfg set ex x a 2 b v2)" 0 ""

new "set 2 v3"
new "set x 2 v3"
expectpart "$($clixon_cli -1 -f $cfg set ex x a 2 b v3)" 0 ""

new "set 1 v2 again"
new "set x 1 v2 again"
expectpart "$($clixon_cli -1 -f $cfg set ex x a 1 b v2)" 0 ""

new "show conf"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>" "" "<rpc-reply $DEFAULTNS><data><ex xmlns=\"urn:example:clixon\"><x><a>1</a><b>v1</b></x><x><a>1</a><b>v2</b></x><x><a>1</a><b>v3</b></x><x><a>2</a><b>v1</b></x><x><a>2</a><b>v2</b></x><x><a>2</a><b>v3</b></x></ex></data></rpc-reply>"

# ordered-by user
new "set 1 v1"
new "ordered-by user"
new "set y 1 v1"
expectpart "$($clixon_cli -1 -f $cfg set ex y a 1 b v1)" 0 ""

new "set 2 v1"
new "set y 2 v1"
expectpart "$($clixon_cli -1 -f $cfg set ex y a 2 b v1)" 0 ""

new "set 1 v2"
new "set y 1 v2"
expectpart "$($clixon_cli -1 -f $cfg set ex y a 1 b v2)" 0 ""

new "set 1 v3"
new "set y 1 v3"
expectpart "$($clixon_cli -1 -f $cfg set ex y a 1 b v3)" 0 ""

new "set 2 v2"
new "set y 2 v2"
expectpart "$($clixon_cli -1 -f $cfg set ex y a 2 b v2)" 0 ""

new "show conf"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>" "" "<rpc-reply $DEFAULTNS><data><ex xmlns=\"urn:example:clixon\"><x><a>1</a><b>v1</b></x><x><a>1</a><b>v2</b></x><x><a>1</a><b>v3</b></x><x><a>2</a><b>v1</b></x><x><a>2</a><b>v2</b></x><x><a>2</a><b>v3</b></x><y><a>1</a><b>v1</b></y><y><a>2</a><b>v1</b></y><y><a>1</a><b>v2</b></y><y><a>1</a><b>v3</b></y><y><a>2</a><b>v2</b></y></ex></data></rpc-reply>"

new "switch keys"
# see https://github.com/clicon/clixon/issues/417
new "set z 1 v1"
expectpart "$(echo "set ex z a v1 ?" | $clixon_cli -f $cfg 2> /dev/null)" 0 b --not-- "<cr>"

if [ $BE -ne 0 ]; then
new "Kill backend"
# Check if premature kill
Expand Down
18 changes: 3 additions & 15 deletions yang/clixon/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ module clixon-config {

revision 2022-12-01 {
description
"Added option:
<nothing yet>
"Removed (previosly marked) obsolete options:
CLICON_MODULE_LIBRARY_RFC7895
Released in Clixon 6.1";
}
revision 2022-11-01 {
Expand Down Expand Up @@ -913,8 +913,7 @@ module clixon-config {
description
"If set, tag datastores with RFC 8525 YANG Module Library
info. When loaded at startup, a check is made if the system
yang modules match.
See also CLICON_MODULE_LIBRARY_RFC7895";
yang modules match.";
}
leaf CLICON_XMLDB_UPGRADE_CHECKOLD {
type boolean;
Expand Down Expand Up @@ -1059,22 +1058,11 @@ module clixon-config {
restconf GET.
The module state data is on the form:
<yang-library><module-set>...
If CLICON_MODULE_LIBRARY_RFC7895 is set (as well), the module state uses RFC7895
instead where the modile state is on the form:
<modules-state>...
See also CLICON_XMLDB_MODSTATE where the module state info is used to tag datastores
with module information.";
}
leaf CLICON_MODULE_LIBRARY_RFC7895 {
type boolean;
default false;
description
"Enable RFC 7895 YANG Module library support as state data, instead of RFC8525.
Note CLICON_YANG_LIBRARY must be enabled for this to have effect.
See also CLICON_YANG_LIBRARY and CLICON_MODULE_SET_ID";
status obsolete;
}

leaf CLICON_MODULE_SET_ID {
type string;
default "0";
Expand Down

0 comments on commit d358387

Please sign in to comment.