From 3707b262011ce966d894692a8b9baf4a130c7da3 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Wed, 25 May 2022 14:18:20 +0100 Subject: [PATCH 1/4] This change adds support for passing Handler Options when registering a new JSON-RPC endpoint handler. To support this a new class Context is introduced as well as a Handler option class, this changes sets the base for future work around the handler registration and the Plugin API. --- include/ts/apidefs.h.in | 9 ++ mgmt2/config/FileManager.cc | 11 +-- mgmt2/rpc/Makefile.am | 4 +- mgmt2/rpc/jsonrpc/Context.cc | 35 +++++++ mgmt2/rpc/jsonrpc/Context.h | 75 +++++++++++++++ mgmt2/rpc/jsonrpc/Defs.h | 13 ++- mgmt2/rpc/jsonrpc/JsonRPC.h | 10 +- mgmt2/rpc/jsonrpc/JsonRPCManager.cc | 91 ++++++++----------- mgmt2/rpc/jsonrpc/JsonRPCManager.h | 56 ++++++++---- mgmt2/rpc/jsonrpc/error/RPCError.cc | 2 + mgmt2/rpc/jsonrpc/error/RPCError.h | 5 +- mgmt2/rpc/jsonrpc/json/YAMLCodec.h | 26 +----- .../jsonrpc/unit_tests/test_basic_protocol.cc | 14 ++- mgmt2/rpc/server/IPCSocketServer.cc | 5 +- mgmt2/rpc/server/unit_tests/test_rpcserver.cc | 7 ++ src/traffic_server/HostStatus.cc | 4 +- src/traffic_server/RpcAdminPubHandlers.cc | 24 ++--- 17 files changed, 260 insertions(+), 131 deletions(-) create mode 100644 mgmt2/rpc/jsonrpc/Context.cc create mode 100644 mgmt2/rpc/jsonrpc/Context.h diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index 0dd3c2abf21..e1b178160af 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -1478,6 +1478,15 @@ namespace ts } #endif +/// +/// @brief JSON-RPC Handler options +/// +/// This class holds information about how a handler will be managed and delivered when called. The JSON-RPC manager would use this +/// object to perform certain validation. +/// +typedef struct TSRPCHandlerOptions_s { +} TSRPCHandlerOptions; + #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/mgmt2/config/FileManager.cc b/mgmt2/config/FileManager.cc index a2880cf833a..d6ffe4ddbdf 100644 --- a/mgmt2/config/FileManager.cc +++ b/mgmt2/config/FileManager.cc @@ -91,12 +91,11 @@ FileManager::FileManager() this->registerCallback(&handle_file_reload); // Register the files registry jsonrpc endpoint - rpc::add_method_handler( - "filemanager.get_files_registry", - [this](std::string_view const &id, const YAML::Node &req) -> ts::Rv { - return get_files_registry_rpc_endpoint(id, req); - }, - &rpc::core_ats_rpc_service_provider_handle); + rpc::add_method_handler("filemanager.get_files_registry", + [this](std::string_view const &id, const YAML::Node &req) -> ts::Rv { + return get_files_registry_rpc_endpoint(id, req); + }, + &rpc::core_ats_rpc_service_provider_handle, {}); } // FileManager::~FileManager diff --git a/mgmt2/rpc/Makefile.am b/mgmt2/rpc/Makefile.am index 26ca1923644..d4de43b0e3d 100644 --- a/mgmt2/rpc/Makefile.am +++ b/mgmt2/rpc/Makefile.am @@ -45,7 +45,9 @@ libjsonrpc_protocol_COMMON = \ jsonrpc/error/RPCError.cc \ jsonrpc/error/RPCError.h \ jsonrpc/JsonRPCManager.cc \ - jsonrpc/JsonRPCManager.h + jsonrpc/JsonRPCManager.h \ + jsonrpc/Context.cc \ + jsonrpc/Context.h libjsonrpc_protocol_la_SOURCES = \ $(libjsonrpc_protocol_COMMON) diff --git a/mgmt2/rpc/jsonrpc/Context.cc b/mgmt2/rpc/jsonrpc/Context.cc new file mode 100644 index 00000000000..9574df91d9b --- /dev/null +++ b/mgmt2/rpc/jsonrpc/Context.cc @@ -0,0 +1,35 @@ +/** + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ +#include "Context.h" + +namespace rpc +{ +// --- Call Context impl +ts::Errata +Context::Auth::any_blockers(TSRPCHandlerOptions const &options) const +{ + ts::Errata out; + // check every registered callback and see if they have something to say. Then report back to the manager + for (auto &&check : _checkers) { + check(options, out); + } + return out; +}; +} // namespace rpc diff --git a/mgmt2/rpc/jsonrpc/Context.h b/mgmt2/rpc/jsonrpc/Context.h new file mode 100644 index 00000000000..6bb43d716e2 --- /dev/null +++ b/mgmt2/rpc/jsonrpc/Context.h @@ -0,0 +1,75 @@ +/** + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ +#pragma once + +#include +#include +#include + +#include "tscore/Errata.h" +#include "ts/apidefs.h" +#include "rpc/handlers/common/ErrorUtils.h" + +namespace rpc +{ +/// +/// @brief RPC call context class. +/// +/// This class is used to carry information from the transport logic to the rpc invocation logic, the transport may need to block +/// some rpc handlers from being executed which at the time of finish ups reading the raw message is yet too early to know the +/// actual handler. +/// +class Context +{ + using checker_cb = std::function; + /// @brief Internal class to hold the permission checker part. + struct Auth { + /// Checks for permissions. This function checks for every registered permission checker. + /// + /// @param options Registered handler options. + /// @return ts::Errata The errata will be filled by each of the registered checkers, if there was any issue validating the + /// call, then the errata reflects that. + ts::Errata any_blockers(TSRPCHandlerOptions const &options) const; + + /// Add permission checkers. + template + void + add_checker(F &&f) + { + _checkers.emplace_back(std::forward(f)); + } + + private: + std::vector _checkers; ///< cb collection. + } _auth; + +public: + Auth & + get_auth() + { + return _auth; + } + Auth const & + get_auth() const + { + return _auth; + } +}; +} // namespace rpc diff --git a/mgmt2/rpc/jsonrpc/Defs.h b/mgmt2/rpc/jsonrpc/Defs.h index 9c321f6f1bf..1dfbd4b5082 100644 --- a/mgmt2/rpc/jsonrpc/Defs.h +++ b/mgmt2/rpc/jsonrpc/Defs.h @@ -45,11 +45,20 @@ class RPCHandlerResponse }; struct RPCResponseInfo { - RPCResponseInfo(std::optional const &id_) : id(id_) {} + RPCResponseInfo(std::optional const &id_) : id{id_} {} + RPCResponseInfo(std::error_code const &ec_) : error{ec_, {}} {} + RPCResponseInfo(std::optional const &id_, std::error_code const &ec_) : error{ec_, {}}, id{id_} {} + RPCResponseInfo(std::optional const &id_, std::error_code const &ec_, ts::Errata const &errata) + : error{ec_, errata}, id{id_} + { + } RPCResponseInfo() = default; RPCHandlerResponse callResult; - std::error_code rpcError; + struct Error { + std::error_code ec; + ts::Errata data; + } error; std::optional id; }; diff --git a/mgmt2/rpc/jsonrpc/JsonRPC.h b/mgmt2/rpc/jsonrpc/JsonRPC.h index 8af97399eff..acf8cfd079b 100644 --- a/mgmt2/rpc/jsonrpc/JsonRPC.h +++ b/mgmt2/rpc/jsonrpc/JsonRPC.h @@ -24,7 +24,7 @@ namespace rpc { -/// Generic and global JSONRPC service provider info object. It's recommended to use this object when registring your new handler +/// Generic and global JSONRPC service provider info object. It's recommended to use this object when registering your new handler /// into the rpc system IF the implementor wants the handler to be listed as ATS's handler. extern RPCRegistryInfo core_ats_rpc_service_provider_handle; // ----------------------------------------------------------------------------- @@ -33,17 +33,17 @@ extern RPCRegistryInfo core_ats_rpc_service_provider_handle; /// @see JsonRPCManager::add_method_handler for details template inline bool -add_method_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info = nullptr) +add_method_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info, TSRPCHandlerOptions const &opt) { - return JsonRPCManager::instance().add_method_handler(name, std::forward(call), info); + return JsonRPCManager::instance().add_method_handler(name, std::forward(call), info, opt); } /// @see JsonRPCManager::add_notification_handler for details template inline bool -add_notification_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info = nullptr) +add_notification_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info, TSRPCHandlerOptions const &opt) { - return JsonRPCManager::instance().add_notification_handler(name, std::forward(call), info); + return JsonRPCManager::instance().add_notification_handler(name, std::forward(call), info, opt); } } // namespace rpc diff --git a/mgmt2/rpc/jsonrpc/JsonRPCManager.cc b/mgmt2/rpc/jsonrpc/JsonRPCManager.cc index f84c7fc9861..39b6f718ea6 100644 --- a/mgmt2/rpc/jsonrpc/JsonRPCManager.cc +++ b/mgmt2/rpc/jsonrpc/JsonRPCManager.cc @@ -40,6 +40,10 @@ const std::string RPC_SERVICE_SCHEMA_KEY{"schema"}; const std::string RPC_SERVICE_METHODS_KEY{"methods"}; const std::string RPC_SERVICE_NOTIFICATIONS_KEY{"notifications"}; const std::string RPC_SERVICE_N_A_STR{"N/A"}; + +// jsonrpc log tag. +constexpr auto logTag = "rpc"; +constexpr auto logTagMsg = "rpc.msg"; } // namespace namespace rpc @@ -54,9 +58,15 @@ std::condition_variable g_rpcHandlingCompletion; ts::Rv g_rpcHandlerResponseData; bool g_rpcHandlerProcessingCompleted{false}; -// jsonrpc log tag. -static constexpr auto logTag = "rpc"; -static constexpr auto logTagMsg = "rpc.msg"; +// --- Helpers +std::pair +check_for_blockers(Context const &ctx, TSRPCHandlerOptions const &options) +{ + if (auto err = ctx.get_auth().any_blockers(options); !err.isOK()) { + return {err, error::RPCErrorCode::Unauthorized}; + } + return {}; +} // --- Dispatcher JsonRPCManager::Dispatcher::Dispatcher() @@ -66,32 +76,36 @@ JsonRPCManager::Dispatcher::Dispatcher() void JsonRPCManager::Dispatcher::register_service_descriptor_handler() { - // TODO: revisit this. if (!this->add_handler( "show_registered_handlers", [this](std::string_view const &id, const YAML::Node &req) -> ts::Rv { return show_registered_handlers(id, req); }, - &core_ats_rpc_service_provider_handle)) { + &core_ats_rpc_service_provider_handle, {})) { Warning("Handler already registered."); } if (!this->add_handler( "get_service_descriptor", [this](std::string_view const &id, const YAML::Node &req) -> ts::Rv { return get_service_descriptor(id, req); }, - &core_ats_rpc_service_provider_handle)) { + &core_ats_rpc_service_provider_handle, {})) { Warning("Handler already registered."); } } JsonRPCManager::Dispatcher::response_type -JsonRPCManager::Dispatcher::dispatch(specs::RPCRequestInfo const &request) const +JsonRPCManager::Dispatcher::dispatch(Context const &ctx, specs::RPCRequestInfo const &request) const { std::error_code ec; auto const &handler = find_handler(request, ec); if (ec) { - return {std::nullopt, ec}; + return specs::RPCResponseInfo{request.id, ec}; + } + + // We have got a valid handler, we will now check if the context holds any restriction for this handler to be called. + if (auto &&[errata, ec] = check_for_blockers(ctx, handler.get_options()); !errata.isOK()) { + return specs::RPCResponseInfo(request.id, ec, errata); } if (request.is_notification()) { @@ -117,7 +131,7 @@ JsonRPCManager::Dispatcher::find_handler(specs::RPCRequestInfo const &request, s return no_handler; } - // we need to make sure we the request is valid against the internal handler. + // Handler's method type should match the requested method type. if ((request.is_method() && search->second.is_method()) || (request.is_notification() && !search->second.is_method())) { return search->second; } @@ -143,10 +157,10 @@ JsonRPCManager::Dispatcher::invoke_method_handler(JsonRPCManager::Dispatcher::In } } catch (std::exception const &e) { Debug(logTag, "Oops, something happened during the callback invocation: %s", e.what()); - return {std::nullopt, error::RPCErrorCode::ExecutionError}; + response.error.ec = error::RPCErrorCode::ExecutionError; } - return {response, {}}; + return response; } JsonRPCManager::Dispatcher::response_type @@ -160,7 +174,7 @@ JsonRPCManager::Dispatcher::invoke_notification_handler(JsonRPCManager::Dispatch // it's a notification so we do not care much. } - return response_type{}; + return {std::nullopt}; } bool @@ -182,32 +196,8 @@ JsonRPCManager::remove_handler(std::string_view name) return _dispatcher.remove_handler(name); } -static inline specs::RPCResponseInfo -make_error_response(specs::RPCRequestInfo const &req, std::error_code const &ec) -{ - specs::RPCResponseInfo resp; - - // we may have been able to collect the id, if so, use it. - if (req.id) { - resp.id = req.id; - } - - resp.rpcError = ec; - - return resp; -} - -static inline specs::RPCResponseInfo -make_error_response(std::error_code const &ec) -{ - specs::RPCResponseInfo resp; - - resp.rpcError = ec; - return resp; -} - std::optional -JsonRPCManager::handle_call(std::string const &request) +JsonRPCManager::handle_call(Context const &ctx, std::string const &request) { Debug(logTagMsg, "--> JSONRPC request\n'%s'", request.c_str()); @@ -219,8 +209,7 @@ JsonRPCManager::handle_call(std::string const &request) // If any error happened within the request, they will be kept inside each // particular request, as they would need to be converted back in a proper error response. if (ec) { - auto response = make_error_response(ec); - return Encoder::encode(response); + return Encoder::encode(specs::RPCResponseInfo{ec}); } specs::RPCResponse response{msg.is_batch()}; @@ -231,23 +220,17 @@ JsonRPCManager::handle_call(std::string const &request) if (!decode_error) { // request seems ok and ready to be dispatched. The dispatcher will tell us if the method exist and if so, it will dispatch // the call and gives us back the response. - auto &&[encodedResponse, ec] = _dispatcher.dispatch(req); - - // On any error, ec will have a value - if (!ec) { - // we only get valid responses if it was a method request, not - // for notifications. - if (encodedResponse) { - response.add_message(*encodedResponse); - } - } else { - // get an error response, we may have the id, so let's try to use it. - response.add_message(make_error_response(req, ec)); - } + const auto &encodedResponse = _dispatcher.dispatch(ctx, req); + + if (encodedResponse) { + // if any error was detected during invocation or before, the response will have the error field set, so this will + // internally be converted to the right response type. + response.add_message(*encodedResponse); + } // else it's a notification and no error. } else { // If the request was marked as an error(decode error), we still need to send the error back, so we save it. - response.add_message(make_error_response(req, decode_error)); + response.add_message(specs::RPCResponseInfo{req.id, decode_error}); } } @@ -263,7 +246,7 @@ JsonRPCManager::handle_call(std::string const &request) ec = error::RPCErrorCode::INTERNAL_ERROR; } - return {Encoder::encode(make_error_response(ec))}; + return {Encoder::encode(specs::RPCResponseInfo{ec})}; } // ---------------------------- InternalHandler --------------------------------- diff --git a/mgmt2/rpc/jsonrpc/JsonRPCManager.h b/mgmt2/rpc/jsonrpc/JsonRPCManager.h index e00b57f5bf8..88f2970609e 100644 --- a/mgmt2/rpc/jsonrpc/JsonRPCManager.h +++ b/mgmt2/rpc/jsonrpc/JsonRPCManager.h @@ -34,6 +34,7 @@ #include "ts/apidefs.h" #include "Defs.h" +#include "Context.h" namespace rpc { @@ -44,9 +45,14 @@ namespace json_codecs class yamlcpp_json_encoder; } // namespace json_codecs +/// +/// @brief This class keeps all relevant @c RPC provider's info. +/// struct RPCRegistryInfo { - std::string_view provider; + std::string_view provider; ///< Who's the rpc endpoint provider, could be ATS or a plugins. When requesting the service info from + ///< the rpc node, this will be part of the service info. }; + /// /// @brief JSONRPC registration and JSONRPC invocation logic https://www.jsonrpc.org/specification /// doc TBC @@ -74,9 +80,11 @@ class JsonRPCManager /// 'get_stats'...} . /// @param call The function handler. /// @param info RPCRegistryInfo pointer. + /// @param opt Handler options, used to pass information about the registered handler. /// @return bool Boolean flag. true if the callback was successfully added, false otherwise /// - template bool add_method_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info); + template + bool add_method_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info, TSRPCHandlerOptions const &opt); /// /// @brief Add new registered notification handler to the JSON RPC engine. @@ -85,18 +93,21 @@ class JsonRPCManager /// @param name Name to be exposed by the RPC Engine. /// @param call The callback function that needs handler. /// @param info RPCRegistryInfo pointer. + /// @param opt Handler options, used to pass information about the registered handler. /// @return bool Boolean flag. true if the callback was successfully added, false otherwise /// - template bool add_notification_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info); + template + bool add_notification_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info, TSRPCHandlerOptions const &opt); /// /// @brief This function handles the incoming jsonrpc request and dispatch the associated registered handler. /// + /// @param ctx @c Context object used pass information between rpc layers. /// @param jsonString The incoming jsonrpc 2.0 message. \link https://www.jsonrpc.org/specification /// @return std::optional For methods, a valid jsonrpc 2.0 json string will be passed back. Notifications will not /// contain any json back. /// - std::optional handle_call(std::string const &jsonString); + std::optional handle_call(Context const &ctx, std::string const &jsonString); /// /// @brief Get the instance of the whole RPC engine. @@ -144,9 +155,9 @@ class JsonRPCManager /// signatures inside a @c std::variant class Dispatcher { - using response_type = std::pair< - std::optional, - std::error_code>; ///< The response type used internally, notifications won't fill in the optional response. @c ec will be set + /// The response type used internally, notifications won't fill in the optional response. Internal response's @ ec will be set + /// in case of any error. + using response_type = std::optional; /// /// @brief Class that wraps the actual std::function. @@ -166,11 +177,11 @@ class JsonRPCManager /// Add a method handler to the internal container /// @return True if was successfully added, False otherwise. template - bool add_handler(std::string_view name, Handler &&handler, const RPCRegistryInfo *info); + bool add_handler(std::string_view name, Handler &&handler, const RPCRegistryInfo *info, TSRPCHandlerOptions const &opt); /// Find and call the request's callback. If any error occurs, the return type will have the specific error. /// For notifications the @c RPCResponseInfo will not be set as part of the response. @c response_type - response_type dispatch(specs::RPCRequestInfo const &request) const; + response_type dispatch(Context const &ctx, specs::RPCRequestInfo const &request) const; /// Find a particular registered handler(method) by its associated name. /// @return A pair. The handler itself and a boolean flag indicating that the handler was found. If not found, second will @@ -206,7 +217,7 @@ class JsonRPCManager /// simplify the logic to insert and fetch callable objects from our container. struct InternalHandler { InternalHandler() = default; - InternalHandler(const RPCRegistryInfo *info) : _regInfo(info) {} + InternalHandler(const RPCRegistryInfo *info, TSRPCHandlerOptions const &opt) : _regInfo(info), _options(opt) {} /// Sets the handler. template void set_callback(F &&t); explicit operator bool() const; @@ -223,6 +234,13 @@ class JsonRPCManager return _regInfo; } + /// Returns the configured options associated with this particular handler. + TSRPCHandlerOptions const & + get_options() const + { + return _options; + } + private: // We need to keep this match with the order of types in the _func variant. This will help us to identify the holding type. enum class VariantTypeIndexId : std::size_t { NOTIFICATION = 1, METHOD = 2, METHOD_FROM_PLUGIN = 3 }; @@ -231,6 +249,7 @@ class JsonRPCManager std::variant _func; const RPCRegistryInfo *_regInfo; ///< Can hold internal information about the handler, this could be null as it is optional. ///< This pointer can eventually holds important information about the call. + TSRPCHandlerOptions _options; }; // We will keep all the handlers wrapped inside the InternalHandler class, this will help us // to have a single container for all the types(method, notification & plugin method(cond var)). @@ -244,19 +263,21 @@ class JsonRPCManager // ------------------------------ JsonRPCManager ------------------------------- template bool -JsonRPCManager::add_method_handler(std::string_view name, Handler &&call, const RPCRegistryInfo *info) +JsonRPCManager::add_method_handler(std::string_view name, Handler &&call, const RPCRegistryInfo *info, + TSRPCHandlerOptions const &opt) { - return _dispatcher.add_handler(name, std::forward(call), info); + return _dispatcher.add_handler(name, std::forward(call), info, opt); } template bool -JsonRPCManager::add_notification_handler(std::string_view name, Handler &&call, const RPCRegistryInfo *info) +JsonRPCManager::add_notification_handler(std::string_view name, Handler &&call, const RPCRegistryInfo *info, + TSRPCHandlerOptions const &opt) { - return _dispatcher.add_handler(name, std::forward(call), info); + return _dispatcher.add_handler(name, std::forward(call), info, opt); } -// ----------------------------- InternalHandler ------------------------------------ +// ----------------------------- InternalHandler ------------------------------- template void JsonRPCManager::Dispatcher::InternalHandler::set_callback(F &&f) @@ -276,10 +297,11 @@ bool inline JsonRPCManager::Dispatcher::InternalHandler::operator!() const // ----------------------------- Dispatcher ------------------------------------ template bool -JsonRPCManager::Dispatcher::add_handler(std::string_view name, Handler &&handler, const RPCRegistryInfo *info) +JsonRPCManager::Dispatcher::add_handler(std::string_view name, Handler &&handler, const RPCRegistryInfo *info, + TSRPCHandlerOptions const &opt) { std::lock_guard lock(_mutex); - InternalHandler call{info}; + InternalHandler call{info, opt}; call.set_callback(std::forward(handler)); return _handlers.emplace(name, std::move(call)).second; } diff --git a/mgmt2/rpc/jsonrpc/error/RPCError.cc b/mgmt2/rpc/jsonrpc/error/RPCError.cc index 6cbb10b870d..03b8f6a66b8 100644 --- a/mgmt2/rpc/jsonrpc/error/RPCError.cc +++ b/mgmt2/rpc/jsonrpc/error/RPCError.cc @@ -74,6 +74,8 @@ RPCErrorCategory::message(int ev) const return {"Use of null as id is discouraged"}; case RPCErrorCode::ExecutionError: return {"Error during execution"}; + case RPCErrorCode::Unauthorized: + return {"Unauthorized action"}; default: return "Rpc error " + std::to_string(ev); } diff --git a/mgmt2/rpc/jsonrpc/error/RPCError.h b/mgmt2/rpc/jsonrpc/error/RPCError.h index b10c8b91742..f945c62bf8c 100644 --- a/mgmt2/rpc/jsonrpc/error/RPCError.h +++ b/mgmt2/rpc/jsonrpc/error/RPCError.h @@ -59,10 +59,13 @@ enum class RPCErrorCode { // execution errors // Internal rpc error when executing the method. - ExecutionError + // + ExecutionError, //!< Handler's general error. + Unauthorized //!< In case we want to block the call based on privileges, access permissions, etc. }; // TODO: force non 0 check std::error_code make_error_code(rpc::error::RPCErrorCode e); + } // namespace rpc::error namespace std diff --git a/mgmt2/rpc/jsonrpc/json/YAMLCodec.h b/mgmt2/rpc/jsonrpc/json/YAMLCodec.h index 2998caf2463..e0781cde844 100644 --- a/mgmt2/rpc/jsonrpc/json/YAMLCodec.h +++ b/mgmt2/rpc/jsonrpc/json/YAMLCodec.h @@ -214,13 +214,6 @@ class yamlcpp_json_encoder json << YAML::EndMap; } - /// Convenience functions to call encode_error. - static void - encode_error(std::error_code error, YAML::Emitter &json) - { - ts::Errata errata{}; - encode_error(error, errata, json); - } /// Convenience functions to call encode_error. static void encode_error(ts::Errata const &errata, YAML::Emitter &json) @@ -228,21 +221,6 @@ class yamlcpp_json_encoder encode_error({error::RPCErrorCode::ExecutionError}, errata, json); } - static void - encode_error_from_callee(ts::Errata const &errata, YAML::Emitter &json) - { - if (!errata.isOK()) { - json << YAML::Key << "errors"; - json << YAML::BeginSeq; - for (auto const &err : errata) { - json << YAML::BeginMap; - json << YAML::Key << "code" << YAML::Value << err.getCode(); - json << YAML::Key << "message" << YAML::Value << err.text(); - json << YAML::EndMap; - } - json << YAML::EndSeq; - } - } /// /// @brief Function to encode a single response(no batch) into an emitter. /// @@ -257,9 +235,9 @@ class yamlcpp_json_encoder // Important! As per specs, errors have preference over the result, we ignore result if error was set. - if (resp.rpcError) { + if (resp.error.ec) { // internal library detected error: Decoding, etc. - encode_error(resp.rpcError, json); + encode_error(resp.error.ec, resp.error.data, json); } // Registered handler error: They have set the error on the response from the registered handler. This uses ExecutionError as // top error. diff --git a/mgmt2/rpc/jsonrpc/unit_tests/test_basic_protocol.cc b/mgmt2/rpc/jsonrpc/unit_tests/test_basic_protocol.cc index ffe272a2d9c..0732a252b6a 100644 --- a/mgmt2/rpc/jsonrpc/unit_tests/test_basic_protocol.cc +++ b/mgmt2/rpc/jsonrpc/unit_tests/test_basic_protocol.cc @@ -37,19 +37,25 @@ struct JsonRpcUnitTest : rpc::JsonRPCManager { bool remove_handler(std::string const &name) { - return rpc::JsonRPCManager::remove_handler(name); + return base::remove_handler(name); } template bool add_notification_handler(const std::string &name, Func &&call) { - return base::add_notification_handler(name, std::forward(call), nullptr); + return base::add_notification_handler(name, std::forward(call), nullptr, {}); } template bool add_method_handler(const std::string &name, Func &&call) { - return base::add_method_handler(name, std::forward(call), nullptr); + return base::add_method_handler(name, std::forward(call), nullptr, {}); + } + + std::optional + handle_call(std::string const &jsonString) + { + return base::handle_call(rpc::Context{}, jsonString); } }; @@ -63,8 +69,6 @@ test_callback_ok_or_error(std::string_view const &id, YAML::Node const ¶ms) if (YAML::Node n = params["return_error"]) { auto yesOrNo = n.as(); if (yesOrNo == "yes") { - // Can we just have a helper for this? - // resp.errata.push(static_cast(TestErrors::ERR1), "Just an error message to add more meaning to the failure"); resp.errata().push(ErratId, static_cast(TestErrors::ERR1), "Just an error message to add more meaning to the failure"); } else { resp.result()["ran"] = "ok"; diff --git a/mgmt2/rpc/server/IPCSocketServer.cc b/mgmt2/rpc/server/IPCSocketServer.cc index ffabea9ff22..54482366a25 100644 --- a/mgmt2/rpc/server/IPCSocketServer.cc +++ b/mgmt2/rpc/server/IPCSocketServer.cc @@ -172,7 +172,8 @@ IPCSocketServer::run() if (auto [ok, errStr] = client.read_all(bw); ok) { const auto json = std::string{bw.data(), bw.size()}; - if (auto response = rpc::JsonRPCManager::instance().handle_call(json); response) { + rpc::Context ctx; // Further use. + if (auto response = rpc::JsonRPCManager::instance().handle_call(ctx, json); response) { // seems a valid response. if (client.write(*response, ec); ec) { Debug(logTag, "Error sending the response: %s", ec.message().c_str()); @@ -440,4 +441,4 @@ check_for_transient_errors() return false; } } -} // namespace \ No newline at end of file +} // namespace diff --git a/mgmt2/rpc/server/unit_tests/test_rpcserver.cc b/mgmt2/rpc/server/unit_tests/test_rpcserver.cc index ee00df85628..7649b3b41c0 100644 --- a/mgmt2/rpc/server/unit_tests/test_rpcserver.cc +++ b/mgmt2/rpc/server/unit_tests/test_rpcserver.cc @@ -55,6 +55,13 @@ test_remove_handler(std::string_view name) { return rpc::JsonRPCManager::instance().remove_handler(name); } + +template +inline bool +add_method_handler(const std::string &name, Func &&call) +{ + return rpc::JsonRPCManager::instance().add_method_handler(name, std::forward(call), nullptr, {}); +} } // namespace rpc static const std::string sockPath{"/tmp/jsonrpc20_test.sock"}; static const std::string lockPath{"/tmp/jsonrpc20_test.lock"}; diff --git a/src/traffic_server/HostStatus.cc b/src/traffic_server/HostStatus.cc index 240f32ade6d..0f436dcb8b2 100644 --- a/src/traffic_server/HostStatus.cc +++ b/src/traffic_server/HostStatus.cc @@ -139,8 +139,8 @@ HostStatus::HostStatus() ink_rwlock_init(&host_status_rwlock); // register JSON-RPC methods. - rpc::add_method_handler("admin_host_set_status", &server_set_status, &rpc::core_ats_rpc_service_provider_handle); - rpc::add_method_handler("admin_host_get_status", &server_get_status, &rpc::core_ats_rpc_service_provider_handle); + rpc::add_method_handler("admin_host_set_status", &server_set_status, &rpc::core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_host_get_status", &server_get_status, &rpc::core_ats_rpc_service_provider_handle, {}); } HostStatus::~HostStatus() diff --git a/src/traffic_server/RpcAdminPubHandlers.cc b/src/traffic_server/RpcAdminPubHandlers.cc index d97d668aa7e..df142658bdb 100644 --- a/src/traffic_server/RpcAdminPubHandlers.cc +++ b/src/traffic_server/RpcAdminPubHandlers.cc @@ -35,29 +35,29 @@ register_admin_jsonrpc_handlers() { // Config using namespace rpc::handlers::config; - rpc::add_method_handler("admin_config_set_records", &set_config_records, &core_ats_rpc_service_provider_handle); - rpc::add_method_handler("admin_config_reload", &reload_config, &core_ats_rpc_service_provider_handle); + rpc::add_method_handler("admin_config_set_records", &set_config_records, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_config_reload", &reload_config, &core_ats_rpc_service_provider_handle, {}); // Records using namespace rpc::handlers::records; - rpc::add_method_handler("admin_lookup_records", &lookup_records, &core_ats_rpc_service_provider_handle); - rpc::add_method_handler("admin_clear_all_metrics_records", &clear_all_metrics_records, &core_ats_rpc_service_provider_handle); - rpc::add_method_handler("admin_clear_metrics_records", &clear_metrics_records, &core_ats_rpc_service_provider_handle); + rpc::add_method_handler("admin_lookup_records", &lookup_records, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_clear_all_metrics_records", &clear_all_metrics_records, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_clear_metrics_records", &clear_metrics_records, &core_ats_rpc_service_provider_handle, {}); // plugin using namespace rpc::handlers::plugins; - rpc::add_method_handler("admin_plugin_send_basic_msg", &plugin_send_basic_msg, &core_ats_rpc_service_provider_handle); + rpc::add_method_handler("admin_plugin_send_basic_msg", &plugin_send_basic_msg, &core_ats_rpc_service_provider_handle, {}); // server using namespace rpc::handlers::server; - rpc::add_method_handler("admin_server_start_drain", &server_start_drain, &core_ats_rpc_service_provider_handle); - rpc::add_method_handler("admin_server_stop_drain", &server_stop_drain, &core_ats_rpc_service_provider_handle); - rpc::add_notification_handler("admin_server_shutdown", &server_shutdown, &core_ats_rpc_service_provider_handle); - rpc::add_notification_handler("admin_server_restart", &server_shutdown, &core_ats_rpc_service_provider_handle); + rpc::add_method_handler("admin_server_start_drain", &server_start_drain, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_server_stop_drain", &server_stop_drain, &core_ats_rpc_service_provider_handle, {}); + rpc::add_notification_handler("admin_server_shutdown", &server_shutdown, &core_ats_rpc_service_provider_handle, {}); + rpc::add_notification_handler("admin_server_restart", &server_shutdown, &core_ats_rpc_service_provider_handle, {}); // storage using namespace rpc::handlers::storage; - rpc::add_method_handler("admin_storage_set_device_offline", &set_storage_offline, &core_ats_rpc_service_provider_handle); - rpc::add_method_handler("admin_storage_get_device_status", &get_storage_status, &core_ats_rpc_service_provider_handle); + rpc::add_method_handler("admin_storage_set_device_offline", &set_storage_offline, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_storage_get_device_status", &get_storage_status, &core_ats_rpc_service_provider_handle, {}); } } // namespace rpc::admin From be7328ff7f9642204ee1c32bdd69324e8b3771ef Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Wed, 25 May 2022 14:24:59 +0100 Subject: [PATCH 2/4] This commit adds documentation to support the JSON-RPC handler registration. It also fixes some bad links and typos. --- doc/admin-guide/files/jsonrpc.yaml.en.rst | 38 +++--- .../command-line/traffic_ctl_jsonrpc.en.rst | 2 +- .../jsonrpc/HandlerError.en.rst | 2 +- doc/developer-guide/jsonrpc/index.en.rst | 1 + .../jsonrpc/jsonrpc-api.en.rst | 19 +-- .../jsonrpc/jsonrpc-architecture.en.rst | 60 +-------- .../jsonrpc-handler-development.en.rst | 7 +- .../jsonrpc/jsonrpc-node-errors.en.rst | 114 ++++++++++++++++++ .../jsonrpc/jsonrpc-node.en.rst | 7 +- .../jsonrpc/traffic_ctl-development.en.rst | 7 +- 10 files changed, 168 insertions(+), 89 deletions(-) create mode 100644 doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst diff --git a/doc/admin-guide/files/jsonrpc.yaml.en.rst b/doc/admin-guide/files/jsonrpc.yaml.en.rst index 4c9970988d1..da3065e6b68 100644 --- a/doc/admin-guide/files/jsonrpc.yaml.en.rst +++ b/doc/admin-guide/files/jsonrpc.yaml.en.rst @@ -1,3 +1,4 @@ + .. Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file distributed with this work for additional information @@ -56,7 +57,7 @@ In order for programs to communicate with |TS|, the server exposes a will find some of the configurable arguments that can be changed. -.. _admnin-jsonrpc-configuration: +.. _admin-jsonrpc-configuration: Configuration ============= @@ -68,9 +69,9 @@ Configuration If a non-default configuration is needed, the following describes the configuration structure. -File `jsonrpc.yaml` is a YAML format. The default configuration is: +File `jsonrpc.yaml` is a YAML format. The default configuration looks like: -.. code:: yaml +.. code-block:: yaml rpc: enabled: true @@ -90,26 +91,27 @@ Field Name Description IPC Socket (``unix``) --------------------- -===================================== ========================================== +Unix Domain Socket related configuration. + +===================================== ========================================================================================= Field Name Description -===================================== ========================================== -``lock_path_name`` Lock path, including the file name. - (changing this may have impacts in - :program:`traffic_ctl`) -``sock_path_name`` Sock path, including the file name. This - will be used as ``sockaddr_un.sun_path`` - (changing this may have impacts in :program:`traffic_ctl`) +===================================== ========================================================================================= +``lock_path_name`` Lock path, including the file name. (changing this may have impacts in :program:`traffic_ctl`) +``sock_path_name`` Sock path, including the file name. This will be used as ``sockaddr_un.sun_path``. (changing + this may have impacts in :program:`traffic_ctl`) ``backlog`` Check https://man7.org/linux/man-pages/man2/listen.2.html -``max_retry_on_transient_errors`` Number of times the implementation is - allowed to retry when a transient error is - encountered. -``restricted_api`` Used to set rpc unix socket permissions. - If restricted `0700` will be set, otherwise - `0777`. ``true`` by default. -===================================== ========================================== +``max_retry_on_transient_errors`` Number of times the implementation is allowed to retry when a transient error is encountered. +``restricted_api`` If restricted, the Unix Domain Socket will be created with `0700` permissions, otherwise `0777`. + ``true`` by default. +===================================== ========================================================================================= .. note:: Currently, there is only 1 communication mechanism supported. Unix Domain Sockets + +See also +======== + +:ref:`traffic_ctl_jsonrpc` diff --git a/doc/appendices/command-line/traffic_ctl_jsonrpc.en.rst b/doc/appendices/command-line/traffic_ctl_jsonrpc.en.rst index acfe4f267c9..94f0e5667fd 100644 --- a/doc/appendices/command-line/traffic_ctl_jsonrpc.en.rst +++ b/doc/appendices/command-line/traffic_ctl_jsonrpc.en.rst @@ -604,5 +604,5 @@ See also :manpage:`records.config(5)`, :manpage:`storage.config(5)`, -:ref:`admnin-jsonrpc-configuration`, +:ref:`admin-jsonrpc-configuration`, :ref:`jsonrpc-protocol` diff --git a/doc/developer-guide/jsonrpc/HandlerError.en.rst b/doc/developer-guide/jsonrpc/HandlerError.en.rst index 03f824b9dfc..cf78d0e9162 100644 --- a/doc/developer-guide/jsonrpc/HandlerError.en.rst +++ b/doc/developer-guide/jsonrpc/HandlerError.en.rst @@ -26,7 +26,7 @@ API Handler error codes *********************** High level handler error codes, each particular handler can be fit into one of the following categories. -A good approach could be the following. This required coordination among all the errors, just for now, this soluction seems ok. +A good approach could be the following. This required coordination among all the errors, just for now, this solution seems ok. .. code-block:: cpp diff --git a/doc/developer-guide/jsonrpc/index.en.rst b/doc/developer-guide/jsonrpc/index.en.rst index 3706257fd52..7d06cc71318 100644 --- a/doc/developer-guide/jsonrpc/index.en.rst +++ b/doc/developer-guide/jsonrpc/index.en.rst @@ -28,6 +28,7 @@ JSONRPC jsonrpc-architecture.en jsonrpc-api.en jsonrpc-node.en + jsonrpc-node-errors.en jsonrpc-handler-development.en jsonrpc-client-api.en traffic_ctl-development.en diff --git a/doc/developer-guide/jsonrpc/jsonrpc-api.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-api.en.rst index 3391909fc05..75179cfb892 100644 --- a/doc/developer-guide/jsonrpc/jsonrpc-api.en.rst +++ b/doc/developer-guide/jsonrpc/jsonrpc-api.en.rst @@ -960,7 +960,7 @@ Request: Response: -The response will contain the default `success_response` or an :cpp:class:`RPCErrorCode`. +The response will contain the default `success_response` or a proper rpc error, check :ref:`jsonrpc-node-errors` for mode details. Validation: @@ -1070,7 +1070,7 @@ Parameters Result ~~~~~~ -This api will only inform for errors during the metric update. Errors will be tracked down in the :cpp:class:`RPCErrorCode` field. +This api will only inform for errors during the metric update. Errors will be tracked down in the `error` field. .. note:: @@ -1095,7 +1095,7 @@ Request: Response: -The response will contain the default `success_response` or an :cpp:class:`RPCErrorCode`. +The response will contain the default `success_response` or an error. :ref:`jsonrpc-node-errors`. .. _admin_host_set_status: @@ -1164,7 +1164,7 @@ marking this reason as "down" in that case. Result ~~~~~~ -The response will contain the default `success_response` or an :cpp:class:`RPCErrorCode`. +The response will contain the default `success_response` or an error. :ref:`jsonrpc-node-errors`. Examples @@ -1260,7 +1260,7 @@ Parameters Result ~~~~~~ -The response will contain the default `success_response` or an :cpp:class:`RPCErrorCode`. +The response will contain the default `success_response` or an error. :ref:`jsonrpc-node-errors`. Examples @@ -1302,7 +1302,7 @@ Field Type Description Result ~~~~~~ -The response will contain the default `success_response` or an :cpp:class:`RPCErrorCode`. +The response will contain the default `success_response` or an error. :ref:`jsonrpc-node-errors`. .. note:: @@ -1380,7 +1380,7 @@ Field Type Description Result ~~~~~~ -The response will contain the default `success_response` or an :cpp:class:`RPCErrorCode`. +The response will contain the default `success_response` or an error. :ref:`jsonrpc-node-errors`. Examples ~~~~~~~~ @@ -1781,3 +1781,8 @@ Response: ] } } + +See also +======== + +:ref:`jsonrpc-node-errors` diff --git a/doc/developer-guide/jsonrpc/jsonrpc-architecture.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-architecture.en.rst index 4025cbd30ff..91489c5e980 100644 --- a/doc/developer-guide/jsonrpc/jsonrpc-architecture.en.rst +++ b/doc/developer-guide/jsonrpc/jsonrpc-architecture.en.rst @@ -55,7 +55,7 @@ IPC The current server implementation runs on an IPC Socket(Unix Domain Socket). This server implements an iterative server style. The implementation runs on a dedicated ``TSThread`` and as their style express, this performs blocking calls to all the registered handlers. -Configuration for this particular server style can be found in the admin section :ref:`admnin-jsonrpc-configuration`. +Configuration for this particular server style can be found in the admin section :ref:`admin-jsonrpc-configuration`. Using the JSONRPC mechanism @@ -464,61 +464,6 @@ Response: } -.. _rpc-error-code: - -Internally we have defined an ``enum`` class that keeps track of the errors that the server will inform in most of the cases. -Some of this errors are already defined by the `JSONRPC`_ specs and some (``>=1``) are defined by |TS|. - -.. class:: RPCErrorCode - - Defines the API error codes that will be used in case of any RPC error. - - .. enumerator:: INVALID_REQUEST = -32600 - .. enumerator:: METHOD_NOT_FOUND = -32601 - .. enumerator:: INVALID_PARAMS = -32602 - .. enumerator:: INTERNAL_ERROR = -32603 - .. enumerator:: PARSE_ERROR = -32700 - - `JSONRPC`_ defined errors. - - .. enumerator:: InvalidVersion = 1 - - The passed version is invalid. must be 2.0 - - .. enumerator:: InvalidVersionType = 2 - - The passed version field type is invalid. must be a ``string`` - - .. enumerator:: MissingVersion = 3 - - Version field is missing from the request. This field is mandatory. - - .. enumerator:: InvalidMethodType = 4 - - The passed method field type is invalid. must be a ``string`` - - .. enumerator:: MissingMethod = 5 - - Method field is missing from the request. This field is mandatory. - - .. enumerator:: InvalidParamType = 6 - - The passed parameter field type is not valid. - - .. enumerator:: InvalidIdType = 7 - - The passed id field type is invalid. - - .. enumerator:: NullId = 8 - - The passed if is ``null`` - - .. enumerator:: ExecutionError = 9 - - An error occurred during the execution of the RPC call. This error is used as a generic High level error. The details details about - the error, in most cases are specified in the ``data`` field. - - .. information: According to the |RPC| specs, if you get an error, the ``result`` field will not be set. |TS| will grant this. @@ -535,5 +480,6 @@ Development Guide See also ======== -:ref:`admnin-jsonrpc-configuration`, +:ref:`admin-jsonrpc-configuration`, +:ref:`jsonrpc-node-errors`, :ref:`traffic_ctl_jsonrpc` diff --git a/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst index 94ae293a9dc..f79b21a2ca9 100644 --- a/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst +++ b/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst @@ -186,7 +186,7 @@ We recommend some ways to deal with this: This can be set in case you would like to let the server to respond with an |RPC| error, ``ExecutionError`` will be used to catch all the errors that are fired from within the function call, either by setting the proper errata or by throwing an exception. -Please check the `rpc-error-code` and in particular ``ExecutionError = 9``. Also check :ref:`jsonrpc-handler-errors` +Please check the :ref:`jsonrpc-node-errors` and in particular ``ExecutionError = 9``. Also check :ref:`jsonrpc-handler-errors` .. important:: @@ -420,7 +420,10 @@ Important Notes * To interact directly with the |RPC| node, please check :ref:`jsonrpc-node` -:ref:`admnin-jsonrpc-configuration` +See also +======== + +:ref:`admin-jsonrpc-configuration` :ref:`jsonrpc-protocol` :ref:`developer-guide-traffic_ctl-development` diff --git a/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst new file mode 100644 index 00000000000..5f6229d7fc3 --- /dev/null +++ b/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst @@ -0,0 +1,114 @@ +.. Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + +.. include:: ../../common.defs + +.. highlight:: cpp +.. default-domain:: cpp + +.. _JSONRPC: https://www.jsonrpc.org/specification +.. _JSON: https://www.json.org/json-en.html + + +.. _jsonrpc-node-errors: + + +JSON RPC errors +*************** + +A list of codes and descriptions of errors that could be send back from the JSONRPC server. JSONRPC response messages could contains +different set of errors in the following format: + +.. note:: + + Check :ref:`jsonrpc-error` for details about the error structure. + + +.. code-block::json + + { + "error": { + "code": -32601, + "message": "Method not found" + }, + "id": "ded7018e-0720-11eb-abe2-001fc69cc946", + "jsonrpc": "2.0" + } + +In some cases the data field could be populated: + +.. code-block:: json + + { + "jsonrpc": "2.0", + "error":{ + "code": 10, + "message": "Unauthorized action", + "data":[ + { + "code": 2, + "message":"Denied privileged API access for uid=XXX gid=XXX" + } + ] + }, + "id":"5e273ec0-3e3b-4a81-90ec-aeee3d38073f" + } + + +.. _jsonrpc-node-errors-standard-errors: + +Standard errors +=============== + +=============== ========================================= ========================================================= +Field Message Description +=============== ========================================= ========================================================= +-32700 Parse error Invalid JSON was received by the server. + An error occurred on the server while parsing the JSON text. +-32600 Invalid Request The JSON sent is not a valid Request object. +-32601 Method not found The method does not exist / is not available. +-32602 Invalid params Invalid method parameter(s). +-32603 Internal error Internal `JSONRPC`_ error. +=============== ========================================= ========================================================= + +.. _jsonrpc-node-errors-custom-errors: + +Custom errors +============= + +The following error list are defined by the server. + +=============== ========================================= ========================================================= +Field Message Description +=============== ========================================= ========================================================= +1 Invalid version, 2.0 only The server only accepts version field equal to `2.0`. +2 Invalid version type, should be a string Version field should be a literal string. +3 Missing version field No version field present, version field is mandatory. +4 Invalid method type, should be a string The method field should be a literal string. +5 Missing method field No method field present, method field is mandatory. +6 Invalid params type. A Structured value Params field should be a structured type, list or structure. + is expected This is similar to `-32602` +7 Invalid id type If field should be a literal string. +8 Use of null as id is discouraged Id field value is null, as per the specs this is discouraged, + the server will not accept it. +9 Error during execution An error occurred during the execution of the RPC call. + This error is used as a generic High level error. The specifics + details about the error, in most cases are specified in the + ``data`` field. +10 Unauthorized action The rpc method will not be invoked because the action is not + permitted by some constraint or authorization issue. +=============== ========================================= ========================================================= diff --git a/doc/developer-guide/jsonrpc/jsonrpc-node.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-node.en.rst index 204e7b35717..25b63a5054b 100644 --- a/doc/developer-guide/jsonrpc/jsonrpc-node.en.rst +++ b/doc/developer-guide/jsonrpc/jsonrpc-node.en.rst @@ -36,7 +36,7 @@ IPC Node ======== You can directly connect to the Unix Domain Socket used for the |RPC| node, the location of the sockets -will depend purely on how did you configure the server, please check :ref:`admnin-jsonrpc-configuration` for +will depend purely on how did you configure the server, please check :ref:`admin-jsonrpc-configuration` for information regarding configuration. @@ -57,3 +57,8 @@ Using traffic_ctl :program:`traffic_ctl` can also be used to directly send raw |RPC| messages to the server's node, :program:`traffic_ctl` provides several options to achieve this, please check ``traffic_ctl_rpc``. + +Error responses +--------------- + +The server will indicate in case of any error processing the call, check :ref:`jsonrpc-node-errors` for more details. diff --git a/doc/developer-guide/jsonrpc/traffic_ctl-development.en.rst b/doc/developer-guide/jsonrpc/traffic_ctl-development.en.rst index 3a219578c54..957c00a3fa8 100644 --- a/doc/developer-guide/jsonrpc/traffic_ctl-development.en.rst +++ b/doc/developer-guide/jsonrpc/traffic_ctl-development.en.rst @@ -208,7 +208,10 @@ There is code that was written in this way by design, ``RecordPrinter`` and ``Re that needs to query and print records without any major hassle. +See also +======== -:ref:`admnin-jsonrpc-configuration`, +:ref:`admin-jsonrpc-configuration`, :ref:`traffic_ctl_jsonrpc`, -:ref:`jsonrpc_development` +:ref:`jsonrpc_development`, +:ref:`jsonrpc-node-errors` From dfe3f544a1fd0261611e7738dc4b190a0e9a4246 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Wed, 25 May 2022 15:31:56 +0100 Subject: [PATCH 3/4] Add late check for peers credentials on client's socket. RPC handler can now decide if they want to be administrative restricted or not. Add documentation support for this. --- doc/admin-guide/files/jsonrpc.yaml.en.rst | 9 ++++- .../jsonrpc/jsonrpc-node-errors.en.rst | 31 ++++++++++++++- include/ts/apidefs.h.in | 3 ++ mgmt2/config/FileManager.cc | 2 +- mgmt2/rpc/jsonrpc/Context.h | 2 + mgmt2/rpc/jsonrpc/JsonRPCManager.cc | 8 ++-- mgmt2/rpc/server/IPCSocketServer.cc | 39 +++++++++++++++++-- mgmt2/rpc/server/IPCSocketServer.h | 6 +++ src/traffic_server/HostStatus.cc | 6 ++- src/traffic_server/RpcAdminPubHandlers.cc | 35 +++++++++++------ 10 files changed, 117 insertions(+), 24 deletions(-) diff --git a/doc/admin-guide/files/jsonrpc.yaml.en.rst b/doc/admin-guide/files/jsonrpc.yaml.en.rst index da3065e6b68..86fb8f36839 100644 --- a/doc/admin-guide/files/jsonrpc.yaml.en.rst +++ b/doc/admin-guide/files/jsonrpc.yaml.en.rst @@ -101,8 +101,15 @@ Field Name Description this may have impacts in :program:`traffic_ctl`) ``backlog`` Check https://man7.org/linux/man-pages/man2/listen.2.html ``max_retry_on_transient_errors`` Number of times the implementation is allowed to retry when a transient error is encountered. -``restricted_api`` If restricted, the Unix Domain Socket will be created with `0700` permissions, otherwise `0777`. +``restricted_api`` This setting specifies whether the jsonrpc node access should be restricted to root processes. + If this is set to ``false``, then on platforms that support passing process credentials, non-root + processes will be allowed to make read-only JSONRPC calls. Any calls that modify server state + (eg. setting a configuration variable) will still be restricted to root processes. If set to ``true`` + then only root processes will be allowed to perform any api call. + If restricted, the Unix Domain Socket will be created with `0700` permissions, otherwise `0777`. ``true`` by default. + In case of an unauthorized call is made, a corresponding rpc error will be returned, you can + check :ref:`jsonrpc-node-errors-unauthorized-action` for details about the errors. ===================================== ========================================================================================= diff --git a/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst index 5f6229d7fc3..0d9253f08ad 100644 --- a/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst +++ b/doc/developer-guide/jsonrpc/jsonrpc-node-errors.en.rst @@ -110,5 +110,34 @@ Field Message Description details about the error, in most cases are specified in the ``data`` field. 10 Unauthorized action The rpc method will not be invoked because the action is not - permitted by some constraint or authorization issue. + permitted by some constraint or authorization issue.Check + :ref:`jsonrpc-node-errors-unauthorized-action` for mode details. +=============== ========================================= ========================================================= + +.. _jsonrpc-node-errors-unauthorized-action: + +Unauthorized action +------------------- + +Under this error, the `data` field could be populated with the following errors, eventually more than one could be in set. + +.. code-block:: json + + "data":[ + { + "code":2, + "message":"Denied privileged API access for uid=XXX gid=XXX" + } + ] + +=============== ========================================= ========================================================= +Field Message Description +=============== ========================================= ========================================================= +1 Error getting peer credentials: {} Something happened while trying to get the peers credentials. + The error string will show the error code(`errno`) returned by the + server. +2 Denied privileged API access for uid={} Permission denied. Unix Socket credentials were checked and they haven't meet + gid={} the required policy. The handler was configured as restricted + and the socket credentials failed to validate. Check TBC for + more information. =============== ========================================= ========================================================= diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index e1b178160af..085fb2c5fa8 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -1485,6 +1485,9 @@ namespace ts /// object to perform certain validation. /// typedef struct TSRPCHandlerOptions_s { + struct Auth { + int restricted; ///< Tells the RPC Manager if the call can be delivered or not based on the config rules. + } auth; } TSRPCHandlerOptions; #ifdef __cplusplus diff --git a/mgmt2/config/FileManager.cc b/mgmt2/config/FileManager.cc index d6ffe4ddbdf..4bb6aa9a378 100644 --- a/mgmt2/config/FileManager.cc +++ b/mgmt2/config/FileManager.cc @@ -95,7 +95,7 @@ FileManager::FileManager() [this](std::string_view const &id, const YAML::Node &req) -> ts::Rv { return get_files_registry_rpc_endpoint(id, req); }, - &rpc::core_ats_rpc_service_provider_handle, {}); + &rpc::core_ats_rpc_service_provider_handle, {{rpc::NON_RESTRICTED_API}}); } // FileManager::~FileManager diff --git a/mgmt2/rpc/jsonrpc/Context.h b/mgmt2/rpc/jsonrpc/Context.h index 6bb43d716e2..e16463a9757 100644 --- a/mgmt2/rpc/jsonrpc/Context.h +++ b/mgmt2/rpc/jsonrpc/Context.h @@ -29,6 +29,8 @@ namespace rpc { +const bool RESTRICTED_API{true}; +const bool NON_RESTRICTED_API{false}; /// /// @brief RPC call context class. /// diff --git a/mgmt2/rpc/jsonrpc/JsonRPCManager.cc b/mgmt2/rpc/jsonrpc/JsonRPCManager.cc index 39b6f718ea6..1d5f5c8420e 100644 --- a/mgmt2/rpc/jsonrpc/JsonRPCManager.cc +++ b/mgmt2/rpc/jsonrpc/JsonRPCManager.cc @@ -39,6 +39,7 @@ const std::string RPC_SERVICE_PROVIDER_KEY{"provider"}; const std::string RPC_SERVICE_SCHEMA_KEY{"schema"}; const std::string RPC_SERVICE_METHODS_KEY{"methods"}; const std::string RPC_SERVICE_NOTIFICATIONS_KEY{"notifications"}; +const std::string RPC_SERVICE_PRIVILEGED_KEY{"privileged"}; const std::string RPC_SERVICE_N_A_STR{"N/A"}; // jsonrpc log tag. @@ -81,14 +82,14 @@ JsonRPCManager::Dispatcher::register_service_descriptor_handler() [this](std::string_view const &id, const YAML::Node &req) -> ts::Rv { return show_registered_handlers(id, req); }, - &core_ats_rpc_service_provider_handle, {})) { + &core_ats_rpc_service_provider_handle, {{NON_RESTRICTED_API}})) { Warning("Handler already registered."); } if (!this->add_handler( "get_service_descriptor", [this](std::string_view const &id, const YAML::Node &req) -> ts::Rv { return get_service_descriptor(id, req); }, - &core_ats_rpc_service_provider_handle, {})) { + &core_ats_rpc_service_provider_handle, {{NON_RESTRICTED_API}})) { Warning("Handler already registered."); } } @@ -332,7 +333,8 @@ JsonRPCManager::Dispatcher::get_service_descriptor(std::string_view const &, con } else { provider = RPC_SERVICE_N_A_STR; } - method[RPC_SERVICE_PROVIDER_KEY] = provider; + method[RPC_SERVICE_PROVIDER_KEY] = provider; + method[RPC_SERVICE_PRIVILEGED_KEY] = handler.get_options().auth.restricted; YAML::Node schema{YAML::NodeType::Map}; // no schema for now, but we have a placeholder for it. Schema should provide // description and all the details about the call method[RPC_SERVICE_SCHEMA_KEY] = std::move(schema); diff --git a/mgmt2/rpc/server/IPCSocketServer.cc b/mgmt2/rpc/server/IPCSocketServer.cc index 54482366a25..6176d3a7f5b 100644 --- a/mgmt2/rpc/server/IPCSocketServer.cc +++ b/mgmt2/rpc/server/IPCSocketServer.cc @@ -37,6 +37,8 @@ #include "tscore/Diags.h" #include "tscore/bwf_std_format.h" #include "records/I_RecProcess.h" +#include "tscore/ink_sock.h" +#include "utils/MgmtSocket.h" #include @@ -46,6 +48,7 @@ namespace { constexpr size_t MAX_REQUEST_BUFFER_SIZE{32000}; +constexpr auto logTag = "rpc.net"; // Quick check for errors(base on the errno); bool check_for_transient_errors(); @@ -72,8 +75,6 @@ poll_on_socket(Func &&check_poll_return, std::chrono::milliseconds timeout, int namespace rpc::comm { -static constexpr auto logTag = "rpc.net"; - IPCSocketServer::~IPCSocketServer() { unlink(_conf.sockPathName.c_str()); @@ -172,7 +173,11 @@ IPCSocketServer::run() if (auto [ok, errStr] = client.read_all(bw); ok) { const auto json = std::string{bw.data(), bw.size()}; - rpc::Context ctx; // Further use. + rpc::Context ctx; + // we want to make sure the peer's credentials are ok. + ctx.get_auth().add_checker( + [&](TSRPCHandlerOptions const &opt, ts::Errata &errata) -> void { return late_check_peer_credentials(fd, opt, errata); }); + if (auto response = rpc::JsonRPCManager::instance().handle_call(ctx, json); response) { // seems a valid response. if (client.write(*response, ec); ec) { @@ -261,7 +266,14 @@ IPCSocketServer::bind(std::error_code &ec) return; } - mode_t mode = _conf.restrictedAccessApi ? 00700 : 00777; + // If the socket is not administratively restricted, check whether we have platform + // support. Otherwise, default to making it restricted. + bool restricted{true}; + if (!_conf.restrictedAccessApi) { + restricted = !mgmt_has_peereid(); + } + + mode_t mode = restricted ? 00700 : 00777; if (chmod(_conf.sockPathName.c_str(), mode) < 0) { ec = std::make_error_code(static_cast(errno)); return; @@ -382,6 +394,25 @@ IPCSocketServer::Config::Config() lockPathName = Layout::relative_to(rundir, "jsonrpc20.lock"); sockPathName = Layout::relative_to(rundir, "jsonrpc20.sock"); } + +void +IPCSocketServer::late_check_peer_credentials(int peedFd, TSRPCHandlerOptions const &options, ts::Errata &errata) const +{ + ts::LocalBufferWriter<256> w; + // For privileged calls, ensure we have caller credentials and that the caller is privileged. + if (mgmt_has_peereid() && options.auth.restricted) { + uid_t euid = -1; + gid_t egid = -1; + if (mgmt_get_peereid(peedFd, &euid, &egid) == -1) { + errata.push(1, static_cast(UnauthorizedErrorCode::PEER_CREDENTIALS_ERROR), + w.print("Error getting peer credentials: {}\0", ts::bwf::Errno{}).data()); + } else if (euid != 0 && euid != geteuid()) { + errata.push(1, static_cast(UnauthorizedErrorCode::PERMISSION_DENIED), + w.print("Denied privileged API access for uid={} gid={}\0", euid, egid).data()); + } + } +} + } // namespace rpc::comm namespace YAML diff --git a/mgmt2/rpc/server/IPCSocketServer.h b/mgmt2/rpc/server/IPCSocketServer.h index 2817c95f303..1df85704dc8 100644 --- a/mgmt2/rpc/server/IPCSocketServer.h +++ b/mgmt2/rpc/server/IPCSocketServer.h @@ -46,6 +46,11 @@ namespace rpc::comm /// Buffer size = 32k class IPCSocketServer : public BaseCommInterface { + // Error codes to track any unauthorized call to a rpc handler. + enum class UnauthorizedErrorCode { + PEER_CREDENTIALS_ERROR = 1, ///< Error while trying to read the peer credentials from the unix socket. + PERMISSION_DENIED = 2 ///< Client's socket credential didn't wasn't sufficient to execute the method. + }; /// /// @brief Connection abstraction class that deals with sending and receiving data from the connected peer. /// @@ -129,6 +134,7 @@ class IPCSocketServer : public BaseCommInterface void bind(std::error_code &ec); void listen(std::error_code &ec); void close(); + void late_check_peer_credentials(int peedFd, TSRPCHandlerOptions const &options, ts::Errata &errata) const; std::atomic_bool _running; diff --git a/src/traffic_server/HostStatus.cc b/src/traffic_server/HostStatus.cc index 0f436dcb8b2..7579a96f51e 100644 --- a/src/traffic_server/HostStatus.cc +++ b/src/traffic_server/HostStatus.cc @@ -139,8 +139,10 @@ HostStatus::HostStatus() ink_rwlock_init(&host_status_rwlock); // register JSON-RPC methods. - rpc::add_method_handler("admin_host_set_status", &server_set_status, &rpc::core_ats_rpc_service_provider_handle, {}); - rpc::add_method_handler("admin_host_get_status", &server_get_status, &rpc::core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_host_set_status", &server_set_status, &rpc::core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); + rpc::add_method_handler("admin_host_get_status", &server_get_status, &rpc::core_ats_rpc_service_provider_handle, + {{rpc::NON_RESTRICTED_API}}); } HostStatus::~HostStatus() diff --git a/src/traffic_server/RpcAdminPubHandlers.cc b/src/traffic_server/RpcAdminPubHandlers.cc index df142658bdb..8ca8b9d70e8 100644 --- a/src/traffic_server/RpcAdminPubHandlers.cc +++ b/src/traffic_server/RpcAdminPubHandlers.cc @@ -35,29 +35,40 @@ register_admin_jsonrpc_handlers() { // Config using namespace rpc::handlers::config; - rpc::add_method_handler("admin_config_set_records", &set_config_records, &core_ats_rpc_service_provider_handle, {}); - rpc::add_method_handler("admin_config_reload", &reload_config, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_config_set_records", &set_config_records, &core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); + rpc::add_method_handler("admin_config_reload", &reload_config, &core_ats_rpc_service_provider_handle, {{rpc::RESTRICTED_API}}); // Records using namespace rpc::handlers::records; - rpc::add_method_handler("admin_lookup_records", &lookup_records, &core_ats_rpc_service_provider_handle, {}); - rpc::add_method_handler("admin_clear_all_metrics_records", &clear_all_metrics_records, &core_ats_rpc_service_provider_handle, {}); - rpc::add_method_handler("admin_clear_metrics_records", &clear_metrics_records, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_lookup_records", &lookup_records, &core_ats_rpc_service_provider_handle, + {{rpc::NON_RESTRICTED_API}}); + rpc::add_method_handler("admin_clear_all_metrics_records", &clear_all_metrics_records, &core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); + rpc::add_method_handler("admin_clear_metrics_records", &clear_metrics_records, &core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); // plugin using namespace rpc::handlers::plugins; - rpc::add_method_handler("admin_plugin_send_basic_msg", &plugin_send_basic_msg, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_plugin_send_basic_msg", &plugin_send_basic_msg, &core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); // server using namespace rpc::handlers::server; - rpc::add_method_handler("admin_server_start_drain", &server_start_drain, &core_ats_rpc_service_provider_handle, {}); - rpc::add_method_handler("admin_server_stop_drain", &server_stop_drain, &core_ats_rpc_service_provider_handle, {}); - rpc::add_notification_handler("admin_server_shutdown", &server_shutdown, &core_ats_rpc_service_provider_handle, {}); - rpc::add_notification_handler("admin_server_restart", &server_shutdown, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_server_start_drain", &server_start_drain, &core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); + rpc::add_method_handler("admin_server_stop_drain", &server_stop_drain, &core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); + rpc::add_notification_handler("admin_server_shutdown", &server_shutdown, &core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); + rpc::add_notification_handler("admin_server_restart", &server_shutdown, &core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); // storage using namespace rpc::handlers::storage; - rpc::add_method_handler("admin_storage_set_device_offline", &set_storage_offline, &core_ats_rpc_service_provider_handle, {}); - rpc::add_method_handler("admin_storage_get_device_status", &get_storage_status, &core_ats_rpc_service_provider_handle, {}); + rpc::add_method_handler("admin_storage_set_device_offline", &set_storage_offline, &core_ats_rpc_service_provider_handle, + {{rpc::RESTRICTED_API}}); + rpc::add_method_handler("admin_storage_get_device_status", &get_storage_status, &core_ats_rpc_service_provider_handle, + {{rpc::NON_RESTRICTED_API}}); } } // namespace rpc::admin From f532c9319d5dbcef4e485819259b4c8c313b19e0 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Mon, 20 Jun 2022 15:24:51 +0100 Subject: [PATCH 4/4] Address review suggestions --- mgmt2/rpc/jsonrpc/Context.cc | 2 +- mgmt2/rpc/jsonrpc/Context.h | 6 +++--- mgmt2/rpc/jsonrpc/Defs.h | 28 ++++++++++++------------- mgmt2/rpc/jsonrpc/JsonRPCManager.cc | 32 +++++++++++++++++++---------- mgmt2/rpc/jsonrpc/json/YAMLCodec.h | 24 +++++++--------------- 5 files changed, 45 insertions(+), 47 deletions(-) diff --git a/mgmt2/rpc/jsonrpc/Context.cc b/mgmt2/rpc/jsonrpc/Context.cc index 9574df91d9b..a0a3793f8ad 100644 --- a/mgmt2/rpc/jsonrpc/Context.cc +++ b/mgmt2/rpc/jsonrpc/Context.cc @@ -23,7 +23,7 @@ namespace rpc { // --- Call Context impl ts::Errata -Context::Auth::any_blockers(TSRPCHandlerOptions const &options) const +Context::Auth::is_blocked(TSRPCHandlerOptions const &options) const { ts::Errata out; // check every registered callback and see if they have something to say. Then report back to the manager diff --git a/mgmt2/rpc/jsonrpc/Context.h b/mgmt2/rpc/jsonrpc/Context.h index e16463a9757..fb78d338ef0 100644 --- a/mgmt2/rpc/jsonrpc/Context.h +++ b/mgmt2/rpc/jsonrpc/Context.h @@ -29,8 +29,8 @@ namespace rpc { -const bool RESTRICTED_API{true}; -const bool NON_RESTRICTED_API{false}; +constexpr bool RESTRICTED_API{true}; +constexpr bool NON_RESTRICTED_API{false}; /// /// @brief RPC call context class. /// @@ -48,7 +48,7 @@ class Context /// @param options Registered handler options. /// @return ts::Errata The errata will be filled by each of the registered checkers, if there was any issue validating the /// call, then the errata reflects that. - ts::Errata any_blockers(TSRPCHandlerOptions const &options) const; + ts::Errata is_blocked(TSRPCHandlerOptions const &options) const; /// Add permission checkers. template diff --git a/mgmt2/rpc/jsonrpc/Defs.h b/mgmt2/rpc/jsonrpc/Defs.h index 1dfbd4b5082..f528ad11109 100644 --- a/mgmt2/rpc/jsonrpc/Defs.h +++ b/mgmt2/rpc/jsonrpc/Defs.h @@ -45,21 +45,18 @@ class RPCHandlerResponse }; struct RPCResponseInfo { - RPCResponseInfo(std::optional const &id_) : id{id_} {} - RPCResponseInfo(std::error_code const &ec_) : error{ec_, {}} {} - RPCResponseInfo(std::optional const &id_, std::error_code const &ec_) : error{ec_, {}}, id{id_} {} - RPCResponseInfo(std::optional const &id_, std::error_code const &ec_, ts::Errata const &errata) - : error{ec_, errata}, id{id_} - { - } + RPCResponseInfo(std::string const &id_) : id{id_} {} // Convenient RPCResponseInfo() = default; - RPCHandlerResponse callResult; struct Error { std::error_code ec; ts::Errata data; - } error; - std::optional id; + }; + + std::string id; //!< incoming request id (only used for method calls, empty means it's a notification as requests with empty id + //!< will not pass the validation) + Error error; //!< Error code and details. + RPCHandlerResponse callResult; //!< the actual handler's response }; /// @@ -70,17 +67,18 @@ struct RPCResponseInfo { struct RPCRequestInfo { RPCRequestInfo() = default; RPCRequestInfo(std::string const &version, std::string const &mid) : jsonrpc(version), id(mid) {} - std::string jsonrpc; //!< JsonRPC version ( we only allow 2.0 ). @see yamlcpp_json_decoder - std::string method; //!< incoming method name. - std::optional id; //!< incoming request if (only used for method calls.) - YAML::Node params; //!< incoming parameter structure. + std::string jsonrpc; //!< JsonRPC version ( we only allow 2.0 ). @see yamlcpp_json_decoder + std::string method; //!< incoming method name. + std::string id; //!< incoming request id (only used for method calls, empty means it's a notification as requests with empty id + //!< will not pass the validation) + YAML::Node params; //!< incoming parameter structure. /// Convenience functions that checks for the type of request. If contains id then it should be handle as method call, otherwise /// will be a notification. bool is_notification() const { - return !id.has_value(); + return id.empty(); } bool is_method() const diff --git a/mgmt2/rpc/jsonrpc/JsonRPCManager.cc b/mgmt2/rpc/jsonrpc/JsonRPCManager.cc index 1d5f5c8420e..050f5a44302 100644 --- a/mgmt2/rpc/jsonrpc/JsonRPCManager.cc +++ b/mgmt2/rpc/jsonrpc/JsonRPCManager.cc @@ -63,7 +63,7 @@ bool g_rpcHandlerProcessingCompleted{false}; std::pair check_for_blockers(Context const &ctx, TSRPCHandlerOptions const &options) { - if (auto err = ctx.get_auth().any_blockers(options); !err.isOK()) { + if (auto err = ctx.get_auth().is_blocked(options); !err.isOK()) { return {err, error::RPCErrorCode::Unauthorized}; } return {}; @@ -101,12 +101,17 @@ JsonRPCManager::Dispatcher::dispatch(Context const &ctx, specs::RPCRequestInfo c auto const &handler = find_handler(request, ec); if (ec) { - return specs::RPCResponseInfo{request.id, ec}; + specs::RPCResponseInfo resp{request.id}; + resp.error.ec = ec; + return resp; } // We have got a valid handler, we will now check if the context holds any restriction for this handler to be called. if (auto &&[errata, ec] = check_for_blockers(ctx, handler.get_options()); !errata.isOK()) { - return specs::RPCResponseInfo(request.id, ec, errata); + specs::RPCResponseInfo resp{request.id}; + resp.error.ec = ec; + resp.error.data = errata; + return resp; } if (request.is_notification()) { @@ -210,7 +215,9 @@ JsonRPCManager::handle_call(Context const &ctx, std::string const &request) // If any error happened within the request, they will be kept inside each // particular request, as they would need to be converted back in a proper error response. if (ec) { - return Encoder::encode(specs::RPCResponseInfo{ec}); + specs::RPCResponseInfo resp; + resp.error.ec = ec; + return Encoder::encode(resp); } specs::RPCResponse response{msg.is_batch()}; @@ -221,17 +228,19 @@ JsonRPCManager::handle_call(Context const &ctx, std::string const &request) if (!decode_error) { // request seems ok and ready to be dispatched. The dispatcher will tell us if the method exist and if so, it will dispatch // the call and gives us back the response. - const auto &encodedResponse = _dispatcher.dispatch(ctx, req); + auto encodedResponse = _dispatcher.dispatch(ctx, req); if (encodedResponse) { // if any error was detected during invocation or before, the response will have the error field set, so this will // internally be converted to the right response type. - response.add_message(*encodedResponse); + response.add_message(std::move(*encodedResponse)); } // else it's a notification and no error. } else { // If the request was marked as an error(decode error), we still need to send the error back, so we save it. - response.add_message(specs::RPCResponseInfo{req.id, decode_error}); + specs::RPCResponseInfo resp{req.id}; + resp.error.ec = decode_error; + response.add_message(std::move(resp)); } } @@ -246,8 +255,9 @@ JsonRPCManager::handle_call(Context const &ctx, std::string const &request) } catch (std::exception const &ex) { ec = error::RPCErrorCode::INTERNAL_ERROR; } - - return {Encoder::encode(specs::RPCResponseInfo{ec})}; + specs::RPCResponseInfo resp; + resp.error.ec = ec; + return {Encoder::encode(resp)}; } // ---------------------------- InternalHandler --------------------------------- @@ -265,13 +275,13 @@ JsonRPCManager::Dispatcher::InternalHandler::invoke(specs::RPCRequestInfo const [&ret, &request](Method const &handler) -> void { // Regular Method Handler call, No cond variable check here, this should have not be created by // a plugin. - ret = handler.cb(*request.id, request.params); + ret = handler.cb(request.id, request.params); }, [&ret, &request](PluginMethod const &handler) -> void { // We call the method handler, we'll lock and wait till the condition_variable // gets set on the other side. The handler may return immediately with no response being set. // cond var will give us green to proceed. - handler.cb(*request.id, request.params); + handler.cb(request.id, request.params); std::unique_lock lock(g_rpcHandlingMutex); g_rpcHandlingCompletion.wait(lock, []() { return g_rpcHandlerProcessingCompleted; }); g_rpcHandlerProcessingCompleted = false; diff --git a/mgmt2/rpc/jsonrpc/json/YAMLCodec.h b/mgmt2/rpc/jsonrpc/json/YAMLCodec.h index e0781cde844..b00919f791d 100644 --- a/mgmt2/rpc/jsonrpc/json/YAMLCodec.h +++ b/mgmt2/rpc/jsonrpc/json/YAMLCodec.h @@ -169,21 +169,6 @@ class yamlcpp_json_decoder /// class yamlcpp_json_encoder { - /// - /// @brief Encode the ID if present. - /// If the id is not present which could be interpret as null(should not happen), it will not be set into the emitter, it will be - /// ignored. This is due the way that yamlcpp deals with the null, which instead of the literal null, it uses ~ - static void - encode_id(const std::optional &id, YAML::Emitter &json) - { - // workaround, we should find a better way, we should be able to use literal null if needed - if (id) { - json << YAML::Key << "id" << YAML::Value << *id; - } - // We do not insert null as it will break the json, we need literal null and not ~ (as per yaml) - // json << YAML::Null; - } - /// /// @brief Function to encode an error. /// Error could be from two sources, presence of @c std::error_code means a high level and the @c ts::Errata a callee . Both will @@ -258,8 +243,13 @@ class yamlcpp_json_encoder } } - // insert the id. - encode_id(resp.id, json); + // ID. Only if present. + if (!resp.id.empty()) { + json << YAML::Key << "id" << YAML::Value << resp.id; + } + // else: We do not insert null as it will break the json, we need literal null and not ~ (as per yaml) + // json << YAML::Null; + json << YAML::EndMap; }