Skip to content
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

wasm: move everything into common/ to match other extensions. #13

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion api/envoy/config/accesslog/v2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ api_proto_library_internal(
name = "wasm",
srcs = ["wasm.proto"],
deps = [
"//envoy/config/wasm/v2:wasm",
"//envoy/config/common/wasm/v2:wasm",
],
)
4 changes: 2 additions & 2 deletions api/envoy/config/accesslog/v2/wasm.proto
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ option go_package = "v2";

import "validate/validate.proto";
import "google/protobuf/struct.proto";
import "envoy/config/wasm/v2/wasm.proto";
import "envoy/config/common/wasm/v2/wasm.proto";

// [#protodoc-title: Wasm access log]

Expand All @@ -20,5 +20,5 @@ message WasmAccessLog {
// VM configure() call.
string configuration = 2;
// Configuration for starting a new VM to be associated with the given vm_id.
envoy.config.wasm.v2.VmConfig vm_config = 3;
envoy.config.common.wasm.v2.VmConfig vm_config = 3;
}
4 changes: 2 additions & 2 deletions api/envoy/config/bootstrap/v2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ api_proto_library_internal(
"//envoy/api/v2/core:address",
"//envoy/api/v2/core:base",
"//envoy/api/v2/core:config_source",
"//envoy/config/common/wasm/v2:wasm",
"//envoy/config/metrics/v2:metrics_service",
"//envoy/config/metrics/v2:stats",
"//envoy/config/overload/v2alpha:overload",
"//envoy/config/ratelimit/v2:rls",
"//envoy/config/trace/v2:trace",
"//envoy/config/wasm/v2:wasm",
],
)

Expand All @@ -32,11 +32,11 @@ api_go_proto_library(
"//envoy/api/v2/core:address_go_proto",
"//envoy/api/v2/core:base_go_proto",
"//envoy/api/v2/core:config_source_go_proto",
"//envoy/config/common/wasm/v2:wasm_go_proto",
"//envoy/config/metrics/v2:metrics_service_go_proto",
"//envoy/config/metrics/v2:stats_go_proto",
"//envoy/config/overload/v2alpha:overload_go_proto",
"//envoy/config/ratelimit/v2:rls_go_grpc",
"//envoy/config/trace/v2:trace_go_proto",
"//envoy/config/wasm/v2:wasm_go_proto",
],
)
6 changes: 3 additions & 3 deletions api/envoy/config/bootstrap/v2/bootstrap.proto
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ import "envoy/api/v2/auth/cert.proto";
import "envoy/api/v2/core/config_source.proto";
import "envoy/api/v2/cds.proto";
import "envoy/api/v2/lds.proto";
import "envoy/config/trace/v2/trace.proto";
import "envoy/config/common/wasm/v2/wasm.proto";
import "envoy/config/metrics/v2/stats.proto";
import "envoy/config/overload/v2alpha/overload.proto";
import "envoy/config/ratelimit/v2/rls.proto";
import "envoy/config/wasm/v2/wasm.proto";
import "envoy/config/trace/v2/trace.proto";

import "google/protobuf/duration.proto";

Expand Down Expand Up @@ -122,7 +122,7 @@ message Bootstrap {
envoy.config.overload.v2alpha.OverloadManager overload_manager = 15;

// Configuration for an wasm service provider(s).
repeated envoy.config.wasm.v2.WasmConfig wasm_service = 16;
repeated envoy.config.common.wasm.v2.WasmConfig wasm_service = 16;
}

// Administration interface :ref:`operations documentation
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
syntax = "proto3";

// [#protodoc-title: Metrics service]

package envoy.config.wasm.v2;
option java_package = "io.envoyproxy.envoy.config.wasm.v2";

import "envoy/api/v2/core/base.proto";

import "validate/validate.proto";

package envoy.config.common.wasm.v2;
option java_package = "io.envoyproxy.envoy.config.common.wasm.v2";

// [#protodoc-title: Common WASM configuration]

