Restriction of the commands during RPC action
Until now, when using RPC action, there were no restrictions on the
commands you could use. However, with RPC actions, only certain commands
should be used, and using others could lead to unexpected problems. This
problem was first noticed when attempting to exit the RPC action
context, as described in the bug link in the bottom. The issue has been
fixed, and this change provides restrictions to additional commands.
This update introduces a new function called `enforceRpcRestrictions`,
which checks if a command is allowed during RPC action.
The following commands are now blocked entirely during RPC action:
- commit
- discard
- copy
- switch
- prepare
The following commands are blocked when leaving or making changes
outside the RPC action:
- create
- set
- delete
- cd
- move
Other commands were not changed.
Bug: https://tree.taiga.io/project/jktjkt-netconf-cli/issue/223
Bug: https://github.com/CESNET/netconf-cli/issues/19
Change-Id: I5bf935bb8eabeea313b506617d1f43274eb77cdf
Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>
diff --git a/src/interpreter.cpp b/src/interpreter.cpp
index 73af6fc..e5aafc4 100644
--- a/src/interpreter.cpp
+++ b/src/interpreter.cpp
@@ -57,30 +57,45 @@
return boost::apply_visitor(pathToStringVisitor(), path);
}
-void Interpreter::checkRpcPath(const dataPath_& commandPath) const
+void Interpreter::enforceRpcRestrictions(const std::variant<commit_, discard_, copy_, prepare_, switch_>& cmd) const {
+ if (m_datastore.inputDatastorePath().has_value()) {
+ std::string commandName = std::visit([](const auto& cmd) { return cmd.name; }, cmd);
+ throw std::runtime_error("Can't execute `" + commandName + "` during an RPC/action execution.");
+ }
+}
+
+void Interpreter::enforceRpcRestrictions(const std::variant<move_, set_, cd_, create_, delete_>& cmd) const
{
if (auto inputPath = m_datastore.inputDatastorePath()) {
+ dataPath_ commandPath = std::visit(overloaded {
+ [](const move_& cmd) { return cmd.m_source; },
+ [](const auto& cmd) { return cmd.m_path; }
+ }, cmd);
dataPath_ newPath = realPath(m_parser.currentPath(), commandPath);
if (!pathToDataString(newPath, Prefixes::WhenNeeded).starts_with(*inputPath)) {
- throw std::runtime_error("Can't execute `cd` outside of the `prepare` context.");
+ std::string commandName = std::visit([](const auto& cmd) { return cmd.name; }, cmd);
+ throw std::runtime_error("Can't execute `" + commandName + "` out of the RPC/action context.");
}
}
}
-void Interpreter::operator()(const commit_&) const
+void Interpreter::operator()(const commit_& commit) const
{
+ enforceRpcRestrictions(commit);
m_datastore.commitChanges();
}
-void Interpreter::operator()(const discard_&) const
+void Interpreter::operator()(const discard_& discard) const
{
+ enforceRpcRestrictions(discard);
m_datastore.discardChanges();
}
void Interpreter::operator()(const set_& set) const
{
- auto data = set.m_data;
+ enforceRpcRestrictions(set);
+ auto data = set.m_data;
// If the user didn't supply a module prefix for identityref, we need to add it ourselves
if (data.type() == typeid(identityRef_)) {
auto identityRef = boost::get<identityRef_>(data);
@@ -108,17 +123,19 @@
void Interpreter::operator()(const cd_& cd) const
{
- checkRpcPath(cd.m_path);
+ enforceRpcRestrictions(cd);
m_parser.changeNode(cd.m_path);
}
void Interpreter::operator()(const create_& create) const
{
+ enforceRpcRestrictions(create);
m_datastore.createItem(pathToString(toCanonicalPath(create.m_path)));
}
void Interpreter::operator()(const delete_& delet) const
{
+ enforceRpcRestrictions(delet);
m_datastore.deleteItem(pathToString(toCanonicalPath(delet.m_path)));
}
@@ -141,6 +158,7 @@
void Interpreter::operator()(const copy_& copy) const
{
+ enforceRpcRestrictions(copy);
m_datastore.copyConfig(copy.m_source, copy.m_destination);
}
@@ -236,6 +254,7 @@
void Interpreter::operator()(const move_& move) const
{
+ enforceRpcRestrictions(move);
m_datastore.moveItem(pathToDataString(move.m_source, Prefixes::WhenNeeded), move.m_destination);
}
@@ -246,6 +265,7 @@
void Interpreter::operator()(const prepare_& prepare) const
{
+ enforceRpcRestrictions(prepare);
m_datastore.initiate(pathToString(toCanonicalPath(prepare.m_path)));
m_parser.changeNode(prepare.m_path);
}
@@ -269,6 +289,7 @@
void Interpreter::operator()(const switch_& switch_cmd) const
{
+ enforceRpcRestrictions(switch_cmd);
m_datastore.setTarget(switch_cmd.m_target);
}
diff --git a/src/interpreter.hpp b/src/interpreter.hpp
index 3723432..5a48d1f 100644
--- a/src/interpreter.hpp
+++ b/src/interpreter.hpp
@@ -36,7 +36,8 @@
void operator()(const quit_&) const;
private:
- void checkRpcPath(const dataPath_& commandPath) const;
+ void enforceRpcRestrictions(const std::variant<commit_, discard_, copy_, prepare_, switch_>& cmd) const;
+ void enforceRpcRestrictions(const std::variant<move_, set_, cd_, create_, delete_>& cmd) const;
[[nodiscard]] std::string buildTypeInfo(const std::string& path) const;
diff --git a/tests/interpreter.cpp b/tests/interpreter.cpp
index 087a9b7..2b16f96 100644
--- a/tests/interpreter.cpp
+++ b/tests/interpreter.cpp
@@ -452,28 +452,127 @@
REQUIRE(parser.currentPath() == rpcPath);
- SECTION("can't leave the context with cd")
- {
- REQUIRE_THROWS(boost::apply_visitor(Interpreter(parser, proxyDatastore), command_{cd_{{}, dataPath_{Scope::Absolute, {dataNode_{module_{{"example"}}, container_{"somewhereElse"}}}}}}));
- REQUIRE_THROWS(boost::apply_visitor(Interpreter(parser, proxyDatastore), command_{cd_{{}, dataPath_{Scope::Relative, {dataNode_{nodeup_{}}}}}}));
+ dataPath_ inRpcContainer = dataPath_{Scope::Relative, {dataNode_{container_{"payload"}}}};
+ dataPath_ inRpcLeaf = dataPath_{Scope::Relative, {dataNode_{leaf_{"description"}}}};
+ dataPath_ outRpcPath = dataPath_{Scope::Absolute, {dataNode_{nodeup_{}}, dataNode_{module_("example"), container_{"somewhere-else"}}}};
- // Test that the parser allows to change the current node within the rpc context
- REQUIRE_NOTHROW(boost::apply_visitor(Interpreter(parser, proxyDatastore), command_{cd_{{}, dataPath_{Scope::Relative, {dataNode_{container_{"payload"}}}}}}));
- boost::apply_visitor(Interpreter(parser, proxyDatastore), command_{cancel_{}});
+ std::string inRpcContainerString = "/example:launch-nukes/payload";
+ std::string inRpcLeafString = "/example:launch-nukes/description";
+
+ SECTION("valid commands")
+ {
+ command_ command;
+ std::vector<std::unique_ptr<trompeloeil::expectation>> expectations;
+
+ SECTION("create inside context")
+ {
+ expectations.emplace_back(NAMED_REQUIRE_CALL(*input_datastore, createItem(inRpcContainerString)));
+ command = create_{{}, inRpcContainer};
+ }
+
+ SECTION("set inside context")
+ {
+ leaf_data_ leafData = std::string{"Nuke the whales"};
+ expectations.emplace_back(NAMED_REQUIRE_CALL(*input_datastore, setLeaf(inRpcLeafString, leafData)));
+ command = set_{{}, inRpcLeaf, leafData};
+ }
+
+ SECTION("delete inside context")
+ {
+ expectations.emplace_back(NAMED_REQUIRE_CALL(*input_datastore, deleteItem(inRpcContainerString)));
+ command = delete_{{}, inRpcContainer};
+ }
+
+ SECTION("cd inside context")
+ {
+ command = cd_{{}, inRpcContainer};
+ }
+
+ SECTION("move inside context")
+ {
+ expectations.emplace_back(NAMED_REQUIRE_CALL(*datastore, moveItem("description", yang::move::Absolute::Begin)));
+ command = move_{{}, inRpcLeaf, yang::move::Absolute::Begin};
+ }
+
+ SECTION("cancel")
+ {
+ command = cancel_{};
+ }
+
+ SECTION("exec")
+ {
+ expectations.emplace_back(NAMED_REQUIRE_CALL(*input_datastore, getItems("/")).RETURN(DatastoreAccess::Tree{}));
+ expectations.emplace_back(NAMED_REQUIRE_CALL(*datastore, execute("/example:launch-nukes", DatastoreAccess::Tree{})).RETURN(DatastoreAccess::Tree{}));
+ command = exec_{};
+ }
+
+ boost::apply_visitor(Interpreter(parser, proxyDatastore), command);
+
+ boost::apply_visitor([&](auto& cmd) {
+ if constexpr (std::is_same_v<std::decay_t<decltype(cmd)>, exec_>) {
+ REQUIRE(parser.currentPath() == dataPath_{});
+ } else if constexpr (std::is_same_v<std::decay_t<decltype(cmd)>, cancel_>) {
+ REQUIRE(parser.currentPath() == dataPath_{});
+ }
+ }, command);
+
}
- SECTION("exec")
+ SECTION("invalid commands")
{
- REQUIRE_CALL(*input_datastore, getItems("/")).RETURN(DatastoreAccess::Tree{});
- REQUIRE_CALL(*datastore, execute("/example:launch-nukes", DatastoreAccess::Tree{})).RETURN(DatastoreAccess::Tree{});
- boost::apply_visitor(Interpreter(parser, proxyDatastore), command_{exec_{}});
- }
+ command_ command;
- SECTION("cancel")
- {
- boost::apply_visitor(Interpreter(parser, proxyDatastore), command_{cancel_{}});
- }
+ SECTION("block commit entirely")
+ {
+ command = commit_{};
+ }
- REQUIRE(parser.currentPath() == dataPath_{});
+ SECTION("block discard entirely")
+ {
+ command = discard_{};
+ }
+
+ SECTION("block copy entirely")
+ {
+ command = copy_{{}, Datastore::Running, Datastore::Startup};
+ }
+
+ SECTION("block switch entirely")
+ {
+ command = switch_{{}, DatastoreTarget::Running};
+ }
+
+ SECTION("block prepare entirely")
+ {
+ command = prepare_{{}, outRpcPath};
+ }
+
+ SECTION("block create outside RPC context")
+ {
+ command = create_{{}, outRpcPath};
+ }
+
+ SECTION("block set outside RPC context")
+ {
+ command = set_{{}, outRpcPath, std::string{"Nuke the whales"}};
+ }
+
+ SECTION("block delete outside RPC context")
+ {
+ command = delete_{{}, outRpcPath};
+ }
+
+ SECTION("block cd outside RPC context")
+ {
+ command = cd_{{}, outRpcPath};
+ }
+
+ SECTION("block move outside RPC context")
+ {
+ command = move_{{}, outRpcPath, yang::move::Absolute::Begin};
+ }
+
+ REQUIRE_THROWS(boost::apply_visitor(Interpreter(parser, proxyDatastore), command));
+ }
}
}