OTA-2488: decouple the secondary base & pure virtual ifc from seconda…

…ry configuration

Signed-off-by: Mike Sul <ext-mykhaylo.sul@here.com>

src/aktualizr_secondary/aktualizr_secondary.cc

@@ -13,8 +13,7 @@
 
 class SecondaryAdapter : public Uptane::SecondaryInterface {
  public:
-  SecondaryAdapter(const Uptane::SecondaryConfig& sconfig_in, AktualizrSecondary& sec)
-      : SecondaryInterface(sconfig_in), secondary(sec) {}
+  SecondaryAdapter(AktualizrSecondary& sec) : secondary(sec) {}
   ~SecondaryAdapter() override = default;
 
   Uptane::EcuSerial getSerial() override { return secondary.getSerialResp(); }
@@ -35,8 +34,7 @@ class SecondaryAdapter : public Uptane::SecondaryInterface {
 AktualizrSecondary::AktualizrSecondary(const AktualizrSecondaryConfig& config,
                                        const std::shared_ptr<INvStorage>& storage)
     : AktualizrSecondaryCommon(config, storage),
-      socket_server_(std_::make_unique<SecondaryAdapter>(Uptane::SecondaryConfig(), *this),
-                     SocketFromSystemdOrPort(config.network.port)) {
+      socket_server_(std_::make_unique<SecondaryAdapter>(*this), SocketFromSystemdOrPort(config.network.port)) {
   // note: we don't use TlsConfig here and supply the default to
   // KeyManagerConf. Maybe we should figure a cleaner way to do that
   // (split KeyManager?)

src/aktualizr_secondary/update_test.cc

@@ -13,8 +13,7 @@ std::string sysroot;
 
 class ShortCircuitSecondary : public Uptane::SecondaryInterface {
  public:
-  ShortCircuitSecondary(const Uptane::SecondaryConfig& sconfig_in, AktualizrSecondary& sec)
-      : SecondaryInterface(sconfig_in), secondary(sec) {}
+  ShortCircuitSecondary(AktualizrSecondary& sec) : secondary(sec) {}
   ~ShortCircuitSecondary() override = default;
 
   Uptane::EcuSerial getSerial() override { return secondary.getSerialResp(); }
@@ -42,8 +41,7 @@ TEST(aktualizr_secondary_protocol, DISABLED_manual_update) {
   AktualizrSecondary as(config, storage);
 
   // secondary interface
-  Uptane::SecondaryConfig config_iface;
-  ShortCircuitSecondary sec_iface{config_iface, as};
+  ShortCircuitSecondary sec_iface{as};
 
   // storage
   TemporaryDirectory temp_dir;

src/libaktualizr-posix/ipuptanesecondary.h

@@ -19,9 +19,6 @@ class IpUptaneSecondary : public SecondaryInterface {
   explicit IpUptaneSecondary(const std::string& address, unsigned short port, EcuSerial serial,
                              HardwareIdentifier hw_id, PublicKey pub_key);
 
-  // what this method for ? Looks like should be removed out of SecondaryInterface
-  void Initialize() override{};
-
   // It looks more natural to return const EcuSerial& and const Uptane::HardwareIdentifier&
   // and they should be 'const' methods
   EcuSerial getSerial() /*const*/ override { return serial_; };

src/libaktualizr/CMakeLists.txt

@@ -43,4 +43,4 @@ target_include_directories(aktualizr_static_lib PUBLIC
     $<TARGET_PROPERTY:package_manager,INCLUDE_DIRECTORIES>)
 
 # To be removed once the refactoring is completed
-target_link_libraries(aktualizr_static_lib virtual-secondary)
+target_link_libraries(aktualizr_static_lib virtual_secondary)

src/libaktualizr/primary/CMakeLists.txt

@@ -16,8 +16,7 @@ target_include_directories(primary PUBLIC ${PROJECT_SOURCE_DIR}/src/virtual_seco
 
 add_aktualizr_test(NAME aktualizr SOURCES aktualizr_test.cc PROJECT_WORKING_DIRECTORY ARGS ${PROJECT_BINARY_DIR}/uptane_repos)
 add_dependencies(t_aktualizr uptane_repo_full_no_correlation_id)
-target_include_directories(t_aktualizr PUBLIC ${PROJECT_SOURCE_DIR}/src/virtual_secondary)
-target_link_libraries(t_aktualizr virtual-secondary)
+target_link_libraries(t_aktualizr virtual_secondary)
 
 if (BUILD_OSTREE)
     add_aktualizr_test(NAME aktualizr_fullostree SOURCES aktualizr_fullostree_test.cc PROJECT_WORKING_DIRECTORY ARGS $<TARGET_FILE:aktualizr-repo> ${PROJECT_BINARY_DIR}/ostree_repo)
@@ -37,6 +36,6 @@ set_tests_properties(test_device_cred_prov PROPERTIES LABELS "crypto")
 add_aktualizr_test(NAME uptane_key SOURCES uptane_key_test.cc PROJECT_WORKING_DIRECTORY)
 set_tests_properties(test_uptane_key PROPERTIES LABELS "crypto")
 target_include_directories(t_uptane_key PUBLIC ${PROJECT_SOURCE_DIR}/src/virtual_secondary)
-target_link_libraries(t_uptane_key virtual-secondary)
+target_link_libraries(t_uptane_key virtual_secondary)
 
 aktualizr_source_file_checks(${SOURCES} ${HEADERS} ${TEST_SOURCES})

src/libaktualizr/primary/initializer.cc

@@ -252,10 +252,6 @@ Initializer::Initializer(
       return;
     }
 
-    for (auto it = secondary_info_.begin(); it != secondary_info_.end(); ++it) {
-      it->second->Initialize();
-    }
-
     // TODO: acknowledge on server _before_ setting the flag
     storage_->storeEcuRegistered();
     success_ = true;

src/libaktualizr/primary/uptane_key_test.cc

@@ -87,7 +87,6 @@ class UptaneKey_Check_Test {
     // Verify that each secondary has valid keys.
     std::map<Uptane::EcuSerial, std::shared_ptr<Uptane::SecondaryInterface> >::iterator it;
     for (it = sota_client->secondaries.begin(); it != sota_client->secondaries.end(); it++) {
-      EXPECT_TRUE(it->second->sconfig.secondary_type == Uptane::SecondaryType::kVirtual);
       std::shared_ptr<Uptane::ManagedSecondary> managed =
           boost::polymorphic_pointer_downcast<Uptane::ManagedSecondary>(it->second);
       std::string public_key;

src/libaktualizr/uptane/isotpsecondary.cc

@@ -33,7 +33,7 @@ enum class IsoTpUptaneMesType {
 namespace Uptane {
 
 IsoTpSecondary::IsoTpSecondary(const SecondaryConfig& sconfig_in)
-    : SecondaryInterface(sconfig_in), conn(sconfig.can_iface, LIBUPTINY_ISOTP_PRIMARY_CANID, sconfig_in.can_id) {}
+    : conn(sconfig_in.can_iface, LIBUPTINY_ISOTP_PRIMARY_CANID, sconfig_in.can_id) {}
 
 EcuSerial IsoTpSecondary::getSerial() {
   std::string out;

src/libaktualizr/uptane/isotpsecondary.h

@@ -2,6 +2,7 @@
 #define UPTANE_ISOTPSECONDARY_H_
 
 #include "isotp_conn/isotp_conn.h"
+#include "secondaryconfig.h"
 #include "secondaryinterface.h"
 
 namespace Uptane {

src/libaktualizr/uptane/partialverificationsecondary.cc

@@ -14,8 +14,8 @@
 
 namespace Uptane {
 
-PartialVerificationSecondary::PartialVerificationSecondary(const SecondaryConfig &sconfig_in)
-    : SecondaryInterface(sconfig_in), root_(Root::Policy::kAcceptAll) {
+PartialVerificationSecondary::PartialVerificationSecondary(SecondaryConfig sconfig_in)
+    : sconfig(std::move(sconfig_in)), root_(Root::Policy::kAcceptAll) {
   boost::filesystem::create_directories(sconfig.metadata_path);
 
   // FIXME Probably we need to generate keys on the secondary

src/libaktualizr/uptane/partialverificationsecondary.h

@@ -15,14 +15,15 @@ namespace Uptane {
 
 class PartialVerificationSecondary : public SecondaryInterface {
  public:
-  explicit PartialVerificationSecondary(const SecondaryConfig& sconfig_in);
+  explicit PartialVerificationSecondary(SecondaryConfig sconfig_in);
 
   EcuSerial getSerial() override {
     if (!sconfig.ecu_serial.empty()) {
       return Uptane::EcuSerial(sconfig.ecu_serial);
     }
     return Uptane::EcuSerial(public_key_.KeyId());
   }
+  Uptane::HardwareIdentifier getHwId() override { return Uptane::HardwareIdentifier(sconfig.ecu_hardware_id); }
   PublicKey getPublicKey() override { return public_key_; }
 
   bool putMetadata(const RawMetaPack& meta) override;
@@ -36,6 +37,7 @@ class PartialVerificationSecondary : public SecondaryInterface {
   void storeKeys(const std::string& public_key, const std::string& private_key);
   bool loadKeys(std::string* public_key, std::string* private_key);
 
+  SecondaryConfig sconfig;
   Uptane::Root root_;
   PublicKey public_key_;
   std::string private_key_;

src/libaktualizr/uptane/secondaryinterface.h

@@ -6,7 +6,6 @@
 
 #include "json/json.h"
 
-#include "uptane/secondaryconfig.h"
 #include "uptane/tuf.h"
 
 /* Json snippet returned by sendMetaXXX():
@@ -22,14 +21,16 @@ class SecondaryInterface {
  public:
   // This ctor should be removed as the secondary configuration SecondaryConfig
   // is the secondaries's specific, see SecondaryConfig declaration
-  explicit SecondaryInterface(SecondaryConfig sconfig_in) : sconfig(std::move(sconfig_in)) {}
+  // explicit SecondaryInterface(SecondaryConfig sconfig_in) : sconfig(std::move(sconfig_in)) {}
   virtual ~SecondaryInterface() = default;
   // not clear what this method for, can be removed
-  virtual void Initialize(){};  // optional step, called after device registration
+  // virtual void Initialize(){};  // optional step, called after device registration
   // should be pure virtual, since the current implementation reads from the secondaries specific config
-  virtual EcuSerial getSerial() { return Uptane::EcuSerial(sconfig.ecu_serial); }
+  // virtual EcuSerial getSerial() { return Uptane::EcuSerial(sconfig.ecu_serial); }
+  virtual EcuSerial getSerial() = 0;
   // should be pure virtual, since the current implementation reads from the secondaries specific config
-  virtual Uptane::HardwareIdentifier getHwId() { return Uptane::HardwareIdentifier(sconfig.ecu_hardware_id); }
+  //  virtual Uptane::HardwareIdentifier getHwId() { return Uptane::HardwareIdentifier(sconfig.ecu_hardware_id); }
+  virtual Uptane::HardwareIdentifier getHwId() = 0;
   virtual PublicKey getPublicKey() = 0;
 
   // getSerial(), getHwId() and getPublicKey() can be moved to seperate interface
@@ -42,10 +43,10 @@ class SecondaryInterface {
   // FIXME: Instead of std::string we should use StorageTargetRHandle
   virtual bool sendFirmware(const std::shared_ptr<std::string>& data) = 0;
   // Should be removes as it's secondary specific
-  const SecondaryConfig sconfig;
+  // const SecondaryConfig sconfig;
 
- protected:
-  SecondaryInterface() : sconfig{} {};
+  // protected:
+  // SecondaryInterface() : sconfig{} {};
 };
 }  // namespace Uptane
 

src/libaktualizr/uptane/uptane_ci_test.cc

@@ -15,7 +15,7 @@
 #include "primary/reportqueue.h"
 #include "primary/sotauptaneclient.h"
 #include "storage/invstorage.h"
-#include "uptane/managedsecondary.h"
+//#include "managedsecondary.h"
 #include "uptane/uptanerepository.h"
 #include "utilities/utils.h"
 
@@ -86,18 +86,20 @@ TEST(UptaneCI, CheckKeys) {
   EXPECT_TRUE(primary_public.size() > 0);
   EXPECT_TRUE(primary_private.size() > 0);
 
-  std::map<Uptane::EcuSerial, std::shared_ptr<Uptane::SecondaryInterface> >::iterator it;
-  for (it = sota_client->secondaries.begin(); it != sota_client->secondaries.end(); it++) {
-    EXPECT_EQ(it->second->sconfig.secondary_type, Uptane::SecondaryType::kVirtual);
-    std::shared_ptr<Uptane::ManagedSecondary> managed =
-        boost::polymorphic_pointer_downcast<Uptane::ManagedSecondary>(it->second);
-    std::string public_key;
-    std::string private_key;
-    EXPECT_TRUE(managed->loadKeys(&public_key, &private_key));
-    EXPECT_TRUE(public_key.size() > 0);
-    EXPECT_TRUE(private_key.size() > 0);
-    EXPECT_NE(public_key, private_key);
-  }
+  // TODO: either remove or move to the virtual secondary folder as it's effectively test of
+  // the test code (managed secondary is used just for test)
+  //  std::map<Uptane::EcuSerial, std::shared_ptr<Uptane::SecondaryInterface> >::iterator it;
+  //  for (it = sota_client->secondaries.begin(); it != sota_client->secondaries.end(); it++) {
+  //    EXPECT_EQ(it->second->sconfig.secondary_type, Uptane::SecondaryType::kVirtual);
+  //    std::shared_ptr<Uptane::ManagedSecondary> managed =
+  //        boost::polymorphic_pointer_downcast<Uptane::ManagedSecondary>(it->second);
+  //    std::string public_key;
+  //    std::string private_key;
+  //    EXPECT_TRUE(managed->loadKeys(&public_key, &private_key));
+  //    EXPECT_TRUE(public_key.size() > 0);
+  //    EXPECT_TRUE(private_key.size() > 0);
+  //    EXPECT_NE(public_key, private_key);
+  //  }
 }
 
 #ifndef __NO_MAIN__

src/libaktualizr/uptane/uptane_test.cc

@@ -390,8 +390,7 @@ class HttpFakeEvents : public HttpFake {
 
 class SecondaryInterfaceMock : public Uptane::SecondaryInterface {
  public:
-  explicit SecondaryInterfaceMock(Uptane::SecondaryConfig sconfig_in)
-      : Uptane::SecondaryInterface(std::move(sconfig_in)) {
+  explicit SecondaryInterfaceMock(Uptane::SecondaryConfig sconfig_in) : sconfig(std::move(sconfig_in)) {
     std::string private_key, public_key;
     Crypto::generateKeyPair(sconfig.key_type, &public_key, &private_key);
     public_key_ = PublicKey(public_key, sconfig.key_type);
@@ -407,15 +406,28 @@ class SecondaryInterfaceMock : public Uptane::SecondaryInterface {
     manifest_["signed"] = manifest_unsigned;
     manifest_["signatures"].append(signature);
   }
-  PublicKey getPublicKey() { return public_key_; };
+  PublicKey getPublicKey() override { return public_key_; }
 
-  Json::Value getManifest() { return manifest_; }
-  MOCK_METHOD1(putMetadata, bool(const Uptane::RawMetaPack &));
-  MOCK_METHOD1(getRootVersion, int32_t(bool));
-  bool putRoot(const std::string &, bool) { return true; }
-  bool sendFirmware(const std::shared_ptr<std::string> &) { return true; }
+  Uptane::HardwareIdentifier getHwId() override { return Uptane::HardwareIdentifier(sconfig.ecu_hardware_id); }
+  Uptane::EcuSerial getSerial() override {
+    if (!sconfig.ecu_serial.empty()) {
+      return Uptane::EcuSerial(sconfig.ecu_serial);
+    }
+    return Uptane::EcuSerial(public_key_.KeyId());
+  }
+  Json::Value getManifest() override { return manifest_; }
+  MOCK_METHOD1(putMetadataMock, bool(const Uptane::RawMetaPack &));
+  MOCK_METHOD1(getRootVersionMock, int32_t(bool));
+
+  bool putMetadata(const Uptane::RawMetaPack &meta_pack) override { return putMetadataMock(meta_pack); }
+  int32_t getRootVersion(bool director) override { return getRootVersionMock(director); }
+
+  bool putRoot(const std::string &, bool) override { return true; }
+  bool sendFirmware(const std::shared_ptr<std::string> &) override { return true; }
   PublicKey public_key_;
   Json::Value manifest_;
+
+  const Uptane::SecondaryConfig sconfig;
 };
 
 MATCHER_P(matchMeta, meta, "") {
@@ -470,7 +482,7 @@ TEST(Uptane, SendMetadataToSeconadry) {
   storage->loadNonRoot(&meta.image_snapshot, Uptane::RepositoryType::Image(), Uptane::Role::Snapshot());
   storage->loadNonRoot(&meta.image_targets, Uptane::RepositoryType::Image(), Uptane::Role::Targets());
 
-  EXPECT_CALL(*sec, putMetadata(matchMeta(meta)));
+  EXPECT_CALL(*sec, putMetadataMock(matchMeta(meta)));
   up->uptaneInstall(packages_to_install);
   EXPECT_TRUE(EcuInstallationStartedReportGot);
 }

src/virtual_secondary/CMakeLists.txt

@@ -2,14 +2,13 @@ set(SOURCES managedsecondary.cc virtualsecondary.cc)
 
 set(HEADERS managedsecondary.h virtualsecondary.h)
 
-set(TARGET virtual-secondary)
+set(TARGET virtual_secondary)
 
 add_library(${TARGET} STATIC
   ${SOURCES}
 )
 
-# target_link_libraries(${TARGET} aktualizr_static_lib)
-
+target_include_directories(${TARGET} PUBLIC ${PROJECT_SOURCE_DIR}/src/virtual_secondary)
 add_aktualizr_test(NAME virtual_secondary SOURCES virtual_secondary_test.cc PROJECT_WORKING_DIRECTORY)
 
 aktualizr_source_file_checks(${HEADERS} ${SOURCES} ${TEST_SOURCES})

src/virtual_secondary/managedsecondary.cc

@@ -13,7 +13,7 @@
 #include <sys/stat.h>
 
 namespace Uptane {
-ManagedSecondary::ManagedSecondary(const SecondaryConfig &sconfig_in) : SecondaryInterface(sconfig_in) {
+ManagedSecondary::ManagedSecondary(SecondaryConfig sconfig_in) : sconfig(std::move(sconfig_in)) {
   // TODO: FIX
   // loadMetadata(meta_pack);
   std::string public_key_string;
@@ -28,6 +28,7 @@ ManagedSecondary::ManagedSecondary(const SecondaryConfig &sconfig_in) : Secondar
     // do not store keys yet, wait until SotaUptaneClient performed device initialization
   }
   public_key_ = PublicKey(public_key_string, sconfig.key_type);
+  Initialize();
 }
 
 void ManagedSecondary::Initialize() {

src/virtual_secondary/managedsecondary.h

@@ -20,17 +20,18 @@ namespace Uptane {
 
 class ManagedSecondary : public SecondaryInterface {
  public:
-  explicit ManagedSecondary(const SecondaryConfig& sconfig_in);
+  explicit ManagedSecondary(SecondaryConfig sconfig_in);
   ~ManagedSecondary() override = default;
 
-  void Initialize() override;
+  void Initialize();
 
   EcuSerial getSerial() override {
     if (!sconfig.ecu_serial.empty()) {
       return EcuSerial(sconfig.ecu_serial);
     }
     return EcuSerial(public_key_.KeyId());
   }
+  Uptane::HardwareIdentifier getHwId() override { return Uptane::HardwareIdentifier(sconfig.ecu_hardware_id); }
   PublicKey getPublicKey() override { return public_key_; }
   bool putMetadata(const RawMetaPack& meta_pack) override;
   int getRootVersion(bool director) override;
@@ -41,6 +42,9 @@ class ManagedSecondary : public SecondaryInterface {
 
   bool loadKeys(std::string* pub_key, std::string* priv_key);
 
+ protected:
+  SecondaryConfig sconfig;
+
  private:
   PublicKey public_key_;
   std::string private_key;