Skip to content

Configuration canary for the second or further plugins. #289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 26 commits into from
Jul 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1ddd4a4
Canary for the second plugins
ingwonsong Jun 12, 2022
44e4019
Fix lint errors
ingwonsong Jun 14, 2022
6a8becc
Add tests for secondary canary test
ingwonsong Jul 1, 2022
0527d4b
Add log checks for checking the root_id is working properly
ingwonsong Jul 1, 2022
c2f3676
Iterate all the combinations of the plugin parameters in tests
ingwonsong Jul 1, 2022
b361a81
Change the canary function to be the member function of WasmHandleBase
ingwonsong Jul 14, 2022
f131a26
Bump up the version of proxy-wasm-cpp-sdk, and fix the CI script
ingwonsong Jul 14, 2022
7089760
Reduce the constructors and fix lint errors
ingwonsong Jul 14, 2022
928635a
Add clean up step to remove unnecessary directories before upload the…
ingwonsong Jul 14, 2022
6660829
Introduce "Global Log" to get the logs happen when canarying
ingwonsong Jul 15, 2022
7b64da7
Remove unnecessary test factors to avoid timeout in s390x.
ingwonsong Jul 15, 2022
d1cb6b0
Kill the VMs in the test case
ingwonsong Jul 15, 2022
ba72bbc
kill wasm_handle if it is not using the same wasm vm
ingwonsong Jul 15, 2022
f89b1df
skip the test in s390x arch
ingwonsong Jul 15, 2022
d3f7de2
Use the same TestContext
ingwonsong Jul 18, 2022
edbe96f
Add the test case for the canary failure
ingwonsong Jul 18, 2022
7bcf22e
Reflect the comments
ingwonsong Jul 18, 2022
ff3d16a
Reflect the comments
ingwonsong Jul 22, 2022
f011993
Defend the case that WasmHandleBase::wasm() is null for `canary` func…
ingwonsong Jul 22, 2022
7e3af54
Add the comments for the reason why we deletes the directories in tes…
ingwonsong Jul 22, 2022
9006f57
Add timeout to Bazel, not github script
ingwonsong Jul 22, 2022
d10641e
Keep the wasm handler reference to reuse the wasm vm
ingwonsong Jul 23, 2022
25f6f58
Change the prefix "Test" to CanaryCheck
ingwonsong Jul 29, 2022
35a0685
Reflect the comments
ingwonsong Jul 29, 2022
d7abc4f
Add the omitted empty line between external and local includes
ingwonsong Jul 29, 2022
0bd8418
Remove trailing white spaces
ingwonsong Jul 29, 2022
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
14 changes: 13 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
Expand Down
12 changes: 12 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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()
6 changes: 3 additions & 3 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 7 additions & 4 deletions include/proxy-wasm/wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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_; }
Expand All @@ -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,
Expand Down
78 changes: 46 additions & 32 deletions src/wasm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
};

Expand Down
1 change: 1 addition & 0 deletions test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
6 changes: 6 additions & 0 deletions test/test_data/BUILD
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -59,3 +60,8 @@ wasm_rust_binary(
srcs = ["random.rs"],
wasi = True,
)

proxy_wasm_cc_binary(
name = "canary_check.wasm",
srcs = ["canary_check.cc"],
)
52 changes: 52 additions & 0 deletions test/test_data/canary_check.cc
Original file line number Diff line number Diff line change
@@ -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");
2 changes: 2 additions & 0 deletions test/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 31 additions & 3 deletions test/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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> {
Expand Down
Loading