Merge remote-tracking branch 'upstream/main' into update_cel

Change-Id: Ib58ea0d1a0f51cb0fd485c40b55a95e1d9963ffb
kyessenov · Mar 26, 2024 · a8d7520 · a8d7520
2 parents 5684394 + d9fe2f8
commit a8d7520
Show file tree

Hide file tree

Showing 39 changed files with 350 additions and 254 deletions.
diff --git a/.github/workflows/mobile-release.yml b/.github/workflows/mobile-release.yml
@@ -24,8 +24,7 @@ jobs:
       packages: read
     if: >-
       ${{
-          (github.repository == 'envoyproxy/envoy'
-           || vars.ENVOY_CI)
+          github.repository == 'envoyproxy/envoy'
           && (github.event.schedule
               || !contains(github.actor, '[bot]'))
       }}

diff --git a/envoy/ssl/BUILD b/envoy/ssl/BUILD
@@ -88,6 +88,7 @@ envoy_cc_library(
         "//envoy/network:connection_interface",
         "//envoy/network:post_io_action_interface",
         "//envoy/protobuf:message_validator_interface",
+        "//envoy/server:lifecycle_notifier_interface",
         "//envoy/server:options_interface",
         "//envoy/singleton:manager_interface",
     ],

diff --git a/envoy/ssl/handshaker.h b/envoy/ssl/handshaker.h
@@ -5,6 +5,7 @@
 #include "envoy/network/connection.h"
 #include "envoy/network/post_io_action.h"
 #include "envoy/protobuf/message_validator.h"
+#include "envoy/server/lifecycle_notifier.h"
 #include "envoy/server/options.h"
 #include "envoy/singleton/manager.h"
 
@@ -90,6 +91,11 @@ class HandshakerFactoryContext {
    * The list of supported protocols exposed via ALPN, from ContextConfig.
    */
   virtual absl::string_view alpnProtocols() const PURE;
+
+  /**
+   * @return reference to the server lifecycle notifier
+   */
+  virtual Server::ServerLifecycleNotifier& lifecycleNotifier() PURE;
 };
 
 struct HandshakerCapabilities {

diff --git a/mobile/.bazelrc b/mobile/.bazelrc
@@ -50,7 +50,7 @@ build:rules_xcodeproj --define=apple.experimental.tree_artifact_outputs=0
 # Disable global index store to work around https://github.com/buildbuddy-io/rules_xcodeproj/issues/1878
 build:rules_xcodeproj --features=-swift.use_global_index_store
 
-# Override PGV validation with NOP functions
+# Override PGV validation with NOP functions for binary size savings.
 build --@com_envoyproxy_protoc_gen_validate//bazel:template-flavor=nop
 
 build:mobile-dbg-common --compilation_mode=dbg
@@ -81,6 +81,8 @@ build:mobile-test-android --define=static_extension_registration=disabled
 # enabled for //test/kotlin/integration:xds_test and
 # //test/java/io/envoyproxy/envoymobile/engine:envoy_configuration_test
 build:mobile-test-android --define=envoy_mobile_xds=enabled
+# TODO(alyssar) fix
+build:mobile-test-android --define=envoy_yaml=enabled
 
 # Default flags for iOS tests.
 build:mobile-test-ios --config=ios
@@ -264,7 +266,6 @@ test:mobile-remote-ci-android --config=mobile-remote-ci
 
 build:mobile-remote-ci-cc --config=mobile-remote-ci
 test:mobile-remote-ci-cc --action_env=LD_LIBRARY_PATH
-test:mobile-remote-ci-cc --copt=-DUSE_API_LISTENER
 
 build:mobile-remote-ci-cc-no-yaml --config=mobile-remote-ci
 build:mobile-remote-ci-cc-no-yaml --define=envoy_yaml=disabled
@@ -279,18 +280,15 @@ build:mobile-remote-ci-cc-no-exceptions --copt=-fno-exceptions
 build:mobile-remote-ci-cc-test --config=mobile-remote-ci
 test:mobile-remote-ci-cc-test --test_output=all
 test:mobile-remote-ci-cc-test --config=mobile-remote-ci
-test:mobile-remote-ci-cc-test --@com_envoyproxy_protoc_gen_validate//bazel:template-flavor=
 test:mobile-remote-ci-cc-test --define=envoy_yaml=disabled
 
 build:mobile-remote-ci-macos-kotlin --config=mobile-remote-ci-macos
 build:mobile-remote-ci-macos-kotlin --fat_apk_cpu=x86_64
 build:mobile-remote-ci-macos-kotlin --define=envoy_yaml=disabled
-build:mobile-remote-ci-macos-kotlin --@com_envoyproxy_protoc_gen_validate//bazel:template-flavor=
 
 build:mobile-remote-ci-macos-swift --config=mobile-remote-ci-macos
 build:mobile-remote-ci-macos-swift --config=mobile-test-ios
 build:mobile-remote-ci-macos-swift --@envoy//bazel:http3=False
-build:mobile-remote-ci-macos-swift --@com_envoyproxy_protoc_gen_validate//bazel:template-flavor=
 
 build:mobile-remote-ci-core --config=mobile-remote-ci
 test:mobile-remote-ci-core --build_tests_only

diff --git a/mobile/library/cc/engine_builder.cc b/mobile/library/cc/engine_builder.cc
@@ -48,6 +48,21 @@ namespace {
 // https://source.chromium.org/chromium/chromium/src/+/main:net/quic/quic_context.h;drc=ccfe61524368c94b138ddf96ae8121d7eb7096cf;l=87
 constexpr int32_t SocketReceiveBufferSize = 1024 * 1024; // 1MB
 
+std::string nativeNameToConfig(absl::string_view name) {
+#ifndef ENVOY_ENABLE_YAML
+  return absl::StrCat("[type.googleapis.com/"
+                      "envoymobile.extensions.filters.http.platform_bridge.PlatformBridge] {"
+                      "platform_filter_name: \"",
+                      name, "\" }");
+#else
+  return absl::StrCat(
+      "{'@type': "
+      "type.googleapis.com/envoymobile.extensions.filters.http.platform_bridge.PlatformBridge, "
+      "platform_filter_name: ",
+      name, "}");
+#endif
+}
+
 } // namespace
 
 #ifdef ENVOY_MOBILE_XDS
@@ -171,6 +186,11 @@ EngineBuilder& EngineBuilder::setOnEngineRunning(std::function<void()> closure)
   return *this;
 }
 
