From f8649f8b53afb95f0e5cc86ce34d13353c69bdc5 Mon Sep 17 00:00:00 2001 From: Martijn Stevenson Date: Sun, 30 Apr 2023 18:12:36 +0000 Subject: [PATCH 1/7] Allow StreamContext testing via TestContext Signed-off-by: Martijn Stevenson --- test/utility.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/utility.h b/test/utility.h index 06114e2c..b8ca4e92 100644 --- a/test/utility.h +++ b/test/utility.h @@ -93,6 +93,9 @@ class TestContext : public ContextBase { TestContext(WasmBase *wasm) : ContextBase(wasm) {} TestContext(WasmBase *wasm, const std::shared_ptr &plugin) : ContextBase(wasm, plugin) {} + TestContext(WasmBase *wasm, uint32_t parent_context_id, + std::shared_ptr &plugin_handle) + : ContextBase(wasm, parent_context_id, plugin_handle) {} WasmResult log(uint32_t /*log_level*/, std::string_view message) override { auto new_log = std::string(message) + "\n"; From 4684af1b3d8184277005f745f2960fafe8942d87 Mon Sep 17 00:00:00 2001 From: Martijn Stevenson Date: Mon, 1 May 2023 01:40:31 +0000 Subject: [PATCH 2/7] Test an example Stream Context. HTTP handlers, logging only, no headers. Signed-off-by: Martijn Stevenson --- test/BUILD | 15 +++++++ test/logging_test.cc | 71 ++++++++++++++++++++++++++++++++++ test/test_data/BUILD | 5 +++ test/test_data/http_logging.cc | 25 ++++++++++++ 4 files changed, 116 insertions(+) create mode 100644 test/logging_test.cc create mode 100644 test/test_data/http_logging.cc diff --git a/test/BUILD b/test/BUILD index a922bbfb..61973ce1 100644 --- a/test/BUILD +++ b/test/BUILD @@ -117,6 +117,21 @@ cc_test( ], ) +cc_test( + name = "logging_test", + srcs = ["logging_test.cc"], + data = [ + "//test/test_data:http_logging.wasm", + ], + linkstatic = 1, + deps = [ + ":utility_lib", + "//:lib", + "@com_google_googletest//:gtest", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "security_test", srcs = ["security_test.cc"], diff --git a/test/logging_test.cc b/test/logging_test.cc new file mode 100644 index 00000000..d31008fe --- /dev/null +++ b/test/logging_test.cc @@ -0,0 +1,71 @@ +// Copyright 2023 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 "include/proxy-wasm/wasm.h" +#include "gtest/gtest.h" +#include +#include "test/utility.h" + +namespace proxy_wasm { + +INSTANTIATE_TEST_SUITE_P(WasmEngines, TestVm, testing::ValuesIn(getWasmEngines()), + [](const testing::TestParamInfo &info) { + return info.param; + }); + +// TestVm is parameterized for each engine and creates a VM on construction. +TEST_P(TestVm, HttpLogging) { + // Read the wasm source. + auto source = readTestWasmFile("http_logging.wasm"); + ASSERT_FALSE(source.empty()); + + // Create a WasmBase and load the plugin. + auto wasm = std::make_shared(std::move(vm_)); + ASSERT_TRUE(wasm->load(source, /*allow_precompiled=*/false)); + ASSERT_TRUE(wasm->initialize()); + + // Create a plugin. + const auto plugin = std::make_shared( + /*name=*/"test", /*root_id=*/"", /*vm_id=*/"", + /*engine=*/wasm->wasm_vm()->getEngineName(), /*plugin_config=*/"", + /*fail_open=*/false, /*key=*/""); + + // Create root context, call onStart(). + ContextBase *root_context = wasm->start(plugin); + ASSERT_TRUE(root_context != nullptr); + + // On the root context, call onConfigure(). + ASSERT_TRUE(wasm->configure(root_context, plugin)); + + // Create a stream context. + { + auto wasm_handle = std::make_shared(wasm); + auto plugin_handle = std::make_shared(wasm_handle, plugin); + auto stream_context = + std::make_unique(wasm.get(), root_context->id(), plugin_handle); + stream_context->onCreate(); + EXPECT_TRUE(stream_context->isLogged("onCreate called")); + stream_context->onRequestHeaders(/*headers=*/0, /*end_of_stream=*/false); + EXPECT_TRUE(stream_context->isLogged("onRequestHeaders called")); + stream_context->onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false); + EXPECT_TRUE(stream_context->isLogged("onResponseHeaders called")); + stream_context->onDone(); + EXPECT_TRUE(stream_context->isLogged("onDone called")); + stream_context->onDelete(); + EXPECT_TRUE(stream_context->isLogged("onDelete called")); + } + EXPECT_FALSE(wasm->isFailed()); +} + +} // namespace proxy_wasm diff --git a/test/test_data/BUILD b/test/test_data/BUILD index d730b4bf..bd70b8eb 100644 --- a/test/test_data/BUILD +++ b/test/test_data/BUILD @@ -84,3 +84,8 @@ proxy_wasm_cc_binary( name = "canary_check.wasm", srcs = ["canary_check.cc"], ) + +proxy_wasm_cc_binary( + name = "http_logging.wasm", + srcs = ["http_logging.cc"], +) diff --git a/test/test_data/http_logging.cc b/test/test_data/http_logging.cc new file mode 100644 index 00000000..31337a2b --- /dev/null +++ b/test/test_data/http_logging.cc @@ -0,0 +1,25 @@ +#include "proxy_wasm_intrinsics.h" + +class LoggingStreamContext : public Context { +public: + explicit LoggingStreamContext(uint32_t id, RootContext *root) : Context(id, root) {} + + void onCreate() override { LOG_INFO("onCreate called"); } + void onDelete() override { LOG_INFO("onDelete called"); } + void onDone() override { LOG_INFO("onDone called"); } + + FilterHeadersStatus onRequestHeaders(uint32_t headers, bool end_of_stream) override { + LOG_INFO("onRequestHeaders called"); + // A real host would implement header getters/setters to be called here. + return FilterHeadersStatus::Continue; + } + + FilterHeadersStatus onResponseHeaders(uint32_t headers, bool end_of_stream) override { + LOG_INFO("onResponseHeaders called"); + // A real host would implement header getters/setters to be called here. + return FilterHeadersStatus::Continue; + } +}; + +static RegisterContextFactory register_StaticContext(CONTEXT_FACTORY(LoggingStreamContext), + ROOT_FACTORY(RootContext)); From bee5be38d7a1eb4d0496cd7483c8b2ae7ab52919 Mon Sep 17 00:00:00 2001 From: Martijn Stevenson Date: Mon, 1 May 2023 01:55:17 +0000 Subject: [PATCH 3/7] Add missing license header. Signed-off-by: Martijn Stevenson --- test/test_data/http_logging.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/test_data/http_logging.cc b/test/test_data/http_logging.cc index 31337a2b..9af38501 100644 --- a/test/test_data/http_logging.cc +++ b/test/test_data/http_logging.cc @@ -1,3 +1,17 @@ +// Copyright 2023 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 "proxy_wasm_intrinsics.h" class LoggingStreamContext : public Context { From ae11dd58a3a009f4461a1ec53f82c61e73db1d86 Mon Sep 17 00:00:00 2001 From: Martijn Stevenson Date: Mon, 1 May 2023 03:40:01 +0000 Subject: [PATCH 4/7] Attempt to fix clang-tidy Signed-off-by: Martijn Stevenson --- test/logging_test.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/logging_test.cc b/test/logging_test.cc index d31008fe..8cf9dc85 100644 --- a/test/logging_test.cc +++ b/test/logging_test.cc @@ -12,9 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "include/proxy-wasm/wasm.h" #include "gtest/gtest.h" -#include +#include "include/proxy-wasm/wasm.h" #include "test/utility.h" namespace proxy_wasm { From d147041aa7423048f95886382d3a5f508f36972c Mon Sep 17 00:00:00 2001 From: Martijn Stevenson Date: Mon, 1 May 2023 05:07:11 +0000 Subject: [PATCH 5/7] Make newVm a static method for broader utility. Signed-off-by: Martijn Stevenson --- test/utility.h | 20 ++++++++++---------- test/wasm_test.cc | 17 +++++++++-------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/test/utility.h b/test/utility.h index b8ca4e92..3ffb611a 100644 --- a/test/utility.h +++ b/test/utility.h @@ -159,39 +159,39 @@ class TestVm : public testing::TestWithParam { public: TestVm() { engine_ = GetParam(); - vm_ = newVm(); + vm_ = MakeVm(engine_); } - std::unique_ptr newVm() { + static std::unique_ptr MakeVm(const std::string &engine) { std::unique_ptr vm; - if (engine_.empty()) { + if (engine.empty()) { ADD_FAILURE() << "engine must not be empty"; #if defined(PROXY_WASM_HOST_ENGINE_V8) - } else if (engine_ == "v8") { + } else if (engine == "v8") { vm = proxy_wasm::createV8Vm(); #endif #if defined(PROXY_WASM_HOST_ENGINE_WAVM) - } else if (engine_ == "wavm") { + } else if (engine == "wavm") { vm = proxy_wasm::createWavmVm(); #endif #if defined(PROXY_WASM_HOST_ENGINE_WASMTIME) - } else if (engine_ == "wasmtime") { + } else if (engine == "wasmtime") { vm = proxy_wasm::createWasmtimeVm(); #endif #if defined(PROXY_WASM_HOST_ENGINE_WASMEDGE) - } else if (engine_ == "wasmedge") { + } else if (engine == "wasmedge") { vm = proxy_wasm::createWasmEdgeVm(); #endif #if defined(PROXY_WASM_HOST_ENGINE_WAMR) - } else if (engine_ == "wamr") { + } else if (engine == "wamr") { vm = proxy_wasm::createWamrVm(); #endif } else { - ADD_FAILURE() << "compiled without support for the requested \"" << engine_ << "\" engine"; + ADD_FAILURE() << "compiled without support for the requested \"" << engine << "\" engine"; } vm->integration() = std::make_unique(); return vm; - }; + } std::unique_ptr vm_; std::string engine_; diff --git a/test/wasm_test.cc b/test/wasm_test.cc index 387348e8..43f3924c 100644 --- a/test/wasm_test.cc +++ b/test/wasm_test.cc @@ -43,7 +43,7 @@ TEST_P(TestVm, GetOrCreateThreadLocalWasmFailCallbacks) { // Define callbacks. WasmHandleFactory wasm_handle_factory = [this, vm_id, vm_config](std::string_view vm_key) -> std::shared_ptr { - auto base_wasm = std::make_shared(newVm(), vm_id, vm_config, vm_key, + auto base_wasm = std::make_shared(MakeVm(engine_), vm_id, vm_config, vm_key, std::unordered_map{}, AllowedCapabilitiesMap{}); return std::make_shared(base_wasm); @@ -52,8 +52,8 @@ TEST_P(TestVm, GetOrCreateThreadLocalWasmFailCallbacks) { WasmHandleCloneFactory wasm_handle_clone_factory = [this](const std::shared_ptr &base_wasm_handle) -> std::shared_ptr { - auto wasm = std::make_shared(base_wasm_handle, - [this]() -> std::unique_ptr { return newVm(); }); + auto wasm = std::make_shared( + base_wasm_handle, [this]() -> std::unique_ptr { return MakeVm(engine_); }); return std::make_shared(wasm); }; @@ -133,8 +133,8 @@ TEST_P(TestVm, AlwaysApplyCanary) { WasmHandleCloneFactory wasm_handle_clone_factory_for_canary = [&canary_count, this](const std::shared_ptr &base_wasm_handle) -> std::shared_ptr { - auto wasm = std::make_shared(base_wasm_handle, - [this]() -> std::unique_ptr { return newVm(); }); + auto wasm = std::make_shared( + base_wasm_handle, [this]() -> std::unique_ptr { return MakeVm(engine_); }); canary_count++; return std::make_shared(wasm); }; @@ -150,8 +150,9 @@ TEST_P(TestVm, AlwaysApplyCanary) { WasmHandleFactory wasm_handle_factory_baseline = [this, vm_ids, vm_configs](std::string_view vm_key) -> std::shared_ptr { - auto base_wasm = std::make_shared( - newVm(), std::unordered_map(), vm_ids[0], vm_configs[0], vm_key); + auto base_wasm = + std::make_shared(MakeVm(engine_), std::unordered_map(), + vm_ids[0], vm_configs[0], vm_key); return std::make_shared(base_wasm); }; @@ -184,7 +185,7 @@ TEST_P(TestVm, AlwaysApplyCanary) { [this, vm_id, vm_config](std::string_view vm_key) -> std::shared_ptr { auto base_wasm = std::make_shared( - newVm(), std::unordered_map(), vm_id, vm_config, + MakeVm(engine_), std::unordered_map(), vm_id, vm_config, vm_key); return std::make_shared(base_wasm); }; From e22cc9e61cf545878b5d6d0182857ea40b8f5faa Mon Sep 17 00:00:00 2001 From: Martijn Stevenson Date: Fri, 5 May 2023 17:42:21 +0000 Subject: [PATCH 6/7] Code review cleanups. Signed-off-by: Martijn Stevenson --- test/test_data/http_logging.cc | 8 +++----- test/utility.h | 4 ++-- test/wasm_test.cc | 10 +++++----- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/test/test_data/http_logging.cc b/test/test_data/http_logging.cc index 9af38501..e25b410c 100644 --- a/test/test_data/http_logging.cc +++ b/test/test_data/http_logging.cc @@ -14,9 +14,9 @@ #include "proxy_wasm_intrinsics.h" -class LoggingStreamContext : public Context { +class LoggingContext : public Context { public: - explicit LoggingStreamContext(uint32_t id, RootContext *root) : Context(id, root) {} + explicit LoggingContext(uint32_t id, RootContext *root) : Context(id, root) {} void onCreate() override { LOG_INFO("onCreate called"); } void onDelete() override { LOG_INFO("onDelete called"); } @@ -24,16 +24,14 @@ class LoggingStreamContext : public Context { FilterHeadersStatus onRequestHeaders(uint32_t headers, bool end_of_stream) override { LOG_INFO("onRequestHeaders called"); - // A real host would implement header getters/setters to be called here. return FilterHeadersStatus::Continue; } FilterHeadersStatus onResponseHeaders(uint32_t headers, bool end_of_stream) override { LOG_INFO("onResponseHeaders called"); - // A real host would implement header getters/setters to be called here. return FilterHeadersStatus::Continue; } }; -static RegisterContextFactory register_StaticContext(CONTEXT_FACTORY(LoggingStreamContext), +static RegisterContextFactory register_StaticContext(CONTEXT_FACTORY(LoggingContext), ROOT_FACTORY(RootContext)); diff --git a/test/utility.h b/test/utility.h index 3ffb611a..27b3b049 100644 --- a/test/utility.h +++ b/test/utility.h @@ -159,10 +159,10 @@ class TestVm : public testing::TestWithParam { public: TestVm() { engine_ = GetParam(); - vm_ = MakeVm(engine_); + vm_ = makeVm(engine_); } - static std::unique_ptr MakeVm(const std::string &engine) { + static std::unique_ptr makeVm(const std::string &engine) { std::unique_ptr vm; if (engine.empty()) { ADD_FAILURE() << "engine must not be empty"; diff --git a/test/wasm_test.cc b/test/wasm_test.cc index 43f3924c..ad0e16ac 100644 --- a/test/wasm_test.cc +++ b/test/wasm_test.cc @@ -43,7 +43,7 @@ TEST_P(TestVm, GetOrCreateThreadLocalWasmFailCallbacks) { // Define callbacks. WasmHandleFactory wasm_handle_factory = [this, vm_id, vm_config](std::string_view vm_key) -> std::shared_ptr { - auto base_wasm = std::make_shared(MakeVm(engine_), vm_id, vm_config, vm_key, + auto base_wasm = std::make_shared(makeVm(engine_), vm_id, vm_config, vm_key, std::unordered_map{}, AllowedCapabilitiesMap{}); return std::make_shared(base_wasm); @@ -53,7 +53,7 @@ TEST_P(TestVm, GetOrCreateThreadLocalWasmFailCallbacks) { [this](const std::shared_ptr &base_wasm_handle) -> std::shared_ptr { auto wasm = std::make_shared( - base_wasm_handle, [this]() -> std::unique_ptr { return MakeVm(engine_); }); + base_wasm_handle, [this]() -> std::unique_ptr { return makeVm(engine_); }); return std::make_shared(wasm); }; @@ -134,7 +134,7 @@ TEST_P(TestVm, AlwaysApplyCanary) { [&canary_count, this](const std::shared_ptr &base_wasm_handle) -> std::shared_ptr { auto wasm = std::make_shared( - base_wasm_handle, [this]() -> std::unique_ptr { return MakeVm(engine_); }); + base_wasm_handle, [this]() -> std::unique_ptr { return makeVm(engine_); }); canary_count++; return std::make_shared(wasm); }; @@ -151,7 +151,7 @@ TEST_P(TestVm, AlwaysApplyCanary) { WasmHandleFactory wasm_handle_factory_baseline = [this, vm_ids, vm_configs](std::string_view vm_key) -> std::shared_ptr { auto base_wasm = - std::make_shared(MakeVm(engine_), std::unordered_map(), + std::make_shared(makeVm(engine_), std::unordered_map(), vm_ids[0], vm_configs[0], vm_key); return std::make_shared(base_wasm); }; @@ -185,7 +185,7 @@ TEST_P(TestVm, AlwaysApplyCanary) { [this, vm_id, vm_config](std::string_view vm_key) -> std::shared_ptr { auto base_wasm = std::make_shared( - MakeVm(engine_), std::unordered_map(), vm_id, vm_config, + makeVm(engine_), std::unordered_map(), vm_id, vm_config, vm_key); return std::make_shared(base_wasm); }; From 42ec22e8953efb92e84f91a1fdc6955f4d01ace8 Mon Sep 17 00:00:00 2001 From: Martijn Stevenson Date: Mon, 8 May 2023 19:51:57 +0000 Subject: [PATCH 7/7] Remove unnecessary pointer / heap allocation Signed-off-by: Martijn Stevenson --- test/logging_test.cc | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/test/logging_test.cc b/test/logging_test.cc index 8cf9dc85..e4cbdcdc 100644 --- a/test/logging_test.cc +++ b/test/logging_test.cc @@ -51,18 +51,17 @@ TEST_P(TestVm, HttpLogging) { { auto wasm_handle = std::make_shared(wasm); auto plugin_handle = std::make_shared(wasm_handle, plugin); - auto stream_context = - std::make_unique(wasm.get(), root_context->id(), plugin_handle); - stream_context->onCreate(); - EXPECT_TRUE(stream_context->isLogged("onCreate called")); - stream_context->onRequestHeaders(/*headers=*/0, /*end_of_stream=*/false); - EXPECT_TRUE(stream_context->isLogged("onRequestHeaders called")); - stream_context->onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false); - EXPECT_TRUE(stream_context->isLogged("onResponseHeaders called")); - stream_context->onDone(); - EXPECT_TRUE(stream_context->isLogged("onDone called")); - stream_context->onDelete(); - EXPECT_TRUE(stream_context->isLogged("onDelete called")); + auto stream_context = TestContext(wasm.get(), root_context->id(), plugin_handle); + stream_context.onCreate(); + EXPECT_TRUE(stream_context.isLogged("onCreate called")); + stream_context.onRequestHeaders(/*headers=*/0, /*end_of_stream=*/false); + EXPECT_TRUE(stream_context.isLogged("onRequestHeaders called")); + stream_context.onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false); + EXPECT_TRUE(stream_context.isLogged("onResponseHeaders called")); + stream_context.onDone(); + EXPECT_TRUE(stream_context.isLogged("onDone called")); + stream_context.onDelete(); + EXPECT_TRUE(stream_context.isLogged("onDelete called")); } EXPECT_FALSE(wasm->isFailed()); }