Skip to content

Commit

Permalink
feat: make agent's command timeout configurable
Browse files Browse the repository at this point in the history
  • Loading branch information
LucioDonda committed Feb 13, 2025
1 parent 19719ce commit e29d94d
Show file tree
Hide file tree
Showing 17 changed files with 83 additions and 25 deletions.
3 changes: 3 additions & 0 deletions src/agent/communicator/include/communicator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,5 +137,8 @@ namespace communicator

/// @brief The verification mode
std::string m_verificationMode;

/// TODO:
std::time_t m_timeoutCommands;
};
} // namespace communicator
15 changes: 14 additions & 1 deletion src/agent/communicator/src/communicator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ namespace communicator
config::agent::DEFAULT_VERIFICATION_MODE);
m_verificationMode = config::agent::DEFAULT_VERIFICATION_MODE;
}

m_timeoutCommands = configurationParser->GetConfig<std::time_t>("agent", "commands_request_timeout")
.value_or(config::agent::DEFAULT_COMMANDS_REQUEST_TIMEOUT);
if (m_timeoutCommands <= 0)
{
LogWarn("Incorrect value for 'commands_request_timeout', should be above 0.",
config::agent::DEFAULT_COMMANDS_REQUEST_TIMEOUT);
m_timeoutCommands = config::agent::DEFAULT_COMMANDS_REQUEST_TIMEOUT;
}
}

bool Communicator::SendAuthenticationRequest()
Expand Down Expand Up @@ -264,7 +273,11 @@ namespace communicator
m_serverUrl,
"/api/v1/commands",
m_getHeaderInfo ? m_getHeaderInfo() : "",
m_verificationMode);
m_verificationMode,
"",
"",
"",
m_timeoutCommands);
co_await ExecuteRequestLoop(reqParams, {}, onSuccess);
}

Expand Down
22 changes: 18 additions & 4 deletions src/agent/communicator/tests/communicator_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,15 @@ TEST(CommunicatorTest, GetCommandsFromManager_CallsWithValidToken)
EXPECT_CALL(*mockHttpClientPtr, PerformHttpRequest(testing::_))
.WillOnce(Invoke([communicatorPtr, &expectedResponse1]() -> intStringTuple { return expectedResponse1; }));

const auto reqParams = http_client::HttpRequestParams(
http_client::MethodType::GET, "https://localhost:27000", "/api/v1/commands", "", "none");
const auto reqParams = http_client::HttpRequestParams(http_client::MethodType::GET,
"https://localhost:27000",
"/api/v1/commands",
"",
"none",
"",
"",
"",
60); // TODO: check value

// NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers)
intStringTuple expectedResponse2 {200, "Dummy response"};
Expand Down Expand Up @@ -265,8 +272,15 @@ TEST(CommunicatorTest, GetCommandsFromManager_Failure)
EXPECT_CALL(*mockHttpClientPtr, PerformHttpRequest(testing::_))
.WillOnce(Invoke([communicatorPtr, &expectedResponse1]() -> intStringTuple { return expectedResponse1; }));

const auto reqParams = http_client::HttpRequestParams(
http_client::MethodType::GET, "https://localhost:27000", "/api/v1/commands", "", "none");
const auto reqParams = http_client::HttpRequestParams(http_client::MethodType::GET,
"https://localhost:27000",
"/api/v1/commands",
"",
"none",
"",
"",
"",
60); // TODO: check value

// NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers)
intStringTuple expectedResponse2 {401, "Dummy response"};
Expand Down
5 changes: 4 additions & 1 deletion src/agent/http_client/include/http_request_params.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ namespace http_client
std::string User_pass;
std::string Body;
bool Use_Https;
time_t RequestTimeout;

/// @brief Constructs HttpRequestParams with specified parameters
/// @param method The HTTP method to use
Expand All @@ -47,14 +48,16 @@ namespace http_client
/// @param token Optional token for authorization
/// @param userPass Optional user credentials for basic authentication
/// @param body Optional body for the request
/// @param RequestTimeout Optional request timeout
HttpRequestParams(MethodType method,
const std::string& serverUrl,
std::string endpoint,
std::string userAgent = "",
std::string verificationMode = "none",
std::string token = "",
std::string userPass = "",
std::string body = "");
std::string body = "",
const time_t RequestTimeout = 0); // TODO: reuse constexpr

/// @brief Equality operator for comparing two HttpRequestParams objects
/// @param other The other HttpRequestParams object to compare with
Expand Down
12 changes: 11 additions & 1 deletion src/agent/http_client/src/http_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,17 @@ namespace http_client

