From fc516dc33de2e24660100ba60542f8a825c8d030 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Thu, 14 Sep 2023 14:49:57 -0700 Subject: [PATCH] [Session Affinity] Update validation and add a test case (#34348) --- .../xds/xds_http_stateful_session_filter.cc | 1 - test/core/xds/xds_http_filters_test.cc | 15 +++-- .../xds/xds_override_host_end2end_test.cc | 59 +++++++++++++++---- 3 files changed, 55 insertions(+), 20 deletions(-) diff --git a/src/core/ext/xds/xds_http_stateful_session_filter.cc b/src/core/ext/xds/xds_http_stateful_session_filter.cc index f58152b4e46..2e9016c21f2 100644 --- a/src/core/ext/xds/xds_http_stateful_session_filter.cc +++ b/src/core/ext/xds/xds_http_stateful_session_filter.cc @@ -77,7 +77,6 @@ Json::Object ValidateStatefulSession( envoy_extensions_filters_http_stateful_session_v3_StatefulSession_session_state( stateful_session); if (session_state == nullptr) { - errors->AddError("field not present"); return {}; } ValidationErrors::ScopedField field2(errors, ".typed_config"); diff --git a/test/core/xds/xds_http_filters_test.cc b/test/core/xds/xds_http_filters_test.cc index ff40ac2e29d..7d859e52430 100644 --- a/test/core/xds/xds_http_filters_test.cc +++ b/test/core/xds/xds_http_filters_test.cc @@ -1333,14 +1333,13 @@ TEST_P(XdsStatefulSessionFilterConfigTest, PathAndTtl) { TEST_P(XdsStatefulSessionFilterConfigTest, SessionStateUnset) { auto config = GenerateConfig(StatefulSession()); - absl::Status status = errors_.status(absl::StatusCode::kInvalidArgument, - "errors validating filter config"); - ASSERT_EQ(status.code(), absl::StatusCode::kInvalidArgument); - EXPECT_EQ( - status.message(), - absl::StrCat("errors validating filter config: [field:", FieldPrefix(), - ".session_state error:field not present]")) - << status; + ASSERT_TRUE(errors_.ok()) << errors_.status( + absl::StatusCode::kInvalidArgument, "unexpected errors"); + ASSERT_TRUE(config.has_value()); + EXPECT_EQ(config->config_proto_type_name, + GetParam() ? filter_->OverrideConfigProtoName() + : filter_->ConfigProtoName()); + EXPECT_EQ(config->config, Json::FromObject({})) << JsonDump(config->config); } TEST_P(XdsStatefulSessionFilterConfigTest, CookieNotPresent) { diff --git a/test/cpp/end2end/xds/xds_override_host_end2end_test.cc b/test/cpp/end2end/xds/xds_override_host_end2end_test.cc index e82540d76d6..09ea788af12 100644 --- a/test/cpp/end2end/xds/xds_override_host_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_override_host_end2end_test.cc @@ -41,12 +41,15 @@ namespace testing { namespace { using ::envoy::config::core::v3::HealthStatus; using ::envoy::extensions::filters::http::stateful_session::v3::StatefulSession; +using ::envoy::extensions::filters::http::stateful_session::v3:: + StatefulSessionPerRoute; using ::envoy::extensions::filters::network::http_connection_manager::v3:: HttpFilter; using ::envoy::extensions::http::stateful_session::cookie::v3 :: CookieBasedSessionState; constexpr absl::string_view kCookieName = "grpc_session_cookie"; +constexpr absl::string_view kFilterName = "envoy.stateful_session"; class OverrideHostTest : public XdsEnd2endTest { protected: @@ -90,7 +93,7 @@ class OverrideHostTest : public XdsEnd2endTest { } EXPECT_EQ(values.size(), 1); if (values.size() == 1) { - return {{"cookie", absl::StrFormat("%s=%s", kCookieName, values[0])}}; + return {{"cookie", absl::StrFormat("%s=%s", cookie_name, values[0])}}; } else { return {}; } @@ -99,12 +102,16 @@ class OverrideHostTest : public XdsEnd2endTest { // Builds a Listener with Fault Injection filter config. If the http_fault // is nullptr, then assign an empty filter config. This filter config is // required to enable the fault injection features. - Listener BuildListenerWithStatefulSessionFilter() { - CookieBasedSessionState cookie_state; - cookie_state.mutable_cookie()->set_name(std::string(kCookieName)); + Listener BuildListenerWithStatefulSessionFilter( + absl::string_view cookie_name = kCookieName) { StatefulSession stateful_session; - stateful_session.mutable_session_state()->mutable_typed_config()->PackFrom( - cookie_state); + if (!cookie_name.empty()) { + CookieBasedSessionState cookie_state; + cookie_state.mutable_cookie()->set_name(std::string(cookie_name)); + stateful_session.mutable_session_state() + ->mutable_typed_config() + ->PackFrom(cookie_state); + } // HttpConnectionManager http_connection_manager; Listener listener = default_listener_; HttpConnectionManager http_connection_manager = @@ -113,7 +120,7 @@ class OverrideHostTest : public XdsEnd2endTest { HttpFilter* session_filter = http_connection_manager.mutable_http_filters(0); *http_connection_manager.add_http_filters() = *session_filter; - session_filter->set_name("envoy.stateful_session"); + session_filter->set_name(kFilterName); session_filter->mutable_typed_config()->PackFrom(stateful_session); ClientHcmAccessor().Pack(http_connection_manager, &listener); return listener; @@ -125,9 +132,10 @@ class OverrideHostTest : public XdsEnd2endTest { // to obtain the cookie. max_requests_per_backend argument specifies // the number of requests per backend to send. std::vector> - GetAffinityCookieHeaderForBackend(grpc_core::DebugLocation debug_location, - size_t backend_index, - size_t max_requests_per_backend = 1) { + GetAffinityCookieHeaderForBackend( + grpc_core::DebugLocation debug_location, size_t backend_index, + size_t max_requests_per_backend = 1, + absl::string_view cookie_name = kCookieName) { EXPECT_LT(backend_index, backends_.size()); if (backend_index >= backends_.size()) { return {}; @@ -149,7 +157,8 @@ class OverrideHostTest : public XdsEnd2endTest { backend->backend_service2()->request_count(); ResetBackendCounters(); if (count == 1) { - return GetHeadersWithSessionCookie(server_initial_metadata); + return GetHeadersWithSessionCookie(server_initial_metadata, + cookie_name); } } ADD_FAILURE_AT(debug_location.file(), debug_location.line()) @@ -440,6 +449,34 @@ TEST_P(OverrideHostTest, ClusterGoneHostStays) { GetAffinityCookieHeaderForBackend(DEBUG_LOCATION, 1, kNumEchoRpcs / 3)); } +TEST_P(OverrideHostTest, EnablePerRoute) { + CreateAndStartBackends(2); + RouteConfiguration route_config = default_route_config_; + StatefulSessionPerRoute stateful_session_per_route; + auto* session_state = stateful_session_per_route.mutable_stateful_session() + ->mutable_session_state(); + session_state->set_name("envoy.http.stateful_session.cookie"); + CookieBasedSessionState cookie_config; + cookie_config.mutable_cookie()->set_name(kCookieName); + session_state->mutable_typed_config()->PackFrom(cookie_config); + auto* route = route_config.mutable_virtual_hosts(0)->mutable_routes(0); + google::protobuf::Any any; + any.PackFrom(stateful_session_per_route); + route->mutable_typed_per_filter_config()->emplace(kFilterName, any); + SetListenerAndRouteConfiguration(balancer_.get(), + BuildListenerWithStatefulSessionFilter(""), + route_config); + balancer_->ads_service()->SetEdsResource(BuildEdsResource(EdsResourceArgs( + {{"locality0", {CreateEndpoint(0), CreateEndpoint(1)}}}))); + WaitForAllBackends(DEBUG_LOCATION); + // Get cookie for backend #0. + auto session_cookie = GetAffinityCookieHeaderForBackend(DEBUG_LOCATION, 0); + ASSERT_FALSE(session_cookie.empty()); + // All requests go to the backend we specified + CheckRpcSendOk(DEBUG_LOCATION, 5, RpcOptions().set_metadata(session_cookie)); + EXPECT_EQ(backends_[0]->backend_service()->request_count(), 5); +} + } // namespace } // namespace testing } // namespace grpc