From a90e7c2bef3048fbf7c260e672b02be7af8e236b Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Thu, 28 Mar 2024 15:56:53 +0100 Subject: [PATCH 01/48] MINIFICPP-2314 - Send asset state hash in heartbeat, implement c2 asset sync --- conf/minifi.properties | 2 +- libminifi/include/FlowController.h | 4 +- libminifi/include/c2/C2Agent.h | 8 +- libminifi/include/c2/C2Payload.h | 7 +- .../core/state/MetricsPublisherStore.h | 3 +- .../core/state/nodes/AssetInformation.h | 40 +++ .../core/state/nodes/ResponseNodeLoader.h | 5 +- libminifi/include/utils/file/AssetManager.h | 66 +++++ libminifi/include/utils/file/PathUtils.h | 22 ++ libminifi/src/FlowController.cpp | 5 +- libminifi/src/c2/C2Agent.cpp | 130 +++++++-- .../src/core/state/MetricsPublisherStore.cpp | 4 +- .../src/core/state/nodes/AssetInformation.cpp | 37 +++ .../core/state/nodes/ResponseNodeLoader.cpp | 14 +- libminifi/src/utils/file/AssetManager.cpp | 151 ++++++++++ .../test/integration/C2AssetSyncTest.cpp | 260 ++++++++++++++++++ minifi_main/MiNiFiMain.cpp | 6 +- 17 files changed, 720 insertions(+), 44 deletions(-) create mode 100644 libminifi/include/core/state/nodes/AssetInformation.h create mode 100644 libminifi/include/utils/file/AssetManager.h create mode 100644 libminifi/src/core/state/nodes/AssetInformation.cpp create mode 100644 libminifi/src/utils/file/AssetManager.cpp create mode 100644 libminifi/test/integration/C2AssetSyncTest.cpp diff --git a/conf/minifi.properties b/conf/minifi.properties index 5e5c295455..debf11e26d 100644 --- a/conf/minifi.properties +++ b/conf/minifi.properties @@ -87,7 +87,7 @@ nifi.content.repository.class.name=DatabaseContentRepository #nifi.c2.rest.url= #nifi.c2.rest.url.ack= #nifi.c2.rest.ssl.context.service= -nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation +nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation,AssetInformation ## Minimize heartbeat payload size by excluding agent manifest from the heartbeat nifi.c2.full.heartbeat=false ## heartbeat twice a minute diff --git a/libminifi/include/FlowController.h b/libminifi/include/FlowController.h index 6f14f04463..6c8308c209 100644 --- a/libminifi/include/FlowController.h +++ b/libminifi/include/FlowController.h @@ -55,6 +55,7 @@ #include "TimerDrivenSchedulingAgent.h" #include "utils/Id.h" #include "utils/file/FileSystem.h" +#include "utils/file/AssetManager.h" #include "core/state/nodes/ResponseNodeLoader.h" #include "core/state/MetricsPublisher.h" #include "core/state/MetricsPublisherStore.h" @@ -72,7 +73,8 @@ class FlowController : public core::controller::ForwardingControllerServiceProvi FlowController(std::shared_ptr provenance_repo, std::shared_ptr flow_file_repo, std::shared_ptr configure, std::shared_ptr flow_configuration, std::shared_ptr content_repo, std::unique_ptr metrics_publisher_store = nullptr, - std::shared_ptr filesystem = std::make_shared(), std::function request_restart = []{}); + std::shared_ptr filesystem = std::make_shared(), std::function request_restart = []{}, + std::shared_ptr asset_manager = {}); ~FlowController() override; diff --git a/libminifi/include/c2/C2Agent.h b/libminifi/include/c2/C2Agent.h index 63829fd2c3..2e7412f739 100644 --- a/libminifi/include/c2/C2Agent.h +++ b/libminifi/include/c2/C2Agent.h @@ -43,6 +43,7 @@ #include "utils/ThreadPool.h" #include "utils/file/FileSystem.h" #include "C2Utils.h" +#include "utils/file/AssetManager.h" namespace org::apache::nifi::minifi::c2 { @@ -62,7 +63,8 @@ class C2Agent : public state::UpdateController { C2Agent(std::shared_ptr configuration, std::weak_ptr node_reporter, std::shared_ptr filesystem, - std::function request_restart); + std::function request_restart, + std::shared_ptr asset_manager); void initialize(core::controller::ControllerServiceProvider *controller, state::Pausable *pause_handler, state::StateMonitor* update_sink); void start() override; @@ -131,6 +133,8 @@ class C2Agent : public state::UpdateController { */ void handle_describe(const C2ContentResponse &resp); + void handle_sync(const C2ContentResponse &resp); + enum class UpdateResult { NO_UPDATE, @@ -235,6 +239,8 @@ class C2Agent : public state::UpdateController { // time point the last time we performed a heartbeat. std::chrono::steady_clock::time_point last_run_; + + std::shared_ptr asset_manager_; }; } // namespace org::apache::nifi::minifi::c2 diff --git a/libminifi/include/c2/C2Payload.h b/libminifi/include/c2/C2Payload.h index 9ff1aaaa7d..c19f198eac 100644 --- a/libminifi/include/c2/C2Payload.h +++ b/libminifi/include/c2/C2Payload.h @@ -43,7 +43,8 @@ enum class Operation : uint8_t { clear, transfer, pause, - resume + resume, + sync }; enum class DescribeOperand : uint8_t { @@ -70,6 +71,10 @@ enum class ClearOperand : uint8_t{ corecomponentstate }; +enum class SyncOperand : uint8_t{ + asset +}; + #define PAYLOAD_NO_STATUS 0 #define PAYLOAD_SUCCESS 1 #define PAYLOAD_FAILURE 2 diff --git a/libminifi/include/core/state/MetricsPublisherStore.h b/libminifi/include/core/state/MetricsPublisherStore.h index 544e824455..86a25a312e 100644 --- a/libminifi/include/core/state/MetricsPublisherStore.h +++ b/libminifi/include/core/state/MetricsPublisherStore.h @@ -27,13 +27,14 @@ #include "core/state/nodes/ResponseNodeLoader.h" #include "utils/gsl.h" #include "core/ProcessGroup.h" +#include "utils/file/AssetManager.h" namespace org::apache::nifi::minifi::state { class MetricsPublisherStore { public: MetricsPublisherStore(std::shared_ptr configuration, const std::vector>& repository_metric_sources, - std::shared_ptr flow_configuration); + std::shared_ptr flow_configuration, std::shared_ptr asset_manager); void initialize(core::controller::ControllerServiceProvider* controller, state::StateMonitor* update_sink); void loadMetricNodes(core::ProcessGroup* root); void clearMetricNodes(); diff --git a/libminifi/include/core/state/nodes/AssetInformation.h b/libminifi/include/core/state/nodes/AssetInformation.h new file mode 100644 index 0000000000..6f12d6482b --- /dev/null +++ b/libminifi/include/core/state/nodes/AssetInformation.h @@ -0,0 +1,40 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include "core/state/nodes/MetricsBase.h" +#include "utils/file/AssetManager.h" + +namespace org::apache::nifi::minifi::state::response { + +class AssetInformation : public ResponseNode { + public: + AssetInformation() = default; + explicit AssetInformation(std::string name, const utils::Identifier& uuid = {}) : ResponseNode(std::move(name), uuid) {} + + MINIFIAPI static constexpr const char* Description = "Metric node that defines hash for all asset identifiers"; + + void setAssetManager(std::shared_ptr asset_manager); + + std::string getName() const override { return "assetInfo"; } + std::vector serialize() override; + + private: + std::shared_ptr asset_manager_; +}; + +} // namespace org::apache::nifi::minifi::state::response diff --git a/libminifi/include/core/state/nodes/ResponseNodeLoader.h b/libminifi/include/core/state/nodes/ResponseNodeLoader.h index af330585c4..1a6a908b59 100644 --- a/libminifi/include/core/state/nodes/ResponseNodeLoader.h +++ b/libminifi/include/core/state/nodes/ResponseNodeLoader.h @@ -34,13 +34,14 @@ #include "utils/Id.h" #include "utils/expected.h" #include "core/RepositoryMetricsSource.h" +#include "utils/file/AssetManager.h" namespace org::apache::nifi::minifi::state::response { class ResponseNodeLoader { public: ResponseNodeLoader(std::shared_ptr configuration, std::vector> repository_metric_sources, - std::shared_ptr flow_configuration); + std::shared_ptr flow_configuration, std::shared_ptr asset_manager); void setNewConfigRoot(core::ProcessGroup* root); void clearConfigRoot(); @@ -62,6 +63,7 @@ class ResponseNodeLoader { void initializeAgentStatus(const SharedResponseNode& response_node) const; void initializeConfigurationChecksums(const SharedResponseNode& response_node) const; void initializeFlowMonitor(const SharedResponseNode& response_node) const; + void initializeAssetInformation(const SharedResponseNode& response_node) const; std::vector getMatchingComponentMetricsNodes(const std::string& regex_str) const; mutable std::mutex root_mutex_; @@ -73,6 +75,7 @@ class ResponseNodeLoader { std::shared_ptr configuration_; std::vector> repository_metric_sources_; std::shared_ptr flow_configuration_; + std::shared_ptr asset_manager_; core::controller::ControllerServiceProvider* controller_{}; state::StateMonitor* update_sink_{}; std::shared_ptr logger_{core::logging::LoggerFactory::getLogger()}; diff --git a/libminifi/include/utils/file/AssetManager.h b/libminifi/include/utils/file/AssetManager.h new file mode 100644 index 0000000000..c989d33c4f --- /dev/null +++ b/libminifi/include/utils/file/AssetManager.h @@ -0,0 +1,66 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include "core/logging/Logger.h" +#include "utils/expected.h" +#include "properties/Configure.h" + +namespace org::apache::nifi::minifi::utils::file { + +struct AssetDescription { + std::string id; + std::filesystem::path path; + std::string url; + + bool operator<(const AssetDescription& other) const { + return id < other.id; + } +}; + +using AssetLayout = std::set; + +class AssetManager { + public: + explicit AssetManager(std::shared_ptr configuration); + + nonstd::expected sync(AssetLayout layout, std::function, std::string>(std::string_view /*url*/)> fetch); + + std::string hash() const; + + std::filesystem::path getRoot() const; + + private: + void refreshState(); + + void persist() const; + + mutable std::recursive_mutex mtx_; + std::filesystem::path root_; + AssetLayout state_; + mutable std::optional cached_hash_; + std::shared_ptr logger_; +}; + +} // namespace org::apache::nifi::minifi::utils::file diff --git a/libminifi/include/utils/file/PathUtils.h b/libminifi/include/utils/file/PathUtils.h index 1df2e9d891..f5cbb44d2a 100644 --- a/libminifi/include/utils/file/PathUtils.h +++ b/libminifi/include/utils/file/PathUtils.h @@ -25,6 +25,7 @@ #include #include #include +#include "utils/expected.h" namespace org::apache::nifi::minifi::utils::file { @@ -42,6 +43,27 @@ inline std::optional canonicalize(const std::filesystem:: return result; } +inline nonstd::expected validateRelativePath(const std::filesystem::path& path) { + if (path.empty()) { + return nonstd::make_unexpected("Empty file path"); + } + if (!path.is_relative()) { + return nonstd::make_unexpected("File path must be a relative path '" + path.string() + "'"); + } + if (!path.has_filename()) { + return nonstd::make_unexpected("Filename missing in output path '" + path.string() + "'"); + } + if (path.filename() == "." || path.filename() == "..") { + return nonstd::make_unexpected("Invalid filename '" + path.filename().string() + "'"); + } + for (const auto& segment : path) { + if (segment == "..") { + return nonstd::make_unexpected("Accessing parent directory is forbidden in file path '" + path.string() + "'"); + } + } + return {}; +} + /** * Represents filesystem space information in bytes diff --git a/libminifi/src/FlowController.cpp b/libminifi/src/FlowController.cpp index 8e5739d9df..fbd58eb34b 100644 --- a/libminifi/src/FlowController.cpp +++ b/libminifi/src/FlowController.cpp @@ -50,7 +50,8 @@ namespace org::apache::nifi::minifi { FlowController::FlowController(std::shared_ptr provenance_repo, std::shared_ptr flow_file_repo, std::shared_ptr configure, std::shared_ptr flow_configuration, std::shared_ptr content_repo, std::unique_ptr metrics_publisher_store, - std::shared_ptr filesystem, std::function request_restart) + std::shared_ptr filesystem, std::function request_restart, + std::shared_ptr asset_manager) : core::controller::ForwardingControllerServiceProvider(core::className()), running_(false), initialized_(false), @@ -82,7 +83,7 @@ FlowController::FlowController(std::shared_ptr provenance_repo if (auto publisher = metrics_publisher_store_->getMetricsPublisher(c2::C2_METRICS_PUBLISHER).lock()) { c2_metrics_publisher = std::dynamic_pointer_cast(publisher); } - c2_agent_ = std::make_unique(configuration_, c2_metrics_publisher, std::move(filesystem), std::move(request_restart)); + c2_agent_ = std::make_unique(configuration_, c2_metrics_publisher, std::move(filesystem), std::move(request_restart), std::move(asset_manager)); } if (c2::isControllerSocketEnabled(configuration_)) { diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index 5f1097fb43..596a9112fe 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -38,6 +38,7 @@ #include "utils/file/FileManager.h" #include "utils/file/FileSystem.h" #include "http/BaseHTTPClient.h" +#include "utils/file/PathUtils.h" #include "utils/Environment.h" #include "utils/Monitors.h" #include "utils/StringUtils.h" @@ -54,7 +55,8 @@ namespace org::apache::nifi::minifi::c2 { C2Agent::C2Agent(std::shared_ptr configuration, std::weak_ptr node_reporter, std::shared_ptr filesystem, - std::function request_restart) + std::function request_restart, + std::shared_ptr asset_manager) : heart_beat_period_(3s), max_c2_responses(5), configuration_(std::move(configuration)), @@ -62,7 +64,8 @@ C2Agent::C2Agent(std::shared_ptr configuration, filesystem_(std::move(filesystem)), thread_pool_(2, nullptr, "C2 threadpool"), request_restart_(std::move(request_restart)), - last_run_(std::chrono::steady_clock::now()) { + last_run_(std::chrono::steady_clock::now()), + asset_manager_(std::move(asset_manager)) { if (!configuration_->getAgentClass()) { logger_->log_info("Agent class is not predefined"); } @@ -381,6 +384,9 @@ void C2Agent::handle_c2_server_response(const C2ContentResponse &resp) { } break; } + case Operation::sync: + handle_sync(resp); + break; default: break; // do nothing @@ -700,6 +706,94 @@ void C2Agent::handle_transfer(const C2ContentResponse &resp) { } } +void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse &resp) { + auto send_error = [&] (std::string_view error) { + logger_->log_error("{}", error); + C2Payload response(Operation::acknowledge, state::UpdateState::SET_ERROR, resp.ident, true); + response.setRawData(as_bytes(std::span(error.begin(), error.end()))); + enqueue_c2_response(std::move(response)); + }; + + if (!asset_manager_) { + send_error("Internal error: no asset manager"); + return; + } + + SyncOperand operand = SyncOperand::asset; + try { + operand = utils::enumCast(resp.name, true); + } catch(const std::runtime_error&) { + send_error("Unknown operand '" + resp.name + "'"); + return; + } + + gsl_Assert(operand == SyncOperand::asset); + + // we are expecting the format + // args: { + // ".path": "a/b/c.txt", + // ".url": "example.com", + // ".path": "a/b/c.txt", + // ".url": "example.com" + // } + std::set ids; + utils::file::AssetLayout asset_layout; + for (auto& arg: resp.operation_arguments) { + auto fragments = utils::string::split(arg.first, "."); + if (fragments.size() == 2) { + ids.insert(fragments.front()); + } else { + send_error("Malformed asset sync command"); + return; + } + } + for (auto& id : ids) { + auto path_it = resp.operation_arguments.find(id + ".path"); + if (path_it == resp.operation_arguments.end()) { + send_error("Malformed path for id '" + id + "'"); + return; + } + auto result = utils::file::validateRelativePath(path_it->second.to_string()); + if (!result) { + send_error(result.error()); + return; + } + auto url_it = resp.operation_arguments.find(id + ".url"); + if (url_it == resp.operation_arguments.end()) { + send_error("Malformed url for id '" + id + "'"); + return; + } + asset_layout.insert(utils::file::AssetDescription{ + .id = id, + .path = path_it->second.to_string(), + .url = url_it->second.to_string() + }); + } + + auto fetch = [&] (std::string_view url) -> nonstd::expected, std::string> { + auto resolved_url = resolveUrl(std::string{url}); + if (!resolved_url) { + return nonstd::make_unexpected("Couldn't resolve url"); + } + C2Payload file_response = protocol_.load()->fetch(resolved_url.value()); + + if (file_response.getStatus().getState() != state::UpdateState::READ_COMPLETE) { + return nonstd::make_unexpected("Failed to fetch file from " + resolved_url.value()); + } + + return std::move(file_response).moveRawData(); + }; + + auto result = asset_manager_->sync(asset_layout, fetch); + if (!result) { + send_error(result.error()); + return; + } + + C2Payload response(Operation::acknowledge, state::UpdateState::FULLY_APPLIED, resp.ident, true); + enqueue_c2_response(std::move(response)); +} + utils::TaskRescheduleInfo C2Agent::produce() { // place priority on messages to send to the c2 server if (protocol_ != nullptr) { @@ -891,27 +985,6 @@ static auto make_path(const std::string& str) { return std::filesystem::path(str); } -static std::optional validateFilePath(const std::filesystem::path& path) { - if (path.empty()) { - return "Empty file path"; - } - if (!path.is_relative()) { - return "File path must be a relative path '" + path.string() + "'"; - } - if (!path.has_filename()) { - return "Filename missing in output path '" + path.string() + "'"; - } - if (path.filename() == "." || path.filename() == "..") { - return "Invalid filename '" + path.filename().string() + "'"; - } - for (const auto& segment : path) { - if (segment == "..") { - return "Accessing parent directory is forbidden in file path '" + path.string() + "'"; - } - } - return std::nullopt; -} - void C2Agent::handleAssetUpdate(const C2ContentResponse& resp) { auto send_error = [&] (std::string_view error) { logger_->log_error("{}", error); @@ -919,19 +992,16 @@ void C2Agent::handleAssetUpdate(const C2ContentResponse& resp) { response.setRawData(as_bytes(std::span(error.begin(), error.end()))); enqueue_c2_response(std::move(response)); }; - std::filesystem::path asset_dir = configuration_->getHome() / "asset"; - if (auto asset_dir_str = configuration_->get(Configuration::nifi_asset_directory)) { - asset_dir = asset_dir_str.value(); - } // output file std::filesystem::path file_path; if (auto file_rel = resp.getArgument("file") | utils::transform(make_path)) { - if (auto error = validateFilePath(file_rel.value())) { - send_error(error.value()); + auto result = utils::file::validateRelativePath(file_rel.value()); + if (!result) { + send_error(result.error()); return; } - file_path = asset_dir / file_rel.value(); + file_path = asset_manager_->getRoot() / file_rel.value(); } else { send_error("Couldn't find 'file' argument"); return; diff --git a/libminifi/src/core/state/MetricsPublisherStore.cpp b/libminifi/src/core/state/MetricsPublisherStore.cpp index 269cb6267a..d670c12bc1 100644 --- a/libminifi/src/core/state/MetricsPublisherStore.cpp +++ b/libminifi/src/core/state/MetricsPublisherStore.cpp @@ -23,9 +23,9 @@ namespace org::apache::nifi::minifi::state { MetricsPublisherStore::MetricsPublisherStore(std::shared_ptr configuration, const std::vector>& repository_metric_sources, - std::shared_ptr flow_configuration) + std::shared_ptr flow_configuration, std::shared_ptr asset_manager) : configuration_(configuration), - response_node_loader_(std::make_shared(std::move(configuration), repository_metric_sources, std::move(flow_configuration))) { + response_node_loader_(std::make_shared(std::move(configuration), repository_metric_sources, std::move(flow_configuration), std::move(asset_manager))) { } void MetricsPublisherStore::initialize(core::controller::ControllerServiceProvider* controller, state::StateMonitor* update_sink) { diff --git a/libminifi/src/core/state/nodes/AssetInformation.cpp b/libminifi/src/core/state/nodes/AssetInformation.cpp new file mode 100644 index 0000000000..0710fd1e7e --- /dev/null +++ b/libminifi/src/core/state/nodes/AssetInformation.cpp @@ -0,0 +1,37 @@ +/** + * 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 "core/state/nodes/AssetInformation.h" +#include "core/Resource.h" + +namespace org::apache::nifi::minifi::state::response { + +void AssetInformation::setAssetManager(std::shared_ptr asset_manager) { + asset_manager_ = asset_manager; +} + +std::vector AssetInformation::serialize() { + SerializedResponseNode node; + node.name = "hash"; + node.value = asset_manager_->hash(); + + return std::vector{node}; +} + +REGISTER_RESOURCE(AssetInformation, DescriptionOnly); + +} // namespace org::apache::nifi::minifi::state::response \ No newline at end of file diff --git a/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp b/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp index 58ff6eceaa..061a96be89 100644 --- a/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp +++ b/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp @@ -25,6 +25,7 @@ #include "core/state/nodes/QueueMetrics.h" #include "core/state/nodes/AgentInformation.h" #include "core/state/nodes/ConfigurationChecksums.h" +#include "core/state/nodes/AssetInformation.h" #include "utils/gsl.h" #include "utils/RegexUtils.h" #include "utils/StringUtils.h" @@ -33,10 +34,11 @@ namespace org::apache::nifi::minifi::state::response { ResponseNodeLoader::ResponseNodeLoader(std::shared_ptr configuration, std::vector> repository_metric_sources, - std::shared_ptr flow_configuration) + std::shared_ptr flow_configuration, std::shared_ptr asset_manager) : configuration_(std::move(configuration)), repository_metric_sources_(std::move(repository_metric_sources)), - flow_configuration_(std::move(flow_configuration)) { + flow_configuration_(std::move(flow_configuration)), + asset_manager_(std::move(asset_manager)) { } void ResponseNodeLoader::clearConfigRoot() { @@ -194,6 +196,13 @@ void ResponseNodeLoader::initializeConfigurationChecksums(const SharedResponseNo } } +void ResponseNodeLoader::initializeAssetInformation(const SharedResponseNode& response_node) const { + auto asset_info = dynamic_cast(response_node.get()); + if (asset_info) { + asset_info->setAssetManager(asset_manager_); + } +} + void ResponseNodeLoader::initializeFlowMonitor(const SharedResponseNode& response_node) const { auto flowMonitor = dynamic_cast(response_node.get()); if (flowMonitor == nullptr) { @@ -231,6 +240,7 @@ std::vector ResponseNodeLoader::loadResponseNodes(const std: initializeAgentStatus(response_node); initializeConfigurationChecksums(response_node); initializeFlowMonitor(response_node); + initializeAssetInformation(response_node); } return response_nodes; } diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp new file mode 100644 index 0000000000..ec46d31961 --- /dev/null +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -0,0 +1,151 @@ +/** + * 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 "utils/file/AssetManager.h" +#include "utils/file/FileUtils.h" +#include "rapidjson/document.h" +#include "rapidjson/writer.h" +#include "core/logging/LoggerFactory.h" +#include "utils/Hash.h" + +namespace org::apache::nifi::minifi::utils::file { + +AssetManager::AssetManager(std::shared_ptr configuration) + : root_(configuration->get(Configure::nifi_asset_directory).value_or(configuration->getHome() / "asset")), + logger_(core::logging::LoggerFactory::getLogger()) { + refreshState(); +} + +void AssetManager::refreshState() { + std::lock_guard lock(mtx_); + state_.clear(); + if (!utils::file::FileUtils::exists(root_)) { + std::filesystem::create_directory(root_); + } + if (!utils::file::FileUtils::exists(root_ / ".state")) { + std::ofstream{root_ / ".state", std::ios::binary} << "{}"; + } + rapidjson::Document doc; + + std::string file_content = utils::file::get_content(root_ / ".state"); + + rapidjson::ParseResult res = doc.Parse(file_content.c_str(), file_content.size()); + if (!res) { + logger_->log_error("Failed to parse asset '.state' file, not a valid json file"); + return; + } + if (!doc.IsObject()) { + logger_->log_error("Asset '.state' file is malformed"); + return; + } + AssetLayout new_state; + for (auto& [id, entry] : doc.GetObject()) { + if (!entry.IsObject()) { + logger_->log_error("Asset '.state' file is malformed"); + return; + } + AssetDescription description; + description.id = std::string{id.GetString(), id.GetStringLength()}; + if (!entry.HasMember("path") || !entry["path"].IsString()) { + logger_->log_error("Asset '.state' file is malformed"); + return; + } + description.path = std::string{entry["path"].GetString(), entry["path"].GetStringLength()}; + if (!entry.HasMember("url") || !entry["url"].IsString()) { + logger_->log_error("Asset '.state' file is malformed"); + return; + } + description.url = std::string{entry["url"].GetString(), entry["url"].GetStringLength()}; + + if (utils::file::FileUtils::exists(root_ / description.id)) { + new_state.insert(std::move(description)); + } else { + logger_->log_error("Asset '.state' file contains entry that does not exist on the filesystem"); + } + + } + state_ = std::move(new_state); +} + +std::string AssetManager::hash() const { + std::lock_guard lock(mtx_); + if (!cached_hash_) { + size_t hash_value{0}; + for (auto& entry : state_) { + hash_value = utils::hash_combine(hash_value, std::hash{}(entry.id)); + } + cached_hash_ = std::to_string(hash_value); + } + return cached_hash_.value(); +} + +nonstd::expected AssetManager::sync(org::apache::nifi::minifi::utils::file::AssetLayout layout, std::function, std::string>(std::string_view /*url*/)> fetch) { + std::lock_guard lock(mtx_); + std::vector>> new_file_contents; + for (auto& new_entry : layout) { + if (std::find_if(state_.begin(), state_.end(), [&] (auto& entry) {return entry.id == new_entry.id;}) == state_.end()) { + if (auto data = fetch(new_entry.url)) { + new_file_contents.emplace_back(new_entry.path, data.value()); + } else { + return nonstd::make_unexpected(data.error()); + } + } + } + cached_hash_.reset(); + + for (auto& old_entry : state_) { + if (std::find_if(layout.begin(), layout.end(), [&] (auto& entry) {return entry.id == old_entry.id;}) == layout.end()) { + // we no longer need this asset + std::filesystem::remove(root_ / old_entry.path); + } + } + + for (auto& [path, content] : new_file_contents) { + utils::file::create_dir((root_ / path).parent_path()); + std::ofstream{root_ / path, std::ios::binary}.write(reinterpret_cast(content.data()), content.size()); + } + + state_ = std::move(layout); + persist(); + return {}; +} + +void AssetManager::persist() const { + std::lock_guard lock(mtx_); + rapidjson::Document doc; + doc.SetObject(); + + for (auto& entry : state_) { + rapidjson::Value entry_val(rapidjson::kObjectType); + entry_val.AddMember(rapidjson::StringRef("path"), rapidjson::Value(entry.path.string().c_str(), doc.GetAllocator()), doc.GetAllocator()); + entry_val.AddMember(rapidjson::StringRef("url"), rapidjson::StringRef(entry.url), doc.GetAllocator()); + doc.AddMember(rapidjson::StringRef(entry.id), entry_val, doc.GetAllocator()); + } + + rapidjson::StringBuffer buffer; + rapidjson::Writer writer(buffer); + doc.Accept(writer); + + std::ofstream{root_ / ".state", std::ios::binary}.write(buffer.GetString(), buffer.GetSize()); +} + +std::filesystem::path AssetManager::getRoot() const { + std::lock_guard lock(mtx_); + return root_; +} + +} // namespace org::apache::nifi::minifi::utils::file diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp new file mode 100644 index 0000000000..324ffac03c --- /dev/null +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -0,0 +1,260 @@ +/** + * + * 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. + */ + +#undef NDEBUG +#include +#include +#include +#include + +#include "HTTPIntegrationBase.h" +#include "HTTPHandlers.h" +#include "utils/IntegrationTestUtils.h" +#include "utils/Environment.h" +#include "utils/file/FileUtils.h" +#include "utils/file/AssetManager.h" + +class FileProvider : public ServerAwareHandler { + public: + explicit FileProvider(std::string file_content): file_content_(std::move(file_content)) {} + + bool handleGet(CivetServer* /*server*/, struct mg_connection* conn) override { + mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: " + "text/plain\r\nContent-Length: %lu\r\nConnection: close\r\n\r\n", + file_content_.length()); + mg_printf(conn, "%s", file_content_.c_str()); + return true; + } + + private: + std::string file_content_; +}; + +class C2HeartbeatHandler : public HeartbeatHandler { + public: + using HeartbeatHandler::HeartbeatHandler; + using AssetDescription = org::apache::nifi::minifi::utils::file::AssetDescription; + + void handleHeartbeat(const rapidjson::Document& root, struct mg_connection* conn) override { + std::string hb_str; + { + rapidjson::StringBuffer buffer; + rapidjson::Writer writer(buffer); + root.Accept(writer); + + hb_str = std::string{buffer.GetString(), buffer.GetSize()}; + } + auto& asset_info_node = root["assetInfo"]; + if (!asset_info_node.IsObject()) { + int x = 0; + } + auto& asset_hash_node = asset_info_node["hash"]; + std::string asset_hash{asset_hash_node.GetString(), asset_hash_node.GetStringLength()}; + + std::vector operations; + { + std::lock_guard guard(asset_mtx_); + agent_asset_hash_ = asset_hash; + if (asset_hash != assetHash()) { + std::unordered_map args; + for (auto& asset : expected_assets_) { + args[asset.id + ".path"] = asset.path; + args[asset.id + ".url"] = asset.url; + } + operations.push_back(C2Operation{ + .operation = "sync", + .operand = "asset", + .operation_id = std::to_string(next_op_id_++), + .args = std::move(args) + }); + } + } + sendHeartbeatResponse(operations, conn); + } + + void addAsset(std::string id, std::string path, std::string url) { + std::lock_guard guard(asset_mtx_); + expected_assets_.insert(AssetDescription{ + .id = id, + .path = path, + .url = url + }); + } + + void removeAsset(std::string id) { + std::lock_guard guard{asset_mtx_}; + expected_assets_.erase(AssetDescription{.id = id}); + } + + std::optional getAgentAssetHash() const { + std::lock_guard lock(asset_mtx_); + return agent_asset_hash_; + } + + std::string assetHash() const { + std::lock_guard guard{asset_mtx_}; + size_t hash_value{0}; + for (auto& asset : expected_assets_) { + hash_value = utils::hash_combine(hash_value, std::hash{}(asset.id)); + } + return std::to_string(hash_value); + } + + std::string assetState() const { + std::lock_guard guard{asset_mtx_}; + rapidjson::Document doc; + doc.SetObject(); + for (auto& asset : expected_assets_) { + auto path_str = asset.path.string(); + doc.AddMember(rapidjson::StringRef(asset.id), rapidjson::kObjectType, doc.GetAllocator()); + doc[asset.id].AddMember(rapidjson::StringRef("path"), rapidjson::Value(path_str, doc.GetAllocator()), doc.GetAllocator()); + doc[asset.id].AddMember(rapidjson::StringRef("url"), rapidjson::StringRef(asset.url), doc.GetAllocator()); + } + rapidjson::StringBuffer buffer; + rapidjson::Writer writer(buffer); + doc.Accept(writer); + + return {buffer.GetString(), buffer.GetSize()}; + } + + private: + mutable std::recursive_mutex asset_mtx_; + std::set expected_assets_; + + std::optional agent_asset_hash_; + + std::atomic next_op_id_{1}; +}; + +class VerifyC2AssetSync : public VerifyC2Base { + public: + void configureC2() override { + configuration->set("nifi.c2.agent.protocol.class", "RESTSender"); + configuration->set("nifi.c2.enable", "true"); + configuration->set("nifi.c2.agent.heartbeat.period", "100"); + configuration->set("nifi.c2.root.classes", "DeviceInfoNode,AgentInformation,FlowInformation,AssetInformation"); + } + + void runAssertions() override { + verify_(); + } + + void setVerifier(std::function verify) { + verify_ = std::move(verify); + } + + private: + std::function verify_; +}; + +int main() { + TestController controller; + + // setup minifi home + const std::filesystem::path home_dir = controller.createTempDirectory(); + const auto asset_dir = home_dir / "asset"; + std::filesystem::current_path(home_dir); + + C2AcknowledgeHandler ack_handler; + std::string file_A = "hello from file A"; + FileProvider file_A_provider{file_A}; + std::string file_B = "hello from file B"; + FileProvider file_B_provider{file_B}; + std::string file_C = "hello from file C"; + FileProvider file_C_provider{file_C}; + std::string file_A_v2 = "hello from file A version 2"; + FileProvider file_Av2_provider{file_A_v2}; + C2HeartbeatHandler hb_handler{std::make_shared()}; + + VerifyC2AssetSync harness; + harness.setUrl("http://localhost:0/api/file/A.txt", &file_A_provider); + harness.setUrl("http://localhost:0/api/file/Av2.txt", &file_Av2_provider); + harness.setUrl("http://localhost:0/api/file/B.txt", &file_B_provider); + harness.setUrl("http://localhost:0/api/file/C.txt", &file_C_provider); + + std::string absolute_file_A_url = "http://localhost:" + harness.getWebPort() + "/api/file/A.txt"; + + hb_handler.addAsset("Av1", "A.txt", "/api/file/A.txt"); + hb_handler.addAsset("Bv1", "nested/dir/B.txt", "/api/file/B.txt"); + hb_handler.addAsset("Cv1", "nested/C.txt", "/api/file/C.txt"); + + harness.setUrl("http://localhost:0/api/heartbeat", &hb_handler); + harness.setUrl("http://localhost:0/api/acknowledge", &ack_handler); + harness.setC2Url("/api/heartbeat", "/api/acknowledge"); + + auto get_asset_structure = [&] () { + std::unordered_map contents; + for (auto file : utils::file::list_dir_all(asset_dir.string(), controller.getLogger())) { + contents[(file.first / file.second).string()] = utils::file::get_content(file.first / file.second); + } + return contents; + }; + + harness.setVerifier([&] () { + assert(utils::verifyEventHappenedInPollTime(10s, [&] {return hb_handler.assetHash() == hb_handler.getAgentAssetHash();})); + + { + std::unordered_map expected_assets{ + {(asset_dir / "A.txt").string(), file_A}, + {(asset_dir / "nested/dir/B.txt").string(), file_B}, + {(asset_dir / "nested/C.txt").string(), file_C}, + {(asset_dir / ".state").string(), hb_handler.assetState()} + }; + auto actual_assets = get_asset_structure(); + if (actual_assets != expected_assets) { + controller.getLogger()->log_error("Mismatch between expected and actual assets"); + for (auto& [path, content] : expected_assets) { + controller.getLogger()->log_error("Expected asset at {}: {}", path, content); + } + for (auto& [path, content] : actual_assets) { + controller.getLogger()->log_error("Actual asset at {}: {}", path, content); + } + assert(false); + } + } + + hb_handler.removeAsset("Av1"); + hb_handler.removeAsset("Cv1"); + hb_handler.addAsset("Av2", "A.txt", "/api/file/Av2.txt"); + + + assert(utils::verifyEventHappenedInPollTime(10s, [&] {return hb_handler.assetHash() == hb_handler.getAgentAssetHash();})); + + { + std::unordered_map expected_assets{ + {(asset_dir / "A.txt").string(), file_A_v2}, + {(asset_dir / "nested/dir/B.txt").string(), file_B}, + {(asset_dir / ".state").string(), hb_handler.assetState()} + }; + + auto actual_assets = get_asset_structure(); + if (actual_assets != expected_assets) { + controller.getLogger()->log_error("Mismatch between expected and actual assets"); + for (auto& [path, content] : expected_assets) { + controller.getLogger()->log_error("Expected asset at {}: {}", path, content); + } + for (auto& [path, content] : actual_assets) { + controller.getLogger()->log_error("Actual asset at {}: {}", path, content); + } + assert(false); + } + } + }); + + harness.run(); +} diff --git a/minifi_main/MiNiFiMain.cpp b/minifi_main/MiNiFiMain.cpp index 782b06c807..a102d63865 100644 --- a/minifi_main/MiNiFiMain.cpp +++ b/minifi_main/MiNiFiMain.cpp @@ -57,6 +57,7 @@ #include "properties/Decryptor.h" #include "utils/file/PathUtils.h" #include "utils/file/FileUtils.h" +#include "utils/file/AssetManager.h" #include "utils/Environment.h" #include "utils/FileMutex.h" #include "FlowController.h" @@ -397,9 +398,10 @@ int main(int argc, char **argv) { .sensitive_values_encryptor = utils::crypto::EncryptionProvider::createSensitivePropertiesEncryptor(minifiHome) }, nifi_configuration_class_name); - std::vector> repo_metric_sources{prov_repo, flow_repo, content_repo}; - auto metrics_publisher_store = std::make_unique(configure, repo_metric_sources, flow_configuration); + auto asset_manager = std::make_shared(configure); + std::vector> repo_metric_sources{prov_repo, flow_repo, content_repo}; + auto metrics_publisher_store = std::make_unique(configure, repo_metric_sources, flow_configuration, asset_manager); const auto controller = std::make_unique( prov_repo, flow_repo, configure, std::move(flow_configuration), content_repo, std::move(metrics_publisher_store), filesystem, request_restart); From c1c4c39de3b8ea11dd67d6d69b3e6344e5a886aa Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 3 Apr 2024 10:59:46 +0200 Subject: [PATCH 02/48] MINIFICPP-2314 - Build fix --- .../test/unit/ControllerSocketMetricsPublisherTest.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libminifi/test/unit/ControllerSocketMetricsPublisherTest.cpp b/libminifi/test/unit/ControllerSocketMetricsPublisherTest.cpp index 702583bbc3..4106441e0f 100644 --- a/libminifi/test/unit/ControllerSocketMetricsPublisherTest.cpp +++ b/libminifi/test/unit/ControllerSocketMetricsPublisherTest.cpp @@ -63,7 +63,11 @@ class ControllerSocketMetricsPublisherTestFixture { public: ControllerSocketMetricsPublisherTestFixture() : configuration_(std::make_shared()), - response_node_loader_(std::make_shared(configuration_, std::vector>{}, nullptr)), + response_node_loader_(std::make_shared( + configuration_, + std::vector>{}, + nullptr, + std::make_shared(configuration_))), test_response_node_(std::make_shared()), controller_socket_metrics_publisher_("test_publisher") { controller_socket_metrics_publisher_.initialize(configuration_, response_node_loader_); From a2c7ab9c3f1e1de3beb445a6274662d2a1911768 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 3 Apr 2024 12:40:45 +0200 Subject: [PATCH 03/48] MINIFICPP-2314 - Build fix --- libminifi/test/unit/LogMetricsPublisherTests.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libminifi/test/unit/LogMetricsPublisherTests.cpp b/libminifi/test/unit/LogMetricsPublisherTests.cpp index 511113d378..7292f0f07a 100644 --- a/libminifi/test/unit/LogMetricsPublisherTests.cpp +++ b/libminifi/test/unit/LogMetricsPublisherTests.cpp @@ -36,8 +36,11 @@ class LogPublisherTestFixture { : configuration_(std::make_shared()), provenance_repo_(core::createRepository("provenancerepository", "provenancerepository")), flow_file_repo_(core::createRepository("flowfilerepository", "flowfilerepository")), - response_node_loader_(std::make_shared(configuration_, - std::vector>{provenance_repo_, flow_file_repo_}, nullptr)), + response_node_loader_(std::make_shared( + configuration_, + std::vector>{provenance_repo_, flow_file_repo_}, + nullptr, + std::make_shared(configuration_))), publisher_("LogMetricsPublisher") { configuration_->setHome(temp_directory_.getPath()); provenance_repo_->initialize(configuration_); From 0c4a15757305fb0caf6fb2bfdf9b4aefae26cf4e Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 3 Apr 2024 13:03:18 +0200 Subject: [PATCH 04/48] MINIFICPP-2314 - Build fix --- libminifi/include/core/state/MetricsPublisherStore.h | 2 +- libminifi/include/core/state/nodes/AssetInformation.h | 4 +++- .../include/core/state/nodes/ResponseNodeLoader.h | 2 +- libminifi/src/core/state/nodes/AssetInformation.cpp | 10 ++++++++++ .../test/unit/ControllerSocketMetricsPublisherTest.cpp | 6 +----- libminifi/test/unit/LogMetricsPublisherTests.cpp | 6 +----- 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/libminifi/include/core/state/MetricsPublisherStore.h b/libminifi/include/core/state/MetricsPublisherStore.h index 86a25a312e..4d827bc00a 100644 --- a/libminifi/include/core/state/MetricsPublisherStore.h +++ b/libminifi/include/core/state/MetricsPublisherStore.h @@ -34,7 +34,7 @@ namespace org::apache::nifi::minifi::state { class MetricsPublisherStore { public: MetricsPublisherStore(std::shared_ptr configuration, const std::vector>& repository_metric_sources, - std::shared_ptr flow_configuration, std::shared_ptr asset_manager); + std::shared_ptr flow_configuration, std::shared_ptr asset_manager = nullptr); void initialize(core::controller::ControllerServiceProvider* controller, state::StateMonitor* update_sink); void loadMetricNodes(core::ProcessGroup* root); void clearMetricNodes(); diff --git a/libminifi/include/core/state/nodes/AssetInformation.h b/libminifi/include/core/state/nodes/AssetInformation.h index 6f12d6482b..2fd048b753 100644 --- a/libminifi/include/core/state/nodes/AssetInformation.h +++ b/libminifi/include/core/state/nodes/AssetInformation.h @@ -18,12 +18,13 @@ #include "core/state/nodes/MetricsBase.h" #include "utils/file/AssetManager.h" +#include "core/logging/Logger.h" namespace org::apache::nifi::minifi::state::response { class AssetInformation : public ResponseNode { public: - AssetInformation() = default; + AssetInformation(); explicit AssetInformation(std::string name, const utils::Identifier& uuid = {}) : ResponseNode(std::move(name), uuid) {} MINIFIAPI static constexpr const char* Description = "Metric node that defines hash for all asset identifiers"; @@ -35,6 +36,7 @@ class AssetInformation : public ResponseNode { private: std::shared_ptr asset_manager_; + std::shared_ptr logger_; }; } // namespace org::apache::nifi::minifi::state::response diff --git a/libminifi/include/core/state/nodes/ResponseNodeLoader.h b/libminifi/include/core/state/nodes/ResponseNodeLoader.h index 1a6a908b59..e59aae372d 100644 --- a/libminifi/include/core/state/nodes/ResponseNodeLoader.h +++ b/libminifi/include/core/state/nodes/ResponseNodeLoader.h @@ -41,7 +41,7 @@ namespace org::apache::nifi::minifi::state::response { class ResponseNodeLoader { public: ResponseNodeLoader(std::shared_ptr configuration, std::vector> repository_metric_sources, - std::shared_ptr flow_configuration, std::shared_ptr asset_manager); + std::shared_ptr flow_configuration, std::shared_ptr asset_manager = nullptr); void setNewConfigRoot(core::ProcessGroup* root); void clearConfigRoot(); diff --git a/libminifi/src/core/state/nodes/AssetInformation.cpp b/libminifi/src/core/state/nodes/AssetInformation.cpp index 0710fd1e7e..7b585a735c 100644 --- a/libminifi/src/core/state/nodes/AssetInformation.cpp +++ b/libminifi/src/core/state/nodes/AssetInformation.cpp @@ -17,14 +17,24 @@ #include "core/state/nodes/AssetInformation.h" #include "core/Resource.h" +#include "core/logging/LoggerFactory.h" namespace org::apache::nifi::minifi::state::response { +AssetInformation::AssetInformation() + : logger_(core::logging::LoggerFactory().getLogger()) {} + void AssetInformation::setAssetManager(std::shared_ptr asset_manager) { asset_manager_ = asset_manager; + if (!asset_manager_) { + logger_->log_error("No asset manager is provided, asset information will not be available"); + } } std::vector AssetInformation::serialize() { + if (!asset_manager_) { + return {}; + } SerializedResponseNode node; node.name = "hash"; node.value = asset_manager_->hash(); diff --git a/libminifi/test/unit/ControllerSocketMetricsPublisherTest.cpp b/libminifi/test/unit/ControllerSocketMetricsPublisherTest.cpp index 4106441e0f..702583bbc3 100644 --- a/libminifi/test/unit/ControllerSocketMetricsPublisherTest.cpp +++ b/libminifi/test/unit/ControllerSocketMetricsPublisherTest.cpp @@ -63,11 +63,7 @@ class ControllerSocketMetricsPublisherTestFixture { public: ControllerSocketMetricsPublisherTestFixture() : configuration_(std::make_shared()), - response_node_loader_(std::make_shared( - configuration_, - std::vector>{}, - nullptr, - std::make_shared(configuration_))), + response_node_loader_(std::make_shared(configuration_, std::vector>{}, nullptr)), test_response_node_(std::make_shared()), controller_socket_metrics_publisher_("test_publisher") { controller_socket_metrics_publisher_.initialize(configuration_, response_node_loader_); diff --git a/libminifi/test/unit/LogMetricsPublisherTests.cpp b/libminifi/test/unit/LogMetricsPublisherTests.cpp index 7292f0f07a..a1ada421f1 100644 --- a/libminifi/test/unit/LogMetricsPublisherTests.cpp +++ b/libminifi/test/unit/LogMetricsPublisherTests.cpp @@ -36,11 +36,7 @@ class LogPublisherTestFixture { : configuration_(std::make_shared()), provenance_repo_(core::createRepository("provenancerepository", "provenancerepository")), flow_file_repo_(core::createRepository("flowfilerepository", "flowfilerepository")), - response_node_loader_(std::make_shared( - configuration_, - std::vector>{provenance_repo_, flow_file_repo_}, - nullptr, - std::make_shared(configuration_))), + response_node_loader_(std::make_shared(configuration_, std::vector>{provenance_repo_, flow_file_repo_}, nullptr)), publisher_("LogMetricsPublisher") { configuration_->setHome(temp_directory_.getPath()); provenance_repo_->initialize(configuration_); From cde24d3ebeb2da9076b6ae066f70a6d8148f648f Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 3 Apr 2024 13:47:38 +0200 Subject: [PATCH 05/48] MINIFICPP-2314 - Fix test --- libminifi/test/integration/C2UpdateAssetTest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libminifi/test/integration/C2UpdateAssetTest.cpp b/libminifi/test/integration/C2UpdateAssetTest.cpp index 71cd0b7376..af775fe21b 100644 --- a/libminifi/test/integration/C2UpdateAssetTest.cpp +++ b/libminifi/test/integration/C2UpdateAssetTest.cpp @@ -248,7 +248,8 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { } size_t file_count = minifi::utils::file::list_dir_all(asset_dir.string(), controller.getLogger()).size(); - if (file_count != expected_files.size()) { + // + 1 is for the .state file from the AssetManager + if (file_count != expected_files.size() + 1) { controller.getLogger()->log_error("Expected {} files, got {}", expected_files.size(), file_count); REQUIRE(false); } From 46889c699c96037ab8346fee1365378eed55ae70 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 3 Apr 2024 13:50:34 +0200 Subject: [PATCH 06/48] MINIFICPP-2314 - Fix test --- libminifi/test/integration/C2AssetSyncTest.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index 324ffac03c..4463b88816 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -60,9 +60,6 @@ class C2HeartbeatHandler : public HeartbeatHandler { hb_str = std::string{buffer.GetString(), buffer.GetSize()}; } auto& asset_info_node = root["assetInfo"]; - if (!asset_info_node.IsObject()) { - int x = 0; - } auto& asset_hash_node = asset_info_node["hash"]; std::string asset_hash{asset_hash_node.GetString(), asset_hash_node.GetStringLength()}; @@ -98,7 +95,7 @@ class C2HeartbeatHandler : public HeartbeatHandler { void removeAsset(std::string id) { std::lock_guard guard{asset_mtx_}; - expected_assets_.erase(AssetDescription{.id = id}); + expected_assets_.erase(AssetDescription{.id = id, .path = {}, .url = {}}); } std::optional getAgentAssetHash() const { From b9993be3c4be272c7404fc28cc99a48af483ef49 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 3 Apr 2024 13:52:46 +0200 Subject: [PATCH 07/48] MINIFICPP-2314 - Fix windows build --- libminifi/src/utils/file/AssetManager.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index ec46d31961..e924e08d3c 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -22,6 +22,8 @@ #include "core/logging/LoggerFactory.h" #include "utils/Hash.h" +#undef GetObject + namespace org::apache::nifi::minifi::utils::file { AssetManager::AssetManager(std::shared_ptr configuration) From 7238f82baa2f055b133455cb12b06cd567cfb548 Mon Sep 17 00:00:00 2001 From: adamdebreceni <64783590+adamdebreceni@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:52:37 +0200 Subject: [PATCH 08/48] Update libminifi/src/core/state/nodes/AssetInformation.cpp Co-authored-by: Martin Zink --- libminifi/src/core/state/nodes/AssetInformation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/src/core/state/nodes/AssetInformation.cpp b/libminifi/src/core/state/nodes/AssetInformation.cpp index 7b585a735c..23a475ee5a 100644 --- a/libminifi/src/core/state/nodes/AssetInformation.cpp +++ b/libminifi/src/core/state/nodes/AssetInformation.cpp @@ -44,4 +44,4 @@ std::vector AssetInformation::serialize() { REGISTER_RESOURCE(AssetInformation, DescriptionOnly); -} // namespace org::apache::nifi::minifi::state::response \ No newline at end of file +} // namespace org::apache::nifi::minifi::state::response From 663500116f75a35ad4324b94d06a71d6a7b48e8d Mon Sep 17 00:00:00 2001 From: adamdebreceni <64783590+adamdebreceni@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:53:03 +0200 Subject: [PATCH 09/48] Update libminifi/src/core/state/nodes/AssetInformation.cpp Co-authored-by: Martin Zink --- libminifi/src/core/state/nodes/AssetInformation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/src/core/state/nodes/AssetInformation.cpp b/libminifi/src/core/state/nodes/AssetInformation.cpp index 23a475ee5a..717be7ee06 100644 --- a/libminifi/src/core/state/nodes/AssetInformation.cpp +++ b/libminifi/src/core/state/nodes/AssetInformation.cpp @@ -25,7 +25,7 @@ AssetInformation::AssetInformation() : logger_(core::logging::LoggerFactory().getLogger()) {} void AssetInformation::setAssetManager(std::shared_ptr asset_manager) { - asset_manager_ = asset_manager; + asset_manager_ = std::move(asset_manager); if (!asset_manager_) { logger_->log_error("No asset manager is provided, asset information will not be available"); } From ed5c62fe9af2e3f1671cbdbfe2e36292e034034a Mon Sep 17 00:00:00 2001 From: adamdebreceni <64783590+adamdebreceni@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:53:40 +0200 Subject: [PATCH 10/48] Update extensions/http-curl/tests/C2AssetSyncTest.cpp Co-authored-by: Martin Zink --- libminifi/test/integration/C2AssetSyncTest.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index 4463b88816..f6b0e79d0d 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -25,7 +25,6 @@ #include "HTTPIntegrationBase.h" #include "HTTPHandlers.h" #include "utils/IntegrationTestUtils.h" -#include "utils/Environment.h" #include "utils/file/FileUtils.h" #include "utils/file/AssetManager.h" From 440f45b35eeebd4d42c1ace9af3ad63cc4ab5f79 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 10 Apr 2024 11:08:36 +0200 Subject: [PATCH 11/48] MINIFICPP-2314 - Review changes --- CONFIGURE.md | 9 +++++++-- libminifi/include/core/state/nodes/AssetInformation.h | 2 +- libminifi/include/utils/file/AssetManager.h | 2 +- libminifi/src/utils/file/AssetManager.cpp | 4 ++-- minifi_main/MiNiFiMain.cpp | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/CONFIGURE.md b/CONFIGURE.md index 9f93c8a445..d422d078b0 100644 --- a/CONFIGURE.md +++ b/CONFIGURE.md @@ -665,8 +665,13 @@ Additionally, a unique hexadecimal uid.minifi.device.segment should be assigned ### Asset directory -It is possible to make agents download an asset (triggered through the c2 protocol). The target directory can be specified -using the `nifi.asset.directory` agent property, which defaults to `${MINIFI_HOME}/asset`. +There is an asset directory specified using the `nifi.asset.directory` agent property, which defaults to `${MINIFI_HOME}/asset`. +The files referenced in the `.state` file in this directory are managed by the agent, there are deleted, updated, downloaded +using the asset sync c2 command. For the asset sync command to work, the c2 server must be made aware of the current state of the +managed assets, for this the `AssetInformation` entry has to be added to the `nifi.c2.root.classes` property. + +Files and directories not referenced in the `.state` file are not directly controlled by the agent although, +it is possible to download an asset (triggered through the c2 protocol) into the asset directory instead. ### Controller Services If you need to reference a controller service in your config.yml file, use the following template. In the example, below, ControllerServiceClass is the name of the class defining the controller Service. ControllerService1 diff --git a/libminifi/include/core/state/nodes/AssetInformation.h b/libminifi/include/core/state/nodes/AssetInformation.h index 2fd048b753..e1af964586 100644 --- a/libminifi/include/core/state/nodes/AssetInformation.h +++ b/libminifi/include/core/state/nodes/AssetInformation.h @@ -25,7 +25,7 @@ namespace org::apache::nifi::minifi::state::response { class AssetInformation : public ResponseNode { public: AssetInformation(); - explicit AssetInformation(std::string name, const utils::Identifier& uuid = {}) : ResponseNode(std::move(name), uuid) {} + explicit AssetInformation(std::string_view name, const utils::Identifier& uuid = {}) : ResponseNode(name, uuid) {} MINIFIAPI static constexpr const char* Description = "Metric node that defines hash for all asset identifiers"; diff --git a/libminifi/include/utils/file/AssetManager.h b/libminifi/include/utils/file/AssetManager.h index c989d33c4f..97669d1343 100644 --- a/libminifi/include/utils/file/AssetManager.h +++ b/libminifi/include/utils/file/AssetManager.h @@ -43,7 +43,7 @@ using AssetLayout = std::set; class AssetManager { public: - explicit AssetManager(std::shared_ptr configuration); + explicit AssetManager(const Configure& configuration); nonstd::expected sync(AssetLayout layout, std::function, std::string>(std::string_view /*url*/)> fetch); diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index e924e08d3c..4eeda94c8a 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -26,8 +26,8 @@ namespace org::apache::nifi::minifi::utils::file { -AssetManager::AssetManager(std::shared_ptr configuration) - : root_(configuration->get(Configure::nifi_asset_directory).value_or(configuration->getHome() / "asset")), +AssetManager::AssetManager(const Configure& configuration) + : root_(configuration.get(Configure::nifi_asset_directory).value_or(configuration.getHome() / "asset")), logger_(core::logging::LoggerFactory::getLogger()) { refreshState(); } diff --git a/minifi_main/MiNiFiMain.cpp b/minifi_main/MiNiFiMain.cpp index a102d63865..9d3f47901c 100644 --- a/minifi_main/MiNiFiMain.cpp +++ b/minifi_main/MiNiFiMain.cpp @@ -398,7 +398,7 @@ int main(int argc, char **argv) { .sensitive_values_encryptor = utils::crypto::EncryptionProvider::createSensitivePropertiesEncryptor(minifiHome) }, nifi_configuration_class_name); - auto asset_manager = std::make_shared(configure); + auto asset_manager = std::make_shared(*configure); std::vector> repo_metric_sources{prov_repo, flow_repo, content_repo}; auto metrics_publisher_store = std::make_unique(configure, repo_metric_sources, flow_configuration, asset_manager); From 1741b3ba91056f5b9a0e60f5cdc7c9664a43604c Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 10 Apr 2024 11:15:21 +0200 Subject: [PATCH 12/48] MINIFICPP-2314 - Linter fix --- libminifi/src/utils/file/AssetManager.cpp | 5 +++-- libminifi/test/unit/LogMetricsPublisherTests.cpp | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index 4eeda94c8a..343c7cdc7a 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -78,7 +78,6 @@ void AssetManager::refreshState() { } else { logger_->log_error("Asset '.state' file contains entry that does not exist on the filesystem"); } - } state_ = std::move(new_state); } @@ -95,7 +94,9 @@ std::string AssetManager::hash() const { return cached_hash_.value(); } -nonstd::expected AssetManager::sync(org::apache::nifi::minifi::utils::file::AssetLayout layout, std::function, std::string>(std::string_view /*url*/)> fetch) { +nonstd::expected AssetManager::sync( + org::apache::nifi::minifi::utils::file::AssetLayout layout, + std::function, std::string>(std::string_view /*url*/)> fetch) { std::lock_guard lock(mtx_); std::vector>> new_file_contents; for (auto& new_entry : layout) { diff --git a/libminifi/test/unit/LogMetricsPublisherTests.cpp b/libminifi/test/unit/LogMetricsPublisherTests.cpp index a1ada421f1..511113d378 100644 --- a/libminifi/test/unit/LogMetricsPublisherTests.cpp +++ b/libminifi/test/unit/LogMetricsPublisherTests.cpp @@ -36,7 +36,8 @@ class LogPublisherTestFixture { : configuration_(std::make_shared()), provenance_repo_(core::createRepository("provenancerepository", "provenancerepository")), flow_file_repo_(core::createRepository("flowfilerepository", "flowfilerepository")), - response_node_loader_(std::make_shared(configuration_, std::vector>{provenance_repo_, flow_file_repo_}, nullptr)), + response_node_loader_(std::make_shared(configuration_, + std::vector>{provenance_repo_, flow_file_repo_}, nullptr)), publisher_("LogMetricsPublisher") { configuration_->setHome(temp_directory_.getPath()); provenance_repo_->initialize(configuration_); From f332f6cafde5946a35a1e2fcbf55daf86e23837e Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 10 Apr 2024 14:21:46 +0200 Subject: [PATCH 13/48] MINIFICPP-2314 - Fix build --- libminifi/src/utils/file/AssetManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index 343c7cdc7a..a125ce2ad5 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -27,7 +27,7 @@ namespace org::apache::nifi::minifi::utils::file { AssetManager::AssetManager(const Configure& configuration) - : root_(configuration.get(Configure::nifi_asset_directory).value_or(configuration.getHome() / "asset")), + : root_(configuration.get(Configure::nifi_asset_directory).value_or((configuration.getHome() / "asset").string())), logger_(core::logging::LoggerFactory::getLogger()) { refreshState(); } From a7b60f59d8cf3a821bdd6e1c0c3492eae9753fdf Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Mon, 15 Apr 2024 09:45:21 +0200 Subject: [PATCH 14/48] MINIFICPP-2314 - Review changes --- C2.md | 3 ++- .../cluster/containers/MinifiContainer.py | 4 ++-- .../tests/resources/minifi.properties | 2 +- ...ditional-sensitive-props.minifi.properties | 2 +- .../test/integration/C2AssetSyncTest.cpp | 24 +++++++++---------- .../resources/encrypted.minifi.properties | 2 +- minifi_main/MiNiFiMain.cpp | 3 ++- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/C2.md b/C2.md index 9d84974ffb..e42fe6be4c 100644 --- a/C2.md +++ b/C2.md @@ -66,9 +66,10 @@ be requested via C2 DESCRIBE manifest command. # DeviceInfoNode: basic info about the system (OS, number of cores etc) # AgentInformation: info about the MiNiFi agent, may include the manifest # FlowInformation: information about the current flow, including queue sizes + # AssetInformation: the state of the asset directory managed by the agent # ConfigurationChecksums: hashes of the configuration files; can be used to detect unexpected modifications # the default is - nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation + nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation,AssetInformation # control c2 heartbeat interval nifi.c2.agent.heartbeat.period=30 sec diff --git a/docker/test/integration/cluster/containers/MinifiContainer.py b/docker/test/integration/cluster/containers/MinifiContainer.py index bbe56a879b..fee054f06a 100644 --- a/docker/test/integration/cluster/containers/MinifiContainer.py +++ b/docker/test/integration/cluster/containers/MinifiContainer.py @@ -99,7 +99,7 @@ def _create_properties(self): f.write(f"nifi.c2.rest.url=http://minifi-c2-server-{self.feature_context.id}:10090/c2/config/heartbeat\n") f.write(f"nifi.c2.rest.url.ack=http://minifi-c2-server-{self.feature_context.id}:10090/c2/config/acknowledge\n") f.write(f"nifi.c2.flow.base.url=http://minifi-c2-server-{self.feature_context.id}:10090/c2/config/\n") - f.write("nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation\n") + f.write("nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation,AssetInformation\n") f.write("nifi.c2.full.heartbeat=false\n") f.write("nifi.c2.agent.class=minifi-test-class\n") f.write("nifi.c2.agent.identifier=minifi-test-id\n") @@ -109,7 +109,7 @@ def _create_properties(self): f.write(f"nifi.c2.rest.url.ack=https://minifi-c2-server-{self.feature_context.id}:10090/c2/config/acknowledge\n") f.write("nifi.c2.rest.ssl.context.service=SSLContextService\n") f.write(f"nifi.c2.flow.base.url=https://minifi-c2-server-{self.feature_context.id}:10090/c2/config/\n") - f.write("nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation\n") + f.write("nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation,AssetInformation\n") f.write("nifi.c2.full.heartbeat=false\n") f.write("nifi.c2.agent.class=minifi-test-class\n") f.write("nifi.c2.agent.identifier=minifi-test-id\n") diff --git a/encrypt-config/tests/resources/minifi.properties b/encrypt-config/tests/resources/minifi.properties index 9bac06775a..2f2db68bb7 100644 --- a/encrypt-config/tests/resources/minifi.properties +++ b/encrypt-config/tests/resources/minifi.properties @@ -55,7 +55,7 @@ nifi.c2.enable=true nifi.c2.flow.base.url=http://localhost:10080/c2-server/api nifi.c2.rest.url=http://localhost:10080/c2-server/api/c2-protocol/heartbeat nifi.c2.rest.url.ack=http://localhost:10080/c2-server/api/c2-protocol/acknowledge -nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation +nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation,AssetInformation ## Minimize heartbeat payload size by excluding agent manifest from the heartbeat #nifi.c2.full.heartbeat=false ## heartbeat 4 times a second diff --git a/encrypt-config/tests/resources/with-additional-sensitive-props.minifi.properties b/encrypt-config/tests/resources/with-additional-sensitive-props.minifi.properties index aff36a0656..d2702c34df 100644 --- a/encrypt-config/tests/resources/with-additional-sensitive-props.minifi.properties +++ b/encrypt-config/tests/resources/with-additional-sensitive-props.minifi.properties @@ -57,7 +57,7 @@ nifi.c2.enable=true nifi.c2.flow.base.url=http://localhost:10080/c2-server/api nifi.c2.rest.url=http://localhost:10080/c2-server/api/c2-protocol/heartbeat nifi.c2.rest.url.ack=http://localhost:10080/c2-server/api/c2-protocol/acknowledge -nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation +nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation,AssetInformation ## Minimize heartbeat payload size by excluding agent manifest from the heartbeat #nifi.c2.full.heartbeat=false ## heartbeat 4 times a second diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index f6b0e79d0d..08e15451fd 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -50,14 +50,12 @@ class C2HeartbeatHandler : public HeartbeatHandler { using AssetDescription = org::apache::nifi::minifi::utils::file::AssetDescription; void handleHeartbeat(const rapidjson::Document& root, struct mg_connection* conn) override { - std::string hb_str; - { + std::string hb_str = [&] { rapidjson::StringBuffer buffer; rapidjson::Writer writer(buffer); root.Accept(writer); - - hb_str = std::string{buffer.GetString(), buffer.GetSize()}; - } + return std::string{buffer.GetString(), buffer.GetSize()}; + }(); auto& asset_info_node = root["assetInfo"]; auto& asset_hash_node = asset_info_node["hash"]; std::string asset_hash{asset_hash_node.GetString(), asset_hash_node.GetStringLength()}; @@ -66,7 +64,7 @@ class C2HeartbeatHandler : public HeartbeatHandler { { std::lock_guard guard(asset_mtx_); agent_asset_hash_ = asset_hash; - if (asset_hash != assetHash()) { + if (asset_hash != calculateAssetHash()) { std::unordered_map args; for (auto& asset : expected_assets_) { args[asset.id + ".path"] = asset.path; @@ -86,15 +84,15 @@ class C2HeartbeatHandler : public HeartbeatHandler { void addAsset(std::string id, std::string path, std::string url) { std::lock_guard guard(asset_mtx_); expected_assets_.insert(AssetDescription{ - .id = id, - .path = path, - .url = url + .id = std::move(id), + .path = std::move(path), + .url = std::move(url) }); } void removeAsset(std::string id) { std::lock_guard guard{asset_mtx_}; - expected_assets_.erase(AssetDescription{.id = id, .path = {}, .url = {}}); + expected_assets_.erase(AssetDescription{.id = std::move(id), .path = {}, .url = {}}); } std::optional getAgentAssetHash() const { @@ -102,7 +100,7 @@ class C2HeartbeatHandler : public HeartbeatHandler { return agent_asset_hash_; } - std::string assetHash() const { + std::string calculateAssetHash() const { std::lock_guard guard{asset_mtx_}; size_t hash_value{0}; for (auto& asset : expected_assets_) { @@ -202,7 +200,7 @@ int main() { }; harness.setVerifier([&] () { - assert(utils::verifyEventHappenedInPollTime(10s, [&] {return hb_handler.assetHash() == hb_handler.getAgentAssetHash();})); + assert(utils::verifyEventHappenedInPollTime(10s, [&] {return hb_handler.calculateAssetHash() == hb_handler.getAgentAssetHash();})); { std::unordered_map expected_assets{ @@ -229,7 +227,7 @@ int main() { hb_handler.addAsset("Av2", "A.txt", "/api/file/Av2.txt"); - assert(utils::verifyEventHappenedInPollTime(10s, [&] {return hb_handler.assetHash() == hb_handler.getAgentAssetHash();})); + assert(utils::verifyEventHappenedInPollTime(10s, [&] {return hb_handler.calculateAssetHash() == hb_handler.getAgentAssetHash();})); { std::unordered_map expected_assets{ diff --git a/libminifi/test/resources/encrypted.minifi.properties b/libminifi/test/resources/encrypted.minifi.properties index c9f3eac068..f19422e43f 100644 --- a/libminifi/test/resources/encrypted.minifi.properties +++ b/libminifi/test/resources/encrypted.minifi.properties @@ -57,7 +57,7 @@ nifi.c2.enable=true nifi.c2.flow.base.url=http://localhost:10080/c2-server/api nifi.c2.rest.url=http://localhost:10080/c2-server/api/c2-protocol/heartbeat nifi.c2.rest.url.ack=http://localhost:10080/c2-server/api/c2-protocol/acknowledge -nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation +nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation,AssetInformation ## Minimize heartbeat payload size by excluding agent manifest from the heartbeat #nifi.c2.full.heartbeat=false ## heartbeat 4 times a second diff --git a/minifi_main/MiNiFiMain.cpp b/minifi_main/MiNiFiMain.cpp index 9d3f47901c..92b5d41166 100644 --- a/minifi_main/MiNiFiMain.cpp +++ b/minifi_main/MiNiFiMain.cpp @@ -403,7 +403,8 @@ int main(int argc, char **argv) { std::vector> repo_metric_sources{prov_repo, flow_repo, content_repo}; auto metrics_publisher_store = std::make_unique(configure, repo_metric_sources, flow_configuration, asset_manager); const auto controller = std::make_unique( - prov_repo, flow_repo, configure, std::move(flow_configuration), content_repo, std::move(metrics_publisher_store), filesystem, request_restart); + prov_repo, flow_repo, configure, std::move(flow_configuration), content_repo, + std::move(metrics_publisher_store), filesystem, request_restart, asset_manager); const bool disk_space_watchdog_enable = configure->get(minifi::Configure::minifi_disk_space_watchdog_enable) | utils::andThen(utils::string::toBool) From 8f9b1257d56083b8801910f16b0f0049ca4df7fe Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 17 Apr 2024 11:26:10 +0200 Subject: [PATCH 15/48] MINIFICPP-2314 - Clang tidy fix --- libminifi/include/utils/file/AssetManager.h | 2 +- libminifi/src/utils/file/AssetManager.cpp | 6 +++--- libminifi/test/integration/C2AssetSyncTest.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libminifi/include/utils/file/AssetManager.h b/libminifi/include/utils/file/AssetManager.h index 97669d1343..bfc9f8429d 100644 --- a/libminifi/include/utils/file/AssetManager.h +++ b/libminifi/include/utils/file/AssetManager.h @@ -45,7 +45,7 @@ class AssetManager { public: explicit AssetManager(const Configure& configuration); - nonstd::expected sync(AssetLayout layout, std::function, std::string>(std::string_view /*url*/)> fetch); + nonstd::expected sync(AssetLayout layout, const std::function, std::string>(std::string_view /*url*/)>& fetch); std::string hash() const; diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index a125ce2ad5..dec2e0088c 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -96,7 +96,7 @@ std::string AssetManager::hash() const { nonstd::expected AssetManager::sync( org::apache::nifi::minifi::utils::file::AssetLayout layout, - std::function, std::string>(std::string_view /*url*/)> fetch) { + const std::function, std::string>(std::string_view /*url*/)>& fetch) { std::lock_guard lock(mtx_); std::vector>> new_file_contents; for (auto& new_entry : layout) { @@ -119,7 +119,7 @@ nonstd::expected AssetManager::sync( for (auto& [path, content] : new_file_contents) { utils::file::create_dir((root_ / path).parent_path()); - std::ofstream{root_ / path, std::ios::binary}.write(reinterpret_cast(content.data()), content.size()); + std::ofstream{root_ / path, std::ios::binary}.write(reinterpret_cast(content.data()), gsl::narrow(content.size())); } state_ = std::move(layout); @@ -143,7 +143,7 @@ void AssetManager::persist() const { rapidjson::Writer writer(buffer); doc.Accept(writer); - std::ofstream{root_ / ".state", std::ios::binary}.write(buffer.GetString(), buffer.GetSize()); + std::ofstream{root_ / ".state", std::ios::binary}.write(buffer.GetString(), gsl::narrow(buffer.GetSize())); } std::filesystem::path AssetManager::getRoot() const { diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index 08e15451fd..02c7570a95 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -193,8 +193,8 @@ int main() { auto get_asset_structure = [&] () { std::unordered_map contents; - for (auto file : utils::file::list_dir_all(asset_dir.string(), controller.getLogger())) { - contents[(file.first / file.second).string()] = utils::file::get_content(file.first / file.second); + for (auto& [dir, file] : utils::file::list_dir_all(asset_dir.string(), controller.getLogger())) { + contents[(dir / file).string()] = utils::file::get_content(dir / file); } return contents; }; From a563fbbb13a4dea0e4f1aebd37b640d6696cf2db Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 17 Apr 2024 13:27:32 +0200 Subject: [PATCH 16/48] MINIFICPP-2314 - Windows fix --- libminifi/test/integration/C2AssetSyncTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index 02c7570a95..053c2f97af 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -67,7 +67,7 @@ class C2HeartbeatHandler : public HeartbeatHandler { if (asset_hash != calculateAssetHash()) { std::unordered_map args; for (auto& asset : expected_assets_) { - args[asset.id + ".path"] = asset.path; + args[asset.id + ".path"] = asset.path.string(); args[asset.id + ".url"] = asset.url; } operations.push_back(C2Operation{ From d23f4b81cfa4af448b1d172b2cf543b274381bb6 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Mon, 29 Apr 2024 09:51:21 +0200 Subject: [PATCH 17/48] MINIFICPP-2314 - Fix test --- libminifi/test/integration/C2AssetSyncTest.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index 053c2f97af..5d2336f050 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -205,8 +205,8 @@ int main() { { std::unordered_map expected_assets{ {(asset_dir / "A.txt").string(), file_A}, - {(asset_dir / "nested/dir/B.txt").string(), file_B}, - {(asset_dir / "nested/C.txt").string(), file_C}, + {(asset_dir / "nested" / "dir" / "B.txt").string(), file_B}, + {(asset_dir / "nested" / "C.txt").string(), file_C}, {(asset_dir / ".state").string(), hb_handler.assetState()} }; auto actual_assets = get_asset_structure(); @@ -232,7 +232,7 @@ int main() { { std::unordered_map expected_assets{ {(asset_dir / "A.txt").string(), file_A_v2}, - {(asset_dir / "nested/dir/B.txt").string(), file_B}, + {(asset_dir / "nested" / "dir" / "B.txt").string(), file_B}, {(asset_dir / ".state").string(), hb_handler.assetState()} }; From cb883e44e70177dab80d44bc077f9a08aec328ab Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Thu, 2 May 2024 16:07:26 +0200 Subject: [PATCH 18/48] MINIFICPP-2314 - Make sure that home dir is not the temp dir --- libminifi/test/integration/C2AssetSyncTest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index 5d2336f050..1abbbb7e35 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -251,4 +251,6 @@ int main() { }); harness.run(); + + std::filesystem::current_path(minifi::utils::file::get_executable_dir()); } From 64934370512070dd884aeb3f851372be6b266dec Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 29 May 2024 10:43:15 +0200 Subject: [PATCH 19/48] MINIFICPP-2314 - Change sync operand --- libminifi/include/c2/C2Payload.h | 2 +- libminifi/src/c2/C2Agent.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libminifi/include/c2/C2Payload.h b/libminifi/include/c2/C2Payload.h index c19f198eac..1329948522 100644 --- a/libminifi/include/c2/C2Payload.h +++ b/libminifi/include/c2/C2Payload.h @@ -72,7 +72,7 @@ enum class ClearOperand : uint8_t{ }; enum class SyncOperand : uint8_t{ - asset + resource }; #define PAYLOAD_NO_STATUS 0 diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index 596a9112fe..3c04a01550 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -719,7 +719,7 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse return; } - SyncOperand operand = SyncOperand::asset; + SyncOperand operand = SyncOperand::resource; try { operand = utils::enumCast(resp.name, true); } catch(const std::runtime_error&) { @@ -727,7 +727,7 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse return; } - gsl_Assert(operand == SyncOperand::asset); + gsl_Assert(operand == SyncOperand::resource); // we are expecting the format // args: { From fc30c1b92d86dc6196e5bccf2dff5e40fac7223a Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 29 May 2024 15:02:19 +0200 Subject: [PATCH 20/48] MINIFICPP-2314 - Update c2 protocol --- libminifi/include/c2/protocols/RESTProtocol.h | 2 +- libminifi/include/utils/file/AssetManager.h | 11 +- libminifi/src/c2/C2Agent.cpp | 124 ++++++++++++++---- libminifi/src/c2/protocols/RESTProtocol.cpp | 5 +- libminifi/src/utils/file/AssetManager.cpp | 51 ++++--- .../test/integration/C2AssetSyncTest.cpp | 57 +++++--- libminifi/test/integration/C2MetricsTest.cpp | 9 +- .../test/libtest/integration/HTTPHandlers.cpp | 43 +++--- .../libtest/integration/IntegrationBase.cpp | 6 +- 9 files changed, 206 insertions(+), 102 deletions(-) diff --git a/libminifi/include/c2/protocols/RESTProtocol.h b/libminifi/include/c2/protocols/RESTProtocol.h index 36c7412788..f3ed0ebce7 100644 --- a/libminifi/include/c2/protocols/RESTProtocol.h +++ b/libminifi/include/c2/protocols/RESTProtocol.h @@ -43,7 +43,7 @@ class RESTProtocol : public HeartbeatJsonSerializer { protected: void initialize(core::controller::ControllerServiceProvider* controller, const std::shared_ptr &configure); void serializeNestedPayload(rapidjson::Value& target, const C2Payload& payload, rapidjson::Document::AllocatorType& alloc) override; - static C2Payload parseJsonResponse(const C2Payload &payload, std::span response); + C2Payload parseJsonResponse(const C2Payload &payload, std::span response); private: bool containsPayload(const C2Payload &o); diff --git a/libminifi/include/utils/file/AssetManager.h b/libminifi/include/utils/file/AssetManager.h index bfc9f8429d..f269988657 100644 --- a/libminifi/include/utils/file/AssetManager.h +++ b/libminifi/include/utils/file/AssetManager.h @@ -39,7 +39,15 @@ struct AssetDescription { } }; -using AssetLayout = std::set; +struct AssetLayout { + std::string digest; + std::set assets; + + void clear() { + digest.clear(); + assets.clear(); + } +}; class AssetManager { public: @@ -59,7 +67,6 @@ class AssetManager { mutable std::recursive_mutex mtx_; std::filesystem::path root_; AssetLayout state_; - mutable std::optional cached_hash_; std::shared_ptr logger_; }; diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index 3c04a01550..2cf710384c 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -47,6 +47,7 @@ #include "utils/Id.h" #include "c2/C2Utils.h" #include "c2/protocols/RESTSender.h" +#include "rapidjson/error/en.h" using namespace std::literals::chrono_literals; @@ -729,44 +730,111 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse gsl_Assert(operand == SyncOperand::resource); - // we are expecting the format - // args: { - // ".path": "a/b/c.txt", - // ".url": "example.com", - // ".path": "a/b/c.txt", - // ".url": "example.com" - // } std::set ids; utils::file::AssetLayout asset_layout; - for (auto& arg: resp.operation_arguments) { - auto fragments = utils::string::split(arg.first, "."); - if (fragments.size() == 2) { - ids.insert(fragments.front()); - } else { - send_error("Malformed asset sync command"); + + auto state_it = resp.operation_arguments.find("globalHash"); + if (state_it == resp.operation_arguments.end()) { + send_error("Malformed request, missing 'globalHash' argument"); + return; + } + + rapidjson::Document state_doc; + rapidjson::ParseResult res = state_doc.Parse(state_it->second.to_string()); + if (res.IsError()) { + send_error(fmt::format("Malformed request, 'globalHash' is not a valid json document: {} at {}", rapidjson::GetParseError_En(res.Code()), res.Offset())); + return; + } + + if (!state_doc.IsObject()) { + send_error("Malformed request, 'globalHash' is not a json object"); + return; + } + + if (!state_doc.HasMember("digest")) { + send_error("Malformed request, 'globalHash' has no member 'digest'"); + return; + } + if (!state_doc["digest"].IsString()) { + send_error("Malformed request, 'globalHash.digest' is not a string"); + return; + } + + asset_layout.digest = std::string{state_doc["digest"].GetString(), state_doc["digest"].GetStringLength()}; + + auto resource_list_it = resp.operation_arguments.find("resourceList"); + if (resource_list_it == resp.operation_arguments.end()) { + send_error("Malformed request, missing 'resourceList' argument"); + return; + } + + rapidjson::Document resource_list; + res = resource_list.Parse(resource_list_it->second.to_string()); + if (res.IsError()) { + send_error(fmt::format("Malformed request, 'resourceList' is not a valid json document: {} at {}", rapidjson::GetParseError_En(res.Code()), res.Offset())); + return; + } + if (!resource_list.IsArray()) { + send_error("Malformed request, 'resourceList' is not a json array"); + return; + } + + for (size_t resource_idx = 0; resource_idx < resource_list.Size(); ++resource_idx) { + auto& resource = resource_list.GetArray()[resource_idx]; + if (!resource.IsObject()) { + send_error(fmt::format("Malformed request, 'resourceList[{}]' is not a json object", resource_idx)); return; } - } - for (auto& id : ids) { - auto path_it = resp.operation_arguments.find(id + ".path"); - if (path_it == resp.operation_arguments.end()) { - send_error("Malformed path for id '" + id + "'"); + auto get_member_str = [&] (const char* key) -> nonstd::expected { + if (!resource.HasMember(key)) { + return nonstd::make_unexpected(fmt::format("Malformed request, 'resourceList[{}]' has no member '{}'", resource_idx, key)); + } + if (!resource[key].IsString()) { + return nonstd::make_unexpected(fmt::format("Malformed request, 'resourceList[{}].{}' is not a string", resource_idx, key)); + } + return std::string_view{resource[key].GetString(), resource[key].GetStringLength()}; + }; + auto id = get_member_str("resourceId"); + if (!id) { + send_error(id.error()); return; } - auto result = utils::file::validateRelativePath(path_it->second.to_string()); - if (!result) { - send_error(result.error()); + auto name = get_member_str("resourceName"); + if (!name) { + send_error(name.error()); return; } - auto url_it = resp.operation_arguments.find(id + ".url"); - if (url_it == resp.operation_arguments.end()) { - send_error("Malformed url for id '" + id + "'"); + auto type = get_member_str("resourceType"); + if (!type) { + send_error(type.error()); return; } - asset_layout.insert(utils::file::AssetDescription{ - .id = id, - .path = path_it->second.to_string(), - .url = url_it->second.to_string() + if (type.value() != "ASSET") { + continue; + } + auto path = get_member_str("resourcePath"); + if (!path) { + send_error(path.error()); + return; + } + auto url = get_member_str("url"); + if (!url) { + send_error(url.error()); + return; + } + + auto full_path = std::filesystem::path{path.value()} / name.value(); + + auto path_valid = utils::file::validateRelativePath(full_path); + if (!path_valid) { + send_error(path_valid.error()); + return; + } + + asset_layout.assets.insert(utils::file::AssetDescription{ + .id = std::string{id.value()}, + .path = full_path, + .url = std::string{url.value()} }); } diff --git a/libminifi/src/c2/protocols/RESTProtocol.cpp b/libminifi/src/c2/protocols/RESTProtocol.cpp index ca4f3faeab..db389778bc 100644 --- a/libminifi/src/c2/protocols/RESTProtocol.cpp +++ b/libminifi/src/c2/protocols/RESTProtocol.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include "core/TypedValues.h" #include "utils/gsl.h" @@ -50,7 +51,7 @@ AnnotatedValue parseAnnotatedValue(const rapidjson::Value& jsonValue) { } else if (jsonValue.IsBool()) { result = jsonValue.GetBool(); } else { - result = jsonValue.GetString(); + result = std::string{jsonValue.GetString(), jsonValue.GetStringLength()}; } return result; } @@ -136,6 +137,8 @@ C2Payload RESTProtocol::parseJsonResponse(const C2Payload &payload, std::spanlog_error("Failed to parse json response: {} at {}", rapidjson::GetParseError_En(ok.Code()), ok.Offset()); } } catch (...) { } diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index dec2e0088c..7a639ecf8a 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -39,7 +39,7 @@ void AssetManager::refreshState() { std::filesystem::create_directory(root_); } if (!utils::file::FileUtils::exists(root_ / ".state")) { - std::ofstream{root_ / ".state", std::ios::binary} << "{}"; + std::ofstream{root_ / ".state", std::ios::binary} << "{\"digest\": \"\", \"assets\": {}}"; } rapidjson::Document doc; @@ -54,8 +54,28 @@ void AssetManager::refreshState() { logger_->log_error("Asset '.state' file is malformed"); return; } + if (!doc.HasMember("digest")) { + logger_->log_error("Asset '.state' file is malformed, missing 'digest'"); + return; + } + if (!doc["digest"].IsString()) { + logger_->log_error("Asset '.state' file is malformed, 'digest' is not a string"); + return; + } + if (!doc.HasMember("assets")) { + logger_->log_error("Asset '.state' file is malformed, missing 'assets'"); + return; + } + if (!doc["assets"].IsObject()) { + logger_->log_error("Asset '.state' file is malformed, 'assets' is not an object"); + return; + } + + AssetLayout new_state; - for (auto& [id, entry] : doc.GetObject()) { + new_state.digest = std::string{doc["digest"].GetString(), doc["digest"].GetStringLength()}; + + for (auto& [id, entry] : doc["assets"].GetObject()) { if (!entry.IsObject()) { logger_->log_error("Asset '.state' file is malformed"); return; @@ -74,7 +94,7 @@ void AssetManager::refreshState() { description.url = std::string{entry["url"].GetString(), entry["url"].GetStringLength()}; if (utils::file::FileUtils::exists(root_ / description.id)) { - new_state.insert(std::move(description)); + new_state.assets.insert(std::move(description)); } else { logger_->log_error("Asset '.state' file contains entry that does not exist on the filesystem"); } @@ -84,14 +104,7 @@ void AssetManager::refreshState() { std::string AssetManager::hash() const { std::lock_guard lock(mtx_); - if (!cached_hash_) { - size_t hash_value{0}; - for (auto& entry : state_) { - hash_value = utils::hash_combine(hash_value, std::hash{}(entry.id)); - } - cached_hash_ = std::to_string(hash_value); - } - return cached_hash_.value(); + return state_.digest.empty() ? "null" : state_.digest; } nonstd::expected AssetManager::sync( @@ -99,8 +112,8 @@ nonstd::expected AssetManager::sync( const std::function, std::string>(std::string_view /*url*/)>& fetch) { std::lock_guard lock(mtx_); std::vector>> new_file_contents; - for (auto& new_entry : layout) { - if (std::find_if(state_.begin(), state_.end(), [&] (auto& entry) {return entry.id == new_entry.id;}) == state_.end()) { + for (auto& new_entry : layout.assets) { + if (std::find_if(state_.assets.begin(), state_.assets.end(), [&] (auto& entry) {return entry.id == new_entry.id;}) == state_.assets.end()) { if (auto data = fetch(new_entry.url)) { new_file_contents.emplace_back(new_entry.path, data.value()); } else { @@ -108,10 +121,9 @@ nonstd::expected AssetManager::sync( } } } - cached_hash_.reset(); - for (auto& old_entry : state_) { - if (std::find_if(layout.begin(), layout.end(), [&] (auto& entry) {return entry.id == old_entry.id;}) == layout.end()) { + for (auto& old_entry : state_.assets) { + if (std::find_if(layout.assets.begin(), layout.assets.end(), [&] (auto& entry) {return entry.id == old_entry.id;}) == layout.assets.end()) { // we no longer need this asset std::filesystem::remove(root_ / old_entry.path); } @@ -132,11 +144,14 @@ void AssetManager::persist() const { rapidjson::Document doc; doc.SetObject(); - for (auto& entry : state_) { + doc.AddMember(rapidjson::StringRef("digest"), rapidjson::Value{state_.digest, doc.GetAllocator()}, doc.GetAllocator()); + doc.AddMember(rapidjson::StringRef("assets"), rapidjson::Value{rapidjson::kObjectType}, doc.GetAllocator()); + + for (auto& entry : state_.assets) { rapidjson::Value entry_val(rapidjson::kObjectType); entry_val.AddMember(rapidjson::StringRef("path"), rapidjson::Value(entry.path.string().c_str(), doc.GetAllocator()), doc.GetAllocator()); entry_val.AddMember(rapidjson::StringRef("url"), rapidjson::StringRef(entry.url), doc.GetAllocator()); - doc.AddMember(rapidjson::StringRef(entry.id), entry_val, doc.GetAllocator()); + doc["assets"].AddMember(rapidjson::StringRef(entry.id), entry_val, doc.GetAllocator()); } rapidjson::StringBuffer buffer; diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index 1abbbb7e35..07d9881861 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -16,17 +16,19 @@ * limitations under the License. */ -#undef NDEBUG #include #include #include #include -#include "HTTPIntegrationBase.h" -#include "HTTPHandlers.h" -#include "utils/IntegrationTestUtils.h" +#include "integration/HTTPIntegrationBase.h" +#include "integration/HTTPHandlers.h" #include "utils/file/FileUtils.h" #include "utils/file/AssetManager.h" +#include "unit/TestUtils.h" +#include "unit/Catch.h" + +namespace org::apache::nifi::minifi::test { class FileProvider : public ServerAwareHandler { public: @@ -66,13 +68,24 @@ class C2HeartbeatHandler : public HeartbeatHandler { agent_asset_hash_ = asset_hash; if (asset_hash != calculateAssetHash()) { std::unordered_map args; + args["globalHash"] = R"({"digest": ")" + calculateAssetHash() + "\"}"; + args["resourceList"] = "["; + for (auto& asset : expected_assets_) { - args[asset.id + ".path"] = asset.path.string(); - args[asset.id + ".url"] = asset.url; + if (args["resourceList"] != "[") args["resourceList"] += ", "; + args["resourceList"] += "{"; + args["resourceList"] += "\"resourceId\": \"" + asset.id + "\", "; + args["resourceList"] += "\"resourceName\": \"" + asset.path.filename().string() + "\", "; + args["resourceList"] += "\"resourceType\": \"ASSET\", "; + args["resourceList"] += "\"resourcePath\": \"" + asset.path.parent_path().string() + "\", "; + args["resourceList"] += "\"url\": \"" + asset.url + "\""; + args["resourceList"] += "}"; } + + args["resourceList"] += "]"; operations.push_back(C2Operation{ .operation = "sync", - .operand = "asset", + .operand = "resource", .operation_id = std::to_string(next_op_id_++), .args = std::move(args) }); @@ -104,7 +117,7 @@ class C2HeartbeatHandler : public HeartbeatHandler { std::lock_guard guard{asset_mtx_}; size_t hash_value{0}; for (auto& asset : expected_assets_) { - hash_value = utils::hash_combine(hash_value, std::hash{}(asset.id)); + hash_value = minifi::utils::hash_combine(hash_value, std::hash{}(asset.id)); } return std::to_string(hash_value); } @@ -113,11 +126,13 @@ class C2HeartbeatHandler : public HeartbeatHandler { std::lock_guard guard{asset_mtx_}; rapidjson::Document doc; doc.SetObject(); + doc.AddMember(rapidjson::StringRef("digest"), rapidjson::Value{calculateAssetHash(), doc.GetAllocator()}, doc.GetAllocator()); + doc.AddMember(rapidjson::StringRef("assets"), rapidjson::Value{rapidjson::kObjectType}, doc.GetAllocator()); for (auto& asset : expected_assets_) { auto path_str = asset.path.string(); - doc.AddMember(rapidjson::StringRef(asset.id), rapidjson::kObjectType, doc.GetAllocator()); - doc[asset.id].AddMember(rapidjson::StringRef("path"), rapidjson::Value(path_str, doc.GetAllocator()), doc.GetAllocator()); - doc[asset.id].AddMember(rapidjson::StringRef("url"), rapidjson::StringRef(asset.url), doc.GetAllocator()); + doc["assets"].AddMember(rapidjson::StringRef(asset.id), rapidjson::kObjectType, doc.GetAllocator()); + doc["assets"][asset.id].AddMember(rapidjson::StringRef("path"), rapidjson::Value(path_str, doc.GetAllocator()), doc.GetAllocator()); + doc["assets"][asset.id].AddMember(rapidjson::StringRef("url"), rapidjson::StringRef(asset.url), doc.GetAllocator()); } rapidjson::StringBuffer buffer; rapidjson::Writer writer(buffer); @@ -156,7 +171,7 @@ class VerifyC2AssetSync : public VerifyC2Base { std::function verify_; }; -int main() { +TEST_CASE("C2AssetSync", "[c2test]") { TestController controller; // setup minifi home @@ -193,14 +208,18 @@ int main() { auto get_asset_structure = [&] () { std::unordered_map contents; - for (auto& [dir, file] : utils::file::list_dir_all(asset_dir.string(), controller.getLogger())) { - contents[(dir / file).string()] = utils::file::get_content(dir / file); + for (auto& [dir, file] : minifi::utils::file::list_dir_all(asset_dir.string(), controller.getLogger())) { + contents[(dir / file).string()] = minifi::utils::file::get_content(dir / file); } return contents; }; harness.setVerifier([&] () { - assert(utils::verifyEventHappenedInPollTime(10s, [&] {return hb_handler.calculateAssetHash() == hb_handler.getAgentAssetHash();})); + REQUIRE(utils::verifyEventHappenedInPollTime(10s, [&] { + std::cout << "calculated hash = " << hb_handler.calculateAssetHash() << std::endl; + std::cout << "reported hash = " << hb_handler.getAgentAssetHash().value_or("") << std::endl; + return hb_handler.calculateAssetHash() == hb_handler.getAgentAssetHash(); + })); { std::unordered_map expected_assets{ @@ -218,7 +237,7 @@ int main() { for (auto& [path, content] : actual_assets) { controller.getLogger()->log_error("Actual asset at {}: {}", path, content); } - assert(false); + REQUIRE(false); } } @@ -227,7 +246,7 @@ int main() { hb_handler.addAsset("Av2", "A.txt", "/api/file/Av2.txt"); - assert(utils::verifyEventHappenedInPollTime(10s, [&] {return hb_handler.calculateAssetHash() == hb_handler.getAgentAssetHash();})); + REQUIRE(utils::verifyEventHappenedInPollTime(10s, [&] {return hb_handler.calculateAssetHash() == hb_handler.getAgentAssetHash();})); { std::unordered_map expected_assets{ @@ -245,7 +264,7 @@ int main() { for (auto& [path, content] : actual_assets) { controller.getLogger()->log_error("Actual asset at {}: {}", path, content); } - assert(false); + REQUIRE(false); } } }); @@ -254,3 +273,5 @@ int main() { std::filesystem::current_path(minifi::utils::file::get_executable_dir()); } + +} // namespace org::apache::nifi::minifi::test diff --git a/libminifi/test/integration/C2MetricsTest.cpp b/libminifi/test/integration/C2MetricsTest.cpp index 4d356fc2bb..392afaa256 100644 --- a/libminifi/test/integration/C2MetricsTest.cpp +++ b/libminifi/test/integration/C2MetricsTest.cpp @@ -62,7 +62,7 @@ class MetricsHandler: public HeartbeatHandler { explicit MetricsHandler(std::atomic_bool& metrics_updated_successfully, std::shared_ptr configuration, const std::filesystem::path& replacement_config_path) : HeartbeatHandler(std::move(configuration)), metrics_updated_successfully_(metrics_updated_successfully), - replacement_config_(getReplacementConfigAsJsonValue(replacement_config_path.string())) { + replacement_config_(getReplacementConfig(replacement_config_path.string())) { } void handleHeartbeat(const rapidjson::Document& root, struct mg_connection* conn) override { @@ -178,12 +178,9 @@ class MetricsHandler: public HeartbeatHandler { processor_metrics["GetTCPMetrics"][GETTCP1_UUID].HasMember("TransferredBytes"); } - [[nodiscard]] static std::string getReplacementConfigAsJsonValue(const std::string& replacement_config_path) { + [[nodiscard]] static std::string getReplacementConfig(const std::string& replacement_config_path) { std::ifstream is(replacement_config_path); - auto content = std::string((std::istreambuf_iterator(is)), std::istreambuf_iterator()); - content = minifi::utils::string::replaceAll(content, "\n", "\\n"); - content = minifi::utils::string::replaceAll(content, "\"", "\\\""); - return content; + return std::string((std::istreambuf_iterator(is)), std::istreambuf_iterator()); } std::atomic_bool& metrics_updated_successfully_; diff --git a/libminifi/test/libtest/integration/HTTPHandlers.cpp b/libminifi/test/libtest/integration/HTTPHandlers.cpp index eb357b2b99..aff494821c 100644 --- a/libminifi/test/libtest/integration/HTTPHandlers.cpp +++ b/libminifi/test/libtest/integration/HTTPHandlers.cpp @@ -235,41 +235,32 @@ bool DeleteTransactionResponder::handleDelete(CivetServer* /*server*/, struct mg } void HeartbeatHandler::sendHeartbeatResponse(const std::vector& operations, struct mg_connection * conn) { - std::string operation_jsons; + rapidjson::Document hb_obj{rapidjson::kObjectType}; + hb_obj.AddMember("operation", "heartbeat", hb_obj.GetAllocator()); + hb_obj.AddMember("requested_operations", rapidjson::kArrayType, hb_obj.GetAllocator()); for (const auto& c2_operation : operations) { - std::string resp_args; + rapidjson::Value op{rapidjson::kObjectType}; + op.AddMember("operation", c2_operation.operation, hb_obj.GetAllocator()); + op.AddMember("operationid", c2_operation.operation_id, hb_obj.GetAllocator()); + op.AddMember("operand", c2_operation.operand, hb_obj.GetAllocator()); if (!c2_operation.args.empty()) { - resp_args = ", \"args\": {"; - auto it = c2_operation.args.begin(); - while (it != c2_operation.args.end()) { - resp_args += "\"" + it->first + "\": \"" + it->second + "\""; - ++it; - if (it != c2_operation.args.end()) { - resp_args += ", "; - } + rapidjson::Value args{rapidjson::kObjectType}; + for (auto& [arg_name, arg_val] : c2_operation.args) { + args.AddMember(rapidjson::StringRef(arg_name), arg_val, hb_obj.GetAllocator()); } - resp_args += "}"; - } - - std::string operation_json = "{" - "\"operation\" : \"" + c2_operation.operation + "\"," - "\"operationid\" : \"" + c2_operation.operation_id + "\"," - "\"operand\": \"" + c2_operation.operand + "\"" + - resp_args + "}"; - - if (operation_jsons.empty()) { - operation_jsons += operation_json; - } else { - operation_jsons += ", " + operation_json; + op.AddMember("args", args, hb_obj.GetAllocator()); } + hb_obj["requested_operations"].PushBack(op, hb_obj.GetAllocator()); } - std::string heartbeat_response = "{\"operation\" : \"heartbeat\",\"requested_operations\": [ " + operation_jsons + " ]}"; + rapidjson::StringBuffer buffer; + rapidjson::Writer writer(buffer); + hb_obj.Accept(writer); mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: " "text/plain\r\nContent-Length: %lu\r\nConnection: close\r\n\r\n", - heartbeat_response.length()); - mg_printf(conn, "%s", heartbeat_response.c_str()); + buffer.GetLength()); + mg_printf(conn, "%s", buffer.GetString()); } void HeartbeatHandler::verifyJsonHasAgentManifest(const rapidjson::Document& root, const std::vector& verify_components, const std::vector& disallowed_properties) { diff --git a/libminifi/test/libtest/integration/IntegrationBase.cpp b/libminifi/test/libtest/integration/IntegrationBase.cpp index 36efc0ceea..19d92f1dd9 100644 --- a/libminifi/test/libtest/integration/IntegrationBase.cpp +++ b/libminifi/test/libtest/integration/IntegrationBase.cpp @@ -23,6 +23,7 @@ #include "utils/HTTPUtils.h" #include "unit/ProvenanceTestHelper.h" #include "utils/FifoExecutor.h" +#include "utils/file/AssetManager.h" #include "core/ConfigurationFactory.h" namespace org::apache::nifi::minifi::test { @@ -117,9 +118,10 @@ void IntegrationBase::run(const std::optional& test_file_ }; std::vector> repo_metric_sources{test_repo, test_flow_repo, content_repo}; - auto metrics_publisher_store = std::make_unique(configuration, repo_metric_sources, flow_config); + auto asset_manager = std::make_shared(*configuration); + auto metrics_publisher_store = std::make_unique(configuration, repo_metric_sources, flow_config, asset_manager); flowController_ = std::make_unique(test_repo, test_flow_repo, configuration, - std::move(flow_config), content_repo, std::move(metrics_publisher_store), filesystem, request_restart); + std::move(flow_config), content_repo, std::move(metrics_publisher_store), filesystem, request_restart, asset_manager); flowController_->load(); updateProperties(*flowController_); flowController_->start(); From 14f00ed22c53ed4b16c249d9e8cc4e887e91cc45 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 5 Jun 2024 13:52:33 +0200 Subject: [PATCH 21/48] MINIFICPP-2314 - Update c2 protocol --- libminifi/include/c2/C2Payload.h | 68 +++++++++++++++---- libminifi/include/c2/PayloadParser.h | 27 +++----- libminifi/include/c2/PayloadSerializer.h | 6 +- libminifi/src/c2/C2Agent.cpp | 55 +++++++-------- libminifi/src/c2/C2Payload.cpp | 14 ++-- libminifi/src/c2/HeartbeatJsonSerializer.cpp | 13 +++- libminifi/src/c2/protocols/RESTProtocol.cpp | 21 +----- .../test/integration/C2AssetSyncTest.cpp | 25 +++---- .../test/integration/C2UpdateAssetTest.cpp | 6 +- .../test/libtest/integration/HTTPHandlers.cpp | 8 ++- .../test/libtest/integration/HTTPHandlers.h | 4 +- 11 files changed, 140 insertions(+), 107 deletions(-) diff --git a/libminifi/include/c2/C2Payload.h b/libminifi/include/c2/C2Payload.h index 1329948522..6f75186492 100644 --- a/libminifi/include/c2/C2Payload.h +++ b/libminifi/include/c2/C2Payload.h @@ -29,6 +29,7 @@ #include "utils/Enum.h" #include "utils/gsl.h" #include "utils/span.h" +#include "rapidjson/document.h" namespace org::apache::nifi::minifi::c2 { @@ -84,21 +85,64 @@ enum Direction { RECEIVE }; -struct AnnotatedValue : state::response::ValueNode { - using state::response::ValueNode::ValueNode; - using state::response::ValueNode::operator=; +class C2Value { + public: + friend std::ostream& operator<<(std::ostream& out, const C2Value& val); + + C2Value() = default; + C2Value(const C2Value& other) { + (*this) = other; + } + C2Value(C2Value&&) = default; + template + requires(std::constructible_from) + C2Value(T&& value) { value_ = state::response::ValueNode{std::forward(value)}; } + C2Value(const rapidjson::Value& json_value) { + value_.emplace(); + get(value_).CopyFrom(json_value, get(value_).GetAllocator()); + } + + C2Value& operator=(const C2Value& other) { + if (auto* other_val_node = get_if(&other.value_)) { + value_ = *other_val_node; + } else { + value_.emplace(); + get(value_).CopyFrom(get(other.value_), get(value_).GetAllocator()); + } + return *this; + } + + C2Value& operator=(C2Value&&) = default; + - [[nodiscard]] std::optional> getAnnotation(const std::string& name) const { - auto it = annotations.find(name); - if (it == annotations.end()) { - return {}; + bool empty() const { + if (auto* val_node = get_if(&value_)) { + return val_node->empty(); } - return std::cref(it->second); + return false; } - friend std::ostream& operator<<(std::ostream& out, const AnnotatedValue& val); + std::string to_string() const { + if (auto* val_node = get_if(&value_)) { + return val_node->to_string(); + } + return std::string{get(value_).GetString(), get(value_).GetStringLength()}; + } + + const rapidjson::Document* json() const { + return get_if(&value_); + } + + const state::response::ValueNode* valueNode() const { + return get_if(&value_); + } + + bool operator==(const C2Value&) const = default; + + friend std::ostream& operator<<(std::ostream& out, const C2Value& val); - std::map annotations; + private: + std::variant value_; }; struct C2ContentResponse { @@ -120,7 +164,7 @@ struct C2ContentResponse { friend std::ostream& operator<<(std::ostream& out, const C2ContentResponse& response); - std::optional getArgument(const std::string& arg_name) const { + std::optional getStringArgument(const std::string& arg_name) const { if (auto it = operation_arguments.find(arg_name); it != operation_arguments.end()) { return it->second.to_string(); } @@ -139,7 +183,7 @@ struct C2ContentResponse { // name applied to commands std::string name; // commands that correspond with the operation. - std::map operation_arguments; + std::map operation_arguments; }; /** diff --git a/libminifi/include/c2/PayloadParser.h b/libminifi/include/c2/PayloadParser.h index d6a97642bc..01b70a58ed 100644 --- a/libminifi/include/c2/PayloadParser.h +++ b/libminifi/include/c2/PayloadParser.h @@ -138,27 +138,20 @@ class PayloadParser { } template - inline T getAs(const std::string &field) { + inline T getAs(const std::string &field, const std::optional& fallback = std::nullopt) { for (const auto &cmd : ref_.getContent()) { - auto exists = cmd.operation_arguments.find(field); - if (exists != cmd.operation_arguments.end()) { - return convert_if(exists->second.getValue())(); + if (auto it = cmd.operation_arguments.find(field); it != cmd.operation_arguments.end()) { + if (auto* val_node = it->second.valueNode()) { + return convert_if(val_node->getValue())(); + } } } - std::stringstream ss; - ss << "Invalid Field. Could not find " << field << " in " << ref_.getLabel(); - throw PayloadParseException(ss.str()); - } - - template - inline T getAs(const std::string &field, const T &fallback) { - for (const auto &cmd : ref_.getContent()) { - auto exists = cmd.operation_arguments.find(field); - if (exists != cmd.operation_arguments.end()) { - return convert_if(exists->second.getValue())(); - } + if (!fallback) { + std::stringstream ss; + ss << "Invalid Field. Could not find " << field << " in " << ref_.getLabel(); + throw PayloadParseException(ss.str()); } - return fallback; + return fallback.value(); } size_t getSize() const { diff --git a/libminifi/include/c2/PayloadSerializer.h b/libminifi/include/c2/PayloadSerializer.h index 17de977927..92fa793899 100644 --- a/libminifi/include/c2/PayloadSerializer.h +++ b/libminifi/include/c2/PayloadSerializer.h @@ -42,7 +42,7 @@ class PayloadSerializer { /** * Static function that serializes the value nodes */ - static void serializeValueNode(state::response::ValueNode &value, std::shared_ptr stream) { + static void serializeValueNode(const state::response::ValueNode &value, std::shared_ptr stream) { auto base_type = value.getValue(); if (!base_type) { uint8_t type = 0; @@ -95,7 +95,7 @@ class PayloadSerializer { stream->write(size); for (auto content : payload_content.operation_arguments) { stream->write(content.first); - serializeValueNode(content.second, stream); + serializeValueNode(*gsl::not_null(content.second.valueNode()), stream); } } if (nested_payload.getNestedPayloads().size() > 0) { @@ -170,7 +170,7 @@ class PayloadSerializer { stream->write(size); for (auto content : payload_content.operation_arguments) { stream->write(content.first); - serializeValueNode(content.second, stream); + serializeValueNode(*gsl::not_null(content.second.valueNode()), stream); } } serialize(op, payload, stream); diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index 2cf710384c..76c3fbb435 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -615,12 +615,15 @@ void C2Agent::handlePropertyUpdate(const C2ContentResponse &resp) { }; for (const auto& [name, value] : resp.operation_arguments) { - bool persist = ( - value.getAnnotation("persist") - | utils::transform(&AnnotatedValue::to_string) - | utils::andThen(utils::string::toBool)).value_or(true); - PropertyChangeLifetime lifetime = persist ? PropertyChangeLifetime::PERSISTENT : PropertyChangeLifetime::TRANSIENT; - changeUpdateState(update_property(name, value.to_string(), lifetime)); + if (auto* json_val = value.json()) { + if (json_val->IsObject() && json_val->HasMember("persist")) { + auto lifetime = (*json_val)["persist"].GetBool() ? PropertyChangeLifetime::PERSISTENT : PropertyChangeLifetime::TRANSIENT; + std::string property_value{(*json_val)["value"].GetString(), (*json_val)["value"].GetStringLength()}; + changeUpdateState(update_property(name, property_value, lifetime)); + continue; + } + } + changeUpdateState(update_property(name, value.to_string(), PropertyChangeLifetime::PERSISTENT)); } // apply changes and persist properties requested to be persisted const bool propertyWasUpdated = result == state::UpdateState::FULLY_APPLIED || result == state::UpdateState::PARTIALLY_APPLIED; @@ -739,28 +742,27 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse return; } - rapidjson::Document state_doc; - rapidjson::ParseResult res = state_doc.Parse(state_it->second.to_string()); - if (res.IsError()) { - send_error(fmt::format("Malformed request, 'globalHash' is not a valid json document: {} at {}", rapidjson::GetParseError_En(res.Code()), res.Offset())); + const rapidjson::Document* state_doc = state_it->second.json(); + if (!state_doc) { + send_error("Argument 'globalHash' is malformed"); return; } - if (!state_doc.IsObject()) { - send_error("Malformed request, 'globalHash' is not a json object"); + if (!state_doc->IsObject()) { + send_error("Malformed request, 'globalHash' is not an object"); return; } - if (!state_doc.HasMember("digest")) { + if (!state_doc->HasMember("digest")) { send_error("Malformed request, 'globalHash' has no member 'digest'"); return; } - if (!state_doc["digest"].IsString()) { + if (!(*state_doc)["digest"].IsString()) { send_error("Malformed request, 'globalHash.digest' is not a string"); return; } - asset_layout.digest = std::string{state_doc["digest"].GetString(), state_doc["digest"].GetStringLength()}; + asset_layout.digest = std::string{(*state_doc)["digest"].GetString(), (*state_doc)["digest"].GetStringLength()}; auto resource_list_it = resp.operation_arguments.find("resourceList"); if (resource_list_it == resp.operation_arguments.end()) { @@ -768,21 +770,20 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse return; } - rapidjson::Document resource_list; - res = resource_list.Parse(resource_list_it->second.to_string()); - if (res.IsError()) { - send_error(fmt::format("Malformed request, 'resourceList' is not a valid json document: {} at {}", rapidjson::GetParseError_En(res.Code()), res.Offset())); + const rapidjson::Document* resource_list = resource_list_it->second.json(); + if (!resource_list) { + send_error("Argument 'resourceList' is malformed"); return; } - if (!resource_list.IsArray()) { - send_error("Malformed request, 'resourceList' is not a json array"); + if (!resource_list->IsArray()) { + send_error("Malformed request, 'resourceList' is not an array"); return; } - for (size_t resource_idx = 0; resource_idx < resource_list.Size(); ++resource_idx) { - auto& resource = resource_list.GetArray()[resource_idx]; + for (size_t resource_idx = 0; resource_idx < resource_list->Size(); ++resource_idx) { + auto& resource = resource_list->GetArray()[resource_idx]; if (!resource.IsObject()) { - send_error(fmt::format("Malformed request, 'resourceList[{}]' is not a json object", resource_idx)); + send_error(fmt::format("Malformed request, 'resourceList[{}]' is not an object", resource_idx)); return; } auto get_member_str = [&] (const char* key) -> nonstd::expected { @@ -1063,7 +1064,7 @@ void C2Agent::handleAssetUpdate(const C2ContentResponse& resp) { // output file std::filesystem::path file_path; - if (auto file_rel = resp.getArgument("file") | utils::transform(make_path)) { + if (auto file_rel = resp.getStringArgument("file") | utils::transform(make_path)) { auto result = utils::file::validateRelativePath(file_rel.value()); if (!result) { send_error(result.error()); @@ -1077,7 +1078,7 @@ void C2Agent::handleAssetUpdate(const C2ContentResponse& resp) { // source url std::string url; - if (auto url_str = resp.getArgument("url")) { + if (auto url_str = resp.getStringArgument("url")) { if (auto resolved_url = resolveUrl(*url_str)) { url = resolved_url.value(); } else { @@ -1091,7 +1092,7 @@ void C2Agent::handleAssetUpdate(const C2ContentResponse& resp) { // forceDownload bool force_download = false; - if (auto force_download_str = resp.getArgument("forceDownload")) { + if (auto force_download_str = resp.getStringArgument("forceDownload")) { if (utils::string::equalsIgnoreCase(force_download_str.value(), "true")) { force_download = true; } else if (utils::string::equalsIgnoreCase(force_download_str.value(), "false")) { diff --git a/libminifi/src/c2/C2Payload.cpp b/libminifi/src/c2/C2Payload.cpp index e70413cd54..f21f60437b 100644 --- a/libminifi/src/c2/C2Payload.cpp +++ b/libminifi/src/c2/C2Payload.cpp @@ -131,14 +131,14 @@ std::ostream& operator<<(std::ostream& out, const C2ContentResponse& response) { << "}"; } -std::ostream& operator<<(std::ostream& out, const AnnotatedValue& val) { - if (val.value_) { - out << '"' << val.value_->c_str() << '"'; +std::ostream& operator<<(std::ostream& out, const C2Value& val) { + if (auto* val_ptr = val.valueNode()) { + out << '"' << val_ptr->to_string() << '"'; } else { - out << ""; - } - if (!val.annotations.empty()) { - out << val.annotations; + rapidjson::StringBuffer buffer; + rapidjson::Writer writer(buffer); + gsl::not_null(val.json())->Accept(writer); + out << std::string_view{buffer.GetString(), buffer.GetLength()}; } return out; } diff --git a/libminifi/src/c2/HeartbeatJsonSerializer.cpp b/libminifi/src/c2/HeartbeatJsonSerializer.cpp index d15ecf71a9..6237a3a20d 100644 --- a/libminifi/src/c2/HeartbeatJsonSerializer.cpp +++ b/libminifi/src/c2/HeartbeatJsonSerializer.cpp @@ -60,9 +60,16 @@ static void serializeOperationInfo(rapidjson::Value& target, const C2Payload& pa target.AddMember("identifier", rapidjson::Value(id.c_str(), alloc), alloc); } -static void setJsonStr(const std::string& key, const state::response::ValueNode& value, rapidjson::Value& parent, rapidjson::Document::AllocatorType& alloc) { // NOLINT +static void setJsonStr(const std::string& key, const c2::C2Value& value, rapidjson::Value& parent, rapidjson::Document::AllocatorType& alloc) { // NOLINT rapidjson::Value valueVal; - auto base_type = value.getValue(); + + if (auto* json_val = value.json()) { + valueVal.CopyFrom(*json_val, alloc); + parent.AddMember(rapidjson::Value(key.c_str(), alloc), valueVal, alloc); + return; + } + + auto base_type = gsl::not_null(value.valueNode())->getValue(); auto type_index = base_type->getTypeIndex(); if (auto sub_type = std::dynamic_pointer_cast(base_type)) { @@ -156,7 +163,7 @@ static rapidjson::Value serializeConnectionQueues(const C2Payload& payload, std: updatedContent.name = uuid; adjusted.setLabel(uuid); adjusted.setIdentifier(uuid); - c2::AnnotatedValue nd; + c2::C2Value nd; // name should be what was previously the TLN ( top level node ) nd = name; updatedContent.operation_arguments.insert(std::make_pair("name", nd)); diff --git a/libminifi/src/c2/protocols/RESTProtocol.cpp b/libminifi/src/c2/protocols/RESTProtocol.cpp index db389778bc..a218752dc2 100644 --- a/libminifi/src/c2/protocols/RESTProtocol.cpp +++ b/libminifi/src/c2/protocols/RESTProtocol.cpp @@ -37,25 +37,6 @@ namespace org::apache::nifi::minifi::c2 { - -AnnotatedValue parseAnnotatedValue(const rapidjson::Value& jsonValue) { - AnnotatedValue result; - if (jsonValue.IsObject() && jsonValue.HasMember("value")) { - result = jsonValue["value"].GetString(); - for (const auto& annotation : jsonValue.GetObject()) { - if (annotation.name.GetString() == std::string("value")) { - continue; - } - result.annotations[annotation.name.GetString()] = parseAnnotatedValue(annotation.value); - } - } else if (jsonValue.IsBool()) { - result = jsonValue.GetBool(); - } else { - result = std::string{jsonValue.GetString(), jsonValue.GetStringLength()}; - } - return result; -} - C2Payload RESTProtocol::parseJsonResponse(const C2Payload &payload, std::span response) { rapidjson::Document root; @@ -124,7 +105,7 @@ C2Payload RESTProtocol::parseJsonResponse(const C2Payload &payload, std::span args; - args["globalHash"] = R"({"digest": ")" + calculateAssetHash() + "\"}"; - args["resourceList"] = "["; + std::unordered_map args; + rapidjson::Document global_hash_doc{rapidjson::kObjectType}; + global_hash_doc.AddMember("digest", calculateAssetHash(), global_hash_doc.GetAllocator()); + args["globalHash"] = std::move(global_hash_doc); + rapidjson::Document resource_list_doc{rapidjson::kArrayType}; for (auto& asset : expected_assets_) { - if (args["resourceList"] != "[") args["resourceList"] += ", "; - args["resourceList"] += "{"; - args["resourceList"] += "\"resourceId\": \"" + asset.id + "\", "; - args["resourceList"] += "\"resourceName\": \"" + asset.path.filename().string() + "\", "; - args["resourceList"] += "\"resourceType\": \"ASSET\", "; - args["resourceList"] += "\"resourcePath\": \"" + asset.path.parent_path().string() + "\", "; - args["resourceList"] += "\"url\": \"" + asset.url + "\""; - args["resourceList"] += "}"; + rapidjson::Value resource_obj{rapidjson::kObjectType}; + resource_obj.AddMember("resourceId", asset.id, resource_list_doc.GetAllocator()); + resource_obj.AddMember("resourceName", asset.path.filename().string(), resource_list_doc.GetAllocator()); + resource_obj.AddMember("resourceType", "ASSET", resource_list_doc.GetAllocator()); + resource_obj.AddMember("resourcePath", asset.path.parent_path().string(), resource_list_doc.GetAllocator()); + resource_obj.AddMember("url", asset.url, resource_list_doc.GetAllocator()); + resource_list_doc.PushBack(resource_obj, resource_list_doc.GetAllocator()); } + args["resourceList"] = std::move(resource_list_doc); - args["resourceList"] += "]"; operations.push_back(C2Operation{ .operation = "sync", .operand = "resource", diff --git a/libminifi/test/integration/C2UpdateAssetTest.cpp b/libminifi/test/integration/C2UpdateAssetTest.cpp index af775fe21b..af67cf2db9 100644 --- a/libminifi/test/integration/C2UpdateAssetTest.cpp +++ b/libminifi/test/integration/C2UpdateAssetTest.cpp @@ -56,7 +56,7 @@ class C2HeartbeatHandler : public HeartbeatHandler { return true; } - void addOperation(std::string id, std::unordered_map args) { + void addOperation(std::string id, std::unordered_map args) { std::lock_guard guard(op_mtx_); operations_.push_back(C2Operation{ .operation = "update", @@ -92,7 +92,7 @@ class VerifyC2AssetUpdate : public VerifyC2Base { struct AssetUpdateOperation { std::string id; - std::unordered_map args; + std::unordered_map args; std::string state; std::optional details; }; @@ -244,7 +244,7 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { // this op failed no file made on the disk continue; } - expected_files[(asset_dir / op.args["file"]).string()] = minifi::utils::string::endsWith(op.args["url"], "A.txt") ? file_A : file_B; + expected_files[(asset_dir / op.args["file"].to_string()).string()] = minifi::utils::string::endsWith(op.args["url"].to_string(), "A.txt") ? file_A : file_B; } size_t file_count = minifi::utils::file::list_dir_all(asset_dir.string(), controller.getLogger()).size(); diff --git a/libminifi/test/libtest/integration/HTTPHandlers.cpp b/libminifi/test/libtest/integration/HTTPHandlers.cpp index aff494821c..dd268bb642 100644 --- a/libminifi/test/libtest/integration/HTTPHandlers.cpp +++ b/libminifi/test/libtest/integration/HTTPHandlers.cpp @@ -246,7 +246,13 @@ void HeartbeatHandler::sendHeartbeatResponse(const std::vector& ope if (!c2_operation.args.empty()) { rapidjson::Value args{rapidjson::kObjectType}; for (auto& [arg_name, arg_val] : c2_operation.args) { - args.AddMember(rapidjson::StringRef(arg_name), arg_val, hb_obj.GetAllocator()); + rapidjson::Value json_arg_val; + if (auto* json_val = arg_val.json()) { + json_arg_val.CopyFrom(*json_val, hb_obj.GetAllocator()); + } else { + json_arg_val.SetString(arg_val.to_string(), hb_obj.GetAllocator()); + } + args.AddMember(rapidjson::StringRef(arg_name), json_arg_val, hb_obj.GetAllocator()); } op.AddMember("args", args, hb_obj.GetAllocator()); } diff --git a/libminifi/test/libtest/integration/HTTPHandlers.h b/libminifi/test/libtest/integration/HTTPHandlers.h index a47d2a5061..1b6857f0ec 100644 --- a/libminifi/test/libtest/integration/HTTPHandlers.h +++ b/libminifi/test/libtest/integration/HTTPHandlers.h @@ -214,11 +214,11 @@ class HeartbeatHandler : public ServerAwareHandler { std::string operation; std::string operand; std::string operation_id; - std::unordered_map args; + std::unordered_map args; }; void sendHeartbeatResponse(const std::string& operation, const std::string& operand, const std::string& operation_id, struct mg_connection* conn, - const std::unordered_map& args = {}) { + const std::unordered_map& args = {}) { sendHeartbeatResponse({{operation, operand, operation_id, args}}, conn); } From c387d6a36c15908b07f955e1058c67683423cf66 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 5 Jun 2024 15:08:59 +0200 Subject: [PATCH 22/48] MINIFICPP-2314 - Linter --- libminifi/include/c2/C2Payload.h | 4 ++-- libminifi/src/c2/C2Agent.cpp | 2 +- libminifi/src/c2/protocols/RESTProtocol.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libminifi/include/c2/C2Payload.h b/libminifi/include/c2/C2Payload.h index 6f75186492..f337d335b2 100644 --- a/libminifi/include/c2/C2Payload.h +++ b/libminifi/include/c2/C2Payload.h @@ -96,8 +96,8 @@ class C2Value { C2Value(C2Value&&) = default; template requires(std::constructible_from) - C2Value(T&& value) { value_ = state::response::ValueNode{std::forward(value)}; } - C2Value(const rapidjson::Value& json_value) { + C2Value(T&& value) { value_ = state::response::ValueNode{std::forward(value)}; } // NOLINT(runtime/explicit) + C2Value(const rapidjson::Value& json_value) { // NOLINT(runtime/explicit) value_.emplace(); get(value_).CopyFrom(json_value, get(value_).GetAllocator()); } diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index 76c3fbb435..d80858534b 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -824,7 +824,7 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse return; } - auto full_path = std::filesystem::path{path.value()} / name.value(); + auto full_path = std::filesystem::path{path.value()} / name.value(); // NOLINT(whitespace/braces) auto path_valid = utils::file::validateRelativePath(full_path); if (!path_valid) { diff --git a/libminifi/src/c2/protocols/RESTProtocol.cpp b/libminifi/src/c2/protocols/RESTProtocol.cpp index a218752dc2..57b42cfcb2 100644 --- a/libminifi/src/c2/protocols/RESTProtocol.cpp +++ b/libminifi/src/c2/protocols/RESTProtocol.cpp @@ -27,8 +27,8 @@ #include #include #include -#include +#include "rapidjson/error/en.h" #include "core/TypedValues.h" #include "utils/gsl.h" #include "properties/Configuration.h" From 1275e974c6c1e4872776f911548cc4a9480f163e Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 5 Jun 2024 15:31:33 +0200 Subject: [PATCH 23/48] MINIFICPP-2314 - Windows fix --- libminifi/src/utils/file/AssetManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index 7a639ecf8a..7642c4f1cd 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -149,7 +149,7 @@ void AssetManager::persist() const { for (auto& entry : state_.assets) { rapidjson::Value entry_val(rapidjson::kObjectType); - entry_val.AddMember(rapidjson::StringRef("path"), rapidjson::Value(entry.path.string().c_str(), doc.GetAllocator()), doc.GetAllocator()); + entry_val.AddMember(rapidjson::StringRef("path"), rapidjson::Value(entry.path.generic_string(), doc.GetAllocator()), doc.GetAllocator()); entry_val.AddMember(rapidjson::StringRef("url"), rapidjson::StringRef(entry.url), doc.GetAllocator()); doc["assets"].AddMember(rapidjson::StringRef(entry.id), entry_val, doc.GetAllocator()); } From e6948984b591957820b245584a0e3188c677b736 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 5 Jun 2024 16:23:56 +0200 Subject: [PATCH 24/48] MINIFICPP-2314 - Linter fix --- libminifi/src/utils/file/AssetManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index 7642c4f1cd..21b74a3d44 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -39,7 +39,7 @@ void AssetManager::refreshState() { std::filesystem::create_directory(root_); } if (!utils::file::FileUtils::exists(root_ / ".state")) { - std::ofstream{root_ / ".state", std::ios::binary} << "{\"digest\": \"\", \"assets\": {}}"; + std::ofstream{root_ / ".state", std::ios::binary} << R"({"digest": "", "assets": {}})"; } rapidjson::Document doc; From 104028ee143446b1a1744a604b2a45d084644e23 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Thu, 6 Jun 2024 09:02:23 +0200 Subject: [PATCH 25/48] MINIFICPP-2314 - Clang tidy fix --- libminifi/include/c2/C2Payload.h | 3 +++ libminifi/test/integration/C2MetricsTest.cpp | 2 +- libminifi/test/libtest/integration/HTTPHandlers.cpp | 11 ++++++----- libminifi/test/libtest/integration/HTTPHandlers.h | 4 ++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/libminifi/include/c2/C2Payload.h b/libminifi/include/c2/C2Payload.h index f337d335b2..487d68a545 100644 --- a/libminifi/include/c2/C2Payload.h +++ b/libminifi/include/c2/C2Payload.h @@ -101,6 +101,9 @@ class C2Value { value_.emplace(); get(value_).CopyFrom(json_value, get(value_).GetAllocator()); } + C2Value(rapidjson::Document&& json_doc) { + value_ = std::move(json_doc); + } C2Value& operator=(const C2Value& other) { if (auto* other_val_node = get_if(&other.value_)) { diff --git a/libminifi/test/integration/C2MetricsTest.cpp b/libminifi/test/integration/C2MetricsTest.cpp index 392afaa256..c62ee0cc91 100644 --- a/libminifi/test/integration/C2MetricsTest.cpp +++ b/libminifi/test/integration/C2MetricsTest.cpp @@ -180,7 +180,7 @@ class MetricsHandler: public HeartbeatHandler { [[nodiscard]] static std::string getReplacementConfig(const std::string& replacement_config_path) { std::ifstream is(replacement_config_path); - return std::string((std::istreambuf_iterator(is)), std::istreambuf_iterator()); + return {(std::istreambuf_iterator(is)), std::istreambuf_iterator()}; } std::atomic_bool& metrics_updated_successfully_; diff --git a/libminifi/test/libtest/integration/HTTPHandlers.cpp b/libminifi/test/libtest/integration/HTTPHandlers.cpp index dd268bb642..31578022ac 100644 --- a/libminifi/test/libtest/integration/HTTPHandlers.cpp +++ b/libminifi/test/libtest/integration/HTTPHandlers.cpp @@ -56,7 +56,7 @@ bool PeerResponder::handleGet(CivetServer* /*server*/, struct mg_connection *con #else std::string hostname = "localhost"; #endif - std::string site2site_rest_resp = "{\"peers\" : [{ \"hostname\": \"" + hostname + "\", \"port\": " + port + ", \"secure\": false, \"flowFileCount\" : 0 }] }"; + std::string site2site_rest_resp = R"({"peers" : [{ "hostname": ")" + hostname + R"(", "port": )" + port + R"(, "secure": false, "flowFileCount" : 0 }] })"; std::stringstream headers; headers << "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: " << site2site_rest_resp.length() << "\r\nConnection: close\r\n\r\n"; mg_printf(conn, "%s", headers.str().c_str()); @@ -134,7 +134,8 @@ bool FlowFileResponder::handlePost(CivetServer* /*server*/, struct mg_connection const auto flow = std::make_shared(); for (uint32_t i = 0; i < num_attributes; i++) { - std::string name, value; + std::string name; + std::string value; { const auto read = stream.read(name, true); if (!isServerRunning()) return false; @@ -204,7 +205,7 @@ bool FlowFileResponder::handleGet(CivetServer* /*server*/, struct mg_connection minifi::io::BufferStream serializer; minifi::io::CRCStream stream(gsl::make_not_null(&serializer)); for (const auto& flow : flows) { - uint32_t num_attributes = gsl::narrow(flow->attributes.size()); + auto num_attributes = gsl::narrow(flow->attributes.size()); stream.write(num_attributes); for (const auto& entry : flow->attributes) { stream.write(entry.first); @@ -474,9 +475,9 @@ bool C2UpdateHandler::handlePost(CivetServer* /*server*/, struct mg_connection * } void C2UpdateHandler::setC2RestResponse(const std::string& url, const std::string& name, const std::optional& persist) { - std::string content = "{\"location\": \"" + url + "\""; + std::string content = R"({"location": ")" + url + "\""; if (persist) { - content += ", \"persist\": \"" + *persist + "\""; + content += R"(, "persist": ")" + *persist + "\""; } content += "}"; response_ = diff --git a/libminifi/test/libtest/integration/HTTPHandlers.h b/libminifi/test/libtest/integration/HTTPHandlers.h index 1b6857f0ec..c73395ed0c 100644 --- a/libminifi/test/libtest/integration/HTTPHandlers.h +++ b/libminifi/test/libtest/integration/HTTPHandlers.h @@ -217,12 +217,12 @@ class HeartbeatHandler : public ServerAwareHandler { std::unordered_map args; }; - void sendHeartbeatResponse(const std::string& operation, const std::string& operand, const std::string& operation_id, struct mg_connection* conn, + static void sendHeartbeatResponse(const std::string& operation, const std::string& operand, const std::string& operation_id, struct mg_connection* conn, const std::unordered_map& args = {}) { sendHeartbeatResponse({{operation, operand, operation_id, args}}, conn); } - void sendHeartbeatResponse(const std::vector& operations, struct mg_connection * conn); + static void sendHeartbeatResponse(const std::vector& operations, struct mg_connection * conn); void verifyJsonHasAgentManifest(const rapidjson::Document& root, const std::vector& verify_components = {}, const std::vector& disallowed_properties = {}); void verify(struct mg_connection *conn); From cb0332518fd966361e6ffdcb10c5e05cac9c7952 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Tue, 11 Jun 2024 09:38:40 +0200 Subject: [PATCH 26/48] MINIFICPP-2314 - Linter fix --- libminifi/include/c2/C2Payload.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/include/c2/C2Payload.h b/libminifi/include/c2/C2Payload.h index 487d68a545..03fc04d029 100644 --- a/libminifi/include/c2/C2Payload.h +++ b/libminifi/include/c2/C2Payload.h @@ -101,7 +101,7 @@ class C2Value { value_.emplace(); get(value_).CopyFrom(json_value, get(value_).GetAllocator()); } - C2Value(rapidjson::Document&& json_doc) { + C2Value(rapidjson::Document&& json_doc) { // NOLINT(runtime/explicit) value_ = std::move(json_doc); } From d5861e48eeb118713bc762f5f99b0f5b08328f96 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 19 Jun 2024 11:10:52 +0200 Subject: [PATCH 27/48] MINIFICPP-2314 - Add nifi.c2.rest.path.base agent property --- C2.md | 6 ++++-- conf/minifi.properties | 1 + .../include/core/state/nodes/AssetInformation.h | 2 +- libminifi/include/properties/Configuration.h | 1 + libminifi/src/Configuration.cpp | 1 + libminifi/src/c2/C2Agent.cpp | 3 +++ libminifi/src/c2/protocols/RESTSender.cpp | 13 +++++++++++++ .../core/state/nodes/SupportedOperations.cpp | 4 ++++ libminifi/src/utils/file/AssetManager.cpp | 17 +++++++++++++++-- libminifi/test/integration/C2AssetSyncTest.cpp | 2 +- 10 files changed, 44 insertions(+), 6 deletions(-) diff --git a/C2.md b/C2.md index e42fe6be4c..f43a8284d1 100644 --- a/C2.md +++ b/C2.md @@ -81,8 +81,10 @@ be requested via C2 DESCRIBE manifest command. nifi.c2.rest.listener.cacert= # specify the rest URIs if using RESTSender - nifi.c2.rest.url=http:////c2-protocol/heartbeat - nifi.c2.rest.url.ack=http:////c2-protocol/acknowledge + nifi.c2.rest.path.base=https:/// + # spicify either absolute url or relative to the nifi.c2.rest.path.base url for hearbeat and acknowledge + nifi.c2.rest.url=/c2-protocol/heartbeat + nifi.c2.rest.url.ack=/c2-protocol/acknowledge nifi.c2.flow.base.url=http:////c2-protocol/ # c2 agent identifier -- must be defined to run agent diff --git a/conf/minifi.properties b/conf/minifi.properties index debf11e26d..425e6e64b0 100644 --- a/conf/minifi.properties +++ b/conf/minifi.properties @@ -84,6 +84,7 @@ nifi.content.repository.class.name=DatabaseContentRepository ## base URL of the c2 server, ## very likely the same base url of rest urls #nifi.c2.flow.base.url= +#nifi.c2.rest.path.base= #nifi.c2.rest.url= #nifi.c2.rest.url.ack= #nifi.c2.rest.ssl.context.service= diff --git a/libminifi/include/core/state/nodes/AssetInformation.h b/libminifi/include/core/state/nodes/AssetInformation.h index e1af964586..39fd578fa9 100644 --- a/libminifi/include/core/state/nodes/AssetInformation.h +++ b/libminifi/include/core/state/nodes/AssetInformation.h @@ -31,7 +31,7 @@ class AssetInformation : public ResponseNode { void setAssetManager(std::shared_ptr asset_manager); - std::string getName() const override { return "assetInfo"; } + std::string getName() const override { return "resourceInfo"; } std::vector serialize() override; private: diff --git a/libminifi/include/properties/Configuration.h b/libminifi/include/properties/Configuration.h index 2d9300636a..cea438ea92 100644 --- a/libminifi/include/properties/Configuration.h +++ b/libminifi/include/properties/Configuration.h @@ -118,6 +118,7 @@ class Configuration : public Properties { static constexpr const char *nifi_c2_root_class_definitions = "nifi.c2.root.class.definitions"; static constexpr const char *nifi_c2_rest_listener_port = "nifi.c2.rest.listener.port"; static constexpr const char *nifi_c2_rest_listener_cacert = "nifi.c2.rest.listener.cacert"; + static constexpr const char *nifi_c2_rest_path_base = "nifi.c2.rest.path.base"; static constexpr const char *nifi_c2_rest_url = "nifi.c2.rest.url"; static constexpr const char *nifi_c2_rest_url_ack = "nifi.c2.rest.url.ack"; static constexpr const char *nifi_c2_rest_ssl_context_service = "nifi.c2.rest.ssl.context.service"; diff --git a/libminifi/src/Configuration.cpp b/libminifi/src/Configuration.cpp index 024393e57f..116746c9c5 100644 --- a/libminifi/src/Configuration.cpp +++ b/libminifi/src/Configuration.cpp @@ -88,6 +88,7 @@ const std::unordered_map C2Agent::resolveUrl(const std::string& url) const { return url; } std::string base; + if (configuration_->get(Configuration::nifi_c2_rest_path_base, "c2.rest.path.base", base)) { + return base + url; + } if (!configuration_->get(Configuration::nifi_c2_rest_url, "c2.rest.url", base)) { logger_->log_error("Missing C2 REST URL"); return std::nullopt; diff --git a/libminifi/src/c2/protocols/RESTSender.cpp b/libminifi/src/c2/protocols/RESTSender.cpp index 98642b009f..da58528580 100644 --- a/libminifi/src/c2/protocols/RESTSender.cpp +++ b/libminifi/src/c2/protocols/RESTSender.cpp @@ -41,10 +41,23 @@ void RESTSender::initialize(core::controller::ControllerServiceProvider* control RESTProtocol::initialize(controller, configure); // base URL when one is not specified. if (nullptr != configure) { + std::optional rest_base_path = configure->getWithFallback(Configuration::nifi_c2_rest_path_base, "c2.rest.path.base"); std::string update_str; std::string ssl_context_service_str; configure->get(Configuration::nifi_c2_rest_url, "c2.rest.url", rest_uri_); configure->get(Configuration::nifi_c2_rest_url_ack, "c2.rest.url.ack", ack_uri_); + if (rest_uri_.starts_with("/")) { + if (!rest_base_path) { + throw Exception(ExceptionType::GENERAL_EXCEPTION, "Cannot use relative nifi.c2.rest.url unless the nifi.c2.rest.path.base is set"); + } + rest_uri_ = rest_base_path.value() + rest_uri_; + } + if (ack_uri_.starts_with("/")) { + if (!rest_base_path) { + throw Exception(ExceptionType::GENERAL_EXCEPTION, "Cannot use relative nifi.c2.rest.url.ack unless the nifi.c2.rest.path.base is set"); + } + ack_uri_ = rest_base_path.value() + ack_uri_; + } if (controller && configure->get(Configuration::nifi_c2_rest_ssl_context_service, "c2.rest.ssl.context.service", ssl_context_service_str)) { if (auto service = controller->getControllerService(ssl_context_service_str)) { ssl_context_service_ = std::static_pointer_cast(service); diff --git a/libminifi/src/core/state/nodes/SupportedOperations.cpp b/libminifi/src/core/state/nodes/SupportedOperations.cpp index b0681415fa..51f86c40b0 100644 --- a/libminifi/src/core/state/nodes/SupportedOperations.cpp +++ b/libminifi/src/core/state/nodes/SupportedOperations.cpp @@ -110,6 +110,10 @@ void SupportedOperations::fillProperties(SerializedResponseNode& properties, min } break; } + case minifi::c2::Operation::sync: { + serializeProperty(properties); + break; + } default: break; } diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index 21b74a3d44..db8768169b 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -111,16 +111,24 @@ nonstd::expected AssetManager::sync( org::apache::nifi::minifi::utils::file::AssetLayout layout, const std::function, std::string>(std::string_view /*url*/)>& fetch) { std::lock_guard lock(mtx_); + org::apache::nifi::minifi::utils::file::AssetLayout new_state{ + .digest = state_.digest + }; + std::string fetch_errors; std::vector>> new_file_contents; for (auto& new_entry : layout.assets) { if (std::find_if(state_.assets.begin(), state_.assets.end(), [&] (auto& entry) {return entry.id == new_entry.id;}) == state_.assets.end()) { if (auto data = fetch(new_entry.url)) { new_file_contents.emplace_back(new_entry.path, data.value()); + new_state.assets.insert(new_entry); } else { - return nonstd::make_unexpected(data.error()); + fetch_errors += "Failed to fetch '" + new_entry.id + "' from '" + new_entry.url + "': " + data.error() + "\n"; } } } + if (fetch_errors.empty()) { + new_state.digest = layout.digest; + } for (auto& old_entry : state_.assets) { if (std::find_if(layout.assets.begin(), layout.assets.end(), [&] (auto& entry) {return entry.id == old_entry.id;}) == layout.assets.end()) { @@ -134,8 +142,13 @@ nonstd::expected AssetManager::sync( std::ofstream{root_ / path, std::ios::binary}.write(reinterpret_cast(content.data()), gsl::narrow(content.size())); } - state_ = std::move(layout); + state_ = std::move(new_state); persist(); + + if (!fetch_errors.empty()) { + return nonstd::make_unexpected(fetch_errors); + } + return {}; } diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index b8109db921..8b27bf8f23 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -58,7 +58,7 @@ class C2HeartbeatHandler : public HeartbeatHandler { root.Accept(writer); return std::string{buffer.GetString(), buffer.GetSize()}; }(); - auto& asset_info_node = root["assetInfo"]; + auto& asset_info_node = root["resourceInfo"]; auto& asset_hash_node = asset_info_node["hash"]; std::string asset_hash{asset_hash_node.GetString(), asset_hash_node.GetStringLength()}; From 5d5708eed5f79c97fd18a888281deb49fdd8deb6 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 19 Jun 2024 11:20:00 +0200 Subject: [PATCH 28/48] MINIFICPP-2314 - Fix test, log errors --- libminifi/src/utils/file/AssetManager.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index db8768169b..f4f88403d9 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -118,12 +118,17 @@ nonstd::expected AssetManager::sync( std::vector>> new_file_contents; for (auto& new_entry : layout.assets) { if (std::find_if(state_.assets.begin(), state_.assets.end(), [&] (auto& entry) {return entry.id == new_entry.id;}) == state_.assets.end()) { + logger_->log_debug("Fetching asset {} from {}", new_entry.id, new_entry.url); if (auto data = fetch(new_entry.url)) { new_file_contents.emplace_back(new_entry.path, data.value()); new_state.assets.insert(new_entry); } else { + logger_->log_error("Failed to fetch asset {} from {}: {}", new_entry.id, new_entry.url, data.error()); fetch_errors += "Failed to fetch '" + new_entry.id + "' from '" + new_entry.url + "': " + data.error() + "\n"; } + } else { + logger_->log_debug("Asset {} already exists", new_entry.id); + new_state.assets.insert(new_entry); } } if (fetch_errors.empty()) { From a6e3991f78ee2f58202f269b1c8192f0aca9b258 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Tue, 2 Jul 2024 09:05:48 +0200 Subject: [PATCH 29/48] MINIFICPP-2314 - Review fixes --- libminifi/include/c2/C2Payload.h | 2 -- libminifi/include/utils/file/PathUtils.h | 2 +- libminifi/src/c2/C2Agent.cpp | 14 ++++++++------ libminifi/src/c2/HeartbeatJsonSerializer.cpp | 2 +- libminifi/src/c2/protocols/RESTSender.cpp | 2 +- libminifi/src/utils/file/AssetManager.cpp | 10 +++++----- libminifi/test/integration/C2MetricsTest.cpp | 7 +------ 7 files changed, 17 insertions(+), 22 deletions(-) diff --git a/libminifi/include/c2/C2Payload.h b/libminifi/include/c2/C2Payload.h index 03fc04d029..57715a0490 100644 --- a/libminifi/include/c2/C2Payload.h +++ b/libminifi/include/c2/C2Payload.h @@ -142,8 +142,6 @@ class C2Value { bool operator==(const C2Value&) const = default; - friend std::ostream& operator<<(std::ostream& out, const C2Value& val); - private: std::variant value_; }; diff --git a/libminifi/include/utils/file/PathUtils.h b/libminifi/include/utils/file/PathUtils.h index f5cbb44d2a..04886c1a67 100644 --- a/libminifi/include/utils/file/PathUtils.h +++ b/libminifi/include/utils/file/PathUtils.h @@ -43,7 +43,7 @@ inline std::optional canonicalize(const std::filesystem:: return result; } -inline nonstd::expected validateRelativePath(const std::filesystem::path& path) { +inline nonstd::expected validateRelativeFilePath(const std::filesystem::path& path) { if (path.empty()) { return nonstd::make_unexpected("Empty file path"); } diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index ee1361120e..f3a41c635f 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -616,8 +616,11 @@ void C2Agent::handlePropertyUpdate(const C2ContentResponse &resp) { for (const auto& [name, value] : resp.operation_arguments) { if (auto* json_val = value.json()) { - if (json_val->IsObject() && json_val->HasMember("persist")) { - auto lifetime = (*json_val)["persist"].GetBool() ? PropertyChangeLifetime::PERSISTENT : PropertyChangeLifetime::TRANSIENT; + if (json_val->IsObject() && json_val->HasMember("value")) { + PropertyChangeLifetime lifetime = PropertyChangeLifetime::PERSISTENT; + if (json_val->HasMember("persist")) { + lifetime = (*json_val)["persist"].GetBool() ? PropertyChangeLifetime::PERSISTENT : PropertyChangeLifetime::TRANSIENT; + } std::string property_value{(*json_val)["value"].GetString(), (*json_val)["value"].GetStringLength()}; changeUpdateState(update_property(name, property_value, lifetime)); continue; @@ -733,7 +736,6 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse gsl_Assert(operand == SyncOperand::resource); - std::set ids; utils::file::AssetLayout asset_layout; auto state_it = resp.operation_arguments.find("globalHash"); @@ -826,7 +828,7 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse auto full_path = std::filesystem::path{path.value()} / name.value(); // NOLINT(whitespace/braces) - auto path_valid = utils::file::validateRelativePath(full_path); + auto path_valid = utils::file::validateRelativeFilePath(full_path); if (!path_valid) { send_error(path_valid.error()); return; @@ -952,7 +954,7 @@ std::optional C2Agent::resolveUrl(const std::string& url) const { return url; } std::string base; - if (configuration_->get(Configuration::nifi_c2_rest_path_base, "c2.rest.path.base", base)) { + if (configuration_->get(Configuration::nifi_c2_rest_path_base, base)) { return base + url; } if (!configuration_->get(Configuration::nifi_c2_rest_url, "c2.rest.url", base)) { @@ -1068,7 +1070,7 @@ void C2Agent::handleAssetUpdate(const C2ContentResponse& resp) { // output file std::filesystem::path file_path; if (auto file_rel = resp.getStringArgument("file") | utils::transform(make_path)) { - auto result = utils::file::validateRelativePath(file_rel.value()); + auto result = utils::file::validateRelativeFilePath(file_rel.value()); if (!result) { send_error(result.error()); return; diff --git a/libminifi/src/c2/HeartbeatJsonSerializer.cpp b/libminifi/src/c2/HeartbeatJsonSerializer.cpp index 6237a3a20d..ba31555d5e 100644 --- a/libminifi/src/c2/HeartbeatJsonSerializer.cpp +++ b/libminifi/src/c2/HeartbeatJsonSerializer.cpp @@ -60,7 +60,7 @@ static void serializeOperationInfo(rapidjson::Value& target, const C2Payload& pa target.AddMember("identifier", rapidjson::Value(id.c_str(), alloc), alloc); } -static void setJsonStr(const std::string& key, const c2::C2Value& value, rapidjson::Value& parent, rapidjson::Document::AllocatorType& alloc) { // NOLINT +static void setJsonStr(const std::string& key, const c2::C2Value& value, rapidjson::Value& parent, rapidjson::Document::AllocatorType& alloc) { rapidjson::Value valueVal; if (auto* json_val = value.json()) { diff --git a/libminifi/src/c2/protocols/RESTSender.cpp b/libminifi/src/c2/protocols/RESTSender.cpp index da58528580..659ed51db0 100644 --- a/libminifi/src/c2/protocols/RESTSender.cpp +++ b/libminifi/src/c2/protocols/RESTSender.cpp @@ -41,7 +41,7 @@ void RESTSender::initialize(core::controller::ControllerServiceProvider* control RESTProtocol::initialize(controller, configure); // base URL when one is not specified. if (nullptr != configure) { - std::optional rest_base_path = configure->getWithFallback(Configuration::nifi_c2_rest_path_base, "c2.rest.path.base"); + std::optional rest_base_path = configure->get(Configuration::nifi_c2_rest_path_base); std::string update_str; std::string ssl_context_service_str; configure->get(Configuration::nifi_c2_rest_url, "c2.rest.url", rest_uri_); diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index f4f88403d9..b8c2c43b11 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -36,7 +36,7 @@ void AssetManager::refreshState() { std::lock_guard lock(mtx_); state_.clear(); if (!utils::file::FileUtils::exists(root_)) { - std::filesystem::create_directory(root_); + std::filesystem::create_directories(root_); } if (!utils::file::FileUtils::exists(root_ / ".state")) { std::ofstream{root_ / ".state", std::ios::binary} << R"({"digest": "", "assets": {}})"; @@ -77,18 +77,18 @@ void AssetManager::refreshState() { for (auto& [id, entry] : doc["assets"].GetObject()) { if (!entry.IsObject()) { - logger_->log_error("Asset '.state' file is malformed"); + logger_->log_error("Asset '.state' file is malformed, 'assets.{}' is not an object", std::string_view{id.GetString(), id.GetStringLength()}); return; } AssetDescription description; description.id = std::string{id.GetString(), id.GetStringLength()}; if (!entry.HasMember("path") || !entry["path"].IsString()) { - logger_->log_error("Asset '.state' file is malformed"); + logger_->log_error("Asset '.state' file is malformed, 'assets.{}.path' does not exist or is not a string", std::string_view{id.GetString(), id.GetStringLength()}); return; } description.path = std::string{entry["path"].GetString(), entry["path"].GetStringLength()}; if (!entry.HasMember("url") || !entry["url"].IsString()) { - logger_->log_error("Asset '.state' file is malformed"); + logger_->log_error("Asset '.state' file is malformed, 'assets.{}.url' does not exist or is not a string", std::string_view{id.GetString(), id.GetStringLength()}); return; } description.url = std::string{entry["url"].GetString(), entry["url"].GetStringLength()}; @@ -96,7 +96,7 @@ void AssetManager::refreshState() { if (utils::file::FileUtils::exists(root_ / description.id)) { new_state.assets.insert(std::move(description)); } else { - logger_->log_error("Asset '.state' file contains entry that does not exist on the filesystem"); + logger_->log_error("Asset '.state' file contains entry '{}' that does not exist on the filesystem at '{}'", std::string_view{id.GetString(), id.GetStringLength()}, (root_ / description.id).string()); } } state_ = std::move(new_state); diff --git a/libminifi/test/integration/C2MetricsTest.cpp b/libminifi/test/integration/C2MetricsTest.cpp index c62ee0cc91..8997228ac9 100644 --- a/libminifi/test/integration/C2MetricsTest.cpp +++ b/libminifi/test/integration/C2MetricsTest.cpp @@ -62,7 +62,7 @@ class MetricsHandler: public HeartbeatHandler { explicit MetricsHandler(std::atomic_bool& metrics_updated_successfully, std::shared_ptr configuration, const std::filesystem::path& replacement_config_path) : HeartbeatHandler(std::move(configuration)), metrics_updated_successfully_(metrics_updated_successfully), - replacement_config_(getReplacementConfig(replacement_config_path.string())) { + replacement_config_(minifi::utils::file::get_content(replacement_config_path.string())) { } void handleHeartbeat(const rapidjson::Document& root, struct mg_connection* conn) override { @@ -178,11 +178,6 @@ class MetricsHandler: public HeartbeatHandler { processor_metrics["GetTCPMetrics"][GETTCP1_UUID].HasMember("TransferredBytes"); } - [[nodiscard]] static std::string getReplacementConfig(const std::string& replacement_config_path) { - std::ifstream is(replacement_config_path); - return {(std::istreambuf_iterator(is)), std::istreambuf_iterator()}; - } - std::atomic_bool& metrics_updated_successfully_; TestState test_state_ = TestState::VERIFY_INITIAL_METRICS; std::string replacement_config_; From 84a42149e7e7398d1b862d23e9bad543f0147507 Mon Sep 17 00:00:00 2001 From: adamdebreceni <64783590+adamdebreceni@users.noreply.github.com> Date: Tue, 2 Jul 2024 09:06:38 +0200 Subject: [PATCH 30/48] Update CONFIGURE.md Co-authored-by: Ferenc Gerlits --- CONFIGURE.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CONFIGURE.md b/CONFIGURE.md index d422d078b0..c645a5bc4b 100644 --- a/CONFIGURE.md +++ b/CONFIGURE.md @@ -666,11 +666,11 @@ Additionally, a unique hexadecimal uid.minifi.device.segment should be assigned ### Asset directory There is an asset directory specified using the `nifi.asset.directory` agent property, which defaults to `${MINIFI_HOME}/asset`. -The files referenced in the `.state` file in this directory are managed by the agent, there are deleted, updated, downloaded +The files referenced in the `.state` file in this directory are managed by the agent. They are deleted, updated, downloaded using the asset sync c2 command. For the asset sync command to work, the c2 server must be made aware of the current state of the -managed assets, for this the `AssetInformation` entry has to be added to the `nifi.c2.root.classes` property. +managed assets by adding the `AssetInformation` entry to the `nifi.c2.root.classes` property. -Files and directories not referenced in the `.state` file are not directly controlled by the agent although, +Files and directories not referenced in the `.state` file are not directly controlled by the agent, although it is possible to download an asset (triggered through the c2 protocol) into the asset directory instead. ### Controller Services From 337856492cd1e8fb121b04ead5f537327dade391 Mon Sep 17 00:00:00 2001 From: adamdebreceni <64783590+adamdebreceni@users.noreply.github.com> Date: Tue, 2 Jul 2024 09:06:54 +0200 Subject: [PATCH 31/48] Update C2.md Co-authored-by: Ferenc Gerlits --- C2.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/C2.md b/C2.md index f43a8284d1..ad492ec57a 100644 --- a/C2.md +++ b/C2.md @@ -82,7 +82,7 @@ be requested via C2 DESCRIBE manifest command. # specify the rest URIs if using RESTSender nifi.c2.rest.path.base=https:/// - # spicify either absolute url or relative to the nifi.c2.rest.path.base url for hearbeat and acknowledge + # specify either absolute url or relative to the nifi.c2.rest.path.base url for hearbeat and acknowledge nifi.c2.rest.url=/c2-protocol/heartbeat nifi.c2.rest.url.ack=/c2-protocol/acknowledge nifi.c2.flow.base.url=http:////c2-protocol/ From 37f3169cab400e9c23d7cb7ab0e1e6e7fa99b9c4 Mon Sep 17 00:00:00 2001 From: adamdebreceni <64783590+adamdebreceni@users.noreply.github.com> Date: Tue, 2 Jul 2024 09:07:06 +0200 Subject: [PATCH 32/48] Update libminifi/test/integration/C2AssetSyncTest.cpp Co-authored-by: Ferenc Gerlits --- libminifi/test/integration/C2AssetSyncTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index 8b27bf8f23..b9ef58ae7a 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -209,7 +209,7 @@ TEST_CASE("C2AssetSync", "[c2test]") { auto get_asset_structure = [&] () { std::unordered_map contents; - for (auto& [dir, file] : minifi::utils::file::list_dir_all(asset_dir.string(), controller.getLogger())) { + for (auto& [dir, file] : minifi::utils::file::list_dir_all(asset_dir, controller.getLogger())) { contents[(dir / file).string()] = minifi::utils::file::get_content(dir / file); } return contents; From 507c549e1a642329279d77bef327dc145098f9cf Mon Sep 17 00:00:00 2001 From: adamdebreceni <64783590+adamdebreceni@users.noreply.github.com> Date: Tue, 2 Jul 2024 09:07:18 +0200 Subject: [PATCH 33/48] Update libminifi/src/utils/file/AssetManager.cpp Co-authored-by: Ferenc Gerlits --- libminifi/src/utils/file/AssetManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index b8c2c43b11..8e19e44cd9 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -22,7 +22,7 @@ #include "core/logging/LoggerFactory.h" #include "utils/Hash.h" -#undef GetObject +#undef GetObject // windows.h #defines GetObject = GetObjectA or GetObjectW, which conflicts with rapidjson namespace org::apache::nifi::minifi::utils::file { From c16c5ca9a418cf7e78ace0fb2e2b1780c545ce56 Mon Sep 17 00:00:00 2001 From: adamdebreceni <64783590+adamdebreceni@users.noreply.github.com> Date: Tue, 2 Jul 2024 09:07:31 +0200 Subject: [PATCH 34/48] Update libminifi/src/utils/file/AssetManager.cpp Co-authored-by: Ferenc Gerlits --- libminifi/src/utils/file/AssetManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index 8e19e44cd9..2562a5a14e 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -108,7 +108,7 @@ std::string AssetManager::hash() const { } nonstd::expected AssetManager::sync( - org::apache::nifi::minifi::utils::file::AssetLayout layout, + const org::apache::nifi::minifi::utils::file::AssetLayout& layout, const std::function, std::string>(std::string_view /*url*/)>& fetch) { std::lock_guard lock(mtx_); org::apache::nifi::minifi::utils::file::AssetLayout new_state{ From 72886860a1c8bdc1ca64358231e6b0b0a38a955e Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Tue, 9 Jul 2024 09:54:17 +0200 Subject: [PATCH 35/48] MINIFICPP-2314 - Properly restore working directory --- libminifi/src/c2/C2Agent.cpp | 2 +- libminifi/test/integration/C2AssetSyncTest.cpp | 6 ++++-- libminifi/test/integration/C2UpdateAssetTest.cpp | 6 ++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index f3a41c635f..10d76cf1c4 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -846,7 +846,7 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse if (!resolved_url) { return nonstd::make_unexpected("Couldn't resolve url"); } - C2Payload file_response = protocol_.load()->fetch(resolved_url.value()); + C2Payload file_response = protocol_->fetch(resolved_url.value()); if (file_response.getStatus().getState() != state::UpdateState::READ_COMPLETE) { return nonstd::make_unexpected("Failed to fetch file from " + resolved_url.value()); diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index b9ef58ae7a..e77ac62937 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -178,7 +178,11 @@ TEST_CASE("C2AssetSync", "[c2test]") { // setup minifi home const std::filesystem::path home_dir = controller.createTempDirectory(); const auto asset_dir = home_dir / "asset"; + std::filesystem::current_path(home_dir); + auto wd_guard = gsl::finally([] { + std::filesystem::current_path(minifi::utils::file::get_executable_dir()); + }); C2AcknowledgeHandler ack_handler; std::string file_A = "hello from file A"; @@ -271,8 +275,6 @@ TEST_CASE("C2AssetSync", "[c2test]") { }); harness.run(); - - std::filesystem::current_path(minifi::utils::file::get_executable_dir()); } } // namespace org::apache::nifi::minifi::test diff --git a/libminifi/test/integration/C2UpdateAssetTest.cpp b/libminifi/test/integration/C2UpdateAssetTest.cpp index af67cf2db9..458a5f2bed 100644 --- a/libminifi/test/integration/C2UpdateAssetTest.cpp +++ b/libminifi/test/integration/C2UpdateAssetTest.cpp @@ -103,7 +103,11 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { // setup minifi home const std::filesystem::path home_dir = controller.createTempDirectory(); const auto asset_dir = home_dir / "asset"; + std::filesystem::current_path(home_dir); + auto wd_guard = gsl::finally([] { + std::filesystem::current_path(minifi::utils::file::get_executable_dir()); + }); C2AcknowledgeHandler ack_handler; std::string file_A = "hello from file A"; @@ -259,8 +263,6 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { REQUIRE(false); } } - - std::filesystem::current_path(minifi::utils::file::get_executable_dir()); } } // namespace org::apache::nifi::minifi::test From 27868113fc38641dba516f75a1b10e04e2d71d02 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 10 Jul 2024 13:02:20 +0200 Subject: [PATCH 36/48] MINIFICPP-2314 - Linter fix --- libminifi/include/utils/file/AssetManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/include/utils/file/AssetManager.h b/libminifi/include/utils/file/AssetManager.h index f269988657..93424a1452 100644 --- a/libminifi/include/utils/file/AssetManager.h +++ b/libminifi/include/utils/file/AssetManager.h @@ -53,7 +53,7 @@ class AssetManager { public: explicit AssetManager(const Configure& configuration); - nonstd::expected sync(AssetLayout layout, const std::function, std::string>(std::string_view /*url*/)>& fetch); + nonstd::expected sync(const AssetLayout& layout, const std::function, std::string>(std::string_view /*url*/)>& fetch); std::string hash() const; From 0e9da8f9c89fedeb96313c1453883da9f621ed64 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 10 Jul 2024 15:00:33 +0200 Subject: [PATCH 37/48] MINIFICPP-2314 - Linter fix --- libminifi/src/utils/file/AssetManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index 2562a5a14e..e199617d23 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -96,7 +96,8 @@ void AssetManager::refreshState() { if (utils::file::FileUtils::exists(root_ / description.id)) { new_state.assets.insert(std::move(description)); } else { - logger_->log_error("Asset '.state' file contains entry '{}' that does not exist on the filesystem at '{}'", std::string_view{id.GetString(), id.GetStringLength()}, (root_ / description.id).string()); + logger_->log_error("Asset '.state' file contains entry '{}' that does not exist on the filesystem at '{}'", + std::string_view{id.GetString(), id.GetStringLength()}, (root_ / description.id).string()); } } state_ = std::move(new_state); From 5464940007757b8e4664b7cf09233aa6e4e3d079 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 10 Jul 2024 17:21:29 +0200 Subject: [PATCH 38/48] MINIFICPP-2314 - Build fix --- libminifi/src/utils/file/AssetManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index e199617d23..fd70cf197c 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -113,7 +113,8 @@ nonstd::expected AssetManager::sync( const std::function, std::string>(std::string_view /*url*/)>& fetch) { std::lock_guard lock(mtx_); org::apache::nifi::minifi::utils::file::AssetLayout new_state{ - .digest = state_.digest + .digest = state_.digest, + .assets = {} }; std::string fetch_errors; std::vector>> new_file_contents; From 13f4619554a6601b8ed987092688313f2089fc24 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 17 Jul 2024 14:41:21 +0200 Subject: [PATCH 39/48] MINIFICPP-2314 - Build fix --- libminifi/src/c2/C2Agent.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index 10d76cf1c4..d337a64e53 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -782,7 +782,7 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse return; } - for (size_t resource_idx = 0; resource_idx < resource_list->Size(); ++resource_idx) { + for (rapidjson::SizeType resource_idx = 0; resource_idx < resource_list->Size(); ++resource_idx) { auto& resource = resource_list->GetArray()[resource_idx]; if (!resource.IsObject()) { send_error(fmt::format("Malformed request, 'resourceList[{}]' is not an object", resource_idx)); From fbf75a4878921e76cbdf3679d4c20c8236fa6d34 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Fri, 19 Jul 2024 09:06:15 +0200 Subject: [PATCH 40/48] MINIFICPP-2314 - Review changes --- libminifi/include/c2/C2Payload.h | 6 +++--- libminifi/include/c2/PayloadSerializer.h | 4 ++-- libminifi/include/c2/protocols/RESTProtocol.h | 2 +- libminifi/src/c2/C2Agent.cpp | 8 ++++---- libminifi/src/c2/HeartbeatJsonSerializer.cpp | 2 +- libminifi/src/c2/protocols/RESTProtocol.cpp | 4 ++-- libminifi/src/c2/triggers/FileUpdateTrigger.cpp | 4 ++-- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/libminifi/include/c2/C2Payload.h b/libminifi/include/c2/C2Payload.h index 57715a0490..09c90540f4 100644 --- a/libminifi/include/c2/C2Payload.h +++ b/libminifi/include/c2/C2Payload.h @@ -96,12 +96,12 @@ class C2Value { C2Value(C2Value&&) = default; template requires(std::constructible_from) - C2Value(T&& value) { value_ = state::response::ValueNode{std::forward(value)}; } // NOLINT(runtime/explicit) - C2Value(const rapidjson::Value& json_value) { // NOLINT(runtime/explicit) + explicit C2Value(T&& value) { value_ = state::response::ValueNode{std::forward(value)}; } + explicit C2Value(const rapidjson::Value& json_value) { value_.emplace(); get(value_).CopyFrom(json_value, get(value_).GetAllocator()); } - C2Value(rapidjson::Document&& json_doc) { // NOLINT(runtime/explicit) + explicit C2Value(rapidjson::Document&& json_doc) { value_ = std::move(json_doc); } diff --git a/libminifi/include/c2/PayloadSerializer.h b/libminifi/include/c2/PayloadSerializer.h index 92fa793899..e9ca060b92 100644 --- a/libminifi/include/c2/PayloadSerializer.h +++ b/libminifi/include/c2/PayloadSerializer.h @@ -251,7 +251,7 @@ class PayloadSerializer { for (uint32_t j = 0; j < args; j++) { std::string first, second; stream->read(first); - content.operation_arguments[first] = deserializeValueNode(stream); + content.operation_arguments[first] = C2Value{deserializeValueNode(stream)}; } subPayload.addContent(std::move(content)); } @@ -293,7 +293,7 @@ class PayloadSerializer { std::string first, second; stream.read(first); // stream.readUTF(second); - content.operation_arguments[first] = deserializeValueNode(&stream); + content.operation_arguments[first] = C2Value{deserializeValueNode(&stream)}; } newPayload.addContent(std::move(content)); } diff --git a/libminifi/include/c2/protocols/RESTProtocol.h b/libminifi/include/c2/protocols/RESTProtocol.h index f3ed0ebce7..cb04a2de19 100644 --- a/libminifi/include/c2/protocols/RESTProtocol.h +++ b/libminifi/include/c2/protocols/RESTProtocol.h @@ -43,7 +43,7 @@ class RESTProtocol : public HeartbeatJsonSerializer { protected: void initialize(core::controller::ControllerServiceProvider* controller, const std::shared_ptr &configure); void serializeNestedPayload(rapidjson::Value& target, const C2Payload& payload, rapidjson::Document::AllocatorType& alloc) override; - C2Payload parseJsonResponse(const C2Payload &payload, std::span response); + C2Payload parseJsonResponse(const C2Payload &payload, std::span response) const; private: bool containsPayload(const C2Payload &o); diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index d337a64e53..3411b514fd 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -255,7 +255,7 @@ void C2Agent::serializeMetrics(C2Payload &metric_payload, const std::string &nam } else { C2ContentResponse response(metric_payload.getOperation()); response.name = name; - response.operation_arguments[metric.name] = metric.value; + response.operation_arguments[metric.name] = C2Value{metric.value}; metric_payload.addContent(std::move(response), is_collapsible); } } @@ -408,7 +408,7 @@ C2Payload C2Agent::prepareConfigurationOptions(const C2ContentResponse &resp) co if (configuration_->get(key, value)) { C2ContentResponse option(Operation::acknowledge); option.name = key; - option.operation_arguments[key] = value; + option.operation_arguments[key] = C2Value{value}; options.addContent(std::move(option)); } } @@ -537,7 +537,7 @@ void C2Agent::handle_describe(const C2ContentResponse &resp) { for (const auto &line : trace.getTraces()) { C2ContentResponse option(Operation::acknowledge); option.name = line; - option.operation_arguments[line] = line; + option.operation_arguments[line] = C2Value{line}; options.addContent(std::move(option)); } response.addPayload(std::move(options)); @@ -560,7 +560,7 @@ void C2Agent::handle_describe(const C2ContentResponse &resp) { for (const auto& kv : core_component_state.second) { C2ContentResponse entry(Operation::acknowledge); entry.name = kv.first; - entry.operation_arguments[kv.first] = kv.second; + entry.operation_arguments[kv.first] = C2Value{kv.second}; state.addContent(std::move(entry)); } states.addPayload(std::move(state)); diff --git a/libminifi/src/c2/HeartbeatJsonSerializer.cpp b/libminifi/src/c2/HeartbeatJsonSerializer.cpp index ba31555d5e..a25b47f772 100644 --- a/libminifi/src/c2/HeartbeatJsonSerializer.cpp +++ b/libminifi/src/c2/HeartbeatJsonSerializer.cpp @@ -165,7 +165,7 @@ static rapidjson::Value serializeConnectionQueues(const C2Payload& payload, std: adjusted.setIdentifier(uuid); c2::C2Value nd; // name should be what was previously the TLN ( top level node ) - nd = name; + nd = C2Value{name}; updatedContent.operation_arguments.insert(std::make_pair("name", nd)); // the rvalue reference is an unfortunate side effect of the underlying API decision. adjusted.addContent(std::move(updatedContent), true); diff --git a/libminifi/src/c2/protocols/RESTProtocol.cpp b/libminifi/src/c2/protocols/RESTProtocol.cpp index 57b42cfcb2..fee4b63275 100644 --- a/libminifi/src/c2/protocols/RESTProtocol.cpp +++ b/libminifi/src/c2/protocols/RESTProtocol.cpp @@ -37,7 +37,7 @@ namespace org::apache::nifi::minifi::c2 { -C2Payload RESTProtocol::parseJsonResponse(const C2Payload &payload, std::span response) { +C2Payload RESTProtocol::parseJsonResponse(const C2Payload &payload, std::span response) const { rapidjson::Document root; try { @@ -105,7 +105,7 @@ C2Payload RESTProtocol::parseJsonResponse(const C2Payload &payload, std::span Date: Fri, 19 Jul 2024 15:44:48 +0200 Subject: [PATCH 41/48] MINIFICPP-2314 - Fix build --- libminifi/test/unit/PayloadParserTests.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libminifi/test/unit/PayloadParserTests.cpp b/libminifi/test/unit/PayloadParserTests.cpp index bdb2f50469..0a9702bfaa 100644 --- a/libminifi/test/unit/PayloadParserTests.cpp +++ b/libminifi/test/unit/PayloadParserTests.cpp @@ -30,7 +30,7 @@ TEST_CASE("Test Valid Payload", "[tv1]") { minifi::c2::C2Payload payload(minifi::c2::Operation::acknowledge, ident); minifi::c2::C2Payload payload2(minifi::c2::Operation::acknowledge, minifi::state::UpdateState::FULLY_APPLIED, cheese); minifi::c2::C2ContentResponse response(minifi::c2::Operation::acknowledge); - response.operation_arguments["type"] = "munster"; + response.operation_arguments["type"] = minifi::c2::C2Value{"munster"}; payload2.addContent(std::move(response)); payload.addPayload(std::move(payload2)); payload.addPayload(minifi::c2::C2Payload(minifi::c2::Operation::acknowledge, chips)); @@ -44,7 +44,7 @@ TEST_CASE("Test Invalid not found", "[tv2]") { minifi::c2::C2Payload payload(minifi::c2::Operation::acknowledge, ident); minifi::c2::C2Payload payload2(minifi::c2::Operation::acknowledge, cheese); minifi::c2::C2ContentResponse response(minifi::c2::Operation::acknowledge); - response.operation_arguments["typeS"] = "munster"; + response.operation_arguments["typeS"] = minifi::c2::C2Value{"munster"}; payload2.addContent(std::move(response)); payload.addPayload(std::move(payload2)); payload.addPayload(minifi::c2::C2Payload(minifi::c2::Operation::acknowledge, chips)); @@ -59,7 +59,7 @@ TEST_CASE("Test Invalid coercion", "[tv3]") { minifi::c2::C2Payload payload(minifi::c2::Operation::acknowledge, ident); minifi::c2::C2Payload payload2(minifi::c2::Operation::acknowledge, minifi::state::UpdateState::FULLY_APPLIED, cheese); minifi::c2::C2ContentResponse response(minifi::c2::Operation::acknowledge); - response.operation_arguments["type"] = "munster"; + response.operation_arguments["type"] = minifi::c2::C2Value{"munster"}; payload2.addContent(std::move(response)); payload.addPayload(std::move(payload2)); payload.addPayload(minifi::c2::C2Payload(minifi::c2::Operation::acknowledge, chips)); @@ -73,7 +73,7 @@ TEST_CASE("Test Invalid not there", "[tv4]") { minifi::c2::C2Payload payload(minifi::c2::Operation::acknowledge, ident); minifi::c2::C2Payload payload2(minifi::c2::Operation::acknowledge, minifi::state::UpdateState::FULLY_APPLIED, cheese); minifi::c2::C2ContentResponse response(minifi::c2::Operation::acknowledge); - response.operation_arguments["type"] = "munster"; + response.operation_arguments["type"] = minifi::c2::C2Value{"munster"}; payload2.addContent(std::move(response)); payload.addPayload(std::move(payload2)); payload.addPayload(minifi::c2::C2Payload(minifi::c2::Operation::acknowledge, chips)); @@ -89,9 +89,9 @@ TEST_CASE("Test typed conversions", "[tv5]") { minifi::c2::C2Payload payload(minifi::c2::Operation::acknowledge, ident); minifi::c2::C2Payload payload2(minifi::c2::Operation::acknowledge, minifi::state::UpdateState::FULLY_APPLIED, cheese); minifi::c2::C2ContentResponse response(minifi::c2::Operation::acknowledge); - response.operation_arguments["type"] = "munster"; - response.operation_arguments["isvalid"] = isvalid; - response.operation_arguments["size"] = size; + response.operation_arguments["type"] = minifi::c2::C2Value{"munster"}; + response.operation_arguments["isvalid"] = minifi::c2::C2Value{isvalid}; + response.operation_arguments["size"] = minifi::c2::C2Value{size}; payload2.addContent(std::move(response)); payload.addPayload(std::move(payload2)); payload.addPayload(minifi::c2::C2Payload(minifi::c2::Operation::acknowledge, chips)); @@ -108,7 +108,7 @@ TEST_CASE("Test Invalid not there deep", "[tv6]") { minifi::c2::C2Payload payload(minifi::c2::Operation::acknowledge, ident); minifi::c2::C2Payload payload2(minifi::c2::Operation::acknowledge, minifi::state::UpdateState::FULLY_APPLIED, cheese); minifi::c2::C2ContentResponse response(minifi::c2::Operation::acknowledge); - response.operation_arguments["type"] = "munster"; + response.operation_arguments["type"] = minifi::c2::C2Value{"munster"}; payload2.addContent(std::move(response)); payload.addPayload(std::move(payload2)); payload.addPayload(minifi::c2::C2Payload(minifi::c2::Operation::acknowledge, chips)); From 116c92081528ef6bfe2017d0b7c839d6244ddafc Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Mon, 22 Jul 2024 14:08:26 +0200 Subject: [PATCH 42/48] MINIFICPP-2314 - Add more explicit constructor calls --- .../test/integration/C2AssetSyncTest.cpp | 4 +-- .../C2ClearCoreComponentStateTest.cpp | 2 +- .../integration/C2DescribeMetricsTest.cpp | 4 +-- libminifi/test/integration/C2MetricsTest.cpp | 2 +- .../test/integration/C2UpdateAssetTest.cpp | 32 +++++++++---------- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/libminifi/test/integration/C2AssetSyncTest.cpp b/libminifi/test/integration/C2AssetSyncTest.cpp index e77ac62937..5731025eb0 100644 --- a/libminifi/test/integration/C2AssetSyncTest.cpp +++ b/libminifi/test/integration/C2AssetSyncTest.cpp @@ -70,7 +70,7 @@ class C2HeartbeatHandler : public HeartbeatHandler { std::unordered_map args; rapidjson::Document global_hash_doc{rapidjson::kObjectType}; global_hash_doc.AddMember("digest", calculateAssetHash(), global_hash_doc.GetAllocator()); - args["globalHash"] = std::move(global_hash_doc); + args["globalHash"] = minifi::c2::C2Value{std::move(global_hash_doc)}; rapidjson::Document resource_list_doc{rapidjson::kArrayType}; for (auto& asset : expected_assets_) { @@ -82,7 +82,7 @@ class C2HeartbeatHandler : public HeartbeatHandler { resource_obj.AddMember("url", asset.url, resource_list_doc.GetAllocator()); resource_list_doc.PushBack(resource_obj, resource_list_doc.GetAllocator()); } - args["resourceList"] = std::move(resource_list_doc); + args["resourceList"] = minifi::c2::C2Value{std::move(resource_list_doc)}; operations.push_back(C2Operation{ .operation = "sync", diff --git a/libminifi/test/integration/C2ClearCoreComponentStateTest.cpp b/libminifi/test/integration/C2ClearCoreComponentStateTest.cpp index 07081d815f..a936bcd88b 100644 --- a/libminifi/test/integration/C2ClearCoreComponentStateTest.cpp +++ b/libminifi/test/integration/C2ClearCoreComponentStateTest.cpp @@ -97,7 +97,7 @@ class ClearCoreComponentStateHandler: public HeartbeatHandler { break; case FlowState::FIRST_DESCRIBE_ACK: case FlowState::CLEAR_SENT: { - sendHeartbeatResponse("CLEAR", "corecomponentstate", "889346", conn, { {"corecomponent1", "TailFile1"} }); + sendHeartbeatResponse("CLEAR", "corecomponentstate", "889346", conn, { {"corecomponent1", minifi::c2::C2Value{"TailFile1"}} }); flow_state_ = FlowState::CLEAR_SENT; break; } diff --git a/libminifi/test/integration/C2DescribeMetricsTest.cpp b/libminifi/test/integration/C2DescribeMetricsTest.cpp index 692e0e37b2..9e5bcbcdff 100644 --- a/libminifi/test/integration/C2DescribeMetricsTest.cpp +++ b/libminifi/test/integration/C2DescribeMetricsTest.cpp @@ -64,11 +64,11 @@ class MetricsHandler: public HeartbeatHandler { void handleHeartbeat(const rapidjson::Document&, struct mg_connection* conn) override { switch (state_) { case TestState::DESCRIBE_SPECIFIC_PROCESSOR_METRIC: { - sendHeartbeatResponse("DESCRIBE", "metrics", "889347", conn, {{"metricsClass", "GetFileMetrics"}}); + sendHeartbeatResponse("DESCRIBE", "metrics", "889347", conn, {{"metricsClass", minifi::c2::C2Value{"GetFileMetrics"}}}); break; } case TestState::DESCRIBE_SPECIFIC_SYSTEM_METRIC: { - sendHeartbeatResponse("DESCRIBE", "metrics", "889347", conn, {{"metricsClass", "QueueMetrics"}}); + sendHeartbeatResponse("DESCRIBE", "metrics", "889347", conn, {{"metricsClass", minifi::c2::C2Value{"QueueMetrics"}}}); break; } case TestState::DESCRIBE_ALL_METRICS: { diff --git a/libminifi/test/integration/C2MetricsTest.cpp b/libminifi/test/integration/C2MetricsTest.cpp index 8997228ac9..39a0a19c75 100644 --- a/libminifi/test/integration/C2MetricsTest.cpp +++ b/libminifi/test/integration/C2MetricsTest.cpp @@ -73,7 +73,7 @@ class MetricsHandler: public HeartbeatHandler { break; } case TestState::SEND_NEW_CONFIG: { - sendHeartbeatResponse("UPDATE", "configuration", "889348", conn, {{"configuration_data", replacement_config_}}); + sendHeartbeatResponse("UPDATE", "configuration", "889348", conn, {{"configuration_data", minifi::c2::C2Value{replacement_config_}}}); test_state_ = TestState::VERIFY_UPDATED_METRICS; break; } diff --git a/libminifi/test/integration/C2UpdateAssetTest.cpp b/libminifi/test/integration/C2UpdateAssetTest.cpp index 458a5f2bed..907b9f9218 100644 --- a/libminifi/test/integration/C2UpdateAssetTest.cpp +++ b/libminifi/test/integration/C2UpdateAssetTest.cpp @@ -134,7 +134,7 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { operations.push_back({ .id = "2", .args = { - {"file", "my_file.txt"} + {"file", minifi::c2::C2Value{"my_file.txt"}} }, .state = "NOT_APPLIED", .details = "Couldn't find 'url' argument" @@ -143,8 +143,8 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { operations.push_back({ .id = "3", .args = { - {"file", "my_file.txt"}, - {"url", "/api/file/A.txt"} + {"file", minifi::c2::C2Value{"my_file.txt"}}, + {"url", minifi::c2::C2Value{"/api/file/A.txt"}} }, .state = "FULLY_APPLIED", .details = std::nullopt @@ -153,8 +153,8 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { operations.push_back({ .id = "4", .args = { - {"file", "my_file.txt"}, - {"url", "/api/file/A.txt"} + {"file", minifi::c2::C2Value{"my_file.txt"}}, + {"url", minifi::c2::C2Value{"/api/file/A.txt"}} }, .state = "NO_OPERATION", .details = std::nullopt @@ -163,9 +163,9 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { operations.push_back({ .id = "5", .args = { - {"file", "my_file.txt"}, - {"url", "/api/file/B.txt"}, - {"forceDownload", "true"} + {"file", minifi::c2::C2Value{"my_file.txt"}}, + {"url", minifi::c2::C2Value{"/api/file/B.txt"}}, + {"forceDownload", minifi::c2::C2Value{"true"}} }, .state = "FULLY_APPLIED", .details = std::nullopt @@ -174,8 +174,8 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { operations.push_back({ .id = "6", .args = { - {"file", "new_dir/inner/my_file.txt"}, - {"url", "/api/file/A.txt"} + {"file", minifi::c2::C2Value{"new_dir/inner/my_file.txt"}}, + {"url", minifi::c2::C2Value{"/api/file/A.txt"}} }, .state = "FULLY_APPLIED", .details = std::nullopt @@ -184,8 +184,8 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { operations.push_back({ .id = "7", .args = { - {"file", "dummy.txt"}, - {"url", "/not_existing_api/file.txt"} + {"file", minifi::c2::C2Value{"dummy.txt"}}, + {"url", minifi::c2::C2Value{"/not_existing_api/file.txt"}} }, .state = "NOT_APPLIED", .details = "Failed to fetch asset" @@ -194,8 +194,8 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { operations.push_back({ .id = "8", .args = { - {"file", "../../system_lib.dll"}, - {"url", "/not_existing_api/file.txt"} + {"file", minifi::c2::C2Value{"../../system_lib.dll"}}, + {"url", minifi::c2::C2Value{"/not_existing_api/file.txt"}} }, .state = "NOT_APPLIED", .details = "Accessing parent directory is forbidden in file path" @@ -204,8 +204,8 @@ TEST_CASE("Test update asset C2 command", "[c2test]") { operations.push_back({ .id = "9", .args = { - {"file", "other_dir/A.txt"}, - {"url", absolute_file_A_url} + {"file", minifi::c2::C2Value{"other_dir/A.txt"}}, + {"url", minifi::c2::C2Value{absolute_file_A_url}} }, .state = "FULLY_APPLIED", .details = std::nullopt From 95bed4a0af892a310cd8ee2432ddeb89f843aae3 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 24 Jul 2024 10:40:41 +0200 Subject: [PATCH 43/48] MINIFICPP-2314 - Review changes --- libminifi/include/FlowController.h | 2 +- libminifi/include/c2/C2Agent.h | 4 ++-- .../include/core/state/MetricsPublisherStore.h | 2 +- .../include/core/state/nodes/AssetInformation.h | 4 ++-- .../include/core/state/nodes/ResponseNodeLoader.h | 4 ++-- libminifi/src/FlowController.cpp | 2 +- libminifi/src/c2/C2Agent.cpp | 2 +- libminifi/src/core/state/MetricsPublisherStore.cpp | 2 +- .../src/core/state/nodes/AssetInformation.cpp | 2 +- .../src/core/state/nodes/ResponseNodeLoader.cpp | 2 +- libminifi/src/utils/file/AssetManager.cpp | 14 +++++++------- .../test/libtest/integration/IntegrationBase.cpp | 6 +++--- .../test/libtest/integration/IntegrationBase.h | 2 ++ minifi_main/MiNiFiMain.cpp | 6 +++--- 14 files changed, 28 insertions(+), 26 deletions(-) diff --git a/libminifi/include/FlowController.h b/libminifi/include/FlowController.h index 6c8308c209..c4a3402b91 100644 --- a/libminifi/include/FlowController.h +++ b/libminifi/include/FlowController.h @@ -74,7 +74,7 @@ class FlowController : public core::controller::ForwardingControllerServiceProvi std::shared_ptr configure, std::shared_ptr flow_configuration, std::shared_ptr content_repo, std::unique_ptr metrics_publisher_store = nullptr, std::shared_ptr filesystem = std::make_shared(), std::function request_restart = []{}, - std::shared_ptr asset_manager = {}); + utils::file::AssetManager* asset_manager = {}); ~FlowController() override; diff --git a/libminifi/include/c2/C2Agent.h b/libminifi/include/c2/C2Agent.h index 2e7412f739..3f0e12a7c1 100644 --- a/libminifi/include/c2/C2Agent.h +++ b/libminifi/include/c2/C2Agent.h @@ -64,7 +64,7 @@ class C2Agent : public state::UpdateController { std::weak_ptr node_reporter, std::shared_ptr filesystem, std::function request_restart, - std::shared_ptr asset_manager); + utils::file::AssetManager* asset_manager); void initialize(core::controller::ControllerServiceProvider *controller, state::Pausable *pause_handler, state::StateMonitor* update_sink); void start() override; @@ -240,7 +240,7 @@ class C2Agent : public state::UpdateController { // time point the last time we performed a heartbeat. std::chrono::steady_clock::time_point last_run_; - std::shared_ptr asset_manager_; + utils::file::AssetManager* asset_manager_; }; } // namespace org::apache::nifi::minifi::c2 diff --git a/libminifi/include/core/state/MetricsPublisherStore.h b/libminifi/include/core/state/MetricsPublisherStore.h index 4d827bc00a..a1deb3cec0 100644 --- a/libminifi/include/core/state/MetricsPublisherStore.h +++ b/libminifi/include/core/state/MetricsPublisherStore.h @@ -34,7 +34,7 @@ namespace org::apache::nifi::minifi::state { class MetricsPublisherStore { public: MetricsPublisherStore(std::shared_ptr configuration, const std::vector>& repository_metric_sources, - std::shared_ptr flow_configuration, std::shared_ptr asset_manager = nullptr); + std::shared_ptr flow_configuration, utils::file::AssetManager* asset_manager = nullptr); void initialize(core::controller::ControllerServiceProvider* controller, state::StateMonitor* update_sink); void loadMetricNodes(core::ProcessGroup* root); void clearMetricNodes(); diff --git a/libminifi/include/core/state/nodes/AssetInformation.h b/libminifi/include/core/state/nodes/AssetInformation.h index 39fd578fa9..247264c35c 100644 --- a/libminifi/include/core/state/nodes/AssetInformation.h +++ b/libminifi/include/core/state/nodes/AssetInformation.h @@ -29,13 +29,13 @@ class AssetInformation : public ResponseNode { MINIFIAPI static constexpr const char* Description = "Metric node that defines hash for all asset identifiers"; - void setAssetManager(std::shared_ptr asset_manager); + void setAssetManager(utils::file::AssetManager* asset_manager); std::string getName() const override { return "resourceInfo"; } std::vector serialize() override; private: - std::shared_ptr asset_manager_; + utils::file::AssetManager* asset_manager_; std::shared_ptr logger_; }; diff --git a/libminifi/include/core/state/nodes/ResponseNodeLoader.h b/libminifi/include/core/state/nodes/ResponseNodeLoader.h index e59aae372d..4949bec234 100644 --- a/libminifi/include/core/state/nodes/ResponseNodeLoader.h +++ b/libminifi/include/core/state/nodes/ResponseNodeLoader.h @@ -41,7 +41,7 @@ namespace org::apache::nifi::minifi::state::response { class ResponseNodeLoader { public: ResponseNodeLoader(std::shared_ptr configuration, std::vector> repository_metric_sources, - std::shared_ptr flow_configuration, std::shared_ptr asset_manager = nullptr); + std::shared_ptr flow_configuration, utils::file::AssetManager* asset_manager = nullptr); void setNewConfigRoot(core::ProcessGroup* root); void clearConfigRoot(); @@ -75,7 +75,7 @@ class ResponseNodeLoader { std::shared_ptr configuration_; std::vector> repository_metric_sources_; std::shared_ptr flow_configuration_; - std::shared_ptr asset_manager_; + utils::file::AssetManager* asset_manager_; core::controller::ControllerServiceProvider* controller_{}; state::StateMonitor* update_sink_{}; std::shared_ptr logger_{core::logging::LoggerFactory::getLogger()}; diff --git a/libminifi/src/FlowController.cpp b/libminifi/src/FlowController.cpp index fbd58eb34b..b8d35deaa2 100644 --- a/libminifi/src/FlowController.cpp +++ b/libminifi/src/FlowController.cpp @@ -51,7 +51,7 @@ FlowController::FlowController(std::shared_ptr provenance_repo std::shared_ptr configure, std::shared_ptr flow_configuration, std::shared_ptr content_repo, std::unique_ptr metrics_publisher_store, std::shared_ptr filesystem, std::function request_restart, - std::shared_ptr asset_manager) + utils::file::AssetManager* asset_manager) : core::controller::ForwardingControllerServiceProvider(core::className()), running_(false), initialized_(false), diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index 3411b514fd..be2ff34d9c 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -57,7 +57,7 @@ C2Agent::C2Agent(std::shared_ptr configuration, std::weak_ptr node_reporter, std::shared_ptr filesystem, std::function request_restart, - std::shared_ptr asset_manager) + utils::file::AssetManager* asset_manager) : heart_beat_period_(3s), max_c2_responses(5), configuration_(std::move(configuration)), diff --git a/libminifi/src/core/state/MetricsPublisherStore.cpp b/libminifi/src/core/state/MetricsPublisherStore.cpp index d670c12bc1..a436c2595b 100644 --- a/libminifi/src/core/state/MetricsPublisherStore.cpp +++ b/libminifi/src/core/state/MetricsPublisherStore.cpp @@ -23,7 +23,7 @@ namespace org::apache::nifi::minifi::state { MetricsPublisherStore::MetricsPublisherStore(std::shared_ptr configuration, const std::vector>& repository_metric_sources, - std::shared_ptr flow_configuration, std::shared_ptr asset_manager) + std::shared_ptr flow_configuration, utils::file::AssetManager* asset_manager) : configuration_(configuration), response_node_loader_(std::make_shared(std::move(configuration), repository_metric_sources, std::move(flow_configuration), std::move(asset_manager))) { } diff --git a/libminifi/src/core/state/nodes/AssetInformation.cpp b/libminifi/src/core/state/nodes/AssetInformation.cpp index 717be7ee06..76997df75a 100644 --- a/libminifi/src/core/state/nodes/AssetInformation.cpp +++ b/libminifi/src/core/state/nodes/AssetInformation.cpp @@ -24,7 +24,7 @@ namespace org::apache::nifi::minifi::state::response { AssetInformation::AssetInformation() : logger_(core::logging::LoggerFactory().getLogger()) {} -void AssetInformation::setAssetManager(std::shared_ptr asset_manager) { +void AssetInformation::setAssetManager(utils::file::AssetManager* asset_manager) { asset_manager_ = std::move(asset_manager); if (!asset_manager_) { logger_->log_error("No asset manager is provided, asset information will not be available"); diff --git a/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp b/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp index 061a96be89..184248c218 100644 --- a/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp +++ b/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp @@ -34,7 +34,7 @@ namespace org::apache::nifi::minifi::state::response { ResponseNodeLoader::ResponseNodeLoader(std::shared_ptr configuration, std::vector> repository_metric_sources, - std::shared_ptr flow_configuration, std::shared_ptr asset_manager) + std::shared_ptr flow_configuration, utils::file::AssetManager* asset_manager) : configuration_(std::move(configuration)), repository_metric_sources_(std::move(repository_metric_sources)), flow_configuration_(std::move(flow_configuration)), diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index fd70cf197c..fde2b838fc 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -35,15 +35,15 @@ AssetManager::AssetManager(const Configure& configuration) void AssetManager::refreshState() { std::lock_guard lock(mtx_); state_.clear(); - if (!utils::file::FileUtils::exists(root_)) { + if (!FileUtils::exists(root_)) { std::filesystem::create_directories(root_); } - if (!utils::file::FileUtils::exists(root_ / ".state")) { + if (!FileUtils::exists(root_ / ".state")) { std::ofstream{root_ / ".state", std::ios::binary} << R"({"digest": "", "assets": {}})"; } rapidjson::Document doc; - std::string file_content = utils::file::get_content(root_ / ".state"); + std::string file_content = get_content(root_ / ".state"); rapidjson::ParseResult res = doc.Parse(file_content.c_str(), file_content.size()); if (!res) { @@ -93,7 +93,7 @@ void AssetManager::refreshState() { } description.url = std::string{entry["url"].GetString(), entry["url"].GetStringLength()}; - if (utils::file::FileUtils::exists(root_ / description.id)) { + if (FileUtils::exists(root_ / description.id)) { new_state.assets.insert(std::move(description)); } else { logger_->log_error("Asset '.state' file contains entry '{}' that does not exist on the filesystem at '{}'", @@ -109,10 +109,10 @@ std::string AssetManager::hash() const { } nonstd::expected AssetManager::sync( - const org::apache::nifi::minifi::utils::file::AssetLayout& layout, + const AssetLayout& layout, const std::function, std::string>(std::string_view /*url*/)>& fetch) { std::lock_guard lock(mtx_); - org::apache::nifi::minifi::utils::file::AssetLayout new_state{ + AssetLayout new_state{ .digest = state_.digest, .assets = {} }; @@ -145,7 +145,7 @@ nonstd::expected AssetManager::sync( } for (auto& [path, content] : new_file_contents) { - utils::file::create_dir((root_ / path).parent_path()); + create_dir((root_ / path).parent_path()); std::ofstream{root_ / path, std::ios::binary}.write(reinterpret_cast(content.data()), gsl::narrow(content.size())); } diff --git a/libminifi/test/libtest/integration/IntegrationBase.cpp b/libminifi/test/libtest/integration/IntegrationBase.cpp index 19d92f1dd9..72ae844fd7 100644 --- a/libminifi/test/libtest/integration/IntegrationBase.cpp +++ b/libminifi/test/libtest/integration/IntegrationBase.cpp @@ -118,10 +118,10 @@ void IntegrationBase::run(const std::optional& test_file_ }; std::vector> repo_metric_sources{test_repo, test_flow_repo, content_repo}; - auto asset_manager = std::make_shared(*configuration); - auto metrics_publisher_store = std::make_unique(configuration, repo_metric_sources, flow_config, asset_manager); + asset_manager_ = std::make_unique(*configuration); + auto metrics_publisher_store = std::make_unique(configuration, repo_metric_sources, flow_config, asset_manager_.get()); flowController_ = std::make_unique(test_repo, test_flow_repo, configuration, - std::move(flow_config), content_repo, std::move(metrics_publisher_store), filesystem, request_restart, asset_manager); + std::move(flow_config), content_repo, std::move(metrics_publisher_store), filesystem, request_restart, asset_manager_.get()); flowController_->load(); updateProperties(*flowController_); flowController_->start(); diff --git a/libminifi/test/libtest/integration/IntegrationBase.h b/libminifi/test/libtest/integration/IntegrationBase.h index 8c374159df..86a46e3f85 100644 --- a/libminifi/test/libtest/integration/IntegrationBase.h +++ b/libminifi/test/libtest/integration/IntegrationBase.h @@ -28,6 +28,7 @@ #include "core/ProcessGroup.h" #include "FlowController.h" #include "properties/Configure.h" +#include "utils/file/AssetManager.h" namespace minifi = org::apache::nifi::minifi; namespace core = minifi::core; @@ -107,6 +108,7 @@ class IntegrationBase { void configureSecurity(); std::shared_ptr configuration; + std::unique_ptr asset_manager_; std::unique_ptr response_node_loader_; std::unique_ptr flowController_; std::chrono::milliseconds wait_time_; diff --git a/minifi_main/MiNiFiMain.cpp b/minifi_main/MiNiFiMain.cpp index 92b5d41166..cf15f5633a 100644 --- a/minifi_main/MiNiFiMain.cpp +++ b/minifi_main/MiNiFiMain.cpp @@ -398,13 +398,13 @@ int main(int argc, char **argv) { .sensitive_values_encryptor = utils::crypto::EncryptionProvider::createSensitivePropertiesEncryptor(minifiHome) }, nifi_configuration_class_name); - auto asset_manager = std::make_shared(*configure); + auto asset_manager = std::make_unique(*configure); std::vector> repo_metric_sources{prov_repo, flow_repo, content_repo}; - auto metrics_publisher_store = std::make_unique(configure, repo_metric_sources, flow_configuration, asset_manager); + auto metrics_publisher_store = std::make_unique(configure, repo_metric_sources, flow_configuration, asset_manager.get()); const auto controller = std::make_unique( prov_repo, flow_repo, configure, std::move(flow_configuration), content_repo, - std::move(metrics_publisher_store), filesystem, request_restart, asset_manager); + std::move(metrics_publisher_store), filesystem, request_restart, asset_manager.get()); const bool disk_space_watchdog_enable = configure->get(minifi::Configure::minifi_disk_space_watchdog_enable) | utils::andThen(utils::string::toBool) From a92d007d83fb6ad40b4cd36943e8c9dd1a84b282 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 24 Jul 2024 11:04:11 +0200 Subject: [PATCH 44/48] MINIFICPP-2314 - Fix state reload --- libminifi/src/utils/file/AssetManager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index fde2b838fc..317fe3f88b 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -93,11 +93,11 @@ void AssetManager::refreshState() { } description.url = std::string{entry["url"].GetString(), entry["url"].GetStringLength()}; - if (FileUtils::exists(root_ / description.id)) { + if (FileUtils::exists(root_ / description.path)) { new_state.assets.insert(std::move(description)); } else { logger_->log_error("Asset '.state' file contains entry '{}' that does not exist on the filesystem at '{}'", - std::string_view{id.GetString(), id.GetStringLength()}, (root_ / description.id).string()); + std::string_view{id.GetString(), id.GetStringLength()}, (root_ / description.path).string()); } } state_ = std::move(new_state); From 61eb6848622ad4954c46493cfee70cbb03d9c249 Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 24 Jul 2024 11:42:22 +0200 Subject: [PATCH 45/48] MINIFICPP-2314 - Remove moves --- libminifi/src/FlowController.cpp | 2 +- libminifi/src/c2/C2Agent.cpp | 2 +- libminifi/src/core/state/MetricsPublisherStore.cpp | 2 +- libminifi/src/core/state/nodes/AssetInformation.cpp | 2 +- libminifi/src/core/state/nodes/ResponseNodeLoader.cpp | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libminifi/src/FlowController.cpp b/libminifi/src/FlowController.cpp index b8d35deaa2..39c24e3208 100644 --- a/libminifi/src/FlowController.cpp +++ b/libminifi/src/FlowController.cpp @@ -83,7 +83,7 @@ FlowController::FlowController(std::shared_ptr provenance_repo if (auto publisher = metrics_publisher_store_->getMetricsPublisher(c2::C2_METRICS_PUBLISHER).lock()) { c2_metrics_publisher = std::dynamic_pointer_cast(publisher); } - c2_agent_ = std::make_unique(configuration_, c2_metrics_publisher, std::move(filesystem), std::move(request_restart), std::move(asset_manager)); + c2_agent_ = std::make_unique(configuration_, c2_metrics_publisher, std::move(filesystem), std::move(request_restart), asset_manager); } if (c2::isControllerSocketEnabled(configuration_)) { diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index be2ff34d9c..ce077a18f1 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -66,7 +66,7 @@ C2Agent::C2Agent(std::shared_ptr configuration, thread_pool_(2, nullptr, "C2 threadpool"), request_restart_(std::move(request_restart)), last_run_(std::chrono::steady_clock::now()), - asset_manager_(std::move(asset_manager)) { + asset_manager_(asset_manager) { if (!configuration_->getAgentClass()) { logger_->log_info("Agent class is not predefined"); } diff --git a/libminifi/src/core/state/MetricsPublisherStore.cpp b/libminifi/src/core/state/MetricsPublisherStore.cpp index a436c2595b..abd1550d88 100644 --- a/libminifi/src/core/state/MetricsPublisherStore.cpp +++ b/libminifi/src/core/state/MetricsPublisherStore.cpp @@ -25,7 +25,7 @@ namespace org::apache::nifi::minifi::state { MetricsPublisherStore::MetricsPublisherStore(std::shared_ptr configuration, const std::vector>& repository_metric_sources, std::shared_ptr flow_configuration, utils::file::AssetManager* asset_manager) : configuration_(configuration), - response_node_loader_(std::make_shared(std::move(configuration), repository_metric_sources, std::move(flow_configuration), std::move(asset_manager))) { + response_node_loader_(std::make_shared(std::move(configuration), repository_metric_sources, std::move(flow_configuration), asset_manager)) { } void MetricsPublisherStore::initialize(core::controller::ControllerServiceProvider* controller, state::StateMonitor* update_sink) { diff --git a/libminifi/src/core/state/nodes/AssetInformation.cpp b/libminifi/src/core/state/nodes/AssetInformation.cpp index 76997df75a..4a243b607c 100644 --- a/libminifi/src/core/state/nodes/AssetInformation.cpp +++ b/libminifi/src/core/state/nodes/AssetInformation.cpp @@ -25,7 +25,7 @@ AssetInformation::AssetInformation() : logger_(core::logging::LoggerFactory().getLogger()) {} void AssetInformation::setAssetManager(utils::file::AssetManager* asset_manager) { - asset_manager_ = std::move(asset_manager); + asset_manager_ = asset_manager; if (!asset_manager_) { logger_->log_error("No asset manager is provided, asset information will not be available"); } diff --git a/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp b/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp index 184248c218..8d9dc1845a 100644 --- a/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp +++ b/libminifi/src/core/state/nodes/ResponseNodeLoader.cpp @@ -38,7 +38,7 @@ ResponseNodeLoader::ResponseNodeLoader(std::shared_ptr configuration, : configuration_(std::move(configuration)), repository_metric_sources_(std::move(repository_metric_sources)), flow_configuration_(std::move(flow_configuration)), - asset_manager_(std::move(asset_manager)) { + asset_manager_(asset_manager) { } void ResponseNodeLoader::clearConfigRoot() { From 323bed5736e2acdd22383bf9fda2d8549e2d802f Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 24 Jul 2024 12:37:20 +0200 Subject: [PATCH 46/48] MINIFICPP-2314 - Init field --- libminifi/include/core/state/nodes/AssetInformation.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/include/core/state/nodes/AssetInformation.h b/libminifi/include/core/state/nodes/AssetInformation.h index 247264c35c..feffa38a0b 100644 --- a/libminifi/include/core/state/nodes/AssetInformation.h +++ b/libminifi/include/core/state/nodes/AssetInformation.h @@ -35,7 +35,7 @@ class AssetInformation : public ResponseNode { std::vector serialize() override; private: - utils::file::AssetManager* asset_manager_; + utils::file::AssetManager* asset_manager_{nullptr}; std::shared_ptr logger_; }; From 5928c8b9ddda06dc050fb5d7c766e74f08cf9361 Mon Sep 17 00:00:00 2001 From: adamdebreceni <64783590+adamdebreceni@users.noreply.github.com> Date: Wed, 24 Jul 2024 16:59:47 +0200 Subject: [PATCH 47/48] Update libminifi/include/core/state/nodes/ResponseNodeLoader.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Márton Szász --- libminifi/include/core/state/nodes/ResponseNodeLoader.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libminifi/include/core/state/nodes/ResponseNodeLoader.h b/libminifi/include/core/state/nodes/ResponseNodeLoader.h index 4949bec234..9eb55f413e 100644 --- a/libminifi/include/core/state/nodes/ResponseNodeLoader.h +++ b/libminifi/include/core/state/nodes/ResponseNodeLoader.h @@ -75,7 +75,7 @@ class ResponseNodeLoader { std::shared_ptr configuration_; std::vector> repository_metric_sources_; std::shared_ptr flow_configuration_; - utils::file::AssetManager* asset_manager_; + utils::file::AssetManager* asset_manager_{}; core::controller::ControllerServiceProvider* controller_{}; state::StateMonitor* update_sink_{}; std::shared_ptr logger_{core::logging::LoggerFactory::getLogger()}; From 1bd0f305d2a3fab24a9fd5531a45209b5d4eec8b Mon Sep 17 00:00:00 2001 From: Adam Debreceni Date: Wed, 24 Jul 2024 19:51:00 +0200 Subject: [PATCH 48/48] MINIFICPP-2314 - Add more logging --- libminifi/src/c2/C2Agent.cpp | 2 ++ libminifi/src/utils/file/AssetManager.cpp | 9 +++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/libminifi/src/c2/C2Agent.cpp b/libminifi/src/c2/C2Agent.cpp index ce077a18f1..009c4293a6 100644 --- a/libminifi/src/c2/C2Agent.cpp +++ b/libminifi/src/c2/C2Agent.cpp @@ -714,6 +714,7 @@ void C2Agent::handle_transfer(const C2ContentResponse &resp) { } void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse &resp) { + logger_->log_info("Requested resource synchronization"); auto send_error = [&] (std::string_view error) { logger_->log_error("{}", error); C2Payload response(Operation::acknowledge, state::UpdateState::SET_ERROR, resp.ident, true); @@ -813,6 +814,7 @@ void C2Agent::handle_sync(const org::apache::nifi::minifi::c2::C2ContentResponse return; } if (type.value() != "ASSET") { + logger_->log_info("Resource (id = '{}', name = '{}') with type '{}' is not yet supported", id.value(), name.value(), type.value()); continue; } auto path = get_member_str("resourcePath"); diff --git a/libminifi/src/utils/file/AssetManager.cpp b/libminifi/src/utils/file/AssetManager.cpp index 317fe3f88b..3444cb55c8 100644 --- a/libminifi/src/utils/file/AssetManager.cpp +++ b/libminifi/src/utils/file/AssetManager.cpp @@ -111,6 +111,7 @@ std::string AssetManager::hash() const { nonstd::expected AssetManager::sync( const AssetLayout& layout, const std::function, std::string>(std::string_view /*url*/)>& fetch) { + logger_->log_info("Synchronizing assets"); std::lock_guard lock(mtx_); AssetLayout new_state{ .digest = state_.digest, @@ -120,16 +121,16 @@ nonstd::expected AssetManager::sync( std::vector>> new_file_contents; for (auto& new_entry : layout.assets) { if (std::find_if(state_.assets.begin(), state_.assets.end(), [&] (auto& entry) {return entry.id == new_entry.id;}) == state_.assets.end()) { - logger_->log_debug("Fetching asset {} from {}", new_entry.id, new_entry.url); + logger_->log_info("Fetching asset (id = '{}', path = '{}') from {}", new_entry.id, new_entry.path.string(), new_entry.url); if (auto data = fetch(new_entry.url)) { new_file_contents.emplace_back(new_entry.path, data.value()); new_state.assets.insert(new_entry); } else { - logger_->log_error("Failed to fetch asset {} from {}: {}", new_entry.id, new_entry.url, data.error()); + logger_->log_error("Failed to fetch asset (id = '{}', path = '{}') from {}: {}", new_entry.id, new_entry.path.string(), new_entry.url, data.error()); fetch_errors += "Failed to fetch '" + new_entry.id + "' from '" + new_entry.url + "': " + data.error() + "\n"; } } else { - logger_->log_debug("Asset {} already exists", new_entry.id); + logger_->log_info("Asset (id = '{}', path = '{}') already exists", new_entry.id, new_entry.path.string()); new_state.assets.insert(new_entry); } } @@ -139,7 +140,7 @@ nonstd::expected AssetManager::sync( for (auto& old_entry : state_.assets) { if (std::find_if(layout.assets.begin(), layout.assets.end(), [&] (auto& entry) {return entry.id == old_entry.id;}) == layout.assets.end()) { - // we no longer need this asset + logger_->log_info("We no longer need asset (id = '{}', path = '{}')", old_entry.id, old_entry.path.string()); std::filesystem::remove(root_ / old_entry.path); } }