const auto req = CreateHttpRequest(params);

co_await socket->AsyncWrite(req, ec);
if (params.RequestTimeout)
{
LogError("/*/*/*/ someTimeOut ");
co_await socket->AsyncWrite(req, ec, params.RequestTimeout);
}
else
{
LogError("/*/*/*/ someTimeOut ");
co_await socket->AsyncWrite(req, ec);
}
LogError("/*/*/*/ resume waiting ");

if (ec)
{
Expand Down
7 changes: 5 additions & 2 deletions src/agent/http_client/src/http_request_params.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@ namespace http_client
std::string verificationMode,
std::string token,
std::string userPass,
std::string body)
std::string body,
const time_t requestTimeout)
: Method(method)
, Endpoint(std::move(endpoint))
, User_agent(std::move(userAgent))
, Verification_Mode(std::move(verificationMode))
, Token(std::move(token))
, User_pass(std::move(userPass))
, Body(std::move(body))
, RequestTimeout(requestTimeout)
{
const auto result = boost::urls::parse_uri(serverUrl);

Expand Down Expand Up @@ -53,6 +55,7 @@ namespace http_client
{
return Method == other.Method && Host == other.Host && Port == other.Port && Endpoint == other.Endpoint &&
User_agent == other.User_agent && Verification_Mode == other.Verification_Mode && Token == other.Token &&
User_pass == other.User_pass && Body == other.Body && Use_Https == other.Use_Https;
User_pass == other.User_pass && Body == other.Body && Use_Https == other.Use_Https &&
RequestTimeout == other.RequestTimeout;
}
} // namespace http_client
6 changes: 4 additions & 2 deletions src/agent/http_client/src/http_socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,13 @@ namespace http_client

boost::asio::awaitable<void>
HttpSocket::AsyncWrite(const boost::beast::http::request<boost::beast::http::string_body>& req,
boost::system::error_code& ec)
boost::system::error_code& ec,
const std::time_t socketTimeout)
{
try
{
m_socket->expires_after(std::chrono::seconds(http_client::SOCKET_TIMEOUT_SECS));
LogError("/*/*/*/ waiting ");
m_socket->expires_after(std::chrono::seconds(socketTimeout));
co_await m_socket->async_write(req, ec);
}
catch (const std::exception& e)
Expand Down
3 changes: 2 additions & 1 deletion src/agent/http_client/src/http_socket.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ namespace http_client
/// @param req The request to write
/// @param ec The error code, if any occurred
boost::asio::awaitable<void> AsyncWrite(const boost::beast::http::request<boost::beast::http::string_body>& req,
boost::system::error_code& ec) override;
boost::system::error_code& ec,
const time_t socketTimeout = http_client::SOCKET_TIMEOUT_SECS) override;

/// @brief Reads a response from the socket
/// @param res The response to read
Expand Down
5 changes: 3 additions & 2 deletions src/agent/http_client/src/https_socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,12 @@ namespace http_client

boost::asio::awaitable<void>
HttpsSocket::AsyncWrite(const boost::beast::http::request<boost::beast::http::string_body>& req,
boost::system::error_code& ec)
boost::system::error_code& ec,
const time_t socketTimeout)
{
try
{
m_ssl_socket->expires_after(std::chrono::seconds(http_client::SOCKET_TIMEOUT_SECS));
m_ssl_socket->expires_after(std::chrono::seconds(socketTimeout));
co_await m_ssl_socket->async_write(req, ec);
}
catch (const std::exception& e)
Expand Down
3 changes: 2 additions & 1 deletion src/agent/http_client/src/https_socket.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ namespace http_client
/// @param req The request to write
/// @param ec The error code, if any occurred
boost::asio::awaitable<void> AsyncWrite(const boost::beast::http::request<boost::beast::http::string_body>& req,
boost::system::error_code& ec) override;
boost::system::error_code& ec,
const time_t socketTimeout = http_client::SOCKET_TIMEOUT_SECS) override;

/// @brief Reads a response from the socket
/// @param res The response to read
Expand Down
4 changes: 3 additions & 1 deletion src/agent/http_client/src/ihttp_socket.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@ namespace http_client
/// @brief Asynchronous version of Write
/// @param req The request to write
/// @param ec The error code, if any occurred
/// TODO: doc
virtual boost::asio::awaitable<void>
AsyncWrite(const boost::beast::http::request<boost::beast::http::string_body>& req,
boost::system::error_code& ec) = 0;
boost::system::error_code& ec,
const time_t socketTimeout = http_client::SOCKET_TIMEOUT_SECS) = 0;

