lordgamez commented on a change in pull request #1253:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1253#discussion_r805990249
##########
File path: libminifi/src/FlowController.cpp
##########
@@ -431,8 +431,9 @@ int16_t FlowController::clearConnection(const std::string
&connection) {
return -1;
}
-std::shared_ptr<state::response::ResponseNode>
FlowController::getAgentManifest() const {
+std::shared_ptr<state::response::ResponseNode>
FlowController::getAgentManifest() {
auto agentInfo =
std::make_shared<state::response::AgentInformation>("agentInfo");
+
agentInfo->setUpdatePolicyController(std::static_pointer_cast<controllers::UpdatePolicyControllerService>(getControllerService(c2::C2Agent::UPDATE_NAME)));
Review comment:
Good point, I was a bit hesitant changing it because it was very low
core level stuff and I thought it would be convoluted, but after trying it, it
seems these controller provider getters could be made const quite easily.
Updated it in 5e37b8b76b97c6e04ee145c80293b5557348ba66
##########
File path: extensions/http-curl/tests/C2NullConfiguration.cpp
##########
@@ -72,15 +72,15 @@ class VerifyC2Server : public HTTPIntegrationBase {
std::string port, scheme, path;
parse_http_components(url, port, scheme, path);
- configuration->set("c2.enable", "true");
- configuration->set("c2.agent.class", "test");
- configuration->set("c2.agent.protocol.class", "RESTSender");
- configuration->set("c2.rest.url", "");
- configuration->set("c2.rest.url.ack", "");
- configuration->set("c2.agent.heartbeat.reporter.classes", "null");
- configuration->set("c2.rest.listener.port", "null");
- configuration->set("c2.agent.heartbeat.period", "null");
- configuration->set("c2.rest.listener.heartbeat.rooturi", "null");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_enable,
"true");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_class,
"test");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_protocol_class,
"RESTSender");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_url,
"");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_url_ack,
"");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_heartbeat_reporter_classes,
"null");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_listener_port,
"null");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_heartbeat_period,
"null");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_listener_heartbeat_rooturi,
"null");
Review comment:
Do you mean moving the VerifyC2Server class under minifi namespace or
something else?
##########
File path: extensions/http-curl/tests/C2NullConfiguration.cpp
##########
@@ -72,15 +72,15 @@ class VerifyC2Server : public HTTPIntegrationBase {
std::string port, scheme, path;
parse_http_components(url, port, scheme, path);
- configuration->set("c2.enable", "true");
- configuration->set("c2.agent.class", "test");
- configuration->set("c2.agent.protocol.class", "RESTSender");
- configuration->set("c2.rest.url", "");
- configuration->set("c2.rest.url.ack", "");
- configuration->set("c2.agent.heartbeat.reporter.classes", "null");
- configuration->set("c2.rest.listener.port", "null");
- configuration->set("c2.agent.heartbeat.period", "null");
- configuration->set("c2.rest.listener.heartbeat.rooturi", "null");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_enable,
"true");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_class,
"test");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_protocol_class,
"RESTSender");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_url,
"");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_url_ack,
"");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_heartbeat_reporter_classes,
"null");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_listener_port,
"null");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_heartbeat_period,
"null");
+
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_listener_heartbeat_rooturi,
"null");
Review comment:
I agree, I was just curious as it is not common currently, updated in
530a1d11cce113c61dbbee173aa515f7baf6b716
##########
File path: extensions/coap/controllerservice/CoapConnector.cpp
##########
@@ -67,7 +67,7 @@ void CoapConnectorService::onEnable() {
core::Property::StringToInt(port_str, port_);
} else {
// this is the case where we aren't being used in the context of a single
controller service.
- if (configuration_->get("nifi.c2.agent.coap.host", host_) &&
configuration_->get("nifi.c2.agent.coap.port", port_str)) {
+ if (configuration_->get(minifi::Configuration::nifi_c2_agent_coap_host,
host_) && configuration_->get(minifi::Configuration::nifi_c2_agent_coap_port,
port_str)) {
Review comment:
Update in 530a1d11cce113c61dbbee173aa515f7baf6b716
##########
File path: extensions/civetweb/protocols/RESTReceiver.cpp
##########
@@ -51,8 +52,8 @@ void
RESTReceiver::initialize(core::controller::ControllerServiceProvider* contr
logger_->log_trace("Initializing rest receiver");
if (nullptr != configuration_) {
std::string listeningPort, rootUri = "/", caCert;
- configuration_->get("nifi.c2.rest.listener.port", "c2.rest.listener.port",
listeningPort);
- configuration_->get("nifi.c2.rest.listener.cacert",
"c2.rest.listener.cacert", caCert);
+ configuration_->get(minifi::Configuration::nifi_c2_rest_listener_port,
"c2.rest.listener.port", listeningPort);
+ configuration_->get(minifi::Configuration::nifi_c2_rest_listener_cacert,
"c2.rest.listener.cacert", caCert);
Review comment:
Update in 530a1d11cce113c61dbbee173aa515f7baf6b716
##########
File path: encrypt-config/tests/ConfigFileTests.cpp
##########
@@ -112,21 +113,21 @@ TEST_CASE("ConfigFile can parse a simple config file",
"[encrypt-config][constru
TEST_CASE("ConfigFile can test whether a key is present",
"[encrypt-config][hasValue]") {
ConfigFile test_file{std::ifstream{"resources/minifi.properties"}};
- REQUIRE(test_file.hasValue("nifi.version"));
- REQUIRE(test_file.hasValue("nifi.c2.flow.id")); // present but blank
- REQUIRE(!test_file.hasValue("nifi.remote.input.secure")); // commented out
+
REQUIRE(test_file.hasValue(org::apache::nifi::minifi::Configuration::nifi_version));
+
REQUIRE(test_file.hasValue(org::apache::nifi::minifi::Configuration::nifi_c2_flow_id));
// present but blank
+
REQUIRE(!test_file.hasValue(org::apache::nifi::minifi::Configuration::nifi_remote_input_secure));
// commented out
Review comment:
Update in 530a1d11cce113c61dbbee173aa515f7baf6b716
##########
File path: encrypt-config/ConfigFile.cpp
##########
@@ -23,11 +23,12 @@
#include <optional>
#include "utils/StringUtils.h"
+#include "properties/Configuration.h"
namespace {
-constexpr std::array<const char*, 2>
DEFAULT_SENSITIVE_PROPERTIES{"nifi.security.client.pass.phrase",
-
"nifi.rest.api.password"};
-constexpr const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME =
"nifi.sensitive.props.additional.keys";
+constexpr std::array<const char*, 2>
DEFAULT_SENSITIVE_PROPERTIES{org::apache::nifi::minifi::Configuration::nifi_security_client_pass_phrase,
+
org::apache::nifi::minifi::Configuration::nifi_rest_api_password};
+constexpr const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME =
org::apache::nifi::minifi::Configuration::nifi_sensitive_props_additional_keys;
Review comment:
Update in 530a1d11cce113c61dbbee173aa515f7baf6b716
##########
File path: encrypt-config/ConfigFile.cpp
##########
@@ -23,11 +23,12 @@
#include <optional>
#include "utils/StringUtils.h"
+#include "properties/Configuration.h"
namespace {
-constexpr std::array<const char*, 2>
DEFAULT_SENSITIVE_PROPERTIES{"nifi.security.client.pass.phrase",
-
"nifi.rest.api.password"};
-constexpr const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME =
"nifi.sensitive.props.additional.keys";
+constexpr std::array<const char*, 2>
DEFAULT_SENSITIVE_PROPERTIES{org::apache::nifi::minifi::Configuration::nifi_security_client_pass_phrase,
+
org::apache::nifi::minifi::Configuration::nifi_rest_api_password};
+constexpr const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME =
org::apache::nifi::minifi::Configuration::nifi_sensitive_props_additional_keys;
Review comment:
Updated in 92ccbbbf2986de81653b471b0d0043ab988cc111
##########
File path: libminifi/include/properties/Configuration.h
##########
@@ -99,6 +139,30 @@ class Configuration : public Properties {
static constexpr const char *minifi_disk_space_watchdog_interval =
"minifi.disk.space.watchdog.interval";
static constexpr const char *minifi_disk_space_watchdog_stop_threshold =
"minifi.disk.space.watchdog.stop.threshold";
static constexpr const char *minifi_disk_space_watchdog_restart_threshold =
"minifi.disk.space.watchdog.restart.threshold";
+
+ // JNI options
+ static constexpr const char *nifi_framework_dir = "nifi.framework.dir";
+ static constexpr const char *nifi_jvm_options = "nifi.jvm.options";
+ static constexpr const char *nifi_nar_directory = "nifi.nar.directory";
+ static constexpr const char *nifi_nar_deploy_directory =
"nifi.nar.deploy.directory";
+
+ // Log options
+ static constexpr const char *nifi_log_spdlog_pattern =
"nifi.log.spdlog.pattern";
+ static constexpr const char *nifi_log_spdlog_shorten_names =
"nifi.log.spdlog.shorten_names";
+ static constexpr const char *nifi_log_appender_rolling =
"nifi.log.appender.rolling";
+ static constexpr const char *nifi_log_appender_rolling_directory =
"nifi.log.appender.rolling.directory";
+ static constexpr const char *nifi_log_appender_rolling_file_name =
"nifi.log.appender.rolling.file_name";
+ static constexpr const char *nifi_log_appender_rolling_max_files =
"nifi.log.appender.rolling.max_files";
+ static constexpr const char *nifi_log_appender_rolling_max_file_size =
"nifi.log.appender.rolling.max_file_size";
+ static constexpr const char *nifi_log_appender_stdout =
"nifi.log.appender.stdout";
+ static constexpr const char *nifi_log_appender_stderr =
"nifi.log.appender.stderr";
+ static constexpr const char *nifi_log_appender_null =
"nifi.log.appender.null";
+ static constexpr const char *nifi_log_appender_syslog =
"nifi.log.appender.syslog";
+ static constexpr const char *nifi_log_logger_root = "nifi.log.logger.root";
+ static constexpr const char *nifi_log_compression_cached_log_max_size =
"nifi.log.compression.cached.log.max.size";
+ static constexpr const char *nifi_log_compression_compressed_log_max_size =
"nifi.log.compression.compressed.log.max.size";
+
+ MINIFIAPI static const std::vector<core::ConfigurationProperty>
CONFIGURATION_PROPERTIES;
Review comment:
The reason the vector was introduced to be able to iterate through all
the available properties to be added to the agent manifest.
##########
File path: libminifi/src/c2/C2Agent.cpp
##########
@@ -334,44 +334,7 @@ struct C2DebugBundleError : public C2TransferError {
void C2Agent::handle_c2_server_response(const C2ContentResponse &resp) {
switch (resp.op.value()) {
case Operation::CLEAR:
- // we've been told to clear something
- if (resp.name == "connection") {
- for (const auto& connection : resp.operation_arguments) {
- logger_->log_debug("Clearing connection %s",
connection.second.to_string());
- update_sink_->clearConnection(connection.second.to_string());
- }
- C2Payload response(Operation::ACKNOWLEDGE, resp.ident, true);
- enqueue_c2_response(std::move(response));
- } else if (resp.name == "repositories") {
- update_sink_->drainRepositories();
- C2Payload response(Operation::ACKNOWLEDGE, resp.ident, true);
- enqueue_c2_response(std::move(response));
- } else if (resp.name == "corecomponentstate") {
- // TODO(bakaid): untested
- std::vector<std::shared_ptr<state::StateController>> components =
update_sink_->getComponents(resp.name);
- auto state_manager_provider =
core::ProcessContext::getStateManagerProvider(logger_, controller_,
configuration_);
- if (state_manager_provider != nullptr) {
- for (auto &component : components) {
- logger_->log_debug("Clearing state for component %s",
component->getComponentName());
- auto state_manager =
state_manager_provider->getCoreComponentStateManager(component->getComponentUUID());
- if (state_manager != nullptr) {
- component->stop();
- state_manager->clear();
- state_manager->persist();
- component->start();
- } else {
- logger_->log_warn("Failed to get StateManager for component %s",
component->getComponentUUID().to_string());
- }
- }
- } else {
- logger_->log_error("Failed to get StateManagerProvider");
- }
- C2Payload response(Operation::ACKNOWLEDGE, resp.ident, true);
- enqueue_c2_response(std::move(response));
- } else {
- logger_->log_debug("Clearing unknown %s", resp.name);
- }
-
+ handle_clear(resp);
Review comment:
The definition of the function changed in both PRs in different ways, so
it will definitely conflict either way. I would keep it this way for now and
would resolve the conflict when the other PR is merged.
##########
File path: encrypt-config/tests/ConfigFileEncryptorTests.cpp
##########
@@ -76,13 +77,13 @@ TEST_CASE("ConfigFileEncryptor can encrypt the sensitive
properties", "[encrypt-
SECTION("default properties") {
ConfigFile test_file{std::ifstream{"resources/minifi.properties"}};
- std::string original_password =
test_file.getValue("nifi.rest.api.password").value();
+ std::string original_password =
test_file.getValue(org::apache::nifi::minifi::Configuration::nifi_rest_api_password).value();
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: encrypt-config/ConfigFile.cpp
##########
@@ -23,11 +23,10 @@
#include <optional>
#include "utils/StringUtils.h"
+#include "properties/Configuration.h"
namespace {
-constexpr std::array<const char*, 2>
DEFAULT_SENSITIVE_PROPERTIES{"nifi.security.client.pass.phrase",
-
"nifi.rest.api.password"};
-constexpr const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME =
"nifi.sensitive.props.additional.keys";
+
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: extensions/http-curl/protocols/RESTSender.cpp
##########
@@ -43,9 +44,9 @@ void
RESTSender::initialize(core::controller::ControllerServiceProvider* control
// base URL when one is not specified.
if (nullptr != configure) {
std::string update_str, ssl_context_service_str;
- configure->get("nifi.c2.rest.url", "c2.rest.url", rest_uri_);
- configure->get("nifi.c2.rest.url.ack", "c2.rest.url.ack", ack_uri_);
- if (configure->get("nifi.c2.rest.ssl.context.service",
"c2.rest.ssl.context.service", ssl_context_service_str)) {
+ configure->get(minifi::Configuration::nifi_c2_rest_url, "c2.rest.url",
rest_uri_);
+ configure->get(minifi::Configuration::nifi_c2_rest_url_ack,
"c2.rest.url.ack", ack_uri_);
+ if
(configure->get(minifi::Configuration::nifi_c2_rest_ssl_context_service,
"c2.rest.ssl.context.service", ssl_context_service_str)) {
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: extensions/http-curl/protocols/RESTSender.cpp
##########
@@ -74,8 +75,8 @@ C2Payload RESTSender::consumePayload(const C2Payload
&payload, Direction directi
void RESTSender::update(const std::shared_ptr<Configure> &configure) {
std::string url;
- configure->get("nifi.c2.rest.url", "c2.rest.url", url);
- configure->get("nifi.c2.rest.url.ack", "c2.rest.url.ack", url);
+ configure->get(minifi::Configuration::nifi_c2_rest_url, "c2.rest.url", url);
+ configure->get(minifi::Configuration::nifi_c2_rest_url_ack,
"c2.rest.url.ack", url);
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: libminifi/include/core/Property.h
##########
@@ -467,6 +468,16 @@ class ConstrainedProperty : public
std::enable_shared_from_this<ConstrainedPrope
friend class PropertyBuilder;
};
+struct ConfigurationProperty {
+ explicit ConfigurationProperty(std::string_view name_, const
gsl::not_null<std::shared_ptr<PropertyValidator>>& validator_ =
StandardValidators::get().VALID_VALIDATOR)
+ : name(name_),
+ validator(validator_) {
+ }
+
+ std::string_view name;
+ gsl::not_null<std::shared_ptr<PropertyValidator>> validator;
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: libminifi/src/Configure.cpp
##########
@@ -68,15 +69,15 @@ bool Configure::isEncrypted(const std::string& key) const {
std::optional<std::string> Configure::getAgentClass() const {
std::string agent_class;
- if (get("nifi.c2.agent.class", "c2.agent.class", agent_class) &&
!agent_class.empty()) {
+ if (get(minifi::Configuration::nifi_c2_agent_class, "c2.agent.class",
agent_class) && !agent_class.empty()) {
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: libminifi/src/Configure.cpp
##########
@@ -68,15 +69,15 @@ bool Configure::isEncrypted(const std::string& key) const {
std::optional<std::string> Configure::getAgentClass() const {
std::string agent_class;
- if (get("nifi.c2.agent.class", "c2.agent.class", agent_class) &&
!agent_class.empty()) {
+ if (get(minifi::Configuration::nifi_c2_agent_class, "c2.agent.class",
agent_class) && !agent_class.empty()) {
return agent_class;
}
return {};
}
std::string Configure::getAgentIdentifier() const {
std::string agent_id;
- if (!get("nifi.c2.agent.identifier", "c2.agent.identifier", agent_id) ||
agent_id.empty()) {
+ if (!get(minifi::Configuration::nifi_c2_agent_identifier,
"c2.agent.identifier", agent_id) || agent_id.empty()) {
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: libminifi/src/c2/C2Agent.cpp
##########
@@ -138,7 +138,7 @@ void C2Agent::configure(const std::shared_ptr<Configure>
&configure, bool reconf
std::string clazz, heartbeat_period, device;
if (!reconfigure) {
- if (!configure->get("nifi.c2.agent.protocol.class",
"c2.agent.protocol.class", clazz)) {
+ if (!configure->get(minifi::Configuration::nifi_c2_agent_protocol_class,
"c2.agent.protocol.class", clazz)) {
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: libminifi/src/c2/C2Agent.cpp
##########
@@ -164,7 +164,7 @@ void C2Agent::configure(const std::shared_ptr<Configure>
&configure, bool reconf
protocol_.load()->update(configure);
}
- if (configure->get("nifi.c2.agent.heartbeat.period",
"c2.agent.heartbeat.period", heartbeat_period)) {
+ if (configure->get(minifi::Configuration::nifi_c2_agent_heartbeat_period,
"c2.agent.heartbeat.period", heartbeat_period)) {
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: libminifi/src/c2/C2Agent.cpp
##########
@@ -181,7 +181,7 @@ void C2Agent::configure(const std::shared_ptr<Configure>
&configure, bool reconf
}
std::string heartbeat_reporters;
- if (configure->get("nifi.c2.agent.heartbeat.reporter.classes",
"c2.agent.heartbeat.reporter.classes", heartbeat_reporters)) {
+ if
(configure->get(minifi::Configuration::nifi_c2_agent_heartbeat_reporter_classes,
"c2.agent.heartbeat.reporter.classes", heartbeat_reporters)) {
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
##########
File path: libminifi/test/unit/SocketTests.cpp
##########
@@ -221,7 +222,7 @@ TEST_CASE("TestTLSContextCreation", "[TestSocket8]") {
*/
TEST_CASE("TestTLSContextCreation2", "[TestSocket9]") {
std::shared_ptr<minifi::Configure> configure =
std::make_shared<minifi::Configure>();
- configure->set("nifi.remote.input.secure", "false");
+
configure->set(org::apache::nifi::minifi::Configuration::nifi_remote_input_secure,
"false");
Review comment:
Updated in 107193764914b719074c45b1ce4b978c3d1292f4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
