From 49bac68ae5be6b2417f7c908c1d4427f81cba4cc Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Thu, 26 Oct 2017 16:51:25 -0400 Subject: [PATCH 1/9] first cut Signed-off-by: Shriram Rajagopalan --- source/common/config/filter_json.cc | 17 +++++++++++ source/common/config/filter_json.h | 12 ++++++++ source/common/mongo/BUILD | 1 + source/common/mongo/proxy.h | 13 ++++---- source/server/config/network/BUILD | 1 - source/server/config/network/mongo_proxy.cc | 33 +++++++++++++++------ source/server/config/network/mongo_proxy.h | 2 ++ 7 files changed, 62 insertions(+), 17 deletions(-) diff --git a/source/common/config/filter_json.cc b/source/common/config/filter_json.cc index 993dc617fa69c..70152a77797d7 100644 --- a/source/common/config/filter_json.cc +++ b/source/common/config/filter_json.cc @@ -214,5 +214,22 @@ void FilterJson::translateHttpConnectionManager( } } +void FilterJson::translateMongoProxy( + const Json::Object& json_mongo_proxy, + envoy::api::v2::filter::network::MongoProxy& mongo_proxy) { + json_mongo_proxy.validateSchema(Json::Schema::MONGO_PROXY_NETWORK_FILTER_SCHEMA); + + JSON_UTIL_SET_STRING(json_mongo_proxy, mongo_proxy, stat_prefix); + JSON_UTIL_SET_STRING(json_mongo_proxy, mongo_proxy, access_log); + if (json_mongo_proxy.hasObject("fault")) { + const auto json_fault = json_mongo_proxy.getObject("fault")->getObject("fixed_delay"); + auto* delay = mongo_proxy.mutable_delay(); + + delay->set_type(envoy::api::v2::filter::FaultDelay::FaultDelayType::FIXED); + delay->set_percent(static_cast(json_fault->getInteger("percent"))); + delay->mutable_fixed_delay()->CopyFrom(Protobuf::util::TimeUtil::MillisecondsToDuration(json_fault->getInteger("duration_ms"))); + } +} + } // namespace Config } // namespace Envoy diff --git a/source/common/config/filter_json.h b/source/common/config/filter_json.h index 4ca780e92fc60..2b48d7bca27b9 100644 --- a/source/common/config/filter_json.h +++ b/source/common/config/filter_json.h @@ -3,6 +3,7 @@ #include "envoy/json/json_object.h" #include "api/filter/http/http_connection_manager.pb.h" +#include "api/filter/network/mongo_proxy.pb.h" namespace Envoy { namespace Config { @@ -37,6 +38,17 @@ class FilterJson { static void translateHttpConnectionManager( const Json::Object& json_http_connection_manager, envoy::api::v2::filter::http::HttpConnectionManager& http_connection_manager); + + /** + * Translate a v1 JSON Mongo proxy object to v2 + * envoy::api::v2::filter::network::MongoProxy. + * @param json_mongo_proxy source v1 JSON HTTP connection manager object. + * @param mongo_proxy destination v2 + * envoy::api::v2::filter::network::MongoProxy. + */ + static void translateMongoProxy( + const Json::Object& json_mongo_proxy, + envoy::api::v2::filter::network::MongoProxy& mongo_proxy); }; } // namespace Config diff --git a/source/common/mongo/BUILD b/source/common/mongo/BUILD index f99889edf0825..c08dfab2d2904 100644 --- a/source/common/mongo/BUILD +++ b/source/common/mongo/BUILD @@ -58,6 +58,7 @@ envoy_cc_library( "//source/common/common:singleton", "//source/common/common:utility_lib", "//source/common/network:filter_lib", + "//source/common/protobuf:utility_lib", ], ) diff --git a/source/common/mongo/proxy.h b/source/common/mongo/proxy.h index 0922fe9d81615..ea499b5053931 100644 --- a/source/common/mongo/proxy.h +++ b/source/common/mongo/proxy.h @@ -19,9 +19,11 @@ #include "common/buffer/buffer_impl.h" #include "common/common/logger.h" #include "common/common/singleton.h" -#include "common/json/json_loader.h" #include "common/mongo/utility.h" #include "common/network/filter_impl.h" +#Include "common/protobuf/utility.h" + +#include "api/filter/network/mongo_proxy.pb.h" namespace Envoy { namespace Mongo { @@ -92,12 +94,9 @@ typedef std::shared_ptr AccessLogSharedPtr; */ class FaultConfig { public: - FaultConfig(const Json::Object& fault_config) - : delay_percent_( - static_cast(fault_config.getObject("fixed_delay")->getInteger("percent"))), - duration_ms_(static_cast( - fault_config.getObject("fixed_delay")->getInteger("duration_ms"))) {} - + FaultConfig(const envoy::api::v2::filter::FaultDelay& fault_config) + : delay_percent_(fault_config.percent()), + duration_ms_(PROTOBUF_GET_MS_REQUIRED(fault_config, fixed_delay)) {} uint32_t delayPercent() const { return delay_percent_; } uint64_t delayDuration() const { return duration_ms_; } diff --git a/source/server/config/network/BUILD b/source/server/config/network/BUILD index 5b6413bb8117e..a79f82edbd2b3 100644 --- a/source/server/config/network/BUILD +++ b/source/server/config/network/BUILD @@ -67,7 +67,6 @@ envoy_cc_library( "//include/envoy/registry", "//include/envoy/server:filter_config_interface", "//source/common/config:well_known_names", - "//source/common/json:config_schemas_lib", "//source/common/json:json_loader_lib", "//source/common/mongo:proxy_lib", ], diff --git a/source/server/config/network/mongo_proxy.cc b/source/server/config/network/mongo_proxy.cc index fbbfe63966061..fe9bc7e371df9 100644 --- a/source/server/config/network/mongo_proxy.cc +++ b/source/server/config/network/mongo_proxy.cc @@ -5,28 +5,28 @@ #include "envoy/network/connection.h" #include "envoy/registry/registry.h" -#include "common/json/config_schemas.h" +#include "common/json/json_loader.h" #include "common/mongo/proxy.h" +#include "api/filter/network/mongo_proxy.pb.h" + namespace Envoy { namespace Server { namespace Configuration { NetworkFilterFactoryCb -MongoProxyFilterConfigFactory::createFilterFactory(const Json::Object& config, +MongoProxyFilterConfigFactory::createMongoProxyFactory(envoy::api::v2::filter::MongoProxy& mongo_proxy, FactoryContext& context) { - config.validateSchema(Json::Schema::MONGO_PROXY_NETWORK_FILTER_SCHEMA); - - std::string stat_prefix = "mongo." + config.getString("stat_prefix") + "."; + std::string stat_prefix = "mongo." + mongo_proxy.stat_prefix() + "."; Mongo::AccessLogSharedPtr access_log; - if (config.hasObject("access_log")) { + if (mongo_proxy.access_log()) { access_log.reset( - new Mongo::AccessLog(config.getString("access_log"), context.accessLogManager())); + new Mongo::AccessLog(mongo_proxy.access_log(), context.accessLogManager())); } Mongo::FaultConfigSharedPtr fault_config; - if (config.hasObject("fault")) { - fault_config = std::make_shared(*config.getObject("fault")); + if (mongo_proxy.delay()) { + fault_config = std::make_shared(*mongo_proxy.delay()); } return [stat_prefix, &context, access_log, @@ -36,6 +36,21 @@ MongoProxyFilterConfigFactory::createFilterFactory(const Json::Object& config, }; } +NetworkFilterFactoryCb +MongoProxyFilterConfigFactory::createFilterFactory(const Json::Object& json_mongo_proxy, + FactoryContext& context) { + envoy::api::v2::filter::network::MongoProxy mongo_proxy; + Config::FilterJson::translateMongoProxy(json_mongo_proxy, mongo_proxy); + + return createMongoProxyFactory(mongo_proxy, context); +} + +NetworkFilterFactoryCb MongoProxyFilterConfigFactory::createFilterFactoryFromProto( + const Protobuf::Message& config, FactoryContext& context) { + return createMongoProxyFactory( + dynamic_cast(config), context); +} + /** * Static registration for the mongo filter. @see RegisterFactory. */ diff --git a/source/server/config/network/mongo_proxy.h b/source/server/config/network/mongo_proxy.h index 99cb274233d3c..34f4350ac48ee 100644 --- a/source/server/config/network/mongo_proxy.h +++ b/source/server/config/network/mongo_proxy.h @@ -18,6 +18,8 @@ class MongoProxyFilterConfigFactory : public NamedNetworkFilterConfigFactory { // NamedNetworkFilterConfigFactory NetworkFilterFactoryCb createFilterFactory(const Json::Object& config, FactoryContext& context) override; + NetworkFilterFactoryCb createFilterFactoryFromProto(const Protobuf::Message& config, + FactoryContext& context) override; std::string name() override { return Config::NetworkFilterNames::get().MONGO_PROXY; } }; From 85d1d3a5fb098afb94695e8bb06ed1e1f790d77a Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Thu, 26 Oct 2017 17:16:38 -0400 Subject: [PATCH 2/9] formatting Signed-off-by: Shriram Rajagopalan --- source/common/config/filter_json.cc | 8 ++++---- source/common/config/filter_json.h | 7 +++---- source/common/mongo/proxy.h | 1 + source/server/config/network/mongo_proxy.cc | 15 +++++++-------- test/common/mongo/proxy_test.cc | 15 +++++---------- 5 files changed, 20 insertions(+), 26 deletions(-) diff --git a/source/common/config/filter_json.cc b/source/common/config/filter_json.cc index 70152a77797d7..b97c0769ee756 100644 --- a/source/common/config/filter_json.cc +++ b/source/common/config/filter_json.cc @@ -214,9 +214,8 @@ void FilterJson::translateHttpConnectionManager( } } -void FilterJson::translateMongoProxy( - const Json::Object& json_mongo_proxy, - envoy::api::v2::filter::network::MongoProxy& mongo_proxy) { +void FilterJson::translateMongoProxy(const Json::Object& json_mongo_proxy, + envoy::api::v2::filter::network::MongoProxy& mongo_proxy) { json_mongo_proxy.validateSchema(Json::Schema::MONGO_PROXY_NETWORK_FILTER_SCHEMA); JSON_UTIL_SET_STRING(json_mongo_proxy, mongo_proxy, stat_prefix); @@ -227,7 +226,8 @@ void FilterJson::translateMongoProxy( delay->set_type(envoy::api::v2::filter::FaultDelay::FaultDelayType::FIXED); delay->set_percent(static_cast(json_fault->getInteger("percent"))); - delay->mutable_fixed_delay()->CopyFrom(Protobuf::util::TimeUtil::MillisecondsToDuration(json_fault->getInteger("duration_ms"))); + delay->mutable_fixed_delay()->CopyFrom( + Protobuf::util::TimeUtil::MillisecondsToDuration(json_fault->getInteger("duration_ms"))); } } diff --git a/source/common/config/filter_json.h b/source/common/config/filter_json.h index 2b48d7bca27b9..d6244b229198a 100644 --- a/source/common/config/filter_json.h +++ b/source/common/config/filter_json.h @@ -40,15 +40,14 @@ class FilterJson { envoy::api::v2::filter::http::HttpConnectionManager& http_connection_manager); /** - * Translate a v1 JSON Mongo proxy object to v2 + * Translate a v1 JSON Mongo proxy object to v2 * envoy::api::v2::filter::network::MongoProxy. * @param json_mongo_proxy source v1 JSON HTTP connection manager object. * @param mongo_proxy destination v2 * envoy::api::v2::filter::network::MongoProxy. */ - static void translateMongoProxy( - const Json::Object& json_mongo_proxy, - envoy::api::v2::filter::network::MongoProxy& mongo_proxy); + static void translateMongoProxy(const Json::Object& json_mongo_proxy, + envoy::api::v2::filter::network::MongoProxy& mongo_proxy); }; } // namespace Config diff --git a/source/common/mongo/proxy.h b/source/common/mongo/proxy.h index ea499b5053931..43f5118e311a9 100644 --- a/source/common/mongo/proxy.h +++ b/source/common/mongo/proxy.h @@ -21,6 +21,7 @@ #include "common/common/singleton.h" #include "common/mongo/utility.h" #include "common/network/filter_impl.h" + #Include "common/protobuf/utility.h" #include "api/filter/network/mongo_proxy.pb.h" diff --git a/source/server/config/network/mongo_proxy.cc b/source/server/config/network/mongo_proxy.cc index fe9bc7e371df9..0c3ef920eb96f 100644 --- a/source/server/config/network/mongo_proxy.cc +++ b/source/server/config/network/mongo_proxy.cc @@ -14,14 +14,12 @@ namespace Envoy { namespace Server { namespace Configuration { -NetworkFilterFactoryCb -MongoProxyFilterConfigFactory::createMongoProxyFactory(envoy::api::v2::filter::MongoProxy& mongo_proxy, - FactoryContext& context) { +NetworkFilterFactoryCb MongoProxyFilterConfigFactory::createMongoProxyFactory( + envoy::api::v2::filter::MongoProxy& mongo_proxy, FactoryContext& context) { std::string stat_prefix = "mongo." + mongo_proxy.stat_prefix() + "."; Mongo::AccessLogSharedPtr access_log; if (mongo_proxy.access_log()) { - access_log.reset( - new Mongo::AccessLog(mongo_proxy.access_log(), context.accessLogManager())); + access_log.reset(new Mongo::AccessLog(mongo_proxy.access_log(), context.accessLogManager())); } Mongo::FaultConfigSharedPtr fault_config; @@ -45,10 +43,11 @@ MongoProxyFilterConfigFactory::createFilterFactory(const Json::Object& json_mong return createMongoProxyFactory(mongo_proxy, context); } -NetworkFilterFactoryCb MongoProxyFilterConfigFactory::createFilterFactoryFromProto( - const Protobuf::Message& config, FactoryContext& context) { +NetworkFilterFactoryCb +MongoProxyFilterConfigFactory::createFilterFactoryFromProto(const Protobuf::Message& config, + FactoryContext& context) { return createMongoProxyFactory( - dynamic_cast(config), context); + dynamic_cast(config), context); } /** diff --git a/test/common/mongo/proxy_test.cc b/test/common/mongo/proxy_test.cc index b6569a9fa4790..b140119529253 100644 --- a/test/common/mongo/proxy_test.cc +++ b/test/common/mongo/proxy_test.cc @@ -15,6 +15,7 @@ #include "test/mocks/runtime/mocks.h" #include "test/test_common/printers.h" +#include "api/filter/fault.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -76,17 +77,11 @@ class MongoProxyFilterTest : public testing::Test { } void setupDelayFault(bool enable_fault) { - const std::string json_config = R"EOF( - { - "fixed_delay": { - "percent": 50, - "duration_ms": 10 - } - } - )EOF"; - Json::ObjectSharedPtr config = Json::Factory::loadFromString(json_config); + envoy::api::v2::filter::FaultDelay fault{}; + fault.set_percent(50); + fault->mutable_fixed_delay()->CopyFrom(Protobuf::util::TimeUtil::MillisecondsToDuration(10)); - fault_config_.reset(new FaultConfig(*config)); + fault_config_.reset(new FaultConfig(fault)); EXPECT_CALL(runtime_.snapshot_, featureEnabled(_, _)).Times(AnyNumber()); EXPECT_CALL(runtime_.snapshot_, featureEnabled("mongo.fault.fixed_delay.percent", 50)) From 40ebeb72c97be7fa31e3fccbbfa4cc033b27f127 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Thu, 26 Oct 2017 17:27:24 -0400 Subject: [PATCH 3/9] nit Signed-off-by: Shriram Rajagopalan --- source/common/mongo/proxy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/mongo/proxy.h b/source/common/mongo/proxy.h index 43f5118e311a9..e9b2e41c7bf9b 100644 --- a/source/common/mongo/proxy.h +++ b/source/common/mongo/proxy.h @@ -22,7 +22,7 @@ #include "common/mongo/utility.h" #include "common/network/filter_impl.h" -#Include "common/protobuf/utility.h" +#include "common/protobuf/utility.h" #include "api/filter/network/mongo_proxy.pb.h" From 943804a780151499c2510be0dfc4ad44d80a8b23 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Fri, 27 Oct 2017 10:51:35 -0400 Subject: [PATCH 4/9] compilation fixes Signed-off-by: Shriram Rajagopalan --- bazel/repositories.bzl | 4 ++-- include/envoy/router/BUILD | 2 +- source/common/config/BUILD | 7 +++++-- source/common/config/filter_json.cc | 2 +- source/common/http/access_log/BUILD | 2 +- source/common/mongo/BUILD | 2 ++ source/common/mongo/proxy.h | 1 - source/common/router/BUILD | 4 ++-- source/server/config/http/BUILD | 2 +- source/server/config/network/mongo_proxy.cc | 9 +++++---- source/server/config/network/mongo_proxy.h | 6 ++++++ test/common/http/access_log/BUILD | 2 +- test/common/mongo/proxy_test.cc | 2 +- 13 files changed, 28 insertions(+), 17 deletions(-) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 90bb10ab79211..0192b6a69aaf0 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -147,7 +147,7 @@ def envoy_api_deps(skip_targets): ] for t in http_filter_bind_targets: native.bind( - name = "envoy_filter_" + t, + name = "envoy_filter_http_" + t, actual = "@envoy_api//api/filter/http:" + t + "_cc", ) network_filter_bind_targets = [ @@ -159,7 +159,7 @@ def envoy_api_deps(skip_targets): ] for t in network_filter_bind_targets: native.bind( - name = "envoy_filter_" + t, + name = "envoy_filter_network_" + t, actual = "@envoy_api//api/filter/network:" + t + "_cc", ) native.bind( diff --git a/include/envoy/router/BUILD b/include/envoy/router/BUILD index c05c964fe3f81..2ed8431718419 100644 --- a/include/envoy/router/BUILD +++ b/include/envoy/router/BUILD @@ -17,7 +17,7 @@ envoy_cc_library( envoy_cc_library( name = "route_config_provider_manager_interface", hdrs = ["route_config_provider_manager.h"], - external_deps = ["envoy_filter_http_connection_manager"], + external_deps = ["envoy_filter_http_http_connection_manager"], deps = [ ":rds_interface", "//include/envoy/event:dispatcher_interface", diff --git a/source/common/config/BUILD b/source/common/config/BUILD index a31fb3344b72f..fababa9383c66 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -87,7 +87,10 @@ envoy_cc_library( name = "filter_json_lib", srcs = ["filter_json.cc"], hdrs = ["filter_json.h"], - external_deps = ["envoy_filter_http_connection_manager"], + external_deps = [ + "envoy_filter_http_http_connection_manager", + "envoy_filter_network_mongo_proxy", + ], deps = [ ":json_utility_lib", ":protocol_json_lib", @@ -274,7 +277,7 @@ envoy_cc_library( "envoy_eds", "envoy_lds", "envoy_rds", - "envoy_filter_http_connection_manager", + "envoy_filter_http_http_connection_manager", ], deps = [ ":json_utility_lib", diff --git a/source/common/config/filter_json.cc b/source/common/config/filter_json.cc index b97c0769ee756..263f7fa68e3a3 100644 --- a/source/common/config/filter_json.cc +++ b/source/common/config/filter_json.cc @@ -224,7 +224,7 @@ void FilterJson::translateMongoProxy(const Json::Object& json_mongo_proxy, const auto json_fault = json_mongo_proxy.getObject("fault")->getObject("fixed_delay"); auto* delay = mongo_proxy.mutable_delay(); - delay->set_type(envoy::api::v2::filter::FaultDelay::FaultDelayType::FIXED); + delay->set_type(envoy::api::v2::filter::FaultDelay::FIXED); delay->set_percent(static_cast(json_fault->getInteger("percent"))); delay->mutable_fixed_delay()->CopyFrom( Protobuf::util::TimeUtil::MillisecondsToDuration(json_fault->getInteger("duration_ms"))); diff --git a/source/common/http/access_log/BUILD b/source/common/http/access_log/BUILD index c824c582c2373..7b5cd505b6c98 100644 --- a/source/common/http/access_log/BUILD +++ b/source/common/http/access_log/BUILD @@ -23,7 +23,7 @@ envoy_cc_library( name = "access_log_lib", srcs = ["access_log_impl.cc"], hdrs = ["access_log_impl.h"], - external_deps = ["envoy_filter_http_connection_manager"], + external_deps = ["envoy_filter_http_http_connection_manager"], deps = [ "//include/envoy/access_log:access_log_interface", "//include/envoy/filesystem:filesystem_interface", diff --git a/source/common/mongo/BUILD b/source/common/mongo/BUILD index c08dfab2d2904..a9416fbd6f35c 100644 --- a/source/common/mongo/BUILD +++ b/source/common/mongo/BUILD @@ -40,6 +40,7 @@ envoy_cc_library( name = "proxy_lib", srcs = ["proxy.cc"], hdrs = ["proxy.h"], + external_deps = ["envoy_filter_network_mongo_proxy"], deps = [ ":codec_lib", ":utility_lib", @@ -57,6 +58,7 @@ envoy_cc_library( "//source/common/common:logger_lib", "//source/common/common:singleton", "//source/common/common:utility_lib", + "//source/common/config:filter_json_lib", "//source/common/network:filter_lib", "//source/common/protobuf:utility_lib", ], diff --git a/source/common/mongo/proxy.h b/source/common/mongo/proxy.h index e9b2e41c7bf9b..8e8b1b057f189 100644 --- a/source/common/mongo/proxy.h +++ b/source/common/mongo/proxy.h @@ -21,7 +21,6 @@ #include "common/common/singleton.h" #include "common/mongo/utility.h" #include "common/network/filter_impl.h" - #include "common/protobuf/utility.h" #include "api/filter/network/mongo_proxy.pb.h" diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 2b13b1c51d4fe..35fc49b97af70 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -56,7 +56,7 @@ envoy_cc_library( srcs = ["rds_impl.cc"], hdrs = ["rds_impl.h"], external_deps = [ - "envoy_filter_http_connection_manager", + "envoy_filter_http_http_connection_manager", "envoy_rds", ], deps = [ @@ -85,7 +85,7 @@ envoy_cc_library( srcs = ["rds_subscription.cc"], hdrs = ["rds_subscription.h"], external_deps = [ - "envoy_filter_http_connection_manager", + "envoy_filter_http_http_connection_manager", ], deps = [ "//include/envoy/config:subscription_interface", diff --git a/source/server/config/http/BUILD b/source/server/config/http/BUILD index cc3923f4717e7..b4b887bf9b7f8 100644 --- a/source/server/config/http/BUILD +++ b/source/server/config/http/BUILD @@ -62,7 +62,7 @@ envoy_cc_library( name = "file_access_log_lib", srcs = ["file_access_log.cc"], hdrs = ["file_access_log.h"], - external_deps = ["envoy_filter_http_connection_manager"], + external_deps = ["envoy_filter_http_http_connection_manager"], deps = [ "//include/envoy/registry", "//include/envoy/server:access_log_config_interface", diff --git a/source/server/config/network/mongo_proxy.cc b/source/server/config/network/mongo_proxy.cc index 0c3ef920eb96f..937955294ddd3 100644 --- a/source/server/config/network/mongo_proxy.cc +++ b/source/server/config/network/mongo_proxy.cc @@ -5,6 +5,7 @@ #include "envoy/network/connection.h" #include "envoy/registry/registry.h" +#include "common/config/filter_json.h" #include "common/json/json_loader.h" #include "common/mongo/proxy.h" @@ -15,16 +16,16 @@ namespace Server { namespace Configuration { NetworkFilterFactoryCb MongoProxyFilterConfigFactory::createMongoProxyFactory( - envoy::api::v2::filter::MongoProxy& mongo_proxy, FactoryContext& context) { + const envoy::api::v2::filter::network::MongoProxy& mongo_proxy, FactoryContext& context) { std::string stat_prefix = "mongo." + mongo_proxy.stat_prefix() + "."; Mongo::AccessLogSharedPtr access_log; - if (mongo_proxy.access_log()) { + if (!mongo_proxy.access_log().empty()) { access_log.reset(new Mongo::AccessLog(mongo_proxy.access_log(), context.accessLogManager())); } Mongo::FaultConfigSharedPtr fault_config; - if (mongo_proxy.delay()) { - fault_config = std::make_shared(*mongo_proxy.delay()); + if (mongo_proxy.has_delay()) { + fault_config = std::make_shared(mongo_proxy.delay()); } return [stat_prefix, &context, access_log, diff --git a/source/server/config/network/mongo_proxy.h b/source/server/config/network/mongo_proxy.h index 34f4350ac48ee..5be3c4e56cc4a 100644 --- a/source/server/config/network/mongo_proxy.h +++ b/source/server/config/network/mongo_proxy.h @@ -6,6 +6,8 @@ #include "common/config/well_known_names.h" +#include "api/filter/network/mongo_proxy.pb.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -15,6 +17,10 @@ namespace Configuration { */ class MongoProxyFilterConfigFactory : public NamedNetworkFilterConfigFactory { public: + NetworkFilterFactoryCb + createMongoProxyFactory(const envoy::api::v2::filter::network::MongoProxy& mongo_proxy, + FactoryContext& context); + // NamedNetworkFilterConfigFactory NetworkFilterFactoryCb createFilterFactory(const Json::Object& config, FactoryContext& context) override; diff --git a/test/common/http/access_log/BUILD b/test/common/http/access_log/BUILD index 277e0d3abdfd1..111313530c445 100644 --- a/test/common/http/access_log/BUILD +++ b/test/common/http/access_log/BUILD @@ -24,7 +24,7 @@ envoy_cc_test( envoy_cc_test( name = "access_log_impl_test", srcs = ["access_log_impl_test.cc"], - external_deps = ["envoy_filter_http_connection_manager"], + external_deps = ["envoy_filter_http_http_connection_manager"], deps = [ "//include/envoy/upstream:cluster_manager_interface", "//include/envoy/upstream:upstream_interface", diff --git a/test/common/mongo/proxy_test.cc b/test/common/mongo/proxy_test.cc index b140119529253..0010666b332ca 100644 --- a/test/common/mongo/proxy_test.cc +++ b/test/common/mongo/proxy_test.cc @@ -79,7 +79,7 @@ class MongoProxyFilterTest : public testing::Test { void setupDelayFault(bool enable_fault) { envoy::api::v2::filter::FaultDelay fault{}; fault.set_percent(50); - fault->mutable_fixed_delay()->CopyFrom(Protobuf::util::TimeUtil::MillisecondsToDuration(10)); + fault.mutable_fixed_delay()->CopyFrom(Protobuf::util::TimeUtil::MillisecondsToDuration(10)); fault_config_.reset(new FaultConfig(fault)); From a6e6b0eb5185095f76c18e72fba310ea34ed87ac Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Fri, 27 Oct 2017 15:03:23 -0400 Subject: [PATCH 5/9] proto tests Signed-off-by: Shriram Rajagopalan --- source/common/config/filter_json.cc | 3 +- source/common/config/json_utility.h | 10 ++++ source/server/config/network/mongo_proxy.cc | 10 ++++ source/server/config/network/mongo_proxy.h | 2 +- .../server/config/network/mongo_proxy_test.cc | 51 ++++++++++++++++++- 5 files changed, 72 insertions(+), 4 deletions(-) diff --git a/source/common/config/filter_json.cc b/source/common/config/filter_json.cc index 263f7fa68e3a3..0e169c644f086 100644 --- a/source/common/config/filter_json.cc +++ b/source/common/config/filter_json.cc @@ -226,8 +226,7 @@ void FilterJson::translateMongoProxy(const Json::Object& json_mongo_proxy, delay->set_type(envoy::api::v2::filter::FaultDelay::FIXED); delay->set_percent(static_cast(json_fault->getInteger("percent"))); - delay->mutable_fixed_delay()->CopyFrom( - Protobuf::util::TimeUtil::MillisecondsToDuration(json_fault->getInteger("duration_ms"))); + JSON_UTIL_SET_DURATION_FROM_FIELD(*json_fault, *delay, fixed_delay, duration); } } diff --git a/source/common/config/json_utility.h b/source/common/config/json_utility.h index a01c8ff5eecc2..888aa2b667950 100644 --- a/source/common/config/json_utility.h +++ b/source/common/config/json_utility.h @@ -50,3 +50,13 @@ 1000 * (json).getInteger(#field_name "_s"))); \ } \ } while (0) + +// Set a google.protobuf.Duration field in a protobuf message with the specified field's +// milliseconds value from a JSON object if the field is set in the JSON object. +#define JSON_UTIL_SET_DURATION_FROM_FIELD(json, message, dst_field, src_field) \ + do { \ + if ((json).hasObject(#src_field "_ms")) { \ + (message).mutable_##dst_field()->CopyFrom( \ + Protobuf::util::TimeUtil::MillisecondsToDuration((json).getInteger(#src_field "_ms"))); \ + } \ + } while (0) diff --git a/source/server/config/network/mongo_proxy.cc b/source/server/config/network/mongo_proxy.cc index 937955294ddd3..fa8766bfdf951 100644 --- a/source/server/config/network/mongo_proxy.cc +++ b/source/server/config/network/mongo_proxy.cc @@ -17,7 +17,12 @@ namespace Configuration { NetworkFilterFactoryCb MongoProxyFilterConfigFactory::createMongoProxyFactory( const envoy::api::v2::filter::network::MongoProxy& mongo_proxy, FactoryContext& context) { + + if (mongo_proxy.stat_prefix().empty()) { + throw MissingFieldException("stat_prefix", mongo_proxy); + } std::string stat_prefix = "mongo." + mongo_proxy.stat_prefix() + "."; + Mongo::AccessLogSharedPtr access_log; if (!mongo_proxy.access_log().empty()) { access_log.reset(new Mongo::AccessLog(mongo_proxy.access_log(), context.accessLogManager())); @@ -25,6 +30,11 @@ NetworkFilterFactoryCb MongoProxyFilterConfigFactory::createMongoProxyFactory( Mongo::FaultConfigSharedPtr fault_config; if (mongo_proxy.has_delay()) { + auto delay = mongo_proxy.delay(); + if (!delay.has_fixed_delay()) { + throw MissingFieldException("delay.fixed_delay", mongo_proxy); + } + fault_config = std::make_shared(mongo_proxy.delay()); } diff --git a/source/server/config/network/mongo_proxy.h b/source/server/config/network/mongo_proxy.h index 5be3c4e56cc4a..c16a24517d4da 100644 --- a/source/server/config/network/mongo_proxy.h +++ b/source/server/config/network/mongo_proxy.h @@ -16,11 +16,11 @@ namespace Configuration { * Config registration for the mongo proxy filter. @see NamedNetworkFilterConfigFactory. */ class MongoProxyFilterConfigFactory : public NamedNetworkFilterConfigFactory { -public: NetworkFilterFactoryCb createMongoProxyFactory(const envoy::api::v2::filter::network::MongoProxy& mongo_proxy, FactoryContext& context); +public: // NamedNetworkFilterConfigFactory NetworkFilterFactoryCb createFilterFactory(const Json::Object& config, FactoryContext& context) override; diff --git a/test/server/config/network/mongo_proxy_test.cc b/test/server/config/network/mongo_proxy_test.cc index e7e5c1ebc24c4..60df153a0938c 100644 --- a/test/server/config/network/mongo_proxy_test.cc +++ b/test/server/config/network/mongo_proxy_test.cc @@ -8,6 +8,8 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "api/filter/network/mongo_proxy.pb.h" + using testing::NiceMock; using testing::_; @@ -32,6 +34,20 @@ TEST(MongoFilterConfigTest, CorrectConfigurationNoFaults) { cb(connection); } +TEST(MongoFilterConfigTest, CorrectProtoConfigurationNoFaults) { + envoy::api::v2::filter::network::MongoProxy config{}; + + config.set_access_log("path/to/access/log"); + config.set_stat_prefix("my_stat_prefix"); + + NiceMock context; + MongoProxyFilterConfigFactory factory; + NetworkFilterFactoryCb cb = factory.createFilterFactoryFromProto(config, context); + Network::MockConnection connection; + EXPECT_CALL(connection, addFilter(_)); + cb(connection); +} + void handleInvalidConfiguration(const std::string& json_string) { Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); NiceMock context; @@ -40,6 +56,13 @@ void handleInvalidConfiguration(const std::string& json_string) { EXPECT_THROW(factory.createFilterFactory(*json_config, context), Json::Exception); } + void handleInvalidProto(const envoy::api::v2::filter::network::MongoProxy& config) { + NiceMock context; + MongoProxyFilterConfigFactory factory; + + EXPECT_THROW(factory.createFilterFactoryFromProto(config, context), MissingFieldException); + } + TEST(MongoFilterConfigTest, InvalidExtraProperty) { std::string json_string = R"EOF( { @@ -54,6 +77,11 @@ TEST(MongoFilterConfigTest, InvalidExtraProperty) { TEST(MongoFilterConfigTest, EmptyConfig) { handleInvalidConfiguration("{}"); } +TEST(MongoFilterConfigTest, EmptyProto) { +envoy::api::v2::filter::network::MongoProxy config{}; + handleInvalidProto(config); + } + TEST(MongoFilterConfigTest, InvalidFaultsEmptyConfig) { std::string json_string = R"EOF( { @@ -95,7 +123,14 @@ TEST(MongoFilterConfigTest, InvalidFaultsMissingMs) { handleInvalidConfiguration(json_string); } -TEST(MongoFilterConfigTest, InvalidFaultsNegativeMs) { + TEST(MongoFilterConfigTest, InvalidFaultsMissingDurationInProto) { + envoy::api::v2::filter::network::MongoProxy config{}; + config.set_stat_prefix("my_stat_prefix"); + config.mutable_delay()->set_percent(50); + handleInvalidProto(config); + } + + TEST(MongoFilterConfigTest, InvalidFaultsNegativeMs) { std::string json_string = R"EOF( { "stat_prefix": "my_stat_prefix", @@ -217,6 +252,20 @@ TEST(MongoFilterConfigTest, CorrectFaultConfiguration) { cb(connection); } + TEST(MongoFilterConfigTest, CorrectFaultConfigurationInProto) { + envoy::api::v2::filter::network::MongoProxy config{}; + config.set_stat_prefix("my_stat_prefix"); + config.mutable_delay()->set_percent(50); + config.mutable_delay()->mutable_fixed_delay()->set_seconds(500); + + NiceMock context; + MongoProxyFilterConfigFactory factory; + NetworkFilterFactoryCb cb = factory.createFilterFactoryFromProto(config, context); + Network::MockConnection connection; + EXPECT_CALL(connection, addFilter(_)); + cb(connection); + } + } // namespace Configuration } // namespace Server } // namespace Envoy From a43e50ca123de662d5dc0bc637313bc378907441 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Fri, 27 Oct 2017 15:04:08 -0400 Subject: [PATCH 6/9] formatting Signed-off-by: Shriram Rajagopalan --- .../server/config/network/mongo_proxy_test.cc | 59 +++++++++---------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/test/server/config/network/mongo_proxy_test.cc b/test/server/config/network/mongo_proxy_test.cc index 60df153a0938c..1eeb772e73f82 100644 --- a/test/server/config/network/mongo_proxy_test.cc +++ b/test/server/config/network/mongo_proxy_test.cc @@ -5,11 +5,10 @@ #include "test/mocks/server/mocks.h" #include "test/test_common/utility.h" +#include "api/filter/network/mongo_proxy.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "api/filter/network/mongo_proxy.pb.h" - using testing::NiceMock; using testing::_; @@ -56,12 +55,12 @@ void handleInvalidConfiguration(const std::string& json_string) { EXPECT_THROW(factory.createFilterFactory(*json_config, context), Json::Exception); } - void handleInvalidProto(const envoy::api::v2::filter::network::MongoProxy& config) { - NiceMock context; - MongoProxyFilterConfigFactory factory; +void handleInvalidProto(const envoy::api::v2::filter::network::MongoProxy& config) { + NiceMock context; + MongoProxyFilterConfigFactory factory; - EXPECT_THROW(factory.createFilterFactoryFromProto(config, context), MissingFieldException); - } + EXPECT_THROW(factory.createFilterFactoryFromProto(config, context), MissingFieldException); +} TEST(MongoFilterConfigTest, InvalidExtraProperty) { std::string json_string = R"EOF( @@ -78,9 +77,9 @@ TEST(MongoFilterConfigTest, InvalidExtraProperty) { TEST(MongoFilterConfigTest, EmptyConfig) { handleInvalidConfiguration("{}"); } TEST(MongoFilterConfigTest, EmptyProto) { -envoy::api::v2::filter::network::MongoProxy config{}; - handleInvalidProto(config); - } + envoy::api::v2::filter::network::MongoProxy config{}; + handleInvalidProto(config); +} TEST(MongoFilterConfigTest, InvalidFaultsEmptyConfig) { std::string json_string = R"EOF( @@ -123,14 +122,14 @@ TEST(MongoFilterConfigTest, InvalidFaultsMissingMs) { handleInvalidConfiguration(json_string); } - TEST(MongoFilterConfigTest, InvalidFaultsMissingDurationInProto) { - envoy::api::v2::filter::network::MongoProxy config{}; - config.set_stat_prefix("my_stat_prefix"); - config.mutable_delay()->set_percent(50); - handleInvalidProto(config); - } +TEST(MongoFilterConfigTest, InvalidFaultsMissingDurationInProto) { + envoy::api::v2::filter::network::MongoProxy config{}; + config.set_stat_prefix("my_stat_prefix"); + config.mutable_delay()->set_percent(50); + handleInvalidProto(config); +} - TEST(MongoFilterConfigTest, InvalidFaultsNegativeMs) { +TEST(MongoFilterConfigTest, InvalidFaultsNegativeMs) { std::string json_string = R"EOF( { "stat_prefix": "my_stat_prefix", @@ -252,19 +251,19 @@ TEST(MongoFilterConfigTest, CorrectFaultConfiguration) { cb(connection); } - TEST(MongoFilterConfigTest, CorrectFaultConfigurationInProto) { - envoy::api::v2::filter::network::MongoProxy config{}; - config.set_stat_prefix("my_stat_prefix"); - config.mutable_delay()->set_percent(50); - config.mutable_delay()->mutable_fixed_delay()->set_seconds(500); - - NiceMock context; - MongoProxyFilterConfigFactory factory; - NetworkFilterFactoryCb cb = factory.createFilterFactoryFromProto(config, context); - Network::MockConnection connection; - EXPECT_CALL(connection, addFilter(_)); - cb(connection); - } +TEST(MongoFilterConfigTest, CorrectFaultConfigurationInProto) { + envoy::api::v2::filter::network::MongoProxy config{}; + config.set_stat_prefix("my_stat_prefix"); + config.mutable_delay()->set_percent(50); + config.mutable_delay()->mutable_fixed_delay()->set_seconds(500); + + NiceMock context; + MongoProxyFilterConfigFactory factory; + NetworkFilterFactoryCb cb = factory.createFilterFactoryFromProto(config, context); + Network::MockConnection connection; + EXPECT_CALL(connection, addFilter(_)); + cb(connection); +} } // namespace Configuration } // namespace Server From 097e4d4a38621f1074440f1243b86cfdffc2678d Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Fri, 27 Oct 2017 17:17:49 -0400 Subject: [PATCH 7/9] nits Signed-off-by: Shriram Rajagopalan --- source/common/config/filter_json.h | 3 +-- source/server/config/network/BUILD | 1 - source/server/config/network/mongo_proxy.h | 8 +++++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/source/common/config/filter_json.h b/source/common/config/filter_json.h index d6244b229198a..7004c57290048 100644 --- a/source/common/config/filter_json.h +++ b/source/common/config/filter_json.h @@ -40,8 +40,7 @@ class FilterJson { envoy::api::v2::filter::http::HttpConnectionManager& http_connection_manager); /** - * Translate a v1 JSON Mongo proxy object to v2 - * envoy::api::v2::filter::network::MongoProxy. + * Translate a v1 JSON Mongo proxy object to v2 envoy::api::v2::filter::network::MongoProxy. * @param json_mongo_proxy source v1 JSON HTTP connection manager object. * @param mongo_proxy destination v2 * envoy::api::v2::filter::network::MongoProxy. diff --git a/source/server/config/network/BUILD b/source/server/config/network/BUILD index a79f82edbd2b3..91ac19309d8ca 100644 --- a/source/server/config/network/BUILD +++ b/source/server/config/network/BUILD @@ -67,7 +67,6 @@ envoy_cc_library( "//include/envoy/registry", "//include/envoy/server:filter_config_interface", "//source/common/config:well_known_names", - "//source/common/json:json_loader_lib", "//source/common/mongo:proxy_lib", ], ) diff --git a/source/server/config/network/mongo_proxy.h b/source/server/config/network/mongo_proxy.h index c16a24517d4da..ce55cbec8fb9e 100644 --- a/source/server/config/network/mongo_proxy.h +++ b/source/server/config/network/mongo_proxy.h @@ -16,9 +16,6 @@ namespace Configuration { * Config registration for the mongo proxy filter. @see NamedNetworkFilterConfigFactory. */ class MongoProxyFilterConfigFactory : public NamedNetworkFilterConfigFactory { - NetworkFilterFactoryCb - createMongoProxyFactory(const envoy::api::v2::filter::network::MongoProxy& mongo_proxy, - FactoryContext& context); public: // NamedNetworkFilterConfigFactory @@ -28,6 +25,11 @@ class MongoProxyFilterConfigFactory : public NamedNetworkFilterConfigFactory { FactoryContext& context) override; std::string name() override { return Config::NetworkFilterNames::get().MONGO_PROXY; } + +private: + NetworkFilterFactoryCb + createMongoProxyFactory(const envoy::api::v2::filter::network::MongoProxy& mongo_proxy, + FactoryContext& context); }; } // namespace Configuration From 33ecf79ea111651ecc89b237c51c8cb43d8cf98b Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Fri, 27 Oct 2017 21:57:38 -0400 Subject: [PATCH 8/9] remove validation checks Signed-off-by: Shriram Rajagopalan --- source/server/config/network/mongo_proxy.cc | 9 ++------- .../server/config/network/mongo_proxy_test.cc | 19 ------------------- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/source/server/config/network/mongo_proxy.cc b/source/server/config/network/mongo_proxy.cc index fa8766bfdf951..6d3d60feb61c3 100644 --- a/source/server/config/network/mongo_proxy.cc +++ b/source/server/config/network/mongo_proxy.cc @@ -18,9 +18,7 @@ namespace Configuration { NetworkFilterFactoryCb MongoProxyFilterConfigFactory::createMongoProxyFactory( const envoy::api::v2::filter::network::MongoProxy& mongo_proxy, FactoryContext& context) { - if (mongo_proxy.stat_prefix().empty()) { - throw MissingFieldException("stat_prefix", mongo_proxy); - } + ASSERT(!mongo_proxy.stat_prefix().empty()); std::string stat_prefix = "mongo." + mongo_proxy.stat_prefix() + "."; Mongo::AccessLogSharedPtr access_log; @@ -31,10 +29,7 @@ NetworkFilterFactoryCb MongoProxyFilterConfigFactory::createMongoProxyFactory( Mongo::FaultConfigSharedPtr fault_config; if (mongo_proxy.has_delay()) { auto delay = mongo_proxy.delay(); - if (!delay.has_fixed_delay()) { - throw MissingFieldException("delay.fixed_delay", mongo_proxy); - } - + ASSERT(delay.has_fixed_delay()); fault_config = std::make_shared(mongo_proxy.delay()); } diff --git a/test/server/config/network/mongo_proxy_test.cc b/test/server/config/network/mongo_proxy_test.cc index 1eeb772e73f82..935327e01c2c7 100644 --- a/test/server/config/network/mongo_proxy_test.cc +++ b/test/server/config/network/mongo_proxy_test.cc @@ -55,13 +55,6 @@ void handleInvalidConfiguration(const std::string& json_string) { EXPECT_THROW(factory.createFilterFactory(*json_config, context), Json::Exception); } -void handleInvalidProto(const envoy::api::v2::filter::network::MongoProxy& config) { - NiceMock context; - MongoProxyFilterConfigFactory factory; - - EXPECT_THROW(factory.createFilterFactoryFromProto(config, context), MissingFieldException); -} - TEST(MongoFilterConfigTest, InvalidExtraProperty) { std::string json_string = R"EOF( { @@ -76,11 +69,6 @@ TEST(MongoFilterConfigTest, InvalidExtraProperty) { TEST(MongoFilterConfigTest, EmptyConfig) { handleInvalidConfiguration("{}"); } -TEST(MongoFilterConfigTest, EmptyProto) { - envoy::api::v2::filter::network::MongoProxy config{}; - handleInvalidProto(config); -} - TEST(MongoFilterConfigTest, InvalidFaultsEmptyConfig) { std::string json_string = R"EOF( { @@ -122,13 +110,6 @@ TEST(MongoFilterConfigTest, InvalidFaultsMissingMs) { handleInvalidConfiguration(json_string); } -TEST(MongoFilterConfigTest, InvalidFaultsMissingDurationInProto) { - envoy::api::v2::filter::network::MongoProxy config{}; - config.set_stat_prefix("my_stat_prefix"); - config.mutable_delay()->set_percent(50); - handleInvalidProto(config); -} - TEST(MongoFilterConfigTest, InvalidFaultsNegativeMs) { std::string json_string = R"EOF( { From b434fa00ad4159741b0550cdbd6410cbd1a95ee5 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Sun, 29 Oct 2017 19:38:41 -0400 Subject: [PATCH 9/9] nits Signed-off-by: Shriram Rajagopalan --- source/server/config/network/mongo_proxy.cc | 1 - source/server/config/network/mongo_proxy.h | 1 - test/server/config/network/mongo_proxy_test.cc | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/source/server/config/network/mongo_proxy.cc b/source/server/config/network/mongo_proxy.cc index 6d3d60feb61c3..9f50c608e8cd1 100644 --- a/source/server/config/network/mongo_proxy.cc +++ b/source/server/config/network/mongo_proxy.cc @@ -6,7 +6,6 @@ #include "envoy/registry/registry.h" #include "common/config/filter_json.h" -#include "common/json/json_loader.h" #include "common/mongo/proxy.h" #include "api/filter/network/mongo_proxy.pb.h" diff --git a/source/server/config/network/mongo_proxy.h b/source/server/config/network/mongo_proxy.h index ce55cbec8fb9e..73f5105db0650 100644 --- a/source/server/config/network/mongo_proxy.h +++ b/source/server/config/network/mongo_proxy.h @@ -16,7 +16,6 @@ namespace Configuration { * Config registration for the mongo proxy filter. @see NamedNetworkFilterConfigFactory. */ class MongoProxyFilterConfigFactory : public NamedNetworkFilterConfigFactory { - public: // NamedNetworkFilterConfigFactory NetworkFilterFactoryCb createFilterFactory(const Json::Object& config, diff --git a/test/server/config/network/mongo_proxy_test.cc b/test/server/config/network/mongo_proxy_test.cc index 935327e01c2c7..ed4f608fd5112 100644 --- a/test/server/config/network/mongo_proxy_test.cc +++ b/test/server/config/network/mongo_proxy_test.cc @@ -33,7 +33,7 @@ TEST(MongoFilterConfigTest, CorrectConfigurationNoFaults) { cb(connection); } -TEST(MongoFilterConfigTest, CorrectProtoConfigurationNoFaults) { +TEST(MongoFilterConfigTest, ValidProtoConfigurationNoFaults) { envoy::api::v2::filter::network::MongoProxy config{}; config.set_access_log("path/to/access/log");