/// @brief Reads a response from the socket
/// @param res The response to read
Expand Down
5 changes: 3 additions & 2 deletions src/agent/http_client/tests/http_client_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,11 @@ class HttpClientTest : public TestWithParam<boost::beast::http::status>

void SetupMockSocketWriteExpectations(boost::system::error_code writeEc = {})
{
EXPECT_CALL(*mockSocket, AsyncWrite(_, _))
EXPECT_CALL(*mockSocket, AsyncWrite(_, _, _))
.WillOnce(Invoke(
[writeEc](const boost::beast::http::request<boost::beast::http::string_body>&,
boost::system::error_code& ec) -> boost::asio::awaitable<void>
boost::system::error_code& ec,
[[maybe_unused]] const time_t timeout) -> boost::asio::awaitable<void>
{
ec = writeEc;
co_return;
Expand Down
6 changes: 3 additions & 3 deletions src/agent/http_client/tests/http_socket_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ TEST_F(HttpSocketTest, AsyncWriteSuccess)
boost::system::error_code ec;
boost::asio::co_spawn(
*m_ioContext,
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec); },
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec, 60); },
boost::asio::detached);

m_ioContext->run();
Expand All @@ -244,7 +244,7 @@ TEST_F(HttpSocketTest, AsyncWriteFailure)
boost::system::error_code ec;
boost::asio::co_spawn(
*m_ioContext,
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec); },
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec, 60); },
boost::asio::detached);

m_ioContext->run();
Expand All @@ -264,7 +264,7 @@ TEST_F(HttpSocketTest, AsyncWriteException)
boost::system::error_code ec;
boost::asio::co_spawn(
*m_ioContext,
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec); },
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec, 60); },
boost::asio::detached);

m_ioContext->run();
Expand Down
6 changes: 3 additions & 3 deletions src/agent/http_client/tests/https_socket_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ TEST_F(HttpsSocketTest, AsyncWriteSuccess)
boost::system::error_code ec;
boost::asio::co_spawn(
*m_ioContext,
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec); },
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec, 60); }, // TODO: Check value
boost::asio::detached);

m_ioContext->run();
Expand All @@ -244,7 +244,7 @@ TEST_F(HttpsSocketTest, AsyncWriteFailure)
boost::system::error_code ec;
boost::asio::co_spawn(
*m_ioContext,
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec); },
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec, 60); }, // TODO: Check value
boost::asio::detached);

m_ioContext->run();
Expand All @@ -264,7 +264,7 @@ TEST_F(HttpsSocketTest, AsyncWriteException)
boost::system::error_code ec;
boost::asio::co_spawn(
*m_ioContext,
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec); },
[&]() -> boost::asio::awaitable<void> { co_await m_socket->AsyncWrite(req, ec, 60); }, // TODO: Check value
boost::asio::detached);

m_ioContext->run();
Expand Down
3 changes: 2 additions & 1 deletion src/agent/http_client/tests/mocks/mock_http_socket.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ class MockHttpSocket : public http_client::IHttpSocket
MOCK_METHOD(boost::asio::awaitable<void>,
AsyncWrite,
(const boost::beast::http::request<boost::beast::http::string_body>& req,
boost::system::error_code& ec),
boost::system::error_code& ec,
const time_t timeout),
(override));

MOCK_METHOD(void,
Expand Down
2 changes: 2 additions & 0 deletions src/cmake/config.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,5 @@ set(DEFAULT_HOTFIXES true CACHE BOOL "Default inventory hotfixes")
set(QUEUE_STATUS_REFRESH_TIMER 100 CACHE STRING "Default Agent's queue refresh timer (100ms)")

set(QUEUE_DEFAULT_SIZE 10000 CACHE STRING "Default Agent's queue size (10000)")

set(DEFAULT_COMMANDS_REQUEST_TIMEOUT 60 CACHE STRING "Default Agent's command request timeout (60s)")
1 change: 1 addition & 0 deletions src/common/config/include/config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ namespace config
constexpr auto QUEUE_DEFAULT_SIZE = @QUEUE_DEFAULT_SIZE@;
constexpr auto DEFAULT_VERIFICATION_MODE = "@DEFAULT_VERIFICATION_MODE@";
constexpr std::array<const char*, 3> VALID_VERIFICATION_MODES = {"full", "certificate", "none"};
constexpr auto DEFAULT_COMMANDS_REQUEST_TIMEOUT = @DEFAULT_COMMANDS_REQUEST_TIMEOUT@;
}

namespace logcollector
Expand Down

0 comments on commit e29d94d

Please sign in to comment.