bootstrap-extensions: fix a crash on http callout

include/envoy/server/bootstrap_extension_config.h

@@ -15,6 +15,12 @@ namespace Server {
 class BootstrapExtension {
 public:
   virtual ~BootstrapExtension() = default;
+
+  /**
+   * Called when server is done initializing and we have the ServerFactoryContext available.
+   * @param context is the context to use for the extension.
+   */
+  virtual void onServerInitialized(Configuration::ServerFactoryContext& context) PURE;
 };
 
 using BootstrapExtensionPtr = std::unique_ptr<BootstrapExtension>;
@@ -34,7 +40,8 @@ class BootstrapExtensionFactory : public Config::TypedFactory {
    * implementation is unable to produce a factory with the provided parameters, it should throw an
    * EnvoyException. The returned pointer should never be nullptr.
    * @param config the custom configuration for this bootstrap extension type.
-   * @param context general filter context through which persistent resources can be accessed.
+   * @param context is the context to use for the extension. Note that the clusterManager is not
+   *    yet initialized at this point and **must not** be used.
    */
   virtual BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config,
                                                          ServerFactoryContext& context) PURE;

source/common/network/socket_interface.h

@@ -17,6 +17,8 @@ namespace Network {
 class SocketInterfaceExtension : public Server::BootstrapExtension {
 public:
   SocketInterfaceExtension(SocketInterface& sock_interface) : sock_interface_(sock_interface) {}
+  // Server::BootstrapExtension
+  void onServerInitialized(Server::Configuration::ServerFactoryContext&) override {}
 
 protected:
   SocketInterface& sock_interface_;

source/common/network/socket_interface_impl.h

@@ -20,6 +20,7 @@ class SocketInterfaceImpl : public SocketInterfaceBase {
   Server::BootstrapExtensionPtr
   createBootstrapExtension(const Protobuf::Message& config,
                            Server::Configuration::ServerFactoryContext& context) override;
+
   ProtobufTypes::MessagePtr createEmptyConfigProto() override;
   std::string name() const override {
     return "envoy.extensions.network.socket_interface.default_socket_interface";

source/extensions/bootstrap/wasm/config.cc

@@ -14,19 +14,19 @@ namespace Extensions {
 namespace Bootstrap {
 namespace Wasm {
 
-static const std::string INLINE_STRING = "<inline>";
+void WasmServiceExtension::onServerInitialized(
+    Server::Configuration::ServerFactoryContext& context) {
+  createWasm(context);
+}
 
-void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& config,
-                             Server::Configuration::ServerFactoryContext& context,
-                             CreateWasmServiceCallback&& cb) {
+void WasmServiceExtension::createWasm(Server::Configuration::ServerFactoryContext& context) {
   auto plugin = std::make_shared<Common::Wasm::Plugin>(
-      config.config().name(), config.config().root_id(), config.config().vm_config().vm_id(),
-      config.config().vm_config().runtime(),
-      Common::Wasm::anyToBytes(config.config().configuration()), config.config().fail_open(),
+      config_.config().name(), config_.config().root_id(), config_.config().vm_config().vm_id(),
+      config_.config().vm_config().runtime(),
+      Common::Wasm::anyToBytes(config_.config().configuration()), config_.config().fail_open(),
       envoy::config::core::v3::TrafficDirection::UNSPECIFIED, context.localInfo(), nullptr);
 
-  bool singleton = config.singleton();
-  auto callback = [&context, singleton, plugin, cb](Common::Wasm::WasmHandleSharedPtr base_wasm) {
+  auto callback = [this, &context, plugin](Common::Wasm::WasmHandleSharedPtr base_wasm) {
     if (!base_wasm) {
       if (plugin->fail_open_) {
         ENVOY_LOG(error, "Unable to create Wasm service {}", plugin->name_);
@@ -35,10 +35,11 @@ void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& con
       }
       return;
     }
-    if (singleton) {
+    if (config_.singleton()) {
       // Return a Wasm VM which will be stored as a singleton by the Server.
-      cb(std::make_unique<WasmService>(plugin, Common::Wasm::getOrCreateThreadLocalPlugin(
-                                                   base_wasm, plugin, context.dispatcher())));
+      wasm_service_ = std::make_unique<WasmService>(
+          plugin,
+          Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, context.dispatcher()));
       return;
     }
     // Per-thread WASM VM.
@@ -48,11 +49,11 @@ void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& con
     tls_slot->set([base_wasm, plugin](Event::Dispatcher& dispatcher) {
       return Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, dispatcher);
     });
-    cb(std::make_unique<WasmService>(plugin, std::move(tls_slot)));
+    wasm_service_ = std::make_unique<WasmService>(plugin, std::move(tls_slot));
   };
 
   if (!Common::Wasm::createWasm(
-          config.config().vm_config(), plugin, context.scope().createScope(""),
+          config_.config().vm_config(), plugin, context.scope().createScope(""),
           context.clusterManager(), context.initManager(), context.dispatcher(), context.api(),
           context.lifecycleNotifier(), remote_data_provider_, std::move(callback))) {
     // NB: throw if we get a synchronous configuration failures as this is how such failures are
@@ -69,12 +70,7 @@ WasmFactory::createBootstrapExtension(const Protobuf::Message& config,
       MessageUtil::downcastAndValidate<const envoy::extensions::wasm::v3::WasmService&>(
           config, context.messageValidationContext().staticValidationVisitor());
 
-  auto wasm_service_extension = std::make_unique<WasmServiceExtension>();
-  createWasm(typed_config, context,
-             [extension = wasm_service_extension.get()](WasmServicePtr wasm) {
-               extension->wasm_service_ = std::move(wasm);
-             });
-  return wasm_service_extension;
+  return std::make_unique<WasmServiceExtension>(typed_config);
 }
 
 // /**

source/extensions/bootstrap/wasm/config.h

@@ -34,37 +34,34 @@ class WasmService {
 };
 
 using WasmServicePtr = std::unique_ptr<WasmService>;
-using CreateWasmServiceCallback = std::function<void(WasmServicePtr)>;
 
-class WasmFactory : public Server::Configuration::BootstrapExtensionFactory,
-                    Logger::Loggable<Logger::Id::wasm> {
+class WasmFactory : public Server::Configuration::BootstrapExtensionFactory {
 public:
   ~WasmFactory() override = default;
   std::string name() const override { return "envoy.bootstrap.wasm"; }
-  void createWasm(const envoy::extensions::wasm::v3::WasmService& config,
-                  Server::Configuration::ServerFactoryContext& context,
-                  CreateWasmServiceCallback&& cb);
   Server::BootstrapExtensionPtr
   createBootstrapExtension(const Protobuf::Message& config,
                            Server::Configuration::ServerFactoryContext& context) override;
   ProtobufTypes::MessagePtr createEmptyConfigProto() override {
     return std::make_unique<envoy::extensions::wasm::v3::WasmService>();
   }
-
-private:
-  Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_;
 };
 
-class WasmServiceExtension : public Server::BootstrapExtension {
+class WasmServiceExtension : public Server::BootstrapExtension, Logger::Loggable<Logger::Id::wasm> {
 public:
+  WasmServiceExtension(const envoy::extensions::wasm::v3::WasmService& config) : config_(config) {}
   WasmService& wasmService() {
     ASSERT(wasm_service_ != nullptr);
     return *wasm_service_;
   }
+  void onServerInitialized(Server::Configuration::ServerFactoryContext& context) override;
 
 private:
+  void createWasm(Server::Configuration::ServerFactoryContext& context);
+
+  envoy::extensions::wasm::v3::WasmService config_;
   WasmServicePtr wasm_service_;
-  friend class WasmFactory;
+  Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_;
 };
 
 } // namespace Wasm

source/server/server.cc

@@ -139,7 +139,10 @@ InstanceImpl::~InstanceImpl() {
   ENVOY_LOG(debug, "destroyed listener manager");
 }
 
-Upstream::ClusterManager& InstanceImpl::clusterManager() { return *config_.clusterManager(); }
+Upstream::ClusterManager& InstanceImpl::clusterManager() {
+  ASSERT(config_.clusterManager() != nullptr);
+  return *config_.clusterManager();
+}
 
 void InstanceImpl::drainListeners() {
   ENVOY_LOG(info, "closing and draining listeners");
@@ -566,6 +569,11 @@ void InstanceImpl::initialize(const Options& options,
     stat_flush_timer_->enableTimer(stats_config.flushInterval());
   }
 
+  // Now that we are initialized, notify the bootstrap extensions.
+  for (auto&& bootstrap_extension : bootstrap_extensions_) {
+    bootstrap_extension->onServerInitialized(serverFactoryContext());
+  }
+
   // GuardDog (deadlock detection) object and thread setup before workers are
   // started and before our own run() loop runs.
   main_thread_guard_dog_ = std::make_unique<Server::GuardDogImpl>(

test/config/utility.cc

@@ -1154,6 +1154,12 @@ void ConfigHelper::addListenerFilter(const std::string& filter_yaml) {
   }
 }
 
+void ConfigHelper::addBootstrapExtension(const std::string& config) {
+  RELEASE_ASSERT(!finalized_, "");
+  auto* extension = bootstrap_.add_bootstrap_extensions();
+  TestUtility::loadFromYaml(config, *extension);
+}
+
 bool ConfigHelper::loadHttpConnectionManager(
     envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) {
   return loadFilter<

test/config/utility.h

@@ -199,6 +199,9 @@ class ConfigHelper {
   // Add a listener filter prior to existing filters.
   void addListenerFilter(const std::string& filter_yaml);
 
+  // Add a new bootstrap extension.
+  void addBootstrapExtension(const std::string& config);
+
   // Sets the client codec to the specified type.
   void setClientCodec(envoy::extensions::filters::network::http_connection_manager::v3::
                           HttpConnectionManager::CodecType type);

test/extensions/bootstrap/wasm/BUILD

@@ -45,6 +45,21 @@ envoy_extension_cc_test(
     ],
 )
 
+envoy_extension_cc_test(
+    name = "wasm_integration_test",
+    srcs = ["wasm_integration_test.cc"],
+    data = envoy_select_wasm([
+        "//test/extensions/bootstrap/wasm/test_data:http_cpp.wasm",
+    ]),
+    extension_name = "envoy.bootstrap.wasm",
+    deps = [
+        "//source/extensions/bootstrap/wasm:config",
+        "//source/extensions/common/wasm:wasm_lib",
+        "//test/extensions/common/wasm:wasm_runtime",
+        "//test/integration:http_protocol_integration_lib",
+    ],
+)
+
 envoy_extension_cc_test(
     name = "config_test",
     srcs = ["config_test.cc"],

test/extensions/bootstrap/wasm/config_test.cc

@@ -53,6 +53,7 @@ class WasmFactoryTest : public testing::TestWithParam<std::string> {
     EXPECT_CALL(context_, lifecycleNotifier())
         .WillRepeatedly(testing::ReturnRef(lifecycle_notifier_));
     extension_ = factory->createBootstrapExtension(config, context_);
+    extension_->onServerInitialized(context_);
     static_cast<Bootstrap::Wasm::WasmServiceExtension*>(extension_.get())->wasmService();
     EXPECT_CALL(init_watcher_, ready());
     init_manager_.initialize(init_watcher_);

test/extensions/bootstrap/wasm/test_data/BUILD

@@ -84,6 +84,11 @@ envoy_wasm_cc_binary(
     srcs = ["emscripten_cpp.cc"],
 )
 
+envoy_wasm_cc_binary(
+    name = "http_cpp.wasm",
+    srcs = ["http_cpp.cc"],
+)
+
 envoy_wasm_cc_binary(
     name = "logging_cpp.wasm",
     srcs = ["logging_cpp.cc"],

test/extensions/bootstrap/wasm/test_data/http_cpp.cc

@@ -0,0 +1,45 @@
+// NOLINT(namespace-envoy)
+#include <string>
+
+#include "proxy_wasm_intrinsics.h"
+
+template <typename T> std::unique_ptr<T> wrap_unique(T* ptr) { return std::unique_ptr<T>(ptr); }
+
+START_WASM_PLUGIN(WasmHttpCpp)
+
+// Required Proxy-Wasm ABI version.
+WASM_EXPORT(void, proxy_abi_version_0_1_0, ()) {}
+
+WASM_EXPORT(uint32_t, proxy_on_configure, (uint32_t, uint32_t)) {
+  proxy_set_tick_period_milliseconds(100);
+  return 1;
+}
+
+WASM_EXPORT(void, proxy_on_tick, (uint32_t)) {
+  HeaderStringPairs headers;
+  headers.push_back(std::make_pair<std::string, std::string>(":method", "GET"));
+  headers.push_back(std::make_pair<std::string, std::string>(":path", "/"));
+  headers.push_back(std::make_pair<std::string, std::string>(":authority", "example.com"));
+  HeaderStringPairs trailers;
+  uint32_t token;
+  WasmResult result = makeHttpCall("wasm_cluster", headers, "", trailers, 10000, &token);
+  // We have sent successfully, stop timer - we only want to send one request.
+  if (result == WasmResult::Ok) {
+    proxy_set_tick_period_milliseconds(0);
+  }
+}
+
+WASM_EXPORT(void, proxy_on_http_call_response, (uint32_t, uint32_t, uint32_t headers, uint32_t, uint32_t)) {
+  if (headers != 0) {
+    auto status = getHeaderMapValue(WasmHeaderMapType::HttpCallResponseHeaders, "status");
+    if ("200" == status->view()) {
+      proxy_set_tick_period_milliseconds(0);
+      return;
+    }
+  }
+  // Request failed - very possibly because of the integration test not being ready.
+  // Try again to prevent flakes.
+  proxy_set_tick_period_milliseconds(100);
+}
+
+END_WASM_PLUGIN

test/extensions/bootstrap/wasm/wasm_integration_test.cc

@@ -0,0 +1,91 @@
+#include "extensions/common/wasm/wasm.h"
+
+#include "test/extensions/common/wasm/wasm_runtime.h"
+#include "test/integration/http_protocol_integration.h"
+
+#include "gtest/gtest.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace Wasm {
+namespace {
+
+class WasmIntegrationTest : public HttpIntegrationTest, public testing::TestWithParam<std::string> {
+public:
+  WasmIntegrationTest()
+      : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, Network::Address::IpVersion::v4) {}
+
+  void createUpstreams() override {
+    HttpIntegrationTest::createUpstreams();
+    addFakeUpstream(FakeHttpConnection::Type::HTTP1);
+  }
+
+  void cleanup() {
+    if (wasm_connection_ != nullptr) {
+      ASSERT_TRUE(wasm_connection_->close());
+      ASSERT_TRUE(wasm_connection_->waitForDisconnect());
+    }
+    cleanupUpstreamAndDownstream();
+  }
+  void initialize() override {
+    auto httpwasm = TestEnvironment::substitute(
+        "{{ test_rundir }}/test/extensions/bootstrap/wasm/test_data/http_cpp.wasm");
+    config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
+      auto* wasm = bootstrap.mutable_static_resources()->add_clusters();
+      wasm->MergeFrom(bootstrap.static_resources().clusters()[0]);
+      wasm->set_name("wasm_cluster");
+    });
+
+    config_helper_.addBootstrapExtension(fmt::format(R"EOF(
+name: envoy.filters.http.wasm
+typed_config:
+  '@type': type.googleapis.com/envoy.extensions.wasm.v3.WasmService
+  singleton: true
+  config:
+    name: "singleton"
+    root_id: "singleton"
+    configuration:
+      '@type': type.googleapis.com/google.protobuf.StringValue
+      value: ""
+    vm_config:
+      vm_id: "my_vm_id"
+      runtime: "envoy.wasm.runtime.{}"
+      code:
+        local:
+          filename: {}
+  )EOF",
+                                                     GetParam(), httpwasm));
+    HttpIntegrationTest::initialize();
+  }
+
+  FakeHttpConnectionPtr wasm_connection_;
+  FakeStreamPtr wasm_request_;
+  IntegrationStreamDecoderPtr response_;
+};
+
+INSTANTIATE_TEST_SUITE_P(Runtimes, WasmIntegrationTest,
+                         Envoy::Extensions::Common::Wasm::sandbox_runtime_values,
+                         Envoy::Extensions::Common::Wasm::wasmTestParamsToString);
+GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WasmIntegrationTest);
+
+TEST_P(WasmIntegrationTest, FilterMakesCallInConfigureTime) {
+  initialize();
+  ASSERT_TRUE(fake_upstreams_.back()->waitForHttpConnection(*dispatcher_, wasm_connection_));
+
+  // Expect the filter to send us an HTTP request
+  ASSERT_TRUE(wasm_connection_->waitForNewStream(*dispatcher_, wasm_request_));
+
+  ASSERT_TRUE(wasm_request_->waitForEndStream(*dispatcher_));
+
+  // Respond back to the filter.
+  Http::TestResponseHeaderMapImpl response_headers{
+      {":status", "200"},
+  };
+  wasm_request_->encodeHeaders(response_headers, true);
+  cleanup();
+}
+
+} // namespace
+} // namespace Wasm
+} // namespace Extensions
+} // namespace Envoy

test/extensions/common/wasm/wasm_runtime.cc

@@ -35,6 +35,10 @@ std::vector<std::tuple<std::string, std::string>> runtimesAndLanguages() {
   return values;
 }
 
+std::string wasmTestParamsToString(const ::testing::TestParamInfo<std::string>& p) {
+  return p.param;
+}
+
 } // namespace Wasm
 } // namespace Common
 } // namespace Extensions