Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Wait for secondaries before installing #1533

Merged
merged 15 commits into from
Feb 14, 2020
Merged

Wait for secondaries before installing #1533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a3dba35
Get rid of ISOTP_SECONDARY_ENABLED
lbonn Jan 22, 2020
f50eebf
Remove one funny use of r-value references
lbonn Feb 10, 2020
6444360
Merge two similar methods in SotaUptaneClient
lbonn Jan 17, 2020
0556925
Remove `hw_ids` from SotaUptaneClient
lbonn Feb 10, 2020
fba444f
More flexible KeyType parsing
lbonn Jan 23, 2020
e0d4f60
Storage for additional secondaries information
lbonn Jan 21, 2020
3da6079
Allow ip secondary initialization even if not online on start
lbonn Jan 17, 2020
98860c3
Add a ping method on SecondaryInterface
lbonn Feb 5, 2020
f495ed9
Wait some time before secondaries to come online
lbonn Jan 15, 2020
c8f6ff1
Cache secondary manifest when not available
lbonn Jan 23, 2020
b1c226a
Test for primary waiting for secondary at install
lbonn Jan 24, 2020
72b47fb
Be more careful with secondary public keys
lbonn Feb 12, 2020
15034ce
Add ip secondary test with multiple secondaries
lbonn Feb 13, 2020
95b9d9b
Switch aktualizr-posix from OBJECT to STATIC library
lbonn Feb 14, 2020
ba40f33
Add test for migration of old ip secondaries systems
lbonn Feb 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,6 @@ if(BUILD_SOTA_TOOLS)
find_program(STRACE NAMES strace)
endif(BUILD_SOTA_TOOLS)

if(BUILD_ISOTP)
add_definitions(-DISOTP_SECONDARY_ENABLED)
endif(BUILD_ISOTP)

if(FAULT_INJECTION)
find_package(Libfiu REQUIRED)
add_definitions(-DFIU_ENABLE)
Expand Down
10 changes: 10 additions & 0 deletions config/sql/migration/migrate.23.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
-- Don't modify this! Create a new migration instead--see docs/schema-migrations.adoc
SAVEPOINT MIGRATION;

ALTER TABLE ecu_serials RENAME TO ecus;
CREATE TABLE secondary_ecus(serial TEXT PRIMARY KEY, sec_type TEXT, public_key_type TEXT, public_key TEXT, extra TEXT, manifest TEXT);

DELETE FROM version;
INSERT INTO version VALUES(23);

RELEASE MIGRATION;
10 changes: 10 additions & 0 deletions config/sql/rollback/rollback.23.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
-- Don't modify this! Create a new migration instead--see docs/schema-migrations.adoc
SAVEPOINT ROLLBACK_MIGRATION;

DROP TABLE secondary_ecus;
ALTER TABLE ecus RENAME TO ecu_serials;

DELETE FROM version;
INSERT INTO version VALUES(22);

