Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions envoy_build_config/extensions_build_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ EXTENSIONS = {
"envoy.filters.http.router": "//source/extensions/filters/http/router:config",
"envoy.filters.network.http_connection_manager": "//source/extensions/filters/network/http_connection_manager:config",
"envoy.http.original_ip_detection.xff": "//source/extensions/http/original_ip_detection/xff:config",
"envoy.key_value.platform": "@envoy_mobile//library/common/extensions/key_value/platform:config",
"envoy.network.dns_resolver.apple": "//source/extensions/network/dns_resolver/apple:config",
"envoy.retry.options.network_configuration": "@envoy_mobile//library/common/extensions/retry/options/network_configuration:config",
"envoy.stat_sinks.metrics_service": "//source/extensions/stat_sinks/metrics_service:config",
Expand Down
31 changes: 31 additions & 0 deletions library/common/extensions/key_value/platform/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
load(
"@envoy//bazel:envoy_build_system.bzl",
"envoy_cc_extension",
"envoy_extension_package",
"envoy_proto_library",
)

licenses(["notice"]) # Apache 2

envoy_extension_package()

envoy_proto_library(
name = "platform",
srcs = ["platform.proto"],
)

envoy_cc_extension(
name = "config",
srcs = ["config.cc"],
hdrs = ["config.h"],
repository = "@envoy",
deps = [
":platform_cc_proto",
"@envoy//envoy/common:key_value_store_interface",
"@envoy//envoy/event:dispatcher_interface",
"@envoy//envoy/filesystem:filesystem_interface",
"@envoy//envoy/registry",
"@envoy//source/common/common:key_value_store_lib",
"@envoy_api//envoy/config/common/key_value/v3:pkg_cc_proto",
],
)
52 changes: 52 additions & 0 deletions library/common/extensions/key_value/platform/config.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#include "library/common/extensions/key_value/platform/config.h"

#include "envoy/config/common/key_value/v3/config.pb.h"
#include "envoy/config/common/key_value/v3/config.pb.validate.h"
#include "envoy/registry/registry.h"

namespace Envoy {
namespace Extensions {
namespace KeyValue {

PlatformKeyValueStore::PlatformKeyValueStore(Event::Dispatcher& dispatcher,
std::chrono::milliseconds flush_interval,
PlatformInterface& platform_interface,
const std::string& key)
: KeyValueStoreBase(dispatcher, flush_interval), platform_interface_(platform_interface),
key_(key) {
const std::string contents = platform_interface.read(key);
if (!parseContents(contents, store_)) {
ENVOY_LOG(warn, "Failed to parse key value store contents {}", key);
}
}

void PlatformKeyValueStore::flush() {
std::string output;
for (const auto& it : store_) {
absl::StrAppend(&output, it.first.length(), "\n", it.first, it.second.length(), "\n",
it.second);
}
platform_interface_.flush(key_, output);
}

KeyValueStorePtr
PlatformKeyValueStoreFactory::createStore(const Protobuf::Message& config,
ProtobufMessage::ValidationVisitor& validation_visitor,
Event::Dispatcher& dispatcher, Filesystem::Instance&) {
const auto& typed_config = MessageUtil::downcastAndValidate<
const ::envoy::config::common::key_value::v3::KeyValueStoreConfig&>(config,
validation_visitor);
const auto file_config = MessageUtil::anyConvertAndValidate<
envoymobile::extensions::key_value::platform::PlatformKeyValueStoreConfig>(
typed_config.config().typed_config(), validation_visitor);
auto milliseconds =
std::chrono::milliseconds(DurationUtil::durationToMilliseconds(file_config.flush_interval()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I understand the value in supporting periodic flushes, would it make sense for the base case to be synchronous/immediate?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to for any of the current use cases - flushing on every dns resolution or every response headers seems expensive. @RyanTheOptimist for thoughts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah for a per-request or per-DNS resolution store, immediate sync would be too expensive. Chrome's analogous system uses periodic writes, fwiw. I could imagine that we might have other use cases for the key-value store for which immediate write might be desirable so maybe there is value in having that as an option. But for the uses we currently have, periodic flushes make sense to me.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay that's fine - you all are the primary users of this right now anyways. I just thought it might make for easier testing and/or open up other simple use cases that aren't as perf sensitive.

return std::make_unique<PlatformKeyValueStore>(
dispatcher, milliseconds, platform_interface_.value().get(), file_config.key());
}

REGISTER_FACTORY(PlatformKeyValueStoreFactory, KeyValueStoreFactory);

} // namespace KeyValue
} // namespace Extensions
} // namespace Envoy
62 changes: 62 additions & 0 deletions library/common/extensions/key_value/platform/config.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#include "envoy/common/key_value_store.h"

#include "source/common/common/key_value_store_base.h"

#include "library/common/extensions/key_value/platform/platform.pb.h"
#include "library/common/extensions/key_value/platform/platform.pb.validate.h"

namespace Envoy {
namespace Extensions {
namespace KeyValue {

class PlatformInterface {
public:
virtual ~PlatformInterface() {}
virtual void flush(const std::string& key, const std::string& contents) PURE;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I would suggest one of store, save, write or put as opposed to flush probably.

virtual std::string read(const std::string& key) const PURE;
};

// A platform file based key value store, which loads from and flushes based on
// a key. An example implementation would be flushing to the android prefs file.
//
// All keys and values are flushed to a single entry as
// [length]\n[key][length]\n[value]
class PlatformKeyValueStore : public KeyValueStoreBase {
public:
PlatformKeyValueStore(Event::Dispatcher& dispatcher, std::chrono::milliseconds flush_interval,
PlatformInterface& platform_interface, const std::string& key);
// KeyValueStore
void flush() override;

private:
PlatformInterface& platform_interface_;
const std::string key_;
};

class PlatformKeyValueStoreFactory : public KeyValueStoreFactory {
public:
PlatformKeyValueStoreFactory() {}
PlatformKeyValueStoreFactory(PlatformInterface& platform_interface)
: platform_interface_(platform_interface) {}

// KeyValueStoreFactory
KeyValueStorePtr createStore(const Protobuf::Message& config,
ProtobufMessage::ValidationVisitor& validation_visitor,
Event::Dispatcher& dispatcher,
Filesystem::Instance& file_system) override;

// TypedFactory
ProtobufTypes::MessagePtr createEmptyConfigProto() override {
return ProtobufTypes::MessagePtr{
new envoymobile::extensions::key_value::platform::PlatformKeyValueStoreConfig()};
}

std::string name() const override { return "envoy.key_value.platform"; }
// TODO(alyssawilk, goaway) the default PlatformInterface should do up calls through Java and this
// can be moved to a non-optional reference.
OptRef<PlatformInterface> platform_interface_;
};

} // namespace KeyValue
} // namespace Extensions
} // namespace Envoy
15 changes: 15 additions & 0 deletions library/common/extensions/key_value/platform/platform.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
syntax = "proto3";

package envoymobile.extensions.key_value.platform;

import "google/protobuf/duration.proto";

import "validate/validate.proto";

message PlatformKeyValueStoreConfig {
// The key to store the contents to.
string key = 1 [(validate.rules).string = {min_len: 1}];

// The interval at which the key value store should be flushed.
google.protobuf.Duration flush_interval = 2;
}
22 changes: 22 additions & 0 deletions test/common/extensions/key_value/platform/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
load("@envoy//bazel:envoy_build_system.bzl", "envoy_package")
load(
"@envoy//test/extensions:extensions_build_system.bzl",
"envoy_extension_cc_test",
)

licenses(["notice"]) # Apache 2

envoy_package()

envoy_extension_cc_test(
name = "platform_store_test",
srcs = ["platform_store_test.cc"],
extension_names = ["envoy.key_value.platform"],
repository = "@envoy",
deps = [
"//library/common/extensions/key_value/platform:config",
"@envoy//source/common/common:key_value_store_lib",
"@envoy//test/mocks/event:event_mocks",
"@envoy//test/test_common:file_system_for_test_lib",
],
)
120 changes: 120 additions & 0 deletions test/common/extensions/key_value/platform/platform_store_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
#include <memory>
#include <string>

#include "test/mocks/event/mocks.h"

#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "library/common/extensions/key_value/platform/config.h"
#include "library/common/extensions/key_value/platform/platform.pb.h"

using testing::NiceMock;

namespace Envoy {
namespace Extensions {
namespace KeyValue {
namespace {

class TestPlatformInterface : public PlatformInterface {
virtual void flush(const std::string& key, const std::string& contents) {
store_.erase(key);
store_.emplace(key, contents);
}
virtual std::string read(const std::string& key) const {
auto it = store_.find(key);
if (it == store_.end()) {
return "";
}
return it->second;
}

absl::flat_hash_map<std::string, std::string> store_;
};

class PlatformStoreTest : public testing::Test {
protected:
PlatformStoreTest() { createStore(); }

void createStore() {
flush_timer_ = new NiceMock<Event::MockTimer>(&dispatcher_);
store_ =
std::make_unique<PlatformKeyValueStore>(dispatcher_, flush_interval_, mock_platform_, key_);
}
NiceMock<Event::MockDispatcher> dispatcher_;
std::string key_{"key"};
std::unique_ptr<PlatformKeyValueStore> store_{};
std::chrono::seconds flush_interval_{5};
Event::MockTimer* flush_timer_ = nullptr;
TestPlatformInterface mock_platform_;
};

TEST_F(PlatformStoreTest, Basic) {
EXPECT_EQ(absl::nullopt, store_->get("foo"));
store_->addOrUpdate("foo", "bar");
EXPECT_EQ("bar", store_->get("foo").value());
store_->addOrUpdate("foo", "eep");
EXPECT_EQ("eep", store_->get("foo").value());
store_->remove("foo");
EXPECT_EQ(absl::nullopt, store_->get("foo"));
}

TEST_F(PlatformStoreTest, Persist) {
store_->addOrUpdate("foo", "bar");
store_->addOrUpdate("by\nz", "ee\np");
ASSERT_TRUE(flush_timer_->enabled_);
flush_timer_->invokeCallback(); // flush
EXPECT_TRUE(flush_timer_->enabled_);
// Not flushed as 5ms didn't pass.
store_->addOrUpdate("baz", "eep");

flush_interval_ = std::chrono::seconds(0);
createStore();
KeyValueStore::ConstIterateCb validate = [](const std::string& key, const std::string&) {
EXPECT_TRUE(key == "foo" || key == "by\nz");
return KeyValueStore::Iterate::Continue;
};

EXPECT_EQ("bar", store_->get("foo").value());
EXPECT_EQ("ee\np", store_->get("by\nz").value());
EXPECT_FALSE(store_->get("baz").has_value());
store_->iterate(validate);

// This will flush due to 0ms flush interval
store_->addOrUpdate("baz", "eep");
createStore();
EXPECT_TRUE(store_->get("baz").has_value());

// This will flush due to 0ms flush interval
store_->remove("bar");
createStore();
EXPECT_FALSE(store_->get("bar").has_value());
}

TEST_F(PlatformStoreTest, Iterate) {
store_->addOrUpdate("foo", "bar");
store_->addOrUpdate("baz", "eep");

int full_counter = 0;
KeyValueStore::ConstIterateCb validate = [&full_counter](const std::string& key,
const std::string&) {
++full_counter;
EXPECT_TRUE(key == "foo" || key == "baz");
return KeyValueStore::Iterate::Continue;
};
store_->iterate(validate);
EXPECT_EQ(2, full_counter);

int stop_early_counter = 0;
KeyValueStore::ConstIterateCb stop_early = [&stop_early_counter](const std::string&,
const std::string&) {
++stop_early_counter;
return KeyValueStore::Iterate::Break;
};
store_->iterate(stop_early);
EXPECT_EQ(1, stop_early_counter);
}

} // namespace
} // namespace KeyValue
} // namespace Extensions
} // namespace Envoy