Skip to content

Commit

Permalink
* Added message-id attributes in error and hello replies
Browse files Browse the repository at this point in the history
  * See [namespace prefix nc is not supported in full #154](#154)
* Removed mandatory loading of clixon_restconf.yang
  • Loading branch information
olofhagsand committed Dec 1, 2020
1 parent 26a4b14 commit c32950c
Show file tree
Hide file tree
Showing 20 changed files with 119 additions and 56 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ Developers may need to change their code

### Corrected Bugs

* Added message-id attributes in error and hello replies
* See [namespace prefix nc is not supported in full #154](https://github.com/clicon/clixon/issues/154)
* Fixed [Clixon backend generates wrong XML on empty string value #144](https://github.com/clicon/clixon/issues/144)

## 4.8.0
Expand Down
9 changes: 7 additions & 2 deletions apps/backend/backend_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -1553,15 +1553,20 @@ from_client_hello(clicon_handle h,
{
int retval = -1;
uint32_t id;
char *msgid;

if (clicon_session_id_get(h, &id) < 0){
clicon_err(OE_NETCONF, ENOENT, "session_id not set");
goto done;
}
id++;
clicon_session_id_set(h, id);
cprintf(cbret, "<hello xmlns=\"%s\"><session-id>%u</session-id></hello>",
NETCONF_BASE_NAMESPACE, id);
if ((msgid = xml_find_value(x, "message-id")) != NULL)
cprintf(cbret, "<hello xmlns=\"%s\" message-id=\"%s\"><session-id>%u</session-id></hello>",
NETCONF_BASE_NAMESPACE, msgid, id);
else
cprintf(cbret, "<hello xmlns=\"%s\"><session-id>%u</session-id></hello>",
NETCONF_BASE_NAMESPACE, id);
retval = 0;
done:
return retval;
Expand Down
7 changes: 5 additions & 2 deletions apps/restconf/restconf_main_evhtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,10 @@ restconf_config(clicon_handle h,
/* Add netconf yang spec, used as internal protocol */
if (netconf_module_load(h) < 0)
goto done;

/* Clixon restconf daemon config */
if (yang_spec_parse_module(h, "clixon-restconf", NULL, yspec)< 0)
goto done;

/* Add system modules */
if (clicon_option_bool(h, "CLICON_STREAM_DISCOVERY_RFC8040") &&
yang_spec_parse_module(h, "ietf-restconf-monitoring", NULL, yspec)< 0)
Expand Down Expand Up @@ -1148,7 +1151,7 @@ restconf_config(clicon_handle h,
sleep(1);
continue;
}
// clicon_err(OE_UNIX, errno, "clicon_session_id_get");
clicon_err(OE_UNIX, errno, "clicon_session_id_get");
goto done;
}
clicon_session_id_set(h, id);
Expand Down
2 changes: 0 additions & 2 deletions example/main/clixon-example@2020-03-11.yang
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ module clixon-example {
import ietf-datastores {
prefix ds;
}

/* Example interface type for tests, local callbacks, etc */
identity eth {
base if:interface-type;
Expand Down Expand Up @@ -90,7 +89,6 @@ module clixon-example {
ex:e4 arg1{
uses bar;
}

/* Example notification as used in RFC 5277 and RFC 8040 */
notification event {
description "Example notification event.";
Expand Down
9 changes: 8 additions & 1 deletion example/main/example_backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -297,14 +297,21 @@ example_rpc(clicon_handle h, /* Clicon handle */
{
int retval = -1;
cxobj *x = NULL;
cxobj *xp;
char *namespace;
char *msgid;

/* get namespace from rpc name, return back in each output parameter */
if ((namespace = xml_find_type_value(xe, NULL, "xmlns", CX_ATTR)) == NULL){
clicon_err(OE_XML, ENOENT, "No namespace given in rpc %s", xml_name(xe));
goto done;
}
cprintf(cbret, "<rpc-reply xmlns=\"%s\">", NETCONF_BASE_NAMESPACE);
cprintf(cbret, "<rpc-reply xmlns=\"%s\"", NETCONF_BASE_NAMESPACE);
if ((xp = xml_parent(xe)) != NULL &&
(msgid = xml_find_value(xp, "message-id"))){
cprintf(cbret, " message-id=\"%s\">", msgid);
}
cprintf(cbret, ">");
if (!xml_child_nr_type(xe, CX_ELMNT))
cprintf(cbret, "<ok/>");
else while ((x = xml_child_each(xe, x, CX_ELMNT)) != NULL) {
Expand Down
2 changes: 1 addition & 1 deletion example/main/example_cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ example_client_rpc(clicon_handle h,
cva = cvec_find(cvv, "a"); /* get a cligen variable from vector */
/* Create XML for example netconf RPC */
if (clixon_xml_parse_va(YB_NONE, NULL, &xtop, NULL,
"<rpc message-id=\"101\" xmlns=\"%s\" username=\"%s\">"
"<rpc xmlns=\"%s\" username=\"%s\" message-id=\"101\">"
"<example xmlns=\"urn:example:clixon\"><x>%s</x></example></rpc>",
NETCONF_BASE_NAMESPACE,
clicon_username_get(h),
Expand Down
1 change: 1 addition & 0 deletions lib/clixon/clixon_xml_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,6 @@ int clixon_xml_parse_va(yang_bind yb, yang_stmt *yspec, cxobj **xt, cxobj **xerr
int clixon_xml_parse_va(yang_bind yb, yang_stmt *yspec, cxobj **xt, cxobj **xerr,
const char *format, ...);
#endif
int clixon_xml_attr_copy(cxobj *xin, cxobj *xout, char *name);

#endif /* _CLIXON_XML_IO_H_ */
5 changes: 1 addition & 4 deletions lib/src/clixon_netconf_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1379,9 +1379,6 @@ netconf_module_load(clicon_handle h)
if (clicon_option_bool(h, "CLICON_XML_CHANGELOG"))
if (yang_spec_parse_module(h, "clixon-xml-changelog", NULL, yspec)< 0)
goto done;
/* Clixon restconf daemon */
if (yang_spec_parse_module(h, "clixon-restconf", NULL, yspec)< 0)
goto done;
retval = 0;
done:
return retval;
Expand Down Expand Up @@ -1538,7 +1535,7 @@ netconf_hello_server(clicon_handle h,

module_set_id = clicon_option_str(h, "CLICON_MODULE_SET_ID");

cprintf(cb, "<hello xmlns=\"%s\">", NETCONF_BASE_NAMESPACE);
cprintf(cb, "<hello xmlns=\"%s\" message-id=\"%u\">", NETCONF_BASE_NAMESPACE, 42);
cprintf(cb, "<capabilities>");
cprintf(cb, "<capability>urn:ietf:params:netconf:base:1.0</capability>");
/* Check if RFC7895 loaded and revision found */
Expand Down
8 changes: 3 additions & 5 deletions lib/src/clixon_proto_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -790,8 +790,8 @@ clicon_rpc_close_session(clicon_handle h)
goto done;
username = clicon_username_get(h);
if ((msg = clicon_msg_encode(session_id,
"<rpc xmlns=\"%s\" username=\"%s\"><close-session/></rpc>",
NETCONF_BASE_NAMESPACE, username?username:"")) == NULL)
"<rpc xmlns=\"%s\" username=\"%s\" message-id=\"%u\"><close-session/></rpc>",
NETCONF_BASE_NAMESPACE, username?username:"", 42)) == NULL)
goto done;
if (clicon_rpc_msg(h, msg, &xret, NULL) < 0)
goto done;
Expand Down Expand Up @@ -1084,7 +1084,7 @@ clicon_hello_req(clicon_handle h,
int ret;

username = clicon_username_get(h);
if ((msg = clicon_msg_encode(0, "<hello username=\"%s\" xmlns=\"%s\"><capabilities><capability>urn:ietf:params:netconf:base:1.0</capability></capabilities></hello>",
if ((msg = clicon_msg_encode(0, "<hello username=\"%s\" xmlns=\"%s\" message-id=\"42\"><capabilities><capability>urn:ietf:params:netconf:base:1.0</capability></capabilities></hello>",
username?username:"",
NETCONF_BASE_NAMESPACE)) == NULL)
goto done;
Expand All @@ -1111,5 +1111,3 @@ clicon_hello_req(clicon_handle h,
xml_free(xret);
return retval;
}


12 changes: 10 additions & 2 deletions lib/src/clixon_validate.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
#include "clixon_netconf_lib.h"
#include "clixon_options.h"
#include "clixon_xml_nsctx.h"
#include "clixon_xml_io.h"
#include "clixon_xpath_ctx.h"
#include "clixon_xpath.h"
#include "clixon_yang_module.h"
Expand Down Expand Up @@ -332,6 +333,7 @@ xml_yang_validate_rpc(clicon_handle h,
cxobj *xn; /* rpc name */
char *rpcprefix;
char *namespace = NULL;
int ret;

if (strcmp(xml_name(xrpc), "rpc")){
clicon_err(OE_XML, EINVAL, "Expected RPC");
Expand All @@ -357,10 +359,14 @@ xml_yang_validate_rpc(clicon_handle h,
goto done;
goto fail;
}
if ((retval = xml_yang_validate_all(h, xn, xret)) < 1)
if ((ret = xml_yang_validate_all(h, xn, xret)) < 0)
goto done; /* error or validation fail */
if ((retval = xml_yang_validate_add(h, xn, xret)) < 1)
if (ret == 0)
goto fail;
if ((ret = xml_yang_validate_add(h, xn, xret)) < 0)
goto done; /* error or validation fail */
if (ret == 0)
goto fail;
if (xml_default_recurse(xn, 0) < 0)
goto done;
}
Expand All @@ -369,6 +375,8 @@ xml_yang_validate_rpc(clicon_handle h,
done:
return retval;
fail:
if (xret && *xret && clixon_xml_attr_copy(xrpc, *xret, "message-id") < 0)
goto done;
retval = 0;
goto done;
}
Expand Down
40 changes: 39 additions & 1 deletion lib/src/clixon_xml_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,11 @@ _xml_parse(const char *str,
case YB_RPC:
if ((ret = xml_bind_yang_rpc(x, yspec, xerr)) < 0)
goto done;
if (ret == 0)
if (ret == 0){ /* Add message-id */
if (*xerr && clixon_xml_attr_copy(x, *xerr, "message-id") < 0)
goto done;
failed++;
}
break;
} /* switch */
}
Expand Down Expand Up @@ -769,3 +772,38 @@ clixon_xml_parse_va(yang_bind yb,
return retval;
}

/*! Copy an attribute value(eg message-id) from one xml (eg rpc input) to another xml (eg rpc outgoing)
* @param[in] xin Get attr value from this XML
* @param[in] xout Set attr value to this XML
* @param[in] name Attribute name
* @retval 0 OK
* @retval -1 Error
* Alternative is to use: char *val = xml_find_value(x, name);
* @code
* if (clixon_xml_attr_copy(xin, xout, "message-id") < 0)
* err;
* @endcode
*/
int
clixon_xml_attr_copy(cxobj *xin,
cxobj *xout,
char *name)
{
int retval = -1;
char *msgid;
cxobj *xa;

if (xin == NULL || xout == NULL){
clicon_err(OE_XML, EINVAL, "xin or xout NULL");
goto done;
}
if ((msgid = xml_find_value(xin, "message-id")) != NULL){
if ((xa = xml_new("message-id", xout, CX_ATTR)) == NULL)
goto done;
if (xml_value_set(xa, msgid) < 0)
goto done;
}
retval = 0;
done:
return retval;
}
7 changes: 4 additions & 3 deletions test/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ testname=
: ${RCLOG:=}

# Default netconf namespace statement, typically as placed on top-level <rpc xmlns=""
DEFAULTNS='xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"'
DEFAULTONLY='xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"'
# Default netconf namespace + message-id
DEFAULTNS="$DEFAULTONLY message-id=\"42\""

# Options passed to curl calls
# -s : silent
Expand Down Expand Up @@ -164,7 +166,6 @@ fi
# This check is optional because some installs, such as vagrant make a non-systemd/direct
# start
: ${NGINXCHECK:=false}

# Sanity nginx running on systemd platforms
if $NGINXCHECK; then
if systemctl > /dev/null 2>&1 ; then
Expand Down Expand Up @@ -295,7 +296,7 @@ wait_backend(){
reply=$(echo '<rpc xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="101" xmlns="http://clicon.org/lib"><ping/></rpc>]]>]]>' | clixon_netconf -qef $cfg 2> /dev/null)
echo "reply:$reply"
let i++;
echo "wait_backend $i"
# echo "wait_backend $i"
if [ $i -ge $DEMLOOP ]; then
err "backend timeout $DEMWAIT seconds"
fi
Expand Down
4 changes: 2 additions & 2 deletions test/test_choice.sh
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ new "netconf commit protocol tcp"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS><commit/></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"

new "netconf changing from TCP to UDP (RFC7950 7.9.6)"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS message-id=\"101\" xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\"><protocol><udp nc:operation=\"create\"/></protocol></system></config></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS message-id=\"101\" xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><ok/></rpc-reply>]]>]]>$"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\"><protocol><udp nc:operation=\"create\"/></protocol></system></config></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><ok/></rpc-reply>]]>]]>$"

new "netconf get protocol udp"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><data><system xmlns=\"urn:example:config\"><protocol><udp/></protocol></system>"
Expand Down Expand Up @@ -216,7 +216,7 @@ new "netconf set shorthand tcp"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\"><shorthand><tcp/></shorthand></system></config></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"

new "netconf changing from TCP to UDP (RFC7950 7.9.6)"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS message-id=\"101\" xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\"><shorthand><udp/></shorthand></system></config></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS message-id=\"101\" xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><ok/></rpc-reply>]]>]]>$"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\"><shorthand><udp/></shorthand></system></config></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><ok/></rpc-reply>]]>]]>$"

new "netconf get shorthand udp"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><data><system xmlns=\"urn:example:config\"><shorthand><udp/></shorthand></system></data></rpc-reply>]]>]]>$"
Expand Down
3 changes: 2 additions & 1 deletion test/test_cli.sh
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ new "cli debug reset"
expectfn "$clixon_cli -1 -f $cfg -l o debug level 0" 0 "^$"

new "cli rpc"
expectpart "$($clixon_cli -1 -f $cfg -l o rpc ipv4)" 0 "<rpc-reply $DEFAULTNS><x xmlns=\"urn:example:clixon\">ipv4</x><y xmlns=\"urn:example:clixon\">42</y></rpc-reply>"
# We dont know which message-id the cli app uses
expectpart "$($clixon_cli -1 -f $cfg -l o rpc ipv4)" 0 "<rpc-reply $DEFAULTONLY message-id=" "><x xmlns=\"urn:example:clixon\">ipv4</x><y xmlns=\"urn:example:clixon\">42</y></rpc-reply>"

if [ $BE -eq 0 ]; then
exit # BE
Expand Down
Loading

0 comments on commit c32950c

Please sign in to comment.