From 0ddf88a472ca2124b963556ad37dc6828edeb736 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 15 Sep 2025 10:40:07 +0200 Subject: [PATCH 1/6] Make `ValueGenerator` more flexible & easy to use This commit refactors the ValueGenerator class to be a template that can work with any container type. Previously, one has to manually take care of the used container by lazily iterating over it within a lambda. Now, the `ValueGenerator` class itself takes care of all the iteration, making it easier to use and less error-prone. The new base `Generator` class is required to allow the `JsonEncoder` to handle generators in a type-erased manner. --- lib/base/generator.hpp | 79 ++++++++++++++++++++++++------- lib/base/json.cpp | 4 +- lib/base/json.hpp | 2 +- lib/remote/objectqueryhandler.cpp | 12 +---- test/base-json.cpp | 30 ++++++++---- 5 files changed, 88 insertions(+), 39 deletions(-) diff --git a/lib/base/generator.hpp b/lib/base/generator.hpp index fa41e67fc1..2a633ee206 100644 --- a/lib/base/generator.hpp +++ b/lib/base/generator.hpp @@ -10,39 +10,86 @@ namespace icinga { /** - * ValueGenerator is a class that defines a generator function type for producing Values on demand. + * Abstract base class for generators that produce a sequence of Values. * - * This class is used to create generator functions that can yield any values that can be represented by the - * Icinga Value type. The generator function is exhausted when it returns `std::nullopt`, indicating that there - * are no more values to produce. Subsequent calls to `Next()` will always return `std::nullopt` after exhaustion. + * @note Any instance of a Generator should be treated as an @c Array -like object that produces its elements + * on-the-fly. * * @ingroup base */ -class ValueGenerator final : public Object +class Generator : public Object { public: - DECLARE_PTR_TYPEDEFS(ValueGenerator); + DECLARE_PTR_TYPEDEFS(Generator); /** - * Generates a Value using the provided generator function. + * Produces the next Value in the sequence. + * + * This method returns the next Value produced by the generator. If the generator is exhausted, + * it returns std::nullopt for all subsequent calls to this method. * - * The generator function should return an `std::optional` which contains the produced Value or - * `std::nullopt` when there are no more values to produce. After the generator function returns `std::nullopt`, - * the generator is considered exhausted, and further calls to `Next()` will always return `std::nullopt`. + * @return The next Value in the sequence, or std::nullopt if the generator is exhausted. */ - using GenFunc = std::function()>; + virtual std::optional Next() = 0; +}; - explicit ValueGenerator(GenFunc generator): m_Generator(std::move(generator)) +/** + * A generator that transforms elements of a container into Values using a provided transformation function. + * + * This class takes a container and a transformation function as input. It uses the transformation function + * to convert each element of the container into a Value. The generator produces Values on-the-fly as they + * are requested via the `Next()` method. If the transformation function returns `std::nullopt` for an element, + * even though there are more elements in the container, the generator will yield that `std::nullopt` value like + * any other Value and continue to the next element on subsequent calls to @c Next(). + * + * @tparam Container The type of the container holding the elements to be transformed. + * + * @ingroup base + */ +template +class ValueGenerator final : public Generator +{ +public: + DECLARE_PTR_TYPEDEFS(ValueGenerator); + + // The type of elements in the container and the type to be passed to the transformation function. + using ValueType = typename Container::value_type; + + // The type of the transformation function that takes a ValueType and returns an optional Value. + using TransFormFunc = std::function (const ValueType&)>; + + /** + * Constructs a ValueGenerator with the given container and transformation function. + * + * The generator will iterate over the elements of the container, applying the transformation function + * to each element to produce values on-the-fly. You must ensure that the container remains valid for + * the lifetime of the generator. + * + * @param container The container holding the elements to be transformed. + * @param generator The transformation function to convert elements into Values. + */ + ValueGenerator(Container& container, TransFormFunc generator) + : m_It{container.begin()}, m_End{container.end()}, m_Func{std::move(generator)} { } - std::optional Next() const + std::optional Next() override { - return m_Generator(); + if (m_It == m_End) { + return std::nullopt; + } + + auto next = m_Func(*m_It); + ++m_It; + return next; } private: - GenFunc m_Generator; // The generator function that produces Values. + using Iterator = typename Container::iterator; + Iterator m_It; // Current iterator position. + Iterator m_End; // End iterator position. + + TransFormFunc m_Func; // The transformation function. }; -} +} // namespace icinga diff --git a/lib/base/json.cpp b/lib/base/json.cpp index 531ab45b22..eed6679a5a 100644 --- a/lib/base/json.cpp +++ b/lib/base/json.cpp @@ -73,7 +73,7 @@ void JsonEncoder::Encode(const Value& value, boost::asio::yield_context* yc) EncodeObject(static_pointer_cast(obj), extractor, yc); } else if (type == Array::TypeInstance) { EncodeArray(static_pointer_cast(obj), yc); - } else if (auto gen(dynamic_pointer_cast(obj)); gen) { + } else if (auto gen(dynamic_pointer_cast(obj)); gen) { EncodeValueGenerator(gen, yc); } else { // Some other non-serializable object type! @@ -126,7 +126,7 @@ void JsonEncoder::EncodeArray(const Array::Ptr& array, boost::asio::yield_contex * @param yc The optional yield context for asynchronous operations. If provided, it allows the encoder * to flush the output stream safely when it has not acquired any object lock on the parent containers. */ -void JsonEncoder::EncodeValueGenerator(const ValueGenerator::Ptr& generator, boost::asio::yield_context* yc) +void JsonEncoder::EncodeValueGenerator(const Generator::Ptr& generator, boost::asio::yield_context* yc) { BeginContainer('['); bool isEmpty = true; diff --git a/lib/base/json.hpp b/lib/base/json.hpp index a5ed462807..95bb6d80c2 100644 --- a/lib/base/json.hpp +++ b/lib/base/json.hpp @@ -73,7 +73,7 @@ class JsonEncoder private: void EncodeArray(const Array::Ptr& array, boost::asio::yield_context* yc); - void EncodeValueGenerator(const ValueGenerator::Ptr& generator, boost::asio::yield_context* yc); + void EncodeValueGenerator(const Generator::Ptr& generator, boost::asio::yield_context* yc); template void EncodeObject(const Iterable& container, const ValExtractor& extractor, boost::asio::yield_context* yc); diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index 4384abb557..84520e7b4f 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -209,15 +209,7 @@ bool ObjectQueryHandler::HandleRequest( std::unordered_map>> typePermissions; std::unordered_map objectAccessAllowed; - auto it = objs.begin(); - auto generatorFunc = [&]() -> std::optional { - if (it == objs.end()) { - return std::nullopt; - } - - ConfigObject::Ptr obj = *it; - ++it; - + auto generatorFunc = [&](const ConfigObject::Ptr& obj) -> std::optional { DictionaryData result1{ { "name", obj->GetName() }, { "type", obj->GetReflectionType()->GetName() } @@ -330,7 +322,7 @@ bool ObjectQueryHandler::HandleRequest( response.set(http::field::content_type, "application/json"); response.StartStreaming(); - Dictionary::Ptr results = new Dictionary{{"results", new ValueGenerator{generatorFunc}}}; + Dictionary::Ptr results = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; results->Freeze(); bool pretty = HttpUtility::GetLastParameter(params, "pretty"); diff --git a/test/base-json.cpp b/test/base-json.cpp index 4ae17bf091..2120cf2fd1 100644 --- a/test/base-json.cpp +++ b/test/base-json.cpp @@ -18,14 +18,10 @@ BOOST_AUTO_TEST_SUITE(base_json) BOOST_AUTO_TEST_CASE(encode) { - auto generate = []() -> std::optional { - static int count = 0; - if (++count == 4) { - count = 0; - return std::nullopt; - } - return Value(count); - }; + int emptyGenCounter = 0; + std::vector empty; + std::vector vec{1, 2, 3}; + auto generate = [](int count) -> std::optional { return Value(count); }; Dictionary::Ptr input (new Dictionary({ { "array", new Array({ new Namespace() }) }, @@ -42,8 +38,17 @@ BOOST_AUTO_TEST_CASE(encode) { "string", "LF\nTAB\tAUml\xC3\xA4Ill\xC3" }, { "true", true }, { "uint", 23u }, - { "generator", new ValueGenerator(generate) }, - { "empty_generator", new ValueGenerator([]() -> std::optional { return std::nullopt; }) }, + { "generator", new ValueGenerator{vec, generate} }, + { + "empty_generator", + new ValueGenerator{ + empty, + [&emptyGenCounter](int) -> std::optional { + emptyGenCounter++; + return std::nullopt; + } + } + }, })); String output (R"EOF({ @@ -72,15 +77,20 @@ BOOST_AUTO_TEST_CASE(encode) auto got(JsonEncode(input, true)); BOOST_CHECK_EQUAL(output, got); + BOOST_CHECK_EQUAL(emptyGenCounter, 0); // Ensure the transformation function was never invoked. + input->Set("generator", new ValueGenerator{vec, generate}); std::ostringstream oss; JsonEncode(input, oss, true); + BOOST_CHECK_EQUAL(emptyGenCounter, 0); // Ensure the transformation function was never invoked. BOOST_CHECK_EQUAL(output, oss.str()); boost::algorithm::replace_all(output, " ", ""); boost::algorithm::replace_all(output, "Objectoftype'Function'", "Object of type 'Function'"); boost::algorithm::replace_all(output, "\n", ""); + input->Set("generator", new ValueGenerator{vec, generate}); + BOOST_CHECK_EQUAL(emptyGenCounter, 0); // Ensure the transformation function was never invoked. BOOST_CHECK(JsonEncode(input, false) == output); } From 9e72b981759af44f39482f794b74d8f69c67516a Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 11 Sep 2025 13:12:23 +0200 Subject: [PATCH 2/6] Add `SendJsonBody()` overload for streaming HTTP response This is similar to the already existing `HttpUtility::SendJsonBody()` function but for streaming. --- lib/remote/httputility.cpp | 22 ++++++++++++++++++++++ lib/remote/httputility.hpp | 2 ++ lib/remote/objectqueryhandler.cpp | 8 ++------ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/lib/remote/httputility.cpp b/lib/remote/httputility.cpp index b53a8721b7..4680cca642 100644 --- a/lib/remote/httputility.cpp +++ b/lib/remote/httputility.cpp @@ -52,6 +52,28 @@ Value HttpUtility::GetLastParameter(const Dictionary::Ptr& params, const String& return arr->Get(arr->GetLength() - 1); } +/** + * Stream a JSON-encoded body to the client. + * + * This function sets the Content-Type header to "application/json", starts the streaming of the response, + * and encodes the given value as JSON to the client. If pretty-print is requested, the JSON output will be + * formatted accordingly. It is assumed that the response status code and other necessary headers have already + * been set. + * + * @param response The HTTP response to send the body to. + * @param params The request parameters. + * @param val The value to encode as JSON and stream to the client. + * @param yc The yield context to use for asynchronous operations. + */ +void HttpUtility::SendJsonBody(HttpResponse& response, const Dictionary::Ptr& params, const Value& val, boost::asio::yield_context& yc) +{ + namespace http = boost::beast::http; + + response.set(http::field::content_type, "application/json"); + response.StartStreaming(); + response.GetJsonEncoder(params && GetLastParameter(params, "pretty")).Encode(val, &yc); +} + void HttpUtility::SendJsonBody(HttpResponse& response, const Dictionary::Ptr& params, const Value& val) { namespace http = boost::beast::http; diff --git a/lib/remote/httputility.hpp b/lib/remote/httputility.hpp index 6f64277136..17f7b1ce4f 100644 --- a/lib/remote/httputility.hpp +++ b/lib/remote/httputility.hpp @@ -6,6 +6,7 @@ #include "remote/url.hpp" #include "base/dictionary.hpp" #include "remote/httpmessage.hpp" +#include #include namespace icinga @@ -23,6 +24,7 @@ class HttpUtility static Dictionary::Ptr FetchRequestParameters(const Url::Ptr& url, const std::string& body); static Value GetLastParameter(const Dictionary::Ptr& params, const String& key); + static void SendJsonBody(HttpResponse& response, const Dictionary::Ptr& params, const Value& val, boost::asio::yield_context& yc); static void SendJsonBody(HttpResponse& response, const Dictionary::Ptr& params, const Value& val); static void SendJsonError(HttpResponse& response, const Dictionary::Ptr& params, const int code, const String& info = {}, const String& diagnosticInformation = {}); diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index 84520e7b4f..acb904b6c4 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -318,15 +318,11 @@ bool ObjectQueryHandler::HandleRequest( return new Dictionary{std::move(result1)}; }; - response.result(http::status::ok); - response.set(http::field::content_type, "application/json"); - response.StartStreaming(); - Dictionary::Ptr results = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; results->Freeze(); - bool pretty = HttpUtility::GetLastParameter(params, "pretty"); - response.GetJsonEncoder(pretty).Encode(results, &yc); + response.result(http::status::ok); + HttpUtility::SendJsonBody(response, params, results, yc); return true; } From b426a88afebcaca7968414cd56d26181bdbd87ff Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 15 Sep 2025 11:17:45 +0200 Subject: [PATCH 3/6] Remove thread-local `AuthenticatedApiUser` & pass it as param instead --- lib/icinga/apiactions.cpp | 101 ++++++++++++++++++++++------------ lib/icinga/apiactions.hpp | 28 +++++----- lib/remote/actionshandler.cpp | 9 +-- lib/remote/actionshandler.hpp | 2 - lib/remote/apiaction.cpp | 4 +- lib/remote/apiaction.hpp | 5 +- 6 files changed, 86 insertions(+), 63 deletions(-) diff --git a/lib/icinga/apiactions.cpp b/lib/icinga/apiactions.cpp index e7c2e35109..f6cb229c57 100644 --- a/lib/icinga/apiactions.cpp +++ b/lib/icinga/apiactions.cpp @@ -53,8 +53,11 @@ Dictionary::Ptr ApiActions::CreateResult(int code, const String& status, return result; } -Dictionary::Ptr ApiActions::ProcessCheckResult(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::ProcessCheckResult( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { using Result = Checkable::ProcessingResult; @@ -140,8 +143,11 @@ Dictionary::Ptr ApiActions::ProcessCheckResult(const ConfigObject::Ptr& object, return ApiActions::CreateResult(500, "Unexpected result (" + std::to_string(static_cast(result)) + ") for object '" + checkable->GetName() + "'. Please submit a bug report at https://github.com/Icinga/icinga2"); } -Dictionary::Ptr ApiActions::RescheduleCheck(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RescheduleCheck( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -165,8 +171,11 @@ Dictionary::Ptr ApiActions::RescheduleCheck(const ConfigObject::Ptr& object, return ApiActions::CreateResult(200, "Successfully rescheduled check for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::SendCustomNotification(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::SendCustomNotification( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -188,8 +197,11 @@ Dictionary::Ptr ApiActions::SendCustomNotification(const ConfigObject::Ptr& obje return ApiActions::CreateResult(200, "Successfully sent custom notification for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::DelayNotification(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::DelayNotification( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -206,8 +218,11 @@ Dictionary::Ptr ApiActions::DelayNotification(const ConfigObject::Ptr& object, return ApiActions::CreateResult(200, "Successfully delayed notifications for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::AcknowledgeProblem(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::AcknowledgeProblem( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -268,8 +283,11 @@ Dictionary::Ptr ApiActions::AcknowledgeProblem(const ConfigObject::Ptr& object, return ApiActions::CreateResult(200, "Successfully acknowledged problem for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::RemoveAcknowledgement(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RemoveAcknowledgement( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -292,8 +310,11 @@ Dictionary::Ptr ApiActions::RemoveAcknowledgement(const ConfigObject::Ptr& objec return ApiActions::CreateResult(200, "Successfully removed acknowledgement for object '" + checkable->GetName() + "'."); } -Dictionary::Ptr ApiActions::AddComment(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::AddComment( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -331,8 +352,11 @@ Dictionary::Ptr ApiActions::AddComment(const ConfigObject::Ptr& object, + "'.", additional); } -Dictionary::Ptr ApiActions::RemoveComment(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RemoveComment( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { ConfigObjectsSharedLock lock (std::try_to_lock); @@ -365,8 +389,11 @@ Dictionary::Ptr ApiActions::RemoveComment(const ConfigObject::Ptr& object, return ApiActions::CreateResult(200, "Successfully removed comment '" + commentName + "'."); } -Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::ScheduleDowntime( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { Checkable::Ptr checkable = static_pointer_cast(object); @@ -535,8 +562,11 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, downtimeName + "' for object '" + checkable->GetName() + "'.", additional); } -Dictionary::Ptr ApiActions::RemoveDowntime(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RemoveDowntime( + const ConfigObject::Ptr& object, + const ApiUser::Ptr&, + const Dictionary::Ptr& params +) { ConfigObjectsSharedLock lock (std::try_to_lock); @@ -588,24 +618,21 @@ Dictionary::Ptr ApiActions::RemoveDowntime(const ConfigObject::Ptr& object, } } -Dictionary::Ptr ApiActions::ShutdownProcess(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::ShutdownProcess(const ConfigObject::Ptr&, const ApiUser::Ptr&, const Dictionary::Ptr&) { Application::RequestShutdown(); return ApiActions::CreateResult(200, "Shutting down Icinga 2."); } -Dictionary::Ptr ApiActions::RestartProcess(const ConfigObject::Ptr& object, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::RestartProcess(const ConfigObject::Ptr&, const ApiUser::Ptr&, const Dictionary::Ptr&) { Application::RequestRestart(); return ApiActions::CreateResult(200, "Restarting Icinga 2."); } -Dictionary::Ptr ApiActions::GenerateTicket(const ConfigObject::Ptr&, - const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::GenerateTicket(const ConfigObject::Ptr&, const ApiUser::Ptr&, const Dictionary::Ptr& params) { if (!params->Contains("cn")) return ApiActions::CreateResult(400, "Option 'cn' is required"); @@ -653,7 +680,11 @@ Value ApiActions::GetSingleObjectByNameUsingPermissions(const String& type, cons return objs.at(0); }; -Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, const Dictionary::Ptr& params) +Dictionary::Ptr ApiActions::ExecuteCommand( + const ConfigObject::Ptr& object, + const ApiUser::Ptr& apiUser, + const Dictionary::Ptr& params +) { ApiListener::Ptr listener = ApiListener::GetInstance(); @@ -717,11 +748,11 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons nullptr, MacroProcessor::EscapeCallback(), nullptr, false ); - if (!ActionsHandler::AuthenticatedApiUser) + if (!apiUser) BOOST_THROW_EXCEPTION(std::invalid_argument("Can't find API user.")); /* Get endpoint */ - Endpoint::Ptr endpointPtr = GetSingleObjectByNameUsingPermissions(Endpoint::GetTypeName(), resolved_endpoint, ActionsHandler::AuthenticatedApiUser); + Endpoint::Ptr endpointPtr = GetSingleObjectByNameUsingPermissions(Endpoint::GetTypeName(), resolved_endpoint, apiUser); if (!endpointPtr) return ApiActions::CreateResult(404, "Can't find a valid endpoint for '" + resolved_endpoint + "'."); @@ -779,7 +810,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons Dictionary::Ptr execParams = new Dictionary(); if (command_type == "CheckCommand") { - CheckCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(CheckCommand::GetTypeName(), resolved_command, ActionsHandler::AuthenticatedApiUser); + CheckCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(CheckCommand::GetTypeName(), resolved_command, apiUser); if (!cmd) return ApiActions::CreateResult(404, "Can't find a valid " + command_type + " for '" + resolved_command + "'."); @@ -791,7 +822,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons cmd->Execute(checkable, cr, listener->GetWaitGroup(), execMacros, false); } } else if (command_type == "EventCommand") { - EventCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(EventCommand::GetTypeName(), resolved_command, ActionsHandler::AuthenticatedApiUser); + EventCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(EventCommand::GetTypeName(), resolved_command, apiUser); if (!cmd) return ApiActions::CreateResult(404, "Can't find a valid " + command_type + " for '" + resolved_command + "'."); @@ -803,7 +834,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons cmd->Execute(checkable, execMacros, false); } } else if (command_type == "NotificationCommand") { - NotificationCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(NotificationCommand::GetTypeName(), resolved_command, ActionsHandler::AuthenticatedApiUser); + NotificationCommand::Ptr cmd = GetSingleObjectByNameUsingPermissions(NotificationCommand::GetTypeName(), resolved_command, apiUser); if (!cmd) return ApiActions::CreateResult(404, "Can't find a valid " + command_type + " for '" + resolved_command + "'."); @@ -820,7 +851,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons MacroProcessor::EscapeCallback(), nullptr, false ); - User::Ptr user = GetSingleObjectByNameUsingPermissions(User::GetTypeName(), resolved_user, ActionsHandler::AuthenticatedApiUser); + User::Ptr user = GetSingleObjectByNameUsingPermissions(User::GetTypeName(), resolved_user, apiUser); if (!user) return ApiActions::CreateResult(404, "Can't find a valid user for '" + resolved_user + "'."); @@ -839,7 +870,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons MacroProcessor::EscapeCallback(), nullptr, false ); - Notification::Ptr notification = GetSingleObjectByNameUsingPermissions(Notification::GetTypeName(), resolved_notification, ActionsHandler::AuthenticatedApiUser); + Notification::Ptr notification = GetSingleObjectByNameUsingPermissions(Notification::GetTypeName(), resolved_notification, apiUser); if (!notification) return ApiActions::CreateResult(404, "Can't find a valid notification for '" + resolved_notification + "'."); @@ -852,7 +883,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons }); cmd->Execute(notification, user, cr, NotificationType::NotificationCustom, - ActionsHandler::AuthenticatedApiUser->GetName(), "", execMacros, false); + apiUser->GetName(), "", execMacros, false); } } diff --git a/lib/icinga/apiactions.hpp b/lib/icinga/apiactions.hpp index b6ba835008..cc5bafe834 100644 --- a/lib/icinga/apiactions.hpp +++ b/lib/icinga/apiactions.hpp @@ -17,20 +17,20 @@ namespace icinga class ApiActions { public: - static Dictionary::Ptr ProcessCheckResult(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RescheduleCheck(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr SendCustomNotification(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr DelayNotification(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr AcknowledgeProblem(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RemoveAcknowledgement(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr AddComment(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RemoveComment(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr ScheduleDowntime(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RemoveDowntime(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr ShutdownProcess(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr RestartProcess(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr GenerateTicket(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); - static Dictionary::Ptr ExecuteCommand(const ConfigObject::Ptr& object, const Dictionary::Ptr& params); + static Dictionary::Ptr ProcessCheckResult(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr RescheduleCheck(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr SendCustomNotification(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr DelayNotification(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr AcknowledgeProblem(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr RemoveAcknowledgement(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr AddComment(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr RemoveComment(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr ScheduleDowntime(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr RemoveDowntime(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr ShutdownProcess(const ConfigObject::Ptr&, const ApiUser::Ptr&, const Dictionary::Ptr&); + static Dictionary::Ptr RestartProcess(const ConfigObject::Ptr&, const ApiUser::Ptr&, const Dictionary::Ptr&); + static Dictionary::Ptr GenerateTicket(const ConfigObject::Ptr& object, const ApiUser::Ptr&, const Dictionary::Ptr& params); + static Dictionary::Ptr ExecuteCommand(const ConfigObject::Ptr& object, const ApiUser::Ptr& apiUser, const Dictionary::Ptr& params); private: static Dictionary::Ptr CreateResult(int code, const String& status, const Dictionary::Ptr& additional = nullptr); diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index b9853945be..442c2ccaef 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -11,8 +11,6 @@ using namespace icinga; -thread_local ApiUser::Ptr ActionsHandler::AuthenticatedApiUser; - REGISTER_URLHANDLER("/v1/actions", ActionsHandler); bool ActionsHandler::HandleRequest( @@ -79,11 +77,6 @@ bool ActionsHandler::HandleRequest( bool verbose = false; - ActionsHandler::AuthenticatedApiUser = user; - Defer a ([]() { - ActionsHandler::AuthenticatedApiUser = nullptr; - }); - if (params) verbose = HttpUtility::GetLastParameter(params, "verbose"); @@ -110,7 +103,7 @@ bool ActionsHandler::HandleRequest( } try { - results.emplace_back(action->Invoke(obj, params)); + results.emplace_back(action->Invoke(obj, user, params)); } catch (const std::exception& ex) { Dictionary::Ptr fail = new Dictionary({ { "code", 500 }, diff --git a/lib/remote/actionshandler.hpp b/lib/remote/actionshandler.hpp index 3ba856f697..4d837cd77f 100644 --- a/lib/remote/actionshandler.hpp +++ b/lib/remote/actionshandler.hpp @@ -13,8 +13,6 @@ class ActionsHandler final : public HttpHandler public: DECLARE_PTR_TYPEDEFS(ActionsHandler); - static thread_local ApiUser::Ptr AuthenticatedApiUser; - bool HandleRequest( const WaitGroup::Ptr& waitGroup, const HttpRequest& request, diff --git a/lib/remote/apiaction.cpp b/lib/remote/apiaction.cpp index ccde28190b..6f7dc2da39 100644 --- a/lib/remote/apiaction.cpp +++ b/lib/remote/apiaction.cpp @@ -9,9 +9,9 @@ ApiAction::ApiAction(std::vector types, Callback action) : m_Types(std::move(types)), m_Callback(std::move(action)) { } -Value ApiAction::Invoke(const ConfigObject::Ptr& target, const Dictionary::Ptr& params) +Value ApiAction::Invoke(const ConfigObject::Ptr& target, const ApiUser::Ptr& user, const Dictionary::Ptr& params) { - return m_Callback(target, params); + return m_Callback(target, user, params); } const std::vector& ApiAction::GetTypes() const diff --git a/lib/remote/apiaction.hpp b/lib/remote/apiaction.hpp index 2bb98b1b6b..f96aa32fd0 100644 --- a/lib/remote/apiaction.hpp +++ b/lib/remote/apiaction.hpp @@ -8,6 +8,7 @@ #include "base/value.hpp" #include "base/dictionary.hpp" #include "base/configobject.hpp" +#include "remote/apiuser.hpp" #include #include @@ -24,11 +25,11 @@ class ApiAction final : public Object public: DECLARE_PTR_TYPEDEFS(ApiAction); - typedef std::function Callback; + typedef std::function Callback; ApiAction(std::vector registerTypes, Callback function); - Value Invoke(const ConfigObject::Ptr& target, const Dictionary::Ptr& params); + Value Invoke(const ConfigObject::Ptr& target, const ApiUser::Ptr& user, const Dictionary::Ptr& params); const std::vector& GetTypes() const; From 36d771d48199200d3912f37242faeece9b6be1af Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 11 Sep 2025 13:19:35 +0200 Subject: [PATCH 4/6] HTTP: stream responses where appropriate --- lib/remote/actionshandler.cpp | 63 +++++++--------------------- lib/remote/configpackageshandler.cpp | 14 ++++--- lib/remote/configpackageshandler.hpp | 2 +- lib/remote/configstageshandler.cpp | 24 +++++------ lib/remote/configstageshandler.hpp | 2 +- lib/remote/deleteobjecthandler.cpp | 39 +++++++---------- lib/remote/modifyobjecthandler.cpp | 57 ++++++++++++------------- lib/remote/templatequeryhandler.cpp | 10 +++-- lib/remote/typequeryhandler.cpp | 32 +++++++------- lib/remote/variablequeryhandler.cpp | 19 ++++----- 10 files changed, 107 insertions(+), 155 deletions(-) diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index 442c2ccaef..1878478436 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -6,7 +6,9 @@ #include "remote/apiaction.hpp" #include "base/defer.hpp" #include "base/exception.hpp" +#include "base/generator.hpp" #include "base/logger.hpp" +#include #include using namespace icinga; @@ -70,13 +72,10 @@ bool ActionsHandler::HandleRequest( objs.emplace_back(nullptr); } - ArrayData results; - Log(LogNotice, "ApiActionHandler") << "Running action " << actionName; bool verbose = false; - if (params) verbose = HttpUtility::GetLastParameter(params, "verbose"); @@ -86,73 +85,43 @@ bool ActionsHandler::HandleRequest( return true; } - for (ConfigObject::Ptr obj : objs) { + auto generatorFunc = [&action, &user, ¶ms, &waitGroup, &wgLock, verbose]( + const ConfigObject::Ptr& obj + ) -> std::optional { if (!waitGroup->IsLockable()) { if (wgLock) { wgLock.unlock(); } - results.emplace_back(new Dictionary({ + return new Dictionary{ { "type", obj->GetReflectionType()->GetName() }, { "name", obj->GetName() }, { "code", 503 }, { "status", "Action skipped: Shutting down."} - })); - - continue; + }; } try { - results.emplace_back(action->Invoke(obj, user, params)); + return action->Invoke(obj, user, params); } catch (const std::exception& ex) { - Dictionary::Ptr fail = new Dictionary({ + Dictionary::Ptr fail = new Dictionary{ { "code", 500 }, { "status", "Action execution failed: '" + DiagnosticInformation(ex, false) + "'." } - }); + }; /* Exception for actions. Normally we would handle this inside SendJsonError(). */ if (verbose) fail->Set("diagnostic_information", DiagnosticInformation(ex)); - results.emplace_back(std::move(fail)); - } - } - - int statusCode = 500; - std::set okStatusCodes, nonOkStatusCodes; - - for (Dictionary::Ptr res : results) { - if (!res->Contains("code")) { - continue; + return fail; } + }; - auto code = res->Get("code"); - - if (code >= 200 && code <= 299) { - okStatusCodes.insert(code); - } else { - nonOkStatusCodes.insert(code); - } - } - - size_t okSize = okStatusCodes.size(); - size_t nonOkSize = nonOkStatusCodes.size(); - - if (okSize == 1u && nonOkSize == 0u) { - statusCode = *okStatusCodes.begin(); - } else if (nonOkSize == 1u) { - statusCode = *nonOkStatusCodes.begin(); - } else if (okSize >= 2u && nonOkSize == 0u) { - statusCode = 200; - } - - response.result(statusCode); - - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; + result->Freeze(); - HttpUtility::SendJsonBody(response, params, result); + response.result(http::status::ok); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index 7e0c7b02c5..937eea2d51 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -28,7 +28,7 @@ bool ConfigPackagesHandler::HandleRequest( return false; if (request.method() == http::verb::get) - HandleGet(request, response); + HandleGet(request, response, yc); else if (request.method() == http::verb::post) HandlePost(request, response); else if (request.method() == http::verb::delete_) @@ -39,7 +39,7 @@ bool ConfigPackagesHandler::HandleRequest( return true; } -void ConfigPackagesHandler::HandleGet(const HttpRequest& request, HttpResponse& response) +void ConfigPackagesHandler::HandleGet(const HttpRequest& request, HttpResponse& response, boost::asio::yield_context& yc) { namespace http = boost::beast::http; @@ -79,12 +79,14 @@ void ConfigPackagesHandler::HandleGet(const HttpRequest& request, HttpResponse& } } - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Array::Ptr resultsArr = new Array(std::move(results)); + resultsArr->Freeze(); + + Dictionary::Ptr result = new Dictionary{{"results", resultsArr}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); } void ConfigPackagesHandler::HandlePost(const HttpRequest& request, HttpResponse& response) diff --git a/lib/remote/configpackageshandler.hpp b/lib/remote/configpackageshandler.hpp index 172690f639..086ef5b70c 100644 --- a/lib/remote/configpackageshandler.hpp +++ b/lib/remote/configpackageshandler.hpp @@ -21,7 +21,7 @@ class ConfigPackagesHandler final : public HttpHandler ) override; private: - void HandleGet(const HttpRequest& request, HttpResponse& response); + void HandleGet(const HttpRequest& request, HttpResponse& response, boost::asio::yield_context& yc); void HandlePost(const HttpRequest& request, HttpResponse& response); void HandleDelete(const HttpRequest& request, HttpResponse& response); diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index b08270e56e..1f3adcd1c3 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -8,6 +8,7 @@ #include "base/application.hpp" #include "base/defer.hpp" #include "base/exception.hpp" +#include using namespace icinga; @@ -35,7 +36,7 @@ bool ConfigStagesHandler::HandleRequest( return false; if (request.method() == http::verb::get) - HandleGet(request, response); + HandleGet(request, response, yc); else if (request.method() == http::verb::post) HandlePost(request, response); else if (request.method() == http::verb::delete_) @@ -46,7 +47,7 @@ bool ConfigStagesHandler::HandleRequest( return true; } -void ConfigStagesHandler::HandleGet(const HttpRequest& request, HttpResponse& response) +void ConfigStagesHandler::HandleGet(const HttpRequest& request, HttpResponse& response, boost::asio::yield_context& yc) { namespace http = boost::beast::http; @@ -71,25 +72,22 @@ void ConfigStagesHandler::HandleGet(const HttpRequest& request, HttpResponse& re if (!ConfigPackageUtility::ValidateStageName(stageName)) return HttpUtility::SendJsonError(response, params, 400, "Invalid stage name '" + stageName + "'."); - ArrayData results; - std::vector > paths = ConfigPackageUtility::GetFiles(packageName, stageName); String prefixPath = ConfigPackageUtility::GetPackageDir() + "/" + packageName + "/" + stageName + "/"; - for (const auto& kv : paths) { - results.push_back(new Dictionary({ + auto generatorFunc = [&prefixPath](const std::pair& kv) -> std::optional { + return new Dictionary{ { "type", kv.second ? "directory" : "file" }, - { "name", kv.first.SubStr(prefixPath.GetLength()) } - })); - } + { "name", kv.first.SubStr(prefixPath.GetLength()) }, + }; + }; - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{paths, generatorFunc}}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); } void ConfigStagesHandler::HandlePost(const HttpRequest& request, HttpResponse& response) diff --git a/lib/remote/configstageshandler.hpp b/lib/remote/configstageshandler.hpp index ec333cc50c..0e5b183074 100644 --- a/lib/remote/configstageshandler.hpp +++ b/lib/remote/configstageshandler.hpp @@ -21,7 +21,7 @@ class ConfigStagesHandler final : public HttpHandler ) override; private: - void HandleGet(const HttpRequest& request, HttpResponse& response); + void HandleGet(const HttpRequest& request, HttpResponse& response, boost::asio::yield_context& yc); void HandlePost(const HttpRequest& request, HttpResponse& response); void HandleDelete(const HttpRequest& request, HttpResponse& response); }; diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index cd99f7b282..f59bc04fc1 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -9,6 +9,7 @@ #include "config/configitem.hpp" #include "base/exception.hpp" #include +#include #include using namespace icinga; @@ -74,32 +75,26 @@ bool DeleteObjectHandler::HandleRequest( return true; } - ArrayData results; - - bool success = true; - std::shared_lock wgLock{*waitGroup, std::try_to_lock}; if (!wgLock) { HttpUtility::SendJsonError(response, params, 503, "Shutting down."); return true; } - for (ConfigObject::Ptr obj : objs) { + auto generatorFunc = [&type, &waitGroup, &wgLock, cascade, verbose]( + const ConfigObject::Ptr& obj + ) -> std::optional { if (!waitGroup->IsLockable()) { if (wgLock) { wgLock.unlock(); } - results.emplace_back(new Dictionary({ + return new Dictionary{ { "type", type->GetName() }, { "name", obj->GetName() }, { "code", 503 }, { "status", "Action skipped: Shutting down."} - })); - - success = false; - - continue; + }; } int code; @@ -113,36 +108,30 @@ bool DeleteObjectHandler::HandleRequest( if (!ConfigObjectUtility::DeleteObject(obj, cascade, errors, diagnosticInformation)) { code = 500; status = "Object could not be deleted."; - success = false; } else { code = 200; status = "Object was deleted."; } - Dictionary::Ptr result = new Dictionary({ + Dictionary::Ptr result = new Dictionary{ { "type", type->GetName() }, { "name", obj->GetName() }, { "code", code }, { "status", status }, { "errors", errors } - }); + }; if (verbose) result->Set("diagnostic_information", diagnosticInformation); - results.push_back(result); - } - - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + return result; + }; - if (!success) - response.result(http::status::internal_server_error); - else - response.result(http::status::ok); + Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; + result->Freeze(); - HttpUtility::SendJsonBody(response, params, result); + response.result(http::status::ok); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index 9264e3c64d..dca08961f3 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -6,7 +6,9 @@ #include "remote/filterutility.hpp" #include "remote/apiaction.hpp" #include "base/exception.hpp" +#include "base/generator.hpp" #include +#include #include using namespace icinga; @@ -102,31 +104,28 @@ bool ModifyObjectHandler::HandleRequest( return true; } - ArrayData results; - std::shared_lock wgLock{*waitGroup, std::try_to_lock}; if (!wgLock) { HttpUtility::SendJsonError(response, params, 503, "Shutting down."); return true; } - for (ConfigObject::Ptr obj : objs) { - Dictionary::Ptr result1 = new Dictionary(); + auto generatorFunc = [&waitGroup, &wgLock, &type, &attrs, &restoreAttrs, verbose]( + const ConfigObject::Ptr& obj + ) -> std::optional { + Dictionary::Ptr result = new Dictionary(); - result1->Set("type", type->GetName()); - result1->Set("name", obj->GetName()); + result->Set("type", type->GetName()); + result->Set("name", obj->GetName()); if (!waitGroup->IsLockable()) { if (wgLock) { wgLock.unlock(); } - result1->Set("code", 503); - result1->Set("status", "Action skipped: Shutting down."); - - results.emplace_back(std::move(result1)); - - continue; + result->Set("code", 503); + result->Set("status", "Action skipped: Shutting down."); + return result; } String key; @@ -144,14 +143,13 @@ bool ModifyObjectHandler::HandleRequest( } } } catch (const std::exception& ex) { - result1->Set("code", 500); - result1->Set("status", "Attribute '" + key + "' could not be restored: " + DiagnosticInformation(ex, false)); + result->Set("code", 500); + result->Set("status", "Attribute '" + key + "' could not be restored: " + DiagnosticInformation(ex, false)); if (verbose) - result1->Set("diagnostic_information", DiagnosticInformation(ex)); + result->Set("diagnostic_information", DiagnosticInformation(ex)); - results.push_back(std::move(result1)); - continue; + return result; } try { @@ -163,28 +161,25 @@ bool ModifyObjectHandler::HandleRequest( } } } catch (const std::exception& ex) { - result1->Set("code", 500); - result1->Set("status", "Attribute '" + key + "' could not be set: " + DiagnosticInformation(ex, false)); + result->Set("code", 500); + result->Set("status", "Attribute '" + key + "' could not be set: " + DiagnosticInformation(ex, false)); if (verbose) - result1->Set("diagnostic_information", DiagnosticInformation(ex)); + result->Set("diagnostic_information", DiagnosticInformation(ex)); - results.push_back(std::move(result1)); - continue; + return result; } - result1->Set("code", 200); - result1->Set("status", "Attributes updated."); - - results.push_back(std::move(result1)); - } + result->Set("code", 200); + result->Set("status", "Attributes updated."); + return result; + }; - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/templatequeryhandler.cpp b/lib/remote/templatequeryhandler.cpp index 81261f02d0..8a8ec1be19 100644 --- a/lib/remote/templatequeryhandler.cpp +++ b/lib/remote/templatequeryhandler.cpp @@ -125,12 +125,14 @@ bool TemplateQueryHandler::HandleRequest( return true; } - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(objs)) } - }); + Array::Ptr resultArr = new Array(std::move(objs)); + resultArr->Freeze(); + + Dictionary::Ptr result = new Dictionary{{"results", resultArr}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/typequeryhandler.cpp b/lib/remote/typequeryhandler.cpp index dda19cd120..a17406df8c 100644 --- a/lib/remote/typequeryhandler.cpp +++ b/lib/remote/typequeryhandler.cpp @@ -4,8 +4,10 @@ #include "remote/httputility.hpp" #include "remote/filterutility.hpp" #include "base/configtype.hpp" +#include "base/generator.hpp" #include "base/scriptglobal.hpp" #include "base/logger.hpp" +#include #include using namespace icinga; @@ -89,23 +91,19 @@ bool TypeQueryHandler::HandleRequest( return true; } - ArrayData results; - - for (Type::Ptr obj : objs) { - Dictionary::Ptr result1 = new Dictionary(); - results.push_back(result1); - + auto generatorFunc = [](const Type::Ptr& obj) -> std::optional { + Dictionary::Ptr result = new Dictionary(); Dictionary::Ptr resultAttrs = new Dictionary(); - result1->Set("name", obj->GetName()); - result1->Set("plural_name", obj->GetPluralName()); + result->Set("name", obj->GetName()); + result->Set("plural_name", obj->GetPluralName()); if (obj->GetBaseType()) - result1->Set("base", obj->GetBaseType()->GetName()); - result1->Set("abstract", obj->IsAbstract()); - result1->Set("fields", resultAttrs); + result->Set("base", obj->GetBaseType()->GetName()); + result->Set("abstract", obj->IsAbstract()); + result->Set("fields", resultAttrs); Dictionary::Ptr prototype = dynamic_pointer_cast(obj->GetPrototype()); Array::Ptr prototypeKeys = new Array(); - result1->Set("prototype_keys", prototypeKeys); + result->Set("prototype_keys", prototypeKeys); if (prototype) { ObjectLock olock(prototype); @@ -143,14 +141,14 @@ bool TypeQueryHandler::HandleRequest( { "deprecated", static_cast(field.Attributes & FADeprecated) } })); } - } + return result; + }; - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/variablequeryhandler.cpp b/lib/remote/variablequeryhandler.cpp index b67fe906fe..eec179048b 100644 --- a/lib/remote/variablequeryhandler.cpp +++ b/lib/remote/variablequeryhandler.cpp @@ -4,10 +4,12 @@ #include "remote/httputility.hpp" #include "remote/filterutility.hpp" #include "base/configtype.hpp" +#include "base/generator.hpp" #include "base/scriptglobal.hpp" #include "base/logger.hpp" #include "base/serializer.hpp" #include "base/namespace.hpp" +#include #include using namespace icinga; @@ -98,22 +100,19 @@ bool VariableQueryHandler::HandleRequest( return true; } - ArrayData results; - - for (Dictionary::Ptr var : objs) { - results.emplace_back(new Dictionary({ + auto generatorFunc = [](const Dictionary::Ptr& var) -> std::optional { + return new Dictionary{ { "name", var->Get("name") }, { "type", var->Get("type") }, { "value", Serialize(var->Get("value"), 0) } - })); - } + }; + }; - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } From 2f538c214d28490cb6357f4f53a92f5cb9a7c158 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 11 Sep 2025 13:22:31 +0200 Subject: [PATCH 5/6] HTTP: send http status `accepted` where appropriate This better reflects the request handling, since the headers are sent before the request is fully processed. This status code just indicates that we successfully accepted the request but is not guaranteed that all the subsequent actions will also be successful. --- lib/remote/actionshandler.cpp | 2 +- lib/remote/deleteobjecthandler.cpp | 2 +- lib/remote/modifyobjecthandler.cpp | 2 +- lib/remote/objectqueryhandler.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index 1878478436..889b487b68 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -120,7 +120,7 @@ bool ActionsHandler::HandleRequest( Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; result->Freeze(); - response.result(http::status::ok); + response.result(http::status::accepted); HttpUtility::SendJsonBody(response, params, result, yc); return true; diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index f59bc04fc1..4414b0e216 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -130,7 +130,7 @@ bool DeleteObjectHandler::HandleRequest( Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; result->Freeze(); - response.result(http::status::ok); + response.result(http::status::accepted); HttpUtility::SendJsonBody(response, params, result, yc); return true; diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index dca08961f3..3cc76cbaea 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -178,7 +178,7 @@ bool ModifyObjectHandler::HandleRequest( Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; result->Freeze(); - response.result(http::status::ok); + response.result(http::status::accepted); HttpUtility::SendJsonBody(response, params, result, yc); return true; diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index acb904b6c4..3691b2fb6f 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -321,7 +321,7 @@ bool ObjectQueryHandler::HandleRequest( Dictionary::Ptr results = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; results->Freeze(); - response.result(http::status::ok); + response.result(http::status::accepted); HttpUtility::SendJsonBody(response, params, results, yc); return true; From 08b5776f8ea10621e24c69d35f44d391002ed859 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 11 Sep 2025 16:06:30 +0200 Subject: [PATCH 6/6] doc: mention the HTTP status code used for streaming & fix client code --- doc/12-icinga2-api.md | 57 +++++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/doc/12-icinga2-api.md b/doc/12-icinga2-api.md index 8ee96aa86b..39a434f388 100644 --- a/doc/12-icinga2-api.md +++ b/doc/12-icinga2-api.md @@ -154,6 +154,30 @@ was malformed. A status in the range of 500 generally means that there was a server-side problem and Icinga 2 is unable to process your request. +Starting with Icinga 2 v2.16.0 we introduced HTTP chunked transfer encoding for some of the API endpoints. +As a consequence, the `/v1/actions/` and `/v1/objects/` endpoints will always return a `202 Accepted` status code as a +general HTTP status, but the status of each individual operation remains the same as before, and will continue to report +`2xx`, `4xx` or `5xx` status codes. For example, you may see such a response when trying to delete multiple objects. + +```json +{ + "results": [ + { + "code": 200, + "status": "Object was deleted." + }, + { + "code": 500, + "status": "Object could not be deleted.", + } + ] +} +``` + +In such cases, you should always check the individual result entries for their status code and not rely solely on the +overall HTTP status code. There are also a number of other endpoints which use chunked transfer encoding, but have no +behavioral difference in terms of the overall HTTP status code and will continue to return `200` for successful requests. + ### Security * HTTPS only. @@ -2742,7 +2766,7 @@ r = requests.post(request_url, print "Request URL: " + str(r.url) print "Status code: " + str(r.status_code) -if (r.status_code == 200): +if (r.status_code == 202): print "Result: " + json.dumps(r.json()) else: print r.text @@ -2791,7 +2815,7 @@ rescue => e end puts "Status: " + response.code.to_s -if response.code == 200 +if response.code == 202 puts "Result: " + (JSON.pretty_generate JSON.parse(response.body)) else puts "Error: " + response @@ -2846,7 +2870,7 @@ $code = curl_getinfo($ch, CURLINFO_HTTP_CODE); curl_close($ch); print "Status: " . $code . "\n"; -if ($code == 200) { +if ($code == 202) { $response = json_decode($response, true); print_r($response); } @@ -2898,7 +2922,7 @@ $client->POST("/v1/objects/services", $data); my $status = $client->responseCode(); print "Status: " . $status . "\n"; my $response = $client->responseContent(); -if ($status == 200) { +if ($status == 202) { print "Result: " . Dumper(decode_json($response)) . "\n"; } else { print "Error: " . $response . "\n"; @@ -2921,14 +2945,13 @@ import ( "bytes" "crypto/tls" "log" - "io/ioutil" "net/http" ) func main() { - var urlBase= "https://localhost:5665" - var apiUser= "root" - var apiPass= "icinga" + var urlBase = "https://localhost:5665" + var apiUser = "root" + var apiPass = "icinga" urlEndpoint := urlBase + "/v1/objects/services" @@ -2943,7 +2966,7 @@ func main() { "filter": "match(\"ping*\", service.name)" }`) - req, err := http.NewRequest("POST", urlEndpoint, bytes.NewBuffer(requestBody)) + req, err := http.NewRequest("POST", urlEndpoint, bytes.NewReader(requestBody)) req.Header.Set("Accept", "application/json") req.Header.Set("X-HTTP-Method-Override", "GET") @@ -2958,13 +2981,16 @@ func main() { log.Print("Response status:", resp.Status) - bodyBytes, _ := ioutil.ReadAll(resp.Body) - bodyString := string(bodyBytes) + buf := new(bytes.Buffer) + if _, err := buf.ReadFrom(resp.Body); err != nil { + log.Fatal("Read error:", err) + return + } - if resp.StatusCode == http.StatusOK { - log.Print("Result: " + bodyString) + if resp.StatusCode == http.StatusAccepted { + log.Print("Result: " + buf.String()) } else { - log.Fatal(bodyString) + log.Fatal(buf.String()) } } ``` @@ -2972,8 +2998,7 @@ func main() { Build the binary: ```bash -go build icinga.go -./icinga +go run ./icinga.go ``` #### Example API Client in Powershell