From 3150088146f7b7d86ce0bce848756fcae685e022 Mon Sep 17 00:00:00 2001 From: Trevor Shoe Date: Thu, 12 Dec 2024 15:27:50 -0500 Subject: [PATCH] Add implementation of image vault utils --- include/multipass/vm_image_vault_utils.h | 8 ++-- src/daemon/default_vm_image_vault.cpp | 19 +++++--- .../backends/lxd/lxd_vm_image_vault.cpp | 11 ++--- .../backends/shared/base_vm_image_vault.h | 4 +- src/utils/file_ops.cpp | 5 ++- src/utils/vm_image_vault_utils.cpp | 33 ++++++++++---- tests/test_file_ops.cpp | 3 +- tests/test_image_vault_utils.cpp | 45 +++++++++++++++++-- 8 files changed, 97 insertions(+), 31 deletions(-) diff --git a/include/multipass/vm_image_vault_utils.h b/include/multipass/vm_image_vault_utils.h index 5da2f77927..97445d11c2 100644 --- a/include/multipass/vm_image_vault_utils.h +++ b/include/multipass/vm_image_vault_utils.h @@ -25,10 +25,7 @@ #include #include -#define MP_IMAGE_VAULT_UTILS \ - multipass::ImageVaultUtils \ - { \ - } +#define MP_IMAGE_VAULT_UTILS multipass::ImageVaultUtils() namespace multipass { @@ -62,6 +59,9 @@ template void ImageVaultUtils::verify_file_hash(const QString& file, const QString& hash, const Hasher& hasher) { auto file_hash = hasher.compute_file_hash(file); + + if (file_hash != hash) + throw std::runtime_error(fmt::format("Hash of {} does not match {}", file, hash)); } template diff --git a/src/daemon/default_vm_image_vault.cpp b/src/daemon/default_vm_image_vault.cpp index 3236a4c23b..7eacc78d86 100644 --- a/src/daemon/default_vm_image_vault.cpp +++ b/src/daemon/default_vm_image_vault.cpp @@ -299,7 +299,7 @@ mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type, } vm_image = prepare(source_image); - vm_image.id = mp::vault::compute_image_hash(vm_image.image_path).toStdString(); + vm_image.id = MP_IMAGE_VAULT_UTILS.compute_file_hash(vm_image.image_path).toStdString(); remove_source_images(source_image, vm_image); @@ -360,7 +360,9 @@ mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type, last_modified.toString(), 0, checksum.has_value()}; - const auto image_filename = mp::vault::filename_for(image_url.path()); + + QFileInfo file_info{image_url.path()}; + const auto image_filename = file_info.fileName(); // Attempt to make a sane directory name based on the filename of the image const auto image_dir_name = @@ -624,8 +626,10 @@ mp::VMImage mp::DefaultVMImageVault::download_and_prepare_source_image( } else { + QFileInfo file_info{info.image_location}; + source_image.id = id.toStdString(); - source_image.image_path = image_dir.filePath(mp::vault::filename_for(info.image_location)); + source_image.image_path = image_dir.filePath(file_info.fileName()); source_image.original_release = info.release_title.toStdString(); source_image.release_date = info.version.toStdString(); @@ -646,12 +650,13 @@ mp::VMImage mp::DefaultVMImageVault::download_and_prepare_source_image( { mpl::log(mpl::Level::debug, category, fmt::format("Verifying hash \"{}\"", id)); monitor(LaunchProgress::VERIFY, -1); - mp::vault::verify_image_download(source_image.image_path, id); + MP_IMAGE_VAULT_UTILS.verify_file_hash(source_image.image_path, id); } if (source_image.image_path.endsWith(".xz")) { - source_image.image_path = mp::vault::extract_image(source_image.image_path, monitor, true); + source_image.image_path = + MP_IMAGE_VAULT_UTILS.extract_file(source_image.image_path, monitor, true, XzImageDecoder{}); } auto prepared_image = prepare(source_image); @@ -678,14 +683,14 @@ QString mp::DefaultVMImageVault::extract_image_from(const VMImage& source_image, const auto image_name = file_info.fileName().remove(".xz"); const auto image_path = QDir(dest_dir).filePath(image_name); - return mp::vault::extract_image(image_path, monitor); + return MP_IMAGE_VAULT_UTILS.extract_file(image_path, monitor); } mp::VMImage mp::DefaultVMImageVault::image_instance_from(const VMImage& prepared_image, const mp::Path& dest_dir) { MP_UTILS.make_dir(dest_dir); - return {mp::vault::copy(prepared_image.image_path, dest_dir), + return {MP_IMAGE_VAULT_UTILS.copy_to_dir(prepared_image.image_path, dest_dir), prepared_image.id, prepared_image.original_release, prepared_image.current_release, diff --git a/src/platform/backends/lxd/lxd_vm_image_vault.cpp b/src/platform/backends/lxd/lxd_vm_image_vault.cpp index 72d4e23479..435e51b6a7 100644 --- a/src/platform/backends/lxd/lxd_vm_image_vault.cpp +++ b/src/platform/backends/lxd/lxd_vm_image_vault.cpp @@ -83,7 +83,7 @@ QString post_process_downloaded_image(const QString& image_path, const mp::Progr if (image_path.endsWith(".xz")) { - new_image_path = mp::vault::extract_image(image_path, monitor, true); + new_image_path = MP_IMAGE_VAULT_UTILS.extract_file(image_path, monitor, true, mp::XzImageDecoder{}); } QString original_image_path{new_image_path}; @@ -263,7 +263,7 @@ mp::VMImage mp::LXDVMImageVault::fetch_image(const FetchType& fetch_type, throw std::runtime_error(fmt::format("Custom image `{}` does not exist.", image_url.path())); source_image.image_path = image_url.path(); - id = mp::vault::compute_image_hash(source_image.image_path); + id = MP_IMAGE_VAULT_UTILS.compute_file_hash(source_image.image_path); last_modified = QDateTime::currentDateTime(); } @@ -307,13 +307,14 @@ mp::VMImage mp::LXDVMImageVault::fetch_image(const FetchType& fetch_type, if (query.query_type != Query::Type::LocalFile) { // TODO: Need to make this async like in DefaultVMImageVault - image_path = lxd_import_dir.filePath(mp::vault::filename_for(info.image_location)); + QFileInfo file_info{info.image_location}; + image_path = lxd_import_dir.filePath(file_info.fileName()); url_download_image(info, image_path, monitor); } else { - image_path = mp::vault::copy(source_image.image_path, lxd_import_dir.path()); + image_path = MP_IMAGE_VAULT_UTILS.copy_to_dir(source_image.image_path, lxd_import_dir.path()); } image_path = post_process_downloaded_image(image_path, monitor); @@ -524,7 +525,7 @@ void mp::LXDVMImageVault::url_download_image(const VMImageInfo& info, const QStr if (info.verify) { monitor(LaunchProgress::VERIFY, -1); - mp::vault::verify_image_download(image_path, info.id); + MP_IMAGE_VAULT_UTILS.verify_file_hash(image_path, info.id); } } diff --git a/src/platform/backends/shared/base_vm_image_vault.h b/src/platform/backends/shared/base_vm_image_vault.h index 09fda0da1b..d636c83c4d 100644 --- a/src/platform/backends/shared/base_vm_image_vault.h +++ b/src/platform/backends/shared/base_vm_image_vault.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -34,7 +35,8 @@ class BaseVMImageVault : public VMImageVault { public: explicit BaseVMImageVault(const std::vector& image_hosts) - : image_hosts{image_hosts}, remote_image_host_map{vault::configure_image_host_map(image_hosts)} {}; + : image_hosts{image_hosts}, + remote_image_host_map{MP_IMAGE_VAULT_UTILS.configure_image_host_map(image_hosts)} {}; VMImageHost* image_host_for(const std::string& remote_name) const override { diff --git a/src/utils/file_ops.cpp b/src/utils/file_ops.cpp index 7e403f52c3..ca8f608394 100644 --- a/src/utils/file_ops.cpp +++ b/src/utils/file_ops.cpp @@ -172,8 +172,9 @@ bool mp::FileOps::flush(QFile& file) const QString mp::FileOps::remove_extension(const QString& path) const { - QFileInfo info{path}; - return info.dir().path() + info.completeBaseName(); + const QFileInfo info{path}; + auto extension_len = info.suffix().size(); + return path.chopped(extension_len != 0 ? extension_len + 1 : 0); } bool mp::FileOps::copy(const QString& from, const QString& to) const diff --git a/src/utils/vm_image_vault_utils.cpp b/src/utils/vm_image_vault_utils.cpp index e24c214398..f839b908f0 100644 --- a/src/utils/vm_image_vault_utils.cpp +++ b/src/utils/vm_image_vault_utils.cpp @@ -30,25 +30,42 @@ namespace mp = multipass; QString mp::ImageVaultUtils::copy_to_dir(const QString& file, const QDir& output_dir) { - return ""; + if (file.isEmpty()) + return ""; + + const QFileInfo info{file}; + if (!MP_FILEOPS.exists(info)) + throw std::runtime_error(fmt::format("File {} not found", file)); + + auto new_location = output_dir.filePath(info.fileName()); + + if (!MP_FILEOPS.copy(file, new_location)) + throw std::runtime_error(fmt::format("Failed to copy {} to {}", file, new_location)); + + return new_location; } QString mp::ImageVaultUtils::compute_hash(QIODevice& device) { - return ""; + QCryptographicHash hash{QCryptographicHash::Sha256}; + if (!hash.addData(std::addressof(device))) + throw std::runtime_error("Failed to read data from device to hash"); + + return hash.result().toHex(); } QString mp::ImageVaultUtils::compute_file_hash(const QString& path) { - return ""; + QFile file{path}; + if (!MP_FILEOPS.open(file, QFile::ReadOnly)) + throw std::runtime_error(fmt::format("Failed to open {}", path)); + + return compute_hash(file); } mp::ImageVaultUtils::HostMap mp::ImageVaultUtils::configure_image_host_map(const Hosts& image_hosts) { - return {}; - /* - std::unordered_map remote_image_host_map; - + HostMap remote_image_host_map{}; for (const auto& image_host : image_hosts) { for (const auto& remote : image_host->supported_remotes()) @@ -57,5 +74,5 @@ mp::ImageVaultUtils::HostMap mp::ImageVaultUtils::configure_image_host_map(const } } - return remote_image_host_map;*/ + return remote_image_host_map; } diff --git a/tests/test_file_ops.cpp b/tests/test_file_ops.cpp index fcc6b84bb0..5a1cd1ddef 100644 --- a/tests/test_file_ops.cpp +++ b/tests/test_file_ops.cpp @@ -195,7 +195,8 @@ TEST_F(FileOps, posix_lseek) TEST_F(FileOps, remove_extension) { + EXPECT_EQ(MP_FILEOPS.remove_extension(""), ""); EXPECT_EQ(MP_FILEOPS.remove_extension("test.txt"), "test"); EXPECT_EQ(MP_FILEOPS.remove_extension("tests/test.test.txt"), "tests/test.test"); - EXPECT_EQ(MP_FILEOPS.remove_extension("/sets/test.png"), "/sets/test.png"); + EXPECT_EQ(MP_FILEOPS.remove_extension("/sets/test.png"), "/sets/test"); } diff --git a/tests/test_image_vault_utils.cpp b/tests/test_image_vault_utils.cpp index 5062358e10..35ccf3157e 100644 --- a/tests/test_image_vault_utils.cpp +++ b/tests/test_image_vault_utils.cpp @@ -18,6 +18,7 @@ #include "common.h" #include "mock_file_ops.h" #include "mock_image_decoder.h" +#include "mock_image_host.h" #include "mock_image_vault_utils.h" #include @@ -116,9 +117,10 @@ TEST_F(TestImageVaultUtils, verify_file_hash_throws_on_bad_hash) mpt::MockImageVaultUtils mock_utils; EXPECT_CALL(mock_utils, compute_file_hash(test_path)).WillOnce(Return(":(")); - MP_EXPECT_THROW_THAT(MP_IMAGE_VAULT_UTILS.verify_file_hash(test_path, ":)", mock_utils), - std::runtime_error, - mpt::match_what(HasSubstr("hash does not match"))); + MP_EXPECT_THROW_THAT( + MP_IMAGE_VAULT_UTILS.verify_file_hash(test_path, ":)", mock_utils), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr(test_path.toStdString()), HasSubstr(":)"), HasSubstr("does not match")))); } TEST_F(TestImageVaultUtils, verify_file_hash_doesnt_throw_on_good_hash) @@ -170,4 +172,41 @@ TEST_F(TestImageVaultUtils, extract_image_extracts_image) EXPECT_EQ(res, dest); } +TEST_F(TestImageVaultUtils, empty_hosts_produces_empty_map) +{ + auto map = MP_IMAGE_VAULT_UTILS.configure_image_host_map({}); + EXPECT_TRUE(map.empty()); +} + +TEST_F(TestImageVaultUtils, configure_image_host_map_maps_hosts) +{ + mpt::MockImageHost mock1{}; + std::vector hosts1{"this", "is", "a", "remotes"}; + EXPECT_CALL(mock1, supported_remotes()).WillOnce(Return(hosts1)); + + mpt::MockImageHost mock2{}; + std::vector hosts2{"hi"}; + EXPECT_CALL(mock2, supported_remotes()).WillOnce(Return(hosts2)); + + auto map = MP_IMAGE_VAULT_UTILS.configure_image_host_map({&mock1, &mock2}); + + EXPECT_EQ(map.size(), hosts1.size() + hosts2.size()); + + for (const auto& host : hosts1) + { + if (auto it = map.find(host); it != map.end()) + EXPECT_EQ(it->second, &mock1); + else + ADD_FAILURE() << fmt::format("{} was not mapped", host); + } + + for (const auto& host : hosts2) + { + if (auto it = map.find(host); it != map.end()) + EXPECT_EQ(it->second, &mock2); + else + ADD_FAILURE() << fmt::format("{} was not mapped", host); + } +} + } // namespace