RELEASE ROLLBACK_MIGRATION;
5 changes: 3 additions & 2 deletions config/sql/schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
CREATE TABLE version(version INTEGER);
INSERT INTO version(rowid,version) VALUES(1,22);
INSERT INTO version(rowid,version) VALUES(1,23);
CREATE TABLE device_info(unique_mark INTEGER PRIMARY KEY CHECK (unique_mark = 0), device_id TEXT, is_registered INTEGER NOT NULL DEFAULT 0 CHECK (is_registered IN (0,1)));
CREATE TABLE ecu_serials(id INTEGER PRIMARY KEY, serial TEXT UNIQUE, hardware_id TEXT NOT NULL, is_primary INTEGER NOT NULL DEFAULT 0 CHECK (is_primary IN (0,1)));
CREATE TABLE ecus(id INTEGER PRIMARY KEY, serial TEXT UNIQUE, hardware_id TEXT NOT NULL, is_primary INTEGER NOT NULL DEFAULT 0 CHECK (is_primary IN (0,1)));
CREATE TABLE secondary_ecus(serial TEXT PRIMARY KEY, sec_type TEXT, public_key_type TEXT, public_key TEXT, extra TEXT, manifest TEXT);
CREATE TABLE misconfigured_ecus(serial TEXT UNIQUE, hardware_id TEXT NOT NULL, state INTEGER NOT NULL CHECK (state IN (0,1)));
CREATE TABLE installed_versions(id INTEGER PRIMARY KEY, ecu_serial TEXT NOT NULL, sha256 TEXT NOT NULL, name TEXT NOT NULL, hashes TEXT NOT NULL, length INTEGER NOT NULL DEFAULT 0, correlation_id TEXT NOT NULL DEFAULT '', is_current INTEGER NOT NULL CHECK (is_current IN (0,1)) DEFAULT 0, is_pending INTEGER NOT NULL CHECK (is_pending IN (0,1)) DEFAULT 0, was_installed INTEGER NOT NULL CHECK (was_installed IN (0,1)) DEFAULT 0, custom_meta TEXT NOT NULL DEFAULT "");
CREATE TABLE primary_keys(unique_mark INTEGER PRIMARY KEY CHECK (unique_mark = 0), private TEXT, public TEXT);
Expand Down
19 changes: 10 additions & 9 deletions src/aktualizr_primary/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
set(TARGET aktualizr)
set(SOURCES main.cc secondary_config.h secondary_config.cc secondary.h secondary.cc)
set(SOURCES main.cc secondary_config.cc secondary.cc)
set(HEADERS secondary_config.h secondary.h)

add_executable(${TARGET} ${SOURCES} $<TARGET_OBJECTS:aktualizr-posix>)
target_link_libraries(${TARGET} aktualizr_lib virtual_secondary)
target_include_directories(${TARGET} PUBLIC
${PROJECT_SOURCE_DIR}/src/libaktualizr-posix
${PROJECT_SOURCE_DIR}/third_party)

aktualizr_source_file_checks(${SOURCES})
add_executable(aktualizr ${SOURCES})
target_link_libraries(aktualizr aktualizr_lib virtual_secondary aktualizr-posix)
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)
target_include_directories(t_primary_secondary_registration PUBLIC ${PROJECT_SOURCE_DIR}/src/libaktualizr-posix)
lbonn marked this conversation as resolved.
Show resolved Hide resolved

aktualizr_source_file_checks(${SOURCES} ${HEADERS} ${TEST_SOURCES})

# vim: set tabstop=4 shiftwidth=4 expandtab:
8 changes: 3 additions & 5 deletions src/aktualizr_primary/main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,9 @@ int main(int argc, char *argv[]) {
try {
Primary::initSecondaries(aktualizr, config.uptane.secondary_config_file);
} catch (const std::exception &e) {
LOG_ERROR << "Secondary initialization failed: " << e.what();
if (!aktualizr.IsRegistered()) {
LOG_ERROR << "Cannot provision without all secondaries present, exiting...";
return EXIT_FAILURE;
}
LOG_ERROR << "Failed to init secondaries :" << e.what();
LOG_ERROR << "Exiting...";
return EXIT_FAILURE;
}
}

Expand Down
127 changes: 127 additions & 0 deletions src/aktualizr_primary/primary_secondary_registration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
#include <gtest/gtest.h>

#include "httpfake.h"
#include "primary/aktualizr.h"
#include "secondary.h"
#include "uptane_test_common.h"
#include "utilities/utils.h"

boost::filesystem::path uptane_repos_dir;
boost::filesystem::path fake_meta_dir;

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

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.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"};

{
// prepare storage the "old" way:
storage->storeDeviceId("device");
storage->storeEcuSerials({{primary_serial, primary_hwid}, {secondary_serial, secondary_hwid}});
storage->storeEcuRegistered();

sec_conf["IP"]["secondary_wait_port"] = 9030;
sec_conf["IP"]["secondary_wait_timeout"] = 1;
sec_conf["IP"]["secondaries"] = Json::arrayValue;
sec_conf["IP"]["secondaries"][0]["addr"] = "127.0.0.1:9061";
Utils::writeFile(sec_conf_path, sec_conf);
}

