From 72bb8755556d265b334d05fa6422093c0db5743f Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 30 Dec 2019 13:49:12 -0500 Subject: [PATCH 01/17] config: distinct resource/transport API versions. This PR extends #9468 to support a distinct notion of transport and resource API version. The intuition here is that the opaque resources (and their type URLs) can be delivered via either v2 or v3 xDS, and the DiscoveryRequest etc. messages have their own versioning. Currently, the v2 and v3 transport protocols are indistinguishable modulo service endpoint. As v3 evolves, e.g. with #9301, differences will be introduced. At this point it will be necessary to have enhanced support in the gRPC mux and HTTP subscription modules to handle the protocol differences. This is technically a breaking v2 API change, but since the field it breaks was only added today, I think it's safe to assume it is not in use yet. Risk level: Low Testing: Integration tests added to validate service endpoint and type URL selection based on transport/resource version. Signed-off-by: Harvey Tuch --- api/envoy/api/v2/core/config_source.proto | 40 ++++--- .../api/v3alpha/core/config_source.proto | 40 ++++--- .../envoy/api/v2/core/config_source.proto | 40 ++++--- .../api/v3alpha/core/config_source.proto | 40 ++++--- source/common/config/BUILD | 4 + source/common/config/grpc_mux_impl.h | 8 +- source/common/config/http_subscription_impl.h | 3 +- source/common/config/new_grpc_mux_impl.h | 4 +- .../config/subscription_factory_impl.cc | 13 +- source/common/config/type_to_endpoint.cc | 70 +++++++---- source/common/config/type_to_endpoint.h | 14 ++- source/common/router/rds_impl.cc | 25 ++-- source/common/router/rds_impl.h | 8 +- source/common/router/scoped_rds.cc | 19 +-- source/common/router/scoped_rds.h | 3 +- source/common/router/vhds.cc | 28 ++--- source/common/router/vhds.h | 8 +- source/common/runtime/runtime_impl.cc | 16 +-- source/common/runtime/runtime_impl.h | 3 +- source/common/secret/sds_api.cc | 20 ++-- source/common/secret/sds_api.h | 3 +- source/common/upstream/cds_api_impl.cc | 19 +-- source/common/upstream/cds_api_impl.h | 3 +- source/common/upstream/eds.cc | 19 +-- source/common/upstream/eds.h | 3 +- source/server/lds_api.cc | 19 +-- source/server/lds_api.h | 3 +- test/integration/BUILD | 9 ++ .../api_version_integration_test.cc | 112 ++++++++++++++++++ 29 files changed, 396 insertions(+), 200 deletions(-) create mode 100644 test/integration/api_version_integration_test.cc diff --git a/api/envoy/api/v2/core/config_source.proto b/api/envoy/api/v2/core/config_source.proto index f9fa9c9ffbb07..596d5384a1c31 100644 --- a/api/envoy/api/v2/core/config_source.proto +++ b/api/envoy/api/v2/core/config_source.proto @@ -15,9 +15,24 @@ option java_multiple_files = true; // [#protodoc-title: Configuration sources] +// xDS API version. This is used to describe both resource and transport +// protocol versions (in distinct configuration fields). +enum ApiVersion { + // When not specified, we asssume v2, to ease migration to Envoy's stable API + // versioning. If a client does not support v2 (e.g. due to deprecation), this + // is an invalid value. + AUTO = 0; + + // Use xDS v2 API. + V2 = 1; + + // Use xDS v3alpha API. + V3ALPHA = 2; +} + // API configuration source. This identifies the API type and cluster that Envoy // will use to fetch an xDS API. -// [#next-free-field: 8] +// [#next-free-field: 9] message ApiConfigSource { // APIs may be fetched via either REST or gRPC. enum ApiType { @@ -43,8 +58,13 @@ message ApiConfigSource { DELTA_GRPC = 3; } + // API type (gRPC, REST, delta gRPC) ApiType api_type = 1 [(validate.rules).enum = {defined_only: true}]; + // API version for xDS transport protocol. This describes the xDS gRPC/REST + // endpoint and version of [Delta]DiscoveryRequest/Response used on the wire. + ApiVersion transport_api_version = 8 [(validate.rules).enum = {defined_only: true}]; + // Cluster names should be used only with REST. If > 1 // cluster is defined, clusters will be cycled through if any kind of failure // occurs. @@ -105,18 +125,6 @@ message RateLimitSettings { // inotify for updates. // [#next-free-field: 7] message ConfigSource { - enum XdsApiVersion { - // use for describing explicitly that xDS API version is set automatically. In default, xDS API - // version is V2 - AUTO = 0; - - // use xDS v2 API - V2 = 1; - - // use xDS v3alpha API - V3ALPHA = 2; - } - oneof config_source_specifier { option (validate.required) = true; @@ -163,6 +171,8 @@ message ConfigSource { // timeout applies). The default is 15s. google.protobuf.Duration initial_fetch_timeout = 4; - // API version for xDS endpoint - XdsApiVersion xds_api_version = 6; + // API version for xDS resources. This implies the type URLs that the client + // will request for resources and the resource type that the client will in + // turn expect to be delivered. + ApiVersion resource_api_version = 6 [(validate.rules).enum = {defined_only: true}]; } diff --git a/api/envoy/api/v3alpha/core/config_source.proto b/api/envoy/api/v3alpha/core/config_source.proto index 6cef3dd6864ad..5b6220e4f84d5 100644 --- a/api/envoy/api/v3alpha/core/config_source.proto +++ b/api/envoy/api/v3alpha/core/config_source.proto @@ -17,9 +17,24 @@ option java_multiple_files = true; // [#protodoc-title: Configuration sources] +// xDS API version. This is used to describe both resource and transport +// protocol versions (in distinct configuration fields). +enum ApiVersion { + // When not specified, we asssume v2, to ease migration to Envoy's stable API + // versioning. If a client does not support v2 (e.g. due to deprecation), this + // is an invalid value. + AUTO = 0; + + // Use xDS v2 API. + V2 = 1; + + // Use xDS v3alpha API. + V3ALPHA = 2; +} + // API configuration source. This identifies the API type and cluster that Envoy // will use to fetch an xDS API. -// [#next-free-field: 8] +// [#next-free-field: 9] message ApiConfigSource { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.ApiConfigSource"; @@ -47,8 +62,13 @@ message ApiConfigSource { DELTA_GRPC = 3; } + // API type (gRPC, REST, delta gRPC) ApiType api_type = 1 [(validate.rules).enum = {defined_only: true}]; + // API version for xDS transport protocol. This describes the xDS gRPC/REST + // endpoint and version of [Delta]DiscoveryRequest/Response used on the wire. + ApiVersion transport_api_version = 8 [(validate.rules).enum = {defined_only: true}]; + // Cluster names should be used only with REST. If > 1 // cluster is defined, clusters will be cycled through if any kind of failure // occurs. @@ -117,18 +137,6 @@ message RateLimitSettings { message ConfigSource { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.ConfigSource"; - enum XdsApiVersion { - // use for describing explicitly that xDS API version is set automatically. In default, xDS API - // version is V2 - AUTO = 0; - - // use xDS v2 API - V2 = 1; - - // use xDS v3alpha API - V3ALPHA = 2; - } - oneof config_source_specifier { option (validate.required) = true; @@ -175,6 +183,8 @@ message ConfigSource { // timeout applies). The default is 15s. google.protobuf.Duration initial_fetch_timeout = 4; - // API version for xDS endpoint - XdsApiVersion xds_api_version = 6; + // API version for xDS resources. This implies the type URLs that the client + // will request for resources and the resource type that the client will in + // turn expect to be delivered. + ApiVersion resource_api_version = 6 [(validate.rules).enum = {defined_only: true}]; } diff --git a/generated_api_shadow/envoy/api/v2/core/config_source.proto b/generated_api_shadow/envoy/api/v2/core/config_source.proto index f9fa9c9ffbb07..596d5384a1c31 100644 --- a/generated_api_shadow/envoy/api/v2/core/config_source.proto +++ b/generated_api_shadow/envoy/api/v2/core/config_source.proto @@ -15,9 +15,24 @@ option java_multiple_files = true; // [#protodoc-title: Configuration sources] +// xDS API version. This is used to describe both resource and transport +// protocol versions (in distinct configuration fields). +enum ApiVersion { + // When not specified, we asssume v2, to ease migration to Envoy's stable API + // versioning. If a client does not support v2 (e.g. due to deprecation), this + // is an invalid value. + AUTO = 0; + + // Use xDS v2 API. + V2 = 1; + + // Use xDS v3alpha API. + V3ALPHA = 2; +} + // API configuration source. This identifies the API type and cluster that Envoy // will use to fetch an xDS API. -// [#next-free-field: 8] +// [#next-free-field: 9] message ApiConfigSource { // APIs may be fetched via either REST or gRPC. enum ApiType { @@ -43,8 +58,13 @@ message ApiConfigSource { DELTA_GRPC = 3; } + // API type (gRPC, REST, delta gRPC) ApiType api_type = 1 [(validate.rules).enum = {defined_only: true}]; + // API version for xDS transport protocol. This describes the xDS gRPC/REST + // endpoint and version of [Delta]DiscoveryRequest/Response used on the wire. + ApiVersion transport_api_version = 8 [(validate.rules).enum = {defined_only: true}]; + // Cluster names should be used only with REST. If > 1 // cluster is defined, clusters will be cycled through if any kind of failure // occurs. @@ -105,18 +125,6 @@ message RateLimitSettings { // inotify for updates. // [#next-free-field: 7] message ConfigSource { - enum XdsApiVersion { - // use for describing explicitly that xDS API version is set automatically. In default, xDS API - // version is V2 - AUTO = 0; - - // use xDS v2 API - V2 = 1; - - // use xDS v3alpha API - V3ALPHA = 2; - } - oneof config_source_specifier { option (validate.required) = true; @@ -163,6 +171,8 @@ message ConfigSource { // timeout applies). The default is 15s. google.protobuf.Duration initial_fetch_timeout = 4; - // API version for xDS endpoint - XdsApiVersion xds_api_version = 6; + // API version for xDS resources. This implies the type URLs that the client + // will request for resources and the resource type that the client will in + // turn expect to be delivered. + ApiVersion resource_api_version = 6 [(validate.rules).enum = {defined_only: true}]; } diff --git a/generated_api_shadow/envoy/api/v3alpha/core/config_source.proto b/generated_api_shadow/envoy/api/v3alpha/core/config_source.proto index 877b5df4ac4d3..abf63d7e2753b 100644 --- a/generated_api_shadow/envoy/api/v3alpha/core/config_source.proto +++ b/generated_api_shadow/envoy/api/v3alpha/core/config_source.proto @@ -17,9 +17,24 @@ option java_multiple_files = true; // [#protodoc-title: Configuration sources] +// xDS API version. This is used to describe both resource and transport +// protocol versions (in distinct configuration fields). +enum ApiVersion { + // When not specified, we asssume v2, to ease migration to Envoy's stable API + // versioning. If a client does not support v2 (e.g. due to deprecation), this + // is an invalid value. + AUTO = 0; + + // Use xDS v2 API. + V2 = 1; + + // Use xDS v3alpha API. + V3ALPHA = 2; +} + // API configuration source. This identifies the API type and cluster that Envoy // will use to fetch an xDS API. -// [#next-free-field: 8] +// [#next-free-field: 9] message ApiConfigSource { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.ApiConfigSource"; @@ -47,8 +62,13 @@ message ApiConfigSource { DELTA_GRPC = 3; } + // API type (gRPC, REST, delta gRPC) ApiType api_type = 1 [(validate.rules).enum = {defined_only: true}]; + // API version for xDS transport protocol. This describes the xDS gRPC/REST + // endpoint and version of [Delta]DiscoveryRequest/Response used on the wire. + ApiVersion transport_api_version = 8 [(validate.rules).enum = {defined_only: true}]; + // Cluster names should be used only with REST. If > 1 // cluster is defined, clusters will be cycled through if any kind of failure // occurs. @@ -117,18 +137,6 @@ message RateLimitSettings { message ConfigSource { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.ConfigSource"; - enum XdsApiVersion { - // use for describing explicitly that xDS API version is set automatically. In default, xDS API - // version is V2 - AUTO = 0; - - // use xDS v2 API - V2 = 1; - - // use xDS v3alpha API - V3ALPHA = 2; - } - oneof config_source_specifier { option (validate.required) = true; @@ -175,6 +183,8 @@ message ConfigSource { // timeout applies). The default is 15s. google.protobuf.Duration initial_fetch_timeout = 4; - // API version for xDS endpoint - XdsApiVersion xds_api_version = 6; + // API version for xDS resources. This implies the type URLs that the client + // will request for resources and the resource type that the client will in + // turn expect to be delivered. + ApiVersion resource_api_version = 6 [(validate.rules).enum = {defined_only: true}]; } diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 9b61f27df4ca7..04a252755a69b 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -64,6 +64,7 @@ envoy_cc_library( srcs = ["delta_subscription_impl.cc"], hdrs = ["delta_subscription_impl.h"], deps = [ + ":api_version_lib", ":grpc_stream_lib", ":new_grpc_mux_lib", ":utility_lib", @@ -136,6 +137,7 @@ envoy_cc_library( srcs = ["grpc_mux_impl.cc"], hdrs = ["grpc_mux_impl.h"], deps = [ + ":api_version_lib", ":grpc_stream_lib", ":utility_lib", "//include/envoy/config:grpc_mux_interface", @@ -197,6 +199,7 @@ envoy_cc_library( "http_api_protos", ], deps = [ + ":api_version_lib", "//include/envoy/config:subscription_interface", "//include/envoy/event:dispatcher_interface", "//source/common/buffer:buffer_lib", @@ -299,6 +302,7 @@ envoy_cc_library( "//source/common/protobuf", "@envoy_api//envoy/api/v2:pkg_cc_proto", "@envoy_api//envoy/api/v2/auth:pkg_cc_proto", + "@envoy_api//envoy/api/v2/core:pkg_cc_proto", "@envoy_api//envoy/api/v2/route:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v2:pkg_cc_proto", ], diff --git a/source/common/config/grpc_mux_impl.h b/source/common/config/grpc_mux_impl.h index d37826a4a8710..ffde14bda6e58 100644 --- a/source/common/config/grpc_mux_impl.h +++ b/source/common/config/grpc_mux_impl.h @@ -13,6 +13,7 @@ #include "common/common/cleanup.h" #include "common/common/logger.h" +#include "common/config/api_version.h" #include "common/config/grpc_stream.h" #include "common/config/utility.h" @@ -59,7 +60,7 @@ class GrpcMuxImpl : public GrpcMux, void onDiscoveryResponse(std::unique_ptr&& message) override; void onWriteable() override; - GrpcStream& + GrpcStream& grpcStreamForTest() { return grpc_stream_; } @@ -102,7 +103,7 @@ class GrpcMuxImpl : public GrpcMux, // Watches on the returned resources for the API; std::list watches_; // Current DiscoveryRequest for API. - envoy::api::v2::DiscoveryRequest request_; + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) request_; // Paused via pause()? bool paused_{}; // Was a DiscoveryRequest elided during a pause? @@ -115,7 +116,8 @@ class GrpcMuxImpl : public GrpcMux, void queueDiscoveryRequest(const std::string& queue_item); void clearRequestQueue(); - GrpcStream grpc_stream_; + GrpcStream + grpc_stream_; const LocalInfo::LocalInfo& local_info_; const bool skip_subsequent_node_; bool first_stream_request_; diff --git a/source/common/config/http_subscription_impl.h b/source/common/config/http_subscription_impl.h index 14af5d8240c13..56fa4fda2ba24 100644 --- a/source/common/config/http_subscription_impl.h +++ b/source/common/config/http_subscription_impl.h @@ -4,6 +4,7 @@ #include "envoy/config/subscription.h" #include "envoy/event/dispatcher.h" +#include "common/config/api_version.h" #include "common/http/rest_api_fetcher.h" namespace Envoy { @@ -45,7 +46,7 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher, std::string path_; Protobuf::RepeatedPtrField resources_; - envoy::api::v2::DiscoveryRequest request_; + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) request_; Config::SubscriptionCallbacks& callbacks_; SubscriptionStats stats_; Event::Dispatcher& dispatcher_; diff --git a/source/common/config/new_grpc_mux_impl.h b/source/common/config/new_grpc_mux_impl.h index 4315138b0d448..3a8beb5704a7d 100644 --- a/source/common/config/new_grpc_mux_impl.h +++ b/source/common/config/new_grpc_mux_impl.h @@ -6,6 +6,7 @@ #include "envoy/config/subscription.h" #include "common/common/logger.h" +#include "common/config/api_version.h" #include "common/config/delta_subscription_state.h" #include "common/config/grpc_stream.h" #include "common/config/pausable_ack_queue.h" @@ -110,7 +111,8 @@ class NewGrpcMuxImpl : public GrpcMux, // the order of Envoy's dependency ordering). std::list subscription_ordering_; - GrpcStream + GrpcStream grpc_stream_; }; diff --git a/source/common/config/subscription_factory_impl.cc b/source/common/config/subscription_factory_impl.cc index 601efc7903355..a066547636f16 100644 --- a/source/common/config/subscription_factory_impl.cc +++ b/source/common/config/subscription_factory_impl.cc @@ -48,8 +48,9 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( result = std::make_unique( local_info_, cm_, api_config_source.cluster_names()[0], dispatcher_, random_, Utility::apiConfigSourceRefreshDelay(api_config_source), - Utility::apiConfigSourceRequestTimeout(api_config_source), restMethod(type_url), - callbacks, stats, Utility::configSourceInitialFetchTimeout(config), validation_visitor_); + Utility::apiConfigSourceRequestTimeout(api_config_source), + restMethod(type_url, api_config_source.transport_api_version()), callbacks, stats, + Utility::configSourceInitialFetchTimeout(config), validation_visitor_); break; case envoy::api::v2::core::ApiConfigSource::GRPC: result = std::make_unique( @@ -57,8 +58,8 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( Config::Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), api_config_source, scope) ->create(), - dispatcher_, random_, sotwGrpcMethod(type_url), type_url, callbacks, stats, scope, - Utility::parseRateLimitSettings(api_config_source), + dispatcher_, random_, sotwGrpcMethod(type_url, api_config_source.transport_api_version()), + type_url, callbacks, stats, scope, Utility::parseRateLimitSettings(api_config_source), Utility::configSourceInitialFetchTimeout(config), api_config_source.set_node_on_first_message_only()); break; @@ -69,8 +70,8 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( Config::Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), api_config_source, scope) ->create(), - dispatcher_, deltaGrpcMethod(type_url), random_, scope, - Utility::parseRateLimitSettings(api_config_source), local_info_), + dispatcher_, deltaGrpcMethod(type_url, api_config_source.transport_api_version()), + random_, scope, Utility::parseRateLimitSettings(api_config_source), local_info_), type_url, callbacks, stats, Utility::configSourceInitialFetchTimeout(config), false); break; } diff --git a/source/common/config/type_to_endpoint.cc b/source/common/config/type_to_endpoint.cc index 15e95d6d47eb6..c36be84310c66 100644 --- a/source/common/config/type_to_endpoint.cc +++ b/source/common/config/type_to_endpoint.cc @@ -9,6 +9,7 @@ #include "envoy/api/v2/srds.pb.h" #include "envoy/service/discovery/v2/rtds.pb.h" +#include "common/common/assert.h" #include "common/grpc/common.h" // API_NO_BOOST_FILE @@ -24,68 +25,87 @@ bool typeUrlIs(absl::string_view type_url, const Protobuf::Message& msg) { return Grpc::Common::typeUrl(msg.GetDescriptor()->full_name()) == type_url; } +std::string versionString(envoy::api::v2::core::ApiVersion transport_api_version) { + switch (transport_api_version) { + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: + return "v2"; + case envoy::api::v2::core::ApiVersion::V3ALPHA: + return "v3alpha"; + default: + NOT_REACHED_GCOVR_EXCL_LINE; + } +} + } // namespace -const Protobuf::MethodDescriptor& deltaGrpcMethod(absl::string_view type_url) { +const Protobuf::MethodDescriptor& +deltaGrpcMethod(absl::string_view type_url, + envoy::api::v2::core::ApiVersion transport_api_version) { std::string method_name = UnknownMethod; + const std::string version = versionString(transport_api_version); if (typeUrlIs(type_url, envoy::api::v2::RouteConfiguration())) { - method_name = "envoy.api.v2.RouteDiscoveryService.DeltaRoutes"; + method_name = "envoy.api." + version + ".RouteDiscoveryService.DeltaRoutes"; } else if (typeUrlIs(type_url, envoy::api::v2::ScopedRouteConfiguration())) { - method_name = "envoy.api.v2.ScopedRoutesDiscoveryService.DeltaScopedRoutes"; + method_name = "envoy.api." + version + ".ScopedRoutesDiscoveryService.DeltaScopedRoutes"; } else if (typeUrlIs(type_url, envoy::api::v2::route::VirtualHost())) { - method_name = "envoy.api.v2.VirtualHostDiscoveryService.DeltaVirtualHosts"; + method_name = "envoy.api." + version + ".VirtualHostDiscoveryService.DeltaVirtualHosts"; } else if (typeUrlIs(type_url, envoy::api::v2::auth::Secret())) { - method_name = "envoy.service.discovery.v2.SecretDiscoveryService.DeltaSecrets"; + method_name = "envoy.service.discovery." + version + ".SecretDiscoveryService.DeltaSecrets"; } else if (typeUrlIs(type_url, envoy::api::v2::Cluster())) { - method_name = "envoy.api.v2.ClusterDiscoveryService.DeltaClusters"; + method_name = "envoy.api." + version + ".ClusterDiscoveryService.DeltaClusters"; } else if (typeUrlIs(type_url, envoy::api::v2::ClusterLoadAssignment())) { - method_name = "envoy.api.v2.EndpointDiscoveryService.DeltaEndpoints"; + method_name = "envoy.api." + version + ".EndpointDiscoveryService.DeltaEndpoints"; } else if (typeUrlIs(type_url, envoy::api::v2::Listener())) { - method_name = "envoy.api.v2.ListenerDiscoveryService.DeltaListeners"; + method_name = "envoy.api." + version + ".ListenerDiscoveryService.DeltaListeners"; } else if (typeUrlIs(type_url, envoy::service::discovery::v2::Runtime())) { - method_name = "envoy.service.discovery.v2.RuntimeDiscoveryService.DeltaRuntime"; + method_name = "envoy.service.discovery." + version + ".RuntimeDiscoveryService.DeltaRuntime"; } ASSERT(method_name != UnknownMethod); return *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method_name); } -const Protobuf::MethodDescriptor& sotwGrpcMethod(absl::string_view type_url) { +const Protobuf::MethodDescriptor& +sotwGrpcMethod(absl::string_view type_url, envoy::api::v2::core::ApiVersion transport_api_version) { std::string method_name = UnknownMethod; + const std::string version = versionString(transport_api_version); if (typeUrlIs(type_url, envoy::api::v2::RouteConfiguration())) { - method_name = "envoy.api.v2.RouteDiscoveryService.StreamRoutes"; + method_name = "envoy.api." + version + ".RouteDiscoveryService.StreamRoutes"; } else if (typeUrlIs(type_url, envoy::api::v2::ScopedRouteConfiguration())) { - method_name = "envoy.api.v2.ScopedRoutesDiscoveryService.StreamScopedRoutes"; + method_name = "envoy.api." + version + ".ScopedRoutesDiscoveryService.StreamScopedRoutes"; } else if (typeUrlIs(type_url, envoy::api::v2::auth::Secret())) { - method_name = "envoy.service.discovery.v2.SecretDiscoveryService.StreamSecrets"; + method_name = "envoy.service.discovery." + version + ".SecretDiscoveryService.StreamSecrets"; } else if (typeUrlIs(type_url, envoy::api::v2::Cluster())) { - method_name = "envoy.api.v2.ClusterDiscoveryService.StreamClusters"; + method_name = "envoy.api." + version + ".ClusterDiscoveryService.StreamClusters"; } else if (typeUrlIs(type_url, envoy::api::v2::ClusterLoadAssignment())) { - method_name = "envoy.api.v2.EndpointDiscoveryService.StreamEndpoints"; + method_name = "envoy.api." + version + ".EndpointDiscoveryService.StreamEndpoints"; } else if (typeUrlIs(type_url, envoy::api::v2::Listener())) { - method_name = "envoy.api.v2.ListenerDiscoveryService.StreamListeners"; + method_name = "envoy.api." + version + ".ListenerDiscoveryService.StreamListeners"; } else if (typeUrlIs(type_url, envoy::service::discovery::v2::Runtime())) { - method_name = "envoy.service.discovery.v2.RuntimeDiscoveryService.StreamRuntime"; + method_name = "envoy.service.discovery." + version + ".RuntimeDiscoveryService.StreamRuntime"; } ASSERT(method_name != UnknownMethod); return *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method_name); } -const Protobuf::MethodDescriptor& restMethod(absl::string_view type_url) { +const Protobuf::MethodDescriptor& +restMethod(absl::string_view type_url, envoy::api::v2::core::ApiVersion transport_api_version) { std::string method_name = UnknownMethod; + const std::string version = versionString(transport_api_version); if (typeUrlIs(type_url, envoy::api::v2::RouteConfiguration())) { - method_name = "envoy.api.v2.RouteDiscoveryService.FetchRoutes"; + method_name = "envoy.api." + version + ".RouteDiscoveryService.FetchRoutes"; } else if (typeUrlIs(type_url, envoy::api::v2::ScopedRouteConfiguration())) { - method_name = "envoy.api.v2.ScopedRoutesDiscoveryService.FetchScopedRoutes"; + method_name = "envoy.api." + version + ".ScopedRoutesDiscoveryService.FetchScopedRoutes"; } else if (typeUrlIs(type_url, envoy::api::v2::auth::Secret())) { - method_name = "envoy.service.discovery.v2.SecretDiscoveryService.FetchSecrets"; + method_name = "envoy.service.discovery." + version + ".SecretDiscoveryService.FetchSecrets"; } else if (typeUrlIs(type_url, envoy::api::v2::Cluster())) { - method_name = "envoy.api.v2.ClusterDiscoveryService.FetchClusters"; + method_name = "envoy.api." + version + ".ClusterDiscoveryService.FetchClusters"; } else if (typeUrlIs(type_url, envoy::api::v2::ClusterLoadAssignment())) { - method_name = "envoy.api.v2.EndpointDiscoveryService.FetchEndpoints"; + method_name = "envoy.api." + version + ".EndpointDiscoveryService.FetchEndpoints"; } else if (typeUrlIs(type_url, envoy::api::v2::Listener())) { - method_name = "envoy.api.v2.ListenerDiscoveryService.FetchListeners"; + method_name = "envoy.api." + version + ".ListenerDiscoveryService.FetchListeners"; } else if (typeUrlIs(type_url, envoy::service::discovery::v2::Runtime())) { - method_name = "envoy.service.discovery.v2.RuntimeDiscoveryService.FetchRuntime"; + method_name = "envoy.service.discovery." + version + ".RuntimeDiscoveryService.FetchRuntime"; } ASSERT(method_name != UnknownMethod); return *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method_name); diff --git a/source/common/config/type_to_endpoint.h b/source/common/config/type_to_endpoint.h index c5ef2d7e7e013..59c3de0f434da 100644 --- a/source/common/config/type_to_endpoint.h +++ b/source/common/config/type_to_endpoint.h @@ -1,5 +1,7 @@ #pragma once +#include "envoy/api/v2/core/config_source.pb.h" + #include "common/protobuf/protobuf.h" #include "absl/strings/string_view.h" @@ -8,12 +10,18 @@ namespace Envoy { namespace Config { // Translates an xDS resource type_url to the name of the delta gRPC service that carries it. -const Protobuf::MethodDescriptor& deltaGrpcMethod(absl::string_view type_url); +const Protobuf::MethodDescriptor& +deltaGrpcMethod(absl::string_view resource_type_url, + envoy::api::v2::core::ApiVersion transport_api_version); // Translates an xDS resource type_url to the name of the state-of-the-world gRPC service that // carries it. -const Protobuf::MethodDescriptor& sotwGrpcMethod(absl::string_view type_url); +const Protobuf::MethodDescriptor& +sotwGrpcMethod(absl::string_view resource_type_url, + envoy::api::v2::core::ApiVersion transport_api_version); // Translates an xDS resource type_url to the name of the REST service that carries it. -const Protobuf::MethodDescriptor& restMethod(absl::string_view type_url); +const Protobuf::MethodDescriptor& +restMethod(absl::string_view resource_type_url, + envoy::api::v2::core::ApiVersion transport_api_version); } // namespace Config } // namespace Envoy diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index f8c01afd6a8c6..c022fa3bea904 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -62,8 +62,7 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( const uint64_t manager_identifier, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, Init::Manager& init_manager, const std::string& stat_prefix, - Envoy::Router::RouteConfigProviderManagerImpl& route_config_provider_manager, - envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version) + Envoy::Router::RouteConfigProviderManagerImpl& route_config_provider_manager) : route_config_name_(rds.route_config_name()), factory_context_(factory_context), validator_(validator), init_manager_(init_manager), init_target_(fmt::format("RdsRouteConfigSubscription {}", route_config_name_), @@ -71,11 +70,12 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( scope_(factory_context.scope().createScope(stat_prefix + "rds." + route_config_name_ + ".")), stat_prefix_(stat_prefix), stats_({ALL_RDS_STATS(POOL_COUNTER(*scope_))}), route_config_provider_manager_(route_config_provider_manager), - manager_identifier_(manager_identifier), xds_api_version_(xds_api_version) { + manager_identifier_(manager_identifier) { subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( - rds.config_source(), loadTypeUrl(), *scope_, *this); + rds.config_source(), loadTypeUrl(rds.config_source().resource_api_version()), *scope_, + *this); config_update_info_ = std::make_unique(factory_context.timeSource(), validator); } @@ -122,7 +122,7 @@ void RdsRouteConfigSubscription::onConfigUpdate( // the listener might have been torn down, need to remove this. vhds_subscription_ = std::make_unique( config_update_info_, factory_context_, stat_prefix_, route_config_providers_, - config_update_info_->routeConfiguration().vhds().config_source().xds_api_version()); + config_update_info_->routeConfiguration().vhds().config_source().resource_api_version()); vhds_subscription_->registerInitTargetWithInitManager( noop_init_manager == nullptr ? getRdsConfigInitManager() : *noop_init_manager); } else { @@ -201,18 +201,19 @@ bool RdsRouteConfigSubscription::validateUpdateSize(int num_resources) { return true; } -std::string RdsRouteConfigSubscription::loadTypeUrl() { - switch (xds_api_version_) { +std::string +RdsRouteConfigSubscription::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { + switch (resource_api_version) { // automatically set api version as V2 - case envoy::api::v2::core::ConfigSource::AUTO: - case envoy::api::v2::core::ConfigSource::V2: + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name())); - case envoy::api::v2::core::ConfigSource::V3ALPHA: + case envoy::api::v2::core::ApiVersion::V3ALPHA: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v3alpha::RouteConfiguration().GetDescriptor()->full_name())); default: - throw EnvoyException(fmt::format("type {} is not supported", xds_api_version_)); + NOT_REACHED_GCOVR_EXCL_LINE; } } @@ -289,7 +290,7 @@ Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::createRdsRo RdsRouteConfigSubscriptionSharedPtr subscription(new RdsRouteConfigSubscription( rds, manager_identifier, factory_context.getServerFactoryContext(), factory_context.messageValidationVisitor(), factory_context.initManager(), stat_prefix, - *this, rds.config_source().xds_api_version())); + *this)); init_manager.add(subscription->init_target_); std::shared_ptr new_provider{ new RdsRouteConfigProviderImpl(std::move(subscription), factory_context)}; diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index ef542c3a2e79e..db4bf3478a061 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -140,12 +140,11 @@ class RdsRouteConfigSubscription : Envoy::Config::SubscriptionCallbacks, const uint64_t manager_identifier, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, Init::Manager& init_manager, - const std::string& stat_prefix, RouteConfigProviderManagerImpl& route_config_provider_manager, - const envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version = - envoy::api::v2::core::ConfigSource::AUTO); + const std::string& stat_prefix, + RouteConfigProviderManagerImpl& route_config_provider_manager); bool validateUpdateSize(int num_resources); - std::string loadTypeUrl(); + static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); Init::Manager& getRdsConfigInitManager() { return init_manager_; } @@ -165,7 +164,6 @@ class RdsRouteConfigSubscription : Envoy::Config::SubscriptionCallbacks, VhdsSubscriptionPtr vhds_subscription_; RouteConfigUpdatePtr config_update_info_; Common::CallbackManager<> update_callback_manager_; - envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version_; friend class RouteConfigProviderManagerImpl; // Access to addUpdateCallback diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index c62d15e181cdc..316ce5e53cc6f 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -100,11 +100,11 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription( stats_({ALL_SCOPED_RDS_STATS(POOL_COUNTER(*scope_))}), rds_config_source_(std::move(rds_config_source)), validation_visitor_(factory_context.messageValidationVisitor()), stat_prefix_(stat_prefix), - route_config_provider_manager_(route_config_provider_manager), - xds_api_version_(rds_config_source_.xds_api_version()) { + route_config_provider_manager_(route_config_provider_manager) { subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( - scoped_rds.scoped_rds_config_source(), loadTypeUrl(), *scope_, *this); + scoped_rds.scoped_rds_config_source(), + loadTypeUrl(rds_config_source_.resource_api_version()), *scope_, *this); initialize([scope_key_builder]() -> Envoy::Config::ConfigProvider::ConfigConstSharedPtr { return std::make_shared( @@ -343,18 +343,19 @@ void ScopedRdsConfigSubscription::onConfigUpdate( onConfigUpdate(to_add_repeated, to_remove_repeated, version_info); } -std::string ScopedRdsConfigSubscription::loadTypeUrl() { - switch (xds_api_version_) { +std::string +ScopedRdsConfigSubscription::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { + switch (resource_api_version) { // automatically set api version as V2 - case envoy::api::v2::core::ConfigSource::AUTO: - case envoy::api::v2::core::ConfigSource::V2: + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name())); - case envoy::api::v2::core::ConfigSource::V3ALPHA: + case envoy::api::v2::core::ApiVersion::V3ALPHA: return Grpc::Common::typeUrl(API_NO_BOOST( envoy::service::route::v3alpha::ScopedRouteConfiguration().GetDescriptor()->full_name())); default: - throw EnvoyException(fmt::format("type {} is not supported", xds_api_version_)); + NOT_REACHED_GCOVR_EXCL_LINE; } } diff --git a/source/common/router/scoped_rds.h b/source/common/router/scoped_rds.h index d7d70effe79ce..07d38ebd4a07c 100644 --- a/source/common/router/scoped_rds.h +++ b/source/common/router/scoped_rds.h @@ -160,7 +160,7 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - std::string loadTypeUrl(); + static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); // Propagate RDS updates to ScopeConfigImpl in workers. void onRdsConfigUpdate(const std::string& scope_name, RdsRouteConfigSubscription& rds_subscription); @@ -184,7 +184,6 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio ProtobufMessage::ValidationVisitor& validation_visitor_; const std::string stat_prefix_; RouteConfigProviderManager& route_config_provider_manager_; - envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version_; }; using ScopedRdsConfigSubscriptionSharedPtr = std::shared_ptr; diff --git a/source/common/router/vhds.cc b/source/common/router/vhds.cc index 339a79fc80a27..717b50c43087b 100644 --- a/source/common/router/vhds.cc +++ b/source/common/router/vhds.cc @@ -21,18 +21,18 @@ namespace Envoy { namespace Router { // Implements callbacks to handle DeltaDiscovery protocol for VirtualHostDiscoveryService -VhdsSubscription::VhdsSubscription( - RouteConfigUpdatePtr& config_update_info, - Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, - std::unordered_set& route_config_providers, - envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version) +VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, + Server::Configuration::ServerFactoryContext& factory_context, + const std::string& stat_prefix, + std::unordered_set& route_config_providers, + envoy::api::v2::core::ApiVersion resource_api_version) : config_update_info_(config_update_info), scope_(factory_context.scope().createScope(stat_prefix + "vhds." + config_update_info_->routeConfigName() + ".")), stats_({ALL_VHDS_STATS(POOL_COUNTER(*scope_))}), init_target_(fmt::format("VhdsConfigSubscription {}", config_update_info_->routeConfigName()), [this]() { subscription_->start({}); }), - route_config_providers_(route_config_providers), xds_api_version_(xds_api_version) { + route_config_providers_(route_config_providers), resource_api_version_(resource_api_version) { const auto& config_source = config_update_info_->routeConfiguration() .vhds() .config_source() @@ -44,8 +44,8 @@ VhdsSubscription::VhdsSubscription( subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( - config_update_info_->routeConfiguration().vhds().config_source(), loadTypeUrl(), *scope_, - *this); + config_update_info_->routeConfiguration().vhds().config_source(), + loadTypeUrl(resource_api_version_), *scope_, *this); } void VhdsSubscription::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason reason, @@ -72,18 +72,18 @@ void VhdsSubscription::onConfigUpdate( init_target_.ready(); } -std::string VhdsSubscription::loadTypeUrl() { - switch (xds_api_version_) { +std::string VhdsSubscription::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { + switch (resource_api_version) { // automatically set api version as V2 - case envoy::api::v2::core::ConfigSource::AUTO: - case envoy::api::v2::core::ConfigSource::V2: + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name())); - case envoy::api::v2::core::ConfigSource::V3ALPHA: + case envoy::api::v2::core::ApiVersion::V3ALPHA: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name())); default: - throw EnvoyException(fmt::format("type {} is not supported", xds_api_version_)); + NOT_REACHED_GCOVR_EXCL_LINE; } } } // namespace Router diff --git a/source/common/router/vhds.h b/source/common/router/vhds.h index 45d95c806890f..6e351a5f70e0f 100644 --- a/source/common/router/vhds.h +++ b/source/common/router/vhds.h @@ -41,8 +41,8 @@ class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, std::unordered_set& route_config_providers, - const envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version = - envoy::api::v2::core::ConfigSource::AUTO); + const envoy::api::v2::core::ApiVersion xds_api_version = + envoy::api::v2::core::ApiVersion::AUTO); ~VhdsSubscription() override { init_target_.ready(); } void registerInitTargetWithInitManager(Init::Manager& m) { m.add(init_target_); } @@ -60,7 +60,7 @@ class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - std::string loadTypeUrl(); + static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); RouteConfigUpdatePtr& config_update_info_; Stats::ScopePtr scope_; @@ -68,7 +68,7 @@ class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, std::unique_ptr subscription_; Init::TargetImpl init_target_; std::unordered_set& route_config_providers_; - envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version_; + envoy::api::v2::core::ApiVersion resource_api_version_; }; using VhdsSubscriptionPtr = std::unique_ptr; diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index acc7e2cbe8e2a..bc18fc18312db 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -552,7 +552,7 @@ RtdsSubscription::RtdsSubscription( : parent_(parent), config_source_(rtds_layer.rtds_config()), store_(store), resource_name_(rtds_layer.name()), init_target_("RTDS " + resource_name_, [this]() { start(); }), - validation_visitor_(validation_visitor), xds_api_version_(config_source_.xds_api_version()) {} + validation_visitor_(validation_visitor) {} void RtdsSubscription::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const std::string&) { @@ -591,7 +591,7 @@ void RtdsSubscription::start() { // cluster manager resources are not available in the constructor when // instantiated in the server instance. subscription_ = parent_.cm_->subscriptionFactory().subscriptionFromConfigSource( - config_source_, loadTypeUrl(), store_, *this); + config_source_, loadTypeUrl(config_source_.resource_api_version()), store_, *this); subscription_->start({resource_name_}); } @@ -603,18 +603,18 @@ void RtdsSubscription::validateUpdateSize(uint32_t num_resources) { } } -std::string RtdsSubscription::loadTypeUrl() { - switch (xds_api_version_) { +std::string RtdsSubscription::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { + switch (resource_api_version) { // automatically set api version as V2 - case envoy::api::v2::core::ConfigSource::AUTO: - case envoy::api::v2::core::ConfigSource::V2: + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::service::discovery::v2::Runtime().GetDescriptor()->full_name())); - case envoy::api::v2::core::ConfigSource::V3ALPHA: + case envoy::api::v2::core::ApiVersion::V3ALPHA: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::service::discovery::v3alpha::Runtime().GetDescriptor()->full_name())); default: - throw EnvoyException(fmt::format("type {} is not supported", xds_api_version_)); + NOT_REACHED_GCOVR_EXCL_LINE; } } diff --git a/source/common/runtime/runtime_impl.h b/source/common/runtime/runtime_impl.h index 548b5ee6b0220..3e038bf619725 100644 --- a/source/common/runtime/runtime_impl.h +++ b/source/common/runtime/runtime_impl.h @@ -219,7 +219,7 @@ struct RtdsSubscription : Config::SubscriptionCallbacks, Logger::Loggable; diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index c0b2fa7642778..39b501bf9efc6 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -7,6 +7,7 @@ #include "envoy/api/v2/core/config_source.pb.h" #include "envoy/api/v2/discovery.pb.h" +#include "common/common/assert.h" #include "common/config/api_version.h" #include "common/config/resources.h" #include "common/protobuf/utility.h" @@ -23,8 +24,7 @@ SdsApi::SdsApi(envoy::api::v2::core::ConfigSource sds_config, absl::string_view secret_hash_(0), clean_up_(std::move(destructor_cb)), validation_visitor_(validation_visitor), subscription_factory_(subscription_factory), time_source_(time_source), secret_data_{sds_config_name_, "uninitialized", - time_source_.systemTime()}, - xds_api_version_(sds_config_.xds_api_version()) { + time_source_.systemTime()} { // TODO(JimmyCYJ): Implement chained_init_manager, so that multiple init_manager // can be chained together to behave as one init_manager. In that way, we let // two listeners which share same SdsApi to register at separate init managers, and @@ -81,23 +81,23 @@ void SdsApi::validateUpdateSize(int num_resources) { } void SdsApi::initialize() { - subscription_ = - subscription_factory_.subscriptionFromConfigSource(sds_config_, loadTypeUrl(), stats_, *this); + subscription_ = subscription_factory_.subscriptionFromConfigSource( + sds_config_, loadTypeUrl(sds_config_.resource_api_version()), stats_, *this); subscription_->start({sds_config_name_}); } -std::string SdsApi::loadTypeUrl() { - switch (xds_api_version_) { +std::string SdsApi::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { + switch (resource_api_version) { // automatically set api version as V2 - case envoy::api::v2::core::ConfigSource::AUTO: - case envoy::api::v2::core::ConfigSource::V2: + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::auth::Secret().GetDescriptor()->full_name())); - case envoy::api::v2::core::ConfigSource::V3ALPHA: + case envoy::api::v2::core::ApiVersion::V3ALPHA: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::auth::Secret().GetDescriptor()->full_name())); default: - throw EnvoyException(fmt::format("type {} is not supported", xds_api_version_)); + NOT_REACHED_GCOVR_EXCL_LINE; } } diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index 5e4bfadaed0b2..04e5f806e48fe 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -62,7 +62,7 @@ class SdsApi : public Config::SubscriptionCallbacks { std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - std::string loadTypeUrl(); + static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); private: void validateUpdateSize(int num_resources); @@ -80,7 +80,6 @@ class SdsApi : public Config::SubscriptionCallbacks { Config::SubscriptionFactory& subscription_factory_; TimeSource& time_source_; SecretData secret_data_; - envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version_; }; class TlsCertificateSdsApi; diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index fb7c252591567..e82f169ba3b08 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -9,6 +9,7 @@ #include "envoy/api/v3alpha/cds.pb.h" #include "envoy/stats/scope.h" +#include "common/common/assert.h" #include "common/common/cleanup.h" #include "common/common/utility.h" #include "common/config/api_version.h" @@ -30,9 +31,9 @@ CdsApiPtr CdsApiImpl::create(const envoy::api::v2::core::ConfigSource& cds_confi CdsApiImpl::CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm, Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor) : cm_(cm), scope_(scope.createScope("cluster_manager.cds.")), - validation_visitor_(validation_visitor), xds_api_version_(cds_config.xds_api_version()) { - subscription_ = cm_.subscriptionFactory().subscriptionFromConfigSource(cds_config, loadTypeUrl(), - *scope_, *this); + validation_visitor_(validation_visitor) { + subscription_ = cm_.subscriptionFactory().subscriptionFromConfigSource( + cds_config, loadTypeUrl(cds_config.resource_api_version()), *scope_, *this); } void CdsApiImpl::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, @@ -125,18 +126,18 @@ void CdsApiImpl::runInitializeCallbackIfAny() { } } -std::string CdsApiImpl::loadTypeUrl() { - switch (xds_api_version_) { +std::string CdsApiImpl::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { + switch (resource_api_version) { // automatically set api version as V2 - case envoy::api::v2::core::ConfigSource::AUTO: - case envoy::api::v2::core::ConfigSource::V2: + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::Cluster().GetDescriptor()->full_name())); - case envoy::api::v2::core::ConfigSource::V3ALPHA: + case envoy::api::v2::core::ApiVersion::V3ALPHA: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v3alpha::Cluster().GetDescriptor()->full_name())); default: - throw EnvoyException(fmt::format("type {} is not supported", xds_api_version_)); + NOT_REACHED_GCOVR_EXCL_LINE; } } diff --git a/source/common/upstream/cds_api_impl.h b/source/common/upstream/cds_api_impl.h index e83a03cfbadfc..15a35e456607d 100644 --- a/source/common/upstream/cds_api_impl.h +++ b/source/common/upstream/cds_api_impl.h @@ -47,7 +47,7 @@ class CdsApiImpl : public CdsApi, std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - std::string loadTypeUrl(); + static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm, Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor); void runInitializeCallbackIfAny(); @@ -58,7 +58,6 @@ class CdsApiImpl : public CdsApi, std::function initialize_callback_; Stats::ScopePtr scope_; ProtobufMessage::ValidationVisitor& validation_visitor_; - envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version_; }; } // namespace Upstream diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index 4af57d49ac8d5..d5e9645fb9bd9 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -8,6 +8,7 @@ #include "envoy/api/v3alpha/cds.pb.h" #include "envoy/common/exception.h" +#include "common/common/assert.h" #include "common/common/utility.h" #include "common/config/api_version.h" @@ -24,8 +25,7 @@ EdsClusterImpl::EdsClusterImpl( cluster_name_(cluster.eds_cluster_config().service_name().empty() ? cluster.name() : cluster.eds_cluster_config().service_name()), - validation_visitor_(factory_context.messageValidationVisitor()), - xds_api_version_(cluster.eds_cluster_config().eds_config().xds_api_version()) { + validation_visitor_(factory_context.messageValidationVisitor()) { Event::Dispatcher& dispatcher = factory_context.dispatcher(); assignment_timeout_ = dispatcher.createTimer([this]() -> void { onAssignmentTimeout(); }); const auto& eds_config = cluster.eds_cluster_config().eds_config(); @@ -37,7 +37,8 @@ EdsClusterImpl::EdsClusterImpl( } subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( - eds_config, loadTypeUrl(), info_->statsScope(), *this); + eds_config, loadTypeUrl(cluster.eds_cluster_config().eds_config().resource_api_version()), + info_->statsScope(), *this); } void EdsClusterImpl::startPreInit() { subscription_->start({cluster_name_}); } @@ -217,18 +218,18 @@ void EdsClusterImpl::reloadHealthyHostsHelper(const HostSharedPtr& host) { } } -std::string EdsClusterImpl::loadTypeUrl() { - switch (xds_api_version_) { +std::string EdsClusterImpl::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { + switch (resource_api_version) { // automatically set api version as V2 - case envoy::api::v2::core::ConfigSource::AUTO: - case envoy::api::v2::core::ConfigSource::V2: + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::ClusterLoadAssignment().GetDescriptor()->full_name())); - case envoy::api::v2::core::ConfigSource::V3ALPHA: + case envoy::api::v2::core::ApiVersion::V3ALPHA: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v3alpha::ClusterLoadAssignment().GetDescriptor()->full_name())); default: - throw EnvoyException(fmt::format("type {} is not supported", xds_api_version_)); + NOT_REACHED_GCOVR_EXCL_LINE; } } diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 909c2cfbd9842..91228336ff556 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -43,7 +43,7 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallba std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).cluster_name(); } - std::string loadTypeUrl(); + static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); using LocalityWeightsMap = std::unordered_map; bool updateHostsPerLocality(const uint32_t priority, const uint32_t overprovisioning_factor, @@ -80,7 +80,6 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallba Event::TimerPtr assignment_timeout_; ProtobufMessage::ValidationVisitor& validation_visitor_; InitializePhase initialize_phase_; - envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version_; }; class EdsClusterFactory : public ClusterFactoryImplBase { diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index 5e72ba513602c..ef0270adf582f 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -10,6 +10,7 @@ #include "envoy/api/v3alpha/lds.pb.h" #include "envoy/stats/scope.h" +#include "common/common/assert.h" #include "common/common/cleanup.h" #include "common/config/api_version.h" #include "common/config/resources.h" @@ -27,9 +28,9 @@ LdsApiImpl::LdsApiImpl(const envoy::api::v2::core::ConfigSource& lds_config, ProtobufMessage::ValidationVisitor& validation_visitor) : listener_manager_(lm), scope_(scope.createScope("listener_manager.lds.")), cm_(cm), init_target_("LDS", [this]() { subscription_->start({}); }), - validation_visitor_(validation_visitor), xds_api_version_(lds_config.xds_api_version()) { - subscription_ = cm.subscriptionFactory().subscriptionFromConfigSource(lds_config, loadTypeUrl(), - *scope_, *this); + validation_visitor_(validation_visitor) { + subscription_ = cm.subscriptionFactory().subscriptionFromConfigSource( + lds_config, loadTypeUrl(lds_config.resource_api_version()), *scope_, *this); init_manager.add(init_target_); } @@ -131,18 +132,18 @@ void LdsApiImpl::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason r init_target_.ready(); } -std::string LdsApiImpl::loadTypeUrl() { - switch (xds_api_version_) { +std::string LdsApiImpl::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { + switch (resource_api_version) { // automatically set api version as V2 - case envoy::api::v2::core::ConfigSource::AUTO: - case envoy::api::v2::core::ConfigSource::V2: + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::Listener().GetDescriptor()->full_name())); - case envoy::api::v2::core::ConfigSource::V3ALPHA: + case envoy::api::v2::core::ApiVersion::V3ALPHA: return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v3alpha::Listener().GetDescriptor()->full_name())); default: - throw EnvoyException(fmt::format("type {} is not supported", xds_api_version_)); + NOT_REACHED_GCOVR_EXCL_LINE; } } } // namespace Server diff --git a/source/server/lds_api.h b/source/server/lds_api.h index 4d7835aee3fbb..8a9fd51359100 100644 --- a/source/server/lds_api.h +++ b/source/server/lds_api.h @@ -44,7 +44,7 @@ class LdsApiImpl : public LdsApi, std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - std::string loadTypeUrl(); + static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); std::unique_ptr subscription_; std::string system_version_info_; @@ -53,7 +53,6 @@ class LdsApiImpl : public LdsApi, Upstream::ClusterManager& cm_; Init::TargetImpl init_target_; ProtobufMessage::ValidationVisitor& validation_visitor_; - envoy::api::v2::core::ConfigSource::XdsApiVersion xds_api_version_; }; } // namespace Server diff --git a/test/integration/BUILD b/test/integration/BUILD index 6d01661a7e7cf..381240b431313 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -58,6 +58,15 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "api_version_integration_test", + srcs = ["api_version_integration_test.cc"], + deps = [ + ":http_integration_lib", + "@envoy_api//envoy/api/v2/core:pkg_cc_proto", + ], +) + py_binary( name = "capture_fuzz_gen", srcs = ["capture_fuzz_gen.py"], diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc new file mode 100644 index 0000000000000..29c1ad12fc935 --- /dev/null +++ b/test/integration/api_version_integration_test.cc @@ -0,0 +1,112 @@ +#include "envoy/api/v2/core/config_source.pb.h" + +#include "test/integration/http_integration.h" + +namespace Envoy { +namespace { + +class ApiVersionIntegrationTest + : public testing::TestWithParam< + std::tuple>, + public HttpIntegrationTest { +public: + ApiVersionIntegrationTest() : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, ipVersion()) { + use_lds_ = false; + create_xds_upstream_ = true; + tls_xds_upstream_ = false; + skipPortUsageValidation(); + } + + Network::Address::IpVersion ipVersion() const { return std::get<0>(GetParam()); } + envoy::api::v2::core::ApiConfigSource::ApiType apiType() const { return std::get<1>(GetParam()); } + envoy::api::v2::core::ApiVersion resourceApiVersion() const { return std::get<2>(GetParam()); } + envoy::api::v2::core::ApiVersion transportApiVersion() const { return std::get<3>(GetParam()); } + + void initialize() override { + setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); + HttpIntegrationTest::initialize(); + if (xds_stream_ == nullptr) { + createXdsConnection(); + AssertionResult result = xds_connection_->waitForNewStream(*dispatcher_, xds_stream_); + RELEASE_ASSERT(result, result.message()); + result = xds_stream_->waitForHeadersComplete(); + RELEASE_ASSERT(result, result.message()); + endpoint_ = std::string(xds_stream_->headers().Path()->value().getStringView()); + ENVOY_LOG_MISC(debug, "xDS endpoint {}", endpoint_); + xds_stream_->startGrpcStream(); + } + } + + AssertionResult validateDiscoveryRequest(const std::string& expected_v2_sotw_endpoint, + const std::string& expected_v2_delta_endpoint, + const std::string& expected_type_url) { + std::string expected_endpoint; + std::string actual_type_url; + if (sotw_or_delta_ == Grpc::SotwOrDelta::Sotw) { + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; + VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); + expected_endpoint = expected_v2_sotw_endpoint; + actual_type_url = discovery_request.type_url(); + } else { + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) delta_discovery_request; + VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, delta_discovery_request)); + expected_endpoint = expected_v2_delta_endpoint; + actual_type_url = delta_discovery_request.type_url(); + } + if (endpoint_ != expected_endpoint) { + return AssertionFailure() << "Expected endpoint " << expected_endpoint << ", got " + << endpoint_; + } + if (expected_type_url != actual_type_url) { + return AssertionFailure() << "Expected type URL " << expected_type_url << ", got " + << actual_type_url; + } + return AssertionSuccess(); + } + + void TearDown() override { + cleanUpXdsConnection(); + test_server_.reset(); + fake_upstreams_.clear(); + } + + std::string endpoint_; +}; + +INSTANTIATE_TEST_SUITE_P( + IpVersionsApiConfigSourcesApiVersions, ApiVersionIntegrationTest, + testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + testing::Values(envoy::api::v2::core::ApiConfigSource::REST, + envoy::api::v2::core::ApiConfigSource::GRPC, + envoy::api::v2::core::ApiConfigSource::DELTA_GRPC), + testing::Values(envoy::api::v2::core::ApiVersion::AUTO, + envoy::api::v2::core::ApiVersion::V2, + envoy::api::v2::core::ApiVersion::V3ALPHA), + testing::Values(envoy::api::v2::core::ApiVersion::AUTO, + envoy::api::v2::core::ApiVersion::V2, + envoy::api::v2::core::ApiVersion::V3ALPHA))); + +TEST_P(ApiVersionIntegrationTest, Lds) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + bootstrap.mutable_static_resources()->clear_listeners(); + auto* lds_config = bootstrap.mutable_dynamic_resources()->mutable_lds_config(); + lds_config->set_resource_api_version(resourceApiVersion()); + auto* api_config_source = lds_config->mutable_api_config_source(); + api_config_source->set_transport_api_version(transportApiVersion()); + api_config_source->set_api_type(apiType()); + auto* grpc_service = api_config_source->add_grpc_services(); + grpc_service->mutable_envoy_grpc()->set_cluster_name("lds_cluster"); + auto* lds_cluster = bootstrap.mutable_static_resources()->add_clusters(); + lds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); + lds_cluster->set_name("lds_cluster"); + lds_cluster->mutable_http2_protocol_options(); + }); + initialize(); + ASSERT_TRUE(validateDiscoveryRequest("/envoy.api.v2.ListenerDiscoveryService/StreamListeners", + "/envoy.api.v2.ListenerDiscoveryService/DeltaListeners", + "type.googleapis.com/envoy.api.v2.Listener")); +} + +} // namespace +} // namespace Envoy From b23f4c60f402bb9c4dcdc08f65278ff596abb26e Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 30 Dec 2019 22:58:36 -0500 Subject: [PATCH 02/17] Type-to-endpoint support for mixed resource/transport versions. Signed-off-by: Harvey Tuch --- source/common/config/BUILD | 6 ++ source/common/config/type_to_endpoint.cc | 99 ++++++++++++++----- .../api_version_integration_test.cc | 86 +++++++++++++--- 3 files changed, 149 insertions(+), 42 deletions(-) diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 04a252755a69b..4b8340b567d40 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -305,6 +305,12 @@ envoy_cc_library( "@envoy_api//envoy/api/v2/core:pkg_cc_proto", "@envoy_api//envoy/api/v2/route:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v2:pkg_cc_proto", + "@envoy_api//envoy/api/v3alpha:pkg_cc_proto", + "@envoy_api//envoy/api/v3alpha/auth:pkg_cc_proto", + "@envoy_api//envoy/api/v3alpha/core:pkg_cc_proto", + "@envoy_api//envoy/api/v3alpha/route:pkg_cc_proto", + "@envoy_api//envoy/service/discovery/v3alpha:pkg_cc_proto", + "@envoy_api//envoy/service/route/v3alpha:pkg_cc_proto", ], ) diff --git a/source/common/config/type_to_endpoint.cc b/source/common/config/type_to_endpoint.cc index c36be84310c66..909cc6c7980cf 100644 --- a/source/common/config/type_to_endpoint.cc +++ b/source/common/config/type_to_endpoint.cc @@ -9,10 +9,22 @@ #include "envoy/api/v2/srds.pb.h" #include "envoy/service/discovery/v2/rtds.pb.h" +#include "envoy/api/v3alpha/auth/cert.pb.h" +#include "envoy/api/v3alpha/cds.pb.h" +#include "envoy/api/v3alpha/eds.pb.h" +#include "envoy/api/v3alpha/lds.pb.h" +#include "envoy/api/v3alpha/rds.pb.h" +#include "envoy/api/v3alpha/route/route.pb.h" +#include "envoy/service/route/v3alpha/srds.pb.h" +#include "envoy/service/discovery/v3alpha/rtds.pb.h" + #include "common/common/assert.h" #include "common/grpc/common.h" // API_NO_BOOST_FILE +// TODO(htuch): the cross product of transport, resource version and type is +// kind of crazy, we should use API annotations to link resources to their +// service methods. See https://github.com/envoyproxy/envoy/issues/9454. namespace Envoy { namespace Config { @@ -21,8 +33,10 @@ const char UnknownMethod[] = "could_not_lookup_method_due_to_unknown_type_url"; namespace { -bool typeUrlIs(absl::string_view type_url, const Protobuf::Message& msg) { - return Grpc::Common::typeUrl(msg.GetDescriptor()->full_name()) == type_url; +bool typeUrlIs(absl::string_view type_url, const Protobuf::Message& v2_msg, + const Protobuf::Message& v3_msg) { + return Grpc::Common::typeUrl(v2_msg.GetDescriptor()->full_name()) == type_url || + Grpc::Common::typeUrl(v3_msg.GetDescriptor()->full_name()) == type_url; } std::string versionString(envoy::api::v2::core::ApiVersion transport_api_version) { @@ -44,21 +58,32 @@ deltaGrpcMethod(absl::string_view type_url, envoy::api::v2::core::ApiVersion transport_api_version) { std::string method_name = UnknownMethod; const std::string version = versionString(transport_api_version); - if (typeUrlIs(type_url, envoy::api::v2::RouteConfiguration())) { + if (typeUrlIs(type_url, envoy::api::v2::RouteConfiguration(), + envoy::api::v3alpha::RouteConfiguration())) { method_name = "envoy.api." + version + ".RouteDiscoveryService.DeltaRoutes"; - } else if (typeUrlIs(type_url, envoy::api::v2::ScopedRouteConfiguration())) { - method_name = "envoy.api." + version + ".ScopedRoutesDiscoveryService.DeltaScopedRoutes"; - } else if (typeUrlIs(type_url, envoy::api::v2::route::VirtualHost())) { + } else if (typeUrlIs(type_url, envoy::api::v2::ScopedRouteConfiguration(), + envoy::service::route::v3alpha::ScopedRouteConfiguration())) { + if (version == "v2") { + method_name = "envoy.api.v2.ScopedRoutesDiscoveryService.DeltaScopedRoutes"; + } else { + ASSERT(version == "v3alpha"); + method_name = "envoy.service.route.v3alpha.ScopedRoutesDiscoveryService.DeltaScopedRoutes"; + } + } else if (typeUrlIs(type_url, envoy::api::v2::route::VirtualHost(), + envoy::api::v3alpha::route::VirtualHost())) { method_name = "envoy.api." + version + ".VirtualHostDiscoveryService.DeltaVirtualHosts"; - } else if (typeUrlIs(type_url, envoy::api::v2::auth::Secret())) { + } else if (typeUrlIs(type_url, envoy::api::v2::auth::Secret(), + envoy::api::v3alpha::auth::Secret())) { method_name = "envoy.service.discovery." + version + ".SecretDiscoveryService.DeltaSecrets"; - } else if (typeUrlIs(type_url, envoy::api::v2::Cluster())) { + } else if (typeUrlIs(type_url, envoy::api::v2::Cluster(), envoy::api::v3alpha::Cluster())) { method_name = "envoy.api." + version + ".ClusterDiscoveryService.DeltaClusters"; - } else if (typeUrlIs(type_url, envoy::api::v2::ClusterLoadAssignment())) { + } else if (typeUrlIs(type_url, envoy::api::v2::ClusterLoadAssignment(), + envoy::api::v3alpha::ClusterLoadAssignment())) { method_name = "envoy.api." + version + ".EndpointDiscoveryService.DeltaEndpoints"; - } else if (typeUrlIs(type_url, envoy::api::v2::Listener())) { + } else if (typeUrlIs(type_url, envoy::api::v2::Listener(), envoy::api::v3alpha::Listener())) { method_name = "envoy.api." + version + ".ListenerDiscoveryService.DeltaListeners"; - } else if (typeUrlIs(type_url, envoy::service::discovery::v2::Runtime())) { + } else if (typeUrlIs(type_url, envoy::service::discovery::v2::Runtime(), + envoy::service::discovery::v3alpha::Runtime())) { method_name = "envoy.service.discovery." + version + ".RuntimeDiscoveryService.DeltaRuntime"; } ASSERT(method_name != UnknownMethod); @@ -69,19 +94,29 @@ const Protobuf::MethodDescriptor& sotwGrpcMethod(absl::string_view type_url, envoy::api::v2::core::ApiVersion transport_api_version) { std::string method_name = UnknownMethod; const std::string version = versionString(transport_api_version); - if (typeUrlIs(type_url, envoy::api::v2::RouteConfiguration())) { + if (typeUrlIs(type_url, envoy::api::v2::RouteConfiguration(), + envoy::api::v3alpha::RouteConfiguration())) { method_name = "envoy.api." + version + ".RouteDiscoveryService.StreamRoutes"; - } else if (typeUrlIs(type_url, envoy::api::v2::ScopedRouteConfiguration())) { - method_name = "envoy.api." + version + ".ScopedRoutesDiscoveryService.StreamScopedRoutes"; - } else if (typeUrlIs(type_url, envoy::api::v2::auth::Secret())) { + } else if (typeUrlIs(type_url, envoy::api::v2::ScopedRouteConfiguration(), + envoy::service::route::v3alpha::ScopedRouteConfiguration())) { + if (version == "v2") { + method_name = "envoy.api.v2.ScopedRoutesDiscoveryService.StreamScopedRoutes"; + } else { + ASSERT(version == "v3alpha"); + method_name = "envoy.service.route.v3alpha.ScopedRoutesDiscoveryService.StreamScopedRoutes"; + } + } else if (typeUrlIs(type_url, envoy::api::v2::auth::Secret(), + envoy::api::v3alpha::auth::Secret())) { method_name = "envoy.service.discovery." + version + ".SecretDiscoveryService.StreamSecrets"; - } else if (typeUrlIs(type_url, envoy::api::v2::Cluster())) { + } else if (typeUrlIs(type_url, envoy::api::v2::Cluster(), envoy::api::v3alpha::Cluster())) { method_name = "envoy.api." + version + ".ClusterDiscoveryService.StreamClusters"; - } else if (typeUrlIs(type_url, envoy::api::v2::ClusterLoadAssignment())) { + } else if (typeUrlIs(type_url, envoy::api::v2::ClusterLoadAssignment(), + envoy::api::v3alpha::ClusterLoadAssignment())) { method_name = "envoy.api." + version + ".EndpointDiscoveryService.StreamEndpoints"; - } else if (typeUrlIs(type_url, envoy::api::v2::Listener())) { + } else if (typeUrlIs(type_url, envoy::api::v2::Listener(), envoy::api::v3alpha::Listener())) { method_name = "envoy.api." + version + ".ListenerDiscoveryService.StreamListeners"; - } else if (typeUrlIs(type_url, envoy::service::discovery::v2::Runtime())) { + } else if (typeUrlIs(type_url, envoy::service::discovery::v2::Runtime(), + envoy::service::discovery::v3alpha::Runtime())) { method_name = "envoy.service.discovery." + version + ".RuntimeDiscoveryService.StreamRuntime"; } ASSERT(method_name != UnknownMethod); @@ -92,19 +127,29 @@ const Protobuf::MethodDescriptor& restMethod(absl::string_view type_url, envoy::api::v2::core::ApiVersion transport_api_version) { std::string method_name = UnknownMethod; const std::string version = versionString(transport_api_version); - if (typeUrlIs(type_url, envoy::api::v2::RouteConfiguration())) { + if (typeUrlIs(type_url, envoy::api::v2::RouteConfiguration(), + envoy::api::v3alpha::RouteConfiguration())) { method_name = "envoy.api." + version + ".RouteDiscoveryService.FetchRoutes"; - } else if (typeUrlIs(type_url, envoy::api::v2::ScopedRouteConfiguration())) { - method_name = "envoy.api." + version + ".ScopedRoutesDiscoveryService.FetchScopedRoutes"; - } else if (typeUrlIs(type_url, envoy::api::v2::auth::Secret())) { + } else if (typeUrlIs(type_url, envoy::api::v2::ScopedRouteConfiguration(), + envoy::service::route::v3alpha::ScopedRouteConfiguration())) { + if (version == "v2") { + method_name = "envoy.api.v2.ScopedRoutesDiscoveryService.FetchScopedRoutes"; + } else { + ASSERT(version == "v3alpha"); + method_name = "envoy.service.route.v3alpha.ScopedRoutesDiscoveryService.FetchScopedRoutes"; + } + } else if (typeUrlIs(type_url, envoy::api::v2::auth::Secret(), + envoy::api::v3alpha::auth::Secret())) { method_name = "envoy.service.discovery." + version + ".SecretDiscoveryService.FetchSecrets"; - } else if (typeUrlIs(type_url, envoy::api::v2::Cluster())) { + } else if (typeUrlIs(type_url, envoy::api::v2::Cluster(), envoy::api::v3alpha::Cluster())) { method_name = "envoy.api." + version + ".ClusterDiscoveryService.FetchClusters"; - } else if (typeUrlIs(type_url, envoy::api::v2::ClusterLoadAssignment())) { + } else if (typeUrlIs(type_url, envoy::api::v2::ClusterLoadAssignment(), + envoy::api::v3alpha::ClusterLoadAssignment())) { method_name = "envoy.api." + version + ".EndpointDiscoveryService.FetchEndpoints"; - } else if (typeUrlIs(type_url, envoy::api::v2::Listener())) { + } else if (typeUrlIs(type_url, envoy::api::v2::Listener(), envoy::api::v3alpha::Listener())) { method_name = "envoy.api." + version + ".ListenerDiscoveryService.FetchListeners"; - } else if (typeUrlIs(type_url, envoy::service::discovery::v2::Runtime())) { + } else if (typeUrlIs(type_url, envoy::service::discovery::v2::Runtime(), + envoy::service::discovery::v3alpha::Runtime())) { method_name = "envoy.service.discovery." + version + ".RuntimeDiscoveryService.FetchRuntime"; } ASSERT(method_name != UnknownMethod); diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index 29c1ad12fc935..d31ca98677cd2 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -1,3 +1,4 @@ +#include "common/common/assert.h" #include "envoy/api/v2/core/config_source.pb.h" #include "test/integration/http_integration.h" @@ -40,19 +41,70 @@ class ApiVersionIntegrationTest AssertionResult validateDiscoveryRequest(const std::string& expected_v2_sotw_endpoint, const std::string& expected_v2_delta_endpoint, - const std::string& expected_type_url) { + const std::string& expected_v3alpha_sotw_endpoint, + const std::string& expected_v3alpha_delta_endpoint, + const std::string& expected_v2_type_url, + const std::string& expected_v3alpha_type_url) { std::string expected_endpoint; + std::string expected_type_url; std::string actual_type_url; - if (sotw_or_delta_ == Grpc::SotwOrDelta::Sotw) { - API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; - VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); - expected_endpoint = expected_v2_sotw_endpoint; - actual_type_url = discovery_request.type_url(); - } else { - API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) delta_discovery_request; - VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, delta_discovery_request)); - expected_endpoint = expected_v2_delta_endpoint; - actual_type_url = delta_discovery_request.type_url(); + switch (transportApiVersion()) { + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: { + switch (apiType()) { + case envoy::api::v2::core::ApiConfigSource::GRPC: { + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; + VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); + actual_type_url = discovery_request.type_url(); + expected_endpoint = expected_v2_sotw_endpoint; + break; + } + case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) delta_discovery_request; + VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, delta_discovery_request)); + actual_type_url = delta_discovery_request.type_url(); + expected_endpoint = expected_v2_delta_endpoint; + break; + } + default: + return AssertionFailure() << "not implemented yet"; + } + break; + } + case envoy::api::v2::core::ApiVersion::V3ALPHA: { + switch (apiType()) { + case envoy::api::v2::core::ApiConfigSource::GRPC: { + API_NO_BOOST(envoy::api::v3alpha::DiscoveryRequest) discovery_request; + VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); + actual_type_url = discovery_request.type_url(); + expected_endpoint = expected_v3alpha_sotw_endpoint; + break; + } + case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { + API_NO_BOOST(envoy::api::v3alpha::DeltaDiscoveryRequest) delta_discovery_request; + VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, delta_discovery_request)); + actual_type_url = delta_discovery_request.type_url(); + expected_endpoint = expected_v3alpha_delta_endpoint; + break; + } + default: + return AssertionFailure() << "not implemented yet"; + } + break; + } + default: + NOT_REACHED_GCOVR_EXCL_LINE; + } + switch (resourceApiVersion()) { + case envoy::api::v2::core::ApiVersion::AUTO: + case envoy::api::v2::core::ApiVersion::V2: + expected_type_url = expected_v2_type_url; + break; + case envoy::api::v2::core::ApiVersion::V3ALPHA: + expected_type_url = expected_v3alpha_type_url; + break; + default: + NOT_REACHED_GCOVR_EXCL_LINE; } if (endpoint_ != expected_endpoint) { return AssertionFailure() << "Expected endpoint " << expected_endpoint << ", got " @@ -77,7 +129,7 @@ class ApiVersionIntegrationTest INSTANTIATE_TEST_SUITE_P( IpVersionsApiConfigSourcesApiVersions, ApiVersionIntegrationTest, testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), - testing::Values(envoy::api::v2::core::ApiConfigSource::REST, + testing::Values(/*envoy::api::v2::core::ApiConfigSource::REST,*/ envoy::api::v2::core::ApiConfigSource::GRPC, envoy::api::v2::core::ApiConfigSource::DELTA_GRPC), testing::Values(envoy::api::v2::core::ApiVersion::AUTO, @@ -103,9 +155,13 @@ TEST_P(ApiVersionIntegrationTest, Lds) { lds_cluster->mutable_http2_protocol_options(); }); initialize(); - ASSERT_TRUE(validateDiscoveryRequest("/envoy.api.v2.ListenerDiscoveryService/StreamListeners", - "/envoy.api.v2.ListenerDiscoveryService/DeltaListeners", - "type.googleapis.com/envoy.api.v2.Listener")); + ASSERT_TRUE( + validateDiscoveryRequest("/envoy.api.v2.ListenerDiscoveryService/StreamListeners", + "/envoy.api.v2.ListenerDiscoveryService/DeltaListeners", + "/envoy.api.v3alpha.ListenerDiscoveryService/StreamListeners", + "/envoy.api.v3alpha.ListenerDiscoveryService/DeltaListeners", + "type.googleapis.com/envoy.api.v2.Listener", + "type.googleapis.com/envoy.api.v3alpha.Listener")); } } // namespace From dc7f074f532a6a40839c83f8292dcc787f10015d Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 12:05:57 -0500 Subject: [PATCH 03/17] REST support. Signed-off-by: Harvey Tuch --- .../common/config/http_subscription_impl.cc | 3 +- source/common/config/http_subscription_impl.h | 2 +- .../config/subscription_factory_impl.cc | 4 +- .../config/http_subscription_test_harness.h | 3 +- .../api_version_integration_test.cc | 67 +++++++++++++++---- 5 files changed, 61 insertions(+), 18 deletions(-) diff --git a/source/common/config/http_subscription_impl.cc b/source/common/config/http_subscription_impl.cc index 5fe413a12da70..073d56bc21caf 100644 --- a/source/common/config/http_subscription_impl.cc +++ b/source/common/config/http_subscription_impl.cc @@ -22,7 +22,7 @@ HttpSubscriptionImpl::HttpSubscriptionImpl( const std::string& remote_cluster_name, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, std::chrono::milliseconds refresh_interval, std::chrono::milliseconds request_timeout, const Protobuf::MethodDescriptor& service_method, - SubscriptionCallbacks& callbacks, SubscriptionStats stats, + absl::string_view type_url, SubscriptionCallbacks& callbacks, SubscriptionStats stats, std::chrono::milliseconds init_fetch_timeout, ProtobufMessage::ValidationVisitor& validation_visitor) : Http::RestApiFetcher(cm, remote_cluster_name, dispatcher, random, refresh_interval, @@ -30,6 +30,7 @@ HttpSubscriptionImpl::HttpSubscriptionImpl( callbacks_(callbacks), stats_(stats), dispatcher_(dispatcher), init_fetch_timeout_(init_fetch_timeout), validation_visitor_(validation_visitor) { request_.mutable_node()->CopyFrom(local_info.node()); + request_.set_type_url(std::string(type_url)); ASSERT(service_method.options().HasExtension(google::api::http)); const auto& http_rule = service_method.options().GetExtension(google::api::http); path_ = http_rule.post(); diff --git a/source/common/config/http_subscription_impl.h b/source/common/config/http_subscription_impl.h index 56fa4fda2ba24..c70e47afaeaeb 100644 --- a/source/common/config/http_subscription_impl.h +++ b/source/common/config/http_subscription_impl.h @@ -25,7 +25,7 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher, const std::string& remote_cluster_name, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, std::chrono::milliseconds refresh_interval, std::chrono::milliseconds request_timeout, - const Protobuf::MethodDescriptor& service_method, + const Protobuf::MethodDescriptor& service_method, absl::string_view type_url, SubscriptionCallbacks& callbacks, SubscriptionStats stats, std::chrono::milliseconds init_fetch_timeout, ProtobufMessage::ValidationVisitor& validation_visitor); diff --git a/source/common/config/subscription_factory_impl.cc b/source/common/config/subscription_factory_impl.cc index a066547636f16..310c7dd0854ba 100644 --- a/source/common/config/subscription_factory_impl.cc +++ b/source/common/config/subscription_factory_impl.cc @@ -49,8 +49,8 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( local_info_, cm_, api_config_source.cluster_names()[0], dispatcher_, random_, Utility::apiConfigSourceRefreshDelay(api_config_source), Utility::apiConfigSourceRequestTimeout(api_config_source), - restMethod(type_url, api_config_source.transport_api_version()), callbacks, stats, - Utility::configSourceInitialFetchTimeout(config), validation_visitor_); + restMethod(type_url, api_config_source.transport_api_version()), type_url, callbacks, + stats, Utility::configSourceInitialFetchTimeout(config), validation_visitor_); break; case envoy::api::v2::core::ApiConfigSource::GRPC: result = std::make_unique( diff --git a/test/common/config/http_subscription_test_harness.h b/test/common/config/http_subscription_test_harness.h index 07862eafdae5d..732d09abf5190 100644 --- a/test/common/config/http_subscription_test_harness.h +++ b/test/common/config/http_subscription_test_harness.h @@ -49,7 +49,7 @@ class HttpSubscriptionTestHarness : public SubscriptionTestHarness { })); subscription_ = std::make_unique( local_info_, cm_, "eds_cluster", dispatcher_, random_gen_, std::chrono::milliseconds(1), - std::chrono::milliseconds(1000), *method_descriptor_, callbacks_, stats_, + std::chrono::milliseconds(1000), *method_descriptor_, Config::TypeUrl::get().ClusterLoadAssignment, callbacks_, stats_, init_fetch_timeout, validation_visitor_); } @@ -92,6 +92,7 @@ class HttpSubscriptionTestHarness : public SubscriptionTestHarness { } expected_request += "\"resource_names\":[\"" + joined_cluster_names + "\"]"; } + expected_request += ",\"type_url\":\"type.googleapis.com/envoy.api.v2.ClusterLoadAssignment\""; expected_request += "}"; EXPECT_EQ(expected_request, request->bodyAsString()); EXPECT_EQ(fmt::format_int(expected_request.size()).str(), diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index d31ca98677cd2..d93bbee63992f 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -6,11 +6,12 @@ namespace Envoy { namespace { -class ApiVersionIntegrationTest - : public testing::TestWithParam< - std::tuple>, - public HttpIntegrationTest { +using Params = + std::tuple; + +class ApiVersionIntegrationTest : public testing::TestWithParam, + public HttpIntegrationTest { public: ApiVersionIntegrationTest() : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, ipVersion()) { use_lds_ = false; @@ -19,6 +20,14 @@ class ApiVersionIntegrationTest skipPortUsageValidation(); } + static std::string paramsToString(const testing::TestParamInfo& p) { + return fmt::format("{}_{}_Resource_{}_Transport_{}", + std::get<0>(p.param) == Network::Address::IpVersion::v4 ? "IPv4" : "IPv6", + envoy::api::v2::core::ApiConfigSource::ApiType_Name(std::get<1>(p.param)), + envoy::api::v2::core::ApiVersion_Name(std::get<2>(p.param)), + envoy::api::v2::core::ApiVersion_Name(std::get<3>(p.param))); + } + Network::Address::IpVersion ipVersion() const { return std::get<0>(GetParam()); } envoy::api::v2::core::ApiConfigSource::ApiType apiType() const { return std::get<1>(GetParam()); } envoy::api::v2::core::ApiVersion resourceApiVersion() const { return std::get<2>(GetParam()); } @@ -35,14 +44,15 @@ class ApiVersionIntegrationTest RELEASE_ASSERT(result, result.message()); endpoint_ = std::string(xds_stream_->headers().Path()->value().getStringView()); ENVOY_LOG_MISC(debug, "xDS endpoint {}", endpoint_); - xds_stream_->startGrpcStream(); } } AssertionResult validateDiscoveryRequest(const std::string& expected_v2_sotw_endpoint, const std::string& expected_v2_delta_endpoint, + const std::string& expected_v2_rest_endpoint, const std::string& expected_v3alpha_sotw_endpoint, const std::string& expected_v3alpha_delta_endpoint, + const std::string& expected_v3alpha_rest_endpoint, const std::string& expected_v2_type_url, const std::string& expected_v3alpha_type_url) { std::string expected_endpoint; @@ -55,6 +65,7 @@ class ApiVersionIntegrationTest case envoy::api::v2::core::ApiConfigSource::GRPC: { API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); + xds_stream_->startGrpcStream(); actual_type_url = discovery_request.type_url(); expected_endpoint = expected_v2_sotw_endpoint; break; @@ -62,15 +73,26 @@ class ApiVersionIntegrationTest case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) delta_discovery_request; VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, delta_discovery_request)); + xds_stream_->startGrpcStream(); actual_type_url = delta_discovery_request.type_url(); expected_endpoint = expected_v2_delta_endpoint; break; } + case envoy::api::v2::core::ApiConfigSource::REST: { + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; + VERIFY_ASSERTION(xds_stream_->waitForEndStream(*dispatcher_)); + MessageUtil::loadFromJson(xds_stream_->body().toString(), discovery_request, + ProtobufMessage::getStrictValidationVisitor()); + ENVOY_LOG_MISC(debug, "HTD {}", xds_stream_->body().toString()); + actual_type_url = discovery_request.type_url(); + expected_endpoint = expected_v2_rest_endpoint; + break; + } default: - return AssertionFailure() << "not implemented yet"; + NOT_REACHED_GCOVR_EXCL_LINE; + break; } break; - } case envoy::api::v2::core::ApiVersion::V3ALPHA: { switch (apiType()) { case envoy::api::v2::core::ApiConfigSource::GRPC: { @@ -87,14 +109,25 @@ class ApiVersionIntegrationTest expected_endpoint = expected_v3alpha_delta_endpoint; break; } + case envoy::api::v2::core::ApiConfigSource::REST: { + API_NO_BOOST(envoy::api::v3alpha::DiscoveryRequest) discovery_request; + VERIFY_ASSERTION(xds_stream_->waitForEndStream(*dispatcher_)); + MessageUtil::loadFromJson(xds_stream_->body().toString(), discovery_request, + ProtobufMessage::getStrictValidationVisitor()); + actual_type_url = discovery_request.type_url(); + expected_endpoint = expected_v3alpha_rest_endpoint; + break; + } default: - return AssertionFailure() << "not implemented yet"; + NOT_REACHED_GCOVR_EXCL_LINE; + break; } break; } default: NOT_REACHED_GCOVR_EXCL_LINE; } + } switch (resourceApiVersion()) { case envoy::api::v2::core::ApiVersion::AUTO: case envoy::api::v2::core::ApiVersion::V2: @@ -129,7 +162,7 @@ class ApiVersionIntegrationTest INSTANTIATE_TEST_SUITE_P( IpVersionsApiConfigSourcesApiVersions, ApiVersionIntegrationTest, testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), - testing::Values(/*envoy::api::v2::core::ApiConfigSource::REST,*/ + testing::Values(envoy::api::v2::core::ApiConfigSource::REST, envoy::api::v2::core::ApiConfigSource::GRPC, envoy::api::v2::core::ApiConfigSource::DELTA_GRPC), testing::Values(envoy::api::v2::core::ApiVersion::AUTO, @@ -137,7 +170,8 @@ INSTANTIATE_TEST_SUITE_P( envoy::api::v2::core::ApiVersion::V3ALPHA), testing::Values(envoy::api::v2::core::ApiVersion::AUTO, envoy::api::v2::core::ApiVersion::V2, - envoy::api::v2::core::ApiVersion::V3ALPHA))); + envoy::api::v2::core::ApiVersion::V3ALPHA)), + ApiVersionIntegrationTest::paramsToString); TEST_P(ApiVersionIntegrationTest, Lds) { config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { @@ -147,8 +181,13 @@ TEST_P(ApiVersionIntegrationTest, Lds) { auto* api_config_source = lds_config->mutable_api_config_source(); api_config_source->set_transport_api_version(transportApiVersion()); api_config_source->set_api_type(apiType()); - auto* grpc_service = api_config_source->add_grpc_services(); - grpc_service->mutable_envoy_grpc()->set_cluster_name("lds_cluster"); + if (apiType() == envoy::api::v2::core::ApiConfigSource::REST) { + api_config_source->add_cluster_names("lds_cluster"); + api_config_source->mutable_refresh_delay()->set_seconds(1); + } else { + auto* grpc_service = api_config_source->add_grpc_services(); + grpc_service->mutable_envoy_grpc()->set_cluster_name("lds_cluster"); + } auto* lds_cluster = bootstrap.mutable_static_resources()->add_clusters(); lds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); lds_cluster->set_name("lds_cluster"); @@ -158,8 +197,10 @@ TEST_P(ApiVersionIntegrationTest, Lds) { ASSERT_TRUE( validateDiscoveryRequest("/envoy.api.v2.ListenerDiscoveryService/StreamListeners", "/envoy.api.v2.ListenerDiscoveryService/DeltaListeners", + "/v2/discovery:listeners", "/envoy.api.v3alpha.ListenerDiscoveryService/StreamListeners", "/envoy.api.v3alpha.ListenerDiscoveryService/DeltaListeners", + "/v3alpha/discovery:listeners", "type.googleapis.com/envoy.api.v2.Listener", "type.googleapis.com/envoy.api.v3alpha.Listener")); } From 8374c0a5470291f407a5b3faf41ecfd61e884d82 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 13:20:55 -0500 Subject: [PATCH 04/17] CDS and RTDS tests. Signed-off-by: Harvey Tuch --- source/common/config/BUILD | 3 +- source/common/config/type_to_endpoint.cc | 5 +- .../config/http_subscription_test_harness.h | 8 +- .../api_version_integration_test.cc | 91 +++++++++++++------ 4 files changed, 71 insertions(+), 36 deletions(-) diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 0fbbb072439b7..69f1eb71d483f 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -304,11 +304,10 @@ envoy_cc_library( "@envoy_api//envoy/api/v2/auth:pkg_cc_proto", "@envoy_api//envoy/api/v2/core:pkg_cc_proto", "@envoy_api//envoy/api/v2/route:pkg_cc_proto", - "@envoy_api//envoy/service/discovery/v2:pkg_cc_proto", "@envoy_api//envoy/api/v3alpha:pkg_cc_proto", "@envoy_api//envoy/api/v3alpha/auth:pkg_cc_proto", - "@envoy_api//envoy/api/v3alpha/core:pkg_cc_proto", "@envoy_api//envoy/api/v3alpha/route:pkg_cc_proto", + "@envoy_api//envoy/service/discovery/v2:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3alpha:pkg_cc_proto", "@envoy_api//envoy/service/route/v3alpha:pkg_cc_proto", ], diff --git a/source/common/config/type_to_endpoint.cc b/source/common/config/type_to_endpoint.cc index 909cc6c7980cf..52b7970487a3d 100644 --- a/source/common/config/type_to_endpoint.cc +++ b/source/common/config/type_to_endpoint.cc @@ -7,16 +7,15 @@ #include "envoy/api/v2/rds.pb.h" #include "envoy/api/v2/route/route.pb.h" #include "envoy/api/v2/srds.pb.h" -#include "envoy/service/discovery/v2/rtds.pb.h" - #include "envoy/api/v3alpha/auth/cert.pb.h" #include "envoy/api/v3alpha/cds.pb.h" #include "envoy/api/v3alpha/eds.pb.h" #include "envoy/api/v3alpha/lds.pb.h" #include "envoy/api/v3alpha/rds.pb.h" #include "envoy/api/v3alpha/route/route.pb.h" -#include "envoy/service/route/v3alpha/srds.pb.h" +#include "envoy/service/discovery/v2/rtds.pb.h" #include "envoy/service/discovery/v3alpha/rtds.pb.h" +#include "envoy/service/route/v3alpha/srds.pb.h" #include "common/common/assert.h" #include "common/grpc/common.h" diff --git a/test/common/config/http_subscription_test_harness.h b/test/common/config/http_subscription_test_harness.h index 732d09abf5190..7d69ecef29669 100644 --- a/test/common/config/http_subscription_test_harness.h +++ b/test/common/config/http_subscription_test_harness.h @@ -49,8 +49,9 @@ class HttpSubscriptionTestHarness : public SubscriptionTestHarness { })); subscription_ = std::make_unique( local_info_, cm_, "eds_cluster", dispatcher_, random_gen_, std::chrono::milliseconds(1), - std::chrono::milliseconds(1000), *method_descriptor_, Config::TypeUrl::get().ClusterLoadAssignment, callbacks_, stats_, - init_fetch_timeout, validation_visitor_); + std::chrono::milliseconds(1000), *method_descriptor_, + Config::TypeUrl::get().ClusterLoadAssignment, callbacks_, stats_, init_fetch_timeout, + validation_visitor_); } ~HttpSubscriptionTestHarness() override { @@ -92,7 +93,8 @@ class HttpSubscriptionTestHarness : public SubscriptionTestHarness { } expected_request += "\"resource_names\":[\"" + joined_cluster_names + "\"]"; } - expected_request += ",\"type_url\":\"type.googleapis.com/envoy.api.v2.ClusterLoadAssignment\""; + expected_request += + ",\"type_url\":\"type.googleapis.com/envoy.api.v2.ClusterLoadAssignment\""; expected_request += "}"; EXPECT_EQ(expected_request, request->bodyAsString()); EXPECT_EQ(fmt::format_int(expected_request.size()).str(), diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index d93bbee63992f..9c877052ab6f0 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -1,6 +1,7 @@ -#include "common/common/assert.h" #include "envoy/api/v2/core/config_source.pb.h" +#include "common/common/assert.h" + #include "test/integration/http_integration.h" namespace Envoy { @@ -34,6 +35,13 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, envoy::api::v2::core::ApiVersion transportApiVersion() const { return std::get<3>(GetParam()); } void initialize() override { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + bootstrap.mutable_static_resources()->clear_listeners(); + auto* xds_cluster = bootstrap.mutable_static_resources()->add_clusters(); + xds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); + xds_cluster->set_name("xds_cluster"); + xds_cluster->mutable_http2_protocol_options(); + }); setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); HttpIntegrationTest::initialize(); if (xds_stream_ == nullptr) { @@ -47,6 +55,20 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, } } + void setupConfigSource(envoy::api::v2::core::ConfigSource& config_source) { + config_source.set_resource_api_version(resourceApiVersion()); + auto* api_config_source = config_source.mutable_api_config_source(); + api_config_source->set_transport_api_version(transportApiVersion()); + api_config_source->set_api_type(apiType()); + if (apiType() == envoy::api::v2::core::ApiConfigSource::REST) { + api_config_source->add_cluster_names("xds_cluster"); + api_config_source->mutable_refresh_delay()->set_seconds(1); + } else { + auto* grpc_service = api_config_source->add_grpc_services(); + grpc_service->mutable_envoy_grpc()->set_cluster_name("xds_cluster"); + } + } + AssertionResult validateDiscoveryRequest(const std::string& expected_v2_sotw_endpoint, const std::string& expected_v2_delta_endpoint, const std::string& expected_v2_rest_endpoint, @@ -83,7 +105,6 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, VERIFY_ASSERTION(xds_stream_->waitForEndStream(*dispatcher_)); MessageUtil::loadFromJson(xds_stream_->body().toString(), discovery_request, ProtobufMessage::getStrictValidationVisitor()); - ENVOY_LOG_MISC(debug, "HTD {}", xds_stream_->body().toString()); actual_type_url = discovery_request.type_url(); expected_endpoint = expected_v2_rest_endpoint; break; @@ -175,34 +196,48 @@ INSTANTIATE_TEST_SUITE_P( TEST_P(ApiVersionIntegrationTest, Lds) { config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { - bootstrap.mutable_static_resources()->clear_listeners(); - auto* lds_config = bootstrap.mutable_dynamic_resources()->mutable_lds_config(); - lds_config->set_resource_api_version(resourceApiVersion()); - auto* api_config_source = lds_config->mutable_api_config_source(); - api_config_source->set_transport_api_version(transportApiVersion()); - api_config_source->set_api_type(apiType()); - if (apiType() == envoy::api::v2::core::ApiConfigSource::REST) { - api_config_source->add_cluster_names("lds_cluster"); - api_config_source->mutable_refresh_delay()->set_seconds(1); - } else { - auto* grpc_service = api_config_source->add_grpc_services(); - grpc_service->mutable_envoy_grpc()->set_cluster_name("lds_cluster"); - } - auto* lds_cluster = bootstrap.mutable_static_resources()->add_clusters(); - lds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); - lds_cluster->set_name("lds_cluster"); - lds_cluster->mutable_http2_protocol_options(); + setupConfigSource(*bootstrap.mutable_dynamic_resources()->mutable_lds_config()); + }); + initialize(); + ASSERT_TRUE(validateDiscoveryRequest( + "/envoy.api.v2.ListenerDiscoveryService/StreamListeners", + "/envoy.api.v2.ListenerDiscoveryService/DeltaListeners", "/v2/discovery:listeners", + "/envoy.api.v3alpha.ListenerDiscoveryService/StreamListeners", + "/envoy.api.v3alpha.ListenerDiscoveryService/DeltaListeners", "/v3alpha/discovery:listeners", + "type.googleapis.com/envoy.api.v2.Listener", + "type.googleapis.com/envoy.api.v3alpha.Listener")); +} + +TEST_P(ApiVersionIntegrationTest, Cds) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + setupConfigSource(*bootstrap.mutable_dynamic_resources()->mutable_cds_config()); + }); + initialize(); + ASSERT_TRUE(validateDiscoveryRequest( + "/envoy.api.v2.ClusterDiscoveryService/StreamClusters", + "/envoy.api.v2.ClusterDiscoveryService/DeltaClusters", "/v2/discovery:clusters", + "/envoy.api.v3alpha.ClusterDiscoveryService/StreamClusters", + "/envoy.api.v3alpha.ClusterDiscoveryService/DeltaClusters", "/v3alpha/discovery:clusters", + "type.googleapis.com/envoy.api.v2.Cluster", "type.googleapis.com/envoy.api.v3alpha.Cluster")); +} + +TEST_P(ApiVersionIntegrationTest, Rtds) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + auto* admin_layer = bootstrap.mutable_layered_runtime()->add_layers(); + admin_layer->set_name("admin layer"); + admin_layer->mutable_admin_layer(); + auto* rtds_layer = bootstrap.mutable_layered_runtime()->add_layers(); + rtds_layer->set_name("rtds_layer"); + setupConfigSource(*rtds_layer->mutable_rtds_layer()->mutable_rtds_config()); }); initialize(); - ASSERT_TRUE( - validateDiscoveryRequest("/envoy.api.v2.ListenerDiscoveryService/StreamListeners", - "/envoy.api.v2.ListenerDiscoveryService/DeltaListeners", - "/v2/discovery:listeners", - "/envoy.api.v3alpha.ListenerDiscoveryService/StreamListeners", - "/envoy.api.v3alpha.ListenerDiscoveryService/DeltaListeners", - "/v3alpha/discovery:listeners", - "type.googleapis.com/envoy.api.v2.Listener", - "type.googleapis.com/envoy.api.v3alpha.Listener")); + ASSERT_TRUE(validateDiscoveryRequest( + "/envoy.service.discovery.v2.RuntimeDiscoveryService/StreamRuntime", + "/envoy.service.discovery.v2.RuntimeDiscoveryService/DeltaRuntime", "/v2/discovery:runtime", + "/envoy.service.discovery.v3alpha.RuntimeDiscoveryService/StreamRuntime", + "/envoy.service.discovery.v3alpha.RuntimeDiscoveryService/DeltaRuntime", + "/v3alpha/discovery:runtime", "type.googleapis.com/envoy.service.discovery.v2.Runtime", + "type.googleapis.com/envoy.service.discovery.v3alpha.Runtime")); } } // namespace From 19c0232dbd15d476f4c1f301d8bf8becf31770be Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 13:54:46 -0500 Subject: [PATCH 05/17] Bracing. Signed-off-by: Harvey Tuch --- test/integration/api_version_integration_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index 9c877052ab6f0..7b22a57d40bb8 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -114,6 +114,7 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, break; } break; + } case envoy::api::v2::core::ApiVersion::V3ALPHA: { switch (apiType()) { case envoy::api::v2::core::ApiConfigSource::GRPC: { @@ -148,7 +149,6 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, default: NOT_REACHED_GCOVR_EXCL_LINE; } - } switch (resourceApiVersion()) { case envoy::api::v2::core::ApiVersion::AUTO: case envoy::api::v2::core::ApiVersion::V2: From 1df88c4a8da8afcaa4d5f94a34985a8487e0bf75 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 13:59:39 -0500 Subject: [PATCH 06/17] SRDS and reduction of combinatorial explosion. Signed-off-by: Harvey Tuch --- .../api_version_integration_test.cc | 62 ++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index 7b22a57d40bb8..46037b6c49a1f 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -18,6 +18,7 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, use_lds_ = false; create_xds_upstream_ = true; tls_xds_upstream_ = false; + defer_listener_finalization_ = true; skipPortUsageValidation(); } @@ -36,7 +37,6 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, void initialize() override { config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { - bootstrap.mutable_static_resources()->clear_listeners(); auto* xds_cluster = bootstrap.mutable_static_resources()->add_clusters(); xds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); xds_cluster->set_name("xds_cluster"); @@ -180,20 +180,35 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, std::string endpoint_; }; +// We manage the permutations below to reduce combinatorial explosion: +// - We only care about testing on one IP version, there should be no +// material difference between v4/v6. +// - We do care about all the different ApiConfigSource variations. +// - We explicitly give the AUTO versions their own independent test suite, +// since they are equivalent to v2, so we want to test them once but they are +// mostly redundant. INSTANTIATE_TEST_SUITE_P( - IpVersionsApiConfigSourcesApiVersions, ApiVersionIntegrationTest, - testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + ApiConfigSourcesExplicitApiVersions, ApiVersionIntegrationTest, + testing::Combine(testing::Values(TestEnvironment::getIpVersionsForTest()[0]), testing::Values(envoy::api::v2::core::ApiConfigSource::REST, envoy::api::v2::core::ApiConfigSource::GRPC, envoy::api::v2::core::ApiConfigSource::DELTA_GRPC), - testing::Values(envoy::api::v2::core::ApiVersion::AUTO, - envoy::api::v2::core::ApiVersion::V2, + testing::Values(envoy::api::v2::core::ApiVersion::V2, envoy::api::v2::core::ApiVersion::V3ALPHA), - testing::Values(envoy::api::v2::core::ApiVersion::AUTO, - envoy::api::v2::core::ApiVersion::V2, + testing::Values(envoy::api::v2::core::ApiVersion::V2, envoy::api::v2::core::ApiVersion::V3ALPHA)), ApiVersionIntegrationTest::paramsToString); +INSTANTIATE_TEST_SUITE_P( + ApiConfigSourcesAutoApiVersions, ApiVersionIntegrationTest, + testing::Combine(testing::Values(TestEnvironment::getIpVersionsForTest()[0]), + testing::Values(envoy::api::v2::core::ApiConfigSource::REST, + envoy::api::v2::core::ApiConfigSource::GRPC, + envoy::api::v2::core::ApiConfigSource::DELTA_GRPC), + testing::Values(envoy::api::v2::core::ApiVersion::AUTO), + testing::Values(envoy::api::v2::core::ApiVersion::AUTO)), + ApiVersionIntegrationTest::paramsToString); + TEST_P(ApiVersionIntegrationTest, Lds) { config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { setupConfigSource(*bootstrap.mutable_dynamic_resources()->mutable_lds_config()); @@ -240,5 +255,38 @@ TEST_P(ApiVersionIntegrationTest, Rtds) { "type.googleapis.com/envoy.service.discovery.v3alpha.Runtime")); } +TEST_P(ApiVersionIntegrationTest, Srds) { + config_helper_.addConfigModifier( + [this](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& + http_connection_manager) { + auto* scoped_routes = http_connection_manager.mutable_scoped_routes(); + scoped_routes->set_name("scoped_routes"); + const std::string& scope_key_builder_config_yaml = R"EOF( +fragments: + - header_value_extractor: + name: Addr + element_separator: ; + element: + key: x-foo-key + separator: = +)EOF"; + envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::ScopeKeyBuilder + scope_key_builder; + TestUtility::loadFromYaml(scope_key_builder_config_yaml, + *scoped_routes->mutable_scope_key_builder()); + setupConfigSource(*scoped_routes->mutable_scoped_rds()->mutable_scoped_rds_config_source()); + setupConfigSource(*scoped_routes->mutable_rds_config_source()); + }); + initialize(); + ASSERT_TRUE(validateDiscoveryRequest( + "/envoy.api.v2.ScopedRoutesDiscoveryService/StreamScopedRoutes", + "/envoy.api.v2.ScopedRoutesDiscoveryService/DeltaScopedRoutes", "/v2/discovery:scoped-routes", + "/envoy.service.route.v3alpha.ScopedRoutesDiscoveryService/StreamScopedRoutes", + "/envoy.service.route.v3alpha.ScopedRoutesDiscoveryService/DeltaScopedRoutes", + "/v3alpha/discovery:scoped-routes", + "type.googleapis.com/envoy.api.v2.ScopedRouteConfiguration", + "type.googleapis.com/envoy.service.route.v3alpha.ScopedRouteConfiguration")); +} + } // namespace } // namespace Envoy From 9ef233ce56e16339f52c1ed1658932fa2812b131 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 15:11:20 -0500 Subject: [PATCH 07/17] RDS, SDS, EDS Signed-off-by: Harvey Tuch --- source/common/secret/BUILD | 1 + source/common/secret/sds_api.cc | 3 +- .../api_version_integration_test.cc | 68 ++++++++++++++++++- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/source/common/secret/BUILD b/source/common/secret/BUILD index 45b4f11f5d398..0d389e952192a 100644 --- a/source/common/secret/BUILD +++ b/source/common/secret/BUILD @@ -63,5 +63,6 @@ envoy_cc_library( "@envoy_api//envoy/api/v2:pkg_cc_proto", "@envoy_api//envoy/api/v2/auth:pkg_cc_proto", "@envoy_api//envoy/api/v2/core:pkg_cc_proto", + "@envoy_api//envoy/api/v3alpha/auth:pkg_cc_proto", ], ) diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index 39b501bf9efc6..d2b054ab930e1 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -3,6 +3,7 @@ #include #include "envoy/api/v2/auth/cert.pb.h" +#include "envoy/api/v3alpha/auth/cert.pb.h" #include "envoy/api/v2/auth/cert.pb.validate.h" #include "envoy/api/v2/core/config_source.pb.h" #include "envoy/api/v2/discovery.pb.h" @@ -95,7 +96,7 @@ std::string SdsApi::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_ve API_NO_BOOST(envoy::api::v2::auth::Secret().GetDescriptor()->full_name())); case envoy::api::v2::core::ApiVersion::V3ALPHA: return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v2::auth::Secret().GetDescriptor()->full_name())); + API_NO_BOOST(envoy::api::v3alpha::auth::Secret().GetDescriptor()->full_name())); default: NOT_REACHED_GCOVR_EXCL_LINE; } diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index 46037b6c49a1f..e1f68534c2210 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -172,7 +172,9 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, } void TearDown() override { - cleanUpXdsConnection(); + if (xds_stream_ != nullptr) { + cleanUpXdsConnection(); + } test_server_.reset(); fake_upstreams_.clear(); } @@ -236,6 +238,23 @@ TEST_P(ApiVersionIntegrationTest, Cds) { "type.googleapis.com/envoy.api.v2.Cluster", "type.googleapis.com/envoy.api.v3alpha.Cluster")); } +TEST_P(ApiVersionIntegrationTest, Eds) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + auto* cluster = bootstrap.mutable_static_resources()->add_clusters(); + cluster->MergeFrom(bootstrap.static_resources().clusters(0)); + cluster->set_name("some_cluster"); + cluster->set_type(envoy::api::v2::Cluster::EDS); + setupConfigSource(*cluster->mutable_eds_cluster_config()->mutable_eds_config()); + }); + initialize(); + ASSERT_TRUE(validateDiscoveryRequest( + "/envoy.api.v2.EndpointDiscoveryService/StreamEndpoints", + "/envoy.api.v2.EndpointDiscoveryService/DeltaEndpoints", "/v2/discovery:endpoints", + "/envoy.api.v3alpha.EndpointDiscoveryService/StreamEndpoints", + "/envoy.api.v3alpha.EndpointDiscoveryService/DeltaEndpoints", "/v3alpha/discovery:endpoints", + "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", "type.googleapis.com/envoy.api.v3alpha.ClusterLoadAssignment")); +} + TEST_P(ApiVersionIntegrationTest, Rtds) { config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { auto* admin_layer = bootstrap.mutable_layered_runtime()->add_layers(); @@ -255,6 +274,30 @@ TEST_P(ApiVersionIntegrationTest, Rtds) { "type.googleapis.com/envoy.service.discovery.v3alpha.Runtime")); } +TEST_P(ApiVersionIntegrationTest, Rds) { + // TODO(htuch): this segfaults, this is likely some undertesting existing + // issue. + if (apiType() == envoy::api::v2::core::ApiConfigSource::DELTA_GRPC) { + return; + } + config_helper_.addConfigModifier( + [this](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& + http_connection_manager) { + auto* rds = http_connection_manager.mutable_rds(); + rds->set_route_config_name("rds"); + setupConfigSource(*rds->mutable_config_source()); + }); + initialize(); + ASSERT_TRUE(validateDiscoveryRequest( + "/envoy.api.v2.RouteDiscoveryService/StreamRoutes", + "/envoy.api.v2.RouteDiscoveryService/DeltaRoutes", "/v2/discovery:routes", + "/envoy.api.v3alpha.RouteDiscoveryService/StreamRoutes", + "/envoy.api.v3alpha.RouteDiscoveryService/DeltaRoutes", + "/v3alpha/discovery:routes", + "type.googleapis.com/envoy.api.v2.RouteConfiguration", + "type.googleapis.com/envoy.api.v3alpha.RouteConfiguration")); +} + TEST_P(ApiVersionIntegrationTest, Srds) { config_helper_.addConfigModifier( [this](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& @@ -288,5 +331,28 @@ TEST_P(ApiVersionIntegrationTest, Srds) { "type.googleapis.com/envoy.service.route.v3alpha.ScopedRouteConfiguration")); } +TEST_P(ApiVersionIntegrationTest, Sds) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); + auto* transport_socket = listener->mutable_filter_chains(0)->mutable_transport_socket(); + envoy::api::v2::auth::DownstreamTlsContext tls_context; + auto* common_tls_context = tls_context.mutable_common_tls_context(); + auto* secret_config = common_tls_context->add_tls_certificate_sds_secret_configs(); + secret_config->set_name("sds"); + setupConfigSource(*secret_config->mutable_sds_config()); + transport_socket->set_name("envoy.transport_sockets.tls"); + transport_socket->mutable_typed_config()->PackFrom(tls_context); + }); + initialize(); + ASSERT_TRUE(validateDiscoveryRequest( + "/envoy.service.discovery.v2.SecretDiscoveryService/StreamSecrets", + "/envoy.service.discovery.v2.SecretDiscoveryService/DeltaSecrets", "/v2/discovery:secrets", + "/envoy.service.discovery.v3alpha.SecretDiscoveryService/StreamSecrets", + "/envoy.service.discovery.v3alpha.SecretDiscoveryService/DeltaSecrets", + "/v3alpha/discovery:secrets", + "type.googleapis.com/envoy.api.v2.auth.Secret", + "type.googleapis.com/envoy.api.v3alpha.auth.Secret")); +} + } // namespace } // namespace Envoy From 0d7a9ba57b1f985f6c631e443ccbbe0f976c9335 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 15:56:49 -0500 Subject: [PATCH 08/17] wip Signed-off-by: Harvey Tuch --- .../api_version_integration_test.cc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index e1f68534c2210..024a66bba6644 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -298,6 +298,24 @@ TEST_P(ApiVersionIntegrationTest, Rds) { "type.googleapis.com/envoy.api.v3alpha.RouteConfiguration")); } +TEST_P(ApiVersionIntegrationTest, Vhds) { + config_helper_.addConfigModifier( + [this](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& + http_connection_manager) { + auto* route_config = http_connection_manager.mutable_route_config(); + setupConfigSource(*route_config->mutable_vhds()->mutable_config_source()); + }); + initialize(); + ASSERT_TRUE(validateDiscoveryRequest( + "/envoy.api.v2.RouteDiscoveryService/StreamRoutes", + "/envoy.api.v2.RouteDiscoveryService/DeltaRoutes", "/v2/discovery:routes", + "/envoy.api.v3alpha.RouteDiscoveryService/StreamRoutes", + "/envoy.api.v3alpha.RouteDiscoveryService/DeltaRoutes", + "/v3alpha/discovery:routes", + "type.googleapis.com/envoy.api.v2.RouteConfiguration", + "type.googleapis.com/envoy.api.v3alpha.RouteConfiguration")); +} + TEST_P(ApiVersionIntegrationTest, Srds) { config_helper_.addConfigModifier( [this](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& From 87da32a2d716612681b7ae0be2c13d9b3ec07378 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 15:59:59 -0500 Subject: [PATCH 09/17] Skip VHDS for now. Signed-off-by: Harvey Tuch --- .../api_version_integration_test.cc | 29 +++++-------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index 024a66bba6644..d761bc448e6f1 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -252,7 +252,8 @@ TEST_P(ApiVersionIntegrationTest, Eds) { "/envoy.api.v2.EndpointDiscoveryService/DeltaEndpoints", "/v2/discovery:endpoints", "/envoy.api.v3alpha.EndpointDiscoveryService/StreamEndpoints", "/envoy.api.v3alpha.EndpointDiscoveryService/DeltaEndpoints", "/v3alpha/discovery:endpoints", - "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", "type.googleapis.com/envoy.api.v3alpha.ClusterLoadAssignment")); + "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", + "type.googleapis.com/envoy.api.v3alpha.ClusterLoadAssignment")); } TEST_P(ApiVersionIntegrationTest, Rtds) { @@ -292,29 +293,14 @@ TEST_P(ApiVersionIntegrationTest, Rds) { "/envoy.api.v2.RouteDiscoveryService/StreamRoutes", "/envoy.api.v2.RouteDiscoveryService/DeltaRoutes", "/v2/discovery:routes", "/envoy.api.v3alpha.RouteDiscoveryService/StreamRoutes", - "/envoy.api.v3alpha.RouteDiscoveryService/DeltaRoutes", - "/v3alpha/discovery:routes", + "/envoy.api.v3alpha.RouteDiscoveryService/DeltaRoutes", "/v3alpha/discovery:routes", "type.googleapis.com/envoy.api.v2.RouteConfiguration", "type.googleapis.com/envoy.api.v3alpha.RouteConfiguration")); } -TEST_P(ApiVersionIntegrationTest, Vhds) { - config_helper_.addConfigModifier( - [this](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& - http_connection_manager) { - auto* route_config = http_connection_manager.mutable_route_config(); - setupConfigSource(*route_config->mutable_vhds()->mutable_config_source()); - }); - initialize(); - ASSERT_TRUE(validateDiscoveryRequest( - "/envoy.api.v2.RouteDiscoveryService/StreamRoutes", - "/envoy.api.v2.RouteDiscoveryService/DeltaRoutes", "/v2/discovery:routes", - "/envoy.api.v3alpha.RouteDiscoveryService/StreamRoutes", - "/envoy.api.v3alpha.RouteDiscoveryService/DeltaRoutes", - "/v3alpha/discovery:routes", - "type.googleapis.com/envoy.api.v2.RouteConfiguration", - "type.googleapis.com/envoy.api.v3alpha.RouteConfiguration")); -} +// TODO(htuch): add VHDS tests once VHDS lands. +// TEST_P(ApiVersionIntegrationTest, Vhds) { +// } TEST_P(ApiVersionIntegrationTest, Srds) { config_helper_.addConfigModifier( @@ -367,8 +353,7 @@ TEST_P(ApiVersionIntegrationTest, Sds) { "/envoy.service.discovery.v2.SecretDiscoveryService/DeltaSecrets", "/v2/discovery:secrets", "/envoy.service.discovery.v3alpha.SecretDiscoveryService/StreamSecrets", "/envoy.service.discovery.v3alpha.SecretDiscoveryService/DeltaSecrets", - "/v3alpha/discovery:secrets", - "type.googleapis.com/envoy.api.v2.auth.Secret", + "/v3alpha/discovery:secrets", "type.googleapis.com/envoy.api.v2.auth.Secret", "type.googleapis.com/envoy.api.v3alpha.auth.Secret")); } From e315260a1b21d97ec474c739b58346adbff34a71 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 16:50:14 -0500 Subject: [PATCH 10/17] ADS support. Signed-off-by: Harvey Tuch --- .../common/upstream/cluster_manager_impl.cc | 7 ++- .../api_version_integration_test.cc | 56 ++++++++++++++----- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index a478ecbb2eda2..bb114630d9060 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -281,7 +281,12 @@ ClusterManagerImpl::ClusterManagerImpl( ->create(), main_thread_dispatcher, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( - "envoy.service.discovery.v2.AggregatedDiscoveryService.StreamAggregatedResources"), + dyn_resources.ads_config().transport_api_version() == + envoy::api::v2::core::ApiVersion::V3ALPHA + ? "envoy.service.discovery.v3alpha.AggregatedDiscoveryService." + "StreamAggregatedResources" + : "envoy.service.discovery.v2.AggregatedDiscoveryService." + "StreamAggregatedResources"), random_, stats_, Envoy::Config::Utility::parseRateLimitSettings(dyn_resources.ads_config()), bootstrap.dynamic_resources().ads_config().set_node_on_first_message_only()); diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index d761bc448e6f1..7a5b89312a0ec 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -8,7 +8,7 @@ namespace Envoy { namespace { using Params = - std::tuple; class ApiVersionIntegrationTest : public testing::TestWithParam, @@ -23,24 +23,33 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, } static std::string paramsToString(const testing::TestParamInfo& p) { - return fmt::format("{}_{}_Resource_{}_Transport_{}", + return fmt::format("{}_{}_{}_Resource_{}_Transport_{}", std::get<0>(p.param) == Network::Address::IpVersion::v4 ? "IPv4" : "IPv6", - envoy::api::v2::core::ApiConfigSource::ApiType_Name(std::get<1>(p.param)), - envoy::api::v2::core::ApiVersion_Name(std::get<2>(p.param)), - envoy::api::v2::core::ApiVersion_Name(std::get<3>(p.param))); + std::get<1>(p.param) ? "ADS" : "SingletonXds", + envoy::api::v2::core::ApiConfigSource::ApiType_Name(std::get<2>(p.param)), + envoy::api::v2::core::ApiVersion_Name(std::get<3>(p.param)), + envoy::api::v2::core::ApiVersion_Name(std::get<4>(p.param))); } Network::Address::IpVersion ipVersion() const { return std::get<0>(GetParam()); } - envoy::api::v2::core::ApiConfigSource::ApiType apiType() const { return std::get<1>(GetParam()); } - envoy::api::v2::core::ApiVersion resourceApiVersion() const { return std::get<2>(GetParam()); } - envoy::api::v2::core::ApiVersion transportApiVersion() const { return std::get<3>(GetParam()); } + bool ads() const { return std::get<1>(GetParam()); } + envoy::api::v2::core::ApiConfigSource::ApiType apiType() const { return std::get<2>(GetParam()); } + envoy::api::v2::core::ApiVersion resourceApiVersion() const { return std::get<3>(GetParam()); } + envoy::api::v2::core::ApiVersion transportApiVersion() const { return std::get<4>(GetParam()); } void initialize() override { - config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { auto* xds_cluster = bootstrap.mutable_static_resources()->add_clusters(); xds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); xds_cluster->set_name("xds_cluster"); xds_cluster->mutable_http2_protocol_options(); + if (ads()) { + auto* api_config_source = bootstrap.mutable_dynamic_resources()->mutable_ads_config(); + api_config_source->set_transport_api_version(transportApiVersion()); + api_config_source->set_api_type(apiType()); + auto* grpc_service = api_config_source->add_grpc_services(); + grpc_service->mutable_envoy_grpc()->set_cluster_name("xds_cluster"); + } }); setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); HttpIntegrationTest::initialize(); @@ -57,6 +66,10 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, void setupConfigSource(envoy::api::v2::core::ConfigSource& config_source) { config_source.set_resource_api_version(resourceApiVersion()); + if (ads()) { + config_source.mutable_ads(); + return; + } auto* api_config_source = config_source.mutable_api_config_source(); api_config_source->set_transport_api_version(transportApiVersion()); api_config_source->set_api_type(apiType()); @@ -80,6 +93,8 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, std::string expected_endpoint; std::string expected_type_url; std::string actual_type_url; + const char ads_v2_sotw_endpoint[] = "/envoy.service.discovery.v2.AggregatedDiscoveryService/StreamAggregatedResources"; + const char ads_v3alpha_delta_endpoint[] = "/envoy.service.discovery.v3alpha.AggregatedDiscoveryService/StreamAggregatedResources"; switch (transportApiVersion()) { case envoy::api::v2::core::ApiVersion::AUTO: case envoy::api::v2::core::ApiVersion::V2: { @@ -89,7 +104,7 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); xds_stream_->startGrpcStream(); actual_type_url = discovery_request.type_url(); - expected_endpoint = expected_v2_sotw_endpoint; + expected_endpoint = ads() ? ads_v2_sotw_endpoint : expected_v2_sotw_endpoint; break; } case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { @@ -121,7 +136,7 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, API_NO_BOOST(envoy::api::v3alpha::DiscoveryRequest) discovery_request; VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); actual_type_url = discovery_request.type_url(); - expected_endpoint = expected_v3alpha_sotw_endpoint; + expected_endpoint = ads() ? ads_v3alpha_delta_endpoint : expected_v3alpha_sotw_endpoint; break; } case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { @@ -189,9 +204,12 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, // - We explicitly give the AUTO versions their own independent test suite, // since they are equivalent to v2, so we want to test them once but they are // mostly redundant. +// - We treat ADS and singleton xDS differently. ADS doesn't care about REST and +// doesn't currently support delta xDS. INSTANTIATE_TEST_SUITE_P( - ApiConfigSourcesExplicitApiVersions, ApiVersionIntegrationTest, + SingletonApiConfigSourcesExplicitApiVersions, ApiVersionIntegrationTest, testing::Combine(testing::Values(TestEnvironment::getIpVersionsForTest()[0]), + testing::Values(false), testing::Values(envoy::api::v2::core::ApiConfigSource::REST, envoy::api::v2::core::ApiConfigSource::GRPC, envoy::api::v2::core::ApiConfigSource::DELTA_GRPC), @@ -202,8 +220,9 @@ INSTANTIATE_TEST_SUITE_P( ApiVersionIntegrationTest::paramsToString); INSTANTIATE_TEST_SUITE_P( - ApiConfigSourcesAutoApiVersions, ApiVersionIntegrationTest, + SingletonApiConfigSourcesAutoApiVersions, ApiVersionIntegrationTest, testing::Combine(testing::Values(TestEnvironment::getIpVersionsForTest()[0]), + testing::Values(false), testing::Values(envoy::api::v2::core::ApiConfigSource::REST, envoy::api::v2::core::ApiConfigSource::GRPC, envoy::api::v2::core::ApiConfigSource::DELTA_GRPC), @@ -211,6 +230,17 @@ INSTANTIATE_TEST_SUITE_P( testing::Values(envoy::api::v2::core::ApiVersion::AUTO)), ApiVersionIntegrationTest::paramsToString); +INSTANTIATE_TEST_SUITE_P( + AdsApiConfigSourcesExplicitApiVersions, ApiVersionIntegrationTest, + testing::Combine(testing::Values(TestEnvironment::getIpVersionsForTest()[0]), + testing::Values(true), + testing::Values(envoy::api::v2::core::ApiConfigSource::GRPC), + testing::Values(envoy::api::v2::core::ApiVersion::V2, + envoy::api::v2::core::ApiVersion::V3ALPHA), + testing::Values(envoy::api::v2::core::ApiVersion::V2, + envoy::api::v2::core::ApiVersion::V3ALPHA)), + ApiVersionIntegrationTest::paramsToString); + TEST_P(ApiVersionIntegrationTest, Lds) { config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { setupConfigSource(*bootstrap.mutable_dynamic_resources()->mutable_lds_config()); From ddb5342cf9fbf38af1dfa75bd889de470e2c38be Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 17:12:35 -0500 Subject: [PATCH 11/17] fix_format Signed-off-by: Harvey Tuch --- source/common/secret/sds_api.cc | 2 +- test/integration/api_version_integration_test.cc | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index d2b054ab930e1..d817abcfb4554 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -3,10 +3,10 @@ #include #include "envoy/api/v2/auth/cert.pb.h" -#include "envoy/api/v3alpha/auth/cert.pb.h" #include "envoy/api/v2/auth/cert.pb.validate.h" #include "envoy/api/v2/core/config_source.pb.h" #include "envoy/api/v2/discovery.pb.h" +#include "envoy/api/v3alpha/auth/cert.pb.h" #include "common/common/assert.h" #include "common/config/api_version.h" diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index 7a5b89312a0ec..7dff0a6176411 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -93,8 +93,10 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, std::string expected_endpoint; std::string expected_type_url; std::string actual_type_url; - const char ads_v2_sotw_endpoint[] = "/envoy.service.discovery.v2.AggregatedDiscoveryService/StreamAggregatedResources"; - const char ads_v3alpha_delta_endpoint[] = "/envoy.service.discovery.v3alpha.AggregatedDiscoveryService/StreamAggregatedResources"; + const char ads_v2_sotw_endpoint[] = + "/envoy.service.discovery.v2.AggregatedDiscoveryService/StreamAggregatedResources"; + const char ads_v3alpha_delta_endpoint[] = + "/envoy.service.discovery.v3alpha.AggregatedDiscoveryService/StreamAggregatedResources"; switch (transportApiVersion()) { case envoy::api::v2::core::ApiVersion::AUTO: case envoy::api::v2::core::ApiVersion::V2: { From fac5ffa654d9f09a648f42093c7997f6da9abc6d Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 17:15:30 -0500 Subject: [PATCH 12/17] fix_format Signed-off-by: Harvey Tuch --- generated_api_shadow/envoy/api/v2/core/grpc_service.proto | 4 +++- .../envoy/api/v3alpha/core/grpc_service.proto | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/generated_api_shadow/envoy/api/v2/core/grpc_service.proto b/generated_api_shadow/envoy/api/v2/core/grpc_service.proto index 02fb63ff5596c..562d999b8e1a2 100644 --- a/generated_api_shadow/envoy/api/v2/core/grpc_service.proto +++ b/generated_api_shadow/envoy/api/v2/core/grpc_service.proto @@ -93,7 +93,9 @@ message GrpcService { // [#next-free-field: 10] message StsService { // URI of the token exchange service that handles token exchange requests. - string token_exchange_service_uri = 1 [(validate.rules).string = {uri: true}]; + // [#comment:TODO(asraa): Add URI validation when implemented. Tracked by + // https://github.com/envoyproxy/protoc-gen-validate/issues/303] + string token_exchange_service_uri = 1; // Location of the target service or resource where the client // intends to use the requested security token. diff --git a/generated_api_shadow/envoy/api/v3alpha/core/grpc_service.proto b/generated_api_shadow/envoy/api/v3alpha/core/grpc_service.proto index 5129aa4590cc8..50e354360febf 100644 --- a/generated_api_shadow/envoy/api/v3alpha/core/grpc_service.proto +++ b/generated_api_shadow/envoy/api/v3alpha/core/grpc_service.proto @@ -128,7 +128,9 @@ message GrpcService { "envoy.api.v2.core.GrpcService.GoogleGrpc.CallCredentials.StsService"; // URI of the token exchange service that handles token exchange requests. - string token_exchange_service_uri = 1 [(validate.rules).string = {uri: true}]; + // [#comment:TODO(asraa): Add URI validation when implemented. Tracked by + // https://github.com/envoyproxy/protoc-gen-validate/issues/303] + string token_exchange_service_uri = 1; // Location of the target service or resource where the client // intends to use the requested security token. From 20cd7fbfd1c3d9329f851e5b17f85ed24f1c7eac Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 2 Jan 2020 17:16:49 -0500 Subject: [PATCH 13/17] Spelling. Signed-off-by: Harvey Tuch --- api/envoy/api/v2/core/config_source.proto | 2 +- api/envoy/api/v3alpha/core/config_source.proto | 2 +- generated_api_shadow/envoy/api/v2/core/config_source.proto | 2 +- .../envoy/api/v3alpha/core/config_source.proto | 2 +- test/integration/api_version_integration_test.cc | 3 +-- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/api/envoy/api/v2/core/config_source.proto b/api/envoy/api/v2/core/config_source.proto index 596d5384a1c31..d090609f9e488 100644 --- a/api/envoy/api/v2/core/config_source.proto +++ b/api/envoy/api/v2/core/config_source.proto @@ -18,7 +18,7 @@ option java_multiple_files = true; // xDS API version. This is used to describe both resource and transport // protocol versions (in distinct configuration fields). enum ApiVersion { - // When not specified, we asssume v2, to ease migration to Envoy's stable API + // When not specified, we assume v2, to ease migration to Envoy's stable API // versioning. If a client does not support v2 (e.g. due to deprecation), this // is an invalid value. AUTO = 0; diff --git a/api/envoy/api/v3alpha/core/config_source.proto b/api/envoy/api/v3alpha/core/config_source.proto index 5b6220e4f84d5..9dc6d86d16c79 100644 --- a/api/envoy/api/v3alpha/core/config_source.proto +++ b/api/envoy/api/v3alpha/core/config_source.proto @@ -20,7 +20,7 @@ option java_multiple_files = true; // xDS API version. This is used to describe both resource and transport // protocol versions (in distinct configuration fields). enum ApiVersion { - // When not specified, we asssume v2, to ease migration to Envoy's stable API + // When not specified, we assume v2, to ease migration to Envoy's stable API // versioning. If a client does not support v2 (e.g. due to deprecation), this // is an invalid value. AUTO = 0; diff --git a/generated_api_shadow/envoy/api/v2/core/config_source.proto b/generated_api_shadow/envoy/api/v2/core/config_source.proto index 596d5384a1c31..d090609f9e488 100644 --- a/generated_api_shadow/envoy/api/v2/core/config_source.proto +++ b/generated_api_shadow/envoy/api/v2/core/config_source.proto @@ -18,7 +18,7 @@ option java_multiple_files = true; // xDS API version. This is used to describe both resource and transport // protocol versions (in distinct configuration fields). enum ApiVersion { - // When not specified, we asssume v2, to ease migration to Envoy's stable API + // When not specified, we assume v2, to ease migration to Envoy's stable API // versioning. If a client does not support v2 (e.g. due to deprecation), this // is an invalid value. AUTO = 0; diff --git a/generated_api_shadow/envoy/api/v3alpha/core/config_source.proto b/generated_api_shadow/envoy/api/v3alpha/core/config_source.proto index abf63d7e2753b..f7988edd72f76 100644 --- a/generated_api_shadow/envoy/api/v3alpha/core/config_source.proto +++ b/generated_api_shadow/envoy/api/v3alpha/core/config_source.proto @@ -20,7 +20,7 @@ option java_multiple_files = true; // xDS API version. This is used to describe both resource and transport // protocol versions (in distinct configuration fields). enum ApiVersion { - // When not specified, we asssume v2, to ease migration to Envoy's stable API + // When not specified, we assume v2, to ease migration to Envoy's stable API // versioning. If a client does not support v2 (e.g. due to deprecation), this // is an invalid value. AUTO = 0; diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index 7dff0a6176411..5b99f090dc544 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -308,8 +308,7 @@ TEST_P(ApiVersionIntegrationTest, Rtds) { } TEST_P(ApiVersionIntegrationTest, Rds) { - // TODO(htuch): this segfaults, this is likely some undertesting existing - // issue. + // TODO(htuch): this segfaults, this is likely some untested existing issue. if (apiType() == envoy::api::v2::core::ApiConfigSource::DELTA_GRPC) { return; } From 7bb6e37e06e6bdd5affb041f64148721b444166f Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Fri, 3 Jan 2020 13:36:06 -0500 Subject: [PATCH 14/17] fix_format Signed-off-by: Harvey Tuch --- .../envoy/api/v2/core/config_source.proto | 2 ++ .../envoy/api/v2/core/grpc_service.proto | 2 ++ .../envoy/config/core/v3alpha/config_source.proto | 12 ++++++------ .../envoy/config/core/v3alpha/grpc_service.proto | 14 +++++++------- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/generated_api_shadow/envoy/api/v2/core/config_source.proto b/generated_api_shadow/envoy/api/v2/core/config_source.proto index d090609f9e488..5a082d8bc793c 100644 --- a/generated_api_shadow/envoy/api/v2/core/config_source.proto +++ b/generated_api_shadow/envoy/api/v2/core/config_source.proto @@ -7,11 +7,13 @@ import "envoy/api/v2/core/grpc_service.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; +import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.api.v2.core"; option java_outer_classname = "ConfigSourceProto"; option java_multiple_files = true; +option (udpa.annotations.file_migrate).move_to_package = "envoy.config.core.v3alpha"; // [#protodoc-title: Configuration sources] diff --git a/generated_api_shadow/envoy/api/v2/core/grpc_service.proto b/generated_api_shadow/envoy/api/v2/core/grpc_service.proto index 562d999b8e1a2..2a423e2dba55c 100644 --- a/generated_api_shadow/envoy/api/v2/core/grpc_service.proto +++ b/generated_api_shadow/envoy/api/v2/core/grpc_service.proto @@ -9,11 +9,13 @@ import "google/protobuf/duration.proto"; import "google/protobuf/empty.proto"; import "google/protobuf/struct.proto"; +import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.api.v2.core"; option java_outer_classname = "GrpcServiceProto"; option java_multiple_files = true; +option (udpa.annotations.file_migrate).move_to_package = "envoy.config.core.v3alpha"; // [#protodoc-title: gRPC services] diff --git a/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto b/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto index f7988edd72f76..c5f6340be317d 100644 --- a/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto +++ b/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto @@ -1,8 +1,8 @@ syntax = "proto3"; -package envoy.api.v3alpha.core; +package envoy.config.core.v3alpha; -import "envoy/api/v3alpha/core/grpc_service.proto"; +import "envoy/config/core/v3alpha/grpc_service.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; @@ -11,7 +11,7 @@ import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; -option java_package = "io.envoyproxy.envoy.api.v3alpha.core"; +option java_package = "io.envoyproxy.envoy.config.core.v3alpha"; option java_outer_classname = "ConfigSourceProto"; option java_multiple_files = true; @@ -98,7 +98,7 @@ message ApiConfigSource { } // Aggregated Discovery Service (ADS) options. This is currently empty, but when -// set in :ref:`ConfigSource ` can be used to +// set in :ref:`ConfigSource ` can be used to // specify that ADS is to be used. message AggregatedConfigSource { option (udpa.annotations.versioning).previous_message_type = @@ -107,7 +107,7 @@ message AggregatedConfigSource { // [#not-implemented-hide:] // Self-referencing config source options. This is currently empty, but when -// set in :ref:`ConfigSource ` can be used to +// set in :ref:`ConfigSource ` can be used to // specify that other data can be obtained from the same server. message SelfConfigSource { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.SelfConfigSource"; @@ -129,7 +129,7 @@ message RateLimitSettings { // Configuration for :ref:`listeners `, :ref:`clusters // `, :ref:`routes -// `, :ref:`endpoints +// `, :ref:`endpoints // ` etc. may either be sourced from the // filesystem or from an xDS API source. Filesystem configs are watched with // inotify for updates. diff --git a/generated_api_shadow/envoy/config/core/v3alpha/grpc_service.proto b/generated_api_shadow/envoy/config/core/v3alpha/grpc_service.proto index 50e354360febf..69fda18e01fa2 100644 --- a/generated_api_shadow/envoy/config/core/v3alpha/grpc_service.proto +++ b/generated_api_shadow/envoy/config/core/v3alpha/grpc_service.proto @@ -1,8 +1,8 @@ syntax = "proto3"; -package envoy.api.v3alpha.core; +package envoy.config.core.v3alpha; -import "envoy/api/v3alpha/core/base.proto"; +import "envoy/config/core/v3alpha/base.proto"; import "google/protobuf/any.proto"; import "google/protobuf/duration.proto"; @@ -13,14 +13,14 @@ import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; -option java_package = "io.envoyproxy.envoy.api.v3alpha.core"; +option java_package = "io.envoyproxy.envoy.config.core.v3alpha"; option java_outer_classname = "GrpcServiceProto"; option java_multiple_files = true; // [#protodoc-title: gRPC services] // gRPC service configuration. This is used by :ref:`ApiConfigSource -// ` and filter configurations. +// ` and filter configurations. // [#next-free-field: 6] message GrpcService { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.GrpcService"; @@ -30,8 +30,8 @@ message GrpcService { "envoy.api.v2.core.GrpcService.EnvoyGrpc"; // The name of the upstream gRPC cluster. SSL credentials will be supplied - // in the :ref:`Cluster ` :ref:`transport_socket - // `. + // in the :ref:`Cluster ` :ref:`transport_socket + // `. string cluster_name = 1 [(validate.rules).string = {min_bytes: 1}]; } @@ -201,7 +201,7 @@ message GrpcService { // The target URI when using the `Google C++ gRPC client // `_. SSL credentials will be supplied in // :ref:`channel_credentials - // `. + // `. string target_uri = 1 [(validate.rules).string = {min_bytes: 1}]; ChannelCredentials channel_credentials = 2; From c0e1995040c927db6782c10aa5ea6850649784ab Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Sun, 5 Jan 2020 17:46:33 -0500 Subject: [PATCH 15/17] fix_format Signed-off-by: Harvey Tuch --- .../envoy/config/core/v3alpha/config_source.proto | 2 +- source/common/config/BUILD | 6 ++++-- source/common/config/type_to_endpoint.cc | 10 +++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto b/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto index c5f6340be317d..b9c58a30ae8d0 100644 --- a/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto +++ b/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto @@ -129,7 +129,7 @@ message RateLimitSettings { // Configuration for :ref:`listeners `, :ref:`clusters // `, :ref:`routes -// `, :ref:`endpoints +// `, :ref:`endpoints // ` etc. may either be sourced from the // filesystem or from an xDS API source. Filesystem configs are watched with // inotify for updates. diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 69f1eb71d483f..c039b85e4aee2 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -304,11 +304,13 @@ envoy_cc_library( "@envoy_api//envoy/api/v2/auth:pkg_cc_proto", "@envoy_api//envoy/api/v2/core:pkg_cc_proto", "@envoy_api//envoy/api/v2/route:pkg_cc_proto", - "@envoy_api//envoy/api/v3alpha:pkg_cc_proto", "@envoy_api//envoy/api/v3alpha/auth:pkg_cc_proto", - "@envoy_api//envoy/api/v3alpha/route:pkg_cc_proto", + "@envoy_api//envoy/config/route/v3alpha/route:pkg_cc_proto", + "@envoy_api//envoy/service/cluster/v3alpha:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v2:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3alpha:pkg_cc_proto", + "@envoy_api//envoy/service/endpoint/v3alpha:pkg_cc_proto", + "@envoy_api//envoy/service/listener/v3alpha:pkg_cc_proto", "@envoy_api//envoy/service/route/v3alpha:pkg_cc_proto", ], ) diff --git a/source/common/config/type_to_endpoint.cc b/source/common/config/type_to_endpoint.cc index 52b7970487a3d..ffadf0ebb5db0 100644 --- a/source/common/config/type_to_endpoint.cc +++ b/source/common/config/type_to_endpoint.cc @@ -8,13 +8,13 @@ #include "envoy/api/v2/route/route.pb.h" #include "envoy/api/v2/srds.pb.h" #include "envoy/api/v3alpha/auth/cert.pb.h" -#include "envoy/api/v3alpha/cds.pb.h" -#include "envoy/api/v3alpha/eds.pb.h" -#include "envoy/api/v3alpha/lds.pb.h" -#include "envoy/api/v3alpha/rds.pb.h" -#include "envoy/api/v3alpha/route/route.pb.h" +#include "envoy/config/route/v3alpha/route/route.pb.h" +#include "envoy/service/cluster/v3alpha/cds.pb.h" #include "envoy/service/discovery/v2/rtds.pb.h" #include "envoy/service/discovery/v3alpha/rtds.pb.h" +#include "envoy/service/endpoint/v3alpha/eds.pb.h" +#include "envoy/service/listener/v3alpha/lds.pb.h" +#include "envoy/service/route/v3alpha/rds.pb.h" #include "envoy/service/route/v3alpha/srds.pb.h" #include "common/common/assert.h" From 4255fc0a9841dc3094eb1c1675dab609d4f70c30 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 6 Jan 2020 18:40:56 -0500 Subject: [PATCH 16/17] Update to v3alpha API boost and other recent PRs. Signed-off-by: Harvey Tuch --- source/common/config/BUILD | 13 ++ .../common/config/delta_subscription_state.h | 2 + source/common/config/grpc_mux_impl.h | 9 +- source/common/config/http_subscription_impl.h | 3 +- source/common/config/new_grpc_mux_impl.h | 3 +- source/common/config/protobuf_link_hacks.h | 39 +++- .../config/subscription_factory_impl.cc | 13 +- source/common/config/type_to_endpoint.cc | 5 +- source/common/config/type_to_endpoint.h | 14 +- source/common/router/rds_impl.cc | 4 +- source/common/router/rds_impl.h | 2 +- source/common/router/scoped_rds.cc | 4 +- source/common/router/scoped_rds.h | 2 +- source/common/router/vhds.cc | 13 +- source/common/router/vhds.h | 4 +- source/common/runtime/runtime_impl.cc | 3 +- source/common/runtime/runtime_impl.h | 2 +- source/common/secret/sds_api.cc | 6 +- source/common/secret/sds_api.h | 2 +- source/common/upstream/cds_api_impl.cc | 2 +- .../common/upstream/cluster_manager_impl.cc | 2 +- source/common/upstream/eds.cc | 3 +- source/common/upstream/eds.h | 2 +- source/server/lds_api.cc | 2 +- source/server/lds_api.h | 2 +- .../config/delta_subscription_test_harness.h | 6 +- test/integration/README.md | 2 +- .../api_version_integration_test.cc | 200 ++++++++++-------- test/integration/cds_integration_test.cc | 2 +- 29 files changed, 213 insertions(+), 153 deletions(-) diff --git a/source/common/config/BUILD b/source/common/config/BUILD index a7cbe6ba76fc8..2f9bdca0ce5a8 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -85,6 +85,7 @@ envoy_cc_library( srcs = ["delta_subscription_state.cc"], hdrs = ["delta_subscription_state.h"], deps = [ + ":api_version_lib", ":pausable_ack_queue_lib", "//include/envoy/config:subscription_interface", "//include/envoy/event:dispatcher_interface", @@ -94,6 +95,7 @@ envoy_cc_library( "//source/common/common:token_bucket_impl_lib", "//source/common/grpc:common_lib", "//source/common/protobuf", + "@envoy_api//envoy/api/v2:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3alpha:pkg_cc_proto", ], ) @@ -145,6 +147,7 @@ envoy_cc_library( "//include/envoy/upstream:cluster_manager_interface", "//source/common/common:minimal_logger_lib", "//source/common/protobuf", + "@envoy_api//envoy/api/v2:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3alpha:pkg_cc_proto", ], ) @@ -187,6 +190,7 @@ envoy_cc_library( ":watch_map_lib", "//include/envoy/event:dispatcher_interface", "//include/envoy/grpc:async_client_interface", + "@envoy_api//envoy/api/v2:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3alpha:pkg_cc_proto", ], ) @@ -209,6 +213,7 @@ envoy_cc_library( "//source/common/http:rest_api_fetcher_lib", "//source/common/protobuf", "//source/common/protobuf:utility_lib", + "@envoy_api//envoy/api/v2:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3alpha:pkg_cc_proto", ], ) @@ -242,8 +247,15 @@ envoy_cc_library( name = "protobuf_link_hacks", hdrs = ["protobuf_link_hacks.h"], deps = [ + "@envoy_api//envoy/api/v2:pkg_cc_proto", + "@envoy_api//envoy/service/cluster/v3alpha:pkg_cc_proto", + "@envoy_api//envoy/service/discovery/v2:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3alpha:pkg_cc_proto", + "@envoy_api//envoy/service/endpoint/v3alpha:pkg_cc_proto", + "@envoy_api//envoy/service/listener/v3alpha:pkg_cc_proto", + "@envoy_api//envoy/service/ratelimit/v2:pkg_cc_proto", "@envoy_api//envoy/service/ratelimit/v3alpha:pkg_cc_proto", + "@envoy_api//envoy/service/route/v3alpha:pkg_cc_proto", "@envoy_api//envoy/service/runtime/v3alpha:pkg_cc_proto", "@envoy_api//envoy/service/secret/v3alpha:pkg_cc_proto", ], @@ -303,6 +315,7 @@ envoy_cc_library( "//source/common/grpc:common_lib", "//source/common/protobuf", "@envoy_api//envoy/annotations:pkg_cc_proto", + "@envoy_api//envoy/config/core/v3alpha:pkg_cc_proto", ], ) diff --git a/source/common/config/delta_subscription_state.h b/source/common/config/delta_subscription_state.h index 3a275e169f372..2d0d53b7fa4fd 100644 --- a/source/common/config/delta_subscription_state.h +++ b/source/common/config/delta_subscription_state.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/api/v2/discovery.pb.h" #include "envoy/config/subscription.h" #include "envoy/event/dispatcher.h" #include "envoy/grpc/status.h" @@ -8,6 +9,7 @@ #include "common/common/assert.h" #include "common/common/logger.h" +#include "common/config/api_version.h" #include "common/config/pausable_ack_queue.h" namespace Envoy { diff --git a/source/common/config/grpc_mux_impl.h b/source/common/config/grpc_mux_impl.h index 762f3b7ae44b3..4cf797c3ade1c 100644 --- a/source/common/config/grpc_mux_impl.h +++ b/source/common/config/grpc_mux_impl.h @@ -3,6 +3,7 @@ #include #include +#include "envoy/api/v2/discovery.pb.h" #include "envoy/common/time.h" #include "envoy/config/grpc_mux.h" #include "envoy/config/subscription.h" @@ -63,7 +64,8 @@ class GrpcMuxImpl std::unique_ptr&& message) override; void onWriteable() override; - GrpcStream& + GrpcStream& grpcStreamForTest() { return grpc_stream_; } @@ -106,7 +108,7 @@ class GrpcMuxImpl // Watches on the returned resources for the API; std::list watches_; // Current DiscoveryRequest for API. - API_NO_BOOST(envoy::api::v2::DiscoveryRequest) request_; + envoy::service::discovery::v3alpha::DiscoveryRequest request_; // Paused via pause()? bool paused_{}; // Was a DiscoveryRequest elided during a pause? @@ -119,7 +121,8 @@ class GrpcMuxImpl void queueDiscoveryRequest(const std::string& queue_item); void clearRequestQueue(); - GrpcStream + GrpcStream grpc_stream_; const LocalInfo::LocalInfo& local_info_; const bool skip_subsequent_node_; diff --git a/source/common/config/http_subscription_impl.h b/source/common/config/http_subscription_impl.h index f580e9c27ccaa..ef2a61569fa4e 100644 --- a/source/common/config/http_subscription_impl.h +++ b/source/common/config/http_subscription_impl.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/api/v2/discovery.pb.h" #include "envoy/config/subscription.h" #include "envoy/event/dispatcher.h" #include "envoy/service/discovery/v3alpha/discovery.pb.h" @@ -46,7 +47,7 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher, std::string path_; Protobuf::RepeatedPtrField resources_; - API_NO_BOOST(envoy::api::v2::DiscoveryRequest) request_; + envoy::service::discovery::v3alpha::DiscoveryRequest request_; Config::SubscriptionCallbacks& callbacks_; SubscriptionStats stats_; Event::Dispatcher& dispatcher_; diff --git a/source/common/config/new_grpc_mux_impl.h b/source/common/config/new_grpc_mux_impl.h index 76a7a38e80c85..8f52d8c463800 100644 --- a/source/common/config/new_grpc_mux_impl.h +++ b/source/common/config/new_grpc_mux_impl.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/api/v2/discovery.pb.h" #include "envoy/common/token_bucket.h" #include "envoy/config/grpc_mux.h" #include "envoy/config/subscription.h" @@ -113,7 +114,7 @@ class NewGrpcMuxImpl // the order of Envoy's dependency ordering). std::list subscription_ordering_; - GrpcStream grpc_stream_; }; diff --git a/source/common/config/protobuf_link_hacks.h b/source/common/config/protobuf_link_hacks.h index d1e4a915b8c36..9ba49faf13746 100644 --- a/source/common/config/protobuf_link_hacks.h +++ b/source/common/config/protobuf_link_hacks.h @@ -1,16 +1,47 @@ #pragma once +#include "envoy/api/v2/cds.pb.h" +#include "envoy/api/v2/eds.pb.h" +#include "envoy/api/v2/lds.pb.h" +#include "envoy/api/v2/rds.pb.h" +#include "envoy/api/v2/srds.pb.h" +#include "envoy/service/cluster/v3alpha/cds.pb.h" +#include "envoy/service/discovery/v2/ads.pb.h" +#include "envoy/service/discovery/v2/rtds.pb.h" +#include "envoy/service/discovery/v2/sds.pb.h" #include "envoy/service/discovery/v3alpha/ads.pb.h" +#include "envoy/service/endpoint/v3alpha/eds.pb.h" +#include "envoy/service/listener/v3alpha/lds.pb.h" +#include "envoy/service/ratelimit/v2/rls.pb.h" #include "envoy/service/ratelimit/v3alpha/rls.pb.h" +#include "envoy/service/route/v3alpha/rds.pb.h" +#include "envoy/service/route/v3alpha/srds.pb.h" #include "envoy/service/runtime/v3alpha/rtds.pb.h" #include "envoy/service/secret/v3alpha/sds.pb.h" +// API_NO_BOOST_FILE + namespace Envoy { // Hack to force linking of the service: https://github.com/google/protobuf/issues/4221. // This file should be included ONLY if this hack is required. -const envoy::service::discovery::v3alpha::AdsDummy _ads_dummy; -const envoy::service::ratelimit::v3alpha::RateLimitRequest _rls_dummy; -const envoy::service::secret::v3alpha::SdsDummy _sds_dummy; -const envoy::service::runtime::v3alpha::RtdsDummy _tds_dummy; +const envoy::service::discovery::v2::AdsDummy _ads_dummy_v2; +const envoy::service::ratelimit::v2::RateLimitRequest _rls_dummy_v2; +const envoy::service::discovery::v2::SdsDummy _sds_dummy_v2; +const envoy::service::discovery::v2::RtdsDummy _tds_dummy_v2; +const envoy::api::v2::LdsDummy _lds_dummy_v2; +const envoy::api::v2::RdsDummy _rds_dummy_v2; +const envoy::api::v2::CdsDummy _cds_dummy_v2; +const envoy::api::v2::EdsDummy _eds_dummy_v2; +const envoy::api::v2::SrdsDummy _srds_dummy_v2; + +const envoy::service::discovery::v3alpha::AdsDummy _ads_dummy_v3; +const envoy::service::ratelimit::v3alpha::RateLimitRequest _rls_dummy_v3; +const envoy::service::secret::v3alpha::SdsDummy _sds_dummy_v3; +const envoy::service::runtime::v3alpha::RtdsDummy _tds_dummy_v3; +const envoy::service::listener::v3alpha::LdsDummy _lds_dummy_v3; +const envoy::service::route::v3alpha::RdsDummy _rds_dummy_v3; +const envoy::service::cluster::v3alpha::CdsDummy _cds_dummy_v3; +const envoy::service::endpoint::v3alpha::EdsDummy _eds_dummy_v3; +const envoy::service::route::v3alpha::SrdsDummy _srds_dummy_v4; } // namespace Envoy diff --git a/source/common/config/subscription_factory_impl.cc b/source/common/config/subscription_factory_impl.cc index 844124df44092..a625ece74830a 100644 --- a/source/common/config/subscription_factory_impl.cc +++ b/source/common/config/subscription_factory_impl.cc @@ -50,9 +50,8 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( result = std::make_unique( local_info_, cm_, api_config_source.cluster_names()[0], dispatcher_, random_, Utility::apiConfigSourceRefreshDelay(api_config_source), - Utility::apiConfigSourceRequestTimeout(api_config_source), - restMethod(type_url, api_config_source.transport_api_version()), type_url, callbacks, - stats, Utility::configSourceInitialFetchTimeout(config), validation_visitor_); + Utility::apiConfigSourceRequestTimeout(api_config_source), restMethod(type_url), type_url, + callbacks, stats, Utility::configSourceInitialFetchTimeout(config), validation_visitor_); break; case envoy::config::core::v3alpha::ApiConfigSource::GRPC: result = std::make_unique( @@ -60,8 +59,8 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( Config::Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), api_config_source, scope) ->create(), - dispatcher_, random_, sotwGrpcMethod(type_url, api_config_source.transport_api_version()), - type_url, callbacks, stats, scope, Utility::parseRateLimitSettings(api_config_source), + dispatcher_, random_, sotwGrpcMethod(type_url), type_url, callbacks, stats, scope, + Utility::parseRateLimitSettings(api_config_source), Utility::configSourceInitialFetchTimeout(config), api_config_source.set_node_on_first_message_only()); break; @@ -72,8 +71,8 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( Config::Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), api_config_source, scope) ->create(), - dispatcher_, deltaGrpcMethod(type_url, api_config_source.transport_api_version()), - random_, scope, Utility::parseRateLimitSettings(api_config_source), local_info_), + dispatcher_, deltaGrpcMethod(type_url), random_, scope, + Utility::parseRateLimitSettings(api_config_source), local_info_), type_url, callbacks, stats, Utility::configSourceInitialFetchTimeout(config), false); break; } diff --git a/source/common/config/type_to_endpoint.cc b/source/common/config/type_to_endpoint.cc index 60cfd65755e0c..5dcd219dbfc17 100644 --- a/source/common/config/type_to_endpoint.cc +++ b/source/common/config/type_to_endpoint.cc @@ -46,9 +46,7 @@ TypeUrlToServiceMap* buildTypeUrlToServiceMap() { const auto* service_desc = Protobuf::DescriptorPool::generated_pool()->FindServiceByName(service_name); // TODO(htuch): this should become an ASSERT once all v3 descriptors are linked in. - if (service_desc == nullptr) { - continue; - } + ASSERT(service_desc != nullptr, fmt::format("{} missing", service_name)); ASSERT(service_desc->options().HasExtension(envoy::annotations::resource)); const std::string resource_type_url = Grpc::Common::typeUrl( service_desc->options().GetExtension(envoy::annotations::resource).type()); @@ -94,6 +92,7 @@ const Protobuf::MethodDescriptor& sotwGrpcMethod(absl::string_view type_url) { const Protobuf::MethodDescriptor& restMethod(absl::string_view type_url) { const auto it = typeUrlToServiceMap().find(static_cast(type_url)); + ENVOY_LOG_MISC(debug, "HTD {}", type_url); ASSERT(it != typeUrlToServiceMap().cend()); return *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(it->second.rest_method_); } diff --git a/source/common/config/type_to_endpoint.h b/source/common/config/type_to_endpoint.h index 59c3de0f434da..0eb56f5bfacd1 100644 --- a/source/common/config/type_to_endpoint.h +++ b/source/common/config/type_to_endpoint.h @@ -1,6 +1,6 @@ #pragma once -#include "envoy/api/v2/core/config_source.pb.h" +#include "envoy/config/core/v3alpha/config_source.pb.h" #include "common/protobuf/protobuf.h" @@ -10,18 +10,12 @@ namespace Envoy { namespace Config { // Translates an xDS resource type_url to the name of the delta gRPC service that carries it. -const Protobuf::MethodDescriptor& -deltaGrpcMethod(absl::string_view resource_type_url, - envoy::api::v2::core::ApiVersion transport_api_version); +const Protobuf::MethodDescriptor& deltaGrpcMethod(absl::string_view resource_type_url); // Translates an xDS resource type_url to the name of the state-of-the-world gRPC service that // carries it. -const Protobuf::MethodDescriptor& -sotwGrpcMethod(absl::string_view resource_type_url, - envoy::api::v2::core::ApiVersion transport_api_version); +const Protobuf::MethodDescriptor& sotwGrpcMethod(absl::string_view resource_type_url); // Translates an xDS resource type_url to the name of the REST service that carries it. -const Protobuf::MethodDescriptor& -restMethod(absl::string_view resource_type_url, - envoy::api::v2::core::ApiVersion transport_api_version); +const Protobuf::MethodDescriptor& restMethod(absl::string_view resource_type_url); } // namespace Config } // namespace Envoy diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index c52e21cebacfa..154cd5e83a7a6 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -205,8 +205,8 @@ bool RdsRouteConfigSubscription::validateUpdateSize(int num_resources) { return true; } -std::string -RdsRouteConfigSubscription::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { +std::string RdsRouteConfigSubscription::loadTypeUrl( + envoy::config::core::v3alpha::ApiVersion resource_api_version) { switch (resource_api_version) { // automatically set api version as V2 case envoy::config::core::v3alpha::ApiVersion::AUTO: diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index b89797265eb2b..8601c4194b093 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -147,7 +147,7 @@ class RdsRouteConfigSubscription : Envoy::Config::SubscriptionCallbacks, RouteConfigProviderManagerImpl& route_config_provider_manager); bool validateUpdateSize(int num_resources); - static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); + static std::string loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version); Init::Manager& getRdsConfigInitManager() { return init_manager_; } diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index 16f273c0a2d27..5acae3ff004d0 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -349,8 +349,8 @@ void ScopedRdsConfigSubscription::onConfigUpdate( onConfigUpdate(to_add_repeated, to_remove_repeated, version_info); } -std::string -ScopedRdsConfigSubscription::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { +std::string ScopedRdsConfigSubscription::loadTypeUrl( + envoy::config::core::v3alpha::ApiVersion resource_api_version) { switch (resource_api_version) { // automatically set api version as V2 case envoy::config::core::v3alpha::ApiVersion::AUTO: diff --git a/source/common/router/scoped_rds.h b/source/common/router/scoped_rds.h index 910b935cf6f45..49e801fffe42b 100644 --- a/source/common/router/scoped_rds.h +++ b/source/common/router/scoped_rds.h @@ -166,7 +166,7 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio resource) .name(); } - static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); + static std::string loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version); // Propagate RDS updates to ScopeConfigImpl in workers. void onRdsConfigUpdate(const std::string& scope_name, RdsRouteConfigSubscription& rds_subscription); diff --git a/source/common/router/vhds.cc b/source/common/router/vhds.cc index ffea27f7dbc56..fdd6f042c018e 100644 --- a/source/common/router/vhds.cc +++ b/source/common/router/vhds.cc @@ -20,11 +20,11 @@ namespace Envoy { namespace Router { // Implements callbacks to handle DeltaDiscovery protocol for VirtualHostDiscoveryService -VhdsSubscription::VhdsSubscription( - RouteConfigUpdatePtr& config_update_info, - Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, - std::unordered_set& route_config_providers, - envoy::config::core::v3alpha::ConfigSource::ApiVersion resource_api_version) +VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, + Server::Configuration::ServerFactoryContext& factory_context, + const std::string& stat_prefix, + std::unordered_set& route_config_providers, + envoy::config::core::v3alpha::ApiVersion resource_api_version) : config_update_info_(config_update_info), scope_(factory_context.scope().createScope(stat_prefix + "vhds." + config_update_info_->routeConfigName() + ".")), @@ -71,7 +71,8 @@ void VhdsSubscription::onConfigUpdate( init_target_.ready(); } -std::string VhdsSubscription::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { +std::string +VhdsSubscription::loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version) { switch (resource_api_version) { // automatically set api version as V2 case envoy::config::core::v3alpha::ApiVersion::AUTO: diff --git a/source/common/router/vhds.h b/source/common/router/vhds.h index 51a4d400eb469..5f1f9d466b4a4 100644 --- a/source/common/router/vhds.h +++ b/source/common/router/vhds.h @@ -41,7 +41,7 @@ class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, std::unordered_set& route_config_providers, - const envoy::config::core::v3alpha::ConfigSource::ApiVersion resource_api_version = + const envoy::config::core::v3alpha::ApiVersion resource_api_version = envoy::config::core::v3alpha::ApiVersion::AUTO); ~VhdsSubscription() override { init_target_.ready(); } @@ -61,7 +61,7 @@ class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); + static std::string loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version); RouteConfigUpdatePtr& config_update_info_; Stats::ScopePtr scope_; diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index d2ebe348df3c1..66aed326f0f16 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -605,7 +605,8 @@ void RtdsSubscription::validateUpdateSize(uint32_t num_resources) { } } -std::string RtdsSubscription::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { +std::string +RtdsSubscription::loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version) { switch (resource_api_version) { // automatically set api version as V2 case envoy::config::core::v3alpha::ApiVersion::AUTO: diff --git a/source/common/runtime/runtime_impl.h b/source/common/runtime/runtime_impl.h index 1c475a6cdfe8e..7f3812a457ae6 100644 --- a/source/common/runtime/runtime_impl.h +++ b/source/common/runtime/runtime_impl.h @@ -222,7 +222,7 @@ struct RtdsSubscription : Config::SubscriptionCallbacks, Logger::Loggablestart({sds_config_name_}); } -std::string SdsApi::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { +std::string SdsApi::loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version) { switch (resource_api_version) { // automatically set api version as V2 case envoy::config::core::v3alpha::ApiVersion::AUTO: @@ -98,8 +98,8 @@ std::string SdsApi::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_ve return Grpc::Common::typeUrl( API_NO_BOOST(envoy::api::v2::auth::Secret().GetDescriptor()->full_name())); case envoy::config::core::v3alpha::ApiVersion::V3ALPHA: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v3alpha::auth::Secret().GetDescriptor()->full_name())); + return Grpc::Common::typeUrl(API_NO_BOOST( + envoy::extensions::transport_sockets::tls::v3alpha::Secret().GetDescriptor()->full_name())); default: NOT_REACHED_GCOVR_EXCL_LINE; } diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index 5625e3f320bf0..66be2d4664109 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -66,7 +66,7 @@ class SdsApi : public Config::SubscriptionCallbacks { resource) .name(); } - static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); + static std::string loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version); private: void validateUpdateSize(int num_resources); diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index f0f3ceecda546..b4bc4ac87e48f 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -129,7 +129,7 @@ void CdsApiImpl::runInitializeCallbackIfAny() { } } -std::string CdsApiImpl::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { +std::string CdsApiImpl::loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version) { switch (resource_api_version) { // automatically set api version as V2 case envoy::config::core::v3alpha::ApiVersion::AUTO: diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 575800dae0138..8293d42d935a4 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -282,7 +282,7 @@ ClusterManagerImpl::ClusterManagerImpl( main_thread_dispatcher, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( dyn_resources.ads_config().transport_api_version() == - envoy::api::v2::core::ApiVersion::V3ALPHA + envoy::config::core::v3alpha::ApiVersion::V3ALPHA ? "envoy.service.discovery.v3alpha.AggregatedDiscoveryService." "StreamAggregatedResources" : "envoy.service.discovery.v2.AggregatedDiscoveryService." diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index c196228ef9001..849eb177313cc 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -224,7 +224,8 @@ void EdsClusterImpl::reloadHealthyHostsHelper(const HostSharedPtr& host) { } } -std::string EdsClusterImpl::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { +std::string +EdsClusterImpl::loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version) { switch (resource_api_version) { // automatically set api version as V2 case envoy::config::core::v3alpha::ApiVersion::AUTO: diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 137fd79d4af70..a872eaefe2b6d 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -46,7 +46,7 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallba resource) .cluster_name(); } - std::string loadTypeUrl(); + static std::string loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version); using LocalityWeightsMap = std::unordered_map; bool updateHostsPerLocality(const uint32_t priority, const uint32_t overprovisioning_factor, diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index 535b00056ccde..97477a00b5544 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -133,7 +133,7 @@ void LdsApiImpl::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason r init_target_.ready(); } -std::string LdsApiImpl::loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version) { +std::string LdsApiImpl::loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version) { switch (resource_api_version) { // automatically set api version as V2 case envoy::config::core::v3alpha::ApiVersion::AUTO: diff --git a/source/server/lds_api.h b/source/server/lds_api.h index fb1b02eefec1e..44d2e581818a7 100644 --- a/source/server/lds_api.h +++ b/source/server/lds_api.h @@ -45,7 +45,7 @@ class LdsApiImpl : public LdsApi, std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - static std::string loadTypeUrl(envoy::api::v2::core::ApiVersion resource_api_version); + static std::string loadTypeUrl(envoy::config::core::v3alpha::ApiVersion resource_api_version); std::unique_ptr subscription_; std::string system_version_info_; diff --git a/test/common/config/delta_subscription_test_harness.h b/test/common/config/delta_subscription_test_harness.h index 8a69277d1f1e9..a21b50aad3275 100644 --- a/test/common/config/delta_subscription_test_harness.h +++ b/test/common/config/delta_subscription_test_harness.h @@ -88,8 +88,8 @@ class DeltaSubscriptionTestHarness : public SubscriptionTestHarness { const std::set& unsubscribe, const Protobuf::int32 error_code, const std::string& error_message, std::map initial_resource_versions) { - envoy::service::discovery::v3alpha::DeltaDiscoveryRequest expected_request; - expected_request.mutable_node()->CopyFrom(node_); + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) expected_request; + expected_request.mutable_node()->CopyFrom(API_DOWNGRADE(node_)); std::copy( subscribe.begin(), subscribe.end(), Protobuf::RepeatedFieldBackInserter(expected_request.mutable_resource_names_subscribe())); @@ -115,7 +115,7 @@ class DeltaSubscriptionTestHarness : public SubscriptionTestHarness { sendMessageRaw_( Grpc::ProtoBufferEqIgnoringField(expected_request, "response_nonce"), false)) .WillOnce([this](Buffer::InstancePtr& buffer, bool) { - envoy::service::discovery::v3alpha::DeltaDiscoveryRequest message; + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) message; EXPECT_TRUE(Grpc::Common::parseBufferInstance(std::move(buffer), message)); const std::string nonce = message.response_nonce(); if (!nonce.empty()) { diff --git a/test/integration/README.md b/test/integration/README.md index f7577409ca569..bd5283ee1c84c 100644 --- a/test/integration/README.md +++ b/test/integration/README.md @@ -79,7 +79,7 @@ config_helper_.addConfigModifier([&](envoy::config::bootstrap::v2::Bootstrap& bo An example of modifying `HttpConnectionManager` to change Envoy’s HTTP/1.1 processing: ```c++ -config_helper_.addConfigModifier([&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) -> void { +config_helper_.addConfigModifier([&](envoy::extensions::filters::network::http_connection_manager::v3alpha::HttpConnectionManager& hcm) -> void { envoy::api::v2::core::Http1ProtocolOptions options; options.mutable_allow_absolute_url()->set_value(true); hcm.mutable_http_protocol_options()->CopyFrom(options); diff --git a/test/integration/api_version_integration_test.cc b/test/integration/api_version_integration_test.cc index 5b99f090dc544..ecb0f9126b8d1 100644 --- a/test/integration/api_version_integration_test.cc +++ b/test/integration/api_version_integration_test.cc @@ -8,8 +8,9 @@ namespace Envoy { namespace { using Params = - std::tuple; + std::tuple; class ApiVersionIntegrationTest : public testing::TestWithParam, public HttpIntegrationTest { @@ -23,34 +24,42 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, } static std::string paramsToString(const testing::TestParamInfo& p) { - return fmt::format("{}_{}_{}_Resource_{}_Transport_{}", - std::get<0>(p.param) == Network::Address::IpVersion::v4 ? "IPv4" : "IPv6", - std::get<1>(p.param) ? "ADS" : "SingletonXds", - envoy::api::v2::core::ApiConfigSource::ApiType_Name(std::get<2>(p.param)), - envoy::api::v2::core::ApiVersion_Name(std::get<3>(p.param)), - envoy::api::v2::core::ApiVersion_Name(std::get<4>(p.param))); + return fmt::format( + "{}_{}_{}_Resource_{}_Transport_{}", + std::get<0>(p.param) == Network::Address::IpVersion::v4 ? "IPv4" : "IPv6", + std::get<1>(p.param) ? "ADS" : "SingletonXds", + envoy::config::core::v3alpha::ApiConfigSource::ApiType_Name(std::get<2>(p.param)), + envoy::config::core::v3alpha::ApiVersion_Name(std::get<3>(p.param)), + envoy::config::core::v3alpha::ApiVersion_Name(std::get<4>(p.param))); } Network::Address::IpVersion ipVersion() const { return std::get<0>(GetParam()); } bool ads() const { return std::get<1>(GetParam()); } - envoy::api::v2::core::ApiConfigSource::ApiType apiType() const { return std::get<2>(GetParam()); } - envoy::api::v2::core::ApiVersion resourceApiVersion() const { return std::get<3>(GetParam()); } - envoy::api::v2::core::ApiVersion transportApiVersion() const { return std::get<4>(GetParam()); } + envoy::config::core::v3alpha::ApiConfigSource::ApiType apiType() const { + return std::get<2>(GetParam()); + } + envoy::config::core::v3alpha::ApiVersion resourceApiVersion() const { + return std::get<3>(GetParam()); + } + envoy::config::core::v3alpha::ApiVersion transportApiVersion() const { + return std::get<4>(GetParam()); + } void initialize() override { - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { - auto* xds_cluster = bootstrap.mutable_static_resources()->add_clusters(); - xds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); - xds_cluster->set_name("xds_cluster"); - xds_cluster->mutable_http2_protocol_options(); - if (ads()) { - auto* api_config_source = bootstrap.mutable_dynamic_resources()->mutable_ads_config(); - api_config_source->set_transport_api_version(transportApiVersion()); - api_config_source->set_api_type(apiType()); - auto* grpc_service = api_config_source->add_grpc_services(); - grpc_service->mutable_envoy_grpc()->set_cluster_name("xds_cluster"); - } - }); + config_helper_.addConfigModifier( + [this](envoy::config::bootstrap::v3alpha::Bootstrap& bootstrap) { + auto* xds_cluster = bootstrap.mutable_static_resources()->add_clusters(); + xds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); + xds_cluster->set_name("xds_cluster"); + xds_cluster->mutable_http2_protocol_options(); + if (ads()) { + auto* api_config_source = bootstrap.mutable_dynamic_resources()->mutable_ads_config(); + api_config_source->set_transport_api_version(transportApiVersion()); + api_config_source->set_api_type(apiType()); + auto* grpc_service = api_config_source->add_grpc_services(); + grpc_service->mutable_envoy_grpc()->set_cluster_name("xds_cluster"); + } + }); setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); HttpIntegrationTest::initialize(); if (xds_stream_ == nullptr) { @@ -64,7 +73,7 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, } } - void setupConfigSource(envoy::api::v2::core::ConfigSource& config_source) { + void setupConfigSource(envoy::config::core::v3alpha::ConfigSource& config_source) { config_source.set_resource_api_version(resourceApiVersion()); if (ads()) { config_source.mutable_ads(); @@ -73,7 +82,7 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, auto* api_config_source = config_source.mutable_api_config_source(); api_config_source->set_transport_api_version(transportApiVersion()); api_config_source->set_api_type(apiType()); - if (apiType() == envoy::api::v2::core::ApiConfigSource::REST) { + if (apiType() == envoy::config::core::v3alpha::ApiConfigSource::REST) { api_config_source->add_cluster_names("xds_cluster"); api_config_source->mutable_refresh_delay()->set_seconds(1); } else { @@ -90,6 +99,10 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, const std::string& expected_v3alpha_rest_endpoint, const std::string& expected_v2_type_url, const std::string& expected_v3alpha_type_url) { + // Only with ADS do we allow mixed transport/resource versions. + if (!ads() && resourceApiVersion() != transportApiVersion()) { + return AssertionSuccess(); + } std::string expected_endpoint; std::string expected_type_url; std::string actual_type_url; @@ -98,10 +111,10 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, const char ads_v3alpha_delta_endpoint[] = "/envoy.service.discovery.v3alpha.AggregatedDiscoveryService/StreamAggregatedResources"; switch (transportApiVersion()) { - case envoy::api::v2::core::ApiVersion::AUTO: - case envoy::api::v2::core::ApiVersion::V2: { + case envoy::config::core::v3alpha::ApiVersion::AUTO: + case envoy::config::core::v3alpha::ApiVersion::V2: { switch (apiType()) { - case envoy::api::v2::core::ApiConfigSource::GRPC: { + case envoy::config::core::v3alpha::ApiConfigSource::GRPC: { API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); xds_stream_->startGrpcStream(); @@ -109,7 +122,7 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, expected_endpoint = ads() ? ads_v2_sotw_endpoint : expected_v2_sotw_endpoint; break; } - case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { + case envoy::config::core::v3alpha::ApiConfigSource::DELTA_GRPC: { API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) delta_discovery_request; VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, delta_discovery_request)); xds_stream_->startGrpcStream(); @@ -117,7 +130,7 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, expected_endpoint = expected_v2_delta_endpoint; break; } - case envoy::api::v2::core::ApiConfigSource::REST: { + case envoy::config::core::v3alpha::ApiConfigSource::REST: { API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; VERIFY_ASSERTION(xds_stream_->waitForEndStream(*dispatcher_)); MessageUtil::loadFromJson(xds_stream_->body().toString(), discovery_request, @@ -132,24 +145,24 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, } break; } - case envoy::api::v2::core::ApiVersion::V3ALPHA: { + case envoy::config::core::v3alpha::ApiVersion::V3ALPHA: { switch (apiType()) { - case envoy::api::v2::core::ApiConfigSource::GRPC: { - API_NO_BOOST(envoy::api::v3alpha::DiscoveryRequest) discovery_request; + case envoy::config::core::v3alpha::ApiConfigSource::GRPC: { + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); actual_type_url = discovery_request.type_url(); expected_endpoint = ads() ? ads_v3alpha_delta_endpoint : expected_v3alpha_sotw_endpoint; break; } - case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { - API_NO_BOOST(envoy::api::v3alpha::DeltaDiscoveryRequest) delta_discovery_request; + case envoy::config::core::v3alpha::ApiConfigSource::DELTA_GRPC: { + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) delta_discovery_request; VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, delta_discovery_request)); actual_type_url = delta_discovery_request.type_url(); expected_endpoint = expected_v3alpha_delta_endpoint; break; } - case envoy::api::v2::core::ApiConfigSource::REST: { - API_NO_BOOST(envoy::api::v3alpha::DiscoveryRequest) discovery_request; + case envoy::config::core::v3alpha::ApiConfigSource::REST: { + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; VERIFY_ASSERTION(xds_stream_->waitForEndStream(*dispatcher_)); MessageUtil::loadFromJson(xds_stream_->body().toString(), discovery_request, ProtobufMessage::getStrictValidationVisitor()); @@ -167,11 +180,11 @@ class ApiVersionIntegrationTest : public testing::TestWithParam, NOT_REACHED_GCOVR_EXCL_LINE; } switch (resourceApiVersion()) { - case envoy::api::v2::core::ApiVersion::AUTO: - case envoy::api::v2::core::ApiVersion::V2: + case envoy::config::core::v3alpha::ApiVersion::AUTO: + case envoy::config::core::v3alpha::ApiVersion::V2: expected_type_url = expected_v2_type_url; break; - case envoy::api::v2::core::ApiVersion::V3ALPHA: + case envoy::config::core::v3alpha::ApiVersion::V3ALPHA: expected_type_url = expected_v3alpha_type_url; break; default: @@ -212,84 +225,85 @@ INSTANTIATE_TEST_SUITE_P( SingletonApiConfigSourcesExplicitApiVersions, ApiVersionIntegrationTest, testing::Combine(testing::Values(TestEnvironment::getIpVersionsForTest()[0]), testing::Values(false), - testing::Values(envoy::api::v2::core::ApiConfigSource::REST, - envoy::api::v2::core::ApiConfigSource::GRPC, - envoy::api::v2::core::ApiConfigSource::DELTA_GRPC), - testing::Values(envoy::api::v2::core::ApiVersion::V2, - envoy::api::v2::core::ApiVersion::V3ALPHA), - testing::Values(envoy::api::v2::core::ApiVersion::V2, - envoy::api::v2::core::ApiVersion::V3ALPHA)), + testing::Values(envoy::config::core::v3alpha::ApiConfigSource::REST, + envoy::config::core::v3alpha::ApiConfigSource::GRPC, + envoy::config::core::v3alpha::ApiConfigSource::DELTA_GRPC), + testing::Values(envoy::config::core::v3alpha::ApiVersion::V2, + envoy::config::core::v3alpha::ApiVersion::V3ALPHA), + testing::Values(envoy::config::core::v3alpha::ApiVersion::V2, + envoy::config::core::v3alpha::ApiVersion::V3ALPHA)), ApiVersionIntegrationTest::paramsToString); INSTANTIATE_TEST_SUITE_P( SingletonApiConfigSourcesAutoApiVersions, ApiVersionIntegrationTest, testing::Combine(testing::Values(TestEnvironment::getIpVersionsForTest()[0]), testing::Values(false), - testing::Values(envoy::api::v2::core::ApiConfigSource::REST, - envoy::api::v2::core::ApiConfigSource::GRPC, - envoy::api::v2::core::ApiConfigSource::DELTA_GRPC), - testing::Values(envoy::api::v2::core::ApiVersion::AUTO), - testing::Values(envoy::api::v2::core::ApiVersion::AUTO)), + testing::Values(envoy::config::core::v3alpha::ApiConfigSource::REST, + envoy::config::core::v3alpha::ApiConfigSource::GRPC, + envoy::config::core::v3alpha::ApiConfigSource::DELTA_GRPC), + testing::Values(envoy::config::core::v3alpha::ApiVersion::AUTO), + testing::Values(envoy::config::core::v3alpha::ApiVersion::AUTO)), ApiVersionIntegrationTest::paramsToString); INSTANTIATE_TEST_SUITE_P( AdsApiConfigSourcesExplicitApiVersions, ApiVersionIntegrationTest, testing::Combine(testing::Values(TestEnvironment::getIpVersionsForTest()[0]), testing::Values(true), - testing::Values(envoy::api::v2::core::ApiConfigSource::GRPC), - testing::Values(envoy::api::v2::core::ApiVersion::V2, - envoy::api::v2::core::ApiVersion::V3ALPHA), - testing::Values(envoy::api::v2::core::ApiVersion::V2, - envoy::api::v2::core::ApiVersion::V3ALPHA)), + testing::Values(envoy::config::core::v3alpha::ApiConfigSource::GRPC), + testing::Values(envoy::config::core::v3alpha::ApiVersion::V2, + envoy::config::core::v3alpha::ApiVersion::V3ALPHA), + testing::Values(envoy::config::core::v3alpha::ApiVersion::V2, + envoy::config::core::v3alpha::ApiVersion::V3ALPHA)), ApiVersionIntegrationTest::paramsToString); TEST_P(ApiVersionIntegrationTest, Lds) { - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3alpha::Bootstrap& bootstrap) { setupConfigSource(*bootstrap.mutable_dynamic_resources()->mutable_lds_config()); }); initialize(); ASSERT_TRUE(validateDiscoveryRequest( "/envoy.api.v2.ListenerDiscoveryService/StreamListeners", "/envoy.api.v2.ListenerDiscoveryService/DeltaListeners", "/v2/discovery:listeners", - "/envoy.api.v3alpha.ListenerDiscoveryService/StreamListeners", - "/envoy.api.v3alpha.ListenerDiscoveryService/DeltaListeners", "/v3alpha/discovery:listeners", - "type.googleapis.com/envoy.api.v2.Listener", - "type.googleapis.com/envoy.api.v3alpha.Listener")); + "/envoy.service.listener.v3alpha.ListenerDiscoveryService/StreamListeners", + "/envoy.service.listener.v3alpha.ListenerDiscoveryService/DeltaListeners", + "/v3alpha/discovery:listeners", "type.googleapis.com/envoy.api.v2.Listener", + "type.googleapis.com/envoy.config.listener.v3alpha.Listener")); } TEST_P(ApiVersionIntegrationTest, Cds) { - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3alpha::Bootstrap& bootstrap) { setupConfigSource(*bootstrap.mutable_dynamic_resources()->mutable_cds_config()); }); initialize(); ASSERT_TRUE(validateDiscoveryRequest( "/envoy.api.v2.ClusterDiscoveryService/StreamClusters", "/envoy.api.v2.ClusterDiscoveryService/DeltaClusters", "/v2/discovery:clusters", - "/envoy.api.v3alpha.ClusterDiscoveryService/StreamClusters", - "/envoy.api.v3alpha.ClusterDiscoveryService/DeltaClusters", "/v3alpha/discovery:clusters", - "type.googleapis.com/envoy.api.v2.Cluster", "type.googleapis.com/envoy.api.v3alpha.Cluster")); + "/envoy.service.cluster.v3alpha.ClusterDiscoveryService/StreamClusters", + "/envoy.service.cluster.v3alpha.ClusterDiscoveryService/DeltaClusters", + "/v3alpha/discovery:clusters", "type.googleapis.com/envoy.api.v2.Cluster", + "type.googleapis.com/envoy.config.cluster.v3alpha.Cluster")); } TEST_P(ApiVersionIntegrationTest, Eds) { - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3alpha::Bootstrap& bootstrap) { auto* cluster = bootstrap.mutable_static_resources()->add_clusters(); cluster->MergeFrom(bootstrap.static_resources().clusters(0)); cluster->set_name("some_cluster"); - cluster->set_type(envoy::api::v2::Cluster::EDS); + cluster->set_type(envoy::config::cluster::v3alpha::Cluster::EDS); setupConfigSource(*cluster->mutable_eds_cluster_config()->mutable_eds_config()); }); initialize(); ASSERT_TRUE(validateDiscoveryRequest( "/envoy.api.v2.EndpointDiscoveryService/StreamEndpoints", "/envoy.api.v2.EndpointDiscoveryService/DeltaEndpoints", "/v2/discovery:endpoints", - "/envoy.api.v3alpha.EndpointDiscoveryService/StreamEndpoints", - "/envoy.api.v3alpha.EndpointDiscoveryService/DeltaEndpoints", "/v3alpha/discovery:endpoints", - "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", - "type.googleapis.com/envoy.api.v3alpha.ClusterLoadAssignment")); + "/envoy.service.endpoint.v3alpha.EndpointDiscoveryService/StreamEndpoints", + "/envoy.service.endpoint.v3alpha.EndpointDiscoveryService/DeltaEndpoints", + "/v3alpha/discovery:endpoints", "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", + "type.googleapis.com/envoy.config.endpoint.v3alpha.ClusterLoadAssignment")); } TEST_P(ApiVersionIntegrationTest, Rtds) { - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3alpha::Bootstrap& bootstrap) { auto* admin_layer = bootstrap.mutable_layered_runtime()->add_layers(); admin_layer->set_name("admin layer"); admin_layer->mutable_admin_layer(); @@ -301,20 +315,20 @@ TEST_P(ApiVersionIntegrationTest, Rtds) { ASSERT_TRUE(validateDiscoveryRequest( "/envoy.service.discovery.v2.RuntimeDiscoveryService/StreamRuntime", "/envoy.service.discovery.v2.RuntimeDiscoveryService/DeltaRuntime", "/v2/discovery:runtime", - "/envoy.service.discovery.v3alpha.RuntimeDiscoveryService/StreamRuntime", - "/envoy.service.discovery.v3alpha.RuntimeDiscoveryService/DeltaRuntime", + "/envoy.service.runtime.v3alpha.RuntimeDiscoveryService/StreamRuntime", + "/envoy.service.runtime.v3alpha.RuntimeDiscoveryService/DeltaRuntime", "/v3alpha/discovery:runtime", "type.googleapis.com/envoy.service.discovery.v2.Runtime", - "type.googleapis.com/envoy.service.discovery.v3alpha.Runtime")); + "type.googleapis.com/envoy.service.runtime.v3alpha.Runtime")); } TEST_P(ApiVersionIntegrationTest, Rds) { // TODO(htuch): this segfaults, this is likely some untested existing issue. - if (apiType() == envoy::api::v2::core::ApiConfigSource::DELTA_GRPC) { + if (apiType() == envoy::config::core::v3alpha::ApiConfigSource::DELTA_GRPC) { return; } config_helper_.addConfigModifier( - [this](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& - http_connection_manager) { + [this](envoy::extensions::filters::network::http_connection_manager::v3alpha:: + HttpConnectionManager& http_connection_manager) { auto* rds = http_connection_manager.mutable_rds(); rds->set_route_config_name("rds"); setupConfigSource(*rds->mutable_config_source()); @@ -323,10 +337,10 @@ TEST_P(ApiVersionIntegrationTest, Rds) { ASSERT_TRUE(validateDiscoveryRequest( "/envoy.api.v2.RouteDiscoveryService/StreamRoutes", "/envoy.api.v2.RouteDiscoveryService/DeltaRoutes", "/v2/discovery:routes", - "/envoy.api.v3alpha.RouteDiscoveryService/StreamRoutes", - "/envoy.api.v3alpha.RouteDiscoveryService/DeltaRoutes", "/v3alpha/discovery:routes", + "/envoy.service.route.v3alpha.RouteDiscoveryService/StreamRoutes", + "/envoy.service.route.v3alpha.RouteDiscoveryService/DeltaRoutes", "/v3alpha/discovery:routes", "type.googleapis.com/envoy.api.v2.RouteConfiguration", - "type.googleapis.com/envoy.api.v3alpha.RouteConfiguration")); + "type.googleapis.com/envoy.config.route.v3alpha.RouteConfiguration")); } // TODO(htuch): add VHDS tests once VHDS lands. @@ -335,8 +349,8 @@ TEST_P(ApiVersionIntegrationTest, Rds) { TEST_P(ApiVersionIntegrationTest, Srds) { config_helper_.addConfigModifier( - [this](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& - http_connection_manager) { + [this](envoy::extensions::filters::network::http_connection_manager::v3alpha:: + HttpConnectionManager& http_connection_manager) { auto* scoped_routes = http_connection_manager.mutable_scoped_routes(); scoped_routes->set_name("scoped_routes"); const std::string& scope_key_builder_config_yaml = R"EOF( @@ -348,8 +362,8 @@ TEST_P(ApiVersionIntegrationTest, Srds) { key: x-foo-key separator: = )EOF"; - envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::ScopeKeyBuilder - scope_key_builder; + envoy::extensions::filters::network::http_connection_manager::v3alpha::ScopedRoutes:: + ScopeKeyBuilder scope_key_builder; TestUtility::loadFromYaml(scope_key_builder_config_yaml, *scoped_routes->mutable_scope_key_builder()); setupConfigSource(*scoped_routes->mutable_scoped_rds()->mutable_scoped_rds_config_source()); @@ -363,14 +377,14 @@ TEST_P(ApiVersionIntegrationTest, Srds) { "/envoy.service.route.v3alpha.ScopedRoutesDiscoveryService/DeltaScopedRoutes", "/v3alpha/discovery:scoped-routes", "type.googleapis.com/envoy.api.v2.ScopedRouteConfiguration", - "type.googleapis.com/envoy.service.route.v3alpha.ScopedRouteConfiguration")); + "type.googleapis.com/envoy.config.route.v3alpha.ScopedRouteConfiguration")); } TEST_P(ApiVersionIntegrationTest, Sds) { - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3alpha::Bootstrap& bootstrap) { auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); auto* transport_socket = listener->mutable_filter_chains(0)->mutable_transport_socket(); - envoy::api::v2::auth::DownstreamTlsContext tls_context; + envoy::extensions::transport_sockets::tls::v3alpha::DownstreamTlsContext tls_context; auto* common_tls_context = tls_context.mutable_common_tls_context(); auto* secret_config = common_tls_context->add_tls_certificate_sds_secret_configs(); secret_config->set_name("sds"); @@ -382,10 +396,10 @@ TEST_P(ApiVersionIntegrationTest, Sds) { ASSERT_TRUE(validateDiscoveryRequest( "/envoy.service.discovery.v2.SecretDiscoveryService/StreamSecrets", "/envoy.service.discovery.v2.SecretDiscoveryService/DeltaSecrets", "/v2/discovery:secrets", - "/envoy.service.discovery.v3alpha.SecretDiscoveryService/StreamSecrets", - "/envoy.service.discovery.v3alpha.SecretDiscoveryService/DeltaSecrets", + "/envoy.service.secret.v3alpha.SecretDiscoveryService/StreamSecrets", + "/envoy.service.secret.v3alpha.SecretDiscoveryService/DeltaSecrets", "/v3alpha/discovery:secrets", "type.googleapis.com/envoy.api.v2.auth.Secret", - "type.googleapis.com/envoy.api.v3alpha.auth.Secret")); + "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3alpha.Secret")); } } // namespace diff --git a/test/integration/cds_integration_test.cc b/test/integration/cds_integration_test.cc index 54418e70e1701..e9bb18187f7ed 100644 --- a/test/integration/cds_integration_test.cc +++ b/test/integration/cds_integration_test.cc @@ -248,7 +248,7 @@ TEST_P(CdsIntegrationTest, VersionsRememberedAfterReconnect) { acceptXdsConnection(); // Upon reconnecting, the Envoy should tell us its current resource versions. - envoy::service::discovery::v3alpha::DeltaDiscoveryRequest request; + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) request; result = xds_stream_->waitForGrpcMessage(*dispatcher_, request); RELEASE_ASSERT(result, result.message()); const auto& initial_resource_versions = request.initial_resource_versions(); From 398e738f7a2ef60c9352305d26f6414b97ee7910 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Tue, 7 Jan 2020 12:50:22 -0500 Subject: [PATCH 17/17] Remove HTD debug messages. Signed-off-by: Harvey Tuch --- source/common/config/type_to_endpoint.cc | 1 - test/integration/load_stats_integration_test.cc | 3 --- 2 files changed, 4 deletions(-) diff --git a/source/common/config/type_to_endpoint.cc b/source/common/config/type_to_endpoint.cc index 5dcd219dbfc17..1d0285a8ca4d7 100644 --- a/source/common/config/type_to_endpoint.cc +++ b/source/common/config/type_to_endpoint.cc @@ -92,7 +92,6 @@ const Protobuf::MethodDescriptor& sotwGrpcMethod(absl::string_view type_url) { const Protobuf::MethodDescriptor& restMethod(absl::string_view type_url) { const auto it = typeUrlToServiceMap().find(static_cast(type_url)); - ENVOY_LOG_MISC(debug, "HTD {}", type_url); ASSERT(it != typeUrlToServiceMap().cend()); return *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(it->second.rest_method_); } diff --git a/test/integration/load_stats_integration_test.cc b/test/integration/load_stats_integration_test.cc index e6036077511f4..94e107d711e40 100644 --- a/test/integration/load_stats_integration_test.cc +++ b/test/integration/load_stats_integration_test.cc @@ -270,9 +270,6 @@ class LoadStatsIntegrationTest : public testing::TestWithParamClear(); } mergeLoadStats(loadstats_request, local_loadstats_request); - if (!loadstats_request.cluster_stats().empty()) { - ENVOY_LOG_MISC(debug, "HTD {}", loadstats_request.cluster_stats()[0].DebugString()); - } EXPECT_EQ("POST", loadstats_stream_->headers().Method()->value().getStringView()); EXPECT_EQ("/envoy.service.load_stats.v2.LoadReportingService/StreamLoadStats",