bmc: config: add skip action
If you specify the action for one of the three fields as "skip" it'll
create a handler object for that action that always returns success and
has no effect. This action therefore "skips" the step, moving the state
machine forward as though it was a successful real action.
Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Ib4c1091745eb96b8381a332bbeb0562625d3bfbe
diff --git a/bmc/Makefile.am b/bmc/Makefile.am
index 63c90b4..993b881 100644
--- a/bmc/Makefile.am
+++ b/bmc/Makefile.am
@@ -45,7 +45,8 @@
firmware_handler.cpp \
file_handler.cpp \
general_systemd.cpp \
- lpc_handler.cpp
+ lpc_handler.cpp \
+ skip_action.cpp
if ENABLE_ASPEED_LPC
libfirmwareblob_common_la_SOURCES += lpc_aspeed.cpp
diff --git a/bmc/buildjson.cpp b/bmc/buildjson.cpp
index 478fb7c..3fdda31 100644
--- a/bmc/buildjson.cpp
+++ b/bmc/buildjson.cpp
@@ -18,6 +18,7 @@
#include "file_handler.hpp"
#include "fs.hpp"
#include "general_systemd.hpp"
+#include "skip_action.hpp"
#include <algorithm>
#include <cstdio>
@@ -109,18 +110,17 @@
const auto& a = item.at("actions");
std::unique_ptr<ActionPack> pack = std::make_unique<ActionPack>();
- /* It hasn't been fully determined if any action being optional is
- * useful, so for now they will be required.
- * TODO: Evaluate how the behaviors change if some actions are
- * missing, does the code just assume it was successful? I would
- * think not.
- */
+ /* to make an action optional, assign type "skip" */
const auto& prep = a.at("preparation");
const std::string prepareType = prep.at("type");
if (prepareType == "systemd")
{
pack->preparation = std::move(buildSystemd(prep));
}
+ else if (prepareType == "skip")
+ {
+ pack->preparation = SkipAction::CreateSkipAction();
+ }
else
{
throw std::runtime_error("Invalid preparation type: " +
@@ -137,6 +137,10 @@
{
pack->verification = std::move(buildSystemd(verify));
}
+ else if (verifyType == "skip")
+ {
+ pack->verification = SkipAction::CreateSkipAction();
+ }
else
{
throw std::runtime_error("Invalid verification type:" +
@@ -159,6 +163,10 @@
{
pack->update = std::move(buildSystemd(update));
}
+ else if (updateType == "skip")
+ {
+ pack->update = SkipAction::CreateSkipAction();
+ }
else
{
throw std::runtime_error("Invalid update type: " + updateType);
diff --git a/bmc/skip_action.cpp b/bmc/skip_action.cpp
new file mode 100644
index 0000000..4392e97
--- /dev/null
+++ b/bmc/skip_action.cpp
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "skip_action.hpp"
+
+#include <memory>
+
+namespace ipmi_flash
+{
+
+std::unique_ptr<TriggerableActionInterface> SkipAction::CreateSkipAction()
+{
+ return std::make_unique<SkipAction>();
+}
+
+} // namespace ipmi_flash
diff --git a/bmc/skip_action.hpp b/bmc/skip_action.hpp
new file mode 100644
index 0000000..2ae9377
--- /dev/null
+++ b/bmc/skip_action.hpp
@@ -0,0 +1,39 @@
+#pragma once
+
+#include "status.hpp"
+
+#include <memory>
+
+namespace ipmi_flash
+{
+
+// This type will just return success upon trigger(), and even before calling
+// trigger.
+class SkipAction : public TriggerableActionInterface
+{
+ public:
+ static std::unique_ptr<TriggerableActionInterface> CreateSkipAction();
+
+ SkipAction() = default;
+ ~SkipAction() = default;
+
+ // Disallow copy and assign.
+ SkipAction(const SkipAction&) = delete;
+ SkipAction& operator=(const SkipAction&) = delete;
+ SkipAction(SkipAction&&) = default;
+ SkipAction& operator=(SkipAction&&) = default;
+
+ bool trigger() override
+ {
+ return true;
+ }
+ void abort() override
+ {
+ }
+ ActionStatus status() override
+ {
+ return ActionStatus::success;
+ }
+};
+
+} // namespace ipmi_flash
diff --git a/bmc/test/Makefile.am b/bmc/test/Makefile.am
index 9f1aa37..cbac3eb 100644
--- a/bmc/test/Makefile.am
+++ b/bmc/test/Makefile.am
@@ -42,7 +42,8 @@
firmware_state_updatecompleted_unittest \
firmware_state_notyetstarted_tarball_unittest \
firmware_multiplebundle_unittest \
- firmware_json_unittest
+ firmware_json_unittest \
+ firmware_skip_unittest
TESTS = $(check_PROGRAMS)
@@ -108,3 +109,6 @@
firmware_json_unittest_SOURCES = firmware_json_unittest.cpp
firmware_json_unittest_LDADD = $(top_builddir)/bmc/libfirmwareblob_common.la
+
+firmware_skip_unittest_SOURCES = firmware_skip_unittest.cpp
+firmware_skip_unittest_LDADD = $(top_builddir)/bmc/libfirmwareblob_common.la
diff --git a/bmc/test/firmware_json_unittest.cpp b/bmc/test/firmware_json_unittest.cpp
index d56a20a..8f3ea30 100644
--- a/bmc/test/firmware_json_unittest.cpp
+++ b/bmc/test/firmware_json_unittest.cpp
@@ -1,5 +1,6 @@
#include "buildjson.hpp"
#include "general_systemd.hpp"
+#include "skip_action.hpp"
#include <nlohmann/json.hpp>
@@ -581,5 +582,39 @@
EXPECT_THAT(updater->getMode(), "replace-fake");
}
+TEST(FirmwareJsonTest, VerifySkipFields)
+{
+ // In this configuration, nothing happens because all actions are set to
+ // skip.
+ auto j2 = R"(
+ [{
+ "blob" : "/flash/image",
+ "handler" : {
+ "type" : "file",
+ "path" : "/run/initramfs/bmc-image"
+ },
+ "actions" : {
+ "preparation" : {
+ "type" : "skip"
+ },
+ "verification" : {
+ "type" : "skip"
+ },
+ "update" : {
+ "type" : "skip"
+ }
+ }
+ }]
+ )"_json;
+
+ auto h = buildHandlerFromJson(j2);
+ EXPECT_EQ(h[0].blobId, "/flash/image");
+ EXPECT_FALSE(h[0].handler == nullptr);
+ EXPECT_FALSE(h[0].actions == nullptr);
+ EXPECT_FALSE(h[0].actions->preparation == nullptr);
+ EXPECT_FALSE(h[0].actions->verification == nullptr);
+ EXPECT_FALSE(h[0].actions->update == nullptr);
+}
+
} // namespace
} // namespace ipmi_flash
diff --git a/bmc/test/firmware_skip_unittest.cpp b/bmc/test/firmware_skip_unittest.cpp
new file mode 100644
index 0000000..a549582
--- /dev/null
+++ b/bmc/test/firmware_skip_unittest.cpp
@@ -0,0 +1,36 @@
+#include "skip_action.hpp"
+#include "status.hpp"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace ipmi_flash
+{
+namespace
+{
+
+TEST(SkipActionTest, ValidateTriggerReturnsTrue)
+{
+ SkipAction skip;
+ EXPECT_TRUE(skip.trigger());
+ EXPECT_TRUE(skip.trigger());
+}
+
+TEST(SkipActionTest, ValidateStatusAlwaysSuccess)
+{
+ SkipAction skip;
+ EXPECT_EQ(ActionStatus::success, skip.status());
+ EXPECT_TRUE(skip.trigger());
+ EXPECT_EQ(ActionStatus::success, skip.status());
+}
+
+TEST(SkipActionTest, AbortHasNoImpactOnStatus)
+{
+ SkipAction skip;
+ EXPECT_EQ(ActionStatus::success, skip.status());
+ skip.abort();
+ EXPECT_EQ(ActionStatus::success, skip.status());
+}
+
+} // namespace
+} // namespace ipmi_flash
diff --git a/bmc_json_config.md b/bmc_json_config.md
index 5aed44b..29c7017 100644
--- a/bmc_json_config.md
+++ b/bmc_json_config.md
@@ -203,6 +203,11 @@
Action types are used to define what to do for a specific requested action, such
as "verify the blob contents."
+#### `skip`
+
+The `skip` type will effectively make that action a no-op and always return
+success.
+
#### `systemd`
The `systemd` type should be used when you wish to start a systemd service or