{
// verifies 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();
aktualizr.CheckUpdates().get();

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, "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);

TemporaryDirectory tmp_dir;
fake_meta_dir = tmp_dir.Path();
MetaFake meta_fake(fake_meta_dir);

return RUN_ALL_TESTS();
}
#endif
102 changes: 85 additions & 17 deletions src/aktualizr_primary/secondary.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <boost/asio/placeholders.hpp>
#include <boost/bind.hpp>

#include <algorithm>
#include <unordered_map>
#include <unordered_set>

Expand All @@ -14,18 +15,19 @@
namespace Primary {

using Secondaries = std::vector<std::shared_ptr<Uptane::SecondaryInterface>>;
using SecondaryFactoryRegistry = std::unordered_map<std::string, std::function<Secondaries(const SecondaryConfig&)>>;
using SecondaryFactoryRegistry =
std::unordered_map<std::string, std::function<Secondaries(const SecondaryConfig&, Aktualizr& aktualizr)>>;

static Secondaries createIPSecondaries(const IPSecondariesConfig& config);
static Secondaries createIPSecondaries(const IPSecondariesConfig& config, Aktualizr& aktualizr);

static SecondaryFactoryRegistry sec_factory_registry = {
{IPSecondariesConfig::Type,
[](const SecondaryConfig& config) {
[](const SecondaryConfig& config, Aktualizr& aktualizr) {
auto ip_sec_cgf = dynamic_cast<const IPSecondariesConfig&>(config);
return createIPSecondaries(ip_sec_cgf);
return createIPSecondaries(ip_sec_cgf, aktualizr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing in the Aktualizr object here seems like a big change, but I don't see any immediate concerns. Are there other ways to do this that would simplify this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the new methods to Aktualizr is the biggest change in this context. We're coming from initSecondaries that already takes an Aktualizr instance. But these are the two sides of the same problem

The alternatives that I know of would be:

  • to pass the storage directly in some way (but it's opaque from here, as it's in Aktualizr). Having a Aktualizr::GetStorage() would be quite ugly.
  • create another storage instance from the storage path and use it there. Probably not SQLStorage but something that derives from SQLStorageBase. I am would be worried about the added boilerplate.
  • use another dedicated storage mechanism (?)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back then when I introduced this 'Primary::initSecondaries' my intention/plan was and still is to consider it as an initial version of generic Abstract Factory that is part of Secondary Registry mechanism that we might introduce in the future. So, I consider 'Primary::initSecondaries' as something that belongs to Primary/main namespace and it's OK to pass an instance of Aktualizr to it. Having said that, I think that it's not OK to pass this instance to a specific secondary factory method, like createIPSecondaries() as it should belong IpUptaneSecondary namespace. Also, createIPSecondaries() should be under IpUptaneSecondary namespace too, I did not put it there in the first place because we didn't want to spend much time to implementation of some secondary registration mechanism (e.g. something similar to what Laurent proposes for the package manager registry).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably fine as is, but my concern is that the Aktualizr class is mostly meant to be an API for library users, and I think we're extending that a bit here. I fear that something similar is happening to what has happened to SotaUptaneClient. I'm not sure how to fix that, and it might just take some time to step back and figure out a good refactoring strategy. Or maybe I'm worrying too much about things that don't matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably fine as is, but my concern is that the Aktualizr class is mostly meant to be an API for library users, and I think we're extending that a bit here.

I share your concern but in this case, adding a secondary implementation is expected by our library users, so this might be in the scope. But it's indeed not clear cut.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patrickvacek Mike suggested that if we want to wait for secondaries, we could so in the implementation of install() for example. I don't think he advocates having abstract notions of wait() and ping() as virtual methods (@mike-sul please correct me if I'm wrong).

In principle I think it's a good design and I also agree that it's dangerous to make this interface grow too much but there are things that would be harder to implement this way:

  • waiting a fixed time for all secondaries globally and not waiting N times
  • do not attempt to update the primary if secondaries are not available and the updates target them. We can still rollback and that's maybe something we should think about anyway (since secondary install can still fail even we found the secondaries)

We might rethink these things or just decide that we don't need it but I think that the clean solution is also less powerful.

Copy link
Collaborator

@mike-sul mike-sul Feb 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think he advocates having abstract notions of wait() and ping() as virtual methods (@mike-sul please correct me if I'm wrong).

that's correct. I strongly against it :).

Do you agree that we need some abstract notion of wait and ping? The implementations can indeed live in the subclasses, but my point was that we need some way to ask if a Secondary is available.

No, I disagree. IMHO, we already have functionality that kind of "waits" for IP Secondary, it's all software stack running under Asn1Rpc(). What we need is just extend/wrap this functionality in order to add additional policy(ies) into it, e.g. desired timeout/wait time as in this specific case and then use parametrized Asn1Rpc() from IpUptaneSecondary methods. For example, in this specific case, we could call Asn1Rpc(timeout=10m) in IpUptaneSecondary::install() and Asn1Rpc(timeout=60s) in another IpUptaneSecondary's methods (but this is also questionable since we might wanna wait "longer" at the stage of sending metadata too).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The solution you're describing just doesn't have the same behavior as what we've agreed on when we took on this task. The two points I've raised are what we want to satisfy right now.

I sympathize with you as you want to keep away the bloat and coupling and I see that this change can seem to go in the wrong direction and might make things more difficult to untangle later.

However after discussing with @patrickvacek , we still think the best way forward right now is to try to get this merged first. The two main reasons are:

  • to implement what we agreed on right now, we believe that overriding the install() step is not sufficient. As we are giving more fine-grained control on the installation process, we will inevitably need more fine-grained interfaces. It does not necessarily have to be something low-level like ping(), we could also imagine having a globalPreInstall() step in SecondaryInterface() but that would maybe require more careful design, that we haven't yet started
  • we do not claim this is a perfect solution that will be set in stone. Though, moving the piece of code later when we have a more satisfying design should be relatively easy. There is a balance to be found between a hacky solution and an all-encompassing plan that is never implemented in practice. And as we are trying to work in an iterative fashion, the current version feels like it's just good enough right now.

Copy link
Collaborator

@mike-sul mike-sul Feb 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just doesn't have the same behavior as what we've agreed on when we took on this task

It won't be exactly the same internally, but effect/outcome from an external standpoint of view will be the same.

overriding the install() step is not sufficient.

I don't think we need to override install() step, we were mistaken referring to install() method at all, it should be rather sendFirmware() or putMetadata() or both.

we could also imagine having a globalPreInstall() step in SecondaryInterface() but that would maybe require more careful design, that we haven't yet started

I am not sure if I understand if we need to have it at all. I think this idea about "waiting" for secondaries prior to sending firmware to it is conceptually flawed if to look into it from a broader perspective and take into account other aspects such as layered nature on networking stack and potential use-case of "dynamic" secondary support.

There is a balance to be found between a hacky solution and an all-encompassing plan that is never implemented in practice. And as we are trying to work in an iterative fashion, the current version feels like it's just good enough right now.

That's fair enough and makes sense in many cases but in this particular case the implementation option that goes in line with an all-encompassing plan (proper design) is simpler and easier compared to the given implementation.

@lbonn @patrickvacek There is no need to persuade me :). We've had a great and constructive discussion around this PR and it's absolutely OK that different people have different opinions. I fully understand that discussions cannot last forever, and a decision should be made. What worked the best for me at my previous companies if there was no full consensus among peers is to let a tech lead/architect (or owner of software under discussion) to make a final decision and everyone else should accept and comply to it.

So, that's absolutely fine with me if this change is merged :) as long as this change fulfill the original goal/issue and the CI jobs are happy.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the robust discussion; I think that leads us to a better solution in the end and helps keep us aware of the trade-offs and alternatives. My suggestion for now is to move forward with this PR as is to satisfy our immediate product goals, and then have a followup discussion (perhaps at the sprint review or the next relevant grooming) to consider the longer-term product goals and then decide where to go from here.

}},
{VirtualSecondaryConfig::Type,
[](const SecondaryConfig& config) {
[](const SecondaryConfig& config, Aktualizr& /* unused */) {
auto virtual_sec_cgf = dynamic_cast<const VirtualSecondaryConfig&>(config);
return Secondaries({std::make_shared<VirtualSecondary>(virtual_sec_cgf)});
}},
Expand All @@ -34,8 +36,8 @@ static SecondaryFactoryRegistry sec_factory_registry = {
// }
};

static Secondaries createSecondaries(const SecondaryConfig& config) {
return (sec_factory_registry.at(config.type()))(config);
static Secondaries createSecondaries(const SecondaryConfig& config, Aktualizr& aktualizr) {
return (sec_factory_registry.at(config.type()))(config, aktualizr);
}

void initSecondaries(Aktualizr& aktualizr, const boost::filesystem::path& config_file) {
Expand All @@ -48,7 +50,7 @@ void initSecondaries(Aktualizr& aktualizr, const boost::filesystem::path& config
for (auto& config : secondary_configs) {
try {
LOG_INFO << "Creating " << config->type() << " secondaries...";
Secondaries secondaries = createSecondaries(*config);
Secondaries secondaries = createSecondaries(*config, aktualizr);

for (const auto& secondary : secondaries) {
LOG_INFO << "Adding Secondary with ECU serial: " << secondary->getSerial()
Expand Down Expand Up @@ -141,20 +143,86 @@ class SecondaryWaiter {
std::unordered_set<std::string> secondaries_to_wait_for_;
};

static Secondaries createIPSecondaries(const IPSecondariesConfig& config) {
static Secondaries createIPSecondaries(const IPSecondariesConfig& config, Aktualizr& aktualizr) {
Secondaries result;
SecondaryWaiter sec_waiter{config.secondaries_wait_port, config.secondaries_timeout_s, result};
const bool provision = !aktualizr.IsRegistered();

for (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);
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();

// 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];
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];
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);
}

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;
}
}
}

sec_waiter.wait();
return result;
}

Expand Down
4 changes: 2 additions & 2 deletions src/aktualizr_primary/secondary_config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ SecondaryConfigParser::Configs SecondaryConfigParser::parse_config_file(const bo
}

auto cfg_file_ext = boost::filesystem::extension(config_file);
std::shared_ptr<SecondaryConfigParser> cfg_parser;
std::unique_ptr<SecondaryConfigParser> cfg_parser;

if (cfg_file_ext == ".json") {
cfg_parser = std::make_shared<JsonConfigParser>(config_file);
cfg_parser = std_::make_unique<JsonConfigParser>(config_file);
} else { // add your format of configuration file + implement SecondaryConfigParser specialization
throw std::invalid_argument("Unsupported type of config format: " + cfg_file_ext);
}
Expand Down
7 changes: 1 addition & 6 deletions src/aktualizr_secondary/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ list(REMOVE_ITEM TEST_LIBS aktualizr_lib)
# TODO: a lot of these should be removed
add_library(aktualizr_secondary_lib SHARED
${AKTUALIZR_SECONDARY_LIB_SRC}
$<TARGET_OBJECTS:aktualizr-posix>
$<TARGET_OBJECTS:asn1>
$<TARGET_OBJECTS:asn1_lib>
$<TARGET_OBJECTS:bootstrap>
Expand All @@ -29,11 +28,7 @@ add_library(aktualizr_secondary_lib SHARED
$<TARGET_OBJECTS:logging>
$<TARGET_OBJECTS:uptane>
$<TARGET_OBJECTS:campaign>)
target_link_libraries(aktualizr_secondary_lib ${AKTUALIZR_EXTERNAL_LIBS})
target_include_directories(aktualizr_secondary_lib PUBLIC
$<TARGET_PROPERTY:asn1_lib,INCLUDE_DIRECTORIES>
${PROJECT_SOURCE_DIR}/src/libaktualizr-posix
)
target_link_libraries(aktualizr_secondary_lib aktualizr-posix ${AKTUALIZR_EXTERNAL_LIBS})

if (BUILD_ISOTP)
target_sources(aktualizr_secondary_lib PRIVATE $<TARGET_OBJECTS:isotp_conn>)
Expand Down
Loading