Add support for String Accessors to the C++ engine builder (envoyprox…

…y#2619)

Add support for String Accessors to the C++ engine builder

Introduces a C++ StringAccessor interface and method to convert to an envoy_string_accessor.
Minor cleanup of key_value_store handling in engine_builder.cc

Part of: envoyproxy#2498

Risk Level: Low
Testing: New unit tests
Docs Changes: N/A
Release Notes: Updated version_history.rst

Signed-off-by: Ryan Hamilton <rch@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>

docs/root/intro/version_history.rst

@@ -21,6 +21,7 @@ Features:
 - kotlin: add a way to tell Envoy Mobile to respect system proxy settings by calling an ``enableProxying(true)`` method on the engine builder. (:issue:`#2416 <2416>`)
 - kotlin: add a ``enableSkipDNSLookupForProxiedRequests(true)`` knob for controlling whether Envoy waits on DNS response in the dynamic forward proxy filter for proxied requests. (:issue:`#2602 <2602>`)
 - api: Add various methods to C++ EngineBuilder to bring it to parity with the Java and Obj-C builders. (:issue:`#2498 <2498>`)
+- api: Add support for String Accessors to the C++ EngineBuilder. (:issue:`#2498 <2498>`)
 - api: added upstream protocol to final stream intel. (:issue:`#2613 <2613>`)
 
 0.5.0 (September 2, 2022)

library/cc/BUILD

@@ -43,6 +43,7 @@ envoy_cc_library(
         "stream_callbacks.cc",
         "stream_client.cc",
         "stream_prototype.cc",
+        "string_accessor.cc",
         "upstream_http_protocol.cc",
     ],
     hdrs = [
@@ -69,6 +70,7 @@ envoy_cc_library(
         "stream_callbacks.h",
         "stream_client.h",
         "stream_prototype.h",
+        "string_accessor.h",
         "trailers.h",
         "upstream_http_protocol.h",
     ],
@@ -77,6 +79,7 @@ envoy_cc_library(
     visibility = ["//visibility:public"],
     deps = [
         "//library/common:envoy_main_interface_lib_no_stamp",
+        "//library/common/api:c_types",
         "//library/common/data:utility_lib",
         "//library/common/extensions/key_value/platform:config",
     ],

library/cc/engine_builder.cc

@@ -201,6 +201,12 @@ EngineBuilder::enablePlatformCertificatesValidation(bool platform_certificates_v
   return *this;
 }
 
+EngineBuilder& EngineBuilder::addStringAccessor(const std::string& name,
+                                                StringAccessorSharedPtr accessor) {
+  string_accessors_[name] = accessor;
+  return *this;
+}
+
 std::string EngineBuilder::generateConfigStr() const {
 #if defined(__APPLE__)
   std::string dns_resolver_name = "envoy.network.dns_resolver.apple";
@@ -322,14 +328,21 @@ EngineSharedPtr EngineBuilder::build() {
   } else {
     config_str = config_override_for_tests_;
   }
-  auto envoy_engine =
+  envoy_engine_t envoy_engine =
       init_engine(this->callbacks_->asEnvoyEngineCallbacks(), null_logger, null_tracker);
 
-  for (auto it = key_value_stores_.begin(); it != key_value_stores_.end(); ++it) {
+  for (const auto& [name, store] : key_value_stores_) {
     // TODO(goaway): This leaks, but it's tied to the life of the engine.
-    envoy_kv_store* api = static_cast<envoy_kv_store*>(safe_malloc(sizeof(envoy_kv_store)));
-    *api = it->second->asEnvoyKeyValueStore();
-    register_platform_api(it->first.c_str(), api);
+    auto* api = new envoy_kv_store();
+    *api = store->asEnvoyKeyValueStore();
+    register_platform_api(name.c_str(), api);
+  }
+
+  for (const auto& [name, accessor] : string_accessors_) {
+    // TODO(RyanTheOptimist): This leaks, but it's tied to the life of the engine.
+    auto* api = new envoy_string_accessor();
+    *api = StringAccessor::asEnvoyStringAccessor(accessor);
+    register_platform_api(name.c_str(), api);
   }
 
   run_engine(envoy_engine, config_str.c_str(), logLevelToString(this->log_level_).c_str(),

library/cc/engine_builder.h

@@ -10,6 +10,7 @@
 #include "engine_callbacks.h"
 #include "key_value_store.h"
 #include "log_level.h"
+#include "string_accessor.h"
 
 namespace Envoy {
 namespace Platform {
@@ -39,6 +40,7 @@ class EngineBuilder {
   EngineBuilder& addStatsFlushSeconds(int stats_flush_seconds);
   EngineBuilder& addVirtualClusters(const std::string& virtual_clusters);
   EngineBuilder& addKeyValueStore(const std::string& name, KeyValueStoreSharedPtr key_value_store);
+  EngineBuilder& addStringAccessor(const std::string& name, StringAccessorSharedPtr accessor);
   EngineBuilder& setAppVersion(const std::string& app_version);
   EngineBuilder& setAppId(const std::string& app_id);
   EngineBuilder& setDeviceOs(const std::string& app_id);
@@ -63,9 +65,10 @@ class EngineBuilder {
 
   // TODO(crockeo): add after filter integration
   // EngineBuilder& addPlatformFilter(name: String = UUID.randomUUID().toString(), factory: () ->
-  // Filter): EngineBuilder& addNativeFilter(name: String = UUID.randomUUID().toString(),
-  // typedConfig: String): EngineBuilder& addStringAccessor(name: String, accessor:
-  // EnvoyStringAccessor): EngineBuilder {
+  // Filter):
+  // EngineBuilder& addNativeFilter(name: String = UUID.randomUUID().toString(),
+  // typedConfig: String):
+
 protected:
   void setOverrideConfigForTests(std::string config) { config_override_for_tests_ = config; }
   void setAdminAddressPathForTests(std::string admin) { admin_address_path_for_tests_ = admin; }
@@ -115,7 +118,7 @@ class EngineBuilder {
   // TODO(crockeo): add after filter integration
   // std::vector<EnvoyHTTPFilterFactory> http_platform_filter_factories_;
   // std::vector<EnvoyNativeFilterConfig> native_filter_chain_;
-  // std::map<std::string, EnvoyStringAccessor> string_accessors_;
+  absl::flat_hash_map<std::string, StringAccessorSharedPtr> string_accessors_;
 };
 
 using EngineBuilderSharedPtr = std::shared_ptr<EngineBuilder>;

library/cc/string_accessor.cc

@@ -0,0 +1,22 @@
+#include "library/cc/string_accessor.h"
+
+#include "library/common/data/utility.h"
+
+namespace Envoy {
+namespace Platform {
+
+namespace {
+
+envoy_data c_string_accessor_read(const void* context) {
+  auto accessor = *static_cast<const StringAccessorSharedPtr*>(context);
+  return Data::Utility::copyToBridgeData(accessor->get());
+}
+
+} // namespace
+
+envoy_string_accessor StringAccessor::asEnvoyStringAccessor(StringAccessorSharedPtr accessor) {
+  return envoy_string_accessor{&c_string_accessor_read, new StringAccessorSharedPtr(accessor)};
+}
+
+} // namespace Platform
+} // namespace Envoy

library/cc/string_accessor.h

@@ -0,0 +1,36 @@
+#pragma once
+
+#include <memory>
+
+#include "envoy/common/pure.h"
+
+#include "absl/types/optional.h"
+#include "library/common/api/c_types.h"
+
+namespace Envoy {
+namespace Platform {
+
+/**
+ * `StringAccessor` is an interface that may be implemented to provide access to an arbitrary
+ * string from the application.
+ */
+class StringAccessor : public std::enable_shared_from_this<StringAccessor> {
+public:
+  virtual ~StringAccessor() = default;
+
+  /**
+   * Returns the string associated with this accessor.
+   */
+  virtual const std::string& get() const PURE;
+
+  /**
+   * Maps an implementation to its internal representation.
+   * @return portable internal type used to call an implementation.
+   */
+  static envoy_string_accessor asEnvoyStringAccessor(std::shared_ptr<StringAccessor> accessor);
+};
+
+using StringAccessorSharedPtr = std::shared_ptr<StringAccessor>;
+
+} // namespace Platform
+} // namespace Envoy

test/cc/unit/envoy_config_test.cc

@@ -8,7 +8,9 @@
 #include "gtest/gtest.h"
 #include "library/cc/engine_builder.h"
 #include "library/cc/log_level.h"
+#include "library/common/api/external.h"
 #include "library/common/config/internal.h"
+#include "library/common/data/utility.h"
 
 #if defined(__APPLE__)
 #include "source/extensions/network/dns_resolver/apple/apple_dns_impl.h"
@@ -323,7 +325,7 @@ TEST(TestConfig, EnableHttp3) {
 }
 
 TEST(TestConfig, RemainingTemplatesThrows) {
-  auto engine_builder = EngineBuilder("{{ template_that_i_will_not_fill }}");
+  EngineBuilder engine_builder("{{ template_that_i_will_not_fill }}");
   try {
     engine_builder.generateConfigStr();
     FAIL() << "Expected std::runtime_error";
@@ -333,7 +335,7 @@ TEST(TestConfig, RemainingTemplatesThrows) {
 }
 
 TEST(TestConfig, EnablePlatformCertificatesValidation) {
-  auto engine_builder = EngineBuilder();
+  EngineBuilder engine_builder;
   envoy::config::bootstrap::v3::Bootstrap bootstrap;
   engine_builder.enablePlatformCertificatesValidation(false);
   auto config_str1 = engine_builder.generateConfigStr();
@@ -355,5 +357,39 @@ TEST(TestConfig, EnablePlatformCertificatesValidation) {
 #endif
 }
 
+// Implementation of StringAccessor which tracks the number of times it was used.
+class TestStringAccessor : public StringAccessor {
+public:
+  explicit TestStringAccessor(std::string data) : data_(data) {}
+  ~TestStringAccessor() override = default;
+
+  // StringAccessor
+  const std::string& get() const override {
+    ++count_;
+    return data_;
+  }
+
+  int count() { return count_; }
+
+private:
+  std::string data_;
+  mutable int count_ = 0;
+};
+
+TEST(TestConfig, StringAccessors) {
+  std::string name("accessor_name");
+  EngineBuilder engine_builder;
+  std::string data_string = "envoy string";
+  auto accessor = std::make_shared<TestStringAccessor>(data_string);
+  engine_builder.addStringAccessor(name, accessor);
+  EngineSharedPtr engine = engine_builder.build();
+  auto c_accessor = static_cast<envoy_string_accessor*>(Envoy::Api::External::retrieveApi(name));
+  ASSERT_TRUE(c_accessor != nullptr);
+  EXPECT_EQ(0, accessor->count());
+  envoy_data data = c_accessor->get_string(c_accessor->context);
+  EXPECT_EQ(1, accessor->count());
+  EXPECT_EQ(data_string, Data::Utility::copyToString(data));
+  release_envoy_data(data);
+}
 } // namespace
 } // namespace Envoy