experiments/clixon
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

* More precise Yang validation and better error messages

Browse source 
* For Example, adding bad-, missing-, or unknown-element error messages, etc instead of operation-failed
* Removed delete-config support for candidate db since it is not supported in RFC6241.
* Switched the order of `error-type` and `error-tag` in all netconf and restconf error messages to comply to RFC order.
* Added example_rpc RPC to example backend
* Renamed xml_namespace[_set]() to xml_prefix[_set]()
* Some restconf error messages contained "rpc-reply" or "rpc-error" which have now been removed.
* Netconf/Restconf RPC extra input arguments are ignored (https://github.com/clicon/clixon/issues/47)
This commit is contained in:
Olof hagsand 2018-12-21 01:33:41 +01:00
parent 03e618b1e5
commit f872c7e295
45 changed files with 807 additions and 405 deletions
Download patch file Download diff file Expand all files Collapse all files

31
apps/backend/backend_client.c
View file

@ -468,12 +468,12 @@ from_client_edit_config(clicon_handle h,
}
}
if ((xc = xpath_first(xn, "config")) == NULL){
if (netconf_missing_element(cbret, "protocol", "<bad-element>config</bad-element>", NULL) < 0)
if (netconf_missing_element(cbret, "protocol", "config", NULL) < 0)
goto done;
goto ok;
}
else{
/* <config> yang spec may be set to anyxml by ingress yang check,...*/
/* <config> yang spec may be set to anyxmly by ingress yang check,...*/
if (xml_spec(xc) != NULL)
xml_spec_set(xc, NULL);
/* Populate XML with Yang spec (why not do this in parser?)
@ -530,7 +530,7 @@ from_client_lock(clicon_handle h,
cbuf *cbx = NULL; /* Assist cbuf */
if ((db = netconf_db_find(xe, "target")) == NULL){
if (netconf_missing_element(cbret, "protocol", "<bad-element>target</bad-element>", NULL) < 0)
if (netconf_missing_element(cbret, "protocol", "target", NULL) < 0)
goto done;
goto ok;
}
@ -589,7 +589,7 @@ from_client_unlock(clicon_handle h,
cbuf *cbx = NULL; /* Assist cbuf */
if ((db = netconf_db_find(xe, "target")) == NULL){
if (netconf_missing_element(cbret, "protocol", "<bad-element>target</bad-element>", NULL) < 0)
if (netconf_missing_element(cbret, "protocol", "target", NULL) < 0)
goto done;
goto ok;
}
@ -651,7 +651,7 @@ from_client_kill_session(clicon_handle h,
if ((x = xml_find(xe, "session-id")) == NULL ||
(str = xml_find_value(x, "body")) == NULL){
if (netconf_missing_element(cbret, "protocol", "<bad-element>session-id</bad-element>", NULL) < 0)
if (netconf_missing_element(cbret, "protocol", "session-id", NULL) < 0)
goto done;
goto ok;
}
@ -709,7 +709,7 @@ from_client_copy_config(clicon_handle h,
cbuf *cbx = NULL; /* Assist cbuf */
if ((source = netconf_db_find(xe, "source")) == NULL){
if (netconf_missing_element(cbret, "protocol", "<bad-element>source</bad-element>", NULL) < 0)
if (netconf_missing_element(cbret, "protocol", "source", NULL) < 0)
goto done;
goto ok;
}
@ -724,7 +724,7 @@ from_client_copy_config(clicon_handle h,
goto ok;
}
if ((target = netconf_db_find(xe, "target")) == NULL){
if (netconf_missing_element(cbret, "protocol", "<bad-element>target</bad-element>", NULL) < 0)
if (netconf_missing_element(cbret, "protocol", "target", NULL) < 0)
goto done;
goto ok;
}
@ -777,7 +777,7 @@ from_client_delete_config(clicon_handle h,
if ((target = netconf_db_find(xe, "target")) == NULL||
strcmp(target, "running")==0){
if (netconf_missing_element(cbret, "protocol", "<bad-element>target</bad-element>", NULL) < 0)
if (netconf_missing_element(cbret, "protocol", "target", NULL) < 0)
goto done;
goto ok;
}
@ -856,7 +856,7 @@ from_client_create_subscription(clicon_handle h,
if ((x = xpath_first(xe, "//stopTime")) != NULL){
if ((stoptime = xml_find_value(x, "body")) != NULL &&
str2time(stoptime, &stop) < 0){
if (netconf_bad_element(cbret, "application", "<bad-element>stopTime</bad-element>", "Expected timestamp") < 0)
if (netconf_bad_element(cbret, "application", "stopTime", "Expected timestamp") < 0)
goto done;
goto ok;
}
@ -864,7 +864,7 @@ from_client_create_subscription(clicon_handle h,
if ((x = xpath_first(xe, "//startTime")) != NULL){
if ((starttime = xml_find_value(x, "body")) != NULL &&
str2time(starttime, &start) < 0){
if (netconf_bad_element(cbret, "application", "<bad-element>startTime</bad-element>", "Expected timestamp") < 0)
if (netconf_bad_element(cbret, "application", "startTime", "Expected timestamp") < 0)
goto done;
goto ok;
}
@ -925,7 +925,7 @@ from_client_debug(clicon_handle h,
char *valstr;
if ((valstr = xml_find_body(xe, "level")) == NULL){
if (netconf_missing_element(cbret, "application", "<bad-element>level</bad-element>", NULL) < 0)
if (netconf_missing_element(cbret, "application", "level", NULL) < 0)
goto done;
goto ok;
}
@ -993,13 +993,10 @@ from_client_msg(clicon_handle h,
/* Populate incoming XML tree with yang */
if (xml_spec_populate_rpc(h, x, yspec) < 0)
goto done;
if ((ret = xml_yang_validate_rpc(x)) < 0)
if ((ret = xml_yang_validate_rpc(x, cbret)) < 0)
goto done;
if (ret == 0){
if (netconf_operation_failed(cbret, "application", "Validation failed")< 0)
goto done;
if (ret == 0)
goto reply;
}
xe = NULL;
username = xml_find_value(x, "username");
while ((xe = xml_child_each(x, xe, CX_ELMNT)) != NULL) {
@ -1059,7 +1056,7 @@ from_client_msg(clicon_handle h,
}
else if (strcmp(rpc, "validate") == 0){
if ((db = netconf_db_find(xe, "source")) == NULL){
if (netconf_missing_element(cbret, "protocol", "<bad-element>source</bad-element>", NULL) < 0)
if (netconf_missing_element(cbret, "protocol", "source", NULL) < 0)
goto done;
goto reply;
}

106
apps/backend/backend_commit.c
View file

@ -81,68 +81,86 @@
* are if code comes via XML/NETCONF.
* @param[in] yspec Yang spec
* @param[in] td Transaction data
* @param[out] cbret Cligen buffer containing netconf error (if retval == 0)
* @retval -1 Error
* @retval 0 Validation failed (with cbret set)
* @retval 1 Validation OK
*/
static int
generic_validate(yang_spec *yspec,
transaction_data_t *td)
transaction_data_t *td,
cbuf *cbret)
{
int retval = -1;
cxobj *x1;
cxobj *x2;
yang_stmt *ys;
int i;
int ret;
/* All entries */
if (xml_apply(td->td_target, CX_ELMNT,
(xml_applyfn_t*)xml_yang_validate_all, NULL) < 0)
if ((ret = xml_yang_validate_all_top(td->td_target, cbret)) < 0)
goto done;
if (ret == 0)
goto fail;
/* changed entries */
for (i=0; i<td->td_clen; i++){
x1 = td->td_scvec[i]; /* source changed */
x2 = td->td_tcvec[i]; /* target changed */
if (xml_yang_validate_add(x2, NULL) < 0)
/* Should this be recursive? */
if ((ret = xml_yang_validate_add(x2, cbret)) < 0)
goto done;
if (ret == 0)
goto fail;
}
/* deleted entries */
for (i=0; i<td->td_dlen; i++){
x1 = td->td_dvec[i];
ys = xml_spec(x1);
if (ys && yang_mandatory(ys)){
clicon_err(OE_CFG, 0,"Removed mandatory variable: %s",
xml_name(x1));
goto done;
if (netconf_missing_element(cbret, "protocol", xml_name(x1), "Removed mandatory variable") < 0)
goto done;
goto fail;
}
}
/* added entries */
for (i=0; i<td->td_alen; i++){
x2 = td->td_avec[i];
if (xml_apply0(x2, CX_ELMNT,
(xml_applyfn_t*)xml_yang_validate_add, NULL) < 0)
if ((ret = xml_yang_validate_add(x2, cbret)) < 0)
goto done;
if (ret == 0)
goto fail;
}
retval = 0;
// ok:
retval = 1;
done:
return retval;
fail:
retval = 0;
goto done;
}
/*! Common code of candidate_validate and candidate_commit
* @param[in] h Clicon handle
* @param[in] candidate The candidate database. The wanted backend state
* @retval 0 OK
* @retval -1 Fatal error or validation fail
* @note Need to differentiate between error and validation fail
* @retval -1 Error - or validation failed (but cbret not set)
* @retval 0 Validation failed (with cbret set)
* @retval 1 Validation OK
* @note Need to differentiate between error and validation fail
* (only done for generic_validate)
*/
static int
validate_common(clicon_handle h,
char *candidate,
transaction_data_t *td)
transaction_data_t *td,
cbuf *cbret)
{
int retval = -1;
yang_spec *yspec;
int i;
cxobj *xn;
int ret;
if ((yspec = clicon_dbspec_yang(h)) == NULL){
clicon_err(OE_FATAL, 0, "No DB_SPEC");
goto done;
@ -193,9 +211,12 @@ validate_common(clicon_handle h,
if (plugin_transaction_begin(h, td) < 0)
goto done;
/* 5. Make generic validation on all new or changed data. */
if (generic_validate(yspec, td) < 0)
/* 5. Make generic validation on all new or changed data.
Note this is only call that uses 3-values */
if ((ret = generic_validate(yspec, td, cbret)) < 0)
goto done;
if (ret == 0)
goto fail;
/* 6. Call plugin transaction validate callbacks */
if (plugin_transaction_validate(h, td) < 0)
@ -204,9 +225,12 @@ validate_common(clicon_handle h,
/* 7. Call plugin transaction complete callbacks */
if (plugin_transaction_complete(h, td) < 0)
goto done;
retval = 0;
retval = 1;
done:
return retval;
fail:
retval = 0;
goto done;
}
/*! Do a diff between candidate and running, then start a commit transaction
@ -216,24 +240,30 @@ validate_common(clicon_handle h,
* do something more drastic?
* @param[in] h Clicon handle
* @param[in] candidate A candidate database, not necessarily "candidate"
* @retval 0 OK
* @retval -1 Fatal error or validation fail
* @retval -1 Error - or validation failed (but cbret not set)
* @retval 0 Validation failed (with cbret set)
* @retval 1 Validation OK
* @note Need to differentiate between error and validation fail
* (only done for validate_common)
*/
int
candidate_commit(clicon_handle h,
char *candidate)
char *candidate,
cbuf *cbret)
{
int retval = -1;
int retval = -1;
transaction_data_t *td = NULL;
int ret;
/* 1. Start transaction */
if ((td = transaction_new()) == NULL)
goto done;
/* Common steps (with validate) */
if (validate_common(h, candidate, td) < 0)
/* Common steps (with validate). Note this is only call that uses 3-values */
if ((ret = validate_common(h, candidate, td, cbret)) < 0)
goto done;
if (ret == 0)
goto fail;
/* 7. Call plugin transaction commit callbacks */
if (plugin_transaction_commit(h, td) < 0)
@ -252,21 +282,23 @@ candidate_commit(clicon_handle h,
/* 9. Call plugin transaction end callbacks */
plugin_transaction_end(h, td);
/* 8. Copy running back to candidate in case end functions updated running */
if (xmldb_copy(h, "running", candidate) < 0){
/* ignore errors or signal major setback ? */
clicon_log(LOG_NOTICE, "Error in rollback, trying to continue");
goto done;
}
retval = 0;
retval = 1;
done:
/* In case of failure, call plugin transaction termination callbacks */
if (retval < 0 && td)
/* In case of failure (or error), call plugin transaction termination callbacks */
if (retval < 1 && td)
plugin_transaction_abort(h, td);
if (td)
transaction_free(td);
return retval;
fail:
retval = 0;
goto done;
}
/*! Commit changes from candidate to running
@ -283,6 +315,7 @@ from_client_commit(clicon_handle h,
int retval = -1;
int piddb;
cbuf *cbx = NULL; /* Assist cbuf */
int ret;
/* Check if target locked by other client */
piddb = xmldb_islocked(h, "running");
@ -296,9 +329,10 @@ from_client_commit(clicon_handle h,
goto done;
goto ok;
}
if (candidate_commit(h, "candidate") < 0){ /* Assume validation fail, nofatal */
if ((ret = candidate_commit(h, "candidate", cbret)) < 0){ /* Assume validation fail, nofatal */
clicon_debug(1, "Commit candidate failed");
if (netconf_invalid_value(cbret, "protocol", clicon_err_reason)< 0)
if (ret < 0)
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done;
goto ok;
}
@ -367,6 +401,7 @@ from_client_validate(clicon_handle h,
{
int retval = -1;
transaction_data_t *td = NULL;
int ret;
if (strcmp(db, "candidate") != 0 && strcmp(db, "tmp") != 0){
if (netconf_invalid_value(cbret, "protocol", "No such database")< 0)
@ -379,11 +414,12 @@ from_client_validate(clicon_handle h,
if ((td = transaction_new()) == NULL)
goto done;
/* Common steps (with commit) */
if (validate_common(h, db, td) < 0){
if ((ret = validate_common(h, db, td, cbret)) < 1){
clicon_debug(1, "Validate %s failed", db);
/* XXX: candidate_validate should have proper error handling */
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done;
if (ret < 0){
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done;
}
goto ok;
}
/* Optionally write (potentially modified) tree back to candidate */

2
apps/backend/backend_commit.h
View file

@ -43,6 +43,6 @@
int from_client_validate(clicon_handle h, char *db, cbuf *cbret);
int from_client_commit(clicon_handle h, int pid, cbuf *cbret);
int from_client_discard_changes(clicon_handle h, int pid, cbuf *cbret);
int candidate_commit(clicon_handle h, char *db);
int candidate_commit(clicon_handle h, char *db, cbuf *cbret);
#endif /* _BACKEND_COMMIT_H_ */

25
apps/backend/backend_main.c
View file

@ -386,6 +386,7 @@ startup_mode_running(clicon_handle h,
char *extraxml_file)
{
int retval = -1;
cbuf *cbret = NULL;
/* Stash original running to candidate for later commit */
if (xmldb_copy(h, "running", "candidate") < 0)
@ -405,15 +406,20 @@ startup_mode_running(clicon_handle h,
/* Clear running db */
if (db_reset(h, "running") < 0)
goto done;
if ((cbret = cbuf_new()) == NULL){
clicon_err(OE_XML, errno, "cbuf_new");
goto done;
}
/* Commit original running. Assume -1 is validate fail */
if (candidate_commit(h, "candidate") < 0){
if (candidate_commit(h, "candidate", cbret) < 1){
/* (1) We cannot differentiate between fatal errors and validation
* failures
* (2) If fatal error, we should exit
* (3) If validation fails we cannot continue. How could we?
* (4) Need to restore the running db since we destroyed it above
*/
clicon_log(LOG_NOTICE, "%s: Commit of saved running failed, exiting.", __FUNCTION__);
clicon_log(LOG_NOTICE, "%s: Commit of saved running failed, exiting: %s.",
__FUNCTION__, cbuf_get(cbret));
/* Reinstate original */
if (xmldb_copy(h, "candidate", "running") < 0)
goto done;
@ -424,6 +430,8 @@ startup_mode_running(clicon_handle h,
goto done;
retval = 0;
done:
if (cbret)
cbuf_free(cbret);
if (xmldb_delete(h, "tmp") < 0)
goto done;
return retval;
@ -455,6 +463,7 @@ startup_mode_startup(clicon_handle h,
char *extraxml_file)
{
int retval = -1;
cbuf *cbret = NULL;
/* Stash original running to backup */
if (xmldb_copy(h, "running", "backup") < 0)
@ -478,13 +487,19 @@ startup_mode_startup(clicon_handle h,
/* Clear running db */
if (db_reset(h, "running") < 0)
goto done;
/* Create return buffer (not used) */
if ((cbret = cbuf_new()) == NULL){
clicon_err(OE_XML, errno, "cbuf_new");
goto done;
}
/* Commit startup */
if (candidate_commit(h, "startup") < 0){ /* diff */
if (candidate_commit(h, "startup", cbret) < 1){ /* diff */
/* We cannot differentiate between fatal errors and validation
* failures
* In both cases we copy back the original running and quit
*/
clicon_log(LOG_NOTICE, "%s: Commit of startup failed, exiting.", __FUNCTION__);
clicon_log(LOG_NOTICE, "%s: Commit of startup failed, exiting: %s.",
__FUNCTION__, cbuf_get(cbret));
if (xmldb_copy(h, "backup", "running") < 0)
goto done;
goto done;
@ -494,6 +509,8 @@ startup_mode_startup(clicon_handle h,
goto done;
retval = 0;
done:
if (cbret)
cbuf_free(cbret);
if (xmldb_delete(h, "backup") < 0)
goto done;
if (xmldb_delete(h, "tmp") < 0)