Merge pull request #1686 from advancedtelematic/feat/OTA-4984/seconda…

…ry-replacement

Feat/ota 4984/secondary replacement

src/aktualizr_info/aktualizr_info_test.cc

@@ -166,9 +166,8 @@ TEST_F(AktualizrInfoTest, PrintSecondaryNotRegisteredOrRemoved) {
   db_storage_->storeEcuSerials({{primary_ecu_serial, primary_hw_id}, {secondary_ecu_serial, secondary_hw_id}});
   db_storage_->storeEcuRegistered();
 
-  db_storage_->storeMisconfiguredEcus(
-      {{secondary_ecu_serial_not_reg, secondary_hw_id_not_reg, EcuState::kNotRegistered},
-       {secondary_ecu_serial_old, secondary_hw_id_old, EcuState::kOld}});
+  db_storage_->saveMisconfiguredEcu({secondary_ecu_serial_not_reg, secondary_hw_id_not_reg, EcuState::kUnused});
+  db_storage_->saveMisconfiguredEcu({secondary_ecu_serial_old, secondary_hw_id_old, EcuState::kOld});
 
   aktualizr_info_process_.run();
   ASSERT_FALSE(aktualizr_info_output.empty());

src/aktualizr_info/main.cc

@@ -357,7 +357,7 @@ int main(int argc, char **argv) {
     std::vector<MisconfiguredEcu> misconfigured_ecus;
     storage->loadMisconfiguredEcus(&misconfigured_ecus);
     if (misconfigured_ecus.size() != 0U) {
-      std::cout << "Removed or not registered ECUs:" << std::endl;
+      std::cout << "Removed or unregistered ECUs (deprecated):" << std::endl;
       std::vector<MisconfiguredEcu>::const_iterator it;
       for (it = misconfigured_ecus.begin(); it != misconfigured_ecus.end(); ++it) {
         std::cout << "   '" << it->serial << "' with hardware_id '" << it->hardware_id << "' "

src/aktualizr_primary/CMakeLists.txt

@@ -7,7 +7,9 @@ target_include_directories(aktualizr PUBLIC ${PROJECT_SOURCE_DIR}/third_party)
 
 install(TARGETS aktualizr RUNTIME DESTINATION bin COMPONENT aktualizr)
 
-add_aktualizr_test(NAME primary_secondary_registration SOURCES primary_secondary_registration_test.cc secondary.cc secondary_config.cc PROJECT_WORKING_DIRECTORY ARGS ${PROJECT_BINARY_DIR}/uptane_repos LIBRARIES PUBLIC aktualizr-posix virtual_secondary uptane_generator_lib)
+add_aktualizr_test(NAME primary_secondary_registration
+                   SOURCES primary_secondary_registration_test.cc secondary.cc secondary_config.cc
+                   PROJECT_WORKING_DIRECTORY LIBRARIES PUBLIC aktualizr-posix virtual_secondary uptane_generator_lib)
 target_include_directories(t_primary_secondary_registration PUBLIC ${PROJECT_SOURCE_DIR}/src/libaktualizr-posix)
 
 aktualizr_source_file_checks(${SOURCES} ${HEADERS} ${TEST_SOURCES})

src/aktualizr_primary/main.cc

@@ -131,7 +131,7 @@ int main(int argc, char *argv[]) {
       try {
         Primary::initSecondaries(aktualizr, config.uptane.secondary_config_file);
       } catch (const std::exception &e) {
-        LOG_ERROR << "Failed to initialize Secondaries :" << e.what();
+        LOG_ERROR << "Failed to initialize Secondaries: " << e.what();
         LOG_ERROR << "Exiting...";
         return EXIT_FAILURE;
       }

src/aktualizr_primary/primary_secondary_registration_test.cc

@@ -6,41 +6,39 @@
 #include "uptane_test_common.h"
 #include "utilities/utils.h"
 
-boost::filesystem::path uptane_repos_dir;
 boost::filesystem::path fake_meta_dir;
 
+/* This tests that a device that had an IP Secondary will still find it after
+ * recent changes, even if it does not connect when the device starts. Note that
+ * this is only supported for a single IP Secondary. */
 TEST(PrimarySecondaryReg, SecondariesMigration) {
-  // This tests that a device that had an ip secondary will still find it after
-  // recent changes, even if it does not connect when the device starts
+  const Uptane::EcuSerial primary_serial{"p_serial"};
+  const Uptane::EcuSerial secondary_serial{"s_serial"};
+  const Uptane::HardwareIdentifier primary_hwid{"p_hwid"};
+  const Uptane::HardwareIdentifier secondary_hwid{"s_hwid"};
 
   TemporaryDirectory temp_dir;
   auto http = std::make_shared<HttpFake>(temp_dir.Path(), "noupdates", fake_meta_dir);
-
   const auto& url = http->tls_server;
+
   Config conf("tests/config/basic.toml");
   conf.uptane.director_server = url + "/director";
   conf.uptane.repo_server = url + "/repo";
   conf.provision.server = url;
-  conf.provision.primary_ecu_serial = "CA:FE:A6:D2:84:9D";
-  conf.provision.primary_ecu_hardware_id = "primary_hw";
+  conf.provision.primary_ecu_serial = primary_serial.ToString();
+  conf.provision.primary_ecu_hardware_id = primary_hwid.ToString();
   conf.storage.path = temp_dir.Path();
   conf.import.base_path = temp_dir.Path() / "import";
   conf.tls.server = url;
   conf.bootloader.reboot_sentinel_dir = temp_dir.Path();
   const boost::filesystem::path sec_conf_path = temp_dir / "s_config.json";
   conf.uptane.secondary_config_file = sec_conf_path;
 
-  Json::Value sec_conf;
-
   auto storage = INvStorage::newStorage(conf.storage);
-
-  const Uptane::EcuSerial primary_serial{"p_serial"};
-  const Uptane::EcuSerial secondary_serial{"s_serial"};
-  const Uptane::HardwareIdentifier primary_hwid{"p_hwid"};
-  const Uptane::HardwareIdentifier secondary_hwid{"s_hwid"};
+  Json::Value sec_conf;
 
   {
-    // prepare storage the "old" way:
+    // Prepare storage the "old" way (without the secondary_ecus table):
     storage->storeDeviceId("device");
     storage->storeEcuSerials({{primary_serial, primary_hwid}, {secondary_serial, secondary_hwid}});
     storage->storeEcuRegistered();
@@ -53,9 +51,8 @@ TEST(PrimarySecondaryReg, SecondariesMigration) {
   }
 
   {
-    // verifies that aktualizr can still start if it can't connect to its
-    // secondary
-
+    // Verify that aktualizr can still start if it can't connect to its
+    // Secondary:
     UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
     Primary::initSecondaries(aktualizr, sec_conf_path);
     aktualizr.Initialize();
@@ -67,53 +64,11 @@ TEST(PrimarySecondaryReg, SecondariesMigration) {
     EXPECT_EQ(secs_info[0].type, "IP");
     EXPECT_EQ(secs_info[0].extra, R"({"ip":"127.0.0.1","port":9061})");
   }
-
-  const Uptane::EcuSerial secondary_serial2{"s_serial2"};
-  const Uptane::HardwareIdentifier secondary_hwid2{"s_hwid2"};
-  {
-    // prepare the storage the "old" way with two secondaries
-    storage->clearEcuSerials();
-    storage->storeEcuSerials({
-        {primary_serial, primary_hwid},
-        {secondary_serial, secondary_hwid},
-        {secondary_serial2, secondary_hwid2},
-    });
-
-    sec_conf["IP"]["secondaries"][1]["addr"] = "127.0.0.1:9062";
-    Utils::writeFile(sec_conf_path, sec_conf);
-  }
-
-  {
-    UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
-
-    // this will fail to actually register the secondaries as there is no way to
-    // tell them apart (since they haven't connected)
-    // however, we still allow aktualizr to go through in case we would like to
-    // update the primary, to maybe fix this situation
-    Primary::initSecondaries(aktualizr, sec_conf_path);
-    aktualizr.Initialize();
-    aktualizr.CheckUpdates().get();
-
-    // there was no way to correlate info here
-    std::vector<SecondaryInfo> secs_info;
-    storage->loadSecondariesInfo(&secs_info);
-    EXPECT_EQ(secs_info[0].serial.ToString(), secondary_serial.ToString());
-    EXPECT_EQ(secs_info[0].type, "");
-    EXPECT_EQ(secs_info[0].extra, "");
-    EXPECT_EQ(secs_info[1].serial.ToString(), secondary_serial2.ToString());
-    EXPECT_EQ(secs_info[1].type, "");
-    EXPECT_EQ(secs_info[1].extra, "");
-  }
 }
 
 #ifndef __NO_MAIN__
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
-  if (argc != 2) {
-    std::cerr << "Error: " << argv[0] << " requires the path to the base directory of Uptane repos.\n";
-    return EXIT_FAILURE;
-  }
-  uptane_repos_dir = argv[1];
 
   logger_init();
   logger_set_threshold(boost::log::trivial::trace);

src/aktualizr_primary/secondary.cc

@@ -49,7 +49,7 @@ void initSecondaries(Aktualizr& aktualizr, const boost::filesystem::path& config
 
   for (auto& config : secondary_configs) {
     try {
-      LOG_INFO << "Registering " << config->type() << " Secondaries...";
+      LOG_INFO << "Initializing " << config->type() << " Secondaries...";
       Secondaries secondaries = createSecondaries(*config, aktualizr);
 
       for (const auto& secondary : secondaries) {
@@ -66,8 +66,9 @@ void initSecondaries(Aktualizr& aktualizr, const boost::filesystem::path& config
 
 class SecondaryWaiter {
  public:
-  SecondaryWaiter(uint16_t wait_port, int timeout_s, Secondaries& secondaries)
-      : endpoint_{boost::asio::ip::tcp::v4(), wait_port},
+  SecondaryWaiter(Aktualizr& aktualizr, uint16_t wait_port, int timeout_s, Secondaries& secondaries)
+      : aktualizr_(aktualizr),
+        endpoint_{boost::asio::ip::tcp::v4(), wait_port},
         timeout_{static_cast<boost::posix_time::seconds>(timeout_s)},
         timer_{io_context_},
         connected_secondaries_(secondaries) {}
@@ -83,10 +84,10 @@ class SecondaryWaiter {
     timer_.async_wait([&](const boost::system::error_code& error_code) {
       if (!!error_code) {
         LOG_ERROR << "Wait for Secondaries has failed: " << error_code;
-        throw std::runtime_error("Error while waiting for Secondaries");
+        throw std::runtime_error("Error while waiting for IP Secondaries");
       } else {
         LOG_ERROR << "Timeout while waiting for Secondaries: " << error_code;
-        throw std::runtime_error("Timeout while waiting for Secondaries");
+        throw std::runtime_error("Timeout while waiting for IP Secondaries");
       }
       io_context_.stop();
     });
@@ -111,6 +112,11 @@ class SecondaryWaiter {
         auto secondary = Uptane::IpUptaneSecondary::create(sec_ip, sec_port, con_socket_.native_handle());
         if (secondary) {
           connected_secondaries_.push_back(secondary);
+          // set ip/port in the db so that we can match everything later
+          Json::Value d;
+          d["ip"] = sec_ip;
+          d["port"] = sec_port;
+          aktualizr_.SetSecondaryData(secondary->getSerial(), Utils::jsonToCanonicalStr(d));
         }
       } catch (const std::exception& exc) {
         LOG_ERROR << "Failed to initialize a Secondary: " << exc.what();
@@ -132,6 +138,8 @@ class SecondaryWaiter {
   static std::string key(const std::string& ip, uint16_t port) { return (ip + std::to_string(port)); }
 
  private:
+  Aktualizr& aktualizr_;
+
   boost::asio::io_service io_context_;
   boost::asio::ip::tcp::endpoint endpoint_;
   boost::asio::ip::tcp::acceptor acceptor_{io_context_, endpoint_};
@@ -143,86 +151,74 @@ class SecondaryWaiter {
   std::unordered_set<std::string> secondaries_to_wait_for_;
 };
 
+// Four options for each Secondary:
+// 1. Secondary is configured and stored: nothing to do.
+// 2. Secondary is configured but not stored: it must be new. Try to connect to get information and store it. This will
+// cause re-registration.
+// 3. Same as 2 but cannot connect: abort.
+// 4. Secondary is stored but not configured: it must have been removed. Skip it. This will cause re-registration.
 static Secondaries createIPSecondaries(const IPSecondariesConfig& config, Aktualizr& aktualizr) {
   Secondaries result;
-  const bool provision = !aktualizr.IsRegistered();
-
-  if (provision) {
-    SecondaryWaiter sec_waiter{config.secondaries_wait_port, config.secondaries_timeout_s, result};
-
-    for (const auto& ip_sec_cfg : config.secondaries_cfg) {
-      auto secondary = Uptane::IpUptaneSecondary::connectAndCreate(ip_sec_cfg.ip, ip_sec_cfg.port);
-      if (secondary) {
-        result.push_back(secondary);
-      } else {
-        sec_waiter.addSecondary(ip_sec_cfg.ip, ip_sec_cfg.port);
-      }
-    }
-
-    sec_waiter.wait();
+  Secondaries new_secondaries;
+  SecondaryWaiter sec_waiter{aktualizr, config.secondaries_wait_port, config.secondaries_timeout_s, result};
+  auto secondaries_info = aktualizr.GetSecondaries();
+
+  for (const auto& cfg : config.secondaries_cfg) {
+    Uptane::SecondaryInterface::Ptr secondary;
+    const SecondaryInfo* info = nullptr;
+
+    // Try to match the configured Secondaries to stored Secondaries.
+    auto f = std::find_if(secondaries_info.cbegin(), secondaries_info.cend(), [&cfg](const SecondaryInfo& i) {
+      Json::Value d = Utils::parseJSON(i.extra);
+      return d["ip"] == cfg.ip && d["port"] == cfg.port;
+    });
 
-    // set ip/port in the db so that we can match everything later
-    for (size_t k = 0; k < config.secondaries_cfg.size(); k++) {
-      const auto cfg = config.secondaries_cfg[k];
-      const auto sec = result[k];
+    if (f == secondaries_info.cend() && config.secondaries_cfg.size() == 1 && secondaries_info.size() == 1 &&
+        secondaries_info[0].extra.empty()) {
+      // /!\ backward compatibility: if we have just one Secondary in the old
+      // storage format (before we had the secondary_ecus table) and the
+      // configuration, migrate it to the new format.
+      info = &secondaries_info[0];
       Json::Value d;
       d["ip"] = cfg.ip;
       d["port"] = cfg.port;
-      aktualizr.SetSecondaryData(sec->getSerial(), Utils::jsonToCanonicalStr(d));
-    }
-  } else {
-    auto secondaries_info = aktualizr.GetSecondaries();
-
-    for (const auto& cfg : config.secondaries_cfg) {
-      Uptane::SecondaryInterface::Ptr secondary;
-      const SecondaryInfo* info = nullptr;
-
-      auto f = std::find_if(secondaries_info.cbegin(), secondaries_info.cend(), [&cfg](const SecondaryInfo& i) {
-        Json::Value d = Utils::parseJSON(i.extra);
-        return d["ip"] == cfg.ip && d["port"] == cfg.port;
-      });
-
-      if (f == secondaries_info.cend() && config.secondaries_cfg.size() == 1 && secondaries_info.size() == 1) {
-        // /!\ backward compatibility: handle the case with one secondary, but
-        // store the info for later anyway
-        info = &secondaries_info[0];
+      aktualizr.SetSecondaryData(info->serial, Utils::jsonToCanonicalStr(d));
+      LOG_INFO << "Migrated a single IP Secondary to new storage format.";
+    } else if (f == secondaries_info.cend()) {
+      // Secondary was not found in storage; it must be new.
+      secondary = Uptane::IpUptaneSecondary::connectAndCreate(cfg.ip, cfg.port);
+      if (secondary == nullptr) {
+        LOG_DEBUG << "Could not connect to IP Secondary at " << cfg.ip << ":" << cfg.port
+                  << "; now trying to wait for it.";
+        sec_waiter.addSecondary(cfg.ip, cfg.port);
+      } else {
+        result.push_back(secondary);
+        // set ip/port in the db so that we can match everything later
         Json::Value d;
         d["ip"] = cfg.ip;
         d["port"] = cfg.port;
-        aktualizr.SetSecondaryData(info->serial, Utils::jsonToCanonicalStr(d));
-        LOG_INFO << "Migrated single IP Secondary to new storage format";
-      } else if (f == secondaries_info.cend()) {
-        // Match the other way if we can
-        secondary = Uptane::IpUptaneSecondary::connectAndCreate(cfg.ip, cfg.port);
-        if (secondary == nullptr) {
-          LOG_ERROR << "Could not instantiate Secondary " << cfg.ip << ":" << cfg.port;
-          continue;
-        }
-        auto f_serial =
-            std::find_if(secondaries_info.cbegin(), secondaries_info.cend(),
-                         [&secondary](const SecondaryInfo& i) { return i.serial == secondary->getSerial(); });
-        if (f_serial == secondaries_info.cend()) {
-          LOG_ERROR << "Could not instantiate Secondary " << cfg.ip << ":" << cfg.port;
-          continue;
-        }
-        info = &(*f_serial);
-      } else {
-        info = &(*f);
+        aktualizr.SetSecondaryData(secondary->getSerial(), Utils::jsonToCanonicalStr(d));
       }
+      continue;
+    } else {
+      // The configured Secondary was found in storage.
+      info = &(*f);
+    }
 
+    if (secondary == nullptr) {
+      secondary =
+          Uptane::IpUptaneSecondary::connectAndCheck(cfg.ip, cfg.port, info->serial, info->hw_id, info->pub_key);
       if (secondary == nullptr) {
-        secondary =
-            Uptane::IpUptaneSecondary::connectAndCheck(cfg.ip, cfg.port, info->serial, info->hw_id, info->pub_key);
-      }
-
-      if (secondary != nullptr) {
-        result.push_back(secondary);
-      } else {
-        LOG_ERROR << "Could not instantiate Secondary " << info->serial;
+        throw std::runtime_error("Unable to connect to or verify IP Secondary at " + cfg.ip + ":" +
+                                 std::to_string(cfg.port));
       }
     }
+
+    result.push_back(secondary);
   }
 
+  sec_waiter.wait();
+
   return result;
 }
 

src/aktualizr_secondary/aktualizr_secondary.cc

@@ -199,7 +199,7 @@ bool AktualizrSecondary::doFullVerification(const Metadata& metadata) {
 
 void AktualizrSecondary::uptaneInitialize() {
   if (keys_->generateUptaneKeyPair().size() == 0) {
-    throw std::runtime_error("Failed to generate uptane key pair");
+    throw std::runtime_error("Failed to generate Uptane key pair");
   }
 
   // from uptane/initialize.cc but we only take care of our own serial/hwid

src/aktualizr_secondary/aktualizr_secondary_factory.cc

@@ -30,6 +30,8 @@ AktualizrSecondary::Ptr AktualizrSecondaryFactory::create(const AktualizrSeconda
 
     if (installed_version_res && !!current_version) {
       current_target_name = current_version->filename();
+    } else {
+      current_target_name = "unknown";
     }
 
     update_agent =