diff --git a/include/multipass/image_host/custom_image_host.h b/include/multipass/image_host/custom_image_host.h index c32fcbbe37a..3af9134c9d7 100644 --- a/include/multipass/image_host/custom_image_host.h +++ b/include/multipass/image_host/custom_image_host.h @@ -44,8 +44,7 @@ class CustomVMImageHost final : public BaseVMImageHost std::optional info_for(const Query& query) override; std::vector> all_info_for(const Query& query) override; - std::vector all_images_for(const std::string& remote_name, - const bool allow_unsupported) override; + std::vector all_images_for(const std::string& remote_name) override; std::vector supported_remotes() override; private: diff --git a/include/multipass/image_host/ubuntu_image_host.h b/include/multipass/image_host/ubuntu_image_host.h index e176c9f57d9..4fc4c410875 100644 --- a/include/multipass/image_host/ubuntu_image_host.h +++ b/include/multipass/image_host/ubuntu_image_host.h @@ -40,8 +40,7 @@ class UbuntuVMImageHost final : public BaseVMImageHost std::optional info_for(const Query& query) override; std::vector> all_info_for(const Query& query) override; - std::vector all_images_for(const std::string& remote_name, - const bool allow_unsupported) override; + std::vector all_images_for(const std::string& remote_name) override; std::vector supported_remotes() override; private: diff --git a/include/multipass/image_host/vm_image_host.h b/include/multipass/image_host/vm_image_host.h index 496eda63e4f..b259f7556bc 100644 --- a/include/multipass/image_host/vm_image_host.h +++ b/include/multipass/image_host/vm_image_host.h @@ -39,8 +39,7 @@ class VMImageHost : private DisabledCopyMove virtual std::optional info_for(const Query& query) = 0; virtual std::vector> all_info_for(const Query& query) = 0; virtual VMImageInfo info_for_full_hash(const std::string& full_hash) = 0; - virtual std::vector all_images_for(const std::string& remote_name, - const bool allow_unsupported) = 0; + virtual std::vector all_images_for(const std::string& remote_name) = 0; virtual void for_each_entry_do(const Action& action) = 0; virtual std::vector supported_remotes() = 0; virtual void update_manifests(const bool force_update) = 0; diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index f17164b823f..007f098d2fd 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -1663,7 +1663,7 @@ try request->force_manifest_network_download()); const auto& remote = request->remote_name(); auto image_host = config->vault->image_host_for(remote); - auto vm_images_info = image_host->all_images_for(remote, request->allow_unsupported()); + auto vm_images_info = image_host->all_images_for(remote); for (const auto& info : vm_images_info) add_aliases(response.mutable_images_info(), remote, info); diff --git a/src/image_host/custom_image_host.cpp b/src/image_host/custom_image_host.cpp index 6e7379015a4..8d063733a5f 100644 --- a/src/image_host/custom_image_host.cpp +++ b/src/image_host/custom_image_host.cpp @@ -144,8 +144,7 @@ std::vector> mp::CustomVMImageHost::all_ return images; } -std::vector mp::CustomVMImageHost::all_images_for(const std::string& remote_name, - const bool allow_unsupported) +std::vector mp::CustomVMImageHost::all_images_for(const std::string& remote_name) { if (auto custom_manifest = manifest_from(remote_name)) return custom_manifest->products; diff --git a/src/image_host/ubuntu_image_host.cpp b/src/image_host/ubuntu_image_host.cpp index 1a4907e37fb..313956028be 100644 --- a/src/image_host/ubuntu_image_host.cpp +++ b/src/image_host/ubuntu_image_host.cpp @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include @@ -108,9 +107,6 @@ std::vector> mp::UbuntuVMImageHost::all_ if (const auto* info = match_alias(key, *manifest); info) { - if (!info->supported && !query.allow_unsupported) - throw mp::UnsupportedImageException(query.release); - images.emplace_back(remote_name, *info); } else @@ -119,7 +115,7 @@ std::vector> mp::UbuntuVMImageHost::all_ for (const auto& entry : manifest->products) { - if (entry.id.startsWith(key) && (entry.supported || query.allow_unsupported) && + if (entry.id.startsWith(key) && found_hashes.find(entry.id.toStdString()) == found_hashes.end()) { images.emplace_back(remote_name, entry); @@ -148,19 +144,13 @@ mp::VMImageInfo mp::UbuntuVMImageHost::info_for_full_hash_impl(const std::string throw mp::ImageNotFoundException(full_hash); } -std::vector mp::UbuntuVMImageHost::all_images_for(const std::string& remote_name, - const bool allow_unsupported) +std::vector mp::UbuntuVMImageHost::all_images_for(const std::string& remote_name) { std::vector images; auto manifest = manifest_from(remote_name); - for (const auto& entry : manifest->products) - { - if (entry.supported || allow_unsupported) - { - images.push_back(entry); - } - } + for (const auto& entry : manifest->products) + images.push_back(entry); if (images.empty()) throw std::runtime_error( diff --git a/src/simplestreams/simple_streams_manifest.cpp b/src/simplestreams/simple_streams_manifest.cpp index 99d0cc7875b..aac02562b8c 100644 --- a/src/simplestreams/simple_streams_manifest.cpp +++ b/src/simplestreams/simple_streams_manifest.cpp @@ -136,6 +136,8 @@ try const auto supported = lookup_or(product, "supported", false) || product_aliases.contains("devel") || (os == "ubuntu-core" && image_type == "stable"); + if (!supported) + continue; const auto* versions = if_contains_object(product, "versions"); if (!versions || versions->empty()) diff --git a/tests/unit/mock_image_host.h b/tests/unit/mock_image_host.h index 23608b83ea5..7c9bd732b14 100644 --- a/tests/unit/mock_image_host.h +++ b/tests/unit/mock_image_host.h @@ -77,7 +77,7 @@ class MockImageHost : public VMImageHost }); ON_CALL(*this, all_info_for(_)).WillByDefault(Return(empty_image_info_vector_pair)); ON_CALL(*this, info_for_full_hash(_)).WillByDefault(Return(empty_vm_image_info)); - ON_CALL(*this, all_images_for(_, _)).WillByDefault(Return(empty_image_info_vector)); + ON_CALL(*this, all_images_for(_)).WillByDefault(Return(empty_image_info_vector)); ON_CALL(*this, for_each_entry_do(_)).WillByDefault([this](const Action& action) { action(release_remote, mock_bionic_image_info); action(release_remote, mock_another_image_info); @@ -95,7 +95,7 @@ class MockImageHost : public VMImageHost MOCK_METHOD(VMImageInfo, info_for_full_hash, (const std::string&), (override)); MOCK_METHOD(std::vector, all_images_for, - (const std::string&, const bool), + (const std::string&), (override)); MOCK_METHOD(void, for_each_entry_do, (const Action&), (override)); MOCK_METHOD(std::vector, supported_remotes, (), (override)); diff --git a/tests/unit/stub_image_host.h b/tests/unit/stub_image_host.h index 5901d9c3f4a..3773b63eed5 100644 --- a/tests/unit/stub_image_host.h +++ b/tests/unit/stub_image_host.h @@ -42,8 +42,7 @@ struct StubVMImageHost final : public multipass::VMImageHost return {{}, {}, {}, {}, {}, {}, {}, {}, {}, {}, -1, {}}; }; - std::vector all_images_for(const std::string& remote_name, - const bool allow_unsupported) override + std::vector all_images_for(const std::string& remote_name) override { return {}; }; diff --git a/tests/unit/test_custom_image_host.cpp b/tests/unit/test_custom_image_host.cpp index c7e6c509637..970c51dafd6 100644 --- a/tests/unit/test_custom_image_host.cpp +++ b/tests/unit/test_custom_image_host.cpp @@ -96,7 +96,7 @@ TEST_F(CustomImageHost, allImagesForNoRemoteReturnsAppropriateMatches) host.update_manifests(false); - auto images = host.all_images_for("", false); + auto images = host.all_images_for(""); int supported_count = num_images_for_arch(payload); EXPECT_EQ(images.size(), supported_count); @@ -166,11 +166,11 @@ TEST_F(CustomImageHost, handlesAndRecoversFromInitialNetworkFailure) int supported_count = num_images_for_arch(payload); host.update_manifests(false); - auto images_info = host.all_images_for("", false); + auto images_info = host.all_images_for(""); EXPECT_EQ(images_info.size(), 0); host.update_manifests(false); - images_info = host.all_images_for("", false); + images_info = host.all_images_for(""); EXPECT_EQ(images_info.size(), supported_count); } @@ -185,13 +185,13 @@ TEST_F(CustomImageHost, handlesAndRecoversFromLaterNetworkFailure) int supported_count = num_images_for_arch(payload); host.update_manifests(false); - EXPECT_EQ(host.all_images_for("", false).size(), supported_count); + EXPECT_EQ(host.all_images_for("").size(), supported_count); host.update_manifests(false); - EXPECT_EQ(host.all_images_for("", false).size(), 0); + EXPECT_EQ(host.all_images_for("").size(), 0); host.update_manifests(false); - EXPECT_EQ(host.all_images_for("", false).size(), supported_count); + EXPECT_EQ(host.all_images_for("").size(), supported_count); } TEST_F(CustomImageHost, infoForFullHashReturnsEmptyImageInfo) @@ -238,7 +238,7 @@ TEST_F(CustomImageHost, badJsonLogsAndReturnsEmptyImages) host.update_manifests(false); - auto images = host.all_images_for("", false); + auto images = host.all_images_for(""); EXPECT_EQ(images.size(), 0); } diff --git a/tests/unit/test_daemon_find.cpp b/tests/unit/test_daemon_find.cpp index 8712bbd831b..1be7cd1c92c 100644 --- a/tests/unit/test_daemon_find.cpp +++ b/tests/unit/test_daemon_find.cpp @@ -136,7 +136,7 @@ TEST_F(DaemonFind, forByRemoteReturnsExpectedData) return &mock_image_host; }); - EXPECT_CALL(mock_image_host, all_images_for(_, _)).WillOnce([&mock_image_host](auto...) { + EXPECT_CALL(mock_image_host, all_images_for(_)).WillOnce([&mock_image_host](auto...) { std::vector images_info; images_info.push_back(mock_image_host.mock_bionic_image_info); diff --git a/tests/unit/test_data/simple_streams_manifest/unsupported_manifest.json.in b/tests/unit/test_data/simple_streams_manifest/unsupported_manifest.json.in new file mode 100644 index 00000000000..d9faababfbb --- /dev/null +++ b/tests/unit/test_data/simple_streams_manifest/unsupported_manifest.json.in @@ -0,0 +1,52 @@ +{ + "content_id": "com.ubuntu.cloud:released:download", + "datatype": "image-downloads", + "format": "products:1.0", + "updated": "Wed, 20 May 2020 16:47:50 +0000", + "products": { + "com.ubuntu.cloud:server:16.04:@MANIFEST_ARCH@": { + "aliases": "16.04,xenial", + "arch": "@MANIFEST_ARCH@", + "os": "ubuntu", + "release": "xenial", + "release_codename": "Xenial Xerus", + "release_title": "16.04 LTS", + "supported": false, + "version": "16.04", + "versions": { + "20170516": { + "items": { + "disk1.img": { + "ftype": "disk1.img", + "path": "server/releases/xenial/release-20170516/ubuntu-16.04-server-cloudimg-@MANIFEST_ARCH@-disk1.img", + "sha256": "1797c5c82016c1e65f4008fcf89deae3a044ef76087a9ec5b907c6d64a3609ac", + "size": 287440896 + } + } + } + } + }, + "com.ubuntu.cloud:server:18.04:@MANIFEST_ARCH@": { + "aliases": "18.04,bionic,default", + "arch": "@MANIFEST_ARCH@", + "os": "ubuntu", + "release": "bionic", + "release_codename": "Bionic Beaver", + "release_title": "18.04 LTS", + "supported": true, + "version": "18.04", + "versions": { + "20200518": { + "items": { + "disk1.img": { + "ftype": "disk1.img", + "path": "server/releases/bionic/release-20200518/ubuntu-18.04-server-cloudimg-@MANIFEST_ARCH@.img", + "sha256": "19f9b706755717ee7b38a4a79d8e7599e3fd96d57fb824b8e68ac92255a8323d", + "size": 345505792 + } + } + } + } + } + } +} diff --git a/tests/unit/test_image_vault.cpp b/tests/unit/test_image_vault.cpp index 95860a0e560..70317634938 100644 --- a/tests/unit/test_image_vault.cpp +++ b/tests/unit/test_image_vault.cpp @@ -33,7 +33,6 @@ #include #include #include -#include #include #include #include @@ -1224,7 +1223,7 @@ TEST_F(ImageVault, allInfoForNoRemoteGivenReturnsExpectedData) {remote_name, host.mock_bionic_image_info}, {remote_name, host.mock_another_image_info}})); - auto images = vault.all_info_for({"", "e3", false, "", mp::Query::Type::Alias, true}); + auto images = vault.all_info_for({"", "e3", false, "", mp::Query::Type::Alias}); EXPECT_EQ(images.size(), 2u); @@ -1254,7 +1253,7 @@ TEST_F(ImageVault, allInfoForRemoteGivenReturnsExpectedData) {remote_name, host.mock_bionic_image_info}, {remote_name, host.mock_another_image_info}})); - auto images = vault.all_info_for({"", "e3", false, remote_name, mp::Query::Type::Alias, true}); + auto images = vault.all_info_for({"", "e3", false, remote_name, mp::Query::Type::Alias}); EXPECT_EQ(images.size(), 2u); @@ -1282,35 +1281,7 @@ TEST_F(ImageVault, allInfoForNoImagesReturnsEmpty) EXPECT_CALL(host, all_info_for(_)) .WillOnce(Return(std::vector>{})); - EXPECT_TRUE(vault.all_info_for({"", name, false, "", mp::Query::Type::Alias, true}).empty()); -} - -TEST_F(ImageVault, updateImagesLogsWarningOnUnsupportedImage) -{ - mpt::MockLogger::Scope logger_scope = mpt::MockLogger::inject(mpl::Level::warning); - mp::DefaultVMImageVault vault{hosts, - &url_downloader, - cache_dir.path(), - data_dir.path(), - mp::days{1}}; - vault.fetch_image(mp::FetchType::ImageOnly, - default_query, - stub_prepare, - stub_monitor, - std::nullopt, - instance_dir); - - EXPECT_CALL(host, info_for(_)) - .WillOnce(Throw(mp::UnsupportedImageException(default_query.release))); - - logger_scope.mock_logger->screen_logs(mpl::Level::warning); - EXPECT_CALL(*logger_scope.mock_logger, - log(mpl::Level::warning, - StrEq("image vault"), - StrEq(fmt::format("Skipping update: The {} release is no longer supported.", - default_query.release)))); - - EXPECT_NO_THROW(vault.update_images(mp::FetchType::ImageOnly, stub_prepare, stub_monitor)); + EXPECT_TRUE(vault.all_info_for({"", name, false, "", mp::Query::Type::Alias}).empty()); } TEST_F(ImageVault, updateImagesLogsWarningOnEmptyVault) diff --git a/tests/unit/test_simple_streams_manifest.cpp b/tests/unit/test_simple_streams_manifest.cpp index e65c71f8afe..28edd74a919 100644 --- a/tests/unit/test_simple_streams_manifest.cpp +++ b/tests/unit/test_simple_streams_manifest.cpp @@ -204,4 +204,20 @@ TEST_F(TestSimpleStreamsManifest, correctlyMutatesCoreImages) EXPECT_EQ(info->release_codename, "Core 22"); } -} // namespace +TEST_F(TestSimpleStreamsManifest, filtersUnsupportedImages) +{ + auto json = mpt::load_test_file("simple_streams_manifest/unsupported_manifest.json"); + auto manifest = mp::SimpleStreamsManifest::fromJson(json, std::nullopt, ""); + + EXPECT_THAT(manifest->products.size(), Eq(1u)); + + auto info = manifest->image_records.find("bionic"); + EXPECT_NE(info, manifest->image_records.end()); + + info = manifest->image_records.find("xenial"); + EXPECT_EQ(info, manifest->image_records.end()); + + info = manifest->image_records.find("16.04"); + EXPECT_EQ(info, manifest->image_records.end()); +} +} // namespace \ No newline at end of file diff --git a/tests/unit/test_ubuntu_image_host.cpp b/tests/unit/test_ubuntu_image_host.cpp index 8b13162d530..17a75ba29f1 100644 --- a/tests/unit/test_ubuntu_image_host.cpp +++ b/tests/unit/test_ubuntu_image_host.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include @@ -174,7 +173,7 @@ TEST_F(UbuntuImageHost, iteratesOverAllEntries) }; host.for_each_entry_do(action); - const size_t expected_entries{5}; + const size_t expected_entries{4}; EXPECT_THAT(ids.size(), Eq(expected_entries)); EXPECT_THAT(ids.count("1797c5c82016c1e65f4008fcf89deae3a044ef76087a9ec5b907c6d64a3609ac"), @@ -185,8 +184,6 @@ TEST_F(UbuntuImageHost, iteratesOverAllEntries) Eq(1u)); EXPECT_THAT(ids.count("ab115b83e7a8bebf3d3a02bf55ad0cb75a0ed515fcbc65fb0c9abe76c752921c"), Eq(1u)); - EXPECT_THAT(ids.count("520224efaaf49b15a976b49c7ce7f2bd2e5b161470d684b37a838933595c0520"), - Eq(1u)); } TEST_F(UbuntuImageHost, canQueryByHash) @@ -295,30 +292,19 @@ TEST_F(UbuntuImageHost, allImagesForReleaseReturnsFourMatches) mp::UbuntuVMImageHost host{all_remote_specs, &url_downloader}; host.update_manifests(false); - auto images = host.all_images_for(release_remote_spec.first, false); + auto images = host.all_images_for(release_remote_spec.first); const size_t expected_matches{4}; EXPECT_THAT(images.size(), Eq(expected_matches)); } -TEST_F(UbuntuImageHost, allImagesForReleaseUnsupportedReturnsFiveMatches) -{ - mp::UbuntuVMImageHost host{all_remote_specs, &url_downloader}; - host.update_manifests(false); - - auto images = host.all_images_for(release_remote_spec.first, true); - - const size_t expected_matches{5}; - EXPECT_THAT(images.size(), Eq(expected_matches)); -} - TEST_F(UbuntuImageHost, allImagesForThrowsForUnknownRemote) { const auto remote_name = "unknown_remote"; mp::UbuntuVMImageHost host{all_remote_specs, &url_downloader}; host.update_manifests(false); - MP_EXPECT_THROW_THAT(host.all_images_for(remote_name, false), + MP_EXPECT_THROW_THAT(host.all_images_for(remote_name), std::runtime_error, mpt::match_what(HasSubstr( fmt::format("Remote \"{}\" is unknown or unreachable", remote_name)))); @@ -329,7 +315,7 @@ TEST_F(UbuntuImageHost, allImagesForDailyReturnsAllMatches) mp::UbuntuVMImageHost host{all_remote_specs, &url_downloader}; host.update_manifests(false); - auto images = host.all_images_for(daily_remote_spec.first, false); + auto images = host.all_images_for(daily_remote_spec.first); const size_t expected_matches{3}; EXPECT_THAT(images.size(), Eq(expected_matches)); @@ -410,13 +396,13 @@ TEST_F(UbuntuImageHost, handlesAndRecoversFromIndependentServerFailures) } } -TEST_F(UbuntuImageHost, throwsUnsupportedImageWhenImageNotSupported) +TEST_F(UbuntuImageHost, unsupportedImageNotFound) { mp::UbuntuVMImageHost host{all_remote_specs, &url_downloader}; host.update_manifests(false); - EXPECT_THROW(host.info_for(make_query("artful", release_remote_spec.first)), - mp::UnsupportedImageException); + auto info = host.info_for(make_query("artful", release_remote_spec.first)); + EXPECT_FALSE(info); } TEST_F(UbuntuImageHost, develRequestWithNoRemoteReturnsExpectedInfo) @@ -481,19 +467,6 @@ TEST_F(UbuntuImageHost, allInfoForNoRemoteQueryDefaultsToRelease) EXPECT_EQ(images_info.size(), expected_matches); } -TEST_F(UbuntuImageHost, allInfoForUnsupportedImageThrow) -{ - mp::UbuntuVMImageHost host{all_remote_specs, &url_downloader}; - host.update_manifests(false); - - const std::string release{"artful"}; - - MP_EXPECT_THROW_THAT( - host.all_info_for(make_query(release, release_remote_spec.first)), - mp::UnsupportedImageException, - mpt::match_what(StrEq(fmt::format("The {} release is no longer supported.", release)))); -} - TEST_F(UbuntuImageHost, infoForFullHashFindsImage) { mp::UbuntuVMImageHost host{all_remote_specs, &url_downloader};