bmc: require at least one action pack configuration
In the event the phosphor-ipmi-flash package is added to an image with
only a default configuration, it should not register itself as a blob
handler. The default installation does not include any json
configuration files. Therefore, this lack of a configuration can be
checked at load time.
Added validation that the firmware image list contains at least two
handlers, and one is always the hash blob handler.
Added validation that the action map contains at least one action pack
configuration.
Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I0b2f6cded78121869e8a01290d29e420c555a16d
diff --git a/bmc/firmware_handler.cpp b/bmc/firmware_handler.cpp
index d58825e..4d06b58 100644
--- a/bmc/firmware_handler.cpp
+++ b/bmc/firmware_handler.cpp
@@ -42,16 +42,20 @@
std::vector<HandlerPack>&& firmwares,
const std::vector<DataHandlerPack>& transports, ActionMap&& actionPacks)
{
- /* There must be at least one. */
- if (!firmwares.size())
+ /* There must be at least one in addition to the hash blob handler. */
+ if (firmwares.size() < 2)
{
- log<level::ERR>("Must provide at least one firmware handler.");
+ log<level::ERR>("Must provide at least two firmware handlers.");
return nullptr;
}
if (!transports.size())
{
return nullptr;
}
+ if (actionPacks.empty())
+ {
+ return nullptr;
+ }
std::vector<std::string> blobs;
for (const auto& item : firmwares)
diff --git a/bmc/test/Makefile.am b/bmc/test/Makefile.am
index a4d79f8..ce8ac08 100644
--- a/bmc/test/Makefile.am
+++ b/bmc/test/Makefile.am
@@ -22,7 +22,6 @@
# Run all 'check' test programs
check_PROGRAMS = \
- firmware_createhandler_unittest \
firmware_handler_unittest \
firmware_stat_unittest \
firmware_canhandle_unittest \
@@ -46,9 +45,6 @@
TESTS = $(check_PROGRAMS)
-firmware_createhandler_unittest_SOURCES = firmware_createhandler_unittest.cpp
-firmware_createhandler_unittest_LDADD = $(top_builddir)/bmc/libfirmwareblob_common.la
-
firmware_handler_unittest_SOURCES = firmware_handler_unittest.cpp
firmware_handler_unittest_LDADD = $(top_builddir)/bmc/libfirmwareblob_common.la
diff --git a/bmc/test/firmware_createhandler_unittest.cpp b/bmc/test/firmware_createhandler_unittest.cpp
deleted file mode 100644
index 8eb36c6..0000000
--- a/bmc/test/firmware_createhandler_unittest.cpp
+++ /dev/null
@@ -1,47 +0,0 @@
-#include "data_mock.hpp"
-#include "firmware_handler.hpp"
-#include "flags.hpp"
-#include "image_mock.hpp"
-#include "triggerable_mock.hpp"
-#include "util.hpp"
-
-#include <memory>
-
-#include <gtest/gtest.h>
-
-namespace ipmi_flash
-{
-namespace
-{
-
-using ::testing::Return;
-using ::testing::StrEq;
-using ::testing::StrictMock;
-
-TEST(FirmwareHandlerBlobTest, VerifyFirmwareCounts)
-{
- /* Verify the firmware count must be greater than zero. */
-
- DataHandlerMock dataMock;
- ImageHandlerMock imageMock;
- // StrictMock<SdJournalMock> journalMock;
- // SwapJouralHandler(&journalMock);
-
- std::vector<HandlerPack> blobs;
- blobs.push_back(std::move(
- HandlerPack(hashBlobId, std::make_unique<ImageHandlerMock>())));
-
- std::vector<DataHandlerPack> data = {
- {FirmwareFlags::UpdateFlags::ipmi, nullptr},
- {FirmwareFlags::UpdateFlags::lpc, &dataMock},
- };
-
- auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- std::move(blobs), data, std::move(CreateActionMap("abcd")));
-
- // EXPECT_EQ(handler, nullptr);
- EXPECT_FALSE(handler == nullptr);
-}
-
-} // namespace
-} // namespace ipmi_flash
diff --git a/bmc/test/firmware_handler_unittest.cpp b/bmc/test/firmware_handler_unittest.cpp
index 700a75a..50801c9 100644
--- a/bmc/test/firmware_handler_unittest.cpp
+++ b/bmc/test/firmware_handler_unittest.cpp
@@ -17,7 +17,7 @@
using ::testing::UnorderedElementsAreArray;
-TEST(FirmwareHandlerTest, CreateEmptyListVerifyFails)
+TEST(FirmwareHandlerTest, CreateEmptyHandlerListVerifyFails)
{
std::vector<DataHandlerPack> data = {
{FirmwareFlags::UpdateFlags::ipmi, nullptr},
@@ -41,6 +41,61 @@
std::move(blobs), {}, std::move(CreateActionMap("asdf")));
EXPECT_EQ(handler, nullptr);
}
+TEST(FirmwareHandlerTest, CreateEmptyActionPackVerifyFails)
+{
+ /* The ActionPack map corresponds to the firmware list passed in, but
+ * they're not checked against each other yet.
+ */
+ std::vector<DataHandlerPack> data = {
+ {FirmwareFlags::UpdateFlags::ipmi, nullptr},
+ };
+
+ std::vector<HandlerPack> blobs;
+ blobs.push_back(
+ std::move(HandlerPack("asdf", std::make_unique<ImageHandlerMock>())));
+ blobs.push_back(std::move(
+ HandlerPack(hashBlobId, std::make_unique<ImageHandlerMock>())));
+
+ ActionMap emptyMap;
+
+ auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
+ std::move(blobs), data, std::move(emptyMap));
+ EXPECT_EQ(handler, nullptr);
+}
+TEST(FirmwareHandlerTest, FirmwareHandlerListRequiresAtLeastTwoEntries)
+{
+ /* The hashblob handler must be one of the entries, but it cannot be the
+ * only entry.
+ */
+ std::vector<DataHandlerPack> data = {
+ {FirmwareFlags::UpdateFlags::ipmi, nullptr},
+ };
+
+ /* Provide a firmware list that has the hash blob, which is the required one
+ * -- tested in a different test.
+ */
+ std::vector<HandlerPack> blobs;
+ blobs.push_back(std::move(
+ HandlerPack(hashBlobId, std::make_unique<ImageHandlerMock>())));
+
+ auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
+ std::move(blobs), data, std::move(CreateActionMap("asdf")));
+ EXPECT_EQ(handler, nullptr);
+
+ /* Add second firmware and it'll now work. */
+ std::vector<HandlerPack> blobs2;
+ blobs2.push_back(std::move(
+ HandlerPack(hashBlobId, std::make_unique<ImageHandlerMock>())));
+ blobs2.push_back(
+ std::move(HandlerPack("asdf", std::make_unique<ImageHandlerMock>())));
+
+ handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
+ std::move(blobs2), data, std::move(CreateActionMap("asdf")));
+
+ auto result = handler->getBlobIds();
+ std::vector<std::string> expectedBlobs = {"asdf", hashBlobId};
+ EXPECT_THAT(result, UnorderedElementsAreArray(expectedBlobs));
+}
TEST(FirmwareHandlerTest, VerifyHashRequiredForHappiness)
{
std::vector<DataHandlerPack> data = {