Configuration canary for the second or further plugins. (#289)

Fixes #175.

Signed-off-by: Ingwon Song <igsong@google.com>

Author: ingwonsong

.github/workflows/test.yml

@@ -56,6 +56,18 @@ jobs:
         --config=clang
         -c opt
         $(bazel query 'kind(was.*_rust_binary, //test/test_data/...)')
+        $(bazel query 'kind(_optimized_wasm_cc_binary, //test/test_data/...)')
+
+    # Currently, in Github Action, "Bazel build step" creates `wasm_canary_check.wasm` directory as a temporal directory.
+    # Since the name of this directory has "*.wasm" pattern, the step "Mangle build rules to use existing test data" fails.
+    # So, here, we clear out the directories in test_data.
+    - name: Clean up test data
+      shell: bash
+      run: |
+        # Remove temporal directories
+        for i in $(find bazel-bin/test/test_data/ -mindepth 1 -maxdepth 1 -type d); do \
+          rm -rf $i; \
+        done
 
     - name: Upload test data
       uses: actions/upload-artifact@v2
@@ -201,7 +213,7 @@ jobs:
           os: ubuntu-20.04
           arch: s390x
           action: test
-          flags: --config=clang
+          flags: --config=clang --test_timeout=1800
           run_under: docker run --rm --env HOME=$HOME --env USER=$(id -un) --volume "$HOME:$HOME" --workdir $(pwd) --user $(id -u):$(id -g) --platform linux/s390x piotrsikora/build-tools:bazel-5.0.0-clang-13-gcc-11
           cache: true
         - name: 'Wasmtime on macOS/x86_64'

WORKSPACE

@@ -7,3 +7,15 @@ proxy_wasm_cpp_host_repositories()
 load("@proxy_wasm_cpp_host//bazel:dependencies.bzl", "proxy_wasm_cpp_host_dependencies")
 
 proxy_wasm_cpp_host_dependencies()
+
+load("@proxy_wasm_cpp_sdk//bazel:repositories.bzl", "proxy_wasm_cpp_sdk_repositories")
+
+proxy_wasm_cpp_sdk_repositories()
+
+load("@proxy_wasm_cpp_sdk//bazel:dependencies.bzl", "proxy_wasm_cpp_sdk_dependencies")
+
+proxy_wasm_cpp_sdk_dependencies()
+
+load("@proxy_wasm_cpp_sdk//bazel:dependencies_extra.bzl", "proxy_wasm_cpp_sdk_dependencies_extra")
+
+proxy_wasm_cpp_sdk_dependencies_extra()

bazel/repositories.bzl

@@ -86,9 +86,9 @@ def proxy_wasm_cpp_host_repositories():
     maybe(
         http_archive,
         name = "proxy_wasm_cpp_sdk",
-        sha256 = "c57de2425b5c61d7f630c5061e319b4557ae1f1c7526e5a51c33dc1299471b08",
-        strip_prefix = "proxy-wasm-cpp-sdk-fd0be8405db25de0264bdb78fae3a82668c03782",
-        urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-sdk/archive/fd0be8405db25de0264bdb78fae3a82668c03782.tar.gz"],
+        sha256 = "89792fc1abca331f29f99870476a04146de5e82ff903bdffca90e6729c1f2470",
+        strip_prefix = "proxy-wasm-cpp-sdk-95bb82ce45c41d9100fd1ec15d2ffc67f7f3ceee",
+        urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-sdk/archive/95bb82ce45c41d9100fd1ec15d2ffc67f7f3ceee.tar.gz"],
     )
 
     # Test dependencies.

include/proxy-wasm/wasm.h

@@ -314,6 +314,10 @@ class WasmBase : public std::enable_shared_from_this<WasmBase> {
   std::shared_ptr<VmIdHandle> vm_id_handle_;
 };
 
+using WasmHandleFactory = std::function<std::shared_ptr<WasmHandleBase>(std::string_view vm_id)>;
+using WasmHandleCloneFactory =
+    std::function<std::shared_ptr<WasmHandleBase>(std::shared_ptr<WasmHandleBase> wasm)>;
+
 // Handle which enables shutdown operations to run post deletion (e.g. post listener drain).
 class WasmHandleBase : public std::enable_shared_from_this<WasmHandleBase> {
 public:
@@ -324,6 +328,9 @@ class WasmHandleBase : public std::enable_shared_from_this<WasmHandleBase> {
     }
   }
 
+  bool canary(const std::shared_ptr<PluginBase> &plugin,
+              const WasmHandleCloneFactory &clone_factory);
+
   void kill() { wasm_base_ = nullptr; }
 
   std::shared_ptr<WasmBase> &wasm() { return wasm_base_; }
@@ -335,10 +342,6 @@ class WasmHandleBase : public std::enable_shared_from_this<WasmHandleBase> {
 std::string makeVmKey(std::string_view vm_id, std::string_view configuration,
                       std::string_view code);
 
-using WasmHandleFactory = std::function<std::shared_ptr<WasmHandleBase>(std::string_view vm_id)>;
-using WasmHandleCloneFactory =
-    std::function<std::shared_ptr<WasmHandleBase>(std::shared_ptr<WasmHandleBase> wasm)>;
-
 // Returns nullptr on failure (i.e. initialization of the VM fails).
 std::shared_ptr<WasmHandleBase> createWasm(const std::string &vm_key, const std::string &code,
                                            const std::shared_ptr<PluginBase> &plugin,

src/wasm.cc

@@ -447,6 +447,35 @@ void WasmBase::finishShutdown() {
   }
 }
 
+bool WasmHandleBase::canary(const std::shared_ptr<PluginBase> &plugin,
+                            const WasmHandleCloneFactory &clone_factory) {
+  if (this->wasm() == nullptr) {
+    return false;
+  }
+  auto configuration_canary_handle = clone_factory(shared_from_this());
+  if (!configuration_canary_handle) {
+    this->wasm()->fail(FailState::UnableToCloneVm, "Failed to clone Base Wasm");
+    return false;
+  }
+  if (!configuration_canary_handle->wasm()->initialize()) {
+    configuration_canary_handle->wasm()->fail(FailState::UnableToInitializeCode,
+                                              "Failed to initialize Wasm code");
+    return false;
+  }
+  auto *root_context = configuration_canary_handle->wasm()->start(plugin);
+  if (root_context == nullptr) {
+    configuration_canary_handle->wasm()->fail(FailState::StartFailed, "Failed to start base Wasm");
+    return false;
+  }
+  if (!configuration_canary_handle->wasm()->configure(root_context, plugin)) {
+    configuration_canary_handle->wasm()->fail(FailState::ConfigureFailed,
+                                              "Failed to configure base Wasm plugin");
+    return false;
+  }
+  configuration_canary_handle->kill();
+  return true;
+}
+
 std::shared_ptr<WasmHandleBase> createWasm(const std::string &vm_key, const std::string &code,
                                            const std::shared_ptr<PluginBase> &plugin,
                                            const WasmHandleFactory &factory,
@@ -465,44 +494,29 @@ std::shared_ptr<WasmHandleBase> createWasm(const std::string &vm_key, const std:
         base_wasms->erase(it);
       }
     }
-    if (wasm_handle) {
-      return wasm_handle;
-    }
-    wasm_handle = factory(vm_key);
     if (!wasm_handle) {
-      return nullptr;
+      // If no cached base_wasm, creates a new base_wasm, loads the code and initializes it.
+      wasm_handle = factory(vm_key);
+      if (!wasm_handle) {
+        return nullptr;
+      }
+      if (!wasm_handle->wasm()->load(code, allow_precompiled)) {
+        wasm_handle->wasm()->fail(FailState::UnableToInitializeCode, "Failed to load Wasm code");
+        return nullptr;
+      }
+      if (!wasm_handle->wasm()->initialize()) {
+        wasm_handle->wasm()->fail(FailState::UnableToInitializeCode,
+                                  "Failed to initialize Wasm code");
+        return nullptr;
+      }
+      (*base_wasms)[vm_key] = wasm_handle;
     }
-    (*base_wasms)[vm_key] = wasm_handle;
   }
 
-  if (!wasm_handle->wasm()->load(code, allow_precompiled)) {
-    wasm_handle->wasm()->fail(FailState::UnableToInitializeCode, "Failed to load Wasm code");
-    return nullptr;
-  }
-  if (!wasm_handle->wasm()->initialize()) {
-    wasm_handle->wasm()->fail(FailState::UnableToInitializeCode, "Failed to initialize Wasm code");
-    return nullptr;
-  }
-  auto configuration_canary_handle = clone_factory(wasm_handle);
-  if (!configuration_canary_handle) {
-    wasm_handle->wasm()->fail(FailState::UnableToCloneVm, "Failed to clone Base Wasm");
-    return nullptr;
-  }
-  if (!configuration_canary_handle->wasm()->initialize()) {
-    wasm_handle->wasm()->fail(FailState::UnableToInitializeCode, "Failed to initialize Wasm code");
-    return nullptr;
-  }
-  auto *root_context = configuration_canary_handle->wasm()->start(plugin);
-  if (root_context == nullptr) {
-    configuration_canary_handle->wasm()->fail(FailState::StartFailed, "Failed to start base Wasm");
+  // Either creating new one or reusing the existing one, apply canary for each plugin.
+  if (!wasm_handle->canary(plugin, clone_factory)) {
     return nullptr;
   }
-  if (!configuration_canary_handle->wasm()->configure(root_context, plugin)) {
-    configuration_canary_handle->wasm()->fail(FailState::ConfigureFailed,
-                                              "Failed to configure base Wasm plugin");
-    return nullptr;
-  }
-  configuration_canary_handle->kill();
   return wasm_handle;
 };
 

test/BUILD

@@ -91,6 +91,7 @@ cc_test(
     srcs = ["wasm_test.cc"],
     data = [
         "//test/test_data:abi_export.wasm",
+        "//test/test_data:canary_check.wasm",
     ],
     linkstatic = 1,
     deps = [

test/test_data/BUILD

@@ -1,4 +1,5 @@
 load("@proxy_wasm_cpp_host//bazel:wasm.bzl", "wasm_rust_binary")
+load("@proxy_wasm_cpp_sdk//bazel:defs.bzl", "proxy_wasm_cc_binary")
 
 licenses(["notice"])  # Apache 2
 
@@ -59,3 +60,8 @@ wasm_rust_binary(
     srcs = ["random.rs"],
     wasi = True,
 )
+
+proxy_wasm_cc_binary(
+    name = "canary_check.wasm",
+    srcs = ["canary_check.cc"],
+)

test/test_data/canary_check.cc

@@ -0,0 +1,52 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <string>
+#include <string_view>
+#include <unordered_map>
+
+#include "proxy_wasm_intrinsics.h"
+
+class CanaryCheckRootContext1 : public RootContext {
+public:
+  explicit CanaryCheckRootContext1(uint32_t id, std::string_view root_id)
+      : RootContext(id, root_id) {}
+  bool onConfigure(size_t s) override {
+    LOG_TRACE("onConfigure: root_id_1");
+    return s != 0;
+  }
+};
+
+class CanaryCheckContext : public Context {
+public:
+  explicit CanaryCheckContext(uint32_t id, RootContext *root) : Context(id, root) {}
+};
+
+class CanaryCheckRootContext2 : public RootContext {
+public:
+  explicit CanaryCheckRootContext2(uint32_t id, std::string_view root_id)
+      : RootContext(id, root_id) {}
+  bool onConfigure(size_t s) override {
+    LOG_TRACE("onConfigure: root_id_2");
+    return s != 0;
+  }
+};
+
+static RegisterContextFactory register_CanaryCheckContext1(CONTEXT_FACTORY(CanaryCheckContext),
+                                                           ROOT_FACTORY(CanaryCheckRootContext1),
+                                                           "root_id_1");
+
+static RegisterContextFactory register_CanaryCheckContext2(CONTEXT_FACTORY(CanaryCheckContext),
+                                                           ROOT_FACTORY(CanaryCheckRootContext2),
+                                                           "root_id_2");

test/utility.cc

@@ -16,6 +16,8 @@
 
 namespace proxy_wasm {
 
+std::string TestContext::global_log_;
+
 std::vector<std::string> getWasmEngines() {
   std::vector<std::string> engines = {
 #if defined(PROXY_WASM_HOST_ENGINE_V8)

test/utility.h

@@ -91,16 +91,34 @@ class TestIntegration : public WasmVmIntegration {
 class TestContext : public ContextBase {
 public:
   TestContext(WasmBase *wasm) : ContextBase(wasm) {}
+  TestContext(WasmBase *wasm, const std::shared_ptr<PluginBase> &plugin)
+      : ContextBase(wasm, plugin) {}
 
   WasmResult log(uint32_t /*log_level*/, std::string_view message) override {
-    log_ += std::string(message) + "\n";
+    auto new_log = std::string(message) + "\n";
+    log_ += new_log;
+    global_log_ += new_log;
     return WasmResult::Ok;
   }
 
+  WasmResult getProperty(std::string_view path, std::string *result) override {
+    if (path == "plugin_root_id") {
+      *result = root_id_;
+      return WasmResult::Ok;
+    }
+    return unimplemented();
+  }
+
   bool isLogEmpty() { return log_.empty(); }
 
   bool isLogged(std::string_view message) { return log_.find(message) != std::string::npos; }
 
+  static bool isGlobalLogged(std::string_view message) {
+    return global_log_.find(message) != std::string::npos;
+  }
+
+  static void resetGlobalLog() { global_log_ = ""; }
+
   uint64_t getCurrentTimeNanoseconds() override {
     return std::chrono::duration_cast<std::chrono::nanoseconds>(
                std::chrono::system_clock::now().time_since_epoch())
@@ -114,14 +132,24 @@ class TestContext : public ContextBase {
 
 private:
   std::string log_;
+  static std::string global_log_;
 };
 
 class TestWasm : public WasmBase {
 public:
-  TestWasm(std::unique_ptr<WasmVm> wasm_vm, std::unordered_map<std::string, std::string> envs = {})
-      : WasmBase(std::move(wasm_vm), "", "", "", std::move(envs), {}) {}
+  TestWasm(std::unique_ptr<WasmVm> wasm_vm, std::unordered_map<std::string, std::string> envs = {},
+           std::string_view vm_id = "", std::string_view vm_configuration = "",
+           std::string_view vm_key = "")
+      : WasmBase(std::move(wasm_vm), vm_id, vm_configuration, vm_key, std::move(envs), {}) {}
+
+  TestWasm(const std::shared_ptr<WasmHandleBase> &base_wasm_handle, const WasmVmFactory &factory)
+      : WasmBase(base_wasm_handle, factory) {}
 
   ContextBase *createVmContext() override { return new TestContext(this); };
+
+  ContextBase *createRootContext(const std::shared_ptr<PluginBase> &plugin) override {
+    return new TestContext(this, plugin);
+  }
 };
 
 class TestVm : public testing::TestWithParam<std::string> {