MINIFICPP-2342 Do not overwrite config files during an upgrade

README.md

@@ -457,8 +457,22 @@ The performance tests can similarly be enabled. To execute them and see their ou
 $ ctest --verbose -L performance
 ```
 
+
 ### Configuring
-The 'conf' directory in the installation root contains a template config.yml document, minifi.properties, and minifi-log.properties. Please see our [Configuration document](CONFIGURE.md) for details on how to configure agents.
+The 'conf' directory in the installation root contains all configuration files.
+
+The files conf/minifi.properties, conf/minifi-log.properties and conf/minifi-uid.properties contain key-value pair configuration settings;
+these are the default settings supplied by the latest MiNiFi version. If you would like to modify these, you should create a corresponding
+.d directory (e.g. conf/minifi.properties.d) and put your settings in a new file inside this directory. These files are read and applied
+in lexicographic order, after the default settings file.
+The Windows installer creates a conf/minifi.properties.d/10_installer.properties file, which contains C2 connection settings.
+If C2 is enabled and settings are added/modified from the C2 server, these will be saved in conf/minifi.properties.d/90_c2.properties.
+
+The conf/config.yml file contains the flow definition (i.e. the layout of processors, controller services etc). When you start MiNiFi for
+the first time, the flow will be fetched from the C2 server (if available), or a file containing an empty flow will be created by MiNiFi.
+
+Please see our [Configuration document](CONFIGURE.md) for details on how to configure agents.
+
 
 ### Installing as a service
 

conf/CMakeLists.txt

@@ -41,12 +41,6 @@ else()
     message(FATAL_ERROR "Invalid MINIFI_PACKAGING_TYPE")
 endif()
 
-configure_file(
-        config.yml.in
-        ${CMAKE_BINARY_DIR}/conf/config.yml
-        @ONLY
-)
-
 configure_file(
         minifi.properties.in
         ${CMAKE_BINARY_DIR}/conf/minifi.properties
@@ -75,15 +69,13 @@ if (MINIFI_PACKAGING_TYPE STREQUAL "RPM")
             ${CMAKE_BINARY_DIR}/conf/minifi.properties
             ${CMAKE_BINARY_DIR}/conf/minifi-log.properties
             ${CMAKE_BINARY_DIR}/conf/minifi-uid.properties
-            ${CMAKE_BINARY_DIR}/conf/config.yml
             DESTINATION /${CMAKE_INSTALL_SYSCONFDIR}/${PROJECT_NAME}
             COMPONENT bin)
 elseif (MINIFI_PACKAGING_TYPE STREQUAL "TGZ")
     install(FILES
             ${CMAKE_BINARY_DIR}/conf/minifi.properties
             ${CMAKE_BINARY_DIR}/conf/minifi-log.properties
             ${CMAKE_BINARY_DIR}/conf/minifi-uid.properties
-            ${CMAKE_BINARY_DIR}/conf/config.yml
             DESTINATION conf
             COMPONENT bin)
 else()

core-framework/include/utils/file/FileUtils.h

@@ -242,6 +242,12 @@ inline int copy_file(const std::filesystem::path& path_from, const std::filesyst
   return 0;
 }
 
+inline bool move_file(const std::filesystem::path& source_path, const std::filesystem::path& dest_path) {
+  std::error_code ec;
+  std::filesystem::rename(source_path, dest_path, ec);
+  return !ec;
+}
+
 inline void addFilesMatchingExtension(const std::shared_ptr<core::logging::Logger> &logger,
                                       const std::filesystem::path& originalPath,
                                       const std::filesystem::path& extension,

docker/test/integration/cluster/containers/MinifiContainer.py

@@ -238,9 +238,6 @@ def deploy(self):
         else:
             image = 'apacheminificpp:' + MinifiContainer.MINIFI_TAG_PREFIX + MinifiContainer.MINIFI_VERSION
 
-        if self.options.use_flow_config_from_url:
-            self.command = ["/bin/sh", "-c", "rm " + MinifiContainer.MINIFI_LOCATIONS.config_path + " && " + MinifiContainer.MINIFI_LOCATIONS.run_minifi_cmd]
-
         ports = {}
         if self.options.enable_prometheus or self.options.enable_prometheus_with_ssl:
             ports = {'9936/tcp': 9936}

extensions/standard-processors/tests/unit/ConfigurationTests.cpp

@@ -15,12 +15,26 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#include <unordered_set>
+
 #include "unit/TestBase.h"
 #include "unit/Catch.h"
-
 #include "properties/Configuration.h"
 #include "utils/Environment.h"
 
+namespace {
+bool settingsInFileAreAsExpected(const std::filesystem::path& file_name, const std::unordered_set<std::string>& expected_contents) {
+  std::unordered_set<std::string> actual_contents;
+  std::ifstream file{file_name};
+  if (file.is_open()) {
+    for (std::string line; std::getline(file, line); ) {
+      actual_contents.insert(line);
+    }
+  }
+  return expected_contents == actual_contents;
+}
+}  // namespace
+
 namespace org::apache::nifi::minifi::test {
 
 TEST_CASE("Configuration can merge lists of property names", "[mergeProperties]") {
@@ -59,122 +73,197 @@ TEST_CASE("Configuration can fix misconfigured timeperiod<->integer validated pr
   LogTestController::getInstance().setInfo<minifi::Properties>();
 
   TestController test_controller;
-  auto properties_path = test_controller.createTempDirectory() /  "test.properties";
+  const auto conf_directory = test_controller.createTempDirectory();
+  const auto original_properties_path = conf_directory / "test.properties";
+  const auto updated_properties_path = conf_directory / "test.properties.d" / PropertiesImpl::C2PropertiesFileName;
 
-  std::ofstream{properties_path}
+  std::ofstream{original_properties_path}
       << "nifi.c2.agent.heartbeat.period=1min\n"
       << "nifi.administrative.yield.duration=30000\n";
-  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(original_properties_path);
   const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::ConfigureImpl>();
 
-  configure->loadConfigureFile(properties_path);
+  configure->loadConfigureFile(original_properties_path);
   CHECK(configure->get("nifi.c2.agent.heartbeat.period") == "60000");
   CHECK(configure->get("nifi.administrative.yield.duration") == "30000 ms");
 
   {
-    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(properties_path));
-    std::ifstream properties_file(properties_path);
-    std::string first_line;
-    std::string second_line;
-    CHECK(std::getline(properties_file, first_line));
-    CHECK(std::getline(properties_file, second_line));
-    CHECK(first_line == "nifi.c2.agent.heartbeat.period=1min");
-    CHECK(second_line == "nifi.administrative.yield.duration=30000");
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(original_properties_path));
+    CHECK(settingsInFileAreAsExpected(original_properties_path, {"nifi.c2.agent.heartbeat.period=1min", "nifi.administrative.yield.duration=30000"}));
   }
 
   CHECK(configure->commitChanges());
 
   {
-    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
-    std::ifstream properties_file(properties_path);
-    std::string first_line;
-    std::string second_line;
-    CHECK(std::getline(properties_file, first_line));
-    CHECK(std::getline(properties_file, second_line));
-    CHECK(first_line == "nifi.c2.agent.heartbeat.period=60000");
-    CHECK(second_line == "nifi.administrative.yield.duration=30000 ms");
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(original_properties_path));
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(updated_properties_path));
+    CHECK(settingsInFileAreAsExpected(updated_properties_path, {"nifi.c2.agent.heartbeat.period=60000", "nifi.administrative.yield.duration=30000 ms"}));
   }
+
+  const std::shared_ptr<minifi::Configure> configure_reread = std::make_shared<minifi::ConfigureImpl>();
+  configure_reread->loadConfigureFile(original_properties_path);
+  CHECK(configure_reread->get("nifi.c2.agent.heartbeat.period") == "60000");
+  CHECK(configure_reread->get("nifi.administrative.yield.duration") == "30000 ms");
 }
 
 TEST_CASE("Configuration can fix misconfigured datasize<->integer validated properties") {
   LogTestController::getInstance().setInfo<minifi::Configure>();
   LogTestController::getInstance().setInfo<minifi::Properties>();
 
   TestController test_controller;
-  auto properties_path = test_controller.createTempDirectory() /  "test.properties";
+  const auto conf_directory = test_controller.createTempDirectory();
+  const auto original_properties_path = conf_directory / "test.properties";
+  const auto updated_properties_path = conf_directory / "test.properties.d" / PropertiesImpl::C2PropertiesFileName;
 
   {
-    std::ofstream properties_file(properties_path);
+    std::ofstream properties_file(original_properties_path);
     properties_file << "appender.rolling.max_file_size=6000" << std::endl;
     properties_file.close();
   }
-  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(original_properties_path);
   const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::ConfigureImpl>();
-
-  configure->loadConfigureFile(properties_path, "nifi.log.");
+  configure->loadConfigureFile(original_properties_path, "nifi.log.");
   CHECK(configure->get("appender.rolling.max_file_size") == "6000 B");
 
   {
-    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
-    std::ifstream properties_file(properties_path);
-    std::string first_line;
-    CHECK(std::getline(properties_file, first_line));
-    CHECK(first_line == "appender.rolling.max_file_size=6000");
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(original_properties_path));
+    CHECK(settingsInFileAreAsExpected(original_properties_path, {"appender.rolling.max_file_size=6000"}));
   }
 
   CHECK(configure->commitChanges());
 
   {
-    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
-    std::ifstream properties_file(properties_path);
-    std::string first_line;
-    CHECK(std::getline(properties_file, first_line));
-    CHECK(first_line == "appender.rolling.max_file_size=6000 B");
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(original_properties_path));
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(updated_properties_path));
+    CHECK(settingsInFileAreAsExpected(updated_properties_path, {"appender.rolling.max_file_size=6000 B"}));
   }
-}
 
+  const std::shared_ptr<minifi::Configure> configure_reread = std::make_shared<minifi::ConfigureImpl>();
+  configure_reread->loadConfigureFile(original_properties_path, "nifi.log.");
+  CHECK(configure_reread->get("appender.rolling.max_file_size") == "6000 B");
+}
 
 TEST_CASE("Configuration can fix misconfigured validated properties within environmental variables") {
   LogTestController::getInstance().setInfo<minifi::Configure>();
   LogTestController::getInstance().setInfo<minifi::Properties>();
+
   TestController test_controller;
-  auto properties_path = test_controller.createTempDirectory() /  "test.properties";
+  const auto conf_directory = test_controller.createTempDirectory();
+  const auto original_properties_path = conf_directory / "test.properties";
+  const auto updated_properties_path = conf_directory / "test.properties.d" / PropertiesImpl::C2PropertiesFileName;
 
   CHECK(minifi::utils::Environment::setEnvironmentVariable("SOME_VARIABLE", "4000"));
 
-  std::ofstream{properties_path}
+  std::ofstream{original_properties_path}
       << "compression.cached.log.max.size=${SOME_VARIABLE}\n"
       << "compression.compressed.log.max.size=3000\n";
-  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(original_properties_path);
   const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::ConfigureImpl>();
 
-  configure->loadConfigureFile(properties_path, "nifi.log.");
+  configure->loadConfigureFile(original_properties_path, "nifi.log.");
   CHECK(configure->get("compression.cached.log.max.size") == "4000 B");
   CHECK(configure->get("compression.compressed.log.max.size") == "3000 B");
 
   {
-    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
-    std::ifstream properties_file(properties_path);
-    std::string first_line;
-    std::string second_line;
-    CHECK(std::getline(properties_file, first_line));
-    CHECK(std::getline(properties_file, second_line));
-    CHECK(first_line == "compression.cached.log.max.size=${SOME_VARIABLE}");
-    CHECK(second_line == "compression.compressed.log.max.size=3000");
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(original_properties_path));
+    CHECK(settingsInFileAreAsExpected(original_properties_path, {"compression.cached.log.max.size=${SOME_VARIABLE}", "compression.compressed.log.max.size=3000"}));
   }
 
   CHECK(configure->commitChanges());
 
   {
-    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
-    std::ifstream properties_file(properties_path);
-    std::string first_line;
-    std::string second_line;
-    CHECK(std::getline(properties_file, first_line));
-    CHECK(std::getline(properties_file, second_line));
-    CHECK(first_line == "compression.cached.log.max.size=${SOME_VARIABLE}");
-    CHECK(second_line == "compression.compressed.log.max.size=3000 B");
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(original_properties_path));
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(updated_properties_path));
+    CHECK(settingsInFileAreAsExpected(updated_properties_path, {"compression.compressed.log.max.size=3000 B"}));
   }
+
+  const std::shared_ptr<minifi::Configure> configure_reread = std::make_shared<minifi::ConfigureImpl>();
+  configure_reread->loadConfigureFile(original_properties_path, "nifi.log.");
+  CHECK(configure_reread->get("compression.cached.log.max.size") == "4000 B");
+  CHECK(configure_reread->get("compression.compressed.log.max.size") == "3000 B");
+}
+
+TEST_CASE("Committing changes to a configuration creates a backup file") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  TestController test_controller;
+  const auto conf_directory = test_controller.createTempDirectory();
+  const auto original_properties_path = conf_directory / "test.properties";
+  const auto updated_properties_path = conf_directory / "test.properties.d" / PropertiesImpl::C2PropertiesFileName;
+  const auto backup_properties_path = [&]() {
+    auto path = updated_properties_path;
+    path += ".bak";
+    return path;
+  }();
+
+  std::ofstream{original_properties_path}
+      << "number.of.lions=7\n"
+      << "number.of.elephants=12\n"
+      << "number.of.giraffes=30\n";
+
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::ConfigureImpl>();
+  configure->loadConfigureFile(original_properties_path);
+
+  CHECK(configure->get("number.of.lions") == "7");
+  CHECK(configure->get("number.of.elephants") == "12");
+  CHECK(configure->get("number.of.giraffes") == "30");
+
+  configure->set("number.of.lions", "8");
+  CHECK(configure->commitChanges());
+  CHECK(settingsInFileAreAsExpected(updated_properties_path, {"number.of.lions=8"}));
+  CHECK_FALSE(std::filesystem::exists(backup_properties_path));
+
+  const std::shared_ptr<minifi::Configure> configure_2 = std::make_shared<minifi::ConfigureImpl>();
+  configure_2->loadConfigureFile(original_properties_path);
+  CHECK(configure_2->get("number.of.lions") == "8");
+  CHECK(configure_2->get("number.of.elephants") == "12");
+  CHECK(configure_2->get("number.of.giraffes") == "30");
+
+  configure->set("number.of.giraffes", "29");
+  CHECK(configure->commitChanges());
+  CHECK(settingsInFileAreAsExpected(updated_properties_path, {"number.of.lions=8", "number.of.giraffes=29"}));
+  CHECK(settingsInFileAreAsExpected(backup_properties_path, {"number.of.lions=8"}));
+
+  const std::shared_ptr<minifi::Configure> configure_3 = std::make_shared<minifi::ConfigureImpl>();
+  configure_3->loadConfigureFile(original_properties_path);
+  CHECK(configure_3->get("number.of.lions") == "8");
+  CHECK(configure_3->get("number.of.elephants") == "12");
+  CHECK(configure_3->get("number.of.giraffes") == "29");
+}
+
+TEST_CASE("Backup file are skipped when reading config files") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  TestController test_controller;
+  const auto conf_directory = test_controller.createTempDirectory();
+  const auto original_properties_path = conf_directory / "test.properties";
+  const auto updated_properties_path = conf_directory / "test.properties.d" / PropertiesImpl::C2PropertiesFileName;
+  const auto backup_properties_path = [&]() {
+    auto path = updated_properties_path;
+    path += ".bak";
+    return path;
+  }();
+
+  std::ofstream{original_properties_path}
+      << "number.of.lions=7\n"
+      << "number.of.elephants=12\n";
+
+  utils::file::create_dir(updated_properties_path.parent_path());
+  std::ofstream{updated_properties_path}
+      << "number.of.lions=8\n";
+
+  std::ofstream{backup_properties_path}
+      << "number.of.elephants=20\n"
+      << "number.of.giraffes=30\n";
+
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::ConfigureImpl>();
+  configure->loadConfigureFile(original_properties_path);
+
+  CHECK(configure->get("number.of.lions") == "8");
+  CHECK(configure->get("number.of.elephants") == "12");
+  CHECK_FALSE(configure->get("number.of.giraffes"));
 }
 
 }  // namespace org::apache::nifi::minifi::test

libminifi/include/core/logging/LoggerProperties.h

@@ -32,7 +32,7 @@ namespace org::apache::nifi::minifi::core::logging {
 class LoggerProperties : public PropertiesImpl {
  public:
   explicit LoggerProperties(std::filesystem::path default_log_dir)
-      : PropertiesImpl("Logger properties"),
+      : PropertiesImpl(PersistTo::MultipleFiles, "Logger properties"),
         default_log_dir_(std::move(default_log_dir)) {
   }
   /**

libminifi/include/properties/Configuration.h

@@ -34,7 +34,7 @@ class PropertyValidator;
 
 class ConfigurationImpl : public PropertiesImpl, public virtual Configuration {
  public:
-  ConfigurationImpl() : PropertiesImpl("MiNiFi configuration") {}
+  ConfigurationImpl() : PropertiesImpl(PersistTo::MultipleFiles, "MiNiFi configuration") {}
 };
 
 }  // namespace org::apache::nifi::minifi