From bd2c8023c75b28704f252a9d87f97e3029c16da7 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Wed, 6 Oct 2021 17:38:06 +0100 Subject: [PATCH 1/3] [JSONRPC] - Introduce TS Plugin API. This commit extends the existing TS API to allow plugins to register and interact with the JSONRPC server. --- .../api/functions/TSRPCRegister.en.rst | 219 +++++++++++++ .../jsonrpc-handler-development.en.rst | 169 +++++++++- include/ts/apidefs.h.in | 4 + include/ts/ts.h | 81 +++++ mgmt/rpc/jsonrpc/JsonRPC.h | 9 + mgmt/rpc/jsonrpc/JsonRPCManager.h | 31 ++ src/traffic_server/InkAPI.cc | 106 ++++++ tests/Makefile.am | 2 + .../jsonrpc/basic_plugin_handler.test.py | 182 +++++++++++ tests/gold_tests/jsonrpc/plugins/Makefile.inc | 27 ++ .../plugins/jsonrpc_plugin_handler_test.cc | 301 ++++++++++++++++++ 11 files changed, 1129 insertions(+), 2 deletions(-) create mode 100644 doc/developer-guide/api/functions/TSRPCRegister.en.rst create mode 100644 tests/gold_tests/jsonrpc/basic_plugin_handler.test.py create mode 100644 tests/gold_tests/jsonrpc/plugins/Makefile.inc create mode 100644 tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc diff --git a/doc/developer-guide/api/functions/TSRPCRegister.en.rst b/doc/developer-guide/api/functions/TSRPCRegister.en.rst new file mode 100644 index 00000000000..e4eddeaf335 --- /dev/null +++ b/doc/developer-guide/api/functions/TSRPCRegister.en.rst @@ -0,0 +1,219 @@ +.. 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 + +.. default-domain:: c + +TSRPCRegister +************* + +Traffic Server JSONRPC method registration. + +Synopsis +======== + +.. code-block:: cpp + + #include + +.. type:: TSYaml + + An opaque pointer to an internal representation of a YAML::Node type from yamlcpp library.. + + +.. type:: TSRPCProviderHandle + + An opaque pointer to an internal representation of rpc::RPCRegistryInfo. This object contains context information about the RPC + handler. + +.. type:: void (*TSRPCMethodCb)(const char *id, TSYaml params); + + JSONRPC callback signature for method. + +.. type:: void (*TSRPCNotificationCb)(TSYaml params); + + JSONRPC callback signature for notification. + +.. function:: TSRPCProviderHandle TSRPCRegister(const char *provider_name, const char *yamlcpp_lib_version); +.. function:: TSReturnCode TSRPCRegisterMethodHandler(const char *name, TSRPCMethodCb callback, TSRPCProviderHandle info); +.. function:: TSReturnCode TSRPCRegisterNotificationHandler(const char *name, TSRPCNotificationCb callback, TSRPCProviderHandle info); +.. function:: TSReturnCode TSRPCHandlerDone(TSYaml resp); +.. function:: void TSRPCHandlerError(int code, const char *descr); + +Description +=========== + +:type:`TSRPCRegister` Should be used to accomplish two basic tasks: + +#. To perform a ``yamlcpp`` version library validation. + To avoid binary compatibility issues with some plugins using a different ``yamlcpp`` library version, plugins should + check-in with TS before registering any handler and validate that their ``yamlcpp`` version is the same as used internally + in TS. + +#. To create the ``TSRPCProviderHandle`` that will be used as a context object for each subsequent handler registration. + The ``provider_name`` will be used as a part of the service descriptor API(:ref:`get_service_descriptor`) which is available as part of the RPC api. + + .. code-block:: cpp + + TSRPCProviderHandle info = TSRPCRegister("FooBar's Plugin!", "0.6.3"); + ... + TSRPCRegisterMethodHandler("my_join_string_handler", &func, info); + + + Then when requesting :ref:`get_service_descriptor` It will then display as follows: + + .. code-block:: json + + { + "jsonrpc":"2.0", + "result":{ + "methods":[ + { + "name":"my_join_string_handler", + "type":"method", + "provider":"FooBar's plugin!", + "schema":{ } + } + ] + } + + +.. note:: + + We will provide binary compatibility within the lifespan of a major release. + +:arg:`yamlcpp_lib_version` should be a string with the yaml-cpp library version +the plugin is using. A null terminated string is expected. + +:arg:`provider_name` should be a string with the Plugin's descriptor. A null terminated string is expected. + +:c:func:`TSRPCRegisterMethodHandler` Add new registered method handler to the JSON RPC engine. +:arg:`name` call name to be exposed by the RPC Engine, this should match the incoming request. +If you register **get_stats** then the incoming jsonrpc call should have this very +same name in the **method** field. .. {...'method': 'get_stats'...}. +:arg:`callback` The function to be registered. Check :c:func:`TSRPCMethodCb`. :arg:`info` TSRPCProviderHandle pointer, +this will be used to provide more context information about this call. It is expected to use the one created by ``TSRPCRegister``. + +Please check :ref:`jsonrpc_development` for examples. + +:c:func:`TSRPCRegisterNotificationHandler` Add new registered method handler to the JSON RPC engine. +:arg:`name` call name to be exposed by the RPC Engine, this should match the incoming request. +If you register **get_stats** then the incoming jsonrpc call should have this very +same name in the **method** field. .. {...'method': 'get_stats'...}. +:arg:`callback` The function to be registered. Check :c:func:`TSRPCNotificationCb`. :arg:`info` TSRPCProviderHandle pointer, +this will be used to provide more context information about this call. It is expected to use the one created by ``TSRPCRegister``. + +Please check :ref:`jsonrpc_development` for examples. + +:c:func:`TSRPCHandlerDone` Function to notify the JSONRPC engine that the plugin handler is finished processing the current request. +This function must be used when implementing a 'method' rpc handler. Once the work is done and the +response is ready to be sent back to the client, this function should be called. +Is expected to set the YAML node as response. If the response is empty a **success** message will be +added to the client's response. The :arg:`resp` holds the *YAML::Node* response for this call. + + +Example: + + .. code-block:: cpp + + void my_join_string_handler(const char *id, TSYaml p) { + // id = "abcd-id" + // join string "["abcd", "efgh"] + std::string join = join_string(p); + YAML::Node resp; + resp["join"] = join; + + TSRPCHandlerDone(reinterpret_cast(&resp)); + } + + This will generate: + + .. code-block:: json + + { + "jsonrpc":"2.0", + "result":{ + "join":"abcdefgh" + }, + "id":"abcd-id" + } + + +:c:func:`TSRPCHandlerError` Function to notify the JSONRPC engine that the plugin handler is finished processing the current request with an error. + +:arg:`code` Should be the error number for this particular error. :arg:`descr` should be a text with a description of the error. It's up to the +developer to specify their own error codes and description. Error will be part of the *data* field in the jsonrpc response. See :ref:`jsonrpc-error` + +Example: + + .. code-block:: cpp + + void my_handler_func(const char *id, TSYaml p) { + try { + // some code + } catch (std::exception const &e) { + std::string buff; + ts::bwprint(buff, "Error during rpc handling: {}.", e.what()); + TSRPCHandlerError(ID_123456, buff.c_str()); + return; + } + } + + This will generate: + + .. code-block:: json + + { + "jsonrpc":"2.0", + "error":{ + "code":9, + "message":"Error during execution", + "data":[ + { + "code":123456, + "message":"Error during rpc handling: File not found." + } + ] + }, + "id":"abcd-id" + } + +.. important:: + + You must always inform the RPC after processing the jsonrpc request. Either by calling :c:func:`TSRPCHandlerDone` or :c:func:`TSRPCHandlerError` + . Calling either of these functions twice is a serious error. You should call exactly one of these functions. + +Return Values +============= + +:c:func:`TSRPCRegister` returns :const:`TS_SUCCESS` if all is good, :const:`TS_ERROR` if the :arg:`yamlcpp_lib_version` +was not set, or the ``yamlcpp`` version does not match with the one used internally in TS. + +:c:func:`TSRPCRegisterMethodHandler` :const:`TS_SUCCESS` if the handler was successfully registered, :const:`TS_ERROR` if the handler is already registered. + +:c:func:`TSRPCRegisterNotificationHandler`:const:`TS_SUCCESS` if the handler was successfully registered, :const:`TS_ERROR` if the handler is already registered. + +:c:func:`TSRPCHandlerDone` Returns :const:`TS_SUCCESS` if no issues, or :const:`TS_ERROR` if an issue was found. + +:c:func:`TSRPCHandlerError` Returns :const:`TS_SUCCESS` if no issues, or :const:`TS_ERROR` if an issue was found. + + +See also +======== + +Please check :ref:`jsonrpc_development` for more details on how to use this API. + diff --git a/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst index f5b4cf58ee5..db831a26e31 100644 --- a/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst +++ b/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst @@ -74,6 +74,8 @@ Methods: ts::Rv your_rpc_handler_function_name(std::string_view const &id, YAML::Node const ¶ms); + // Plugins: + void (*TSRPCMethodCb)(const char *id, TSYaml params); Notifications: @@ -82,12 +84,20 @@ Notifications: void your_rpc_handler_function_name(YAML::Node const ¶ms); + // Plugins: + void (*TSRPCNotificationCb)(TSYaml params); + * Incoming method request's id will be passed to the handler, this is read only value as the server is expected to respond with the same value. * ``YAML::Node`` params is expected to be a ``Sequence`` or a ``Map``, as per protocol this cannot be a single value, so do not expect things like: ``param=123`` or ``param=SomeString``. * The ``params`` can be empty and contains no data at all. +.. note:: + + Plugins should cast ``TSYaml`` to a ``YAML::Node``. Regarding the `yamlcpp` library we will provide binary + compatibility within the lifespan of a major release only. + It is important to know that ``method`` handlers are expected to respond to the requests, while ``notifications``` will not respond with any data nor error. You can find more information in :ref:`jsonrpc-protocol` or directly in the protocol specs `JSONRPC`_. @@ -139,7 +149,7 @@ The actual registration: This API also accepts a RPCRegistryInfo pointer which will provide a context data for the particular handler, for instance it will display the provider's name when the service descriptor gets called. There is a global object created for this purpose which can be used As a generic registry context object, ``core_ats_rpc_service_provider_handle`` is defined in the ``JsonRPC.h`` header. Please check -`get_service_descriptor` for more information. +:ref:`get_service_descriptor` for more information. Notification example: @@ -165,7 +175,128 @@ Registration for notifications uses a different API: The registration API allows the client to pass a ``RPCRegistryInfo`` which provides extra information for a particular handler. Non plugins handlers -should use the default provided Registry Info, located in the `JsonRPC.h` header file. +should use the default provided Registry Info, located in the `JsonRPC.h` header file, ``core_ats_rpc_service_provider_handle``. Plugins should use the +one created by ``TSRPCRegister`` + +Plugin API +~~~~~~~~~~ + + +Plugins have a different API to register and handle RPC. Unlike registering and handling rpc using the JSONRPC Manager directly, this +API provides a different approach, it's understood that plugins may not be able to respond a rpc with a valid response at the very +same moment that they are called, this could be because the rpc needs to perform an intensive operation or because the data that should +be returned by the handler is not yet ready, in any case plugin have the flexibility to say when they finished processing the request, either with +a successful response or with an error. **The JSONRPC manager will wait for the plugin to "mark" the current call as done**. + +.. note: + + Check :c:func:`TSRPCRegister` for API details. + + +RPC method registration and implementation examples + +#. No reschedule work on the rpc handler, we set the response in the same call. This runs on the RPC thread. + + .. code-block:: cpp + + #include + + namespace { + static const std::string MY_YAML_VERSION{"0.6.3"}; + } + + void + my_join_string_handler(const char *id, TSYaml p) + { + YAML::Node params = *(YAML::Node *)p; + // extract the strings from the params node + std::vector passedStrings; + if (auto node = params["strings"]) { + passedStrings = node.as>(); + } else { + // We can't continue, let the JSONRPC Manager know that we have finished + TSRPCHandlerError(NO_STRINGS_ERROR_CODE, " no strings field passed"); + return; + } + + std::string join; + std::for_each(std::begin(passedStrings), std::end(passedStrings), [&join](auto &&s) { join += s; }); + YAML::Node resp; // start building the response. + resp["join"] = join; // add the join string into a "join" field. + TSRPCHandlerDone(reinterpret_cast(&resp)); // Let the JSONRPC Manager know that we have finished + } + + void + TSPluginInit(int argc, const char *argv[]) + { + ... + // Check-in to make sure we are compliant with the YAML version in TS. + TSRPCProviderHandle rpcRegistrationInfo = TSRPCRegister("My plugin's info", "0.6.3"); + if (rpcRegistrationInfo == nullptr) { + TSError("[%s] RPC handler registration failed, yaml version not supported.", PLUGIN_NAME); + } + + if (TSRPCRegisterMethodHandler("join_strings", my_join_string_handler, rpcRegistrationInfo) == TS_ERROR) { + TSDebug(PLUGIN_NAME, "%s failed to register", rpcCallName.c_str()); + } else { + TSDebug(PLUGIN_NAME, "%s successfully registered", rpcCallName.c_str()); + } + } + +#. RPC handling rescheduled to run on a ET TASK thread. + + .. code-block:: cpp + + #include + + namespace { + static const std::string MY_YAML_VERSION{"0.6.3"}; + } + + int + merge_yaml_file_on_et_task(TSCont contp, TSEvent event, void *data) + { + // read the incoming node and merge it with the copy on disk. + YAML::Node params = *static_cast(TSContDataGet(contp)); + + // we only care for a map type {} + if (params.Type() != YAML::NodeType::Map) { + TSRPCHandlerError(INVALID_PARAM_TYPE_CODE, "Handler is expecting a map."); + return TS_SUCCESS; + } + + // do the actual work. + merge_yaml(params); + // ... + YAML::Node resp; + TSContDestroy(contp); + TSRPCHandlerDone(reinterpret_cast(&resp)); + } + + void + merge_yaml_file(const char *id, TSYaml p) + { + TSCont c = TSContCreate(merge_yaml_file_on_et_task, TSMutexCreate()); + TSContDataSet(c, p); + TSContScheduleOnPool(c, 3000 /* no particular reason */, TS_THREAD_POOL_TASK); + } + + void + TSPluginInit(int argc, const char *argv[]) + { + // ... + // Check-in to make sure we are compliant with the YAML version in TS. + TSRPCProviderHandle rpcRegistrationInfo = TSRPCRegister("My plugin's info", "0.6.3"); + if (rpcRegistrationInfo == nullptr) { + TSError("[%s] RPC handler registration failed, yaml version not supported.", PLUGIN_NAME); + } + + if (TSRPCRegisterMethodHandler("merge_yaml_file", merge_yaml_file, rpcRegistrationInfo) == TS_ERROR) { + TSDebug(PLUGIN_NAME, "%s failed to register", rpcCallName.c_str()); + } else { + TSDebug(PLUGIN_NAME, "%s successfully registered", rpcCallName.c_str()); + } + } Error Handling @@ -294,6 +425,40 @@ Examples: We have selected the ``ts::Rv`` as a message interface as this can hold the actual response/error. +Plugin API +~~~~~~~~~~ + +When implementing rpc handlers inside a plugin, errors should be handled differently: + + +.. code-block:: + + #include + + ... + + void + foo_handler(const char *id, TSYaml p) + { + // read the incoming node and merge it with the copy on disk. + YAML::Node params = *static_cast(TSContDataGet(contp)); + + if (check_if_error(params)) { + TSRPCHandlerError(FOO_ERROR_CODE, "Some error descr."); + return; + } + + ... + } + + +.. note:: + + Errors can be a part of the request response(result field), this depends on the API design. The above example + shows how to set an error that will be sent back as part of the Error field. + + +For more information check the :c:func:`TSRPCRegister` for API details and :ref:`jsonrpc-error`. .. _jsonrpc-handler-unit-test: diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index 12904e066bb..dcefc368977 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -1480,6 +1480,10 @@ namespace ts } #endif +// JSONRPC 2.0 related interface. +typedef struct tsapi_rpcproviderhandle *TSRPCProviderHandle; +typedef struct tsapi_yaml *TSYaml; + /// /// @brief JSON-RPC Handler options /// diff --git a/include/ts/ts.h b/include/ts/ts.h index 8d277dc55fb..b00288b6d4c 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -2757,6 +2757,87 @@ tsapi void TSHostStatusSet(const char *hostname, const size_t hostname_len, TSHo tsapi bool TSHttpTxnCntlGet(TSHttpTxn txnp, TSHttpCntlType ctrl); tsapi TSReturnCode TSHttpTxnCntlSet(TSHttpTxn txnp, TSHttpCntlType ctrl, bool data); +/** + * JSONRPC callback signature for method calls. + */ +typedef void (*TSRPCMethodCb)(const char *id, TSYaml params); +/** + * JSONRPC callback signature for notification calls + */ +typedef void (*TSRPCNotificationCb)(TSYaml params); + +/** + * @brief Method to perform a registration and validation when a plugin is expected to handle JSONRPC calls. + * + * @note YAMLCPP The JSONRPC library will only provide binary compatibility within the life-span of a major release. Plugins must + * check-in if they intent to handle RPC commands, passing their yamlcpp library version this function will validate it against the + * one used internally in TS. + * + * @param yamlcpp_lib_version a string with the yamlcpp library version. A null terminated string is expected. + * @return A new TSRPCProviderHandle, nullptr if the yamlcpp_lib_version was not set, or the yamlcpp version does not match with + * the one used internally in TS. The returned TSRPCProviderHandle will be set with the provider's name. The caller should pass the + * returned TSRPCProviderHandle object to each subsequent TSRPCRegisterMethod/Notification* call. + */ +tsapi TSRPCProviderHandle TSRPCRegister(const char *provider_name, const char *yamlcpp_lib_version); + +/** + * @brief Add new registered method handler to the JSON RPC engine. + * + * @param name Call name to be exposed by the RPC Engine, this should match the incoming request. i.e: If you register 'get_stats' + * then the incoming jsonrpc call should have this very same name in the 'method' field. .. {...'method': + * 'get_stats'...} . + * @param callback The function to be registered. See @c TSRPCMethodCb + * @param info TSRPCProviderHandle pointer, this will be used to provide more context information about this call. This object + * ideally should be the one returned by the TSRPCRegister API. + * @param opt Pointer to @c TSRPCHandlerOptions object. This will be used to store specifics about a particular call, the rpc + * manager will use this object to perform certain actions. A copy of this object wil be stored by the rpc manager. + * + * @return TS_SUCCESS if the handler was successfully registered, TS_ERROR if the handler is already registered. + */ +tsapi TSReturnCode TSRPCRegisterMethodHandler(const char *name, TSRPCMethodCb callback, TSRPCProviderHandle info, + const TSRPCHandlerOptions *opt); + +/** + * @brief Add new registered notification handler to the JSON RPC engine. + * + * @param name Call name to be exposed by the RPC Engine, this should match the incoming request. i.e: If you register 'get_stats' + * then the incoming jsonrpc call should have this very same name in the 'method' field. .. {...'method': + * 'get_stats'...} . + * @param callback The function to be registered. See @c TSRPCNotificationCb + * @param info TSRPCProviderHandle pointer, this will be used to provide more description for instance, when logging before or after + * a call. This object ideally should be the one returned by the TSRPCRegister API. + * @param opt Pointer to @c TSRPCHandlerOptions object. This will be used to store specifics about a particular call, the rpc + * manager will use this object to perform certain actions. A copy of this object wil be stored by the rpc manager. + * @return TS_SUCCESS if the handler was successfully registered, TS_ERROR if the handler is already registered. + */ +tsapi TSReturnCode TSRPCRegisterNotificationHandler(const char *name, TSRPCNotificationCb callback, TSRPCProviderHandle info, + const TSRPCHandlerOptions *opt); + +/** + * @brief Function to notify the JSONRPC engine that the current handler is done working. + * + * This function must be used when implementing a 'method' rpc handler. Once the work is done and the response is ready to be sent + * back to the client, this function should be called. Is expected to set the YAML node as response. If the response is empty a + * 'success' message will be added to the client's response. + * + * @note This should not be used if you registered your handler as a notification: @c TSRPCNotificationCb + * @param resp The YAML node that contains the call response. + * @return TS_SUCCESS if no issues. TS_ERROR otherwise. + */ +tsapi TSReturnCode TSRPCHandlerDone(TSYaml resp); + +/** + * @brief Function to notify the JSONRPC engine that the current handler is done working and an error has arisen. + * + * @note This should not be used if you registered your handler as a notification: @c TSRPCNotificationCb + * call. + * @param code Error code. + * @param descr A text with a description of the error. + * @note The @c code and @c descr will be part of the @c 'data' field in the jsonrpc error response. + * @return TS_SUCCESS if no issues. TS_ERROR otherwise. + */ +tsapi TSReturnCode TSRPCHandlerError(int code, const char *descr); + #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/mgmt/rpc/jsonrpc/JsonRPC.h b/mgmt/rpc/jsonrpc/JsonRPC.h index acf8cfd079b..c3827237429 100644 --- a/mgmt/rpc/jsonrpc/JsonRPC.h +++ b/mgmt/rpc/jsonrpc/JsonRPC.h @@ -38,7 +38,16 @@ add_method_handler(std::string_view name, Func &&call, const RPCRegistryInfo *in return JsonRPCManager::instance().add_method_handler(name, std::forward(call), info, opt); } +/// As a plugin you should use @c TSRPCRegisterMethodHandlerfor to register your function to handle RPC messages. +template +inline bool +add_method_handler_from_plugin(const std::string &name, Func &&call, const RPCRegistryInfo *info, TSRPCHandlerOptions const &opt) +{ + return JsonRPCManager::instance().add_method_handler_from_plugin(name, std::forward(call), info, opt); +} + /// @see JsonRPCManager::add_notification_handler for details +/// @note Plugins must use @c TSRPCRegisterNotificationHandler to register. template inline bool add_notification_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info, TSRPCHandlerOptions const &opt) diff --git a/mgmt/rpc/jsonrpc/JsonRPCManager.h b/mgmt/rpc/jsonrpc/JsonRPCManager.h index 88f2970609e..4baeda00a35 100644 --- a/mgmt/rpc/jsonrpc/JsonRPCManager.h +++ b/mgmt/rpc/jsonrpc/JsonRPCManager.h @@ -86,6 +86,30 @@ class JsonRPCManager template bool add_method_handler(std::string_view name, Func &&call, const RPCRegistryInfo *info, TSRPCHandlerOptions const &opt); + /// + /// @brief Add new registered method handler(from a plugin scope) to the JSON RPC engine. + /// + /// Function to add a new RPC handler from a plugin context. This will be invoked by @c TSRPCRegisterMethodHandler. If you + /// register your handler by using this API then you have to express the result of the processing by either calling + /// @c TSInternalHandlerDone or @c TSInternalHandlerError in case of an error. + /// When a function registered by this mechanism gets called, the response from the handler will not matter, instead we will rely + /// on what the @c TSInternalHandlerDone or @c TSInternalHandlerError have set. + /// + /// @note If you are not a plugin, do not call this function. Use @c add_method_handler instead. + /// + /// @tparam Func The callback function type. See @c PluginMethodHandlerSignature + /// @param name Name to be exposed by the RPC Engine, this should match the incoming request. i.e: If you register 'get_stats' + /// then the incoming jsonrpc call should have this very same name in the 'method' field. .. {...'method': + /// '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_from_plugin(const std::string &name, Func &&call, const RPCRegistryInfo *info, + TSRPCHandlerOptions const &opt); + /// /// @brief Add new registered notification handler to the JSON RPC engine. /// @@ -268,6 +292,13 @@ JsonRPCManager::add_method_handler(std::string_view name, Handler &&call, const { return _dispatcher.add_handler(name, std::forward(call), info, opt); } +template +bool +JsonRPCManager::add_method_handler_from_plugin(const std::string &name, Handler &&call, const RPCRegistryInfo *info, + TSRPCHandlerOptions const &opt) +{ + return _dispatcher.add_handler(name, std::forward(call), info, opt); +} template bool diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index 9f907da9421..b328c8acc78 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -23,6 +23,8 @@ #include #include +#include +#include #include #include "tscore/ink_platform.h" @@ -75,6 +77,8 @@ #include "HttpProxyServerMain.h" #include "shared/overridable_txn_vars.h" +#include "rpc/jsonrpc/JsonRPC.h" + #include "ts/ts.h" /**************************************************************** @@ -754,6 +758,20 @@ sdk_sanity_check_stat_id(int id) return TS_SUCCESS; } +static TSReturnCode +sdk_sanity_check_rpc_handler_options(const TSRPCHandlerOptions *opt) +{ + if (nullptr == opt) { + return TS_ERROR; + } + + if (opt->auth.restricted < 0 || opt->auth.restricted > 1) { + return TS_ERROR; + } + + return TS_SUCCESS; +} + /** The function checks if the buffer is Modifiable and returns true if it is modifiable, else returns false. @@ -10321,3 +10339,91 @@ TSDbgCtlCreate(char const *tag) return DbgCtl::_get_ptr(tag); } + +namespace rpc +{ +extern std::mutex g_rpcHandlingMutex; +extern std::condition_variable g_rpcHandlingCompletion; +extern ts::Rv g_rpcHandlerResponseData; +extern bool g_rpcHandlerProcessingCompleted; +} // namespace rpc + +tsapi TSRPCProviderHandle +TSRPCRegister(const char *provider_name, const char *yaml_version) +{ + sdk_assert(sdk_sanity_check_null_ptr(yaml_version) == TS_SUCCESS); + sdk_assert(sdk_sanity_check_null_ptr(provider_name) == TS_SUCCESS); + + // We want to make sure that plugins are using the same yaml library version as we use internally. Plugins have to cast the TSYaml + // to the YAML::Node, in order for them to make sure the version compatibility they need to register here and make sure the + // version is ok. + + // IMPORTANT: YAML library version should be available to query from here. This could + // be an issue if the user specify their own library and it's a different version + // This should be discussed, can we guarantee not changing the YAML version in minor + // releases can only change it in major versions. + if (std::string_view{yaml_version} != "0.6.3") { + return nullptr; + } + + rpc::RPCRegistryInfo *info = new rpc::RPCRegistryInfo(); + info->provider = provider_name; + + return (TSRPCProviderHandle)info; +} + +tsapi TSReturnCode +TSRPCRegisterMethodHandler(const char *name, TSRPCMethodCb callback, TSRPCProviderHandle info, const TSRPCHandlerOptions *opt) +{ + sdk_assert(sdk_sanity_check_rpc_handler_options(opt) == TS_SUCCESS); + + if (!rpc::add_method_handler_from_plugin( + name, + [callback](std::string_view const &id, const YAML::Node ¶ms) -> void { + std::string msgId{id.data(), id.size()}; + callback(msgId.c_str(), (TSYaml)¶ms); + }, + (const rpc::RPCRegistryInfo *)info, *opt)) { + return TS_ERROR; + } + return TS_SUCCESS; +} + +tsapi TSReturnCode +TSRPCRegisterNotificationHandler(const char *name, TSRPCNotificationCb callback, TSRPCProviderHandle info, + const TSRPCHandlerOptions *opt) +{ + sdk_assert(sdk_sanity_check_rpc_handler_options(opt) == TS_SUCCESS); + + if (!rpc::add_notification_handler( + name, [callback](const YAML::Node ¶ms) -> void { callback((TSYaml)¶ms); }, (const rpc::RPCRegistryInfo *)info, + *opt)) { + return TS_ERROR; + } + return TS_SUCCESS; +} + +tsapi TSReturnCode +TSRPCHandlerDone(TSYaml resp) +{ + Debug("rpc.api", ">> Handler seems to be done"); + std::lock_guard lock(rpc::g_rpcHandlingMutex); + auto data = *(YAML::Node *)resp; + rpc::g_rpcHandlerResponseData = data; + rpc::g_rpcHandlerProcessingCompleted = true; + rpc::g_rpcHandlingCompletion.notify_one(); + Debug("rpc.api", ">> all set."); + return TS_SUCCESS; +} + +tsapi TSReturnCode +TSRPCHandlerError(int ec, const char *descr) +{ + Debug("rpc.api", ">> Handler seems to be done with an error"); + std::lock_guard lock(rpc::g_rpcHandlingMutex); + rpc::g_rpcHandlerResponseData = ts::Errata{}.push(1, ec, std::string{descr}); + rpc::g_rpcHandlerProcessingCompleted = true; + rpc::g_rpcHandlingCompletion.notify_one(); + Debug("rpc.api", ">> error flagged."); + return TS_SUCCESS; +} diff --git a/tests/Makefile.am b/tests/Makefile.am index 391078815e5..7e848033a07 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -19,6 +19,7 @@ include $(top_srcdir)/build/tidy.mk noinst_LTLIBRARIES = noinst_PROGRAMS = +pkglib_LTLIBRARIES = SUBDIRS = @@ -37,6 +38,7 @@ include gold_tests/pluginTest/tsapi/Makefile.inc include gold_tests/timeout/Makefile.inc include gold_tests/tls/Makefile.inc include tools/plugins/Makefile.inc +include gold_tests/jsonrpc/plugins/Makefile.inc TESTS = $(check_PROGRAMS) diff --git a/tests/gold_tests/jsonrpc/basic_plugin_handler.test.py b/tests/gold_tests/jsonrpc/basic_plugin_handler.test.py new file mode 100644 index 00000000000..073a9888e3f --- /dev/null +++ b/tests/gold_tests/jsonrpc/basic_plugin_handler.test.py @@ -0,0 +1,182 @@ +''' +''' +# 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. + + +import os +import sys +from jsonrpc import Notification, Request, Response + +Test.Summary = 'Test JSONRPC Handler inside a plugin' +Test.ContinueOnFail = True +# Define default ATS, we will use the runroot file for traffic_ctl +ts = Test.MakeATSProcess('ts', dump_runroot=True) + +Test.testName = 'Basic plugin handler test' + +ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'rpc|jsonrpc_plugin_handler_test' +}) + +# Load plugin +Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsBuildGoldTestsDir, + 'jsonrpc', 'plugins', '.libs', 'jsonrpc_plugin_handler_test.so'), ts) + +tr = Test.AddTestRun() +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = "echo run jsonrpc_plugin_handler_test plugin" +tr.Processes.Default.ReturnCode = 0 +tr.Processes.StillRunningAfter = ts +ts.Ready = 0 +ts.Disk.traffic_out.Content = All( + Testers.IncludesExpression( + 'Test Plugin Initialized.', + 'plugin should be properly initialized'), + Testers.IncludesExpression( + 'test_join_hosts_method successfully registered', + 'test_join_hosts_method should be properly registered'), + Testers.IncludesExpression( + 'test_join_hosts_notification successfully registered', + 'test_join_hosts_notification should be properly registered')) + + +# 1 - now let's try the registered api. +tr = Test.AddTestRun("Test registered API") +tr.AddJsonRPCClientRequest(ts, Request.show_registered_handlers()) +tr.DelayStart = 2 +# Ok we can just check like this: +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('test_join_hosts_method', 'Should be listed'), + Testers.IncludesExpression('test_join_hosts_notification', 'Should be listed') +) + +# 2 - We perform the same as above but without implicit 'show_registered_handlers()' call. +tr = Test.AddTestRun("Test registered API - using AddJsonRPCShowRegisterHandlerRequest") +tr.AddJsonRPCShowRegisterHandlerRequest(ts) + +# Ok we can just check like this: +tr.Processes.Default.Streams.stdout = All( + Testers.IncludesExpression('test_join_hosts_method', 'Should be listed'), + Testers.IncludesExpression('test_join_hosts_notification', 'Should be listed') +) + +# 3 - Call the actual plugin method handler: +tr = Test.AddTestRun("Test JSONRPC test_join_hosts_method") +tr.AddJsonRPCClientRequest(ts, Request.test_join_hosts_method(hosts=["yahoo.com", "aol.com", "vz.com"])) + + +def validate_host_response(resp: Response): + ''' + Custom check for a particular response values. Also error validation. + ''' + if resp.is_error(): + return (False, resp.error_as_str()) + + join_str = resp.result['join'] + expected = "yahoo.comaol.comvz.com" + if join_str != expected: + return (False, f"Invalid response, expected yahoo.comaol.comvz.com, got {join_str}") + + return (True, "All good") + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_host_response) + +# Custom response validator: + + +def _validate_response_for_test_io_on_et_task(added, updated, resp: Response): + ''' + Custom check for a particular response values. Also error validation. + ''' + if resp.is_error(): + return (False, resp.error_as_str()) + + addedHosts = resp.result['addedHosts'] + updatedHosts = resp.result['updatedHosts'] + + if addedHosts == added and updatedHosts == updated: + return (True, "All good") + + return ( + False, + f"Invalid response, expected addedHosts == {added} and updatedHosts == {updated}, got {addedHosts} and {updatedHosts}") + + +# 4 - Call plugin handler to perform a IO task on the ET_TASK thread. +tr = Test.AddTestRun("Test test_io_on_et_task") +newHosts = [ + {'name': 'brbzull', 'status': 'up'}, + {'name': 'brbzull1', 'status': 'down'}, + {'name': 'brbzull3', 'status': 'up'}, + {'name': 'brbzull4', 'status': 'down'}, + {'name': 'yahoo', 'status': 'down'}, + {'name': 'trafficserver', 'status': 'down'} +] + +# the jsonrpc query +tr.AddJsonRPCClientRequest(ts, Request.test_io_on_et_task(hosts=newHosts)) + + +def validate_response_for_test_io_on_et_task_1(resp: Response): + return _validate_response_for_test_io_on_et_task('6', '0', resp) + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_response_for_test_io_on_et_task_1) + +# 5 +tr = Test.AddTestRun("Test test_io_on_et_task - update, yahoo up") +updateYahoo = [ + {'name': 'yahoo', 'status': 'up'} +] + +# the jsonrpc query +tr.AddJsonRPCClientRequest(ts, Request.test_io_on_et_task(hosts=updateYahoo)) + + +def validate_response_for_test_io_on_et_task_2(resp: Response): + return _validate_response_for_test_io_on_et_task('0', '1', resp) + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_response_for_test_io_on_et_task_2) + +# 6 +tr = Test.AddTestRun("Test privileged field from plugin handlers") +tr.AddJsonRPCClientRequest(ts, Request.get_service_descriptor()) + + +def validate_privileged_field_for_method(resp: Response, params): + for (name, val) in params: + if resp.is_error(): + return (False, resp.error_as_str()) + + data = resp.result['methods'] + e = list(filter(lambda x: x['name'] == name, data))[0] + # Method should be registered. + if e is None or e['privileged'] != val: + return (False, f"{name} privileged != {val}") + + return (True, "All good") + + +def validate_privileged_field(resp: Response): + return validate_privileged_field_for_method(resp, [('test_join_hosts_method', "1"), ('test_io_on_et_task', '1'), + ('test_join_hosts_notification', "0")]) + + +tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(validate_privileged_field) diff --git a/tests/gold_tests/jsonrpc/plugins/Makefile.inc b/tests/gold_tests/jsonrpc/plugins/Makefile.inc new file mode 100644 index 00000000000..33ab4eb0574 --- /dev/null +++ b/tests/gold_tests/jsonrpc/plugins/Makefile.inc @@ -0,0 +1,27 @@ +# 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. + +noinst_LTLIBRARIES += gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.la +gold_tests_jsonrpc_plugins_jsonrpc_plugin_handler_test_la_SOURCES = gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc + +AM_CPPFLAGS += \ + -I$(abs_top_srcdir)/mgmt \ + @YAMLCPP_INCLUDES@ + +gold_tests_jsonrpc_plugins_jsonrpc_plugin_handler_test_la_LDFLAGS = \ + $(AM_LDFLAGS) + + diff --git a/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc b/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc new file mode 100644 index 00000000000..0ee8707cbad --- /dev/null +++ b/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc @@ -0,0 +1,301 @@ +/** @file + + Test JSONRPC method and notification handling inside a plugin. + + @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 +#include +#include +#include +#include + +#include "yaml-cpp/yaml.h" +#include "tscore/ts_file.h" +#include "tscore/Errata.h" +#include "tscore/I_Layout.h" +#include "tscore/BufferWriter.h" +#include "rpc/jsonrpc/JsonRPC.h" + +namespace +{ +static constexpr char PLUGIN_NAME[] = "jsonrpc_plugin_handler_test"; + +static const std::string MY_YAML_VERSION{"0.6.3"}; +static const std::string RPC_PROVIDER_NAME{"RPC Plugin test"}; +} // namespace + +namespace +{ +void +test_join_hosts_method(const char *id, TSYaml p) +{ + TSDebug(PLUGIN_NAME, "Got a call! id: %s", id); + YAML::Node params = *reinterpret_cast(p); + + // This handler errors. + enum HandlerErrors { NO_HOST_ERROR = 10001, EMPTY_HOSTS_ERROR, UNKNOWN_ERROR }; + try { + std::vector hosts; + if (auto node = params["hosts"]) { + hosts = node.as>(); + } else { + // We can't continue. Notify the RPC manager. + TSRPCHandlerError(NO_HOST_ERROR, "No host provided"); + return; + } + + if (0 == hosts.size()) { + // We can't continue. Notify the RPC manager. + TSRPCHandlerError(EMPTY_HOSTS_ERROR, "At least one host should be provided"); + return; + } + + std::string join; + std::for_each(std::begin(hosts), std::end(hosts), [&join](auto &&s) { join += s; }); + YAML::Node resp; + resp["join"] = join; + // All done. Notify the RPC manager. + TSRPCHandlerDone(reinterpret_cast(&resp)); + } catch (YAML::Exception const &ex) { + TSDebug(PLUGIN_NAME, "Oops, something went wrong: %s", ex.what()); + TSRPCHandlerError(UNKNOWN_ERROR, ex.what()); + } +} + +// It's a notificaion, we do not care to respond back to the JSONRPC manager. +void +test_join_hosts_notification(TSYaml p) +{ + TSDebug(PLUGIN_NAME, "Got a call!"); + try { + YAML::Node params = *reinterpret_cast(p); + + std::vector hosts; + if (auto hosts = params["hosts"]) { + hosts = hosts.as>(); + } + if (0 == hosts.size()) { + TSDebug(PLUGIN_NAME, "No hosts field provided. Nothing we can do. No response back."); + return; + } + std::string join; + std::for_each(std ::begin(hosts), std::end(hosts), [&join](auto &&s) { join += s; }); + TSDebug(PLUGIN_NAME, "Notification properly handled: %s", join.c_str()); + } catch (YAML::Exception const &ex) { + TSDebug(PLUGIN_NAME, "Oops, something went wrong: %s", ex.what()); + } +} +} // namespace +namespace +{ +// Incoming host info structure. +struct HostItem { + std::string name; + std::string status; +}; + +int +CB_handle_rpc_io_call(TSCont contp, TSEvent event, void *data) +{ + namespace fs = ts::file; + + TSDebug(PLUGIN_NAME, "Working on the update now"); + YAML::Node params = *static_cast(TSContDataGet(contp)); + + // This handler errors. + enum HandlerErrors { INVALID_PARAM_TYPE = 10010, INVALID_HOST_PARAM_TYPE, FILE_UPDATE_ERROR, UNKNOWN_ERROR }; + + // we only care for a map type {} + if (params.Type() != YAML::NodeType::Map) { + TSRPCHandlerError(INVALID_PARAM_TYPE, "Handler is expecting a map."); + return TS_SUCCESS; + } + + // we want to keep track of the work done! This will become part of the response + // and it will be validated on the client side. The whole test validation is base + // on this. + int updatedHosts{0}; + int addedHosts{0}; + + std::vector incHosts; + if (auto &&passedHosts = params["hosts"]; passedHosts.Type() == YAML::NodeType::Sequence) { + // fill in + for (auto &&h : passedHosts) { + std::string name, status; + if (auto &&n = h["name"]) { + name = n.as(); + } + + if (auto &&s = h["status"]) { + status = s.as(); + } + incHosts.push_back({name, status}); + } + } else { + TSRPCHandlerError(INVALID_HOST_PARAM_TYPE, "not a sequence, we expect a list of hosts"); + return TS_SUCCESS; + } + + // Basic stuffs here. + // We open the file if exist, we update/add the host in the structure. For simplicity we do not delete anything. + fs::path sandbox = ts::file::current_path(); + fs::path dumpFile = sandbox / "my_test_plugin_dump.yaml"; + bool newFile{false}; + if (!ts::file::exists(dumpFile)) { + newFile = true; + } + + // handle function to add new hosts to a node. + auto add_host = [](std::string const &name, std::string const &status, YAML::Node &out) -> void { + YAML::Node newHost; + newHost["name"] = name; + newHost["status"] = status; + out.push_back(newHost); + }; + + YAML::Node dump; + if (!newFile) { + try { + dump = YAML::LoadFile(dumpFile.c_str()); + if (dump.IsSequence()) { + std::vector tobeAdded; + for (auto &&incHost : incHosts) { + auto search = std::find_if(std::begin(dump), std::end(dump), [&incHost](YAML::Node const &node) { + if (auto &&n = node["name"]) { + return incHost.name == n.as(); + } else { + throw std::runtime_error("We couldn't find 'name' field."); + } + }); + + if (search != std::end(dump)) { + (*search)["status"] = incHost.status; + ++updatedHosts; + } else { + add_host(incHost.name, incHost.status, dump); + ++addedHosts; + } + } + } + } catch (YAML::Exception const &e) { + std::string buff; + ts::bwprint(buff, "Error during file handling: {}", e.what()); + TSRPCHandlerError(UNKNOWN_ERROR, buff.c_str()); + return TS_SUCCESS; + } + } else { + for (auto &&incHost : incHosts) { + add_host(incHost.name, incHost.status, dump); + ++addedHosts; + } + } + + // Dump it.. + YAML::Emitter out; + out << dump; + + fs::path tmpFile = sandbox / "tmpfile.yaml"; + std::ofstream ofs(tmpFile.c_str()); + ofs << out.c_str(); + ofs.close(); + + std::error_code ec; + if (fs::copy(tmpFile, dumpFile, ec); ec) { + std::string buff; + ts::bwprint(buff, "Error during file handling: {}, {}", ec.value(), ec.message()); + TSRPCHandlerError(FILE_UPDATE_ERROR, buff.c_str()); + return TS_SUCCESS; + } + + // clean up the temp file if possible. + if (fs::remove(tmpFile, ec); ec) { + TSDebug(PLUGIN_NAME, "Temp file could not be removed: %s", tmpFile.c_str()); + } + + // make the response. For complex structures YAML::convert::encode() would be the preferred way. + YAML::Node resp; + resp["updatedHosts"] = updatedHosts; + resp["addedHosts"] = addedHosts; + resp["dumpFile"] = dumpFile.c_str(); // In case you need this + + // we are done!! + TSContDestroy(contp); + TSRPCHandlerDone(reinterpret_cast(&resp)); + return TS_SUCCESS; +} + +// Perform a field updated on a yaml file. Host will be added or updated +void +test_io_on_et_task(const char *id, TSYaml p) +{ + // A possible scenario would be that a handler needs to perform a "heavy" operation or that the handler + // is not yet ready to perform the operation when is called, under this scenarios(and some others) + // we can use the RPC API to easily achieve this and respond just when we are ready. + TSCont c = TSContCreate(CB_handle_rpc_io_call, TSMutexCreate()); + TSContDataSet(c, p); + TSContScheduleOnPool(c, 1000 /* no particular reason */, TS_THREAD_POOL_TASK); +} +} // namespace + +void +TSPluginInit(int argc, const char *argv[]) +{ + TSPluginRegistrationInfo info; + + info.plugin_name = PLUGIN_NAME; + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Plugin registration failed", PLUGIN_NAME); + } + + // Check-in to make sure we are compliant with the YAML version in TS. + TSRPCProviderHandle rpcRegistrationInfo = TSRPCRegister(RPC_PROVIDER_NAME.c_str(), MY_YAML_VERSION.c_str()); + + if (rpcRegistrationInfo == nullptr) { + TSError("[%s] RPC handler registration failed, yaml version not supported.", PLUGIN_NAME); + } + + TSRPCHandlerOptions opt{{true}}; + if (TSRPCRegisterMethodHandler("test_join_hosts_method", test_join_hosts_method, rpcRegistrationInfo, &opt) == TS_ERROR) { + TSDebug(PLUGIN_NAME, "test_join_hosts_method failed to register"); + } else { + TSDebug(PLUGIN_NAME, "test_join_hosts_method successfully registered"); + } + + // TASK thread + if (TSRPCRegisterMethodHandler("test_io_on_et_task", test_io_on_et_task, rpcRegistrationInfo, &opt) == TS_ERROR) { + TSDebug(PLUGIN_NAME, "test_io_on_et_task failed to register"); + } else { + TSDebug(PLUGIN_NAME, "test_io_on_et_task successfully registered"); + } + + // Notification - not restricted + TSRPCHandlerOptions nOpt{{false}}; + if (TSRPCRegisterNotificationHandler("test_join_hosts_notification", test_join_hosts_notification, rpcRegistrationInfo, &nOpt) == + TS_ERROR) { + TSDebug(PLUGIN_NAME, "test_join_hosts_notification failed to register"); + } else { + TSDebug(PLUGIN_NAME, "test_join_hosts_notification successfully registered"); + } + + TSDebug(PLUGIN_NAME, "Test Plugin Initialized."); +} From 4f4eda91a9cb1371c44d93d3db3da537df3f9bfe Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Mon, 21 Feb 2022 17:13:29 +0000 Subject: [PATCH 2/3] YAML: Define macro to hold the supported yaml version. The idea is to remove this from the code and make it defined by the automake scripts. Eventually the version could be read from the yaml source when added. This is needed to validate the YAML version used by plugins when registering to the JSONRPC manager to avoid ABI compatibility issues. --- build/yaml-cpp.m4 | 1 + doc/developer-guide/api/functions/TSRPCRegister.en.rst | 2 +- .../jsonrpc/jsonrpc-handler-development.en.rst | 8 ++++---- src/traffic_server/InkAPI.cc | 9 ++------- .../jsonrpc/plugins/jsonrpc_plugin_handler_test.cc | 2 +- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/build/yaml-cpp.m4 b/build/yaml-cpp.m4 index e11dc28efc3..2f618f442a8 100644 --- a/build/yaml-cpp.m4 +++ b/build/yaml-cpp.m4 @@ -82,6 +82,7 @@ fi AC_SUBST([YAMLCPP_INCLUDES]) AC_SUBST([YAMLCPP_LIBS]) AC_SUBST([YAMLCPP_LDFLAGS]) +AC_DEFINE([YAMLCPP_LIB_VERSION], ["0.7.0"], [yamlcpp library version]) ]) diff --git a/doc/developer-guide/api/functions/TSRPCRegister.en.rst b/doc/developer-guide/api/functions/TSRPCRegister.en.rst index e4eddeaf335..baac68070a7 100644 --- a/doc/developer-guide/api/functions/TSRPCRegister.en.rst +++ b/doc/developer-guide/api/functions/TSRPCRegister.en.rst @@ -69,7 +69,7 @@ Description .. code-block:: cpp - TSRPCProviderHandle info = TSRPCRegister("FooBar's Plugin!", "0.6.3"); + TSRPCProviderHandle info = TSRPCRegister("FooBar's Plugin!", "0.7.0"); ... TSRPCRegisterMethodHandler("my_join_string_handler", &func, info); diff --git a/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst b/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst index db831a26e31..c87f8e455f8 100644 --- a/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst +++ b/doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst @@ -202,7 +202,7 @@ RPC method registration and implementation examples #include namespace { - static const std::string MY_YAML_VERSION{"0.6.3"}; + static const std::string MY_YAML_VERSION{"0.7.0"}; } void @@ -231,7 +231,7 @@ RPC method registration and implementation examples { ... // Check-in to make sure we are compliant with the YAML version in TS. - TSRPCProviderHandle rpcRegistrationInfo = TSRPCRegister("My plugin's info", "0.6.3"); + TSRPCProviderHandle rpcRegistrationInfo = TSRPCRegister("My plugin's info", "0.7.0"); if (rpcRegistrationInfo == nullptr) { TSError("[%s] RPC handler registration failed, yaml version not supported.", PLUGIN_NAME); } @@ -250,7 +250,7 @@ RPC method registration and implementation examples #include namespace { - static const std::string MY_YAML_VERSION{"0.6.3"}; + static const std::string MY_YAML_VERSION{"0.7.0"}; } int @@ -286,7 +286,7 @@ RPC method registration and implementation examples { // ... // Check-in to make sure we are compliant with the YAML version in TS. - TSRPCProviderHandle rpcRegistrationInfo = TSRPCRegister("My plugin's info", "0.6.3"); + TSRPCProviderHandle rpcRegistrationInfo = TSRPCRegister("My plugin's info", "0.7.0"); if (rpcRegistrationInfo == nullptr) { TSError("[%s] RPC handler registration failed, yaml version not supported.", PLUGIN_NAME); } diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index b328c8acc78..ff82c42a548 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -10356,13 +10356,8 @@ TSRPCRegister(const char *provider_name, const char *yaml_version) // We want to make sure that plugins are using the same yaml library version as we use internally. Plugins have to cast the TSYaml // to the YAML::Node, in order for them to make sure the version compatibility they need to register here and make sure the - // version is ok. - - // IMPORTANT: YAML library version should be available to query from here. This could - // be an issue if the user specify their own library and it's a different version - // This should be discussed, can we guarantee not changing the YAML version in minor - // releases can only change it in major versions. - if (std::string_view{yaml_version} != "0.6.3") { + // version is the same. + if (std::string_view{yaml_version} != YAMLCPP_LIB_VERSION) { return nullptr; } diff --git a/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc b/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc index 0ee8707cbad..2f55e635d70 100644 --- a/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc +++ b/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc @@ -37,7 +37,7 @@ namespace { static constexpr char PLUGIN_NAME[] = "jsonrpc_plugin_handler_test"; -static const std::string MY_YAML_VERSION{"0.6.3"}; +static const std::string MY_YAML_VERSION{"0.7.0"}; static const std::string RPC_PROVIDER_NAME{"RPC Plugin test"}; } // namespace From 3efe1afdc294118bd9c576025600adb60e7d8ae0 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 11 Oct 2022 12:05:57 +0100 Subject: [PATCH 3/3] Address code view suggestions, add length to make string view friendly api. --- include/ts/ts.h | 23 +++++--- src/traffic_server/InkAPI.cc | 21 +++---- .../plugins/jsonrpc_plugin_handler_test.cc | 57 +++++++++++-------- 3 files changed, 60 insertions(+), 41 deletions(-) diff --git a/include/ts/ts.h b/include/ts/ts.h index b00288b6d4c..bb3551100e8 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -2773,12 +2773,16 @@ typedef void (*TSRPCNotificationCb)(TSYaml params); * check-in if they intent to handle RPC commands, passing their yamlcpp library version this function will validate it against the * one used internally in TS. * - * @param yamlcpp_lib_version a string with the yamlcpp library version. A null terminated string is expected. + * @param provider_name The name of the provider. + * @param provider_len The length of the provider string. + * @param yamlcpp_lib_version a string with the yamlcpp library version. + * @param yamlcpp_lib_len The length of the yamlcpp_lib_len string. * @return A new TSRPCProviderHandle, nullptr if the yamlcpp_lib_version was not set, or the yamlcpp version does not match with * the one used internally in TS. The returned TSRPCProviderHandle will be set with the provider's name. The caller should pass the * returned TSRPCProviderHandle object to each subsequent TSRPCRegisterMethod/Notification* call. */ -tsapi TSRPCProviderHandle TSRPCRegister(const char *provider_name, const char *yamlcpp_lib_version); +tsapi TSRPCProviderHandle TSRPCRegister(const char *provider_name, size_t provider_len, const char *yamlcpp_lib_version, + size_t yamlcpp_lib_len); /** * @brief Add new registered method handler to the JSON RPC engine. @@ -2786,15 +2790,16 @@ tsapi TSRPCProviderHandle TSRPCRegister(const char *provider_name, const char *y * @param name Call name to be exposed by the RPC Engine, this should match the incoming request. i.e: If you register 'get_stats' * then the incoming jsonrpc call should have this very same name in the 'method' field. .. {...'method': * 'get_stats'...} . + * @param name_len The length of the name string. * @param callback The function to be registered. See @c TSRPCMethodCb * @param info TSRPCProviderHandle pointer, this will be used to provide more context information about this call. This object - * ideally should be the one returned by the TSRPCRegister API. + * ideally should be the one returned by the TSRPCRegister API. * @param opt Pointer to @c TSRPCHandlerOptions object. This will be used to store specifics about a particular call, the rpc * manager will use this object to perform certain actions. A copy of this object wil be stored by the rpc manager. * * @return TS_SUCCESS if the handler was successfully registered, TS_ERROR if the handler is already registered. */ -tsapi TSReturnCode TSRPCRegisterMethodHandler(const char *name, TSRPCMethodCb callback, TSRPCProviderHandle info, +tsapi TSReturnCode TSRPCRegisterMethodHandler(const char *name, size_t name_len, TSRPCMethodCb callback, TSRPCProviderHandle info, const TSRPCHandlerOptions *opt); /** @@ -2803,15 +2808,16 @@ tsapi TSReturnCode TSRPCRegisterMethodHandler(const char *name, TSRPCMethodCb ca * @param name Call name to be exposed by the RPC Engine, this should match the incoming request. i.e: If you register 'get_stats' * then the incoming jsonrpc call should have this very same name in the 'method' field. .. {...'method': * 'get_stats'...} . + * @param name_len The length of the name string. * @param callback The function to be registered. See @c TSRPCNotificationCb * @param info TSRPCProviderHandle pointer, this will be used to provide more description for instance, when logging before or after - * a call. This object ideally should be the one returned by the TSRPCRegister API. + * a call. This object ideally should be the one returned by the TSRPCRegister API. * @param opt Pointer to @c TSRPCHandlerOptions object. This will be used to store specifics about a particular call, the rpc * manager will use this object to perform certain actions. A copy of this object wil be stored by the rpc manager. * @return TS_SUCCESS if the handler was successfully registered, TS_ERROR if the handler is already registered. */ -tsapi TSReturnCode TSRPCRegisterNotificationHandler(const char *name, TSRPCNotificationCb callback, TSRPCProviderHandle info, - const TSRPCHandlerOptions *opt); +tsapi TSReturnCode TSRPCRegisterNotificationHandler(const char *name, size_t name_len, TSRPCNotificationCb callback, + TSRPCProviderHandle info, const TSRPCHandlerOptions *opt); /** * @brief Function to notify the JSONRPC engine that the current handler is done working. @@ -2833,10 +2839,11 @@ tsapi TSReturnCode TSRPCHandlerDone(TSYaml resp); * call. * @param code Error code. * @param descr A text with a description of the error. + * @param descr_len The length of the descrition string. * @note The @c code and @c descr will be part of the @c 'data' field in the jsonrpc error response. * @return TS_SUCCESS if no issues. TS_ERROR otherwise. */ -tsapi TSReturnCode TSRPCHandlerError(int code, const char *descr); +tsapi TSReturnCode TSRPCHandlerError(int code, const char *descr, size_t descr_len); #ifdef __cplusplus } diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index ff82c42a548..640f010efef 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -10349,7 +10349,7 @@ extern bool g_rpcHandlerProcessingCompleted; } // namespace rpc tsapi TSRPCProviderHandle -TSRPCRegister(const char *provider_name, const char *yaml_version) +TSRPCRegister(const char *provider_name, size_t provider_len, const char *yaml_version, size_t yamlcpp_lib_len) { sdk_assert(sdk_sanity_check_null_ptr(yaml_version) == TS_SUCCESS); sdk_assert(sdk_sanity_check_null_ptr(provider_name) == TS_SUCCESS); @@ -10357,23 +10357,24 @@ TSRPCRegister(const char *provider_name, const char *yaml_version) // We want to make sure that plugins are using the same yaml library version as we use internally. Plugins have to cast the TSYaml // to the YAML::Node, in order for them to make sure the version compatibility they need to register here and make sure the // version is the same. - if (std::string_view{yaml_version} != YAMLCPP_LIB_VERSION) { + if (std::string_view{yaml_version, yamlcpp_lib_len} != YAMLCPP_LIB_VERSION) { return nullptr; } rpc::RPCRegistryInfo *info = new rpc::RPCRegistryInfo(); - info->provider = provider_name; + info->provider = {provider_name, provider_len}; return (TSRPCProviderHandle)info; } tsapi TSReturnCode -TSRPCRegisterMethodHandler(const char *name, TSRPCMethodCb callback, TSRPCProviderHandle info, const TSRPCHandlerOptions *opt) +TSRPCRegisterMethodHandler(const char *name, size_t name_len, TSRPCMethodCb callback, TSRPCProviderHandle info, + const TSRPCHandlerOptions *opt) { sdk_assert(sdk_sanity_check_rpc_handler_options(opt) == TS_SUCCESS); if (!rpc::add_method_handler_from_plugin( - name, + {name, name_len}, [callback](std::string_view const &id, const YAML::Node ¶ms) -> void { std::string msgId{id.data(), id.size()}; callback(msgId.c_str(), (TSYaml)¶ms); @@ -10385,14 +10386,14 @@ TSRPCRegisterMethodHandler(const char *name, TSRPCMethodCb callback, TSRPCProvid } tsapi TSReturnCode -TSRPCRegisterNotificationHandler(const char *name, TSRPCNotificationCb callback, TSRPCProviderHandle info, +TSRPCRegisterNotificationHandler(const char *name, size_t name_len, TSRPCNotificationCb callback, TSRPCProviderHandle info, const TSRPCHandlerOptions *opt) { sdk_assert(sdk_sanity_check_rpc_handler_options(opt) == TS_SUCCESS); if (!rpc::add_notification_handler( - name, [callback](const YAML::Node ¶ms) -> void { callback((TSYaml)¶ms); }, (const rpc::RPCRegistryInfo *)info, - *opt)) { + {name, name_len}, [callback](const YAML::Node ¶ms) -> void { callback((TSYaml)¶ms); }, + (const rpc::RPCRegistryInfo *)info, *opt)) { return TS_ERROR; } return TS_SUCCESS; @@ -10412,11 +10413,11 @@ TSRPCHandlerDone(TSYaml resp) } tsapi TSReturnCode -TSRPCHandlerError(int ec, const char *descr) +TSRPCHandlerError(int ec, const char *descr, size_t descr_len) { Debug("rpc.api", ">> Handler seems to be done with an error"); std::lock_guard lock(rpc::g_rpcHandlingMutex); - rpc::g_rpcHandlerResponseData = ts::Errata{}.push(1, ec, std::string{descr}); + rpc::g_rpcHandlerResponseData = ts::Errata{}.push(1, ec, std::string{descr, descr_len}); rpc::g_rpcHandlerProcessingCompleted = true; rpc::g_rpcHandlingCompletion.notify_one(); Debug("rpc.api", ">> error flagged."); diff --git a/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc b/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc index 2f55e635d70..eec8b07adb4 100644 --- a/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc +++ b/tests/gold_tests/jsonrpc/plugins/jsonrpc_plugin_handler_test.cc @@ -35,10 +35,10 @@ namespace { -static constexpr char PLUGIN_NAME[] = "jsonrpc_plugin_handler_test"; +constexpr char PLUGIN_NAME[] = "jsonrpc_plugin_handler_test"; -static const std::string MY_YAML_VERSION{"0.7.0"}; -static const std::string RPC_PROVIDER_NAME{"RPC Plugin test"}; +const std::string MY_YAML_VERSION{"0.7.0"}; +const std::string RPC_PROVIDER_NAME{"RPC Plugin test"}; } // namespace namespace @@ -57,13 +57,15 @@ test_join_hosts_method(const char *id, TSYaml p) hosts = node.as>(); } else { // We can't continue. Notify the RPC manager. - TSRPCHandlerError(NO_HOST_ERROR, "No host provided"); + std::string descr{"No host provided"}; + TSRPCHandlerError(NO_HOST_ERROR, descr.c_str(), descr.size()); return; } if (0 == hosts.size()) { // We can't continue. Notify the RPC manager. - TSRPCHandlerError(EMPTY_HOSTS_ERROR, "At least one host should be provided"); + std::string descr{"At least one host should be provided"}; + TSRPCHandlerError(EMPTY_HOSTS_ERROR, descr.c_str(), descr.size()); return; } @@ -75,7 +77,8 @@ test_join_hosts_method(const char *id, TSYaml p) TSRPCHandlerDone(reinterpret_cast(&resp)); } catch (YAML::Exception const &ex) { TSDebug(PLUGIN_NAME, "Oops, something went wrong: %s", ex.what()); - TSRPCHandlerError(UNKNOWN_ERROR, ex.what()); + std::string descr{ex.what()}; + TSRPCHandlerError(UNKNOWN_ERROR, descr.c_str(), descr.size()); } } @@ -124,7 +127,8 @@ CB_handle_rpc_io_call(TSCont contp, TSEvent event, void *data) // we only care for a map type {} if (params.Type() != YAML::NodeType::Map) { - TSRPCHandlerError(INVALID_PARAM_TYPE, "Handler is expecting a map."); + std::string descr{"Handler is expecting a map."}; + TSRPCHandlerError(INVALID_PARAM_TYPE, descr.c_str(), descr.size()); return TS_SUCCESS; } @@ -149,7 +153,8 @@ CB_handle_rpc_io_call(TSCont contp, TSEvent event, void *data) incHosts.push_back({name, status}); } } else { - TSRPCHandlerError(INVALID_HOST_PARAM_TYPE, "not a sequence, we expect a list of hosts"); + std::string descr{"not a sequence, we expect a list of hosts"}; + TSRPCHandlerError(INVALID_HOST_PARAM_TYPE, descr.c_str(), descr.size()); return TS_SUCCESS; } @@ -197,7 +202,7 @@ CB_handle_rpc_io_call(TSCont contp, TSEvent event, void *data) } catch (YAML::Exception const &e) { std::string buff; ts::bwprint(buff, "Error during file handling: {}", e.what()); - TSRPCHandlerError(UNKNOWN_ERROR, buff.c_str()); + TSRPCHandlerError(UNKNOWN_ERROR, buff.c_str(), buff.size()); return TS_SUCCESS; } } else { @@ -220,7 +225,7 @@ CB_handle_rpc_io_call(TSCont contp, TSEvent event, void *data) if (fs::copy(tmpFile, dumpFile, ec); ec) { std::string buff; ts::bwprint(buff, "Error during file handling: {}, {}", ec.value(), ec.message()); - TSRPCHandlerError(FILE_UPDATE_ERROR, buff.c_str()); + TSRPCHandlerError(FILE_UPDATE_ERROR, buff.c_str(), buff.size()); return TS_SUCCESS; } @@ -268,33 +273,39 @@ TSPluginInit(int argc, const char *argv[]) } // Check-in to make sure we are compliant with the YAML version in TS. - TSRPCProviderHandle rpcRegistrationInfo = TSRPCRegister(RPC_PROVIDER_NAME.c_str(), MY_YAML_VERSION.c_str()); + TSRPCProviderHandle rpcRegistrationInfo = + TSRPCRegister(RPC_PROVIDER_NAME.c_str(), RPC_PROVIDER_NAME.size(), MY_YAML_VERSION.c_str(), MY_YAML_VERSION.size()); if (rpcRegistrationInfo == nullptr) { TSError("[%s] RPC handler registration failed, yaml version not supported.", PLUGIN_NAME); } TSRPCHandlerOptions opt{{true}}; - if (TSRPCRegisterMethodHandler("test_join_hosts_method", test_join_hosts_method, rpcRegistrationInfo, &opt) == TS_ERROR) { - TSDebug(PLUGIN_NAME, "test_join_hosts_method failed to register"); + std::string method_name{"test_join_hosts_method"}; + if (TSRPCRegisterMethodHandler(method_name.c_str(), method_name.size(), test_join_hosts_method, rpcRegistrationInfo, &opt) == + TS_ERROR) { + TSDebug(PLUGIN_NAME, "%s failed to register", method_name.c_str()); } else { - TSDebug(PLUGIN_NAME, "test_join_hosts_method successfully registered"); + TSDebug(PLUGIN_NAME, "%s successfully registered", method_name.c_str()); } - // TASK thread - if (TSRPCRegisterMethodHandler("test_io_on_et_task", test_io_on_et_task, rpcRegistrationInfo, &opt) == TS_ERROR) { - TSDebug(PLUGIN_NAME, "test_io_on_et_task failed to register"); + // TASK thread. + method_name = "test_io_on_et_task"; + if (TSRPCRegisterMethodHandler(method_name.c_str(), method_name.size(), test_io_on_et_task, rpcRegistrationInfo, &opt) == + TS_ERROR) { + TSDebug(PLUGIN_NAME, "%s failed to register", method_name.c_str()); } else { - TSDebug(PLUGIN_NAME, "test_io_on_et_task successfully registered"); + TSDebug(PLUGIN_NAME, "%s successfully registered", method_name.c_str()); } - // Notification - not restricted + // Notification TSRPCHandlerOptions nOpt{{false}}; - if (TSRPCRegisterNotificationHandler("test_join_hosts_notification", test_join_hosts_notification, rpcRegistrationInfo, &nOpt) == - TS_ERROR) { - TSDebug(PLUGIN_NAME, "test_join_hosts_notification failed to register"); + method_name = "test_join_hosts_notification"; + if (TSRPCRegisterNotificationHandler(method_name.c_str(), method_name.size(), test_join_hosts_notification, rpcRegistrationInfo, + &nOpt) == TS_ERROR) { + TSDebug(PLUGIN_NAME, "%s failed to register", method_name.c_str()); } else { - TSDebug(PLUGIN_NAME, "test_join_hosts_notification successfully registered"); + TSDebug(PLUGIN_NAME, "%s successfully registered", method_name.c_str()); } TSDebug(PLUGIN_NAME, "Test Plugin Initialized.");