+EngineBuilder& EngineBuilder::setEventTracker(std::unique_ptr<EnvoyEventTracker> event_tracker) {
+  event_tracker_ = std::move(event_tracker);
+  return *this;
+}
+
 EngineBuilder& EngineBuilder::addConnectTimeoutSeconds(int connect_timeout_seconds) {
   connect_timeout_seconds_ = connect_timeout_seconds;
   return *this;
@@ -380,13 +400,7 @@ EngineBuilder& EngineBuilder::addNativeFilter(std::string name, std::string type
 }
 
 EngineBuilder& EngineBuilder::addPlatformFilter(const std::string& name) {
-  addNativeFilter(
-      "envoy.filters.http.platform_bridge",
-      absl::StrCat(
-          "{'@type': "
-          "type.googleapis.com/envoymobile.extensions.filters.http.platform_bridge.PlatformBridge, "
-          "platform_filter_name: ",
-          name, "}"));
+  addNativeFilter("envoy.filters.http.platform_bridge", nativeNameToConfig(name));
   return *this;
 }
 
@@ -450,13 +464,16 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> EngineBuilder::generate
 
   for (auto filter = native_filter_chain_.rbegin(); filter != native_filter_chain_.rend();
        ++filter) {
-#ifdef ENVOY_ENABLE_YAML
     auto* native_filter = hcm->add_http_filters();
+#ifdef ENVOY_ENABLE_YAML
     native_filter->set_name((*filter).name_);
     MessageUtil::loadFromYaml((*filter).typed_config_, *native_filter->mutable_typed_config(),
                               ProtobufMessage::getStrictValidationVisitor());
 #else
-    IS_ENVOY_BUG("native filter chains can not be added when YAML is compiled out.");
+    Protobuf::TextFormat::ParseFromString((*filter).typed_config_,
+                                          native_filter->mutable_typed_config());
+    RELEASE_ASSERT(!native_filter->typed_config().DebugString().empty(),
+                   "Failed to parse" + (*filter).typed_config_);
 #endif
   }
 
@@ -904,10 +921,8 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> EngineBuilder::generate
 }
 
 EngineSharedPtr EngineBuilder::build() {
-  envoy_event_tracker null_tracker{};
-
   InternalEngine* envoy_engine =
-      new InternalEngine(std::move(callbacks_), std::move(logger_), null_tracker);
+      new InternalEngine(std::move(callbacks_), std::move(logger_), std::move(event_tracker_));
 
   for (const auto& [name, store] : key_value_stores_) {
     // TODO(goaway): This leaks, but it's tied to the life of the engine.

diff --git a/mobile/library/cc/engine_builder.h b/mobile/library/cc/engine_builder.h
@@ -131,6 +131,7 @@ class EngineBuilder {
   EngineBuilder& setEngineCallbacks(std::unique_ptr<EngineCallbacks> callbacks);
   [[deprecated("Use EngineBuilder::setEngineCallbacks instead")]] EngineBuilder&
   setOnEngineRunning(std::function<void()> closure);
+  EngineBuilder& setEventTracker(std::unique_ptr<EnvoyEventTracker> event_tracker);
   EngineBuilder& addConnectTimeoutSeconds(int connect_timeout_seconds);
   EngineBuilder& addDnsRefreshSeconds(int dns_refresh_seconds);
   EngineBuilder& addDnsFailureRefreshSeconds(int base, int max);
@@ -219,6 +220,7 @@ class EngineBuilder {
   Logger::Logger::Levels log_level_ = Logger::Logger::Levels::info;
   std::unique_ptr<EnvoyLogger> logger_{nullptr};
   std::unique_ptr<EngineCallbacks> callbacks_;
+  std::unique_ptr<EnvoyEventTracker> event_tracker_{nullptr};
 
   int connect_timeout_seconds_ = 30;
   int dns_refresh_seconds_ = 60;

diff --git a/mobile/library/common/BUILD b/mobile/library/common/BUILD
@@ -82,6 +82,8 @@ envoy_cc_library(
     ],
     repository = "@envoy",
     deps = [
+        "@com_google_absl//absl/container:flat_hash_map",
+        "@com_google_absl//absl/strings",
         "@envoy//source/common/common:base_logger_lib",
     ],
 )
diff --git a/mobile/library/common/engine_types.h b/mobile/library/common/engine_types.h
@@ -15,6 +15,9 @@
 
 #include "source/common/common/base_logger.h"
 
+#include "absl/container/flat_hash_map.h"
+#include "absl/strings/string_view.h"
+
 namespace Envoy {
 
 /** The callbacks for the Envoy Engine. */
@@ -30,4 +33,13 @@ struct EnvoyLogger {
   std::function<void()> on_exit = [] {};
 };
 
+inline constexpr absl::string_view ENVOY_EVENT_TRACKER_API_NAME = "event_tracker_api";
+
+/** The callbacks for Envoy Event Tracker. */
+struct EnvoyEventTracker {
+  std::function<void(const absl::flat_hash_map<std::string, std::string>&)> on_track =
+      [](const absl::flat_hash_map<std::string, std::string>&) {};
+  std::function<void()> on_exit = [] {};
+};
+
 } // namespace Envoy
diff --git a/mobile/library/common/internal_engine.cc b/mobile/library/common/internal_engine.cc
@@ -5,8 +5,6 @@
 #include "source/common/runtime/runtime_features.h"
 
 #include "absl/synchronization/notification.h"
-#include "library/common/bridge/utility.h"
-#include "library/common/data/utility.h"
 #include "library/common/stats/utility.h"
 
 namespace Envoy {
@@ -17,17 +15,14 @@ envoy_stream_t InternalEngine::initStream() { return current_stream_handle_++; }
 
 InternalEngine::InternalEngine(std::unique_ptr<EngineCallbacks> callbacks,
                                std::unique_ptr<EnvoyLogger> logger,
-                               envoy_event_tracker event_tracker,
+                               std::unique_ptr<EnvoyEventTracker> event_tracker,
                                Thread::PosixThreadFactoryPtr thread_factory)
     : thread_factory_(std::move(thread_factory)), callbacks_(std::move(callbacks)),
-      logger_(std::move(logger)), event_tracker_(event_tracker),
+      logger_(std::move(logger)), event_tracker_(std::move(event_tracker)),
       dispatcher_(std::make_unique<Event::ProvisionalDispatcher>()) {
   ExtensionRegistry::registerFactories();
 
-  // TODO(Augustyniak): Capturing an address of event_tracker_ and registering it in the API
-  // registry may lead to crashes at Engine shutdown. To be figured out as part of
-  // https://github.com/envoyproxy/envoy-mobile/issues/332
-  Envoy::Api::External::registerApi(std::string(envoy_event_tracker_api_name), &event_tracker_);
+  Api::External::registerApi(std::string(ENVOY_EVENT_TRACKER_API_NAME), &event_tracker_);
   // Envoy Mobile always requires dfp_mixed_scheme for the TLS and cleartext DFP clusters.
   // While dfp_mixed_scheme defaults to true, some environments force it to false (e.g. within
   // Google), so we force it back to true in Envoy Mobile.
@@ -37,8 +32,8 @@ InternalEngine::InternalEngine(std::unique_ptr<EngineCallbacks> callbacks,
 
 InternalEngine::InternalEngine(std::unique_ptr<EngineCallbacks> callbacks,
                                std::unique_ptr<EnvoyLogger> logger,
-                               envoy_event_tracker event_tracker)
-    : InternalEngine(std::move(callbacks), std::move(logger), event_tracker,
+                               std::unique_ptr<EnvoyEventTracker> event_tracker)
+    : InternalEngine(std::move(callbacks), std::move(logger), std::move(event_tracker),
                      Thread::PosixThreadFactory::create()) {}
 
 envoy_status_t InternalEngine::run(const std::string& config, const std::string& log_level) {
@@ -68,18 +63,18 @@ envoy_status_t InternalEngine::main(std::shared_ptr<Envoy::OptionsImplBase> opti
   {
     Thread::LockGuard lock(mutex_);
     TRY_NEEDS_AUDIT {
-      if (event_tracker_.track != nullptr) {
+      if (event_tracker_ != nullptr) {
         assert_handler_registration_ =
             Assert::addDebugAssertionFailureRecordAction([this](const char* location) {
-              const auto event = Bridge::Utility::makeEnvoyMap(
-                  {{"name", "assertion"}, {"location", std::string(location)}});
-              event_tracker_.track(event, event_tracker_.context);
+              absl::flat_hash_map<std::string, std::string> event{
+                  {{"name", "assertion"}, {"location", std::string(location)}}};
+              event_tracker_->on_track(event);
             });
         bug_handler_registration_ =
             Assert::addEnvoyBugFailureRecordAction([this](const char* location) {
-              const auto event = Bridge::Utility::makeEnvoyMap(
-                  {{"name", "bug"}, {"location", std::string(location)}});
-              event_tracker_.track(event, event_tracker_.context);
+              absl::flat_hash_map<std::string, std::string> event{
+                  {{"name", "bug"}, {"location", std::string(location)}}};
+              event_tracker_->on_track(event);
             });
       }
 
@@ -149,6 +144,9 @@ envoy_status_t InternalEngine::main(std::shared_ptr<Envoy::OptionsImplBase> opti
   bug_handler_registration_.reset(nullptr);
   assert_handler_registration_.reset(nullptr);
 
+  if (event_tracker_ != nullptr) {
+    event_tracker_->on_exit();
+  }
   callbacks_->on_exit();
 
   return run_success ? ENVOY_SUCCESS : ENVOY_FAILURE;

diff --git a/mobile/library/common/internal_engine.h b/mobile/library/common/internal_engine.h
@@ -26,7 +26,7 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
    * @param event_tracker, the event tracker to use for the emission of events.
    */
   InternalEngine(std::unique_ptr<EngineCallbacks> callbacks, std::unique_ptr<EnvoyLogger> logger,
-                 envoy_event_tracker event_tracker);
+                 std::unique_ptr<EnvoyEventTracker> event_tracker);
 
   /**
    * InternalEngine destructor.
@@ -123,7 +123,8 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
   GTEST_FRIEND_CLASS(InternalEngineTest, ThreadCreationFailed);
 
   InternalEngine(std::unique_ptr<EngineCallbacks> callbacks, std::unique_ptr<EnvoyLogger> logger,
-                 envoy_event_tracker event_tracker, Thread::PosixThreadFactoryPtr thread_factory);
+                 std::unique_ptr<EnvoyEventTracker> event_tracker,
+                 Thread::PosixThreadFactoryPtr thread_factory);
 
   envoy_status_t main(std::shared_ptr<Envoy::OptionsImplBase> options);
   static void logInterfaces(absl::string_view event,
@@ -135,7 +136,7 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
   Stats::StatNameSetPtr stat_name_set_;
   std::unique_ptr<EngineCallbacks> callbacks_;
   std::unique_ptr<EnvoyLogger> logger_;
-  envoy_event_tracker event_tracker_;
+  std::unique_ptr<EnvoyEventTracker> event_tracker_;
   Assert::ActionRegistrationPtr assert_handler_registration_;
   Assert::ActionRegistrationPtr bug_handler_registration_;
   Thread::MutexBasicLockable mutex_;

diff --git a/mobile/library/common/logger/logger_delegate.cc b/mobile/library/common/logger/logger_delegate.cc
@@ -9,14 +9,14 @@ namespace Logger {
 
 void EventTrackingDelegate::logWithStableName(absl::string_view stable_name, absl::string_view,
                                               absl::string_view, absl::string_view msg) {
-  if (event_tracker_.track == nullptr) {
+  if (event_tracker_ == nullptr || (*event_tracker_) == nullptr) {
     return;
   }
 
-  event_tracker_.track(Bridge::Utility::makeEnvoyMap({{"name", "event_log"},
-                                                      {"log_name", std::string(stable_name)},
-                                                      {"message", std::string(msg)}}),
-                       event_tracker_.context);
+  (*event_tracker_)
+      ->on_track({{"name", "event_log"},
+                  {"log_name", std::string(stable_name)},
+                  {"message", std::string(msg)}});
 }
 
 LambdaDelegate::LambdaDelegate(std::unique_ptr<EnvoyLogger> logger,

diff --git a/mobile/library/common/logger/logger_delegate.h b/mobile/library/common/logger/logger_delegate.h
@@ -15,14 +15,14 @@ namespace Logger {
 class EventTrackingDelegate : public SinkDelegate {
 public:
   explicit EventTrackingDelegate(DelegatingLogSinkSharedPtr log_sink)
-      : SinkDelegate(log_sink), event_tracker_(*static_cast<envoy_event_tracker*>(
-                                    Api::External::retrieveApi(envoy_event_tracker_api_name))) {}
+      : SinkDelegate(log_sink), event_tracker_(static_cast<std::unique_ptr<EnvoyEventTracker>*>(
+                                    Api::External::retrieveApi(ENVOY_EVENT_TRACKER_API_NAME))) {}
 
   void logWithStableName(absl::string_view stable_name, absl::string_view level,
                          absl::string_view component, absl::string_view msg) override;
 
 private:
-  envoy_event_tracker& event_tracker_;
+  std::unique_ptr<EnvoyEventTracker>* event_tracker_;
 };
 
 using EventTrackingDelegatePtr = std::unique_ptr<EventTrackingDelegate>;

diff --git a/mobile/library/common/types/c_types.cc b/mobile/library/common/types/c_types.cc
@@ -72,5 +72,3 @@ const envoy_data envoy_nodata = {0, NULL, envoy_noop_release, NULL};
 const envoy_headers envoy_noheaders = {0, NULL};
 
 const envoy_stats_tags envoy_stats_notags = {0, NULL};
-
-const char* envoy_event_tracker_api_name = "event_tracker_api";
Original file line number	Diff line number	Diff line change
Expand Up		@@ -72,5 +72,3 @@ const envoy_data envoy_nodata = {0, NULL, envoy_noop_release, NULL};
		const envoy_headers envoy_noheaders = {0, NULL};

		const envoy_stats_tags envoy_stats_notags = {0, NULL};

		const char* envoy_event_tracker_api_name = "event_tracker_api";