message VmConfig {
// The Wasm VM type (see source/extensions/commmon/wasm/well_known_names.h).
string vm = 1;
Expand Down
2 changes: 1 addition & 1 deletion api/envoy/config/filter/http/wasm/v2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ api_proto_library_internal(
name = "wasm",
srcs = ["wasm.proto"],
deps = [
"//envoy/config/wasm/v2:wasm",
"//envoy/config/common/wasm/v2:wasm",
],
)
4 changes: 2 additions & 2 deletions api/envoy/config/filter/http/wasm/v2/wasm.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package envoy.config.filter.http.wasm.v2;
option java_package = "io.envoyproxy.envoy.config.filter.http.wasm.v2";
option go_package = "v2";
import "validate/validate.proto";
import "envoy/config/wasm/v2/wasm.proto";
import "envoy/config/common/wasm/v2/wasm.proto";

// [#protodoc-title: Wasm]
// Wasm :ref:`configuration overview <config_http_filters_wasm>`.
Expand All @@ -13,7 +13,7 @@ message Wasm {
// A unique ID so that multiple filters/services can call into the same VM.
string id = 1;
// Configuration for starting a new VM (optionally associated with the given 'id').
envoy.config.wasm.v2.VmConfig vm_config = 2;
envoy.config.common.wasm.v2.VmConfig vm_config = 2;
// Wasm service configuration string e.g. a serialized protobuf which will be the
// argument to the VM configure() call.
string configuration = 3;
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/server/wasm_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class WasmFactory {
* @throw EnvoyException if the implementation is unable to produce an instance with
* the provided parameters.
*/
virtual WasmPtr createWasm(const envoy::config::wasm::v2::WasmConfig& config,
virtual WasmPtr createWasm(const envoy::config::common::wasm::v2::WasmConfig& config,
WasmFactoryContext& context) PURE;
};

Expand Down
16 changes: 16 additions & 0 deletions source/extensions/common/wasm/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,22 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "config",
srcs = ["config.cc"],
hdrs = ["config.h"],
deps = [
"//include/envoy/registry",
"//include/envoy/server:wasm_config_interface",
"//source/common/common:assert_lib",
"//source/common/common:empty_string",
"//source/common/config:datasource_lib",
"//source/common/protobuf:utility_lib",
"//source/extensions/common/wasm:wasm_lib",
"//source/extensions/grpc_credentials:well_known_names",
],
)

envoy_cc_library(
name = "wasm_hdr",
hdrs = ["wasm.h"],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include "extensions/wasm/config.h"
#include "extensions/common/wasm/config.h"

#include <stdio.h>

Expand All @@ -16,7 +16,7 @@ namespace Wasm {

static const std::string INLINE_STRING = "<inline>";

Server::WasmPtr WasmFactory::createWasm(const envoy::config::wasm::v2::WasmConfig& config,
Server::WasmPtr WasmFactory::createWasm(const envoy::config::common::wasm::v2::WasmConfig& config,
Server::Configuration::WasmFactoryContext& context) {
if (config.singleton()) {
auto wasm = Common::Wasm::createWasm(config.id(), config.vm_config(), context.api());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

#include "envoy/config/wasm/v2/wasm.pb.validate.h"
#include "envoy/config/common/wasm/v2/wasm.pb.validate.h"
#include "envoy/server/wasm_config.h"

#include "extensions/common/wasm/wasm.h"
Expand All @@ -13,7 +13,7 @@ class WasmFactory : public Server::Configuration::WasmFactory {
public:
~WasmFactory() override {}
std::string name() override { return "envoy.wasm"; }
Server::WasmPtr createWasm(const envoy::config::wasm::v2::WasmConfig& config,
Server::WasmPtr createWasm(const envoy::config::common::wasm::v2::WasmConfig& config,
Server::Configuration::WasmFactoryContext& context) override;
};

Expand Down
12 changes: 6 additions & 6 deletions source/extensions/common/wasm/wasm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include <vector>

#include "envoy/common/exception.h"
#include "envoy/config/wasm/v2/wasm.pb.validate.h"
#include "envoy/config/common/wasm/v2/wasm.pb.validate.h"
#include "envoy/http/codes.h"
#include "envoy/server/wasm.h"
#include "envoy/thread_local/thread_local.h"
Expand Down Expand Up @@ -1191,7 +1191,7 @@ std::unique_ptr<WasmVm> createWasmVm(absl::string_view wasm_vm) {
}

std::unique_ptr<Wasm> createWasm(absl::string_view id,
const envoy::config::wasm::v2::VmConfig& vm_config,
const envoy::config::common::wasm::v2::VmConfig& vm_config,
Api::Api& api) {
auto wasm = std::make_unique<Wasm>(vm_config.vm(), id);
const auto& code = Config::DataSource::read(vm_config.code(), true, api);
Expand All @@ -1207,10 +1207,10 @@ std::unique_ptr<Wasm> createWasm(absl::string_view id,
return wasm;
}

std::shared_ptr<Wasm> createThreadLocalWasm(Wasm& base_wasm,
const envoy::config::wasm::v2::VmConfig& vm_config,
Event::Dispatcher& dispatcher,
absl::string_view configuration, Api::Api& api) {
std::shared_ptr<Wasm>
createThreadLocalWasm(Wasm& base_wasm, const envoy::config::common::wasm::v2::VmConfig& vm_config,
Event::Dispatcher& dispatcher, absl::string_view configuration,
Api::Api& api) {
std::shared_ptr<Wasm> wasm;
if (base_wasm.wasmVm()->clonable()) {
wasm = std::make_shared<Wasm>(base_wasm);
Expand Down
13 changes: 7 additions & 6 deletions source/extensions/common/wasm/wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "envoy/access_log/access_log.h"
#include "envoy/buffer/buffer.h"
#include "envoy/common/exception.h"
#include "envoy/config/wasm/v2/wasm.pb.validate.h"
#include "envoy/config/common/wasm/v2/wasm.pb.validate.h"
#include "envoy/http/filter.h"
#include "envoy/server/wasm.h"
#include "envoy/thread_local/thread_local.h"
Expand Down Expand Up @@ -434,12 +434,13 @@ std::unique_ptr<WasmVm> createWasmVm(absl::string_view vm);
// Create a new Wasm VM not attached to any thread. Note: 'id' may be empty if this VM will not be
// shared by APIs (e.g. HTTP Filter + AccessLog).
std::unique_ptr<Wasm> createWasm(absl::string_view id,
const envoy::config::wasm::v2::VmConfig& vm_config, Api::Api& api);
const envoy::config::common::wasm::v2::VmConfig& vm_config,
Api::Api& api);
// Create a ThreadLocal VM from an existing VM (e.g. from createWasm() above).
std::shared_ptr<Wasm> createThreadLocalWasm(Wasm& base_wasm,
const envoy::config::wasm::v2::VmConfig& vm_config,
Event::Dispatcher& dispatcher,
absl::string_view configuration, Api::Api& api);
std::shared_ptr<Wasm>
createThreadLocalWasm(Wasm& base_wasm, const envoy::config::common::wasm::v2::VmConfig& vm_config,
Event::Dispatcher& dispatcher, absl::string_view configuration,
Api::Api& api);
// Get an existing ThreadLocal VM matching 'id'.
std::shared_ptr<Wasm> getThreadLocalWasm(absl::string_view id, absl::string_view configuration);

Expand Down
2 changes: 1 addition & 1 deletion source/extensions/common/wasm/wavm/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ envoy_cc_library(
"//source/common/protobuf",
"//source/extensions/common/wasm:wasm_hdr",
"//source/extensions/common/wasm:well_known_names",
"@envoy_api//envoy/config/wasm/v2:wasm_cc",
"@envoy_api//envoy/config/common/wasm/v2:wasm_cc",
],
)
2 changes: 1 addition & 1 deletion source/extensions/common/wasm/wavm/wavm.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include <vector>

#include "envoy/common/exception.h"
#include "envoy/config/wasm/v2/wasm.pb.validate.h"
#include "envoy/config/common/wasm/v2/wasm.pb.validate.h"
#include "envoy/server/wasm.h"
#include "envoy/thread_local/thread_local.h"

Expand Down
5 changes: 0 additions & 5 deletions source/extensions/extensions_build_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ EXTENSIONS = {

"envoy.grpc_credentials.file_based_metadata": "//source/extensions/grpc_credentials/file_based_metadata:config",

#
# WASM
#
"envoy.wasm": "//source/extensions/wasm:config",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jplevyak it seems that this isn't necessary, since we're including each extension separately.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extensions/wasm is independent of extensions/filter/http/wasm but they both depend on extensions/common/wasm. And you can include a general service which just handles onsStart/onTIck by just using extensions/wasm.

I don't understand why you are moving extensions/wasm/config -> extensions/wasm/common/config
as the former is for creating a general WASM service while the latter is the common library used by all
WASM things, i.e. AccessLog Filter::Http as well as the WASM service created in server.cc.

Let's chat tomorrow.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the VmConfig should be on config/common because it is the shared bit among all the services and filters and the WasmConfig bit from config/wasm.proto should stay where it is as it is specificallyi the service config.


#
# Health checkers
#
Expand Down
27 changes: 0 additions & 27 deletions source/extensions/wasm/BUILD

This file was deleted.

2 changes: 1 addition & 1 deletion source/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ envoy_cc_library(
"//source/extensions/filters/common/ratelimit:ratelimit_registration_lib",
"@envoy_api//envoy/api/v2:lds_cc",
"@envoy_api//envoy/config/bootstrap/v2:bootstrap_cc",
"@envoy_api//envoy/config/wasm/v2:wasm_cc",
"@envoy_api//envoy/config/common/wasm/v2:wasm_cc",
],
)

Expand Down
2 changes: 1 addition & 1 deletion source/server/configuration_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include <utility>

#include "envoy/config/bootstrap/v2/bootstrap.pb.h"
#include "envoy/config/wasm/v2/wasm.pb.h"
#include "envoy/config/common/wasm/v2/wasm.pb.h"
#include "envoy/http/filter.h"
#include "envoy/network/filter.h"
#include "envoy/server/configuration.h"
Expand Down
21 changes: 8 additions & 13 deletions test/extensions/wasm/BUILD → test/extensions/common/wasm/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,50 +2,45 @@ licenses(["notice"]) # Apache 2

load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_test",
"envoy_package",
)
load(
"//test/extensions:extensions_build_system.bzl",
"envoy_extension_cc_test",
)

envoy_package()

envoy_extension_cc_test(
envoy_cc_test(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the differences between envoy_cc_test and envoy_extension_cc_test?

name = "wasm_test",
srcs = ["wasm_test.cc"],
data = [
"//test/extensions/wasm/test_data:modules",
"//test/extensions/common/wasm/test_data:modules",
],
extension_name = "envoy.wasm",
external_deps = ["abseil_optional"],
deps = [
"//source/common/event:dispatcher_lib",
"//source/common/stats:isolated_store_lib",
"//source/common/stats:stats_lib",
"//source/extensions/common/wasm:config",
"//source/extensions/common/wasm:wasm_lib",
"//source/extensions/wasm:config",
"//test/test_common:environment_lib",
"//test/test_common:simulated_time_system_lib",
],
)

envoy_extension_cc_test(
envoy_cc_test(
name = "config_test",
srcs = ["config_test.cc"],
data = [
"//test/extensions/wasm/test_data:modules",
"//test/extensions/common/wasm/test_data:modules",
],
extension_name = "envoy.wasm",
deps = [
"//include/envoy/registry",
"//source/common/stats:isolated_store_lib",
"//source/extensions/common/wasm:config",
"//source/extensions/common/wasm:wasm_lib",
"//source/extensions/wasm:config",
"//source/server:wasm_config_lib",
"//test/mocks/event:event_mocks",
"//test/mocks/thread_local:thread_local_mocks",
"//test/test_common:environment_lib",
"@envoy_api//envoy/config/wasm/v2:wasm_cc",
"@envoy_api//envoy/config/common/wasm/v2:wasm_cc",
],
)
Loading