bmc: add ActionPack notion to bundle actions
Each firmware type will provide its own set of action implementations
for each step, preparation, verification, and update.
Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Id6409ac356a74e9094272b37709861e2a33d9862
diff --git a/bmc/test/Makefile.am b/bmc/test/Makefile.am
index 1dcc3ae..feaee78 100644
--- a/bmc/test/Makefile.am
+++ b/bmc/test/Makefile.am
@@ -40,7 +40,8 @@
firmware_state_updatepending_unittest \
firmware_state_updatestarted_unittest \
firmware_state_updatecompleted_unittest \
- firmware_state_notyetstarted_tarball_unittest
+ firmware_state_notyetstarted_tarball_unittest \
+ firmware_multiplebundle_unittest
TESTS = $(check_PROGRAMS)
@@ -100,3 +101,6 @@
firmware_state_notyetstarted_tarball_unittest_SOURCES = firmware_state_notyetstarted_tarball_unittest.cpp
firmware_state_notyetstarted_tarball_unittest_LDADD = $(top_builddir)/bmc/libfirmwareblob_common.la
+
+firmware_multiplebundle_unittest_SOURCES = firmware_multiplebundle_unittest.cpp
+firmware_multiplebundle_unittest_LDADD = $(top_builddir)/bmc/libfirmwareblob_common.la
diff --git a/bmc/test/firmware_canhandle_unittest.cpp b/bmc/test/firmware_canhandle_unittest.cpp
index 13bf943..80bcce9 100644
--- a/bmc/test/firmware_canhandle_unittest.cpp
+++ b/bmc/test/firmware_canhandle_unittest.cpp
@@ -34,8 +34,7 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
for (const auto& item : items)
{
diff --git a/bmc/test/firmware_commit_unittest.cpp b/bmc/test/firmware_commit_unittest.cpp
index a6ab172..16d9881 100644
--- a/bmc/test/firmware_commit_unittest.cpp
+++ b/bmc/test/firmware_commit_unittest.cpp
@@ -12,6 +12,8 @@
namespace ipmi_flash
{
+namespace
+{
using ::testing::_;
using ::testing::IsNull;
using ::testing::NotNull;
@@ -50,8 +52,7 @@
std::make_unique<StrictMock<TriggerMock>>();
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), std::move(verifyMock),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
EXPECT_CALL(imageMock2, open("asdf")).WillOnce(Return(true));
@@ -72,8 +73,7 @@
std::make_unique<StrictMock<TriggerMock>>();
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), std::move(verifyMock),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
EXPECT_CALL(imageMock1, open(StrEq(hashBlobId))).WillOnce(Return(true));
@@ -84,4 +84,5 @@
EXPECT_FALSE(handler->commit(0, {}));
}
+} // namespace
} // namespace ipmi_flash
diff --git a/bmc/test/firmware_createhandler_unittest.cpp b/bmc/test/firmware_createhandler_unittest.cpp
index d7b99a6..2800f2e 100644
--- a/bmc/test/firmware_createhandler_unittest.cpp
+++ b/bmc/test/firmware_createhandler_unittest.cpp
@@ -9,6 +9,9 @@
namespace ipmi_flash
{
+namespace
+{
+
using ::testing::Return;
using ::testing::StrEq;
using ::testing::StrictMock;
@@ -32,11 +35,11 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ 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 7ab224d..29df507 100644
--- a/bmc/test/firmware_handler_unittest.cpp
+++ b/bmc/test/firmware_handler_unittest.cpp
@@ -23,8 +23,7 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- {}, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ {}, data, std::move(CreateActionMap("abcd")));
EXPECT_EQ(handler, nullptr);
}
TEST(FirmwareHandlerTest, CreateEmptyDataHandlerListFails)
@@ -37,8 +36,7 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, {}, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, {}, std::move(CreateActionMap("asdf")));
EXPECT_EQ(handler, nullptr);
}
TEST(FirmwareHandlerTest, VerifyHashRequiredForHappiness)
@@ -54,15 +52,14 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
EXPECT_EQ(handler, nullptr);
blobs.push_back({hashBlobId, &imageMock});
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
+
auto result = handler->getBlobIds();
std::vector<std::string> expectedBlobs = {"asdf", hashBlobId};
EXPECT_THAT(result, UnorderedElementsAreArray(expectedBlobs));
diff --git a/bmc/test/firmware_multiplebundle_unittest.cpp b/bmc/test/firmware_multiplebundle_unittest.cpp
new file mode 100644
index 0000000..b8d738d
--- /dev/null
+++ b/bmc/test/firmware_multiplebundle_unittest.cpp
@@ -0,0 +1,155 @@
+/* The goal of these tests is to verify that once a host-client has started the
+ * process with one blob bundle, they cannot pivot to upload data to another.
+ *
+ * This prevent someone from starting to upload a BMC firmware, and then midway
+ * through start uploading a BIOS image.
+ */
+#include "firmware_handler.hpp"
+#include "flags.hpp"
+#include "image_mock.hpp"
+#include "status.hpp"
+#include "triggerable_mock.hpp"
+#include "util.hpp"
+
+#include <memory>
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+namespace ipmi_flash
+{
+namespace
+{
+
+using ::testing::Return;
+
+class IpmiOnlyTwoFirmwaresTest : public ::testing::Test
+{
+ protected:
+ void SetUp() override
+ {
+ blobs = {
+ {hashBlobId, &hashImageMock},
+ {staticLayoutBlobId, &staticImageMock},
+ {biosId, &biosImageMock},
+ };
+
+ std::unique_ptr<TriggerableActionInterface> bmcPrepareMock =
+ std::make_unique<TriggerMock>();
+ bmcPrepareMockPtr =
+ reinterpret_cast<TriggerMock*>(bmcPrepareMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> bmcVerifyMock =
+ std::make_unique<TriggerMock>();
+ bmcVerifyMockPtr = reinterpret_cast<TriggerMock*>(bmcVerifyMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> bmcUpdateMock =
+ std::make_unique<TriggerMock>();
+ bmcUpdateMockPtr = reinterpret_cast<TriggerMock*>(bmcUpdateMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> biosPrepareMock =
+ std::make_unique<TriggerMock>();
+ biosPrepareMockPtr =
+ reinterpret_cast<TriggerMock*>(biosPrepareMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> biosVerifyMock =
+ std::make_unique<TriggerMock>();
+ biosVerifyMockPtr =
+ reinterpret_cast<TriggerMock*>(biosVerifyMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> biosUpdateMock =
+ std::make_unique<TriggerMock>();
+ biosUpdateMockPtr =
+ reinterpret_cast<TriggerMock*>(biosUpdateMock.get());
+
+ ActionMap packs;
+
+ std::unique_ptr<ActionPack> bmcPack = std::make_unique<ActionPack>();
+ bmcPack->preparation = std::move(bmcPrepareMock);
+ bmcPack->verification = std::move(bmcVerifyMock);
+ bmcPack->update = std::move(bmcUpdateMock);
+
+ std::unique_ptr<ActionPack> biosPack = std::make_unique<ActionPack>();
+ biosPack->preparation = std::move(biosPrepareMock);
+ biosPack->verification = std::move(biosVerifyMock);
+ biosPack->update = std::move(biosUpdateMock);
+
+ packs[staticLayoutBlobId] = std::move(bmcPack);
+ packs[biosId] = std::move(biosPack);
+
+ handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
+ blobs, data, std::move(packs));
+ }
+
+ void expectedState(FirmwareBlobHandler::UpdateState state)
+ {
+ auto realHandler = dynamic_cast<FirmwareBlobHandler*>(handler.get());
+ EXPECT_EQ(state, realHandler->getCurrentState());
+ }
+
+ ImageHandlerMock hashImageMock, staticImageMock, biosImageMock;
+
+ std::vector<HandlerPack> blobs;
+ std::vector<DataHandlerPack> data = {
+ {FirmwareFlags::UpdateFlags::ipmi, nullptr}};
+
+ std::unique_ptr<blobs::GenericBlobInterface> handler;
+
+ TriggerMock *bmcPrepareMockPtr, *bmcVerifyMockPtr, *bmcUpdateMockPtr;
+ TriggerMock *biosPrepareMockPtr, *biosVerifyMockPtr, *biosUpdateMockPtr;
+
+ std::uint16_t session = 1;
+ std::uint16_t flags =
+ blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::ipmi;
+
+ const std::string biosId = "/flash/bios";
+};
+
+TEST_F(IpmiOnlyTwoFirmwaresTest, OpeningBiosAfterBlobFails)
+{
+ /* You can only have one file open at a time, and the first firmware file
+ * you open locks it down
+ */
+ EXPECT_CALL(staticImageMock, open(staticLayoutBlobId))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*bmcPrepareMockPtr, trigger()).WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->open(session, flags, staticLayoutBlobId));
+ expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
+
+ EXPECT_CALL(staticImageMock, close()).WillOnce(Return());
+ handler->close(session);
+
+ expectedState(FirmwareBlobHandler::UpdateState::verificationPending);
+
+ EXPECT_CALL(biosImageMock, open(biosId)).Times(0);
+ EXPECT_FALSE(handler->open(session, flags, biosId));
+}
+
+TEST_F(IpmiOnlyTwoFirmwaresTest, OpeningHashBeforeBiosSucceeds)
+{
+ /* Opening the hash blob does nothing special in this regard. */
+ EXPECT_CALL(hashImageMock, open(hashBlobId)).WillOnce(Return(true));
+ EXPECT_TRUE(handler->open(session, flags, hashBlobId));
+
+ expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
+
+ EXPECT_CALL(hashImageMock, close()).WillOnce(Return());
+ handler->close(session);
+
+ expectedState(FirmwareBlobHandler::UpdateState::verificationPending);
+ ASSERT_FALSE(handler->canHandleBlob(verifyBlobId));
+
+ EXPECT_CALL(biosImageMock, open(biosId)).WillOnce(Return(true));
+ EXPECT_TRUE(handler->open(session, flags, biosId));
+
+ expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
+
+ EXPECT_CALL(biosImageMock, close()).WillOnce(Return());
+ handler->close(session);
+}
+
+} // namespace
+} // namespace ipmi_flash
diff --git a/bmc/test/firmware_stat_unittest.cpp b/bmc/test/firmware_stat_unittest.cpp
index 021dca8..b71d5c7 100644
--- a/bmc/test/firmware_stat_unittest.cpp
+++ b/bmc/test/firmware_stat_unittest.cpp
@@ -30,8 +30,7 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
blobs::BlobMeta meta;
EXPECT_TRUE(handler->stat("asdf", &meta));
diff --git a/bmc/test/firmware_state_notyetstarted_tarball_unittest.cpp b/bmc/test/firmware_state_notyetstarted_tarball_unittest.cpp
index bdf6046..3ad118c 100644
--- a/bmc/test/firmware_state_notyetstarted_tarball_unittest.cpp
+++ b/bmc/test/firmware_state_notyetstarted_tarball_unittest.cpp
@@ -35,9 +35,16 @@
std::make_unique<TriggerMock>();
updateMockPtr = reinterpret_cast<TriggerMock*>(updateMock.get());
+ std::unique_ptr<ActionPack> actionPack = std::make_unique<ActionPack>();
+ actionPack->preparation = CreateTriggerMock();
+ actionPack->verification = std::move(verifyMock);
+ actionPack->update = std::move(updateMock);
+
+ ActionMap packs;
+ packs[ubiTarballBlobId] = std::move(actionPack);
+
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), std::move(verifyMock),
- std::move(updateMock));
+ blobs, data, std::move(packs));
}
void expectedState(FirmwareBlobHandler::UpdateState state)
diff --git a/bmc/test/firmware_state_notyetstarted_unittest.cpp b/bmc/test/firmware_state_notyetstarted_unittest.cpp
index 8681064..3c0eb67 100644
--- a/bmc/test/firmware_state_notyetstarted_unittest.cpp
+++ b/bmc/test/firmware_state_notyetstarted_unittest.cpp
@@ -105,7 +105,10 @@
TEST_F(FirmwareHandlerNotYetStartedTest, OpenHashFileVerifyStateChange)
{
EXPECT_CALL(imageMock, open(hashBlobId)).WillOnce(Return(true));
- EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
+ /* Opening the hash blob id doesn't trigger a preparation, only a firmware
+ * blob.
+ */
+ EXPECT_CALL(*prepareMockPtr, trigger()).Times(0);
EXPECT_TRUE(handler->open(session, flags, hashBlobId));
diff --git a/bmc/test/firmware_state_verificationstarted_unittest.cpp b/bmc/test/firmware_state_verificationstarted_unittest.cpp
index b183db0..8c0d476 100644
--- a/bmc/test/firmware_state_verificationstarted_unittest.cpp
+++ b/bmc/test/firmware_state_verificationstarted_unittest.cpp
@@ -173,7 +173,7 @@
TEST_F(FirmwareHandlerVerificationStartedTest, StatOnActiveHashReturnsFailure)
{
- getToVerificationStarted(hashBlobId);
+ getToVerificationStartedWitHashBlob();
ASSERT_TRUE(handler->canHandleBlob(activeHashBlobId));
blobs::BlobMeta meta;
diff --git a/bmc/test/firmware_unittest.hpp b/bmc/test/firmware_unittest.hpp
index 6b97289..129b940 100644
--- a/bmc/test/firmware_unittest.hpp
+++ b/bmc/test/firmware_unittest.hpp
@@ -8,6 +8,7 @@
#include <memory>
#include <string>
+#include <unordered_map>
#include <vector>
#include <gmock/gmock.h>
@@ -42,9 +43,16 @@
std::make_unique<TriggerMock>();
updateMockPtr = reinterpret_cast<TriggerMock*>(updateMock.get());
+ std::unique_ptr<ActionPack> actionPack = std::make_unique<ActionPack>();
+ actionPack->preparation = std::move(prepareMock);
+ actionPack->verification = std::move(verifyMock);
+ actionPack->update = std::move(updateMock);
+
+ ActionMap packs;
+ packs[staticLayoutBlobId] = std::move(actionPack);
+
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, std::move(prepareMock), std::move(verifyMock),
- std::move(updateMock));
+ blobs, data, std::move(packs));
}
void expectedState(FirmwareBlobHandler::UpdateState state)
@@ -56,7 +64,10 @@
void openToInProgress(const std::string& blobId)
{
EXPECT_CALL(imageMock, open(blobId)).WillOnce(Return(true));
- EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
+ if (blobId != hashBlobId)
+ {
+ EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
+ }
EXPECT_TRUE(handler->open(session, flags, blobId));
expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
}
@@ -81,6 +92,24 @@
expectedState(FirmwareBlobHandler::UpdateState::verificationStarted);
}
+ void getToVerificationStartedWitHashBlob()
+ {
+ /* Open both static and hash to check for activeHashBlobId. */
+ getToVerificationPending(staticLayoutBlobId);
+
+ openToInProgress(hashBlobId);
+ EXPECT_CALL(imageMock, close()).WillRepeatedly(Return());
+ handler->close(session);
+ expectedState(FirmwareBlobHandler::UpdateState::verificationPending);
+
+ /* Now the hash is active AND the static image is active. */
+ EXPECT_TRUE(handler->open(session, flags, verifyBlobId));
+ EXPECT_CALL(*verifyMockPtr, trigger()).WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->commit(session, {}));
+ expectedState(FirmwareBlobHandler::UpdateState::verificationStarted);
+ }
+
void getToVerificationCompleted(ActionStatus checkResponse)
{
getToVerificationStarted(staticLayoutBlobId);
@@ -151,8 +180,7 @@
{"asdf", &imageMock},
};
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
}
};
@@ -176,8 +204,7 @@
{FirmwareFlags::UpdateFlags::lpc, &dataMock},
};
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
}
};
diff --git a/bmc/test/triggerable_mock.hpp b/bmc/test/triggerable_mock.hpp
index c485d8e..8c83d52 100644
--- a/bmc/test/triggerable_mock.hpp
+++ b/bmc/test/triggerable_mock.hpp
@@ -1,7 +1,11 @@
#pragma once
+#include "firmware_handler.hpp"
#include "status.hpp"
+#include <memory>
+#include <string>
+
#include <gtest/gtest.h>
namespace ipmi_flash
@@ -21,4 +25,16 @@
return std::make_unique<TriggerMock>();
}
+ActionMap CreateActionMap(const std::string& blobPath)
+{
+ std::unique_ptr<ActionPack> actionPack = std::make_unique<ActionPack>();
+ actionPack->preparation = std::move(CreateTriggerMock());
+ actionPack->verification = std::move(CreateTriggerMock());
+ actionPack->update = std::move(CreateTriggerMock());
+
+ ActionMap map;
+ map[blobPath] = std::move(actionPack);
+ return map;
+}
+
} // namespace ipmi_flash