From b5a58c5f9e223805193b08608194477d847f3b45 Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Thu, 31 Aug 2017 16:46:30 -0700 Subject: [PATCH 1/3] rds: allow v1 rds config to use a v2 subscription --- source/common/config/utility.cc | 13 ++++++++++++- source/common/json/config_schemas.cc | 4 ++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index ea42a65328606..0d1fb35b38df7 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -14,8 +14,15 @@ namespace Config { namespace { void translateApiConfigSource(const std::string& cluster, uint32_t refresh_delay_ms, + const std::string& api_type, envoy::api::v2::ApiConfigSource& api_config_source) { - api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::REST_LEGACY); + if (api_type == "REST_LEGACY") { + api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::REST_LEGACY); + } else if (api_type == "REST") { + api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::REST); + } else if (api_type == "GRPC") { + api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::GRPC); + } api_config_source.add_cluster_name(cluster); api_config_source.mutable_refresh_delay()->CopyFrom( Protobuf::util::TimeUtil::MillisecondsToDuration(refresh_delay_ms)); @@ -63,6 +70,7 @@ void Utility::translateEdsConfig(const Json::Object& json_config, envoy::api::v2::ConfigSource& eds_config) { translateApiConfigSource(json_config.getObject("cluster")->getString("name"), json_config.getInteger("refresh_delay_ms", 30000), + json_config.getString("api_type", "REST_LEGACY"), *eds_config.mutable_api_config_source()); } @@ -70,6 +78,7 @@ void Utility::translateCdsConfig(const Json::Object& json_config, envoy::api::v2::ConfigSource& cds_config) { translateApiConfigSource(json_config.getObject("cluster")->getString("name"), json_config.getInteger("refresh_delay_ms", 30000), + json_config.getString("api_type", "REST_LEGACY"), *cds_config.mutable_api_config_source()); } @@ -77,6 +86,7 @@ void Utility::translateRdsConfig(const Json::Object& json_rds, envoy::api::v2::f json_rds.validateSchema(Json::Schema::RDS_CONFIGURATION_SCHEMA); translateApiConfigSource(json_rds.getString("cluster"), json_rds.getInteger("refresh_delay_ms", 30000), + json_rds.getString("api_type", "REST_LEGACY"), *rds.mutable_config_source()->mutable_api_config_source()); JSON_UTIL_SET_STRING(json_rds, rds, route_config_name); } @@ -86,6 +96,7 @@ void Utility::translateLdsConfig(const Json::Object& json_lds, json_lds.validateSchema(Json::Schema::LDS_CONFIG_SCHEMA); translateApiConfigSource(json_lds.getString("cluster"), json_lds.getInteger("refresh_delay_ms", 30000), + json_lds.getString("api_type", "REST_LEGACY"), *lds_config.mutable_api_config_source()); } diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index d6c8617f04651..177d0cea2bfe4 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -103,6 +103,10 @@ const std::string Json::Schema::RDS_CONFIGURATION_SCHEMA(R"EOF( "type" : "integer", "minimum" : 0, "exclusiveMinimum" : true + }, + "api_type" : { + "type" : "string", + "enum" : ["REST_LEGACY", "REST", "GRPC"] } }, "required" : ["cluster", "route_config_name"], From d5d31b2c88290b3b26261a9177e8463abb72050d Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Fri, 1 Sep 2017 11:33:57 -0700 Subject: [PATCH 2/3] tests --- source/common/config/utility.cc | 35 +++++++++++++++--------------- source/common/config/utility.h | 15 +++++++++++++ test/common/config/utility_test.cc | 26 ++++++++++++++++++++++ 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index 0d1fb35b38df7..fddd383dfabdb 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -11,16 +11,20 @@ namespace Envoy { namespace Config { -namespace { - -void translateApiConfigSource(const std::string& cluster, uint32_t refresh_delay_ms, - const std::string& api_type, - envoy::api::v2::ApiConfigSource& api_config_source) { - if (api_type == "REST_LEGACY") { +const std::string Utility::REST_LEGACY = "REST_LEGACY"; +const std::string Utility::REST = "REST"; +const std::string Utility::GRPC = "GRPC"; + +void Utility::translateApiConfigSource(const std::string& cluster, uint32_t refresh_delay_ms, + const std::string& api_type, + envoy::api::v2::ApiConfigSource& api_config_source) { + // TODO(junr03): document the option to chose an api type once we have created + // stronger constraints around v2. + if (api_type == REST_LEGACY) { api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::REST_LEGACY); - } else if (api_type == "REST") { + } else if (api_type == REST) { api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::REST); - } else if (api_type == "GRPC") { + } else if (api_type == GRPC) { api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::GRPC); } api_config_source.add_cluster_name(cluster); @@ -28,8 +32,6 @@ void translateApiConfigSource(const std::string& cluster, uint32_t refresh_delay Protobuf::util::TimeUtil::MillisecondsToDuration(refresh_delay_ms)); } -} // namespace - void Utility::checkCluster(const std::string& error_prefix, const std::string& cluster_name, Upstream::ClusterManager& cm) { Upstream::ThreadLocalCluster* cluster = cm.get(cluster_name); @@ -70,7 +72,7 @@ void Utility::translateEdsConfig(const Json::Object& json_config, envoy::api::v2::ConfigSource& eds_config) { translateApiConfigSource(json_config.getObject("cluster")->getString("name"), json_config.getInteger("refresh_delay_ms", 30000), - json_config.getString("api_type", "REST_LEGACY"), + json_config.getString("api_type", REST_LEGACY), *eds_config.mutable_api_config_source()); } @@ -78,7 +80,7 @@ void Utility::translateCdsConfig(const Json::Object& json_config, envoy::api::v2::ConfigSource& cds_config) { translateApiConfigSource(json_config.getObject("cluster")->getString("name"), json_config.getInteger("refresh_delay_ms", 30000), - json_config.getString("api_type", "REST_LEGACY"), + json_config.getString("api_type", REST_LEGACY), *cds_config.mutable_api_config_source()); } @@ -86,7 +88,7 @@ void Utility::translateRdsConfig(const Json::Object& json_rds, envoy::api::v2::f json_rds.validateSchema(Json::Schema::RDS_CONFIGURATION_SCHEMA); translateApiConfigSource(json_rds.getString("cluster"), json_rds.getInteger("refresh_delay_ms", 30000), - json_rds.getString("api_type", "REST_LEGACY"), + json_rds.getString("api_type", REST_LEGACY), *rds.mutable_config_source()->mutable_api_config_source()); JSON_UTIL_SET_STRING(json_rds, rds, route_config_name); } @@ -94,10 +96,9 @@ void Utility::translateRdsConfig(const Json::Object& json_rds, envoy::api::v2::f void Utility::translateLdsConfig(const Json::Object& json_lds, envoy::api::v2::ConfigSource& lds_config) { json_lds.validateSchema(Json::Schema::LDS_CONFIG_SCHEMA); - translateApiConfigSource(json_lds.getString("cluster"), - json_lds.getInteger("refresh_delay_ms", 30000), - json_lds.getString("api_type", "REST_LEGACY"), - *lds_config.mutable_api_config_source()); + translateApiConfigSource( + json_lds.getString("cluster"), json_lds.getInteger("refresh_delay_ms", 30000), + json_lds.getString("api_type", REST_LEGACY), *lds_config.mutable_api_config_source()); } } // namespace Config diff --git a/source/common/config/utility.h b/source/common/config/utility.h index 57c65472f12d2..615183c7902e3 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -40,6 +40,17 @@ class Utility { static std::chrono::milliseconds apiConfigSourceRefreshDelay(const envoy::api::v2::ApiConfigSource& api_config_source); + /** + * Populate an envoy::api::v2::ApiConfigSource. + * @param cluster supplies the cluster name for the ApiConfigSource. + * @param refresh_delay_ms supplies the refresh delay for the ApiConfigSource in ms. + * @param api_type supplies the type of subscription to use for the ApiConfigSource. + * @param api_config_source a reference to the envoy::api::v2::ApiConfigSource object to populate. + */ + static void translateApiConfigSource(const std::string& cluster, uint32_t refresh_delay_ms, + const std::string& api_type, + envoy::api::v2::ApiConfigSource& api_config_source); + /** * Check cluster info for API config sanity. Throws on error. * @param error_prefix supplies the prefix to use in error messages. @@ -108,6 +119,10 @@ class Utility { static SubscriptionStats generateStats(Stats::Scope& scope) { return {ALL_SUBSCRIPTION_STATS(POOL_COUNTER(scope))}; } + + static const std::string REST_LEGACY; + static const std::string REST; + static const std::string GRPC; }; } // namespace Config diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index 72b6415c23b08..f8c7e0314a63d 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -38,5 +38,31 @@ TEST(UtilityTest, ApiConfigSourceRefreshDelay) { EXPECT_EQ(1234, Utility::apiConfigSourceRefreshDelay(api_config_source).count()); } +TEST(UtilityTest, TranslateApiConfigSource) { + envoy::api::v2::ApiConfigSource api_config_source_rest_legacy; + Utility::translateApiConfigSource("test_rest_legacy_cluster", 10000, Utility::REST_LEGACY, + api_config_source_rest_legacy); + EXPECT_EQ(envoy::api::v2::ApiConfigSource::REST_LEGACY, api_config_source_rest_legacy.api_type()); + EXPECT_EQ(10000, Protobuf::util::TimeUtil::DurationToMilliseconds( + api_config_source_rest_legacy.refresh_delay())); + EXPECT_EQ("test_rest_legacy_cluster", api_config_source_rest_legacy.cluster_name(0)); + + envoy::api::v2::ApiConfigSource api_config_source_rest; + Utility::translateApiConfigSource("test_rest_cluster", 20000, Utility::REST, + api_config_source_rest); + EXPECT_EQ(envoy::api::v2::ApiConfigSource::REST, api_config_source_rest.api_type()); + EXPECT_EQ(20000, Protobuf::util::TimeUtil::DurationToMilliseconds( + api_config_source_rest.refresh_delay())); + EXPECT_EQ("test_rest_cluster", api_config_source_rest.cluster_name(0)); + + envoy::api::v2::ApiConfigSource api_config_source_grpc; + Utility::translateApiConfigSource("test_grpc_cluster", 30000, Utility::GRPC, + api_config_source_grpc); + EXPECT_EQ(envoy::api::v2::ApiConfigSource::GRPC, api_config_source_grpc.api_type()); + EXPECT_EQ(30000, Protobuf::util::TimeUtil::DurationToMilliseconds( + api_config_source_grpc.refresh_delay())); + EXPECT_EQ("test_grpc_cluster", api_config_source_grpc.cluster_name(0)); +} + } // namespace Config } // namespace Envoy From 6f52abdea138ec43fc3a1a16886d444245f4fe57 Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Fri, 1 Sep 2017 12:57:00 -0700 Subject: [PATCH 3/3] nits --- source/common/config/BUILD | 1 + source/common/config/utility.cc | 25 ++++++++++++------------- source/common/config/utility.h | 17 +++++++++++++---- test/common/config/utility_test.cc | 6 +++--- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 8955c6cd16a61..d41b50d4347c6 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -237,6 +237,7 @@ envoy_cc_library( "//include/envoy/upstream:cluster_manager_interface", "//source/common/common:assert_lib", "//source/common/common:hex_lib", + "//source/common/common:singleton", "//source/common/json:config_schemas_lib", ], ) diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index fddd383dfabdb..ae979a0274e94 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -1,5 +1,6 @@ #include "common/config/utility.h" +#include "common/common/assert.h" #include "common/common/hex.h" #include "common/common/utility.h" #include "common/config/json_utility.h" @@ -11,20 +12,17 @@ namespace Envoy { namespace Config { -const std::string Utility::REST_LEGACY = "REST_LEGACY"; -const std::string Utility::REST = "REST"; -const std::string Utility::GRPC = "GRPC"; - void Utility::translateApiConfigSource(const std::string& cluster, uint32_t refresh_delay_ms, const std::string& api_type, envoy::api::v2::ApiConfigSource& api_config_source) { // TODO(junr03): document the option to chose an api type once we have created // stronger constraints around v2. - if (api_type == REST_LEGACY) { + if (api_type == ApiType::get().RestLegacy) { api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::REST_LEGACY); - } else if (api_type == REST) { + } else if (api_type == ApiType::get().Rest) { api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::REST); - } else if (api_type == GRPC) { + } else { + ASSERT(api_type == ApiType::get().Grpc); api_config_source.set_api_type(envoy::api::v2::ApiConfigSource::GRPC); } api_config_source.add_cluster_name(cluster); @@ -72,7 +70,7 @@ void Utility::translateEdsConfig(const Json::Object& json_config, envoy::api::v2::ConfigSource& eds_config) { translateApiConfigSource(json_config.getObject("cluster")->getString("name"), json_config.getInteger("refresh_delay_ms", 30000), - json_config.getString("api_type", REST_LEGACY), + json_config.getString("api_type", ApiType::get().RestLegacy), *eds_config.mutable_api_config_source()); } @@ -80,7 +78,7 @@ void Utility::translateCdsConfig(const Json::Object& json_config, envoy::api::v2::ConfigSource& cds_config) { translateApiConfigSource(json_config.getObject("cluster")->getString("name"), json_config.getInteger("refresh_delay_ms", 30000), - json_config.getString("api_type", REST_LEGACY), + json_config.getString("api_type", ApiType::get().RestLegacy), *cds_config.mutable_api_config_source()); } @@ -88,7 +86,7 @@ void Utility::translateRdsConfig(const Json::Object& json_rds, envoy::api::v2::f json_rds.validateSchema(Json::Schema::RDS_CONFIGURATION_SCHEMA); translateApiConfigSource(json_rds.getString("cluster"), json_rds.getInteger("refresh_delay_ms", 30000), - json_rds.getString("api_type", REST_LEGACY), + json_rds.getString("api_type", ApiType::get().RestLegacy), *rds.mutable_config_source()->mutable_api_config_source()); JSON_UTIL_SET_STRING(json_rds, rds, route_config_name); } @@ -96,9 +94,10 @@ void Utility::translateRdsConfig(const Json::Object& json_rds, envoy::api::v2::f void Utility::translateLdsConfig(const Json::Object& json_lds, envoy::api::v2::ConfigSource& lds_config) { json_lds.validateSchema(Json::Schema::LDS_CONFIG_SCHEMA); - translateApiConfigSource( - json_lds.getString("cluster"), json_lds.getInteger("refresh_delay_ms", 30000), - json_lds.getString("api_type", REST_LEGACY), *lds_config.mutable_api_config_source()); + translateApiConfigSource(json_lds.getString("cluster"), + json_lds.getInteger("refresh_delay_ms", 30000), + json_lds.getString("api_type", ApiType::get().RestLegacy), + *lds_config.mutable_api_config_source()); } } // namespace Config diff --git a/source/common/config/utility.h b/source/common/config/utility.h index 615183c7902e3..ade756ca2ac6c 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -5,6 +5,7 @@ #include "envoy/local_info/local_info.h" #include "envoy/upstream/cluster_manager.h" +#include "common/common/singleton.h" #include "common/protobuf/protobuf.h" #include "api/base.pb.h" @@ -13,6 +14,18 @@ namespace Envoy { namespace Config { +/** + * Constant Api Type Values, used by envoy::api::v2::ApiConfigSource. + */ +class ApiTypeValues { +public: + const std::string RestLegacy{"REST_LEGACY"}; + const std::string Rest{"REST"}; + const std::string Grpc{"GRPC"}; +}; + +typedef ConstSingleton ApiType; + /** * General config API utilities. */ @@ -119,10 +132,6 @@ class Utility { static SubscriptionStats generateStats(Stats::Scope& scope) { return {ALL_SUBSCRIPTION_STATS(POOL_COUNTER(scope))}; } - - static const std::string REST_LEGACY; - static const std::string REST; - static const std::string GRPC; }; } // namespace Config diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index f8c7e0314a63d..1c400020b5f39 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -40,7 +40,7 @@ TEST(UtilityTest, ApiConfigSourceRefreshDelay) { TEST(UtilityTest, TranslateApiConfigSource) { envoy::api::v2::ApiConfigSource api_config_source_rest_legacy; - Utility::translateApiConfigSource("test_rest_legacy_cluster", 10000, Utility::REST_LEGACY, + Utility::translateApiConfigSource("test_rest_legacy_cluster", 10000, ApiType::get().RestLegacy, api_config_source_rest_legacy); EXPECT_EQ(envoy::api::v2::ApiConfigSource::REST_LEGACY, api_config_source_rest_legacy.api_type()); EXPECT_EQ(10000, Protobuf::util::TimeUtil::DurationToMilliseconds( @@ -48,7 +48,7 @@ TEST(UtilityTest, TranslateApiConfigSource) { EXPECT_EQ("test_rest_legacy_cluster", api_config_source_rest_legacy.cluster_name(0)); envoy::api::v2::ApiConfigSource api_config_source_rest; - Utility::translateApiConfigSource("test_rest_cluster", 20000, Utility::REST, + Utility::translateApiConfigSource("test_rest_cluster", 20000, ApiType::get().Rest, api_config_source_rest); EXPECT_EQ(envoy::api::v2::ApiConfigSource::REST, api_config_source_rest.api_type()); EXPECT_EQ(20000, Protobuf::util::TimeUtil::DurationToMilliseconds( @@ -56,7 +56,7 @@ TEST(UtilityTest, TranslateApiConfigSource) { EXPECT_EQ("test_rest_cluster", api_config_source_rest.cluster_name(0)); envoy::api::v2::ApiConfigSource api_config_source_grpc; - Utility::translateApiConfigSource("test_grpc_cluster", 30000, Utility::GRPC, + Utility::translateApiConfigSource("test_grpc_cluster", 30000, ApiType::get().Grpc, api_config_source_grpc); EXPECT_EQ(envoy::api::v2::ApiConfigSource::GRPC, api_config_source_grpc.api_type()); EXPECT_EQ(30000, Protobuf::util::TimeUtil::DurationToMilliseconds(