entity-manager: Handle left-over template vars
With current implementations for template variable replacement, if a
variable does not exist on D-Bus, it will be left as is and will appear
in the final configuration. However, if the string is intended to be
replaced and not handled properly after a replacement failure, the
variable itself will possibly be shown to users via Redfish/IPMI if the
consuming properties are used by those.
After performing the template variable replacement, a post-processing
step is added to check for any leftover template variables. These
leftovers may occur if the probed property is not available on D-Bus.
Any such variables are replaced with an empty string to avoid
unresolved placeholders.
The boundary of a template var is decided to be from the character "$"
to before a " " character, or the end of the keyPair's value string.
This conforms to the implicit rules currently applied to template
variable configuration that a template variable should not be
immediately followed by an arbitrary string without a space separating
them. This pattern can also be found in how the math configurations are
extracted, which looks for a math operator at the position that is one
character away from the end of the template variable [1].
The unit tests are also updated to test the new function that handles
left-over template variables.
[1]: https://github.com/openbmc/entity-manager/blob/3911d80afb6956b17c4f4b9a76c7eb45bb76e3b9/src/entity_manager/utils.cpp#L134
Tested: Unit tests pass
Tested with real configuration:
`$PRODUCT_PRODUCT_KK_NAME`, `$PRODUCT_KK_VERSION` and
`$PRODUCT_SERIAL_KK_NUMBER` are non-existent probed property on D-Bus.
```
{
"Exposes": [
...
],
"Name": "Mt.Mitchell_Motherboard",
"Probe": [
"xyz.openbmc_project.FruDevice({'BOARD_PRODUCT_NAME': 'MB'})",
"AND",
"FOUND('Mt.Mitchell_DCSCM_BMC')"
],
"Type": "Board",
"xyz.openbmc_project.Common.UUID": {
"UUID": "$MULTIRECORD_UUID"
},
"xyz.openbmc_project.Inventory.Decorator.Asset": {
"BuildDate": "$BOARD_MANUFACTURE_DATE",
"Manufacturer": "$PRODUCT_MANUFACTURER",
"Model": "$PRODUCT_PRODUCT_KK_NAME $PRODUCT_KK_VERSION",
"PartNumber": "$PRODUCT_PART_NUMBER",
"SerialNumber": "$PRODUCT_SERIAL_KK_NUMBER"
}
}
```
D-Bus results after the configuration is processed:
- `Model` has a space character which is the space between
`$PRODUCT_PRODUCT_KK_NAME` and `$PRODUCT_KK_VERSION` in the
configuration.
- SerialNumber is empty.
```
$ busctl introspect xyz.openbmc_project.EntityManager \
/xyz/openbmc_project/inventory/system/board/Mt_Mitchell_Motherboard
xyz.openbmc_project.Common.UUID interface - -
.UUID property s "xx-xx"
xyz.openbmc_project.Inventory.Decorator.Asset interface - -
.BuildDate property s "20221018Z"
.Manufacturer property s "NULL"
.Model property s " "
.PartNumber property s "ProductPN"
.SerialNumber property s ""
Change-Id: I4499baf3ebe9560e13932a49e324d1c8b0255623
Signed-off-by: Chau Ly <chaul@amperecomputing.com>
diff --git a/src/entity_manager/utils.cpp b/src/entity_manager/utils.cpp
index 0f6f079..7528117 100644
--- a/src/entity_manager/utils.cpp
+++ b/src/entity_manager/utils.cpp
@@ -9,6 +9,7 @@
#include <boost/algorithm/string/find.hpp>
#include <boost/algorithm/string/replace.hpp>
#include <boost/algorithm/string/split.hpp>
+#include <phosphor-logging/lg2.hpp>
#include <sdbusplus/bus/match.hpp>
#include <fstream>
@@ -69,6 +70,63 @@
return false;
}
+void handleLeftOverTemplateVars(nlohmann::json::iterator& keyPair)
+{
+ if (keyPair.value().type() == nlohmann::json::value_t::object ||
+ keyPair.value().type() == nlohmann::json::value_t::array)
+ {
+ for (auto nextLayer = keyPair.value().begin();
+ nextLayer != keyPair.value().end(); nextLayer++)
+ {
+ handleLeftOverTemplateVars(nextLayer);
+ }
+ return;
+ }
+
+ std::string* strPtr = keyPair.value().get_ptr<std::string*>();
+ if (strPtr == nullptr)
+ {
+ return;
+ }
+
+ // Walking through the string to find $<templateVar>
+ while (true)
+ {
+ boost::iterator_range<std::string::const_iterator> findStart =
+ boost::ifind_first(*strPtr, std::string_view(templateChar));
+
+ if (!findStart)
+ {
+ break;
+ }
+
+ boost::iterator_range<std::string::iterator> searchRange(
+ strPtr->begin() + (findStart.end() - strPtr->begin()),
+ strPtr->end());
+ boost::iterator_range<std::string::const_iterator> findSpace =
+ boost::ifind_first(searchRange, " ");
+
+ std::string::const_iterator templateVarEnd;
+
+ if (!findSpace)
+ {
+ // No space means the template var spans to the end of
+ // of the keyPair value
+ templateVarEnd = strPtr->end();
+ }
+ else
+ {
+ // A space marks the end of a template var
+ templateVarEnd = findSpace.begin();
+ }
+
+ lg2::error(
+ "There's still template variable {VAR} un-replaced. Removing it from the string.\n",
+ "VAR", std::string(findStart.begin(), templateVarEnd));
+ strPtr->erase(findStart.begin(), templateVarEnd);
+ }
+}
+
// Replaces the template character like the other version of this function,
// but checks all properties on all interfaces provided to do the substitution
// with.
@@ -81,9 +139,11 @@
auto ret = templateCharReplace(keyPair, interface, index, replaceStr);
if (ret)
{
+ handleLeftOverTemplateVars(keyPair);
return ret;
}
}
+ handleLeftOverTemplateVars(keyPair);
return std::nullopt;
}
diff --git a/src/entity_manager/utils.hpp b/src/entity_manager/utils.hpp
index b638239..d7a4857 100644
--- a/src/entity_manager/utils.hpp
+++ b/src/entity_manager/utils.hpp
@@ -26,6 +26,8 @@
bool fwVersionIsSame();
+void handleLeftOverTemplateVars(nlohmann::json::iterator& keyPair);
+
std::optional<std::string> templateCharReplace(
nlohmann::json::iterator& keyPair, const DBusObject& object, size_t index,
const std::optional<std::string>& replaceStr = std::nullopt);
diff --git a/test/entity_manager/test_entity-manager.cpp b/test/entity_manager/test_entity-manager.cpp
index 7966090..cc59e44 100644
--- a/test/entity_manager/test_entity-manager.cpp
+++ b/test/entity_manager/test_entity-manager.cpp
@@ -240,6 +240,34 @@
EXPECT_EQ(expected, j["foo"]);
}
+TEST(TemplateCharReplace, leftOverTemplateVars)
+{
+ nlohmann::json j = {{"foo", "$EXISTENT_VAR and $NON_EXISTENT_VAR"}};
+ auto it = j.begin();
+
+ DBusInterface data;
+ data["EXISTENT_VAR"] = std::string("Replaced");
+
+ DBusObject object;
+ object["PATH"] = data;
+
+ em_utils::templateCharReplace(it, object, 0);
+
+ nlohmann::json expected = "Replaced and ";
+ EXPECT_EQ(expected, j["foo"]);
+}
+
+TEST(HandleLeftOverTemplateVars, replaceLeftOverTemplateVar)
+{
+ nlohmann::json j = {{"foo", "the Test $TEST is $TESTED"}};
+ auto it = j.begin();
+
+ em_utils::handleLeftOverTemplateVars(it);
+
+ nlohmann::json expected = "the Test is ";
+ EXPECT_EQ(expected, j["foo"]);
+}
+
TEST(MatchProbe, stringEqString)
{
nlohmann::json j = R"("foo")"_json;