peter/infix
1
0
Fork 0
mirror of https://github.com/kernelkit/infix.git synced 2026-08-06 07:33:01 +02:00
Code Issues Packages Projects Releases Wiki Activity

RESTCONF: Update rousette to upstream

Browse Source 
This fix #781
This commit is contained in:
Mattias Walström
2024-12-11 10:13:50 +01:00
parent 83238aad7e
commit e08576b008
21 changed files with 3829 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
buildroot
+1 -1
Submodule buildroot updated: 80d4761b48...aade1bf5a0
package/rousette/0001-restconf-report-the-list-key-values-when-they-differ.patch
+199
View File
@@ -0,0 +1,199 @@
From 9622a68eba4aeaa60619b4c33d050ce91b27653d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Tue, 8 Oct 2024 12:22:49 +0200
Subject: [PATCH 01/20] restconf: report the list key values when they differ
between URI and data
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
For creating (leaf-)list instances with PUT and PATCH methods one has
to specify the list key values in the URI and in the data as well.
Those values must be the same. In case they are not, it is an error.
We reported that the values mismatch in case this happens, but the error
message did not report what the values were.
Knowing that might be beneficial when one is about to insert key values
that can be namespace qualified (like identityrefs) and that are
sometimes manipulated by libyang (e.g., when the identity belongs to the
same namespace as the list node, it is not necessary for it to be
namespace qualified by the client, but libyang adds the namespace
internally).
Change-Id: Ie0d42511bde01ab4c39d61370b6601f8808e40c5
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
src/restconf/Server.cpp | 29 +++++++++++++++++++++--------
tests/restconf-plain-patch.cpp | 2 +-
tests/restconf-writing.cpp | 14 +++++++-------
tests/restconf-yang-patch.cpp | 2 +-
4 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/src/restconf/Server.cpp b/src/restconf/Server.cpp
index 53d6625..5f560ed 100644
--- a/src/restconf/Server.cpp
+++ b/src/restconf/Server.cpp
@@ -154,9 +154,22 @@ auto rejectYangPatch(const std::string& patchId, const std::string& editId)
};
}
+struct KeyMismatch {
+ libyang::DataNode offendingNode;
+ std::optional<std::string> uriKeyValue;
+
+ std::string message() const {
+ if (uriKeyValue) {
+ return "List key mismatch between URI path ('"s + *uriKeyValue + "') and data ('" + offendingNode.asTerm().valueStr() + "').";
+ } else {
+ return "List key mismatch (key missing in the data).";
+ }
+ }
+};
+
/** @brief In case node is a (leaf-)list check if the key values are the same as the keys specified in the lastPathSegment.
* @return The node where the mismatch occurs */
-std::optional<libyang::DataNode> checkKeysMismatch(const libyang::DataNode& node, const PathSegment& lastPathSegment)
+std::optional<KeyMismatch> checkKeysMismatch(const libyang::DataNode& node, const PathSegment& lastPathSegment)
{
if (node.schema().nodeType() == libyang::NodeType::List) {
const auto& listKeys = node.schema().asList().keys();
@@ -164,18 +177,18 @@ std::optional<libyang::DataNode> checkKeysMismatch(const libyang::DataNode& node
const auto& keyValueURI = lastPathSegment.keys[i];
auto keyNodeData = node.findPath(listKeys[i].module().name() + ':' + listKeys[i].name());
if (!keyNodeData) {
- return node;
+ return KeyMismatch{node, std::nullopt};
}
const auto& keyValueData = keyNodeData->asTerm().valueStr();
if (keyValueURI != keyValueData) {
- return keyNodeData;
+ return KeyMismatch{*keyNodeData, keyValueURI};
}
}
} else if (node.schema().nodeType() == libyang::NodeType::Leaflist) {
if (lastPathSegment.keys[0] != node.asTerm().valueStr()) {
- return node;
+ return KeyMismatch{node, lastPathSegment.keys[0]};
}
}
return std::nullopt;
@@ -350,8 +363,8 @@ libyang::CreatedNodes createEditForPutAndPatch(libyang::Context& ctx, const std:
if (isSameNode(child, lastPathSegment)) {
// 1) a single child that is created by parseSubtree(), its name is the same as `lastPathSegment`.
// It could be a list; then we need to check if the keys in provided data match the keys in URI.
- if (auto offendingNode = checkKeysMismatch(child, lastPathSegment)) {
- throw ErrorResponse(400, "protocol", "invalid-value", "List key mismatch between URI path and data.", offendingNode->path());
+ if (auto keyMismatch = checkKeysMismatch(child, lastPathSegment)) {
+ throw ErrorResponse(400, "protocol", "invalid-value", keyMismatch->message(), keyMismatch->offendingNode.path());
}
replacementNode = child;
} else if (isKeyNode(*node, child)) {
@@ -373,8 +386,8 @@ libyang::CreatedNodes createEditForPutAndPatch(libyang::Context& ctx, const std:
if (!isSameNode(*replacementNode, lastPathSegment)) {
throw ErrorResponse(400, "protocol", "invalid-value", "Data contains invalid node.", replacementNode->path());
}
- if (auto offendingNode = checkKeysMismatch(*parent, lastPathSegment)) {
- throw ErrorResponse(400, "protocol", "invalid-value", "List key mismatch between URI path and data.", offendingNode->path());
+ if (auto keyMismatch = checkKeysMismatch(*parent, lastPathSegment)) {
+ throw ErrorResponse(400, "protocol", "invalid-value", keyMismatch->message(), keyMismatch->offendingNode.path());
}
}
}
diff --git a/tests/restconf-plain-patch.cpp b/tests/restconf-plain-patch.cpp
index 10d653a..d4f3952 100644
--- a/tests/restconf-plain-patch.cpp
+++ b/tests/restconf-plain-patch.cpp
@@ -72,7 +72,7 @@ TEST_CASE("Plain patch")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='blabla']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('libyang') and data ('blabla')."
}
]
}
diff --git a/tests/restconf-writing.cpp b/tests/restconf-writing.cpp
index d46952f..96dbb25 100644
--- a/tests/restconf-writing.cpp
+++ b/tests/restconf-writing.cpp
@@ -432,7 +432,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='ahoj']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('netconf') and data ('ahoj')."
}
]
}
@@ -447,7 +447,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:top-level-list[name='ahoj']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('netconf') and data ('ahoj')."
}
]
}
@@ -505,7 +505,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='netconf']/collection[.='666']",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('667') and data ('666')."
}
]
}
@@ -520,7 +520,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:top-level-leaf-list[.='666']",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('667') and data ('666')."
}
]
}
@@ -535,7 +535,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='sysrepo']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('netconf') and data ('sysrepo')."
}
]
}
@@ -550,7 +550,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='sysrepo']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('netconf') and data ('sysrepo')."
}
]
}
@@ -565,7 +565,7 @@ TEST_CASE("writing data")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='libyang']/collection[.='42']",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('5') and data ('42')."
}
]
}
diff --git a/tests/restconf-yang-patch.cpp b/tests/restconf-yang-patch.cpp
index 9d70912..7cc8946 100644
--- a/tests/restconf-yang-patch.cpp
+++ b/tests/restconf-yang-patch.cpp
@@ -436,7 +436,7 @@ TEST_CASE("YANG patch")
"error-type": "protocol",
"error-tag": "invalid-value",
"error-path": "/example:tlc/list[name='asdasdauisbdhaijbsdad']/name",
- "error-message": "List key mismatch between URI path and data."
+ "error-message": "List key mismatch between URI path ('libyang') and data ('asdasdauisbdhaijbsdad')."
}
]
}
--
2.43.0
package/rousette/0002-uri-rename-url-encoding-to-percent-encoding.patch
+50
View File
@@ -0,0 +1,50 @@
From 070cffb48fda789910581930265d4624a7213e1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Wed, 16 Oct 2024 11:02:45 +0200
Subject: [PATCH 02/20] uri: rename url-encoding to percent-encoding
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
RFC 3986 [1] calls it the percent-encoding, let's be consistent.
[1] https://datatracker.ietf.org/doc/html/rfc3986
Change-Id: Iee8b76c980b2694b6643e627b462f8bfc2c21c45
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
src/restconf/uri.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/restconf/uri.cpp b/src/restconf/uri.cpp
index 6e27168..b144d92 100644
--- a/src/restconf/uri.cpp
+++ b/src/restconf/uri.cpp
@@ -29,12 +29,12 @@ auto add = [](auto& ctx) {
char c = std::tolower(_attr(ctx));
_val(ctx) = _val(ctx) * 16 + (c >= 'a' ? c - 'a' + 10 : c - '0');
};
-const auto urlEncodedChar = x3::rule<class urlEncodedChar, unsigned>{"urlEncodedChar"} = x3::lit('%')[set_zero] >> x3::xdigit[add] >> x3::xdigit[add];
+const auto percentEncodedChar = x3::rule<class percentEncodedChar, unsigned>{"percentEncodedChar"} = x3::lit('%')[set_zero] >> x3::xdigit[add] >> x3::xdigit[add];
/* reserved characters according to RFC 3986, sec. 2.2 with '%' added. The '%' character is not specified as reserved but it effectively is because
* "Percent sign serves as the indicator for percent-encoded octets, it must be percent-encoded (...)" [RFC 3986, sec. 2.4]. */
const auto reservedChars = x3::lit(':') | '/' | '?' | '#' | '[' | ']' | '@' | '!' | '$' | '&' | '\'' | '(' | ')' | '*' | '+' | ',' | ',' | ';' | '=' | '%';
-const auto keyValue = x3::rule<class keyValue, std::string>{"keyValue"} = *(urlEncodedChar | (x3::char_ - reservedChars));
+const auto keyValue = x3::rule<class keyValue, std::string>{"keyValue"} = *(percentEncodedChar | (x3::char_ - reservedChars));
const auto keyList = x3::rule<class keyList, std::vector<std::string>>{"keyList"} = keyValue % ',';
const auto identifier = x3::rule<class apiIdentifier, std::string>{"identifier"} = (x3::alpha | x3::char_('_')) >> *(x3::alnum | x3::char_('_') | x3::char_('-') | x3::char_('.'));
@@ -117,7 +117,7 @@ const auto dateAndTime = x3::rule<class dateAndTime, std::string>{"dateAndTime"}
x3::repeat(4)[x3::digit] >> x3::char_('-') >> x3::repeat(2)[x3::digit] >> x3::char_('-') >> x3::repeat(2)[x3::digit] >> x3::char_('T') >>
x3::repeat(2)[x3::digit] >> x3::char_(':') >> x3::repeat(2)[x3::digit] >> x3::char_(':') >> x3::repeat(2)[x3::digit] >> -(x3::char_('.') >> +x3::digit) >>
(x3::char_('Z') | (-(x3::char_('+')|x3::char_('-')) >> x3::repeat(2)[x3::digit] >> x3::char_(':') >> x3::repeat(2)[x3::digit]));
-const auto filter = x3::rule<class filter, std::string>{"filter"} = +(urlEncodedChar | (x3::char_ - '&'));
+const auto filter = x3::rule<class filter, std::string>{"filter"} = +(percentEncodedChar | (x3::char_ - '&'));
const auto depthParam = x3::rule<class depthParam, queryParams::QueryParamValue>{"depthParam"} = x3::uint_[validDepthValues] | (x3::string("unbounded") >> x3::attr(queryParams::UnboundedDepth{}));
const auto queryParamPair = x3::rule<class queryParamPair, std::pair<std::string, queryParams::QueryParamValue>>{"queryParamPair"} =
(x3::string("depth") >> "=" >> depthParam) |
--
2.43.0
package/rousette/0003-uri-correct-x3-rule-class-names.patch
+34
View File
@@ -0,0 +1,34 @@
From 8a233202647f24538f2bbb8fff1e38d52e3599a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Wed, 16 Oct 2024 11:05:09 +0200
Subject: [PATCH 03/20] uri: correct x3 rule class names
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
Fixes: e06d5bf ("server: parser for data resources in URI paths")
Change-Id: Ic953e568d841032113ede1c0e896574361c0ebe2
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
src/restconf/uri.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/restconf/uri.cpp b/src/restconf/uri.cpp
index b144d92..8e8dc23 100644
--- a/src/restconf/uri.cpp
+++ b/src/restconf/uri.cpp
@@ -37,8 +37,8 @@ const auto reservedChars = x3::lit(':') | '/' | '?' | '#' | '[' | ']' | '@' | '!
const auto keyValue = x3::rule<class keyValue, std::string>{"keyValue"} = *(percentEncodedChar | (x3::char_ - reservedChars));
const auto keyList = x3::rule<class keyList, std::vector<std::string>>{"keyList"} = keyValue % ',';
-const auto identifier = x3::rule<class apiIdentifier, std::string>{"identifier"} = (x3::alpha | x3::char_('_')) >> *(x3::alnum | x3::char_('_') | x3::char_('-') | x3::char_('.'));
-const auto apiIdentifier = x3::rule<class identifier, ApiIdentifier>{"apiIdentifier"} = -(identifier >> ':') >> identifier;
+const auto identifier = x3::rule<class identifier, std::string>{"identifier"} = (x3::alpha | x3::char_('_')) >> *(x3::alnum | x3::char_('_') | x3::char_('-') | x3::char_('.'));
+const auto apiIdentifier = x3::rule<class apiIdentifier, ApiIdentifier>{"apiIdentifier"} = -(identifier >> ':') >> identifier;
const auto listInstance = x3::rule<class keyList, PathSegment>{"listInstance"} = apiIdentifier >> -('=' >> keyList);
const auto fullyQualifiedApiIdentifier = x3::rule<class identifier, ApiIdentifier>{"apiIdentifier"} = identifier >> ':' >> identifier;
const auto fullyQualifiedListInstance = x3::rule<class keyList, PathSegment>{"listInstance"} = fullyQualifiedApiIdentifier >> -('=' >> keyList);
--
2.43.0
package/rousette/0004-restconf-parser-should-work-on-raw-percent-encoded-p.patch
+153
View File
@@ -0,0 +1,153 @@
From 96cbf730010ee9539d05d0d72697dc960b3a938c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Mon, 7 Oct 2024 20:46:24 +0200
Subject: [PATCH 04/20] restconf: parser should work on raw percent-encoded
paths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
The difference between nghttp2's uri_ref::path and raw_path is that the
raw_path keeps the percent encoding, while the path converts the percent
encoded chars to their "normal" form.
Our parser expects some parts of the URI to be percent encoded so we
have to use raw_paths everywhere.
I thought about rewriting the parser not to expect percent encoded
characters but that would bring some complications. For instance when
querying lists, the RESTCONF RFC specifies that every key value is
percent encoded and individual key values are delimited by commas [1].
So, when somebody sends a request like /restconf/data/list=a%2Cb,c the
"normal" form is /restconf/data/list=a,b,c and in that case we obtain
three keys, but the client sent only two, where the first one contained
comma.
I am adding few tests to check for percent encoded values.
We realized this while working on one of the reported bugs [1]. The
query sent by the client there is wrong, the ':' char should be
percent-encoded.
[1] https://github.com/CESNET/rousette/issues/12
Bug: https://github.com/CESNET/rousette/issues/12
Change-Id: I473501cef3c8eae9af0c5d0751393cdad647e23c
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
src/restconf/Server.cpp | 8 ++++----
src/restconf/uri.cpp | 4 ++--
tests/restconf-reading.cpp | 15 +++++++++++++++
tests/uri-parser.cpp | 5 +++--
4 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/src/restconf/Server.cpp b/src/restconf/Server.cpp
index 5f560ed..79d8ff6 100644
--- a/src/restconf/Server.cpp
+++ b/src/restconf/Server.cpp
@@ -416,7 +416,7 @@ void processActionOrRPC(std::shared_ptr<RequestContext> requestCtx, const std::c
* - The data node exists but might get deleted right after this check: Sysrepo throws an error when this happens.
* - The data node does not exist but might get created right after this check: The node was not there when the request was issues so it should not be a problem
*/
- auto [pathToParent, pathSegment] = asLibyangPathSplit(ctx, requestCtx->req.uri().path);
+ auto [pathToParent, pathSegment] = asLibyangPathSplit(ctx, requestCtx->req.uri().raw_path);
if (!requestCtx->sess.getData(pathToParent, 0, sysrepo::GetOptions::Default, timeout)) {
throw ErrorResponse(400, "application", "operation-failed", "Action data node '" + requestCtx->restconfRequest.path + "' does not exist.");
}
@@ -539,7 +539,7 @@ void processYangPatchEdit(const std::shared_ptr<RequestContext>& requestCtx, con
auto target = childLeafValue(editContainer, "target");
auto operation = childLeafValue(editContainer, "operation");
- auto [singleEdit, replacementNode] = createEditForPutAndPatch(ctx, requestCtx->req.uri().path + target, yangPatchValueAsJSON(editContainer), libyang::DataFormat::JSON);
+ auto [singleEdit, replacementNode] = createEditForPutAndPatch(ctx, requestCtx->req.uri().raw_path + target, yangPatchValueAsJSON(editContainer), libyang::DataFormat::JSON);
validateInputMetaAttributes(ctx, *singleEdit);
// insert and move are not defined in RFC6241. sec 7.3 and sysrepo does not support them directly
@@ -658,7 +658,7 @@ void processPutOrPlainPatch(std::shared_ptr<RequestContext> requestCtx, const st
throw ErrorResponse(400, "protocol", "invalid-value", "Target resource does not exist");
}
- auto [edit, replacementNode] = createEditForPutAndPatch(ctx, requestCtx->req.uri().path, requestCtx->payload, *requestCtx->dataFormat.request /* caller checks if the dataFormat.request is present */);
+ auto [edit, replacementNode] = createEditForPutAndPatch(ctx, requestCtx->req.uri().raw_path, requestCtx->payload, *requestCtx->dataFormat.request /* caller checks if the dataFormat.request is present */);
validateInputMetaAttributes(ctx, *edit);
if (requestCtx->req.method() == "PUT") {
@@ -954,7 +954,7 @@ Server::Server(sysrepo::Connection conn, const std::string& address, const std::
dataFormat = chooseDataEncoding(req.header());
authorizeRequest(nacm, sess, req);
- auto restconfRequest = asRestconfRequest(sess.getContext(), req.method(), req.uri().path, req.uri().raw_query);
+ auto restconfRequest = asRestconfRequest(sess.getContext(), req.method(), req.uri().raw_path, req.uri().raw_query);
switch (restconfRequest.type) {
case RestconfRequest::Type::RestconfRoot:
diff --git a/src/restconf/uri.cpp b/src/restconf/uri.cpp
index 8e8dc23..ac399b7 100644
--- a/src/restconf/uri.cpp
+++ b/src/restconf/uri.cpp
@@ -34,9 +34,9 @@ const auto percentEncodedChar = x3::rule<class percentEncodedChar, unsigned>{"pe
/* reserved characters according to RFC 3986, sec. 2.2 with '%' added. The '%' character is not specified as reserved but it effectively is because
* "Percent sign serves as the indicator for percent-encoded octets, it must be percent-encoded (...)" [RFC 3986, sec. 2.4]. */
const auto reservedChars = x3::lit(':') | '/' | '?' | '#' | '[' | ']' | '@' | '!' | '$' | '&' | '\'' | '(' | ')' | '*' | '+' | ',' | ',' | ';' | '=' | '%';
-const auto keyValue = x3::rule<class keyValue, std::string>{"keyValue"} = *(percentEncodedChar | (x3::char_ - reservedChars));
+const auto percentEncodedString = x3::rule<class percentEncodedString, std::string>{"percentEncodedString"} = *(percentEncodedChar | (x3::char_ - reservedChars));
-const auto keyList = x3::rule<class keyList, std::vector<std::string>>{"keyList"} = keyValue % ',';
+const auto keyList = x3::rule<class keyList, std::vector<std::string>>{"keyList"} = percentEncodedString % ',';
const auto identifier = x3::rule<class identifier, std::string>{"identifier"} = (x3::alpha | x3::char_('_')) >> *(x3::alnum | x3::char_('_') | x3::char_('-') | x3::char_('.'));
const auto apiIdentifier = x3::rule<class apiIdentifier, ApiIdentifier>{"apiIdentifier"} = -(identifier >> ':') >> identifier;
const auto listInstance = x3::rule<class keyList, PathSegment>{"listInstance"} = apiIdentifier >> -('=' >> keyList);
diff --git a/tests/restconf-reading.cpp b/tests/restconf-reading.cpp
index fa1cbcc..2898839 100644
--- a/tests/restconf-reading.cpp
+++ b/tests/restconf-reading.cpp
@@ -261,6 +261,21 @@ TEST_CASE("reading data")
}
)"});
+ // percent-encoded comma is a part of the key value, it is not a delimiter
+ REQUIRE(get(RESTCONF_DATA_ROOT "/ietf-system:system/radius/server=a%2Cb", {AUTH_DWDM}) == Response{404, jsonHeaders, R"({
+ "ietf-restconf:errors": {
+ "error": [
+ {
+ "error-type": "application",
+ "error-tag": "invalid-value",
+ "error-message": "No data from sysrepo."
+ }
+ ]
+ }
+}
+)"});
+
+ // comma is a delimiter of list key values
REQUIRE(get(RESTCONF_DATA_ROOT "/ietf-system:system/radius/server=a,b", {AUTH_DWDM}) == Response{400, jsonHeaders, R"({
"ietf-restconf:errors": {
"error": [
diff --git a/tests/uri-parser.cpp b/tests/uri-parser.cpp
index a748e09..5977afc 100644
--- a/tests/uri-parser.cpp
+++ b/tests/uri-parser.cpp
@@ -158,9 +158,9 @@ TEST_CASE("URI path parser")
{{"prefix", "lst"}, {"key1"}},
{{"prefix", "leaf"}},
}}},
- {"/restconf/data/foo:bar/lst=key1,,key3", {{
+ {"/restconf/data/foo:bar/lst=module%3Akey1,,key3", {{
{{"foo", "bar"}},
- {{"lst"}, {"key1", "", "key3"}},
+ {{"lst"}, {"module:key1", "", "key3"}},
}}},
{"/restconf/data/foo:bar/lst=key%2CWithCommas,,key2C", {{
{{"foo", "bar"}},
@@ -240,6 +240,7 @@ TEST_CASE("URI path parser")
"/restconf/data/foo:list=A%2",
"/restconf/data/foo:list=A%2,",
"/restconf/data/foo:bar/list1=%%",
+ "/restconf/data/foo:bar/list1=module:smth",
"/restconf/data/foo:bar/",
"/restconf/data/ foo : bar",
"/rest conf/data / foo:bar",
--
2.43.0
package/rousette/0005-restconf-list-key-values-checking-must-respect-libya.patch
+428
View File
@@ -0,0 +1,428 @@
From 8b13c1e4ccaa61a241674c27063439e257fa88de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Wed, 2 Oct 2024 20:21:28 +0200
Subject: [PATCH 05/20] restconf: list key values checking must respect
libyang's canonicalization
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
While dealing with the issue that was fixed in the previous commit we
realized that the issue is deeper. Not only that our parser rejected
the input when someone used identityref with module prefix in the URI,
but also, our internal code for creating/modifying list entries was
wrong.
In PUT requests we have to check if user entered the same key value in
URI and yang-data payload. However, some values are canonicalized by
libyang (e.g. decimal64 type with fraction-digits=2 or even the
identityrefs) and so if the client entered two different but
canonically equivalent values, we would reject such request.
Bug: https://github.com/CESNET/rousette/issues/12
Change-Id: I44245d831e8de6d0e6f991fcd18319c095b49b1d
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
CMakeLists.txt | 3 +-
src/restconf/Server.cpp | 49 +++++++++++++++++----
src/restconf/utils/yang.cpp | 5 +++
src/restconf/utils/yang.h | 1 +
tests/restconf-reading.cpp | 59 +++++++++++++++++++++++++
tests/restconf-writing.cpp | 82 +++++++++++++++++++++++++++++++++++
tests/uri-parser.cpp | 2 +
tests/yang/example-types.yang | 13 ++++++
tests/yang/example.yang | 25 +++++++++++
9 files changed, 229 insertions(+), 10 deletions(-)
create mode 100644 tests/yang/example-types.yang
diff --git a/CMakeLists.txt b/CMakeLists.txt
index c563dcf..465bef9 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -204,7 +204,8 @@ if(BUILD_TESTING)
--install ${CMAKE_CURRENT_SOURCE_DIR}/tests/yang/example.yang --enable-feature f1
--install ${CMAKE_CURRENT_SOURCE_DIR}/tests/yang/example-delete.yang
--install ${CMAKE_CURRENT_SOURCE_DIR}/tests/yang/example-augment.yang
- --install ${CMAKE_CURRENT_SOURCE_DIR}/tests/yang/example-notif.yang)
+ --install ${CMAKE_CURRENT_SOURCE_DIR}/tests/yang/example-notif.yang
+ --install ${CMAKE_CURRENT_SOURCE_DIR}/tests/yang/example-types.yang)
rousette_test(NAME restconf-reading LIBRARIES rousette-restconf FIXTURE common-models WRAP_PAM)
rousette_test(NAME restconf-writing LIBRARIES rousette-restconf FIXTURE common-models WRAP_PAM)
rousette_test(NAME restconf-delete LIBRARIES rousette-restconf FIXTURE common-models WRAP_PAM)
diff --git a/src/restconf/Server.cpp b/src/restconf/Server.cpp
index 79d8ff6..b515d66 100644
--- a/src/restconf/Server.cpp
+++ b/src/restconf/Server.cpp
@@ -154,6 +154,15 @@ auto rejectYangPatch(const std::string& patchId, const std::string& editId)
};
}
+/** @brief Check if these two paths compare the same after path canonicalization */
+bool compareKeyValue(const libyang::Context& ctx, const std::string& pathA, const std::string& pathB)
+{
+ auto [parentA, nodeA] = ctx.newPath2(pathA, std::nullopt);
+ auto [parentB, nodeB] = ctx.newPath2(pathB, std::nullopt);
+
+ return nodeA->asTerm().valueStr() == nodeB->asTerm().valueStr();
+}
+
struct KeyMismatch {
libyang::DataNode offendingNode;
std::optional<std::string> uriKeyValue;
@@ -169,25 +178,46 @@ struct KeyMismatch {
/** @brief In case node is a (leaf-)list check if the key values are the same as the keys specified in the lastPathSegment.
* @return The node where the mismatch occurs */
-std::optional<KeyMismatch> checkKeysMismatch(const libyang::DataNode& node, const PathSegment& lastPathSegment)
+std::optional<KeyMismatch> checkKeysMismatch(libyang::Context& ctx, const libyang::DataNode& node, const std::string& lyParentPath, const PathSegment& lastPathSegment)
{
+ const auto pathPrefix = (lyParentPath.empty() ? "" : lyParentPath) + "/" + lastPathSegment.apiIdent.name();
+
if (node.schema().nodeType() == libyang::NodeType::List) {
const auto& listKeys = node.schema().asList().keys();
for (size_t i = 0; i < listKeys.size(); ++i) {
- const auto& keyValueURI = lastPathSegment.keys[i];
auto keyNodeData = node.findPath(listKeys[i].module().name() + ':' + listKeys[i].name());
if (!keyNodeData) {
return KeyMismatch{node, std::nullopt};
}
- const auto& keyValueData = keyNodeData->asTerm().valueStr();
-
- if (keyValueURI != keyValueData) {
- return KeyMismatch{*keyNodeData, keyValueURI};
+ /*
+ * If the key's value has a canonical form then libyang makes the value canonical
+ * but there is no guarantee that the client provided the value in the canonical form.
+ *
+ * Let libyang do the work. Create two data nodes, one with the key value from the data and the other
+ * with the key value from the URI. Then compare the values from the two nodes. If they are different,
+ * they certainly mismatch.
+ *
+ * This can happen in cases like
+ * * The key's type is identityref and the client provided the key value as a string without the module name. Libyang will canonicalize the value by adding the module name.
+ * * The key's type is decimal64 with fractional-digits 2; then the client can provide the value as 1.0 or 1.00 and they should be the same. Libyang will canonicalize the value.
+ */
+
+ auto keysWithValueFromData = lastPathSegment.keys;
+ keysWithValueFromData[i] = keyNodeData->asTerm().valueStr();
+
+ const auto suffix = "/" + listKeys[i].name();
+ const auto pathFromData = pathPrefix + listKeyPredicate(listKeys, keysWithValueFromData) + suffix;
+ const auto pathFromURI = pathPrefix + listKeyPredicate(listKeys, lastPathSegment.keys) + suffix;
+
+ if (!compareKeyValue(ctx, pathFromData, pathFromURI)) {
+ return KeyMismatch{*keyNodeData, lastPathSegment.keys[i]};
}
}
} else if (node.schema().nodeType() == libyang::NodeType::Leaflist) {
- if (lastPathSegment.keys[0] != node.asTerm().valueStr()) {
+ const auto pathFromData = pathPrefix + leaflistKeyPredicate(node.asTerm().valueStr());
+ const auto pathFromURI = pathPrefix + leaflistKeyPredicate(lastPathSegment.keys[0]);
+ if (!compareKeyValue(ctx, pathFromData, pathFromURI)) {
return KeyMismatch{node, lastPathSegment.keys[0]};
}
}
@@ -363,7 +393,7 @@ libyang::CreatedNodes createEditForPutAndPatch(libyang::Context& ctx, const std:
if (isSameNode(child, lastPathSegment)) {
// 1) a single child that is created by parseSubtree(), its name is the same as `lastPathSegment`.
// It could be a list; then we need to check if the keys in provided data match the keys in URI.
- if (auto keyMismatch = checkKeysMismatch(child, lastPathSegment)) {
+ if (auto keyMismatch = checkKeysMismatch(ctx, child, lyParentPath, lastPathSegment)) {
throw ErrorResponse(400, "protocol", "invalid-value", keyMismatch->message(), keyMismatch->offendingNode.path());
}
replacementNode = child;
@@ -386,7 +416,8 @@ libyang::CreatedNodes createEditForPutAndPatch(libyang::Context& ctx, const std:
if (!isSameNode(*replacementNode, lastPathSegment)) {
throw ErrorResponse(400, "protocol", "invalid-value", "Data contains invalid node.", replacementNode->path());
}
- if (auto keyMismatch = checkKeysMismatch(*parent, lastPathSegment)) {
+
+ if (auto keyMismatch = checkKeysMismatch(ctx, *parent, lyParentPath, lastPathSegment)) {
throw ErrorResponse(400, "protocol", "invalid-value", keyMismatch->message(), keyMismatch->offendingNode.path());
}
}
diff --git a/src/restconf/utils/yang.cpp b/src/restconf/utils/yang.cpp
index 15cceb6..4c4d619 100644
--- a/src/restconf/utils/yang.cpp
+++ b/src/restconf/utils/yang.cpp
@@ -50,6 +50,11 @@ std::string listKeyPredicate(const std::vector<libyang::Leaf>& listKeyLeafs, con
return res;
}
+std::string leaflistKeyPredicate(const std::string& keyValue)
+{
+ return "[.=" + escapeListKey(keyValue) + ']';
+}
+
bool isUserOrderedList(const libyang::DataNode& node)
{
if (node.schema().nodeType() == libyang::NodeType::List) {
diff --git a/src/restconf/utils/yang.h b/src/restconf/utils/yang.h
index 677d049..e91ba8a 100644
--- a/src/restconf/utils/yang.h
+++ b/src/restconf/utils/yang.h
@@ -16,6 +16,7 @@ namespace rousette::restconf {
std::string escapeListKey(const std::string& str);
std::string listKeyPredicate(const std::vector<libyang::Leaf>& listKeyLeafs, const std::vector<std::string>& keyValues);
+std::string leaflistKeyPredicate(const std::string& keyValue);
bool isUserOrderedList(const libyang::DataNode& node);
bool isKeyNode(const libyang::DataNode& maybeList, const libyang::DataNode& node);
}
diff --git a/tests/restconf-reading.cpp b/tests/restconf-reading.cpp
index 2898839..96c38ab 100644
--- a/tests/restconf-reading.cpp
+++ b/tests/restconf-reading.cpp
@@ -287,6 +287,65 @@ TEST_CASE("reading data")
]
}
}
+)"});
+
+ srSess.setItem("/example:list-with-identity-key[type='example:derived-identity'][name='name']", std::nullopt);
+ srSess.setItem("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']", std::nullopt);
+ srSess.setItem("/example:tlc/decimal-list[.='1.00']", std::nullopt);
+ srSess.applyChanges();
+
+ // dealing with keys which can have prefixes (YANG identities)
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-identity-key=derived-identity,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
+ "example:list-with-identity-key": [
+ {
+ "type": "derived-identity",
+ "name": "name"
+ }
+ ]
+}
+)"});
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example%3Aderived-identity,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
+ "example:list-with-identity-key": [
+ {
+ "type": "derived-identity",
+ "name": "name"
+ }
+ ]
+}
+)"});
+
+ // an identity from another module must be namespace-qualified
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-identity-key=another-derived-identity,name", {AUTH_ROOT}) == Response{404, jsonHeaders, R"({
+ "ietf-restconf:errors": {
+ "error": [
+ {
+ "error-type": "application",
+ "error-tag": "invalid-value",
+ "error-message": "No data from sysrepo."
+ }
+ ]
+ }
+}
+)"});
+
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example-types%3Aanother-derived-identity,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
+ "example:list-with-identity-key": [
+ {
+ "type": "example-types:another-derived-identity",
+ "name": "name"
+ }
+ ]
+}
+)"});
+
+ // test canonicalization of list key values; the key value was inserted as "1.00"
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:tlc/decimal-list=1", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
+ "example:tlc": {
+ "decimal-list": [
+ "1.0"
+ ]
+ }
+}
)"});
}
diff --git a/tests/restconf-writing.cpp b/tests/restconf-writing.cpp
index 96dbb25..8418554 100644
--- a/tests/restconf-writing.cpp
+++ b/tests/restconf-writing.cpp
@@ -389,6 +389,88 @@ TEST_CASE("writing data")
REQUIRE(put(RESTCONF_DATA_ROOT "/example:tlc/list=libyang/nested=11,12,13", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:nested": [{"first": "11", "second": 12, "third": "13"}]}]})") == Response{201, noContentTypeHeaders, ""});
}
+ SECTION("Test canonicalization of keys")
+ {
+ EXPECT_CHANGE(
+ CREATED("/example:list-with-identity-key[type='example:derived-identity'][name='name']", std::nullopt),
+ CREATED("/example:list-with-identity-key[type='example:derived-identity'][name='name']/type", "example:derived-identity"),
+ CREATED("/example:list-with-identity-key[type='example:derived-identity'][name='name']/name", "name"),
+ CREATED("/example:list-with-identity-key[type='example:derived-identity'][name='name']/text", "blabla"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "derived-identity", "text": "blabla"}]}]})") == Response{201, noContentTypeHeaders, ""});
+
+ // prefixed in the URI, not prefixed in the data
+ EXPECT_CHANGE(
+ MODIFIED("/example:list-with-identity-key[type='example:derived-identity'][name='name']/text", "hehe"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example%3Aderived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "derived-identity", "text": "hehe"}]}]})") == Response{204, noContentTypeHeaders, ""});
+
+
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=another-derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "another-derived-identity", "text": "blabla"}]}]})") == Response{400, jsonHeaders, R"({
+ "ietf-restconf:errors": {
+ "error": [
+ {
+ "error-type": "protocol",
+ "error-tag": "invalid-value",
+ "error-message": "Validation failure: Can't parse data: LY_EVALID"
+ }
+ ]
+ }
+}
+)"});
+
+ EXPECT_CHANGE(
+ CREATED("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']", std::nullopt),
+ CREATED("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']/type", "example-types:another-derived-identity"),
+ CREATED("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']/name", "name"),
+ CREATED("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']/text", "blabla"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example-types%3Aanother-derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "example-types:another-derived-identity", "text": "blabla"}]}]})") == Response{201, noContentTypeHeaders, ""});
+
+ // missing namespace in the data
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example-types%3Aanother-derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "another-derived-identity", "text": "blabla"}]}]})") == Response{400, jsonHeaders, R"({
+ "ietf-restconf:errors": {
+ "error": [
+ {
+ "error-type": "protocol",
+ "error-tag": "invalid-value",
+ "error-message": "Validation failure: Can't parse data: LY_EVALID"
+ }
+ ]
+ }
+}
+)"});
+
+ EXPECT_CHANGE(CREATED("/example:leaf-list-with-identity-key[.='example-types:another-derived-identity']", "example-types:another-derived-identity"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:leaf-list-with-identity-key=example-types%3Aanother-derived-identity", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:leaf-list-with-identity-key": ["example-types:another-derived-identity"]})") == Response{201, noContentTypeHeaders, ""});
+
+ // missing namespace in the URI
+ EXPECT_CHANGE(CREATED("/example:leaf-list-with-identity-key[.='example:derived-identity']", "example:derived-identity"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:leaf-list-with-identity-key=derived-identity", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:leaf-list-with-identity-key": ["example:derived-identity"]})") == Response{201, noContentTypeHeaders, ""});
+
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:leaf-list-with-identity-key=example-types%3Aanother-derived-identity", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:leaf-list-with-identity-key": ["example:derived-identity"]})") == Response{400, jsonHeaders, R"({
+ "ietf-restconf:errors": {
+ "error": [
+ {
+ "error-type": "protocol",
+ "error-tag": "invalid-value",
+ "error-path": "/example:leaf-list-with-identity-key[.='example:derived-identity']",
+ "error-message": "List key mismatch between URI path ('example-types:another-derived-identity') and data ('example:derived-identity')."
+ }
+ ]
+ }
+}
+)"});
+
+ // value in the URI and in the data have the same canonical form
+ EXPECT_CHANGE(CREATED("/example:tlc/decimal-list[.='1.0']", "1.0"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:tlc/decimal-list=1.00", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:decimal-list": ["1.0"]})") == Response{201, noContentTypeHeaders, ""});
+
+ // nothing is changed, still the same value
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:tlc/decimal-list=1.000", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:decimal-list": ["1"]})") == Response{204, noContentTypeHeaders, ""});
+
+ // different value
+ EXPECT_CHANGE(CREATED("/example:tlc/decimal-list[.='1.01']", "1.01"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:tlc/decimal-list=1.010", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:decimal-list": ["1.01"]})") == Response{201, noContentTypeHeaders, ""});
+ }
+
SECTION("Modify a leaf in a list entry")
{
EXPECT_CHANGE(MODIFIED("/example:tlc/list[name='libyang']/choice1", "restconf"));
diff --git a/tests/uri-parser.cpp b/tests/uri-parser.cpp
index 5977afc..7320c3c 100644
--- a/tests/uri-parser.cpp
+++ b/tests/uri-parser.cpp
@@ -293,6 +293,7 @@ TEST_CASE("URI path parser")
{"/restconf/data/example:tlc/list=eth0/choice1", "/example:tlc/list[name='eth0']/choice1", std::nullopt},
{"/restconf/data/example:tlc/list=eth0/choice2", "/example:tlc/list[name='eth0']/choice2", std::nullopt},
{"/restconf/data/example:tlc/list=eth0/collection=val", "/example:tlc/list[name='eth0']/collection[.='val']", std::nullopt},
+ {"/restconf/data/example:list-with-identity-key=example-types%3Aanother-derived-identity,aaa", "/example:list-with-identity-key[type='example-types:another-derived-identity'][name='aaa']", std::nullopt},
{"/restconf/data/example:tlc/status", "/example:tlc/status", std::nullopt},
// container example:a has a container b inserted locally and also via an augment. Check that we return the correct one
{"/restconf/data/example:a/b", "/example:a/b", std::nullopt},
@@ -327,6 +328,7 @@ TEST_CASE("URI path parser")
{"/restconf/data/example:tlc/status", "/example:tlc", {{"example", "status"}}},
{"/restconf/data/example:a/example-augment:b/c", "/example:a/example-augment:b", {{"example-augment", "c"}}},
{"/restconf/ds/ietf-datastores:startup/example:a/example-augment:b/c", "/example:a/example-augment:b", {{"example-augment", "c"}}},
+ {"/restconf/data/example:list-with-identity-key=example-types%3Aanother-derived-identity,aaa", "", {{"example", "list-with-identity-key"}, {"example-types:another-derived-identity", "aaa"}}},
}) {
CAPTURE(httpMethod);
CAPTURE(expectedRequestType);
diff --git a/tests/yang/example-types.yang b/tests/yang/example-types.yang
new file mode 100644
index 0000000..5bc2fb0
--- /dev/null
+++ b/tests/yang/example-types.yang
@@ -0,0 +1,13 @@
+module example-types {
+ yang-version 1.1;
+ namespace "http://example.tld/example-types";
+ prefix ex-types;
+
+ import example {
+ prefix ex;
+ }
+
+ identity another-derived-identity {
+ base ex:base-identity;
+ }
+}
diff --git a/tests/yang/example.yang b/tests/yang/example.yang
index df1301f..c46273c 100644
--- a/tests/yang/example.yang
+++ b/tests/yang/example.yang
@@ -6,6 +6,13 @@ module example {
feature f1 { }
feature f2 { }
+ identity base-identity {
+ }
+
+ identity derived-identity {
+ base base-identity;
+ }
+
leaf top-level-leaf { type string; }
leaf top-level-leaf2 { type string; default "x"; }
@@ -50,6 +57,11 @@ module example {
config false;
leaf name { type string; }
}
+ leaf-list decimal-list {
+ type decimal64 {
+ fraction-digits 2;
+ }
+ }
leaf status {
type enumeration {
enum on { }
@@ -109,6 +121,19 @@ module example {
}
}
+ list list-with-identity-key {
+ key "type name";
+ leaf type {
+ type identityref { base base-identity; }
+ }
+ leaf name { type string; }
+ leaf text { type string; }
+ }
+
+ leaf-list leaf-list-with-identity-key {
+ type identityref { base base-identity; }
+ }
+
rpc test-rpc {
input {
leaf i {
--
2.43.0
package/rousette/0006-tests-test-querying-lists-with-union-keys.patch
+300
View File
@@ -0,0 +1,300 @@
From fda47b6a6cfdaecc24e96c4d6138c6de3ef116e0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Mon, 7 Oct 2024 21:21:22 +0200
Subject: [PATCH 06/20] tests: test querying lists with union keys
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
Test that we correctly work with keys that are unions of something
that can have a module namespace and that must not have it.
By the way, enum values are not supposed to have a namespace prefix,
good to know [1].
[1] https://www.rfc-editor.org/rfc/rfc7951#section-6.4
Change-Id: I5a70f18117bb453330b4bb2ce0d2fb47d35b6ea6
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
tests/restconf-reading.cpp | 53 ++++++++++++++++++++++++-----
tests/restconf-writing.cpp | 68 ++++++++++++++++++++++++++++----------
tests/uri-parser.cpp | 2 +-
tests/yang/example.yang | 26 +++++++++++++--
4 files changed, 120 insertions(+), 29 deletions(-)
diff --git a/tests/restconf-reading.cpp b/tests/restconf-reading.cpp
index 96c38ab..2ded3f0 100644
--- a/tests/restconf-reading.cpp
+++ b/tests/restconf-reading.cpp
@@ -289,14 +289,16 @@ TEST_CASE("reading data")
}
)"});
- srSess.setItem("/example:list-with-identity-key[type='example:derived-identity'][name='name']", std::nullopt);
- srSess.setItem("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']", std::nullopt);
+ srSess.setItem("/example:list-with-union-keys[type='example:derived-identity'][name='name']", std::nullopt);
+ srSess.setItem("/example:list-with-union-keys[type='example-types:another-derived-identity'][name='name']", std::nullopt);
+ srSess.setItem("/example:list-with-union-keys[type='fiii'][name='name']", std::nullopt);
+ srSess.setItem("/example:list-with-union-keys[type='zero'][name='name']", std::nullopt); // enum value
srSess.setItem("/example:tlc/decimal-list[.='1.00']", std::nullopt);
srSess.applyChanges();
// dealing with keys which can have prefixes (YANG identities)
- REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-identity-key=derived-identity,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
- "example:list-with-identity-key": [
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-union-keys=derived-identity,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
+ "example:list-with-union-keys": [
{
"type": "derived-identity",
"name": "name"
@@ -304,8 +306,8 @@ TEST_CASE("reading data")
]
}
)"});
- REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example%3Aderived-identity,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
- "example:list-with-identity-key": [
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-union-keys=example%3Aderived-identity,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
+ "example:list-with-union-keys": [
{
"type": "derived-identity",
"name": "name"
@@ -315,7 +317,7 @@ TEST_CASE("reading data")
)"});
// an identity from another module must be namespace-qualified
- REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-identity-key=another-derived-identity,name", {AUTH_ROOT}) == Response{404, jsonHeaders, R"({
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-union-keys=another-derived-identity,name", {AUTH_ROOT}) == Response{404, jsonHeaders, R"({
"ietf-restconf:errors": {
"error": [
{
@@ -328,8 +330,8 @@ TEST_CASE("reading data")
}
)"});
- REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example-types%3Aanother-derived-identity,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
- "example:list-with-identity-key": [
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-union-keys=example-types%3Aanother-derived-identity,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
+ "example:list-with-union-keys": [
{
"type": "example-types:another-derived-identity",
"name": "name"
@@ -346,6 +348,39 @@ TEST_CASE("reading data")
]
}
}
+)"});
+
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-union-keys=zero,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
+ "example:list-with-union-keys": [
+ {
+ "type": "zero",
+ "name": "name"
+ }
+ ]
+}
+)"});
+
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-union-keys=example%3Azero,name", {AUTH_ROOT}) == Response{404, jsonHeaders, R"({
+ "ietf-restconf:errors": {
+ "error": [
+ {
+ "error-type": "application",
+ "error-tag": "invalid-value",
+ "error-message": "No data from sysrepo."
+ }
+ ]
+ }
+}
+)"});
+
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:list-with-union-keys=fiii,name", {AUTH_ROOT}) == Response{200, jsonHeaders, R"({
+ "example:list-with-union-keys": [
+ {
+ "type": "fiii",
+ "name": "name"
+ }
+ ]
+}
)"});
}
diff --git a/tests/restconf-writing.cpp b/tests/restconf-writing.cpp
index 8418554..c1a9515 100644
--- a/tests/restconf-writing.cpp
+++ b/tests/restconf-writing.cpp
@@ -392,46 +392,69 @@ TEST_CASE("writing data")
SECTION("Test canonicalization of keys")
{
EXPECT_CHANGE(
- CREATED("/example:list-with-identity-key[type='example:derived-identity'][name='name']", std::nullopt),
- CREATED("/example:list-with-identity-key[type='example:derived-identity'][name='name']/type", "example:derived-identity"),
- CREATED("/example:list-with-identity-key[type='example:derived-identity'][name='name']/name", "name"),
- CREATED("/example:list-with-identity-key[type='example:derived-identity'][name='name']/text", "blabla"));
- REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "derived-identity", "text": "blabla"}]}]})") == Response{201, noContentTypeHeaders, ""});
+ CREATED("/example:list-with-union-keys[type='example:derived-identity'][name='name']", std::nullopt),
+ CREATED("/example:list-with-union-keys[type='example:derived-identity'][name='name']/type", "example:derived-identity"),
+ CREATED("/example:list-with-union-keys[type='example:derived-identity'][name='name']/name", "name"),
+ CREATED("/example:list-with-union-keys[type='example:derived-identity'][name='name']/text", "blabla"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-union-keys=derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-union-keys": [{"name": "name", "type": "derived-identity", "text": "blabla"}]}]})") == Response{201, noContentTypeHeaders, ""});
// prefixed in the URI, not prefixed in the data
EXPECT_CHANGE(
- MODIFIED("/example:list-with-identity-key[type='example:derived-identity'][name='name']/text", "hehe"));
- REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example%3Aderived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "derived-identity", "text": "hehe"}]}]})") == Response{204, noContentTypeHeaders, ""});
+ MODIFIED("/example:list-with-union-keys[type='example:derived-identity'][name='name']/text", "hehe"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-union-keys=example%3Aderived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-union-keys": [{"name": "name", "type": "derived-identity", "text": "hehe"}]}]})") == Response{204, noContentTypeHeaders, ""});
+ // 'another-derived-identity' comes from a different module than the list itself, so this parses as string
+ EXPECT_CHANGE(
+ CREATED("/example:list-with-union-keys[type='another-derived-identity'][name='name']", std::nullopt),
+ CREATED("/example:list-with-union-keys[type='another-derived-identity'][name='name']/type", "another-derived-identity"),
+ CREATED("/example:list-with-union-keys[type='another-derived-identity'][name='name']/name", "name"),
+ CREATED("/example:list-with-union-keys[type='another-derived-identity'][name='name']/text", "blabla"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-union-keys=another-derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-union-keys": [{"name": "name", "type": "another-derived-identity", "text": "blabla"}]}]})") == Response{201, noContentTypeHeaders, ""});
+
+ EXPECT_CHANGE(
+ CREATED("/example:list-with-union-keys[type='example-types:another-derived-identity'][name='name']", std::nullopt),
+ CREATED("/example:list-with-union-keys[type='example-types:another-derived-identity'][name='name']/type", "example-types:another-derived-identity"),
+ CREATED("/example:list-with-union-keys[type='example-types:another-derived-identity'][name='name']/name", "name"),
+ CREATED("/example:list-with-union-keys[type='example-types:another-derived-identity'][name='name']/text", "blabla"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-union-keys=example-types%3Aanother-derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-union-keys": [{"name": "name", "type": "example-types:another-derived-identity", "text": "blabla"}]}]})") == Response{201, noContentTypeHeaders, ""});
- REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=another-derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "another-derived-identity", "text": "blabla"}]}]})") == Response{400, jsonHeaders, R"({
+ // missing namespace in the data
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-union-keys=example-types%3Aanother-derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-union-keys": [{"name": "name", "type": "another-derived-identity", "text": "blabla"}]}]})") == Response{400, jsonHeaders, R"({
"ietf-restconf:errors": {
"error": [
{
"error-type": "protocol",
"error-tag": "invalid-value",
- "error-message": "Validation failure: Can't parse data: LY_EVALID"
+ "error-path": "/example:list-with-union-keys[type='another-derived-identity'][name='name']/type",
+ "error-message": "List key mismatch between URI path ('example-types:another-derived-identity') and data ('another-derived-identity')."
}
]
}
}
)"});
+ // zero is enum value
EXPECT_CHANGE(
- CREATED("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']", std::nullopt),
- CREATED("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']/type", "example-types:another-derived-identity"),
- CREATED("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']/name", "name"),
- CREATED("/example:list-with-identity-key[type='example-types:another-derived-identity'][name='name']/text", "blabla"));
- REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example-types%3Aanother-derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "example-types:another-derived-identity", "text": "blabla"}]}]})") == Response{201, noContentTypeHeaders, ""});
+ CREATED("/example:list-with-union-keys[type='zero'][name='name']", std::nullopt),
+ CREATED("/example:list-with-union-keys[type='zero'][name='name']/type", "zero"),
+ CREATED("/example:list-with-union-keys[type='zero'][name='name']/name", "name"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-union-keys=zero,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-union-keys": [{"name": "name", "type": "zero"}]}]})") == Response{201, noContentTypeHeaders, ""});
- // missing namespace in the data
- REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-identity-key=example-types%3Aanother-derived-identity,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-identity-key": [{"name": "name", "type": "another-derived-identity", "text": "blabla"}]}]})") == Response{400, jsonHeaders, R"({
+ // example:zero is string, enum values are not namespace-prefixed
+ EXPECT_CHANGE(
+ CREATED("/example:list-with-union-keys[type='example:zero'][name='name']", std::nullopt),
+ CREATED("/example:list-with-union-keys[type='example:zero'][name='name']/type", "example:zero"),
+ CREATED("/example:list-with-union-keys[type='example:zero'][name='name']/name", "name"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-union-keys=example%3Azero,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-union-keys": [{"name": "name", "type": "example:zero"}]}]})") == Response{201, noContentTypeHeaders, ""});
+
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-union-keys=zero,name", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list-with-union-keys": [{"name": "name", "type": "example:zero"}]}]})") == Response{400, jsonHeaders, R"({
"ietf-restconf:errors": {
"error": [
{
"error-type": "protocol",
"error-tag": "invalid-value",
- "error-message": "Validation failure: Can't parse data: LY_EVALID"
+ "error-path": "/example:list-with-union-keys[type='example:zero'][name='name']/type",
+ "error-message": "List key mismatch between URI path ('zero') and data ('example:zero')."
}
]
}
@@ -459,6 +482,17 @@ TEST_CASE("writing data")
}
)"});
+ EXPECT_CHANGE(CREATED("/example:fruit-list[.='example:apple']", "example:apple"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:fruit-list=example%3Aapple", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:fruit-list": ["apple"]})") == Response{201, noContentTypeHeaders, ""});
+
+ // leafref
+ EXPECT_CHANGE(
+ CREATED("/example:list-with-union-keys[type='example:apple'][name='name']", std::nullopt),
+ CREATED("/example:list-with-union-keys[type='example:apple'][name='name']/type", "example:apple"),
+ CREATED("/example:list-with-union-keys[type='example:apple'][name='name']/name", "name"));
+ REQUIRE(put(RESTCONF_DATA_ROOT "/example:list-with-union-keys=example%3Aapple,name", {AUTH_ROOT, CONTENT_TYPE_JSON},
+ R"({"example:list-with-union-keys": [{"name": "name", "type": "apple"}]}]})") == Response{201, noContentTypeHeaders, ""});
+
// value in the URI and in the data have the same canonical form
EXPECT_CHANGE(CREATED("/example:tlc/decimal-list[.='1.0']", "1.0"));
REQUIRE(put(RESTCONF_DATA_ROOT "/example:tlc/decimal-list=1.00", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:decimal-list": ["1.0"]})") == Response{201, noContentTypeHeaders, ""});
diff --git a/tests/uri-parser.cpp b/tests/uri-parser.cpp
index 7320c3c..000fe5d 100644
--- a/tests/uri-parser.cpp
+++ b/tests/uri-parser.cpp
@@ -293,7 +293,7 @@ TEST_CASE("URI path parser")
{"/restconf/data/example:tlc/list=eth0/choice1", "/example:tlc/list[name='eth0']/choice1", std::nullopt},
{"/restconf/data/example:tlc/list=eth0/choice2", "/example:tlc/list[name='eth0']/choice2", std::nullopt},
{"/restconf/data/example:tlc/list=eth0/collection=val", "/example:tlc/list[name='eth0']/collection[.='val']", std::nullopt},
- {"/restconf/data/example:list-with-identity-key=example-types%3Aanother-derived-identity,aaa", "/example:list-with-identity-key[type='example-types:another-derived-identity'][name='aaa']", std::nullopt},
+ {"/restconf/data/example:list-with-union-keys=example-types%3Aanother-derived-identity,aaa", "/example:list-with-union-keys[type='example-types:another-derived-identity'][name='aaa']", std::nullopt},
{"/restconf/data/example:tlc/status", "/example:tlc/status", std::nullopt},
// container example:a has a container b inserted locally and also via an augment. Check that we return the correct one
{"/restconf/data/example:a/b", "/example:a/b", std::nullopt},
diff --git a/tests/yang/example.yang b/tests/yang/example.yang
index c46273c..75cd7a6 100644
--- a/tests/yang/example.yang
+++ b/tests/yang/example.yang
@@ -13,6 +13,11 @@ module example {
base base-identity;
}
+ identity fruit { }
+ identity apple {
+ base fruit;
+ }
+
leaf top-level-leaf { type string; }
leaf top-level-leaf2 { type string; default "x"; }
@@ -121,10 +126,23 @@ module example {
}
}
- list list-with-identity-key {
+ list list-with-union-keys {
key "type name";
leaf type {
- type identityref { base base-identity; }
+ type union {
+ type identityref {
+ base base-identity;
+ }
+ type enumeration {
+ enum zero;
+ enum one;
+ }
+ type leafref {
+ require-instance true;
+ path "/fruit-list";
+ }
+ type string;
+ }
}
leaf name { type string; }
leaf text { type string; }
@@ -134,6 +152,10 @@ module example {
type identityref { base base-identity; }
}
+ leaf-list fruit-list {
+ type identityref { base fruit; }
+ }
+
rpc test-rpc {
input {
leaf i {
--
2.43.0
package/rousette/0007-error-handling-in-sysrepo-has-changed.patch
+32
View File
@@ -0,0 +1,32 @@
From 27ef5bc87fdeb70a77609da6ec18ee5c28656bb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= <jan.kundrat@cesnet.cz>
Date: Tue, 29 Oct 2024 18:54:55 +0100
Subject: [PATCH 07/20] error handling in sysrepo has changed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
Depends-on: https://gerrit.cesnet.cz/c/CzechLight/dependencies/+/7969
Change-Id: Id028806ed49114cba4c55e2874bcf3fc98308bdc
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
tests/restconf-rpc.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/restconf-rpc.cpp b/tests/restconf-rpc.cpp
index 4f66f10..9bc1dbc 100644
--- a/tests/restconf-rpc.cpp
+++ b/tests/restconf-rpc.cpp
@@ -301,7 +301,7 @@ TEST_CASE("invoking actions and rpcs")
{
"error-type": "application",
"error-tag": "operation-failed",
- "error-message": "Internal server error due to sysrepo exception: Couldn't send RPC: SR_ERR_CALLBACK_FAILED\u000A Operation failed (SR_ERR_OPERATION_FAILED)\u000A User callback failed. (SR_ERR_CALLBACK_FAILED)"
+ "error-message": "Internal server error due to sysrepo exception: Couldn't send RPC: SR_ERR_OPERATION_FAILED\u000A Operation failed (SR_ERR_OPERATION_FAILED)"
}
]
}
--
2.43.0
package/rousette/0008-restconf-support-fields-query-parameter.patch
+721
View File
@@ -0,0 +1,721 @@
From 6819561d97e38569c319e36ca2e99768036b4032 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Wed, 21 Aug 2024 19:18:08 +0200
Subject: [PATCH 08/20] restconf: support fields query parameter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
This patch adds support for fields query parameter [1].
I had to modify the original grammar for fields parameter a bit to
allow for the lowest precedence while parsing `;` expression. Also we
allow for more strings than the original grammar specifies to make the
syntax more user friendly.
The fields expression is parsed into an AST which corresponds 1:1
to the parse tree. The tree representing the expression could be
simplified but I chose not to as it would complicate the code even
more (although the translation to XPath would be simpler).
The tree is then transformed into a valid XPath 1.0 expression.
The XPath 1.0 expressions are limited and I could not find a way how to
transform the fields string into a valid XPath. I realized that the
easiest way will be to "unwrap" the expression into individual paths
and join them via union operator.
For example, input `a(b;c/d)` would result into `a/b | a/c/d` XPath.
[1] https://datatracker.ietf.org/doc/html/rfc8040#section-4.8.3
[2] https://www.w3.org/TR/1999/REC-xpath-19991116/#section-Expressions
Change-Id: I3c96bbcf49b38ecf08f56912afd3a8f50c15cd44
Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
README.md | 1 -
src/restconf/Server.cpp | 9 ++-
src/restconf/uri.cpp | 95 +++++++++++++++++++++++-
src/restconf/uri.h | 57 ++++++++++++++-
src/restconf/uri_impl.h | 4 ++
tests/pretty_printers.h | 23 ++++++
tests/restconf-reading.cpp | 144 ++++++++++++++++++++++++++++++++++++-
tests/restconf-writing.cpp | 12 +++-
tests/uri-parser.cpp | 93 ++++++++++++++++++++++++
tests/yang/example.yang | 10 +++
10 files changed, 438 insertions(+), 10 deletions(-)
diff --git a/README.md b/README.md
index 1689584..3fbfd21 100644
--- a/README.md
+++ b/README.md
@@ -22,7 +22,6 @@ This is a [RESTCONF](https://datatracker.ietf.org/doc/html/rfc8040.html) server
- TLS certificate authentication (see [Access control model](#access-control-model) below)
- the [`Last-Modified`](https://datatracker.ietf.org/doc/html/rfc8040.html#section-3.4.1.1) and [`ETag`](https://datatracker.ietf.org/doc/html/rfc8040.html#section-3.4.1.2) headers for [edit collision prevention](https://datatracker.ietf.org/doc/html/rfc8040.html#section-3.4.1) in the datastore resource
- the [`Last-Modified`](https://datatracker.ietf.org/doc/html/rfc8040.html#section-3.5.1) and [`ETag`](https://datatracker.ietf.org/doc/html/rfc8040.html#section-3.5.2) headers in the data resource
- - The [`fields`](https://datatracker.ietf.org/doc/html/rfc8040.html#section-4.8.3) query parameter
- [NMDA](https://datatracker.ietf.org/doc/html/rfc8527.html) datastore access
- no [`with-operational-default`](https://datatracker.ietf.org/doc/html/rfc8527#section-3.2.1) capability
- no [`with-origin`](https://datatracker.ietf.org/doc/html/rfc8527#section-3.2.2) capability
diff --git a/src/restconf/Server.cpp b/src/restconf/Server.cpp
index b515d66..7c66ea4 100644
--- a/src/restconf/Server.cpp
+++ b/src/restconf/Server.cpp
@@ -834,6 +834,7 @@ Server::Server(sysrepo::Connection conn, const std::string& address, const std::
m_monitoringSession.setItem("/ietf-restconf-monitoring:restconf-state/capabilities/capability[2]", "urn:ietf:params:restconf:capability:depth:1.0");
m_monitoringSession.setItem("/ietf-restconf-monitoring:restconf-state/capabilities/capability[3]", "urn:ietf:params:restconf:capability:with-defaults:1.0");
m_monitoringSession.setItem("/ietf-restconf-monitoring:restconf-state/capabilities/capability[4]", "urn:ietf:params:restconf:capability:filter:1.0");
+ m_monitoringSession.setItem("/ietf-restconf-monitoring:restconf-state/capabilities/capability[5]", "urn:ietf:params:restconf:capability:fields:1.0");
m_monitoringSession.applyChanges();
m_monitoringOperSub = m_monitoringSession.onOperGet(
@@ -1017,7 +1018,13 @@ Server::Server(sysrepo::Connection conn, const std::string& address, const std::
}
}
- if (auto data = sess.getData(restconfRequest.path, maxDepth, getOptions, timeout); data) {
+ auto xpath = restconfRequest.path;
+ if (auto it = restconfRequest.queryParams.find("fields"); it != restconfRequest.queryParams.end()) {
+ auto fields = std::get<queryParams::fields::Expr>(it->second);
+ xpath = fieldsToXPath(sess.getContext(), xpath == "/*" ? "" : xpath, fields);
+ }
+
+ if (auto data = sess.getData(xpath, maxDepth, getOptions, timeout); data) {
res.write_head(
200,
{
diff --git a/src/restconf/uri.cpp b/src/restconf/uri.cpp
index ac399b7..da1e3a5 100644
--- a/src/restconf/uri.cpp
+++ b/src/restconf/uri.cpp
@@ -4,7 +4,10 @@
* Written by Tomáš Pecka <tomas.pecka@cesnet.cz>
*/
+#include <boost/algorithm/string/join.hpp>
+#include <boost/fusion/adapted/struct/adapt_struct.hpp>
#include <boost/fusion/include/std_pair.hpp>
+#include <boost/spirit/home/x3/support/ast/variant.hpp>
#include <experimental/iterator>
#include <libyang-cpp/Enum.hpp>
#include <map>
@@ -112,6 +115,28 @@ struct insertTable: x3::symbols<queryParams::QueryParamValue> {
}
} const insertParam;
+/* This grammar is implemented a little bit differently than the RFC states. The ABNF from RFC is:
+ *
+ * fields-expr = path "(" fields-expr ")" / path ";" fields-expr / path
+ * path = api-identifier [ "/" path ]
+ *
+ * Firstly, the grammar from the RFC doesn't allow for expression like `a(b);c` but allows for `c;a(b)`.
+ * I think both should be valid (as user I would expect that the order of such expression does not matter).
+ * Hence our grammar allows for more strings than the grammar from RFC.
+ * This issue was already raised on IETF mailing list: https://mailarchive.ietf.org/arch/msg/netconf/TYBpTE_ELzzMOe6amrw6fQF07nE/
+ * but neither a formal errata was issued nor there was a resolution on the mailing list.
+ */
+const auto fieldsExpr = x3::rule<class fieldsExpr, queryParams::fields::Expr>{"fieldsExpr"};
+const auto fieldsSemi = x3::rule<class fieldsSemiExpr, queryParams::fields::SemiExpr>{"fieldsSemi"};
+const auto fieldsSlash = x3::rule<class fieldsSlashExpr, queryParams::fields::SlashExpr>{"fieldsSlash"};
+const auto fieldsParen = x3::rule<class fieldsParen, queryParams::fields::ParenExpr>{"fieldsParen"};
+
+const auto fieldsSemi_def = fieldsParen >> -(x3::lit(";") >> fieldsSemi);
+const auto fieldsParen_def = fieldsSlash >> -(x3::lit("(") >> fieldsExpr >> x3::lit(")"));
+const auto fieldsSlash_def = apiIdentifier >> -(x3::lit("/") >> fieldsSlash);
+const auto fieldsExpr_def = fieldsSemi;
+BOOST_SPIRIT_DEFINE(fieldsParen, fieldsExpr, fieldsSlash, fieldsSemi);
+
// early sanity check, this timestamp will be parsed by libyang::fromYangTimeFormat anyways
const auto dateAndTime = x3::rule<class dateAndTime, std::string>{"dateAndTime"} =
x3::repeat(4)[x3::digit] >> x3::char_('-') >> x3::repeat(2)[x3::digit] >> x3::char_('-') >> x3::repeat(2)[x3::digit] >> x3::char_('T') >>
@@ -127,7 +152,8 @@ const auto queryParamPair = x3::rule<class queryParamPair, std::pair<std::string
(x3::string("point") >> "=" >> uriPath) |
(x3::string("filter") >> "=" >> filter) |
(x3::string("start-time") >> "=" >> dateAndTime) |
- (x3::string("stop-time") >> "=" >> dateAndTime);
+ (x3::string("stop-time") >> "=" >> dateAndTime) |
+ (x3::string("fields") >> "=" >> fieldsExpr);
const auto queryParamGrammar = x3::rule<class grammar, queryParams::QueryParams>{"queryParamGrammar"} = queryParamPair % "&" | x3::eps;
@@ -384,7 +410,7 @@ void validateQueryParameters(const std::multimap<std::string, queryParams::Query
}
}
- for (const auto& param : {"depth", "with-defaults", "content"}) {
+ for (const auto& param : {"depth", "with-defaults", "content", "fields"}) {
if (auto it = params.find(param); it != params.end() && httpMethod != "GET" && httpMethod != "HEAD") {
throw ErrorResponse(400, "protocol", "invalid-value", "Query parameter '"s + param + "' can be used only with GET and HEAD methods");
}
@@ -658,4 +684,69 @@ std::set<std::string> allowedHttpMethodsForUri(const libyang::Context& ctx, cons
return allowedHttpMethods;
}
+
+/** @brief Traverses the AST of the fields input expression and collects all the possible paths
+ *
+ * @param expr The fields expressions
+ * @param currentPath The current path in the AST, it serves as a stack for the DFS
+ * @param output The collection of all collected paths
+ * @param end If this is the terminal node, i.e., the last node in the expression. This is needed for the correct handling of the leafs under paren expression, which does not "split" the paths but rather concatenates.
+ * */
+void fieldsToXPath(const queryParams::fields::Expr& expr, std::vector<std::string>& currentPath, std::vector<std::string>& output, bool end = false)
+{
+ boost::apply_visitor([&](auto&& node) {
+ using T = std::decay_t<decltype(node)>;
+
+ if constexpr (std::is_same_v<T, queryParams::fields::ParenExpr>) {
+ // the paths from left and right subtree are concatenated, i.e., the nodes we collect in the left tree
+ // are joined together with the nodes from the right tree
+ fieldsToXPath(node.lhs, currentPath, output, !node.rhs.has_value());
+ if (node.rhs) {
+ fieldsToXPath(*node.rhs, currentPath, output, end);
+ }
+ } else if constexpr (std::is_same_v<T, queryParams::fields::SemiExpr>) {
+ // the two paths are now independent and nodes from left subtree do not affect the right subtree
+ // hence we need to copy the current path
+ auto pathCopy = currentPath;
+ fieldsToXPath(node.lhs, currentPath, output, !node.rhs.has_value());
+ if (node.rhs) {
+ fieldsToXPath(*node.rhs, pathCopy, output, false);
+ }
+ } else if constexpr (std::is_same_v<T, queryParams::fields::SlashExpr>) {
+ // the paths from left and right subtree are concatenated, i.e., the the nodes we collect in the left tree
+ // are joined together with the nodes from the right tree, but if this is the terminal node, we need to
+ // add it to the collection of all the gathered paths
+ currentPath.push_back(node.lhs.name());
+
+ if (node.rhs) {
+ fieldsToXPath(*node.rhs, currentPath, output, end);
+ } else if (end) {
+ output.emplace_back(boost::algorithm::join(currentPath, "/"));
+ }
+ }
+ }, expr);
+}
+
+/** @brief Translates the fields expression into a XPath expression and checks for schema validity of the resulting nodes
+ *
+ * The expressions are "unwrapped" into a linear structure and then a union of such paths is made.
+ * E.g., the expression "a(b;c)" is translated into "a/b | a/c".
+ * */
+std::string fieldsToXPath(const libyang::Context& ctx, const std::string& prefix, const queryParams::fields::Expr& expr)
+{
+ std::vector<std::string> currentPath{prefix};
+ std::vector<std::string> paths;
+
+ fieldsToXPath(expr, currentPath, paths);
+
+ for (auto& xpath : paths) {
+ try {
+ validateMethodForNode("GET", impl::URIPrefix{impl::URIPrefix::Type::BasicRestconfData}, ctx.findPath(xpath));
+ } catch (const libyang::Error& e) {
+ throw ErrorResponse(400, "application", "operation-failed", "Can't find schema node for '" + xpath + "'");
+ }
+ }
+
+ return boost::algorithm::join(paths, " | ");
+}
}
diff --git a/src/restconf/uri.h b/src/restconf/uri.h
index 5e079ef..f6df724 100644
--- a/src/restconf/uri.h
+++ b/src/restconf/uri.h
@@ -6,6 +6,7 @@
#pragma once
#include <boost/optional.hpp>
+#include <boost/variant.hpp>
#include <libyang-cpp/Module.hpp>
#include <libyang-cpp/SchemaNode.hpp>
#include <map>
@@ -101,6 +102,57 @@ struct After {
using PointParsed = std::vector<PathSegment>;
}
+namespace fields {
+struct ParenExpr;
+struct SemiExpr;
+struct SlashExpr;
+
+using Expr = boost::variant<boost::recursive_wrapper<SlashExpr>, boost::recursive_wrapper<ParenExpr>, boost::recursive_wrapper<SemiExpr>>;
+
+struct ParenExpr {
+ Expr lhs;
+ boost::optional<Expr> rhs;
+
+ ParenExpr() = default;
+ ParenExpr(const Expr& lhs, const Expr& rhs) : ParenExpr(lhs, boost::optional<Expr>(rhs)) {}
+ ParenExpr(const Expr& lhs, const boost::optional<Expr>& rhs = boost::none)
+ : lhs(lhs)
+ , rhs(rhs)
+ {
+ }
+
+ bool operator==(const ParenExpr&) const = default;
+};
+struct SemiExpr {
+ Expr lhs;
+ boost::optional<Expr> rhs;
+
+ SemiExpr() = default;
+ SemiExpr(const Expr& lhs, const Expr& rhs) : SemiExpr(lhs, boost::optional<Expr>(rhs)) {}
+ SemiExpr(const Expr& lhs, const boost::optional<Expr>& rhs = boost::none)
+ : lhs(lhs)
+ , rhs(rhs)
+ {
+ }
+
+ bool operator==(const SemiExpr&) const = default;
+};
+struct SlashExpr {
+ ApiIdentifier lhs;
+ boost::optional<Expr> rhs;
+
+ SlashExpr() = default;
+ SlashExpr(const ApiIdentifier& lhs, const Expr& rhs) : SlashExpr(lhs, boost::optional<Expr>(rhs)) {}
+ SlashExpr(const ApiIdentifier& lhs, const boost::optional<Expr>& rhs = boost::none)
+ : lhs(lhs)
+ , rhs(rhs)
+ {
+ }
+
+ bool operator==(const SlashExpr&) const = default;
+};
+}
+
using QueryParamValue = std::variant<
UnboundedDepth,
unsigned int,
@@ -116,7 +168,8 @@ using QueryParamValue = std::variant<
insert::Last,
insert::Before,
insert::After,
- insert::PointParsed>;
+ insert::PointParsed,
+ fields::Expr>;
using QueryParams = std::multimap<std::string, QueryParamValue>;
}
@@ -159,4 +212,6 @@ std::vector<PathSegment> asPathSegments(const std::string& uriPath);
std::optional<std::variant<libyang::Module, libyang::SubmoduleParsed>> asYangModule(const libyang::Context& ctx, const std::string& uriPath);
RestconfStreamRequest asRestconfStreamRequest(const std::string& httpMethod, const std::string& uriPath, const std::string& uriQueryString);
std::set<std::string> allowedHttpMethodsForUri(const libyang::Context& ctx, const std::string& uriPath);
+
+std::string fieldsToXPath(const libyang::Context& ctx, const std::string& prefix, const queryParams::fields::Expr& expr);
}
diff --git a/src/restconf/uri_impl.h b/src/restconf/uri_impl.h
index 8a2e166..2bcdb3f 100644
--- a/src/restconf/uri_impl.h
+++ b/src/restconf/uri_impl.h
@@ -65,3 +65,7 @@ BOOST_FUSION_ADAPT_STRUCT(rousette::restconf::impl::URIPath, prefix, segments);
BOOST_FUSION_ADAPT_STRUCT(rousette::restconf::impl::YangModule, name, revision);
BOOST_FUSION_ADAPT_STRUCT(rousette::restconf::PathSegment, apiIdent, keys);
BOOST_FUSION_ADAPT_STRUCT(rousette::restconf::ApiIdentifier, prefix, identifier);
+
+BOOST_FUSION_ADAPT_STRUCT(rousette::restconf::queryParams::fields::ParenExpr, lhs, rhs);
+BOOST_FUSION_ADAPT_STRUCT(rousette::restconf::queryParams::fields::SlashExpr, lhs, rhs);
+BOOST_FUSION_ADAPT_STRUCT(rousette::restconf::queryParams::fields::SemiExpr, lhs, rhs);
diff --git a/tests/pretty_printers.h b/tests/pretty_printers.h
index ec87250..a2befeb 100644
--- a/tests/pretty_printers.h
+++ b/tests/pretty_printers.h
@@ -8,6 +8,7 @@
#pragma once
#include "trompeloeil_doctest.h"
+#include <boost/variant.hpp>
#include <experimental/iterator>
#include <optional>
#include <sstream>
@@ -159,6 +160,28 @@ struct StringMaker<rousette::restconf::queryParams::QueryParamValue> {
[](const rousette::restconf::queryParams::insert::PointParsed& p) -> std::string {
return ("PointParsed{" + StringMaker<decltype(p)>::convert(p) + "}").c_str();
},
+ [](const rousette::restconf::queryParams::fields::Expr& expr) -> std::string {
+ return boost::apply_visitor([&](auto&& next) {
+ using T = std::decay_t<decltype(next)>;
+ std::string res;
+
+ if constexpr (std::is_same_v<T, rousette::restconf::queryParams::fields::ParenExpr> || std::is_same_v<T, rousette::restconf::queryParams::fields::SemiExpr>) {
+ if constexpr (std::is_same_v<T, rousette::restconf::queryParams::fields::ParenExpr>) {
+ res = "ParenExpr{";
+ } else {
+ res = "SemiExpr{";
+ }
+ res += StringMaker<rousette::restconf::queryParams::QueryParamValue>::convert(next.lhs).c_str();
+ } else if constexpr (std::is_same_v<T, rousette::restconf::queryParams::fields::SlashExpr>) {
+ res = "SlashExpr{" + next.lhs.name();
+ }
+
+ if (next.rhs) {
+ res += std::string(", ") + StringMaker<rousette::restconf::queryParams::QueryParamValue>::convert(*next.rhs).c_str();
+ }
+ return res += "}";
+ }, expr);
+ },
}, obj).c_str();
}
};
diff --git a/tests/restconf-reading.cpp b/tests/restconf-reading.cpp
index 2ded3f0..d7d507b 100644
--- a/tests/restconf-reading.cpp
+++ b/tests/restconf-reading.cpp
@@ -72,7 +72,8 @@ TEST_CASE("reading data")
"urn:ietf:params:restconf:capability:defaults:1.0?basic-mode=explicit",
"urn:ietf:params:restconf:capability:depth:1.0",
"urn:ietf:params:restconf:capability:with-defaults:1.0",
- "urn:ietf:params:restconf:capability:filter:1.0"
+ "urn:ietf:params:restconf:capability:filter:1.0",
+ "urn:ietf:params:restconf:capability:fields:1.0"
]
},
"streams": {
@@ -116,7 +117,8 @@ TEST_CASE("reading data")
"urn:ietf:params:restconf:capability:defaults:1.0?basic-mode=explicit",
"urn:ietf:params:restconf:capability:depth:1.0",
"urn:ietf:params:restconf:capability:with-defaults:1.0",
- "urn:ietf:params:restconf:capability:filter:1.0"
+ "urn:ietf:params:restconf:capability:filter:1.0",
+ "urn:ietf:params:restconf:capability:fields:1.0"
]
},
"streams": {
@@ -672,7 +674,8 @@ TEST_CASE("reading data")
"urn:ietf:params:restconf:capability:defaults:1.0?basic-mode=explicit",
"urn:ietf:params:restconf:capability:depth:1.0",
"urn:ietf:params:restconf:capability:with-defaults:1.0",
- "urn:ietf:params:restconf:capability:filter:1.0"
+ "urn:ietf:params:restconf:capability:filter:1.0",
+ "urn:ietf:params:restconf:capability:fields:1.0"
]
},
"streams": {
@@ -894,6 +897,141 @@ TEST_CASE("reading data")
)"});
}
+ SECTION("fields filtering")
+ {
+ srSess.switchDatastore(sysrepo::Datastore::Running);
+ srSess.setItem("/example:tlc/list[name='blabla']/choice1", "c1");
+ srSess.setItem("/example:tlc/list[name='blabla']/collection[.='42']", std::nullopt);
+ srSess.setItem("/example:tlc/list[name='blabla']/nested[first='1'][second='2'][third='3']/fourth", "4");
+ srSess.setItem("/example:tlc/list[name='blabla']/nested[first='1'][second='2'][third='3']/data/a", "a");
+ srSess.setItem("/example:tlc/list[name='blabla']/nested[first='1'][second='2'][third='3']/data/other-data/b", "b");
+ srSess.setItem("/example:tlc/list[name='blabla']/nested[first='1'][second='2'][third='3']/data/other-data/c", "c");
+ srSess.applyChanges();
+
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:tlc/list=blabla?fields=choice1;collection", {}) == Response{200, jsonHeaders, R"({
+ "example:tlc": {
+ "list": [
+ {
+ "name": "blabla",
+ "collection": [
+ 42
+ ],
+ "choice1": "c1"
+ }
+ ]
+ }
+}
+)"});
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:tlc/list=blabla?fields=choice1;choice2;nested/data(a;other-data/b)", {}) == Response{200, jsonHeaders, R"({
+ "example:tlc": {
+ "list": [
+ {
+ "name": "blabla",
+ "nested": [
+ {
+ "first": "1",
+ "second": 2,
+ "third": "3",
+ "data": {
+ "a": "a",
+ "other-data": {
+ "b": "b"
+ }
+ }
+ }
+ ],
+ "choice1": "c1"
+ }
+ ]
+ }
+}
+)"});
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:tlc/list=blabla?fields=hehe", {}) == Response{400, jsonHeaders, R"({
+ "ietf-restconf:errors": {
+ "error": [
+ {
+ "error-type": "application",
+ "error-tag": "operation-failed",
+ "error-message": "Can't find schema node for '/example:tlc/list[name='blabla']/hehe'"
+ }
+ ]
+ }
+}
+)"});
+ REQUIRE(get(RESTCONF_DATA_ROOT "/example:tlc/list=blabla?fields=nested/data&depth=1", {}) == Response{200, jsonHeaders, R"({
+ "example:tlc": {
+ "list": [
+ {
+ "name": "blabla",
+ "nested": [
+ {
+ "first": "1",
+ "second": 2,
+ "third": "3",
+ "data": {
+ "a": "a",
+ "other-data": {}
+ }
+ }
+ ]
+ }
+ ]
+ }
+}
+)"});
+
+ // whole datastore with fields filtering
+ REQUIRE(get(RESTCONF_DATA_ROOT "?fields=example:tlc/list/nested/data&depth=1", {}) == Response{200, jsonHeaders, R"({
+ "example:tlc": {
+ "list": [
+ {
+ "name": "blabla",
+ "nested": [
+ {
+ "first": "1",
+ "second": 2,
+ "third": "3",
+ "data": {
+ "a": "a",
+ "other-data": {}
+ }
+ }
+ ]
+ }
+ ]
+ }
+}
+)"});
+
+ // nonexistent schema node in fields: missing prefix in tlc because we query root node so libyang can't infer the prefix from the path
+ REQUIRE(get(RESTCONF_DATA_ROOT "?fields=tlc", {}) == Response{400, jsonHeaders, R"({
+ "ietf-restconf:errors": {
+ "error": [
+ {
+ "error-type": "application",
+ "error-tag": "operation-failed",
+ "error-message": "Can't find schema node for '/tlc'"
+ }
+ ]
+ }
+}
+)"});
+
+ // nonexistent schema node in fields
+ REQUIRE(get(RESTCONF_DATA_ROOT "?fields=example:tlc/ob-la-di-ob-la-da", {}) == Response{400, jsonHeaders, R"({
+ "ietf-restconf:errors": {
+ "error": [
+ {
+ "error-type": "application",
+ "error-tag": "operation-failed",
+ "error-message": "Can't find schema node for '/example:tlc/ob-la-di-ob-la-da'"
+ }
+ ]
+ }
+}
+)"});
+ }
+
SECTION("OPTIONS method")
{
// RPC node
diff --git a/tests/restconf-writing.cpp b/tests/restconf-writing.cpp
index c1a9515..582a262 100644
--- a/tests/restconf-writing.cpp
+++ b/tests/restconf-writing.cpp
@@ -375,6 +375,8 @@ TEST_CASE("writing data")
CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/first", "1"),
CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/second", "2"),
CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/third", "3"),
+ CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/data", std::nullopt),
+ CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/data/other-data", std::nullopt),
CREATED("/example:tlc/list[name='large']/choice2", "large"));
REQUIRE(put(RESTCONF_DATA_ROOT "/example:tlc/list=large", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list":[{"name": "large", "choice2": "large", "example:nested": [{"first": "1", "second": 2, "third": "3"}]}]})") == Response{201, noContentTypeHeaders, ""});
}
@@ -385,7 +387,9 @@ TEST_CASE("writing data")
CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']", std::nullopt),
CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/first", "11"),
CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/second", "12"),
- CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/third", "13"));
+ CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/third", "13"),
+ CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/data", std::nullopt),
+ CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/data/other-data", std::nullopt));
REQUIRE(put(RESTCONF_DATA_ROOT "/example:tlc/list=libyang/nested=11,12,13", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:nested": [{"first": "11", "second": 12, "third": "13"}]}]})") == Response{201, noContentTypeHeaders, ""});
}
@@ -1339,6 +1343,8 @@ TEST_CASE("writing data")
CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/first", "1"),
CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/second", "2"),
CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/third", "3"),
+ CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/data", std::nullopt),
+ CREATED("/example:tlc/list[name='large']/nested[first='1'][second='2'][third='3']/data/other-data", std::nullopt),
CREATED("/example:tlc/list[name='large']/choice2", "large"));
REQUIRE(post(RESTCONF_DATA_ROOT "/example:tlc", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:list":[{"name": "large", "choice2": "large", "example:nested": [{"first": "1", "second": 2, "third": "3"}]}]})") == Response{201, jsonHeaders, ""});
}
@@ -1349,7 +1355,9 @@ TEST_CASE("writing data")
CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']", std::nullopt),
CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/first", "11"),
CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/second", "12"),
- CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/third", "13"));
+ CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/third", "13"),
+ CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/data", std::nullopt),
+ CREATED("/example:tlc/list[name='libyang']/nested[first='11'][second='12'][third='13']/data/other-data", std::nullopt));
REQUIRE(post(RESTCONF_DATA_ROOT "/example:tlc/list=libyang", {AUTH_ROOT, CONTENT_TYPE_JSON}, R"({"example:nested": [{"first": "11", "second": 12, "third": "13"}]}]})") == Response{201, jsonHeaders, ""});
}
diff --git a/tests/uri-parser.cpp b/tests/uri-parser.cpp
index 000fe5d..fd5dd8b 100644
--- a/tests/uri-parser.cpp
+++ b/tests/uri-parser.cpp
@@ -828,6 +828,65 @@ TEST_CASE("URI path parser")
REQUIRE(parseQueryParams("stop-time=2023-05-20T18:30:00") == std::nullopt);
REQUIRE(parseQueryParams("stop-time=20230520T18:30:00Z") == std::nullopt);
REQUIRE(parseQueryParams("stop-time=2023-05-a0T18:30:00+05:30") == std::nullopt);
+ REQUIRE(parseQueryParams("fields=mod:leaf") == QueryParams{{"fields", fields::SemiExpr{fields::ParenExpr{fields::SlashExpr{{"mod", "leaf"}}}}}});
+ REQUIRE(parseQueryParams("fields=b(c;d);e(f)") == QueryParams{{"fields",
+ fields::SemiExpr{
+ fields::ParenExpr{
+ fields::SlashExpr{{"b"}},
+ fields::SemiExpr{
+ fields::ParenExpr{
+ fields::SlashExpr{{"c"}}
+ },
+ fields::SemiExpr{
+ fields::ParenExpr{
+ fields::SlashExpr{{"d"}}
+ }
+ }
+ }
+ },
+ fields::SemiExpr{
+ fields::ParenExpr{
+ fields::SlashExpr{{"e"}},
+ fields::SemiExpr{
+ fields::ParenExpr{
+ fields::SlashExpr{{"f"}}
+ }
+ }
+ }
+ }
+ }
+ }});
+ REQUIRE(parseQueryParams("fields=(xyz)") == std::nullopt);
+ REQUIRE(parseQueryParams("fields=a;(xyz)") == std::nullopt);
+ REQUIRE(parseQueryParams("fields=") == std::nullopt);
+
+ for (const auto& [prefix, fields, xpath] : {
+ std::tuple<std::string, std::string, std::string>{"/example:a", "b", "/example:a/b"},
+ {"/example:a", "b/c", "/example:a/b/c"},
+ {"/example:a/b", "c(enabled;blower)", "/example:a/b/c/enabled | /example:a/b/c/blower"},
+ {"/example:a", "b(c(enabled;blower))", "/example:a/b/c/enabled | /example:a/b/c/blower"},
+ {"/example:a", "b(c)", "/example:a/b/c"},
+ {"/example:a", "example:b;something", "/example:a/example:b | /example:a/something"},
+ {"/example:a", "something;b1;b(c/enabled;c/blower)", "/example:a/something | /example:a/b1 | /example:a/b/c/enabled | /example:a/b/c/blower"},
+ {"/example:a", "b(c/enabled;c/blower);something;b1", "/example:a/b/c/enabled | /example:a/b/c/blower | /example:a/something | /example:a/b1"}, // not allowed by RFC 8040
+ {"", "example:a(b;b1)", "/example:a/b | /example:a/b1"},
+ }) {
+ CAPTURE(fields);
+ CAPTURE(xpath);
+ auto qp = parseQueryParams("fields=" + fields);
+ REQUIRE(qp);
+ REQUIRE(qp->count("fields") == 1);
+ auto fieldExpr = qp->find("fields")->second;
+ REQUIRE(std::holds_alternative<fields::Expr>(fieldExpr));
+ REQUIRE(rousette::restconf::fieldsToXPath(ctx, prefix, std::get<fields::Expr>(fieldExpr)) == xpath);
+ }
+
+ auto qp = parseQueryParams("fields=xxx/xyz(a;b)");
+ REQUIRE(qp);
+ REQUIRE_THROWS_WITH_AS(
+ rousette::restconf::fieldsToXPath(ctx, "/example:a", std::get<fields::Expr>(qp->find("fields")->second)),
+ serializeErrorResponse(400, "application", "operation-failed", "Can't find schema node for '/example:a/xxx/xyz/a'").c_str(),
+ rousette::restconf::ErrorResponse);
}
SECTION("Full requests with validation")
@@ -885,6 +944,40 @@ TEST_CASE("URI path parser")
rousette::restconf::ErrorResponse);
}
+ SECTION("fields")
+ {
+ auto resp = asRestconfRequest(ctx, "GET", "/restconf/data/example:a", "fields=b/c(enabled;blower)");
+ REQUIRE(resp.queryParams == QueryParams({{"fields",
+ fields::SemiExpr{
+ fields::ParenExpr{
+ fields::SlashExpr{
+ {"b"},
+ fields::SlashExpr{{"c"}}
+ },
+ fields::SemiExpr{
+ fields::ParenExpr{
+ fields::SlashExpr{{"enabled"}}
+ },
+ fields::SemiExpr{
+ fields::ParenExpr{
+ fields::SlashExpr{{"blower"}}
+ }
+ }
+ }
+ }
+ }
+ }
+ }));
+
+ REQUIRE_THROWS_WITH_AS(asRestconfRequest(ctx, "POST", "/restconf/data/example:a", "fields=b/c(enabled;blower)"),
+ serializeErrorResponse(400, "protocol", "invalid-value", "Query parameter 'fields' can be used only with GET and HEAD methods").c_str(),
+ rousette::restconf::ErrorResponse);
+
+ REQUIRE_THROWS_WITH_AS(asRestconfStreamRequest("GET", "/streams/NETCONF/XML", "fields=a"),
+ serializeErrorResponse(400, "protocol", "invalid-value", "Query parameter 'fields' can't be used with streams").c_str(),
+ rousette::restconf::ErrorResponse);
+ }
+
SECTION("insert first/last")
{
auto resp = asRestconfRequest(ctx, "PUT", "/restconf/data/example:tlc", "insert=first");
diff --git a/tests/yang/example.yang b/tests/yang/example.yang
index 75cd7a6..5d586a0 100644
--- a/tests/yang/example.yang
+++ b/tests/yang/example.yang
@@ -38,6 +38,14 @@ module example {
leaf first { type string; }
leaf second { type int32; }
leaf third { type string; }
+ leaf fourth { type string; }
+ container data {
+ leaf a { type string; }
+ container other-data {
+ leaf b { type string; }
+ leaf c { type string; }
+ }
+ }
}
choice choose {
mandatory true;
@@ -92,6 +100,8 @@ module example {
}
}
}
+ container b1 { }
+ leaf something { type string; }
}
container two-leafs {
--
2.43.0
package/rousette/0009-cmake-adhere-to-CMP0167.patch
+36
View File
@@ -0,0 +1,36 @@
From 48d9b6ba3f3f892b9060b76b505d2f9a3aeb9e02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Mon, 25 Nov 2024 09:15:55 +0100
Subject: [PATCH 09/20] cmake: adhere to CMP0167
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
After locally updating to cmake 3.30 I have seen a warning that our way
of finding boost library is deprecated [1].
[1] https://cmake.org/cmake/help/latest/policy/CMP0167.html
Change-Id: I0cfc6cd0077fac48723487a280daac5fe8218ebb
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
CMakeLists.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 465bef9..01dd2c2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -72,7 +72,7 @@ find_package(spdlog REQUIRED)
find_package(date REQUIRED) # FIXME: Remove when we have STL with __cpp_lib_chrono >= 201907 (gcc 14)
find_package(PkgConfig)
pkg_check_modules(nghttp2 REQUIRED IMPORTED_TARGET libnghttp2_asio>=0.0.90 libnghttp2)
-find_package(Boost REQUIRED COMPONENTS system thread)
+find_package(Boost REQUIRED CONFIG COMPONENTS system thread)
pkg_check_modules(SYSREPO-CPP REQUIRED IMPORTED_TARGET sysrepo-cpp>=3)
pkg_check_modules(LIBYANG-CPP REQUIRED IMPORTED_TARGET libyang-cpp>=3)
--
2.43.0
package/rousette/0010-Fix-compatibility-with-pam_wrapper-1.1.6.patch
+45
View File
@@ -0,0 +1,45 @@
From 64997543d48236cd2aae417568bc54d32c54df21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= <jan.kundrat@cesnet.cz>
Date: Mon, 2 Dec 2024 14:43:36 +0100
Subject: [PATCH 10/20] Fix compatibility with pam_wrapper 1.1.6+
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
An year ago I reported a bug that the pam_wrapper project says that they
use a variable called `PAM_WRAPPER_DISABLE_DEEPBIND`, but in fact they
check `UID_WRAPPER_DISABLE_DEEPBIND`. The upstream listened to me, and
they fixed it [1]. Unfortunately, the old variable name is not read from
as of pam_wrapper 1.1.6, so we require setting *both* variables for
random version compatibility.
[1] https://git.samba.org/?p=pam_wrapper.git;a=commitdiff;h=9f0cccf7432dd9be1de953f9b13a7f9b06c40442
Change-Id: I2959f505f5325950606c68b0b324be7181dd6e4f
Reported-by: Tomáš Pecka <tomas.pecka@cesnet.cz>
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
CMakeLists.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 01dd2c2..731d7cb 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -179,10 +179,11 @@ if(BUILD_TESTING)
endif()
if(TEST_WRAP_PAM)
+ # FIXME: remove UID_WRAPPER_... (and keep PAM_WRAPPER_...) once we require pam_wrapper 1.1.6+
set(TEST_COMMAND
${UNSHARE_EXECUTABLE} -r -m sh -c "set -ex $<SEMICOLON>
${MOUNT_EXECUTABLE} -t tmpfs none /tmp $<SEMICOLON>
- export LD_PRELOAD=${pam_wrapper_LDFLAGS} PAM_WRAPPER_SERVICE_DIR=${CMAKE_CURRENT_BINARY_DIR}/tests/pam PAM_WRAPPER=1 UID_WRAPPER_DISABLE_DEEPBIND=1 $<SEMICOLON>
+ export LD_PRELOAD=${pam_wrapper_LDFLAGS} PAM_WRAPPER_SERVICE_DIR=${CMAKE_CURRENT_BINARY_DIR}/tests/pam PAM_WRAPPER=1 UID_WRAPPER_DISABLE_DEEPBIND=1 PAM_WRAPPER_DISABLE_DEEPBIND=1 $<SEMICOLON>
$<TARGET_FILE:test-${TEST_NAME}>")
else()
set(TEST_COMMAND test-${TEST_NAME})
--
2.43.0
package/rousette/0011-tests-add-missing-pragma-once.patch
+30
View File
@@ -0,0 +1,30 @@
From 849aa35274d5e07726cb849fe724962754b3fa29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Mon, 2 Dec 2024 20:20:48 +0100
Subject: [PATCH 11/20] tests: add missing pragma once
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
Change-Id: I269c20e5a914aa8c7bc9431147dd9785ea1aedda
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
tests/aux-utils.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/aux-utils.h b/tests/aux-utils.h
index a3923bb..7482945 100644
--- a/tests/aux-utils.h
+++ b/tests/aux-utils.h
@@ -6,6 +6,7 @@
*
*/
+#pragma once
#include "trompeloeil_doctest.h"
#include <iostream>
#include <nghttp2/asio_http2_client.h>
--
2.43.0
package/rousette/0012-tests-extend-clientRequest-wrappers-interface-and-us.patch
+205
View File
@@ -0,0 +1,205 @@
From 60eac2b2d60f8f1918a0914272975dd53f527c01 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Mon, 2 Dec 2024 19:36:10 +0100
Subject: [PATCH 12/20] tests: extend clientRequest wrappers interface and use
it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
This is a preparation for refactoring in the next few commits.
I will generalize the clientRequest interface to accept server
address and port too.
The head/get/put/post/... helper methods will not require those server
port and address parameters.
Change-Id: Iee54a3b3017ef9875fcd20640b74c7aa42813b9f
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
tests/aux-utils.h | 34 +++++++++++++++++---------------
tests/restconf-nacm.cpp | 14 +++++--------
tests/restconf-notifications.cpp | 12 +++++------
tests/restconf-yang-schema.cpp | 19 ++++++++++--------
4 files changed, 40 insertions(+), 39 deletions(-)
diff --git a/tests/aux-utils.h b/tests/aux-utils.h
index 7482945..9afd7bc 100644
--- a/tests/aux-utils.h
+++ b/tests/aux-utils.h
@@ -153,12 +153,14 @@ const ng::header_map eventStreamHeaders {
#define ACCESS_CONTROL_ALLOW_ORIGIN {"access-control-allow-origin", "*"}
#define ACCEPT_PATCH {"accept-patch", "application/yang-data+json, application/yang-data+xml, application/yang-patch+xml, application/yang-patch+json"}
+// this is a test, and the server is expected to reply "soon"
+static const boost::posix_time::time_duration CLIENT_TIMEOUT = boost::posix_time::seconds(3);
+
Response clientRequest(auto method,
auto uri,
const std::string& data,
const std::map<std::string, std::string>& headers,
- // this is a test, and the server is expected to reply "soon"
- const boost::posix_time::time_duration timeout=boost::posix_time::seconds(3))
+ const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
boost::asio::io_service io_service;
auto client = std::make_shared<ng_client::session>(io_service, SERVER_ADDRESS, SERVER_PORT);
@@ -199,39 +201,39 @@ Response clientRequest(auto method,
return {statusCode, resHeaders, oss.str()};
}
-Response get(auto uri, const std::map<std::string, std::string>& headers)
+Response get(auto uri, const std::map<std::string, std::string>& headers, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("GET", uri, "", headers);
+ return clientRequest("GET", uri, "", headers, timeout);
}
-Response options(auto uri, const std::map<std::string, std::string>& headers)
+Response options(auto uri, const std::map<std::string, std::string>& headers, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("OPTIONS", uri, "", headers);
+ return clientRequest("OPTIONS", uri, "", headers, timeout);
}
-Response head(auto uri, const std::map<std::string, std::string>& headers)
+Response head(auto uri, const std::map<std::string, std::string>& headers, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("HEAD", uri, "", headers);
+ return clientRequest("HEAD", uri, "", headers, timeout);
}
-Response put(auto xpath, const std::map<std::string, std::string>& headers, const std::string& data)
+Response put(auto xpath, const std::map<std::string, std::string>& headers, const std::string& data, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("PUT", xpath, data, headers);
+ return clientRequest("PUT", xpath, data, headers, timeout);
}
-Response post(auto xpath, const std::map<std::string, std::string>& headers, const std::string& data)
+Response post(auto xpath, const std::map<std::string, std::string>& headers, const std::string& data, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("POST", xpath, data, headers);
+ return clientRequest("POST", xpath, data, headers, timeout);
}
-Response patch(auto uri, const std::map<std::string, std::string>& headers, const std::string& data)
+Response patch(auto uri, const std::map<std::string, std::string>& headers, const std::string& data, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("PATCH", uri, data, headers);
+ return clientRequest("PATCH", uri, data, headers, timeout);
}
-Response httpDelete(auto uri, const std::map<std::string, std::string>& headers)
+Response httpDelete(auto uri, const std::map<std::string, std::string>& headers, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("DELETE", uri, "", headers);
+ return clientRequest("DELETE", uri, "", headers, timeout);
}
auto manageNacm(sysrepo::Session session)
diff --git a/tests/restconf-nacm.cpp b/tests/restconf-nacm.cpp
index 68497c9..29d7723 100644
--- a/tests/restconf-nacm.cpp
+++ b/tests/restconf-nacm.cpp
@@ -225,9 +225,7 @@ TEST_CASE("NACM")
{
// wrong password: the server should delay its response, so let the client wait "long enough"
const auto start = std::chrono::steady_clock::now();
- REQUIRE(clientRequest("GET",
- RESTCONF_DATA_ROOT "/ietf-system:system",
- "",
+ REQUIRE(get(RESTCONF_DATA_ROOT "/ietf-system:system",
{AUTH_WRONG_PASSWORD},
boost::posix_time::seconds(5))
== Response{401, jsonHeaders, R"({
@@ -251,12 +249,10 @@ TEST_CASE("NACM")
// wrong password: the server should delay its response, in this case let the client terminate its
// request and check that the server doesn't crash
const auto start = std::chrono::steady_clock::now();
- REQUIRE_THROWS_WITH(clientRequest("GET",
- RESTCONF_DATA_ROOT "/ietf-system:system",
- "",
- {AUTH_WRONG_PASSWORD},
- boost::posix_time::milliseconds(100)),
- "HTTP client error: Connection timed out");
+ REQUIRE_THROWS_WITH(get(RESTCONF_DATA_ROOT "/ietf-system:system",
+ {AUTH_WRONG_PASSWORD},
+ boost::posix_time::milliseconds(100)),
+ "HTTP client error: Connection timed out");
auto processingMS = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start).count();
REQUIRE(processingMS <= 500);
}
diff --git a/tests/restconf-notifications.cpp b/tests/restconf-notifications.cpp
index 905ae01..d479f3c 100644
--- a/tests/restconf-notifications.cpp
+++ b/tests/restconf-notifications.cpp
@@ -277,18 +277,18 @@ TEST_CASE("NETCONF notification streams")
SECTION("Other methods")
{
- REQUIRE(clientRequest("HEAD", "/streams/NETCONF/XML", "", {AUTH_ROOT}) == Response{200, eventStreamHeaders, ""});
- REQUIRE(clientRequest("OPTIONS", "/streams/NETCONF/XML", "", {AUTH_ROOT}) == Response{200, Response::Headers{ACCESS_CONTROL_ALLOW_ORIGIN, {"allow", "GET, HEAD, OPTIONS"}}, ""});
+ REQUIRE(head("/streams/NETCONF/XML", {AUTH_ROOT}) == Response{200, eventStreamHeaders, ""});
+ REQUIRE(options("/streams/NETCONF/XML", {AUTH_ROOT}) == Response{200, Response::Headers{ACCESS_CONTROL_ALLOW_ORIGIN, {"allow", "GET, HEAD, OPTIONS"}}, ""});
const std::multimap<std::string, std::string> headers = {
{"access-control-allow-origin", "*"},
{"allow", "GET, HEAD, OPTIONS"},
{"content-type", "text/plain"},
};
- REQUIRE(clientRequest("PUT", "/streams/NETCONF/XML", "", {AUTH_ROOT}) == Response{405, headers, "Method not allowed."});
- REQUIRE(clientRequest("POST", "/streams/NETCONF/XML", "", {AUTH_ROOT}) == Response{405, headers, "Method not allowed."});
- REQUIRE(clientRequest("PATCH", "/streams/NETCONF/XML", "", {AUTH_ROOT}) == Response{405, headers, "Method not allowed."});
- REQUIRE(clientRequest("DELETE", "/streams/NETCONF/XML", "", {AUTH_ROOT}) == Response{405, headers, "Method not allowed."});
+ REQUIRE(put("/streams/NETCONF/XML", {AUTH_ROOT}, "") == Response{405, headers, "Method not allowed."});
+ REQUIRE(post("/streams/NETCONF/XML", {AUTH_ROOT}, "") == Response{405, headers, "Method not allowed."});
+ REQUIRE(patch("/streams/NETCONF/XML", {AUTH_ROOT}, "") == Response{405, headers, "Method not allowed."});
+ REQUIRE(httpDelete("/streams/NETCONF/XML", {AUTH_ROOT}) == Response{405, headers, "Method not allowed."});
}
SECTION("Invalid URLs")
diff --git a/tests/restconf-yang-schema.cpp b/tests/restconf-yang-schema.cpp
index 73821e0..6e374b1 100644
--- a/tests/restconf-yang-schema.cpp
+++ b/tests/restconf-yang-schema.cpp
@@ -156,11 +156,14 @@ TEST_CASE("obtaining YANG schemas")
{
SECTION("unsupported methods")
{
- for (const std::string httpMethod : {"POST", "PUT", "PATCH", "DELETE"}) {
- CAPTURE(httpMethod);
- REQUIRE(clientRequest(httpMethod, YANG_ROOT "/ietf-yang-library@2019-01-04", "", {AUTH_ROOT})
- == Response{405, Response::Headers{ACCESS_CONTROL_ALLOW_ORIGIN, {"allow", "GET, HEAD, OPTIONS"}}, ""});
- }
+ REQUIRE(post(YANG_ROOT "/ietf-yang-library@2019-01-04", {AUTH_ROOT}, "")
+ == Response{405, Response::Headers{ACCESS_CONTROL_ALLOW_ORIGIN, {"allow", "GET, HEAD, OPTIONS"}}, ""});
+ REQUIRE(put(YANG_ROOT "/ietf-yang-library@2019-01-04", {AUTH_ROOT}, "")
+ == Response{405, Response::Headers{ACCESS_CONTROL_ALLOW_ORIGIN, {"allow", "GET, HEAD, OPTIONS"}}, ""});
+ REQUIRE(patch(YANG_ROOT "/ietf-yang-library@2019-01-04", {AUTH_ROOT}, "")
+ == Response{405, Response::Headers{ACCESS_CONTROL_ALLOW_ORIGIN, {"allow", "GET, HEAD, OPTIONS"}}, ""});
+ REQUIRE(httpDelete(YANG_ROOT "/ietf-yang-library@2019-01-04", {AUTH_ROOT})
+ == Response{405, Response::Headers{ACCESS_CONTROL_ALLOW_ORIGIN, {"allow", "GET, HEAD, OPTIONS"}}, ""});
}
REQUIRE(options(YANG_ROOT "/ietf-yang-library@2019-01-04", {}) == Response{200, Response::Headers{ACCESS_CONTROL_ALLOW_ORIGIN, {"allow", "GET, HEAD, OPTIONS"}}, ""});
@@ -190,12 +193,12 @@ TEST_CASE("obtaining YANG schemas")
SECTION("auth failure")
{
// wrong password
- REQUIRE(clientRequest("GET", YANG_ROOT "/ietf-system@2014-08-06", "", {AUTH_WRONG_PASSWORD}, boost::posix_time::seconds{5})
+ REQUIRE(get(YANG_ROOT "/ietf-system@2014-08-06", {AUTH_WRONG_PASSWORD}, boost::posix_time::seconds{5})
== Response{401, plaintextHeaders, "Access denied."});
- REQUIRE(clientRequest("HEAD", YANG_ROOT "/ietf-system@2014-08-06", "", {AUTH_WRONG_PASSWORD}, boost::posix_time::seconds{5})
+ REQUIRE(head(YANG_ROOT "/ietf-system@2014-08-06", {AUTH_WRONG_PASSWORD}, boost::posix_time::seconds{5})
== Response{401, plaintextHeaders, ""});
// anonymous request
- REQUIRE(clientRequest("HEAD", YANG_ROOT "/ietf-system@2014-08-06", "", {FORWARDED}, boost::posix_time::seconds{5})
+ REQUIRE(head(YANG_ROOT "/ietf-system@2014-08-06", {FORWARDED}, boost::posix_time::seconds{5})
== Response{401, plaintextHeaders, ""});
}
}
--
2.43.0
package/rousette/0013-tests-move-stuff-from-the-header-file-into-cpp-file.patch
+586
View File
@@ -0,0 +1,586 @@
From 0beaee041fd4fcfbebcbb2912eb6b26ec71f50c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Mon, 2 Dec 2024 20:01:54 +0100
Subject: [PATCH 13/20] tests: move stuff from the header file into cpp file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
Allright, I have had enough. I am no longer waiting for a minute for a
recompilation of one test.
This patch splits the aux-utils.h file into a cpp file and two headers.
One of the headers is only for declarations of the functions and
datatypes in the cpp file, the second header implements all the helper
functions that require SERVER_PORT and further helper constants for the
restconf tests.
Compile times are a little better. I have measured compilation times of
two arbitrary restconf tests with asan+ubsan (and ccache disabled):
- restconf-reading: 17.05s -> 13.85s
- restconf-delete: 15.18s -> 11.87s
Not ideal, but it is certainly better.
Change-Id: If529cbc8954d50494711a408231ea4c2c4daf072
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
CMakeLists.txt | 1 +
tests/aux-utils.h | 214 ++-----------------------------
tests/restconf-notifications.cpp | 3 +-
tests/restconf_utils.cpp | 157 +++++++++++++++++++++++
tests/restconf_utils.h | 83 ++++++++++++
5 files changed, 256 insertions(+), 202 deletions(-)
create mode 100644 tests/restconf_utils.cpp
create mode 100644 tests/restconf_utils.h
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 731d7cb..b8a41a7 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -158,6 +158,7 @@ if(BUILD_TESTING)
add_library(DoctestIntegration STATIC
tests/datastoreUtils.cpp
tests/doctest_integration.cpp
+ tests/restconf_utils.cpp
tests/trompeloeil_doctest.h
tests/wait-a-bit-longer.cpp
)
diff --git a/tests/aux-utils.h b/tests/aux-utils.h
index 9afd7bc..d99b3f9 100644
--- a/tests/aux-utils.h
+++ b/tests/aux-utils.h
@@ -8,99 +8,16 @@
#pragma once
#include "trompeloeil_doctest.h"
-#include <iostream>
#include <nghttp2/asio_http2_client.h>
-#include <optional>
-#include <sstream>
-#include <sysrepo-cpp/Session.hpp>
-#include "tests/UniqueResource.h"
+#include "restconf_utils.h"
-using namespace std::string_literals;
-namespace ng = nghttp2::asio_http2;
-namespace ng_client = ng::client;
-
-struct Response {
- int statusCode;
- ng::header_map headers;
- std::string data;
-
- using Headers = std::multimap<std::string, std::string>;
-
- Response(int statusCode, const Headers& headers, const std::string& data)
- : Response(statusCode, transformHeaders(headers), data)
- {
- }
-
- Response(int statusCode, const ng::header_map& headers, const std::string& data)
- : statusCode(statusCode)
- , headers(headers)
- , data(data)
- {
- }
-
- bool equalStatusCodeAndHeaders(const Response& o) const
- {
- // Skipping 'date' header. Its value will not be reproducible in simple tests
- ng::header_map myHeaders(headers);
- ng::header_map otherHeaders(o.headers);
- myHeaders.erase("date");
- otherHeaders.erase("date");
-
- return statusCode == o.statusCode && std::equal(myHeaders.begin(), myHeaders.end(), otherHeaders.begin(), otherHeaders.end(), [](const auto& a, const auto& b) {
- return a.first == b.first && a.second.value == b.second.value; // Skipping 'sensitive' field from ng::header_value which does not seem important for us.
- });
- }
-
- bool operator==(const Response& o) const
- {
- return equalStatusCodeAndHeaders(o) && data == o.data;
- }
-
- static ng::header_map transformHeaders(const Headers& headers)
- {
- ng::header_map res;
- std::transform(headers.begin(), headers.end(), std::inserter(res, res.end()), [](const auto& h) -> std::pair<std::string, ng::header_value> { return {h.first, {h.second, false}}; });
- return res;
- }
-};
-
-namespace doctest {
-
-template <>
-struct StringMaker<ng::header_map> {
- static String convert(const ng::header_map& m)
- {
- std::ostringstream oss;
- oss << "{\n";
- for (const auto& [k, v] : m) {
- oss << "\t"
- << "{\"" << k << "\", "
- << "{\"" << v.value << "\", " << std::boolalpha << v.sensitive << "}},\n";
- }
- oss << "}";
- return oss.str().c_str();
- }
-};
-
-template <>
-struct StringMaker<Response> {
- static String convert(const Response& o)
- {
- std::ostringstream oss;
-
- oss << "{"
- << std::to_string(o.statusCode) << ", "
- << StringMaker<decltype(o.headers)>::convert(o.headers) << ",\n"
- << "\"" << o.data << "\",\n"
- << "}";
-
- return oss.str().c_str();
- }
-};
+namespace sysrepo {
+class Session;
}
+namespace ng = nghttp2::asio_http2;
+
static const auto SERVER_ADDRESS = "::1";
-static const auto SERVER_ADDRESS_AND_PORT = "http://["s + SERVER_ADDRESS + "]" + ":" + SERVER_PORT;
#define AUTH_DWDM {"authorization", "Basic ZHdkbTpEV0RN"}
#define AUTH_NORULES {"authorization", "Basic bm9ydWxlczplbXB0eQ=="}
@@ -145,7 +62,7 @@ const ng::header_map plaintextHeaders{
{"content-type", {"text/plain", false}},
};
-const ng::header_map eventStreamHeaders {
+const ng::header_map eventStreamHeaders{
{"access-control-allow-origin", {"*", false}},
{"content-type", {"text/event-stream", false}},
};
@@ -153,142 +70,37 @@ const ng::header_map eventStreamHeaders {
#define ACCESS_CONTROL_ALLOW_ORIGIN {"access-control-allow-origin", "*"}
#define ACCEPT_PATCH {"accept-patch", "application/yang-data+json, application/yang-data+xml, application/yang-patch+xml, application/yang-patch+json"}
-// this is a test, and the server is expected to reply "soon"
-static const boost::posix_time::time_duration CLIENT_TIMEOUT = boost::posix_time::seconds(3);
-
-Response clientRequest(auto method,
- auto uri,
- const std::string& data,
- const std::map<std::string, std::string>& headers,
- const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
-{
- boost::asio::io_service io_service;
- auto client = std::make_shared<ng_client::session>(io_service, SERVER_ADDRESS, SERVER_PORT);
-
- client->read_timeout(timeout);
-
- std::ostringstream oss;
- ng::header_map resHeaders;
- int statusCode;
-
- client->on_connect([&](auto) {
- boost::system::error_code ec;
-
- ng::header_map reqHeaders;
- for (const auto& [name, value] : headers) {
- reqHeaders.insert({name, {value, false}});
- }
-
- auto req = client->submit(ec, method, SERVER_ADDRESS_AND_PORT + uri, data, reqHeaders);
- req->on_response([&](const ng_client::response& res) {
- res.on_data([&oss](const uint8_t* data, std::size_t len) {
- oss.write(reinterpret_cast<const char*>(data), len);
- });
- statusCode = res.status_code();
- resHeaders = res.header();
- });
- req->on_close([maybeClient = std::weak_ptr<ng_client::session>{client}](auto) {
- if (auto client = maybeClient.lock()) {
- client->shutdown();
- }
- });
- });
- client->on_error([](const boost::system::error_code& ec) {
- throw std::runtime_error{"HTTP client error: " + ec.message()};
- });
- io_service.run();
-
- return {statusCode, resHeaders, oss.str()};
-}
-
Response get(auto uri, const std::map<std::string, std::string>& headers, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("GET", uri, "", headers, timeout);
+ return clientRequest(SERVER_ADDRESS, SERVER_PORT, "GET", uri, "", headers, timeout);
}
Response options(auto uri, const std::map<std::string, std::string>& headers, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("OPTIONS", uri, "", headers, timeout);
+ return clientRequest(SERVER_ADDRESS, SERVER_PORT, "OPTIONS", uri, "", headers, timeout);
}
Response head(auto uri, const std::map<std::string, std::string>& headers, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("HEAD", uri, "", headers, timeout);
+ return clientRequest(SERVER_ADDRESS, SERVER_PORT, "HEAD", uri, "", headers, timeout);
}
Response put(auto xpath, const std::map<std::string, std::string>& headers, const std::string& data, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("PUT", xpath, data, headers, timeout);
+ return clientRequest(SERVER_ADDRESS, SERVER_PORT, "PUT", xpath, data, headers, timeout);
}
Response post(auto xpath, const std::map<std::string, std::string>& headers, const std::string& data, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("POST", xpath, data, headers, timeout);
+ return clientRequest(SERVER_ADDRESS, SERVER_PORT, "POST", xpath, data, headers, timeout);
}
Response patch(auto uri, const std::map<std::string, std::string>& headers, const std::string& data, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("PATCH", uri, data, headers, timeout);
+ return clientRequest(SERVER_ADDRESS, SERVER_PORT, "PATCH", uri, data, headers, timeout);
}
Response httpDelete(auto uri, const std::map<std::string, std::string>& headers, const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT)
{
- return clientRequest("DELETE", uri, "", headers, timeout);
-}
-
-auto manageNacm(sysrepo::Session session)
-{
- return make_unique_resource(
- [session]() mutable {
- session.switchDatastore(sysrepo::Datastore::Running);
- session.copyConfig(sysrepo::Datastore::Startup, "ietf-netconf-acm");
- },
- [session]() mutable {
- session.switchDatastore(sysrepo::Datastore::Running);
-
- /* cleanup running DS of ietf-netconf-acm module
- because it contains XPaths to other modules that we
- can't uninstall because the running DS content would be invalid
- */
- session.copyConfig(sysrepo::Datastore::Startup, "ietf-netconf-acm");
- });
-}
-
-void setupRealNacm(sysrepo::Session session)
-{
- session.switchDatastore(sysrepo::Datastore::Running);
- session.setItem("/ietf-netconf-acm:nacm/enable-external-groups", "false");
- session.setItem("/ietf-netconf-acm:nacm/groups/group[name='optics']/user-name[.='dwdm']", "");
- session.setItem("/ietf-netconf-acm:nacm/groups/group[name='yangnobody']/user-name[.='yangnobody']", "");
- session.setItem("/ietf-netconf-acm:nacm/groups/group[name='norules']/user-name[.='norules']", "");
-
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/group[.='yangnobody']", "");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='10']/module-name", "ietf-system");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='10']/action", "permit");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='10']/access-operations", "read");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='10']/path", "/ietf-system:system/contact");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='11']/module-name", "ietf-system");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='11']/action", "permit");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='11']/access-operations", "read");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='11']/path", "/ietf-system:system/hostname");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='12']/module-name", "ietf-system");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='12']/action", "permit");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='12']/access-operations", "read");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='12']/path", "/ietf-system:system/location");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='13']/module-name", "example");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='13']/action", "permit");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='13']/access-operations", "read");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='14']/module-name", "ietf-restconf-monitoring");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='14']/action", "permit");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='14']/access-operations", "read");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='15']/module-name", "example-delete");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='15']/action", "permit");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='15']/access-operations", "read");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='15']/path", "/example-delete:immutable");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='99']/module-name", "*");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='99']/action", "deny");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='dwdm rule']/group[.='optics']", "");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='dwdm rule']/rule[name='1']/module-name", "ietf-system");
- session.setItem("/ietf-netconf-acm:nacm/rule-list[name='dwdm rule']/rule[name='1']/action", "permit"); // overrides nacm:default-deny-* rules in ietf-system model
- session.applyChanges();
+ return clientRequest(SERVER_ADDRESS, SERVER_PORT, "DELETE", uri, "", headers, timeout);
}
diff --git a/tests/restconf-notifications.cpp b/tests/restconf-notifications.cpp
index d479f3c..db9a3f5 100644
--- a/tests/restconf-notifications.cpp
+++ b/tests/restconf-notifications.cpp
@@ -87,7 +87,8 @@ struct SSEClient {
client->on_connect([&, uri, reqHeaders, silenceTimeout](auto) {
boost::system::error_code ec;
- auto req = client->submit(ec, "GET", SERVER_ADDRESS_AND_PORT + uri, "", reqHeaders);
+ static const auto server_address_and_port = std::string("http://[") + SERVER_ADDRESS + "]" + ":" + SERVER_PORT;
+ auto req = client->submit(ec, "GET", server_address_and_port + uri, "", reqHeaders);
req->on_response([&, silenceTimeout](const ng_client::response& res) {
requestSent.count_down();
res.on_data([&, silenceTimeout](const uint8_t* data, std::size_t len) {
diff --git a/tests/restconf_utils.cpp b/tests/restconf_utils.cpp
new file mode 100644
index 0000000..ea9a18d
--- /dev/null
+++ b/tests/restconf_utils.cpp
@@ -0,0 +1,157 @@
+/*
+ * Copyright (C) 2023 CESNET, https://photonics.cesnet.cz/
+ *
+ * Written by Tomáš Pecka <tomas.pecka@cesnet.cz>
+ * Written by Jan Kundrát <jan.kundrat@cesnet.cz>
+ *
+ */
+
+#include "restconf_utils.h"
+#include "sysrepo-cpp/Session.hpp"
+
+using namespace std::string_literals;
+namespace ng = nghttp2::asio_http2;
+namespace ng_client = ng::client;
+
+Response::Response(int statusCode, const Response::Headers& headers, const std::string& data)
+ : Response(statusCode, transformHeaders(headers), data)
+{
+}
+
+Response::Response(int statusCode, const ng::header_map& headers, const std::string& data)
+ : statusCode(statusCode)
+ , headers(headers)
+ , data(data)
+{
+}
+
+bool Response::equalStatusCodeAndHeaders(const Response& o) const
+{
+ // Skipping 'date' header. Its value will not be reproducible in simple tests
+ ng::header_map myHeaders(headers);
+ ng::header_map otherHeaders(o.headers);
+ myHeaders.erase("date");
+ otherHeaders.erase("date");
+
+ return statusCode == o.statusCode && std::equal(myHeaders.begin(), myHeaders.end(), otherHeaders.begin(), otherHeaders.end(), [](const auto& a, const auto& b) {
+ return a.first == b.first && a.second.value == b.second.value; // Skipping 'sensitive' field from ng::header_value which does not seem important for us.
+ });
+}
+
+bool Response::operator==(const Response& o) const
+{
+ return equalStatusCodeAndHeaders(o) && data == o.data;
+}
+
+ng::header_map Response::transformHeaders(const Response::Headers& headers)
+{
+ ng::header_map res;
+ std::transform(headers.begin(), headers.end(), std::inserter(res, res.end()), [](const auto& h) -> std::pair<std::string, ng::header_value> { return {h.first, {h.second, false}}; });
+ return res;
+}
+
+Response clientRequest(const std::string& server_address,
+ const std::string& server_port,
+ const std::string& method,
+ const std::string& uri,
+ const std::string& data,
+ const std::map<std::string, std::string>& headers,
+ const boost::posix_time::time_duration timeout)
+{
+ boost::asio::io_service io_service;
+ auto client = std::make_shared<ng_client::session>(io_service, server_address, server_port);
+
+ client->read_timeout(timeout);
+
+ std::ostringstream oss;
+ ng::header_map resHeaders;
+ int statusCode;
+
+ client->on_connect([&](auto) {
+ boost::system::error_code ec;
+
+ ng::header_map reqHeaders;
+ for (const auto& [name, value] : headers) {
+ reqHeaders.insert({name, {value, false}});
+ }
+
+ const auto server_address_and_port = std::string("http://[") + server_address + "]" + ":" + server_port;
+ auto req = client->submit(ec, method, server_address_and_port + uri, data, reqHeaders);
+ req->on_response([&](const ng_client::response& res) {
+ res.on_data([&oss](const uint8_t* data, std::size_t len) {
+ oss.write(reinterpret_cast<const char*>(data), len);
+ });
+ statusCode = res.status_code();
+ resHeaders = res.header();
+ });
+ req->on_close([maybeClient = std::weak_ptr<ng_client::session>{client}](auto) {
+ if (auto client = maybeClient.lock()) {
+ client->shutdown();
+ }
+ });
+ });
+ client->on_error([](const boost::system::error_code& ec) {
+ throw std::runtime_error{"HTTP client error: " + ec.message()};
+ });
+ io_service.run();
+
+ return {statusCode, resHeaders, oss.str()};
+}
+
+UniqueResource manageNacm(sysrepo::Session session)
+{
+ return make_unique_resource(
+ [session]() mutable {
+ session.switchDatastore(sysrepo::Datastore::Running);
+ session.copyConfig(sysrepo::Datastore::Startup, "ietf-netconf-acm");
+ },
+ [session]() mutable {
+ session.switchDatastore(sysrepo::Datastore::Running);
+
+ /* cleanup running DS of ietf-netconf-acm module
+ because it contains XPaths to other modules that we
+ can't uninstall because the running DS content would be invalid
+ */
+ session.copyConfig(sysrepo::Datastore::Startup, "ietf-netconf-acm");
+ });
+}
+
+void setupRealNacm(sysrepo::Session session)
+{
+ session.switchDatastore(sysrepo::Datastore::Running);
+ session.setItem("/ietf-netconf-acm:nacm/enable-external-groups", "false");
+ session.setItem("/ietf-netconf-acm:nacm/groups/group[name='optics']/user-name[.='dwdm']", "");
+ session.setItem("/ietf-netconf-acm:nacm/groups/group[name='yangnobody']/user-name[.='yangnobody']", "");
+ session.setItem("/ietf-netconf-acm:nacm/groups/group[name='norules']/user-name[.='norules']", "");
+
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/group[.='yangnobody']", "");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='10']/module-name", "ietf-system");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='10']/action", "permit");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='10']/access-operations", "read");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='10']/path", "/ietf-system:system/contact");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='11']/module-name", "ietf-system");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='11']/action", "permit");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='11']/access-operations", "read");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='11']/path", "/ietf-system:system/hostname");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='12']/module-name", "ietf-system");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='12']/action", "permit");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='12']/access-operations", "read");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='12']/path", "/ietf-system:system/location");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='13']/module-name", "example");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='13']/action", "permit");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='13']/access-operations", "read");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='14']/module-name", "ietf-restconf-monitoring");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='14']/action", "permit");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='14']/access-operations", "read");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='15']/module-name", "example-delete");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='15']/action", "permit");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='15']/access-operations", "read");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='15']/path", "/example-delete:immutable");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='99']/module-name", "*");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='anon rule']/rule[name='99']/action", "deny");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='dwdm rule']/group[.='optics']", "");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='dwdm rule']/rule[name='1']/module-name", "ietf-system");
+ session.setItem("/ietf-netconf-acm:nacm/rule-list[name='dwdm rule']/rule[name='1']/action", "permit"); // overrides nacm:default-deny-* rules in ietf-system model
+ session.applyChanges();
+}
+
diff --git a/tests/restconf_utils.h b/tests/restconf_utils.h
new file mode 100644
index 0000000..26f0803
--- /dev/null
+++ b/tests/restconf_utils.h
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2023 CESNET, https://photonics.cesnet.cz/
+ *
+ * Written by Tomáš Pecka <tomas.pecka@cesnet.cz>
+ * Written by Jan Kundrát <jan.kundrat@cesnet.cz>
+ *
+ */
+
+#pragma once
+#include "trompeloeil_doctest.h"
+#include <nghttp2/asio_http2_client.h>
+#include "UniqueResource.h"
+
+namespace sysrepo {
+class Session;
+}
+
+namespace ng = nghttp2::asio_http2;
+namespace ng_client = ng::client;
+
+struct Response {
+ int statusCode;
+ ng::header_map headers;
+ std::string data;
+
+ using Headers = std::multimap<std::string, std::string>;
+
+ Response(int statusCode, const Headers& headers, const std::string& data);
+ Response(int statusCode, const ng::header_map& headers, const std::string& data);
+ bool equalStatusCodeAndHeaders(const Response& o) const;
+ bool operator==(const Response& o) const;
+ static ng::header_map transformHeaders(const Headers& headers);
+};
+
+namespace doctest {
+
+template <>
+struct StringMaker<ng::header_map> {
+ static String convert(const ng::header_map& m)
+ {
+ std::ostringstream oss;
+ oss << "{\n";
+ for (const auto& [k, v] : m) {
+ oss << "\t"
+ << "{\"" << k << "\", "
+ << "{\"" << v.value << "\", " << std::boolalpha << v.sensitive << "}},\n";
+ }
+ oss << "}";
+ return oss.str().c_str();
+ }
+};
+
+template <>
+struct StringMaker<Response> {
+ static String convert(const Response& o)
+ {
+ std::ostringstream oss;
+
+ oss << "{"
+ << std::to_string(o.statusCode) << ", "
+ << StringMaker<decltype(o.headers)>::convert(o.headers) << ",\n"
+ << "\"" << o.data << "\",\n"
+ << "}";
+
+ return oss.str().c_str();
+ }
+};
+}
+
+// this is a test, and the server is expected to reply "soon"
+static const boost::posix_time::time_duration CLIENT_TIMEOUT = boost::posix_time::seconds(3);
+
+Response clientRequest(
+ const std::string& server_address,
+ const std::string& server_port,
+ const std::string& method,
+ const std::string& uri,
+ const std::string& data,
+ const std::map<std::string, std::string>& headers,
+ const boost::posix_time::time_duration timeout = CLIENT_TIMEOUT);
+
+UniqueResource manageNacm(sysrepo::Session session);
+void setupRealNacm(sysrepo::Session session);
--
2.43.0
package/rousette/0014-tests-rename-datastoreUtils-to-event_watchers.patch
+169
View File
@@ -0,0 +1,169 @@
From 53aa2e23ee8acc1881487f3969c2c58fb29437be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Mon, 2 Dec 2024 20:08:53 +0100
Subject: [PATCH 14/20] tests: rename datastoreUtils to event_watchers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
The next commit will move RESTCONF notification watchers there, I think
the new name is more appropriate.
Change-Id: Ia8e8cd5fe89bd827fcde4531fe801298bd6f71d2
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
CMakeLists.txt | 2 +-
tests/{datastoreUtils.cpp => event_watchers.cpp} | 2 +-
tests/{datastoreUtils.h => event_watchers.h} | 0
tests/pretty_printers.h | 2 +-
tests/restconf-defaults.cpp | 2 +-
tests/restconf-delete.cpp | 2 +-
tests/restconf-plain-patch.cpp | 2 +-
tests/restconf-reading.cpp | 2 +-
tests/restconf-rpc.cpp | 2 +-
tests/restconf-writing.cpp | 2 +-
tests/restconf-yang-patch.cpp | 2 +-
11 files changed, 10 insertions(+), 10 deletions(-)
rename tests/{datastoreUtils.cpp => event_watchers.cpp} (98%)
rename tests/{datastoreUtils.h => event_watchers.h} (100%)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b8a41a7..22bce32 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -156,8 +156,8 @@ if(BUILD_TESTING)
include(cmake/SysrepoTest.cmake)
add_library(DoctestIntegration STATIC
- tests/datastoreUtils.cpp
tests/doctest_integration.cpp
+ tests/event_watchers.cpp
tests/restconf_utils.cpp
tests/trompeloeil_doctest.h
tests/wait-a-bit-longer.cpp
diff --git a/tests/datastoreUtils.cpp b/tests/event_watchers.cpp
similarity index 98%
rename from tests/datastoreUtils.cpp
rename to tests/event_watchers.cpp
index 56d8090..c696bc5 100644
--- a/tests/datastoreUtils.cpp
+++ b/tests/event_watchers.cpp
@@ -1,5 +1,5 @@
#include "UniqueResource.h"
-#include "datastoreUtils.h"
+#include "event_watchers.h"
namespace {
void datastoreChanges(auto session, auto& dsChangesMock, auto path)
diff --git a/tests/datastoreUtils.h b/tests/event_watchers.h
similarity index 100%
rename from tests/datastoreUtils.h
rename to tests/event_watchers.h
diff --git a/tests/pretty_printers.h b/tests/pretty_printers.h
index a2befeb..ce64e91 100644
--- a/tests/pretty_printers.h
+++ b/tests/pretty_printers.h
@@ -13,7 +13,7 @@
#include <optional>
#include <sstream>
#include <trompeloeil.hpp>
-#include "datastoreUtils.h"
+#include "event_watchers.h"
#include "restconf/uri.h"
#include "restconf/uri_impl.h"
diff --git a/tests/restconf-defaults.cpp b/tests/restconf-defaults.cpp
index 129dae2..dd8b4da 100644
--- a/tests/restconf-defaults.cpp
+++ b/tests/restconf-defaults.cpp
@@ -11,7 +11,7 @@ static const auto SERVER_PORT = "10087";
#include <spdlog/spdlog.h>
#include "restconf/Server.h"
#include "tests/aux-utils.h"
-#include "tests/datastoreUtils.h"
+#include "tests/event_watchers.h"
TEST_CASE("default handling")
{
diff --git a/tests/restconf-delete.cpp b/tests/restconf-delete.cpp
index 75a6916..4818ff3 100644
--- a/tests/restconf-delete.cpp
+++ b/tests/restconf-delete.cpp
@@ -10,7 +10,7 @@ static const auto SERVER_PORT = "10086";
#include <spdlog/spdlog.h>
#include "restconf/Server.h"
#include "tests/aux-utils.h"
-#include "tests/datastoreUtils.h"
+#include "tests/event_watchers.h"
#include "tests/pretty_printers.h"
TEST_CASE("deleting data")
diff --git a/tests/restconf-plain-patch.cpp b/tests/restconf-plain-patch.cpp
index d4f3952..b550f54 100644
--- a/tests/restconf-plain-patch.cpp
+++ b/tests/restconf-plain-patch.cpp
@@ -10,7 +10,7 @@ static const auto SERVER_PORT = "10089";
#include <spdlog/spdlog.h>
#include "restconf/Server.h"
#include "tests/aux-utils.h"
-#include "tests/datastoreUtils.h"
+#include "tests/event_watchers.h"
#include "tests/pretty_printers.h"
TEST_CASE("Plain patch")
diff --git a/tests/restconf-reading.cpp b/tests/restconf-reading.cpp
index d7d507b..e709486 100644
--- a/tests/restconf-reading.cpp
+++ b/tests/restconf-reading.cpp
@@ -11,7 +11,7 @@ static const auto SERVER_PORT = "10081";
#include <nghttp2/asio_http2.h>
#include <spdlog/spdlog.h>
#include "restconf/Server.h"
-#include "tests/datastoreUtils.h"
+#include "tests/event_watchers.h"
TEST_CASE("reading data")
{
diff --git a/tests/restconf-rpc.cpp b/tests/restconf-rpc.cpp
index 9bc1dbc..c4229a0 100644
--- a/tests/restconf-rpc.cpp
+++ b/tests/restconf-rpc.cpp
@@ -10,7 +10,7 @@ static const auto SERVER_PORT = "10084";
#include <spdlog/spdlog.h>
#include "restconf/Server.h"
#include "tests/aux-utils.h"
-#include "tests/datastoreUtils.h"
+#include "tests/event_watchers.h"
#include "tests/pretty_printers.h"
struct RpcCall {
diff --git a/tests/restconf-writing.cpp b/tests/restconf-writing.cpp
index 582a262..0932984 100644
--- a/tests/restconf-writing.cpp
+++ b/tests/restconf-writing.cpp
@@ -10,7 +10,7 @@ static const auto SERVER_PORT = "10083";
#include <spdlog/spdlog.h>
#include "restconf/Server.h"
#include "tests/aux-utils.h"
-#include "tests/datastoreUtils.h"
+#include "tests/event_watchers.h"
#include "tests/pretty_printers.h"
TEST_CASE("writing data")
diff --git a/tests/restconf-yang-patch.cpp b/tests/restconf-yang-patch.cpp
index 7cc8946..2b35c59 100644
--- a/tests/restconf-yang-patch.cpp
+++ b/tests/restconf-yang-patch.cpp
@@ -10,7 +10,7 @@ static const auto SERVER_PORT = "10090";
#include <spdlog/spdlog.h>
#include "restconf/Server.h"
#include "tests/aux-utils.h"
-#include "tests/datastoreUtils.h"
+#include "tests/event_watchers.h"
#include "tests/pretty_printers.h"
TEST_CASE("YANG patch")
--
2.43.0
package/rousette/0015-tests-rename-NotificationWatcher.patch
+57
View File
@@ -0,0 +1,57 @@
From f86df829979a26d5192a86c3b43c1393dcba7140 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Tue, 3 Dec 2024 11:45:41 +0100
Subject: [PATCH 15/20] tests: rename NotificationWatcher
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
This watches notifications going through RESTCONF, so I think
RestconfNotificatonWatcher is better name for this.
Change-Id: Ieb8d7ce489e683348f065e9e84aae06af7c8ccc4
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
tests/restconf-notifications.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/restconf-notifications.cpp b/tests/restconf-notifications.cpp
index db9a3f5..d873512 100644
--- a/tests/restconf-notifications.cpp
+++ b/tests/restconf-notifications.cpp
@@ -21,11 +21,11 @@ static const auto SERVER_PORT = "10088";
using namespace std::chrono_literals;
-struct NotificationWatcher {
+struct RestconfNotificationWatcher {
libyang::Context ctx;
libyang::DataFormat dataFormat;
- NotificationWatcher(const libyang::Context& ctx)
+ RestconfNotificationWatcher(const libyang::Context& ctx)
: ctx(ctx)
, dataFormat(libyang::DataFormat::JSON)
{
@@ -62,7 +62,7 @@ struct SSEClient {
SSEClient(
boost::asio::io_service& io,
std::latch& requestSent,
- const NotificationWatcher& notification,
+ const RestconfNotificationWatcher& notification,
const std::string& uri,
const std::map<std::string, std::string>& headers,
const boost::posix_time::seconds silenceTimeout = boost::posix_time::seconds(1)) // test code; the server should respond "soon"
@@ -186,7 +186,7 @@ TEST_CASE("NETCONF notification streams")
R"({"example:tlc":{"list":[{"name":"k1","notif":{"message":"nested"}}]}})",
};
- NotificationWatcher netconfWatcher(srConn.sessionStart().getContext());
+ RestconfNotificationWatcher netconfWatcher(srConn.sessionStart().getContext());
SECTION("NETCONF streams")
{
--
2.43.0
package/rousette/0016-tests-make-NotificationWatcher-reusable.patch
+133
View File
@@ -0,0 +1,133 @@
From 30d704588fa7eb9d32f66296ec5f6784f082869e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Mon, 2 Dec 2024 20:11:32 +0100
Subject: [PATCH 16/20] tests: make NotificationWatcher reusable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
We will need some parts of this in yang push tests
Change-Id: I72e60d993f75bbc848af096ac325a75fa3dea61a
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
tests/event_watchers.cpp | 28 +++++++++++++++++++++++++
tests/event_watchers.h | 13 ++++++++++++
tests/restconf-notifications.cpp | 35 --------------------------------
3 files changed, 41 insertions(+), 35 deletions(-)
diff --git a/tests/event_watchers.cpp b/tests/event_watchers.cpp
index c696bc5..bde96ec 100644
--- a/tests/event_watchers.cpp
+++ b/tests/event_watchers.cpp
@@ -1,3 +1,4 @@
+#include <spdlog/spdlog.h>
#include "UniqueResource.h"
#include "event_watchers.h"
@@ -50,3 +51,30 @@ sysrepo::Subscription datastoreNewStateSubscription(sysrepo::Session& session, D
0,
sysrepo::SubscribeOptions::DoneOnly);
}
+
+RestconfNotificationWatcher::RestconfNotificationWatcher(const libyang::Context& ctx)
+ : ctx(ctx)
+ , dataFormat(libyang::DataFormat::JSON)
+{
+}
+
+void RestconfNotificationWatcher::setDataFormat(const libyang::DataFormat dataFormat)
+{
+ this->dataFormat = dataFormat;
+}
+
+void RestconfNotificationWatcher::operator()(const std::string& msg) const
+{
+ spdlog::trace("Client received data: {}", msg);
+ auto notifDataNode = ctx.parseOp(msg,
+ dataFormat,
+ dataFormat == libyang::DataFormat::JSON ? libyang::OperationType::NotificationRestconf : libyang::OperationType::NotificationNetconf);
+
+ // parsing nested notifications does not return the data tree root node but the notification data node
+ auto dataRoot = notifDataNode.op;
+ while (dataRoot->parent()) {
+ dataRoot = *dataRoot->parent();
+ }
+
+ data(*dataRoot->printStr(libyang::DataFormat::JSON, libyang::PrintFlags::Shrink));
+}
diff --git a/tests/event_watchers.h b/tests/event_watchers.h
index 5b2429c..b6f2dd2 100644
--- a/tests/event_watchers.h
+++ b/tests/event_watchers.h
@@ -32,3 +32,16 @@ static DatastoreChangesMock testMockForUntrackedModuleWrites;
#define SUBSCRIBE_MODULE(SUBNAME, SESSION, MODULE) \
ALLOW_CALL(testMockForUntrackedModuleWrites, change(trompeloeil::_)); \
auto SUBNAME = datastoreChangesSubscription(SESSION, testMockForUntrackedModuleWrites, MODULE);
+
+struct RestconfNotificationWatcher {
+ libyang::Context ctx;
+ libyang::DataFormat dataFormat;
+
+ RestconfNotificationWatcher(const libyang::Context& ctx);
+ void setDataFormat(const libyang::DataFormat dataFormat);
+ void operator()(const std::string& msg) const;
+
+ MAKE_CONST_MOCK1(data, void(const std::string&));
+};
+
+#define EXPECT_NOTIFICATION(DATA, SEQ) expectations.emplace_back(NAMED_REQUIRE_CALL(netconfWatcher, data(DATA)).IN_SEQUENCE(SEQ));
diff --git a/tests/restconf-notifications.cpp b/tests/restconf-notifications.cpp
index d873512..6c8c51a 100644
--- a/tests/restconf-notifications.cpp
+++ b/tests/restconf-notifications.cpp
@@ -16,45 +16,10 @@ static const auto SERVER_PORT = "10088";
#include "tests/aux-utils.h"
#include "tests/pretty_printers.h"
-#define EXPECT_NOTIFICATION(DATA, SEQ) expectations.emplace_back(NAMED_REQUIRE_CALL(netconfWatcher, data(DATA)).IN_SEQUENCE(SEQ));
#define SEND_NOTIFICATION(DATA) notifSession.sendNotification(*ctx.parseOp(DATA, libyang::DataFormat::JSON, libyang::OperationType::NotificationYang).op, sysrepo::Wait::No);
using namespace std::chrono_literals;
-struct RestconfNotificationWatcher {
- libyang::Context ctx;
- libyang::DataFormat dataFormat;
-
- RestconfNotificationWatcher(const libyang::Context& ctx)
- : ctx(ctx)
- , dataFormat(libyang::DataFormat::JSON)
- {
- }
-
- void setDataFormat(const libyang::DataFormat dataFormat)
- {
- this->dataFormat = dataFormat;
- }
-
- void operator()(const std::string& msg) const
- {
- spdlog::trace("Client received data: {}", msg);
- auto notifDataNode = ctx.parseOp(msg,
- dataFormat,
- dataFormat == libyang::DataFormat::JSON ? libyang::OperationType::NotificationRestconf : libyang::OperationType::NotificationNetconf);
-
- // parsing nested notifications does not return the data tree root node but the notification data node
- auto dataRoot = notifDataNode.op;
- while (dataRoot->parent()) {
- dataRoot = *dataRoot->parent();
- }
-
- data(*dataRoot->printStr(libyang::DataFormat::JSON, libyang::PrintFlags::Shrink));
- }
-
- MAKE_CONST_MOCK1(data, void(const std::string&));
-};
-
struct SSEClient {
std::shared_ptr<ng_client::session> client;
boost::asio::deadline_timer t;
--
2.43.0
package/rousette/0017-tests-helper-function-to-construct-server-URI.patch
+45
View File
@@ -0,0 +1,45 @@
From 36943051d8563bee0c9cf89eec28acf5d3617272 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Tue, 3 Dec 2024 12:02:24 +0100
Subject: [PATCH 17/20] tests: helper function to construct server URI
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
Change-Id: Ic7c03b32a29b07464291f99027530010a7902f77
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
tests/restconf_utils.cpp | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/restconf_utils.cpp b/tests/restconf_utils.cpp
index ea9a18d..83f568f 100644
--- a/tests/restconf_utils.cpp
+++ b/tests/restconf_utils.cpp
@@ -13,6 +13,12 @@ using namespace std::string_literals;
namespace ng = nghttp2::asio_http2;
namespace ng_client = ng::client;
+namespace {
+std::string serverAddressAndPort(const std::string& server_address, const std::string& server_port) {
+ return "http://["s + server_address + "]" + ":" + server_port;
+}
+}
+
Response::Response(int statusCode, const Response::Headers& headers, const std::string& data)
: Response(statusCode, transformHeaders(headers), data)
{
@@ -75,8 +81,7 @@ Response clientRequest(const std::string& server_address,
reqHeaders.insert({name, {value, false}});
}
- const auto server_address_and_port = std::string("http://[") + server_address + "]" + ":" + server_port;
- auto req = client->submit(ec, method, server_address_and_port + uri, data, reqHeaders);
+ auto req = client->submit(ec, method, serverAddressAndPort(server_address, server_port) + uri, data, reqHeaders);
req->on_response([&](const ng_client::response& res) {
res.on_data([&oss](const uint8_t* data, std::size_t len) {
oss.write(reinterpret_cast<const char*>(data), len);
--
2.43.0
package/rousette/0018-tests-make-SSEClient-reusable.patch
+307
View File
@@ -0,0 +1,307 @@
From 7d15f59d20079ba94224e0bc308682aa5a004483 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Mon, 2 Dec 2024 20:15:05 +0100
Subject: [PATCH 18/20] tests: make SSEClient reusable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
We will need it in yang push tests
Change-Id: I22432553f3abff0de91b3c406abc5567de656065
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
tests/restconf-notifications.cpp | 108 +------------------------------
tests/restconf_utils.cpp | 69 ++++++++++++++++++++
tests/restconf_utils.h | 47 ++++++++++++++
3 files changed, 119 insertions(+), 105 deletions(-)
diff --git a/tests/restconf-notifications.cpp b/tests/restconf-notifications.cpp
index 6c8c51a..4496b63 100644
--- a/tests/restconf-notifications.cpp
+++ b/tests/restconf-notifications.cpp
@@ -7,121 +7,19 @@
#include "trompeloeil_doctest.h"
static const auto SERVER_PORT = "10088";
-#include <latch>
#include <libyang-cpp/Time.hpp>
#include <nghttp2/asio_http2.h>
#include <spdlog/spdlog.h>
#include <sysrepo-cpp/utils/utils.hpp>
#include "restconf/Server.h"
#include "tests/aux-utils.h"
+#include "tests/event_watchers.h"
#include "tests/pretty_printers.h"
#define SEND_NOTIFICATION(DATA) notifSession.sendNotification(*ctx.parseOp(DATA, libyang::DataFormat::JSON, libyang::OperationType::NotificationYang).op, sysrepo::Wait::No);
using namespace std::chrono_literals;
-struct SSEClient {
- std::shared_ptr<ng_client::session> client;
- boost::asio::deadline_timer t;
-
- SSEClient(
- boost::asio::io_service& io,
- std::latch& requestSent,
- const RestconfNotificationWatcher& notification,
- const std::string& uri,
- const std::map<std::string, std::string>& headers,
- const boost::posix_time::seconds silenceTimeout = boost::posix_time::seconds(1)) // test code; the server should respond "soon"
- : client(std::make_shared<ng_client::session>(io, SERVER_ADDRESS, SERVER_PORT))
- , t(io, silenceTimeout)
- {
- ng::header_map reqHeaders;
- for (const auto& [name, value] : headers) {
- reqHeaders.insert({name, {value, false}});
- }
-
- // shutdown the client after a period of no traffic
- t.async_wait([maybeClient = std::weak_ptr<ng_client::session>{client}](const boost::system::error_code& ec) {
- if (ec == boost::asio::error::operation_aborted) {
- return;
- }
- if (auto client = maybeClient.lock()) {
- client->shutdown();
- }
- });
-
- client->on_connect([&, uri, reqHeaders, silenceTimeout](auto) {
- boost::system::error_code ec;
-
- static const auto server_address_and_port = std::string("http://[") + SERVER_ADDRESS + "]" + ":" + SERVER_PORT;
- auto req = client->submit(ec, "GET", server_address_and_port + uri, "", reqHeaders);
- req->on_response([&, silenceTimeout](const ng_client::response& res) {
- requestSent.count_down();
- res.on_data([&, silenceTimeout](const uint8_t* data, std::size_t len) {
- // not a production-ready code. In real-life condition the data received in one callback might probably be incomplete
- for (const auto& event : parseEvents(std::string(reinterpret_cast<const char*>(data), len))) {
- notification(event);
- }
- t.expires_from_now(silenceTimeout);
- });
- });
- });
-
- client->on_error([&](const boost::system::error_code& ec) {
- throw std::runtime_error{"HTTP client error: " + ec.message()};
- });
- }
-
- static std::vector<std::string> parseEvents(const std::string& msg)
- {
- static const std::string prefix = "data:";
-
- std::vector<std::string> res;
- std::istringstream iss(msg);
- std::string line;
- std::string event;
-
- while (std::getline(iss, line)) {
- if (line.compare(0, prefix.size(), prefix) == 0) {
- event += line.substr(prefix.size());
- } else if (line.empty()) {
- res.emplace_back(std::move(event));
- event.clear();
- } else {
- FAIL("Unprefixed response");
- }
- }
- return res;
- }
-};
-
-#define PREPARE_LOOP_WITH_EXCEPTIONS \
- boost::asio::io_service io; \
- std::promise<void> bg; \
- std::latch requestSent(1);
-
-#define RUN_LOOP_WITH_EXCEPTIONS \
- do { \
- io.run(); \
- auto fut = bg.get_future(); \
- REQUIRE(fut.wait_for(666ms /* "plenty of time" for the notificationThread to exit after it has called io.stop() */) == std::future_status::ready); \
- fut.get(); \
- } while (false)
-
-auto wrap_exceptions_and_asio(std::promise<void>& bg, boost::asio::io_service& io, std::function<void()> func)
-{
- return [&bg, &io, func]()
- {
- try {
- func();
- } catch (...) {
- bg.set_exception(std::current_exception());
- return;
- }
- bg.set_value();
- io.stop();
- };
-}
-
TEST_CASE("NETCONF notification streams")
{
trompeloeil::sequence seqMod1, seqMod2;
@@ -237,7 +135,7 @@ TEST_CASE("NETCONF notification streams")
waitForCompletionAndBitMore(seqMod2);
}));
- SSEClient cli(io, requestSent, netconfWatcher, uri, headers);
+ SSEClient cli(io, SERVER_ADDRESS, SERVER_PORT, requestSent, netconfWatcher, uri, headers);
RUN_LOOP_WITH_EXCEPTIONS;
}
@@ -399,7 +297,7 @@ TEST_CASE("NETCONF notification streams")
}
oldNotificationsDone.wait();
- SSEClient cli(io, requestSent, netconfWatcher, uri, {AUTH_ROOT});
+ SSEClient cli(io, SERVER_ADDRESS, SERVER_PORT, requestSent, netconfWatcher, uri, {AUTH_ROOT});
RUN_LOOP_WITH_EXCEPTIONS;
}
}
diff --git a/tests/restconf_utils.cpp b/tests/restconf_utils.cpp
index 83f568f..8252bba 100644
--- a/tests/restconf_utils.cpp
+++ b/tests/restconf_utils.cpp
@@ -160,3 +160,72 @@ void setupRealNacm(sysrepo::Session session)
session.applyChanges();
}
+SSEClient::SSEClient(
+ boost::asio::io_service& io,
+ const std::string& server_address,
+ const std::string& server_port,
+ std::latch& requestSent,
+ const RestconfNotificationWatcher& notification,
+ const std::string& uri,
+ const std::map<std::string, std::string>& headers,
+ const boost::posix_time::seconds silenceTimeout)
+ : client(std::make_shared<ng_client::session>(io, server_address, server_port))
+ , t(io, silenceTimeout)
+{
+ ng::header_map reqHeaders;
+ for (const auto& [name, value] : headers) {
+ reqHeaders.insert({name, {value, false}});
+ }
+
+ // shutdown the client after a period of no traffic
+ t.async_wait([maybeClient = std::weak_ptr<ng_client::session>{client}](const boost::system::error_code& ec) {
+ if (ec == boost::asio::error::operation_aborted) {
+ return;
+ }
+ if (auto client = maybeClient.lock()) {
+ client->shutdown();
+ }
+ });
+
+ client->on_connect([&, uri, reqHeaders, silenceTimeout, server_address, server_port](auto) {
+ boost::system::error_code ec;
+
+ auto req = client->submit(ec, "GET", serverAddressAndPort(server_address, server_port) + uri, "", reqHeaders);
+ req->on_response([&, silenceTimeout](const ng_client::response& res) {
+ requestSent.count_down();
+ res.on_data([&, silenceTimeout](const uint8_t* data, std::size_t len) {
+ // not a production-ready code. In real-life condition the data received in one callback might probably be incomplete
+ for (const auto& event : parseEvents(std::string(reinterpret_cast<const char*>(data), len))) {
+ notification(event);
+ }
+ t.expires_from_now(silenceTimeout);
+ });
+ });
+ });
+
+ client->on_error([&](const boost::system::error_code& ec) {
+ throw std::runtime_error{"HTTP client error: " + ec.message()};
+ });
+}
+
+std::vector<std::string> SSEClient::parseEvents(const std::string& msg)
+{
+ static const std::string prefix = "data:";
+
+ std::vector<std::string> res;
+ std::istringstream iss(msg);
+ std::string line;
+ std::string event;
+
+ while (std::getline(iss, line)) {
+ if (line.compare(0, prefix.size(), prefix) == 0) {
+ event += line.substr(prefix.size());
+ } else if (line.empty()) {
+ res.emplace_back(std::move(event));
+ event.clear();
+ } else {
+ FAIL("Unprefixed response");
+ }
+ }
+ return res;
+}
diff --git a/tests/restconf_utils.h b/tests/restconf_utils.h
index 26f0803..8b7386e 100644
--- a/tests/restconf_utils.h
+++ b/tests/restconf_utils.h
@@ -8,7 +8,9 @@
#pragma once
#include "trompeloeil_doctest.h"
+#include <latch>
#include <nghttp2/asio_http2_client.h>
+#include "event_watchers.h"
#include "UniqueResource.h"
namespace sysrepo {
@@ -81,3 +83,48 @@ Response clientRequest(
UniqueResource manageNacm(sysrepo::Session session);
void setupRealNacm(sysrepo::Session session);
+
+struct SSEClient {
+ std::shared_ptr<ng_client::session> client;
+ boost::asio::deadline_timer t;
+
+ SSEClient(
+ boost::asio::io_service& io,
+ const std::string& server_address,
+ const std::string& server_port,
+ std::latch& requestSent,
+ const RestconfNotificationWatcher& notification,
+ const std::string& uri,
+ const std::map<std::string, std::string>& headers,
+ const boost::posix_time::seconds silenceTimeout = boost::posix_time::seconds(1)); // test code; the server should respond "soon"
+
+ static std::vector<std::string> parseEvents(const std::string& msg);
+};
+
+#define PREPARE_LOOP_WITH_EXCEPTIONS \
+ boost::asio::io_service io; \
+ std::promise<void> bg; \
+ std::latch requestSent(1);
+
+#define RUN_LOOP_WITH_EXCEPTIONS \
+ do { \
+ io.run(); \
+ auto fut = bg.get_future(); \
+ REQUIRE(fut.wait_for(666ms /* "plenty of time" for the notificationThread to exit after it has called io.stop() */) == std::future_status::ready); \
+ fut.get(); \
+ } while (false)
+
+inline auto wrap_exceptions_and_asio(std::promise<void>& bg, boost::asio::io_service& io, std::function<void()> func)
+{
+ return [&bg, &io, func]()
+ {
+ try {
+ func();
+ } catch (...) {
+ bg.set_exception(std::current_exception());
+ return;
+ }
+ bg.set_value();
+ io.stop();
+ };
+}
--
2.43.0
package/rousette/0019-tests-fix-deadlock-in-tests.patch
+139
View File
@@ -0,0 +1,139 @@
From 3e3e492f4801df56226a5aff5d82bfa86c9d3812 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Tue, 3 Dec 2024 13:59:52 +0100
Subject: [PATCH 19/20] tests: fix deadlock in tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
In the case the client never requests (because of misconfiguration or
something else) the test would still be waiting for somebody to
decrement the latch.
Unfortunately std::latch does not allow waiting for specified amount of
time, so this patch replaces it with std::binary_semaphore in which it
is possible to fail the acquiring operation after some time.
Change-Id: I38c007555bb5ab543329d724a16f024a9d80903e
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
tests/restconf-notifications.cpp | 10 +++++-----
tests/restconf_utils.cpp | 4 ++--
tests/restconf_utils.h | 8 +++++---
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/tests/restconf-notifications.cpp b/tests/restconf-notifications.cpp
index 4496b63..04131d7 100644
--- a/tests/restconf-notifications.cpp
+++ b/tests/restconf-notifications.cpp
@@ -7,6 +7,7 @@
#include "trompeloeil_doctest.h"
static const auto SERVER_PORT = "10088";
+#include <latch>
#include <libyang-cpp/Time.hpp>
#include <nghttp2/asio_http2.h>
#include <spdlog/spdlog.h>
@@ -119,8 +120,7 @@ TEST_CASE("NETCONF notification streams")
auto notifSession = sysrepo::Connection{}.sessionStart();
auto ctx = notifSession.getContext();
- // wait until the client sends its HTTP request
- requestSent.wait();
+ WAIT_UNTIL_SSE_CLIENT_REQUESTS;
SEND_NOTIFICATION(notificationsJSON[0]);
SEND_NOTIFICATION(notificationsJSON[1]);
@@ -239,7 +239,7 @@ TEST_CASE("NETCONF notification streams")
SEND_NOTIFICATION(notificationsJSON[3]);
SEND_NOTIFICATION(notificationsJSON[4]);
oldNotificationsDone.count_down();
- requestSent.wait();
+ WAIT_UNTIL_SSE_CLIENT_REQUESTS;
waitForCompletionAndBitMore(seqMod1);
waitForCompletionAndBitMore(seqMod2);
@@ -256,7 +256,7 @@ TEST_CASE("NETCONF notification streams")
SEND_NOTIFICATION(notificationsJSON[1]);
oldNotificationsDone.count_down();
- requestSent.wait();
+ WAIT_UNTIL_SSE_CLIENT_REQUESTS;
SEND_NOTIFICATION(notificationsJSON[2]);
SEND_NOTIFICATION(notificationsJSON[3]);
@@ -290,7 +290,7 @@ TEST_CASE("NETCONF notification streams")
SEND_NOTIFICATION(notificationsJSON[4]);
oldNotificationsDone.count_down();
- requestSent.wait();
+ WAIT_UNTIL_SSE_CLIENT_REQUESTS;
waitForCompletionAndBitMore(seqMod1);
waitForCompletionAndBitMore(seqMod2);
}));
diff --git a/tests/restconf_utils.cpp b/tests/restconf_utils.cpp
index 8252bba..c3ff1de 100644
--- a/tests/restconf_utils.cpp
+++ b/tests/restconf_utils.cpp
@@ -164,7 +164,7 @@ SSEClient::SSEClient(
boost::asio::io_service& io,
const std::string& server_address,
const std::string& server_port,
- std::latch& requestSent,
+ std::binary_semaphore& requestSent,
const RestconfNotificationWatcher& notification,
const std::string& uri,
const std::map<std::string, std::string>& headers,
@@ -192,7 +192,7 @@ SSEClient::SSEClient(
auto req = client->submit(ec, "GET", serverAddressAndPort(server_address, server_port) + uri, "", reqHeaders);
req->on_response([&, silenceTimeout](const ng_client::response& res) {
- requestSent.count_down();
+ requestSent.release();
res.on_data([&, silenceTimeout](const uint8_t* data, std::size_t len) {
// not a production-ready code. In real-life condition the data received in one callback might probably be incomplete
for (const auto& event : parseEvents(std::string(reinterpret_cast<const char*>(data), len))) {
diff --git a/tests/restconf_utils.h b/tests/restconf_utils.h
index 8b7386e..9efe398 100644
--- a/tests/restconf_utils.h
+++ b/tests/restconf_utils.h
@@ -8,8 +8,8 @@
#pragma once
#include "trompeloeil_doctest.h"
-#include <latch>
#include <nghttp2/asio_http2_client.h>
+#include <semaphore>
#include "event_watchers.h"
#include "UniqueResource.h"
@@ -92,7 +92,7 @@ struct SSEClient {
boost::asio::io_service& io,
const std::string& server_address,
const std::string& server_port,
- std::latch& requestSent,
+ std::binary_semaphore& requestSent,
const RestconfNotificationWatcher& notification,
const std::string& uri,
const std::map<std::string, std::string>& headers,
@@ -104,7 +104,7 @@ struct SSEClient {
#define PREPARE_LOOP_WITH_EXCEPTIONS \
boost::asio::io_service io; \
std::promise<void> bg; \
- std::latch requestSent(1);
+ std::binary_semaphore requestSent(0);
#define RUN_LOOP_WITH_EXCEPTIONS \
do { \
@@ -114,6 +114,8 @@ struct SSEClient {
fut.get(); \
} while (false)
+#define WAIT_UNTIL_SSE_CLIENT_REQUESTS requestSent.try_acquire_for(std::chrono::seconds(3))
+
inline auto wrap_exceptions_and_asio(std::promise<void>& bg, boost::asio::io_service& io, std::function<void()> func)
{
return [&bg, &io, func]()
--
2.43.0
package/rousette/0020-restconf-make-as_restconf_notification-reusable.patch
+159
View File
@@ -0,0 +1,159 @@
From ed0ff23f7ad341d663484f0b2a617cd3bc4923c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Pecka?= <tomas.pecka@cesnet.cz>
Date: Tue, 3 Dec 2024 19:27:49 +0100
Subject: [PATCH 20/20] restconf: make as_restconf_notification reusable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Addiva Elektronik
The YANG-PUSH notifications are supposed to be wrapped in this too, so
this needs to be accessible from multiple places.
Change-Id: Icf25caf5f3be3917c524bf6111b5d92db6e287b0
Signed-off-by: Mattias Walström <lazzer@gmail.com>
---
src/restconf/NotificationStream.cpp | 37 +--------------------------
src/restconf/utils/yang.cpp | 39 +++++++++++++++++++++++++++++
src/restconf/utils/yang.h | 4 +++
3 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/src/restconf/NotificationStream.cpp b/src/restconf/NotificationStream.cpp
index 001d19c..eeddc04 100644
--- a/src/restconf/NotificationStream.cpp
+++ b/src/restconf/NotificationStream.cpp
@@ -20,41 +20,6 @@ namespace {
const auto streamListXPath = "/ietf-restconf-monitoring:restconf-state/streams/stream"s;
-/** @brief Wraps a notification data tree with RESTCONF notification envelope. */
-std::string as_restconf_notification(const libyang::Context& ctx, libyang::DataFormat dataFormat, libyang::DataNode notification, const sysrepo::NotificationTimeStamp& time)
-{
- static const auto jsonNamespace = "ietf-restconf";
- static const auto xmlNamespace = "urn:ietf:params:xml:ns:netconf:notification:1.0";
-
- std::optional<libyang::DataNode> envelope;
- std::optional<libyang::DataNode> eventTime;
- std::string timeStr = libyang::yangTimeFormat(time, libyang::TimezoneInterpretation::Local);
-
- /* The namespaces for XML and JSON envelopes are different. See https://datatracker.ietf.org/doc/html/rfc8040#section-6.4 */
- if (dataFormat == libyang::DataFormat::JSON) {
- envelope = ctx.newOpaqueJSON(jsonNamespace, "notification", std::nullopt);
- eventTime = ctx.newOpaqueJSON(jsonNamespace, "eventTime", libyang::JSON{timeStr});
- } else {
- envelope = ctx.newOpaqueXML(xmlNamespace, "notification", std::nullopt);
- eventTime = ctx.newOpaqueXML(xmlNamespace, "eventTime", libyang::XML{timeStr});
- }
-
- // the notification data node holds only the notification data tree but for nested notification we should print the whole YANG data tree
- while (notification.parent()) {
- notification = *notification.parent();
- }
-
- envelope->insertChild(*eventTime);
- envelope->insertChild(notification);
-
- auto res = *envelope->printStr(dataFormat, libyang::PrintFlags::WithSiblings);
-
- // notification node comes from sysrepo and sysrepo will free this; if not unlinked then envelope destructor would try to free this as well
- notification.unlink();
-
- return res;
-}
-
void subscribe(
std::optional<sysrepo::Subscription>& sub,
sysrepo::Session& session,
@@ -70,7 +35,7 @@ void subscribe(
return;
}
- signal(as_restconf_notification(session.getContext(), dataFormat, *notificationTree, time));
+ signal(rousette::restconf::as_restconf_notification(session.getContext(), dataFormat, *notificationTree, time));
};
if (!sub) {
diff --git a/src/restconf/utils/yang.cpp b/src/restconf/utils/yang.cpp
index 4c4d619..30661fc 100644
--- a/src/restconf/utils/yang.cpp
+++ b/src/restconf/utils/yang.cpp
@@ -6,8 +6,11 @@
*/
#include <algorithm>
+#include <libyang-cpp/Context.hpp>
#include <libyang-cpp/DataNode.hpp>
#include <libyang-cpp/SchemaNode.hpp>
+#include <libyang-cpp/Time.hpp>
+#include <sysrepo-cpp/Subscription.hpp>
namespace rousette::restconf {
@@ -79,4 +82,40 @@ bool isKeyNode(const libyang::DataNode& maybeList, const libyang::DataNode& node
}
return false;
}
+
+
+/** @brief Wraps a notification data tree with RESTCONF notification envelope. */
+std::string as_restconf_notification(const libyang::Context& ctx, libyang::DataFormat dataFormat, libyang::DataNode notification, const sysrepo::NotificationTimeStamp& time)
+{
+ static const auto jsonNamespace = "ietf-restconf";
+ static const auto xmlNamespace = "urn:ietf:params:xml:ns:netconf:notification:1.0";
+
+ std::optional<libyang::DataNode> envelope;
+ std::optional<libyang::DataNode> eventTime;
+ std::string timeStr = libyang::yangTimeFormat(time, libyang::TimezoneInterpretation::Local);
+
+ /* The namespaces for XML and JSON envelopes are different. See https://datatracker.ietf.org/doc/html/rfc8040#section-6.4 */
+ if (dataFormat == libyang::DataFormat::JSON) {
+ envelope = ctx.newOpaqueJSON(jsonNamespace, "notification", std::nullopt);
+ eventTime = ctx.newOpaqueJSON(jsonNamespace, "eventTime", libyang::JSON{timeStr});
+ } else {
+ envelope = ctx.newOpaqueXML(xmlNamespace, "notification", std::nullopt);
+ eventTime = ctx.newOpaqueXML(xmlNamespace, "eventTime", libyang::XML{timeStr});
+ }
+
+ // the notification data node holds only the notification data tree but for nested notification we should print the whole YANG data tree
+ while (notification.parent()) {
+ notification = *notification.parent();
+ }
+
+ envelope->insertChild(*eventTime);
+ envelope->insertChild(notification);
+
+ auto res = *envelope->printStr(dataFormat, libyang::PrintFlags::WithSiblings);
+
+ // notification node comes from sysrepo and sysrepo will free this; if not unlinked then envelope destructor would try to free this as well
+ notification.unlink();
+
+ return res;
+}
}
diff --git a/src/restconf/utils/yang.h b/src/restconf/utils/yang.h
index e91ba8a..a558eae 100644
--- a/src/restconf/utils/yang.h
+++ b/src/restconf/utils/yang.h
@@ -6,10 +6,13 @@
*/
#include <chrono>
+#include <sysrepo-cpp/Subscription.hpp>
namespace libyang {
class Leaf;
class DataNode;
+class Context;
+enum class DataFormat;
}
namespace rousette::restconf {
@@ -19,4 +22,5 @@ std::string listKeyPredicate(const std::vector<libyang::Leaf>& listKeyLeafs, con
std::string leaflistKeyPredicate(const std::string& keyValue);
bool isUserOrderedList(const libyang::DataNode& node);
bool isKeyNode(const libyang::DataNode& maybeList, const libyang::DataNode& node);
+std::string as_restconf_notification(const libyang::Context& ctx, libyang::DataFormat dataFormat, libyang::DataNode notification, const sysrepo::NotificationTimeStamp& time);
}
--
2.43.0