binarystore: Add ReadOnly Mode
Add Read Only mode to the binary store. In this mode,
all blobs are not writable, no new blobs can be created and
the storage protobuf can't be modified in any way.
Also added simple API for reading/examining blob store
without modifying it.
Signed-off-by: Maksym Sloyko <maxims@google.com>
Change-Id: I12e3473351be98820d8e5df1b71a6d43699aa671
diff --git a/binarystore.cpp b/binarystore.cpp
index 1307c3a..32547d7 100644
--- a/binarystore.cpp
+++ b/binarystore.cpp
@@ -9,6 +9,7 @@
#include <blobs-ipmid/blobs.hpp>
#include <boost/endian/arithmetic.hpp>
#include <cstdint>
+#include <ipmid/handler.hpp>
#include <memory>
#include <phosphor-logging/elog.hpp>
#include <string>
@@ -48,6 +49,25 @@
return std::move(store);
}
+std::unique_ptr<BinaryStoreInterface>
+ BinaryStore::createFromFile(std::unique_ptr<SysFile> file, bool readOnly)
+{
+ if (!file)
+ {
+ log<level::ERR>("Unable to create binarystore from invalid file");
+ return nullptr;
+ }
+
+ auto store = std::make_unique<BinaryStore>(std::move(file), readOnly);
+
+ if (!store->loadSerializedData())
+ {
+ return nullptr;
+ }
+
+ return std::move(store);
+}
+
bool BinaryStore::loadSerializedData()
{
/* Load blob from sysfile if we know it might not match what we have.
@@ -93,7 +113,7 @@
return true;
}
- if (blob_.blob_base_id() != baseBlobId_)
+ if (blob_.blob_base_id() != baseBlobId_ && !readOnly_)
{
/* Uh oh, stale data loaded. Clean it and commit. */
// TODO: it might be safer to add an option in config to error out
@@ -111,13 +131,18 @@
std::string BinaryStore::getBaseBlobId() const
{
- return baseBlobId_;
+ if (!baseBlobId_.empty())
+ {
+ return baseBlobId_;
+ }
+
+ return blob_.blob_base_id();
}
std::vector<std::string> BinaryStore::getBlobIds() const
{
std::vector<std::string> result;
- result.push_back(baseBlobId_);
+ result.push_back(getBaseBlobId());
for (const auto& blob : blob_.blobs())
{
@@ -144,6 +169,13 @@
return false;
}
+ if (readOnly_ && (flags & blobs::OpenFlags::write))
+ {
+ log<level::ERR>("Can't open the blob for writing: read-only store",
+ entry("BLOB_ID=%s", blobId.c_str()));
+ return false;
+ }
+
writable_ = flags & blobs::OpenFlags::write;
/* If there are uncommitted data, discard them. */
@@ -166,11 +198,20 @@
}
/* Otherwise, create the blob and append it */
- currentBlob_ = blob_.add_blobs();
- currentBlob_->set_blob_id(blobId);
+ if (readOnly_)
+ {
+ return false;
+ }
+ else
+ {
+ currentBlob_ = blob_.add_blobs();
+ currentBlob_->set_blob_id(blobId);
- commitState_ = CommitState::Dirty;
- log<level::NOTICE>("Created new blob", entry("BLOB_ID=%s", blobId.c_str()));
+ commitState_ = CommitState::Dirty;
+ log<level::NOTICE>("Created new blob",
+ entry("BLOB_ID=%s", blobId.c_str()));
+ }
+
return true;
}
@@ -208,6 +249,23 @@
return result;
}
+std::vector<uint8_t> BinaryStore::readBlob(const std::string& blobId) const
+{
+ const auto blobs = blob_.blobs();
+ const auto blobIt =
+ std::find_if(blobs.begin(), blobs.end(),
+ [&](const auto& b) { return b.blob_id() == blobId; });
+
+ if (blobIt == blobs.end())
+ {
+ throw ipmi::HandlerCompletion(ipmi::ccUnspecifiedError);
+ }
+
+ const auto blobData = blobIt->data();
+
+ return std::vector<uint8_t>(blobData.begin(), blobData.end());
+}
+
bool BinaryStore::write(uint32_t offset, const std::vector<uint8_t>& data)
{
if (!currentBlob_)
@@ -242,6 +300,12 @@
bool BinaryStore::commit()
{
+ if (readOnly_)
+ {
+ log<level::ERR>("ReadOnly blob, not committing");
+ return false;
+ }
+
/* Store as little endian to be platform agnostic. Consistent with read. */
auto blobData = blob_.SerializeAsString();
boost::endian::little_uint64_t sizeLE = blobData.size();
diff --git a/binarystore.hpp b/binarystore.hpp
index c86934e..f46f69c 100644
--- a/binarystore.hpp
+++ b/binarystore.hpp
@@ -48,6 +48,11 @@
blob_.set_blob_base_id(baseBlobId_);
}
+ BinaryStore(std::unique_ptr<SysFile> file, bool readOnly = false) :
+ readOnly_{readOnly}, file_(std::move(file))
+ {
+ }
+
~BinaryStore() = default;
BinaryStore(const BinaryStore&) = delete;
@@ -60,6 +65,7 @@
bool openOrCreateBlob(const std::string& blobId, uint16_t flags) override;
bool deleteBlob(const std::string& blobId) override;
std::vector<uint8_t> read(uint32_t offset, uint32_t requestedSize) override;
+ std::vector<uint8_t> readBlob(const std::string& blobId) const override;
bool write(uint32_t offset, const std::vector<uint8_t>& data) override;
bool commit() override;
bool close() override;
@@ -76,6 +82,17 @@
createFromConfig(const std::string& baseBlobId,
std::unique_ptr<SysFile> file);
+ /**
+ * Helper factory method to create a BinaryStore instance
+ * This function should be used with existing stores. It reads
+ * the baseBlobId name from the storage.
+ * @param sysFile: system file object for storing binary
+ * @param readOnly: if true, open the store in read only mode
+ * @returns unique_ptr to constructed BinaryStore.
+ */
+ static std::unique_ptr<BinaryStoreInterface>
+ createFromFile(std::unique_ptr<SysFile> file, bool readOnly = true);
+
private:
/* Load the serialized data from sysfile if commit state is dirty.
* Returns False if encountered error when loading */
@@ -84,7 +101,10 @@
std::string baseBlobId_;
binaryblobproto::BinaryBlobBase blob_;
binaryblobproto::BinaryBlob* currentBlob_ = nullptr;
+ /* True if current blob is writable */
bool writable_ = false;
+ /* True if the entire store (not just individual blobs) is read only */
+ bool readOnly_ = false;
std::unique_ptr<SysFile> file_ = nullptr;
CommitState commitState_ = CommitState::Dirty;
};
diff --git a/binarystore_interface.hpp b/binarystore_interface.hpp
index 73ce17e..ba6dca3 100644
--- a/binarystore_interface.hpp
+++ b/binarystore_interface.hpp
@@ -61,6 +61,14 @@
uint32_t requestedSize) = 0;
/**
+ * Reads all data from the blob
+ * @param blobId: The blob id to operate on.
+ * @returns Bytes able to read. Returns empty if nothing can be read or
+ * if there is no such blob.
+ */
+ virtual std::vector<uint8_t> readBlob(const std::string& blobId) const = 0;
+
+ /**
* Writes data to the currently openend blob.
* @param offset: offset into the blob to write
* @param data: bytes to write
diff --git a/binarystore_mock.hpp b/binarystore_mock.hpp
index 8f60ebe..fa6c787 100644
--- a/binarystore_mock.hpp
+++ b/binarystore_mock.hpp
@@ -51,6 +51,11 @@
MOCK_METHOD0(close, bool());
MOCK_METHOD1(stat, bool(blobs::BlobMeta* meta));
+ std::vector<uint8_t> readBlob(const std::string& blobId) const override
+ {
+ return real_store_.readBlob(blobId);
+ }
+
private:
BinaryStore real_store_;
};
diff --git a/test/Makefile.am b/test/Makefile.am
index 742e7e2..2e07b58 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -19,7 +19,8 @@
handler_stat_unittest \
handler_open_unittest \
handler_readwrite_unittest \
- handler_unittest
+ handler_unittest \
+ binarystore_unittest
TESTS = $(check_PROGRAMS)
parse_config_unittest_SOURCES = parse_config_unittest.cpp
@@ -71,3 +72,10 @@
$(top_builddir)/libbinarystore_la-binaryblob.pb.o \
-lprotobuf
handler_readwrite_unittest_CXXFLAGS = $(PHOSPHOR_LOGGING_CFLAGS)
+
+binarystore_unittest_SOURCES = binarystore_unittest.cpp
+binarystore_unittest_LDADD = $(PHOSPHOR_LOGGING_LIBS) \
+ $(top_builddir)/binarystore.o \
+ $(top_builddir)/libbinarystore_la-binaryblob.pb.o \
+ -lprotobuf
+binarystore_unittest_CXXFLAGS = $(PHOSPHOR_LOGGING_CFLAGS)
diff --git a/test/binarystore_unittest.cpp b/test/binarystore_unittest.cpp
new file mode 100644
index 0000000..f0e8048
--- /dev/null
+++ b/test/binarystore_unittest.cpp
@@ -0,0 +1,169 @@
+#include "binarystore.hpp"
+#include "binarystore_interface.hpp"
+#include "sys_file.hpp"
+
+#include <google/protobuf/text_format.h>
+
+#include <iostream>
+#include <ipmid/handler.hpp>
+#include <iterator>
+#include <memory>
+#include <sstream>
+
+#include <gmock/gmock.h>
+
+const std::string blobData = "jW7jiID}kD&gm&Azi:^]JT]'Ov4"
+ "Y.Oey]mw}yak9Wf3[S`+$!g]@[0}gikis^";
+const std::string inputProto = "blob_base_id: \"/blob/my-test\""
+ "blobs: [{ "
+ " blob_id: \"/blob/my-test/0\""
+ " data:\"" +
+ blobData +
+ "\""
+ "}, { "
+ " blob_id: \"/blob/my-test/1\""
+ " data:\"" +
+ blobData +
+ "\""
+ "}, { "
+ " blob_id: \"/blob/my-test/2\""
+ " data:\"" +
+ blobData +
+ "\""
+ "}, {"
+ " blob_id: \"/blob/my-test/3\""
+ " data:\"" +
+ blobData +
+ "\""
+ "}] "
+ "max_size_bytes: 64";
+
+class SysFileBuf : public binstore::SysFile
+{
+ public:
+ SysFileBuf(std::string* storage) : data_{storage}
+ {
+ }
+
+ size_t readToBuf(size_t pos, size_t count, char* buf) const override
+ {
+ std::cout << "Read " << count << " bytes at " << pos << std::endl;
+ return data_->copy(buf, count, pos);
+ }
+
+ std::string readAsStr(size_t pos, size_t count) const override
+ {
+ std::cout << "Read as str " << count << " bytes at " << pos
+ << std::endl;
+ return data_->substr(pos, count);
+ }
+
+ std::string readRemainingAsStr(size_t pos) const override
+ {
+ return data_->substr(pos);
+ }
+
+ void writeStr(const std::string& data, size_t pos) override
+ {
+ data_->replace(pos, data.size(), data);
+ }
+
+ std::string* data_;
+};
+
+using binstore::binaryblobproto::BinaryBlobBase;
+using google::protobuf::TextFormat;
+
+using testing::ElementsAreArray;
+using testing::UnorderedElementsAre;
+
+class BinaryStoreTest : public testing::Test
+{
+ public:
+ std::unique_ptr<SysFileBuf> createBlobStorage(const std::string& textProto)
+ {
+ BinaryBlobBase storeProto;
+ TextFormat::ParseFromString(textProto, &storeProto);
+
+ std::stringstream storage;
+ std::string data;
+ storeProto.SerializeToString(&data);
+
+ const uint64_t dataSize = data.size();
+ storage.write(reinterpret_cast<const char*>(&dataSize),
+ sizeof(dataSize));
+ storage << data;
+
+ blobDataStorage = storage.str();
+ return std::make_unique<SysFileBuf>(&blobDataStorage);
+ }
+
+ std::string blobDataStorage;
+};
+
+TEST_F(BinaryStoreTest, SimpleLoad)
+{
+ auto testDataFile = createBlobStorage(inputProto);
+ auto initialData = blobDataStorage;
+ auto store = binstore::BinaryStore::createFromConfig(
+ "/blob/my-test", std::move(testDataFile));
+ EXPECT_THAT(store->getBlobIds(),
+ UnorderedElementsAre("/blob/my-test", "/blob/my-test/0",
+ "/blob/my-test/1", "/blob/my-test/2",
+ "/blob/my-test/3"));
+ EXPECT_EQ(initialData, blobDataStorage);
+}
+
+TEST_F(BinaryStoreTest, TestCreateFromFile)
+{
+ auto testDataFile = createBlobStorage(inputProto);
+ auto initialData = blobDataStorage;
+ auto store =
+ binstore::BinaryStore::createFromFile(std::move(testDataFile), true);
+ ASSERT_TRUE(store);
+ EXPECT_EQ("/blob/my-test", store->getBaseBlobId());
+ EXPECT_THAT(store->getBlobIds(),
+ UnorderedElementsAre("/blob/my-test", "/blob/my-test/0",
+ "/blob/my-test/1", "/blob/my-test/2",
+ "/blob/my-test/3"));
+ // Check that the storage has not changed
+ EXPECT_EQ(initialData, blobDataStorage);
+}
+
+TEST_F(BinaryStoreTest, TestReadBlob)
+{
+ auto testDataFile = createBlobStorage(inputProto);
+ auto store =
+ binstore::BinaryStore::createFromFile(std::move(testDataFile), true);
+ ASSERT_TRUE(store);
+
+ const auto blobStoredData = store->readBlob("/blob/my-test/1");
+ EXPECT_FALSE(blobStoredData.empty());
+
+ decltype(blobStoredData) origData(blobData.begin(), blobData.end());
+
+ EXPECT_THAT(blobStoredData, ElementsAreArray(origData));
+}
+
+TEST_F(BinaryStoreTest, TestReadBlobError)
+{
+ auto testDataFile = createBlobStorage(inputProto);
+ auto store =
+ binstore::BinaryStore::createFromFile(std::move(testDataFile), true);
+ ASSERT_TRUE(store);
+
+ EXPECT_THROW(store->readBlob("/nonexistent/1"), ipmi::HandlerCompletion);
+}
+
+TEST_F(BinaryStoreTest, TestOpenReadOnlyBlob)
+{
+ auto testDataFile = createBlobStorage(inputProto);
+ auto store =
+ binstore::BinaryStore::createFromFile(std::move(testDataFile), true);
+ ASSERT_TRUE(store);
+
+ EXPECT_TRUE(
+ store->openOrCreateBlob("/blob/my-test/2", blobs::OpenFlags::read));
+ EXPECT_FALSE(store->openOrCreateBlob(
+ "/blob/my-test/2", blobs::OpenFlags::read & blobs::OpenFlags::write));
+}