Activation: check compatiblity of uploaded software
Before activation, check if the PSU inventory's manufacturer and model
matches the uploaded software, to make sure the software is not updated
to a incompatible PSU.
The model check is mandatory, and if the PSU manufacturer is empty,
ignore the manufacturer check.
Tested: Upload a dummy tarball with incompatible model, verify the
activation fails;
Upload a dummy tarball with compatible model, verify the
activation succeeds with a dummy update service.
Also added unit tests for several cases:
* Update on a PSU that model is incompatible;
* Update on a PSU that the manufacture is incompatible;
* Update on a PSU that the menufacture is empty;
* Update on 4 PSUs that the second one is incompatible.
Signed-off-by: Lei YU <mine260309@gmail.com>
Change-Id: Ia1b6a3fa6c98cdea1ea93c917c0938d4a60f0911
diff --git a/meson.build b/meson.build
index a18b935..9e5b3b3 100644
--- a/meson.build
+++ b/meson.build
@@ -18,6 +18,7 @@
# Common configurations for src and test
cdata = configuration_data()
cdata.set_quoted('ITEM_IFACE', 'xyz.openbmc_project.Inventory.Item')
+cdata.set_quoted('ASSET_IFACE', 'xyz.openbmc_project.Inventory.Decorator.Asset')
cdata.set_quoted('VERSION_IFACE', 'xyz.openbmc_project.Software.Version')
cdata.set_quoted('FILEPATH_IFACE', 'xyz.openbmc_project.Common.FilePath')
cdata.set_quoted('BUSNAME_UPDATER', 'xyz.openbmc_project.Software.Psu.Updater')
@@ -30,6 +31,8 @@
cdata.set_quoted('FUNCTIONAL_REV_ASSOCIATION', 'software_version')
cdata.set_quoted('VERSION', 'Version')
cdata.set_quoted('PRESENT', 'Present')
+cdata.set_quoted('MANUFACTURER', 'Manufacturer')
+cdata.set_quoted('MODEL', 'Model')
cdata.set_quoted('SOFTWARE_OBJPATH', get_option('SOFTWARE_OBJPATH'))
cdata.set_quoted('MANIFEST_FILE', get_option('MANIFEST_FILE'))
diff --git a/src/activation.cpp b/src/activation.cpp
index f1181fb..5023712 100644
--- a/src/activation.cpp
+++ b/src/activation.cpp
@@ -183,7 +183,21 @@
for (const auto& p : psuPaths)
{
- psuQueue.push(p);
+ if (isCompatible(p))
+ {
+ psuQueue.push(p);
+ }
+ else
+ {
+ log<level::NOTICE>("PSU not compatible",
+ entry("PSU=%s", p.c_str()));
+ }
+ }
+
+ if (psuQueue.empty())
+ {
+ log<level::ERR>("No PSU compatible with the software");
+ return Status::Failed;
}
// The progress to be increased for each successful update of PSU
@@ -257,6 +271,28 @@
}
}
+bool Activation::isCompatible(const std::string& psuInventoryPath)
+{
+ auto service =
+ utils::getService(bus, psuInventoryPath.c_str(), ASSET_IFACE);
+ auto psuManufacturer = utils::getProperty<std::string>(
+ bus, service.c_str(), psuInventoryPath.c_str(), ASSET_IFACE,
+ MANUFACTURER);
+ auto psuModel = utils::getProperty<std::string>(
+ bus, service.c_str(), psuInventoryPath.c_str(), ASSET_IFACE, MODEL);
+ if (psuModel != model)
+ {
+ // The model shall match
+ return false;
+ }
+ if (!psuManufacturer.empty())
+ {
+ // If PSU inventory has manufacturer property, it shall match
+ return psuManufacturer == manufacturer;
+ }
+ return true;
+}
+
} // namespace updater
} // namespace software
} // namespace phosphor
diff --git a/src/activation.hpp b/src/activation.hpp
index d0fdaba..b46105b 100644
--- a/src/activation.hpp
+++ b/src/activation.hpp
@@ -4,6 +4,7 @@
#include "association_interface.hpp"
#include "types.hpp"
+#include "version.hpp"
#include <queue>
#include <sdbusplus/server.hpp>
@@ -142,6 +143,10 @@
activation(activationStatus);
associations(assocs);
+ auto info = Version::getExtVersionInfo(extVersion);
+ manufacturer = info["manufacturer"];
+ model = info["model"];
+
// Emit deferred signal.
emit_object_added();
}
@@ -212,6 +217,9 @@
/** @brief Finish PSU update */
void finishActivation();
+ /** @brief Check if the PSU is comaptible with this software*/
+ bool isCompatible(const std::string& psuInventoryPath);
+
/** @brief Persistent sdbusplus DBus bus connection */
sdbusplus::bus::bus& bus;
@@ -241,6 +249,12 @@
/** @brief The AssociationInterface pointer */
AssociationInterface* associationInterface;
+
+ /** @brief The PSU manufacturer of the software */
+ std::string manufacturer;
+
+ /** @brief The PSU model of the software */
+ std::string model;
};
} // namespace updater
diff --git a/src/version.cpp b/src/version.cpp
index 0ea6cc9..02cf6af 100644
--- a/src/version.cpp
+++ b/src/version.cpp
@@ -53,6 +53,29 @@
return ret;
}
+std::map<std::string, std::string>
+ Version::getExtVersionInfo(const std::string& extVersion)
+{
+ // The extVersion shall be key/value pairs separated by comma,
+ // e.g. key1=value1,key2=value2
+ std::map<std::string, std::string> result;
+ std::stringstream ss(extVersion);
+
+ while (ss.good())
+ {
+ std::string substr;
+ getline(ss, substr, ',');
+ auto pos = substr.find('=');
+ if (pos != std::string::npos)
+ {
+ std::string key = substr.substr(0, pos);
+ std::string value = substr.substr(pos + 1);
+ result.emplace(key, value);
+ }
+ }
+ return result;
+}
+
void Delete::delete_()
{
if (version.eraseCallback)
diff --git a/src/version.hpp b/src/version.hpp
index c3c2c54..d4106e6 100644
--- a/src/version.hpp
+++ b/src/version.hpp
@@ -129,6 +129,16 @@
getValues(const std::string& filePath,
const std::vector<std::string>& keys);
+ /** @brief Get information from extVersion
+ *
+ * @param[in] extVersion - The extended version string that contains
+ * key/value pairs separated by comma.
+ *
+ * @return The map of key/value pairs
+ */
+ static std::map<std::string, std::string>
+ getExtVersionInfo(const std::string& extVersion);
+
/** @brief The temUpdater's erase callback. */
eraseFunc eraseCallback;
diff --git a/test/test_activation.cpp b/test/test_activation.cpp
index 891cdb6..0c40d97 100644
--- a/test/test_activation.cpp
+++ b/test/test_activation.cpp
@@ -11,16 +11,25 @@
using ::testing::_;
using ::testing::Return;
+using ::testing::StrEq;
+
+using std::experimental::any;
class TestActivation : public ::testing::Test
{
public:
+ using PropertyType = utils::UtilsInterface::PropertyType;
using Status = Activation::Status;
using RequestedStatus = Activation::RequestedActivations;
TestActivation() :
mockedUtils(
reinterpret_cast<const utils::MockedUtils&>(utils::getUtils()))
{
+ // By default make it compatible with the test software
+ ON_CALL(mockedUtils, getPropertyImpl(_, _, _, _, StrEq(MANUFACTURER)))
+ .WillByDefault(Return(any(PropertyType(std::string("TestManu")))));
+ ON_CALL(mockedUtils, getPropertyImpl(_, _, _, _, StrEq(MODEL)))
+ .WillByDefault(Return(any(PropertyType(std::string("TestModel")))));
}
~TestActivation()
{
@@ -38,13 +47,18 @@
{
return activation->activationProgress->progress();
}
+ const auto& getPsuQueue()
+ {
+ return activation->psuQueue;
+ }
+
sdbusplus::SdBusMock sdbusMock;
sdbusplus::bus::bus mockedBus = sdbusplus::get_mocked_new(&sdbusMock);
const utils::MockedUtils& mockedUtils;
MockedAssociationInterface mockedAssociationInterface;
std::unique_ptr<Activation> activation;
std::string versionId = "abcdefgh";
- std::string extVersion = "Some Ext Version";
+ std::string extVersion = "manufacturer=TestManu,model=TestModel";
std::string dBusPath = std::string(SOFTWARE_OBJPATH) + "/" + versionId;
Status status = Status::Ready;
AssociationList associations;
@@ -195,3 +209,97 @@
EXPECT_EQ(Status::Failed, activation->activation());
}
+
+TEST_F(TestActivation, doUpdateOnePSUModelNotCompatible)
+{
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ extVersion = "manufacturer=TestManu,model=DifferentModel";
+ activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
+ extVersion, status, associations,
+ &mockedAssociationInterface);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(Return(std::vector<std::string>({psu0})));
+ activation->requestedActivation(RequestedStatus::Active);
+
+ EXPECT_EQ(Status::Failed, activation->activation());
+}
+
+TEST_F(TestActivation, doUpdateOnePSUManufactureNotCompatible)
+{
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ extVersion = "manufacturer=DifferentManu,model=TestModel";
+ activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
+ extVersion, status, associations,
+ &mockedAssociationInterface);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(Return(std::vector<std::string>({psu0})));
+ activation->requestedActivation(RequestedStatus::Active);
+
+ EXPECT_EQ(Status::Failed, activation->activation());
+}
+
+TEST_F(TestActivation, doUpdateOnePSUSelfManufactureIsEmpty)
+{
+ ON_CALL(mockedUtils, getPropertyImpl(_, _, _, _, StrEq(MANUFACTURER)))
+ .WillByDefault(Return(any(PropertyType(std::string("")))));
+ extVersion = "manufacturer=AnyManu,model=TestModel";
+ // Below is the same as doUpdateOnePSUOK case
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
+ extVersion, status, associations,
+ &mockedAssociationInterface);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(
+ Return(std::vector<std::string>({psu0}))); // One PSU inventory
+ activation->requestedActivation(RequestedStatus::Active);
+
+ EXPECT_EQ(Status::Activating, activation->activation());
+
+ EXPECT_CALL(mockedAssociationInterface, createActiveAssociation(dBusPath))
+ .Times(1);
+ EXPECT_CALL(mockedAssociationInterface, addFunctionalAssociation(dBusPath))
+ .Times(1);
+ onUpdateDone();
+ EXPECT_EQ(Status::Active, activation->activation());
+}
+
+TEST_F(TestActivation, doUpdateFourPSUsSecondPSUNotCompatible)
+{
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ constexpr auto psu1 = "/com/example/inventory/psu1";
+ constexpr auto psu2 = "/com/example/inventory/psu2";
+ constexpr auto psu3 = "/com/example/inventory/psu3";
+ ON_CALL(mockedUtils, getPropertyImpl(_, _, StrEq(psu1), _, StrEq(MODEL)))
+ .WillByDefault(
+ Return(any(PropertyType(std::string("DifferentModel")))));
+ activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
+ extVersion, status, associations,
+ &mockedAssociationInterface);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(Return(
+ std::vector<std::string>({psu0, psu1, psu2, psu3}))); // 4 PSUs
+ activation->requestedActivation(RequestedStatus::Active);
+
+ const auto& psuQueue = getPsuQueue();
+ EXPECT_EQ(3u, psuQueue.size());
+
+ // Only 3 PSUs shall be updated, and psu1 shall be skipped
+ EXPECT_EQ(Status::Activating, activation->activation());
+ EXPECT_EQ(10, getProgress());
+
+ onUpdateDone();
+ EXPECT_EQ(Status::Activating, activation->activation());
+ EXPECT_EQ(36, getProgress());
+
+ onUpdateDone();
+ EXPECT_EQ(Status::Activating, activation->activation());
+ EXPECT_EQ(62, getProgress());
+
+ EXPECT_CALL(mockedAssociationInterface, createActiveAssociation(dBusPath))
+ .Times(1);
+ EXPECT_CALL(mockedAssociationInterface, addFunctionalAssociation(dBusPath))
+ .Times(1);
+
+ onUpdateDone();
+ EXPECT_EQ(Status::Active, activation->activation());
+}
diff --git a/test/test_version.cpp b/test/test_version.cpp
index cba599f..8481952 100644
--- a/test/test_version.cpp
+++ b/test/test_version.cpp
@@ -65,3 +65,16 @@
EXPECT_EQ("psu-dummy-test.v0.1", version);
EXPECT_EQ("model=dummy_model,manufacture=dummy_manufacture", extVersion);
}
+
+TEST_F(TestVersion, getExtVersionInfo)
+{
+ std::string extVersion = "";
+ auto ret = Version::getExtVersionInfo(extVersion);
+ EXPECT_TRUE(ret.empty());
+
+ extVersion = "manufacturer=TestManu,model=TestModel";
+ ret = Version::getExtVersionInfo(extVersion);
+ EXPECT_EQ(2u, ret.size());
+ EXPECT_EQ("TestManu", ret["manufacturer"]);
+ EXPECT_EQ("TestModel", ret["model"]);
+}