From ce2bca66e1725f3cad5b684826357a4bbde829b2 Mon Sep 17 00:00:00 2001 From: Rudrakh Panigrahi Date: Fri, 27 Mar 2026 15:35:30 +0530 Subject: [PATCH 1/2] feat: ability to set status for destination not found in stateful session Signed-off-by: Rudrakh Panigrahi --- .../v3/stateful_session.proto | 14 ++- changelogs/current.yaml | 10 ++ envoy/upstream/load_balancer.h | 18 +++- source/common/router/router.cc | 24 +++-- source/common/router/router.h | 9 +- .../common/upstream/cluster_manager_impl.cc | 30 ++++-- source/common/upstream/host_utility.cc | 18 ++-- source/common/upstream/host_utility.h | 26 +++++- .../http/stateful_session/stateful_session.cc | 9 +- .../http/stateful_session/stateful_session.h | 5 + .../upstream/cluster_manager_impl_test.cc | 17 ++++ test/common/upstream/host_utility_test.cc | 87 +++++++++--------- .../stateful_session/stateful_session_test.cc | 91 ++++++++++++++++++- 13 files changed, 270 insertions(+), 88 deletions(-) diff --git a/api/envoy/extensions/filters/http/stateful_session/v3/stateful_session.proto b/api/envoy/extensions/filters/http/stateful_session/v3/stateful_session.proto index b3e5e53af852f..9639055b8f3a0 100644 --- a/api/envoy/extensions/filters/http/stateful_session/v3/stateful_session.proto +++ b/api/envoy/extensions/filters/http/stateful_session/v3/stateful_session.proto @@ -25,9 +25,11 @@ message StatefulSession { config.core.v3.TypedExtensionConfig session_state = 1; // Determines whether the HTTP request must be strictly routed to the requested destination. When set to ``true``, - // if the requested destination is unavailable, Envoy will return a 503 status code. The default value is ``false``, - // which allows Envoy to fall back to its load balancing mechanism. In this case, if the requested destination is not - // found, the request will be routed according to the load balancing algorithm. + // if the requested destination is not found in the set of available endpoints, Envoy will return a status code + // determined by ``status_on_strict_destination_not_found``. If the destination exists but is unhealthy, Envoy will + // always return ``503`` regardless of ``status_on_strict_destination_not_found``. The default value is ``false``, + // which allows Envoy to fall back to its load balancing mechanism and route the request according to the load + // balancing algorithm. bool strict = 2; // Optional stat prefix. If specified, the filter will emit statistics in the @@ -38,6 +40,12 @@ message StatefulSession { // Per-route configuration overrides do not support statistics and will not emit stats even if this field is set // in the per-route config. string stat_prefix = 3; + + // The HTTP status code to return when ``strict`` mode is enabled and the requested destination + // is not found in the set of available endpoints. This does not apply when the destination exists + // but is unhealthy. This field has no effect when ``strict`` is set to ``false`` and will be + // ignored. Defaults to 503 (Service Unavailable) if not specified or set to 0. + uint32 status_on_strict_destination_not_found = 4; } message StatefulSessionPerRoute { diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 8141f70f186e8..f94ff93baaac2 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -861,4 +861,14 @@ new_features: ``clear_route_cache`` is set, the filter will clear the route cache for the current request after applying filter state updates. This is necessary if the route configuration may depend on the filter state values set. +- area: stateful_session + change: | + Added :ref:`status_on_strict_destination_not_found + ` + to the :ref:`stateful session filter `. When + :ref:`strict ` + mode is enabled and the requested destination is not found in the set of available endpoints, Envoy + will return the configured HTTP status code instead of the default ``503``. This does not apply when + the destination exists but is unhealthy. + deprecated: diff --git a/envoy/upstream/load_balancer.h b/envoy/upstream/load_balancer.h index 3ef70a0c09fbb..32f7f2737b7ba 100644 --- a/envoy/upstream/load_balancer.h +++ b/envoy/upstream/load_balancer.h @@ -6,6 +6,7 @@ #include "envoy/common/optref.h" #include "envoy/common/pure.h" #include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/http/codes.h" #include "envoy/network/transport_socket.h" #include "envoy/router/router.h" #include "envoy/stream_info/stream_info.h" @@ -30,14 +31,14 @@ namespace Upstream { using ClusterProto = envoy::config::cluster::v3::Cluster; /* - * A handle to allow cancelation of asynchronous host selection. + * A handle to allow cancellation of asynchronous host selection. * If chooseHost returns a HostSelectionResponse with an AsyncHostSelectionHandle - * handle, and the endpoint does not wish to receive onAsyncHostSelction call, + * handle, and the endpoint does not wish to receive onAsyncHostSelection call, * it must call cancel() on the provided handle. * * Please note that the AsyncHostSelectionHandle may be deleted after the - * cancel() call. It is up to the implemention of the asynchronous load balancer - * to ensure the cancelation state persists until the load balancer checks it. + * cancel() call. It is up to the implementation of the asynchronous load balancer + * to ensure the cancellation state persists until the load balancer checks it. */ class AsyncHostSelectionHandle { public: @@ -53,7 +54,7 @@ class AsyncHostSelectionHandle { * load balancing, returns an AsyncHostSelectionHandle handle. * * If it returns a AsyncHostSelectionHandle handle, the load balancer guarantees an - * eventual call to LoadBalancerContext::onAsyncHostSelction unless + * eventual call to LoadBalancerContext::onAsyncHostSelection unless * AsyncHostSelectionHandle::cancel is called. */ struct HostSelectionResponse { @@ -66,6 +67,9 @@ struct HostSelectionResponse { // Optional details if host selection fails (empty string implies no details). std::string details; std::unique_ptr cancelable; + // Optional HTTP status code to use when host selection fails because the strict override + // destination is missing from available endpoints. If not set, defaults to 503. + absl::optional failure_status; }; /** @@ -156,6 +160,10 @@ class LoadBalancerContext { // If strict and no valid host is found, the load balancer should return nullptr. // If not strict, the load balancer will select another host if the target host is not valid. bool strict{false}; + // The HTTP status code to return when strict mode is enabled and the target host + // is not found in the set of available endpoints. Does not apply when the host + // exists but is unhealthy. Defaults to 503 (ServiceUnavailable). + Http::Code status_on_strict_destination_not_found{Http::Code::ServiceUnavailable}; }; /** * Returns the host the load balancer should select directly. If the expected host exists and diff --git a/source/common/router/router.cc b/source/common/router/router.cc index fe02a3b8bf84f..f33ae8c4c4813 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -712,7 +712,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, // well as handling unsupported asynchronous host selection by treating it // as host selection failure and calling sendNoHealthyUpstreamResponse. continueDecodeHeaders(cluster, headers, end_stream, std::move(host_selection_response.host), - host_selection_response.details); + host_selection_response.details, host_selection_response.failure_status); return Http::FilterHeadersStatus::StopIteration; } @@ -754,13 +754,14 @@ void Filter::onAsyncHostSelection(Upstream::HostConstSharedPtr&& host, std::stri bool Filter::continueDecodeHeaders(Upstream::ThreadLocalCluster* cluster, Http::RequestHeaderMap& headers, bool end_stream, Upstream::HostConstSharedPtr&& selected_host, - absl::string_view host_selection_details) { + absl::string_view host_selection_details, + absl::optional failure_status) { callbacks_->streamInfo().downstreamTiming().setValue( "envoy.router.host_selection_end_ms", callbacks_->dispatcher().timeSource().monotonicTime()); std::unique_ptr generic_conn_pool = createConnPool(*cluster, selected_host); if (!generic_conn_pool) { - sendNoHealthyUpstreamResponse(host_selection_details); + sendNoHealthyUpstreamResponse(host_selection_details, failure_status); return false; } Upstream::HostDescriptionConstSharedPtr host = generic_conn_pool->host(); @@ -950,14 +951,16 @@ std::unique_ptr Filter::createConnPool(Upstream::ThreadLocalClu callbacks_->streamInfo().protocol(), this, *message); } -void Filter::sendNoHealthyUpstreamResponse(absl::string_view optional_details) { +void Filter::sendNoHealthyUpstreamResponse(absl::string_view optional_details, + absl::optional failure_status) { + const Http::Code status_code = failure_status.value_or(Http::Code::ServiceUnavailable); callbacks_->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::NoHealthyUpstream); - chargeUpstreamCode(Http::Code::ServiceUnavailable, {}, false); + chargeUpstreamCode(status_code, {}, false); absl::string_view details = optional_details.empty() ? StreamInfo::ResponseCodeDetails::get().NoHealthyUpstream : optional_details; - callbacks_->sendLocalReply(Http::Code::ServiceUnavailable, "no healthy upstream", modify_headers_, - absl::nullopt, details); + callbacks_->sendLocalReply(status_code, "no healthy upstream", modify_headers_, absl::nullopt, + details); } uint64_t Filter::calculateEffectiveBufferLimit() const { @@ -2265,7 +2268,7 @@ void Filter::doRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry // as host selection failure). continueDoRetry(can_send_early_data, can_use_http3, is_timeout_retry, std::move(host_selection_response.host), *cluster, - host_selection_response.details); + host_selection_response.details, host_selection_response.failure_status); } ENVOY_STREAM_LOG(debug, "Handling asynchronous host selection for retry\n", *callbacks_); @@ -2283,12 +2286,13 @@ void Filter::doRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry void Filter::continueDoRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry is_timeout_retry, Upstream::HostConstSharedPtr&& host, Upstream::ThreadLocalCluster& cluster, - absl::string_view host_selection_details) { + absl::string_view host_selection_details, + absl::optional failure_status) { callbacks_->streamInfo().downstreamTiming().setValue( "envoy.router.host_selection_end_ms", callbacks_->dispatcher().timeSource().monotonicTime()); std::unique_ptr generic_conn_pool = createConnPool(cluster, host); if (!generic_conn_pool) { - sendNoHealthyUpstreamResponse(host_selection_details); + sendNoHealthyUpstreamResponse(host_selection_details, failure_status); cleanup(); return; } diff --git a/source/common/router/router.h b/source/common/router/router.h index c6c3dd77e9041..86b09203fa21d 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -333,7 +333,8 @@ class Filter : Logger::Loggable, bool continueDecodeHeaders(Upstream::ThreadLocalCluster* cluster, Http::RequestHeaderMap& headers, bool end_stream, Upstream::HostConstSharedPtr&& host, - absl::string_view host_selection_details = {}); + absl::string_view host_selection_details = {}, + absl::optional failure_status = absl::nullopt); Http::FilterDataStatus decodeData(Buffer::Instance& data, bool end_stream) override; Http::FilterTrailersStatus decodeTrailers(Http::RequestTrailerMap& trailers) override; @@ -603,7 +604,8 @@ class Filter : Logger::Loggable, // if a "good" response comes back and we return downstream, so there is no point in waiting // for the remaining upstream requests to return. void resetOtherUpstreams(UpstreamRequest& upstream_request); - void sendNoHealthyUpstreamResponse(absl::string_view details); + void sendNoHealthyUpstreamResponse(absl::string_view details, + absl::optional failure_status = absl::nullopt); bool setupRedirect(const Http::ResponseHeaderMap& headers); bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream_headers, const Http::ResponseHeaderMap& upstream_headers, @@ -614,7 +616,8 @@ class Filter : Logger::Loggable, void doRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry is_timeout_retry); void continueDoRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry is_timeout_retry, Upstream::HostConstSharedPtr&& host, Upstream::ThreadLocalCluster& cluster, - absl::string_view host_selection_details); + absl::string_view host_selection_details, + absl::optional failure_status = absl::nullopt); void updateStatsOnNoRetry(RetryStatus retry_status); void updateStatsOnDoRetry(RetryState::DoRetryType do_retry_type); diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 6c01ffbc1742e..b21d23a497fd8 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -2064,13 +2064,13 @@ void ClusterManagerImpl::ThreadLocalClusterManagerImpl::httpConnPoolIsIdle( HostSelectionResponse ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::chooseHost( LoadBalancerContext* context) { auto cross_priority_host_map = priority_set_.crossPriorityHostMap(); - auto host_and_strict_mode = HostUtility::selectOverrideHost(cross_priority_host_map.get(), - override_host_statuses_, context); - if (host_and_strict_mode.first != nullptr) { - return {std::move(host_and_strict_mode.first)}; + auto override_result = HostUtility::selectOverrideHost(cross_priority_host_map.get(), + override_host_statuses_, context); + if (override_result.host != nullptr) { + return {std::move(override_result.host)}; } - if (!host_and_strict_mode.second) { + if (!override_result.strict) { Upstream::HostSelectionResponse host_selection = lb_->chooseHost(context); if (host_selection.host || host_selection.cancelable) { return host_selection; @@ -2082,16 +2082,26 @@ HostSelectionResponse ClusterManagerImpl::ThreadLocalClusterManagerImpl::Cluster cluster_info_->trafficStats()->upstream_cx_none_healthy_.inc(); ENVOY_LOG(debug, "no healthy host"); - return {nullptr}; + HostSelectionResponse response{nullptr}; + // Only apply the custom failure status when the destination is missing from + // available endpoints, not when it exists but is unhealthy. + if (override_result.status == HostUtility::OverrideHostSelectionStatus::NotFound) { + if (context != nullptr) { + if (auto override_host = context->overrideHostToSelect(); override_host.has_value()) { + response.failure_status = override_host->status_on_strict_destination_not_found; + } + } + } + return response; } HostConstSharedPtr ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::peekAnotherHost( LoadBalancerContext* context) { auto cross_priority_host_map = priority_set_.crossPriorityHostMap(); - auto host_and_strict_mode = HostUtility::selectOverrideHost(cross_priority_host_map.get(), - override_host_statuses_, context); - if (host_and_strict_mode.first != nullptr) { - return std::move(host_and_strict_mode.first); + auto override_result = HostUtility::selectOverrideHost(cross_priority_host_map.get(), + override_host_statuses_, context); + if (override_result.host != nullptr) { + return std::move(override_result.host); } // TODO(wbpcode): should we do strict mode check of override host here? return lb_->peekAnotherHost(context); diff --git a/source/common/upstream/host_utility.cc b/source/common/upstream/host_utility.cc index d2d0335689cd8..2d5005bd29d41 100644 --- a/source/common/upstream/host_utility.cc +++ b/source/common/upstream/host_utility.cc @@ -137,39 +137,39 @@ HostUtility::HostStatusSet HostUtility::createOverrideHostStatus( return override_host_status; } -std::pair HostUtility::selectOverrideHost(const HostMap* host_map, - HostStatusSet status, - LoadBalancerContext* context) { +HostUtility::OverrideHostSelectionResult +HostUtility::selectOverrideHost(const HostMap* host_map, HostStatusSet status, + LoadBalancerContext* context) { if (context == nullptr) { - return {nullptr, false}; + return {}; } OptRef override_host = context->overrideHostToSelect(); if (!override_host.has_value()) { - return {nullptr, false}; + return {}; } const bool strict_mode = override_host->strict; if (host_map == nullptr) { - return {nullptr, strict_mode}; + return {nullptr, strict_mode, OverrideHostSelectionStatus::NotFound}; } auto host_iter = host_map->find(override_host->host); // The override host cannot be found in the host map. if (host_iter == host_map->end()) { - return {nullptr, strict_mode}; + return {nullptr, strict_mode, OverrideHostSelectionStatus::NotFound}; } HostConstSharedPtr host = host_iter->second; ASSERT(host != nullptr); if (status[static_cast(host->healthStatus())]) { - return {host, strict_mode}; + return {host, strict_mode, OverrideHostSelectionStatus::Success}; } - return {nullptr, strict_mode}; + return {nullptr, strict_mode, OverrideHostSelectionStatus::Unhealthy}; } void HostUtility::forEachHostMetric( diff --git a/source/common/upstream/host_utility.h b/source/common/upstream/host_utility.h index 0d9324da02fcc..f2c38b103d3c5 100644 --- a/source/common/upstream/host_utility.h +++ b/source/common/upstream/host_utility.h @@ -26,13 +26,33 @@ class HostUtility { // A utility function to create override host status from lb config. static HostStatusSet createOverrideHostStatus(const CommonLbConfigProto& common_config); + // Status of override host selection. + enum class OverrideHostSelectionStatus { + // Host was successfully selected. + Success, + // Host was not found in the host map. + NotFound, + // Host was found but is not healthy. + Unhealthy, + }; + + // Result of attempting to select an override host from the host map. + struct OverrideHostSelectionResult { + // The selected host, or nullptr if selection failed. + HostConstSharedPtr host; + // Whether strict mode was requested for the override. + bool strict{false}; + // The status of the override host selection. + OverrideHostSelectionStatus status{OverrideHostSelectionStatus::Success}; + }; + /** * A utility function to select override host from host map according to load balancer context. * - * @return pair the first element is the selected host and the second - * element is a boolean indicating whether the host should be selected strictly or not. + * @return OverrideHostSelectionResult containing the selected host, whether strict mode was + * requested, and the reason for the selection outcome. */ - static std::pair + static OverrideHostSelectionResult selectOverrideHost(const HostMap* host_map, HostStatusSet status, LoadBalancerContext* context); // Iterate over all per-endpoint metrics, for clusters with `per_endpoint_stats` enabled. diff --git a/source/extensions/filters/http/stateful_session/stateful_session.cc b/source/extensions/filters/http/stateful_session/stateful_session.cc index bccf25ab044a1..f9d0d83a9652b 100644 --- a/source/extensions/filters/http/stateful_session/stateful_session.cc +++ b/source/extensions/filters/http/stateful_session/stateful_session.cc @@ -26,7 +26,11 @@ class EmptySessionStateFactory : public Envoy::Http::SessionStateFactory { StatefulSessionConfig::StatefulSessionConfig(const ProtoConfig& config, Server::Configuration::GenericFactoryContext& context, const std::string& stats_prefix, Stats::Scope& scope) - : strict_(config.strict()) { + : strict_(config.strict()), + status_on_strict_destination_not_found_( + config.strict() && config.status_on_strict_destination_not_found() != 0 + ? static_cast(config.status_on_strict_destination_not_found()) + : Http::Code::ServiceUnavailable) { // Only construct stats if stat_prefix is explicitly set. if (!config.stat_prefix().empty()) { const std::string final_prefix = @@ -83,7 +87,8 @@ Http::FilterHeadersStatus StatefulSession::decodeHeaders(Http::RequestHeaderMap& if (auto upstream_address = session_state_->upstreamAddress(); upstream_address.has_value()) { decoder_callbacks_->setUpstreamOverrideHost(Upstream::LoadBalancerContext::OverrideHost{ - std::string(upstream_address.value()), effective_config_->isStrict()}); + std::string(upstream_address.value()), effective_config_->isStrict(), + effective_config_->statusOnMissingStrictDestination()}); } return Http::FilterHeadersStatus::Continue; } diff --git a/source/extensions/filters/http/stateful_session/stateful_session.h b/source/extensions/filters/http/stateful_session/stateful_session.h index cdf578feb195c..9f3c6b38e7cc4 100644 --- a/source/extensions/filters/http/stateful_session/stateful_session.h +++ b/source/extensions/filters/http/stateful_session/stateful_session.h @@ -54,11 +54,16 @@ class StatefulSessionConfig { bool isStrict() const { return strict_; } + Http::Code statusOnMissingStrictDestination() const { + return status_on_strict_destination_not_found_; + } + OptRef stats() { return makeOptRefFromPtr(stats_.get()); } private: Http::SessionStateFactorySharedPtr factory_; bool strict_{false}; + Http::Code status_on_strict_destination_not_found_{Http::Code::ServiceUnavailable}; std::shared_ptr stats_; }; using StatefulSessionConfigSharedPtr = std::shared_ptr; diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 1c9ab5bbd41b6..e09bd1fcd83a5 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -1700,6 +1700,23 @@ TEST_F(ClusterManagerImplTest, SelectOverrideHostTestWithNonExistingHostStrict) EXPECT_FALSE(opt_cp.has_value()); } +TEST_F(ClusterManagerImplTest, StrictOverrideHostNotFoundReturnsCustomFailureStatus) { + createWithBasicStaticCluster(); + NiceMock context; + + // Non-existing host with strict mode and custom failure status (421). + Upstream::LoadBalancerContext::OverrideHost override_host{"127.0.0.2:12345", true, + Http::Code::MisdirectedRequest}; + EXPECT_CALL(context, overrideHostToSelect()) + .WillRepeatedly( + Return(OptRef(override_host))); + + auto result = cluster_manager_->getThreadLocalCluster("cluster_1")->chooseHost(&context); + EXPECT_EQ(nullptr, result.host); + ASSERT_TRUE(result.failure_status.has_value()); + EXPECT_EQ(Http::Code::MisdirectedRequest, result.failure_status.value()); +} + TEST_F(ClusterManagerImplTest, UpstreamSocketOptionsPassedToConnPool) { createWithBasicStaticCluster(); NiceMock context; diff --git a/test/common/upstream/host_utility_test.cc b/test/common/upstream/host_utility_test.cc index 107eb5a50f943..60dcd29b46572 100644 --- a/test/common/upstream/host_utility_test.cc +++ b/test/common/upstream/host_utility_test.cc @@ -148,10 +148,13 @@ TEST(HostUtilityTest, CreateOverrideHostStatus) { } TEST(HostUtilityTest, SelectOverrideHostTest) { - constexpr auto expect_helper = [](std::pair expected_result, - HostConstSharedPtr host, bool strict_mode) { - EXPECT_EQ(expected_result.first, host); - EXPECT_EQ(expected_result.second, strict_mode); + using Status = HostUtility::OverrideHostSelectionStatus; + + constexpr auto expect_result = [](HostUtility::OverrideHostSelectionResult result, + HostConstSharedPtr host, bool strict_mode, Status status) { + EXPECT_EQ(result.host, host); + EXPECT_EQ(result.strict, strict_mode); + EXPECT_EQ(result.status, status); }; NiceMock context; @@ -163,8 +166,8 @@ TEST(HostUtilityTest, SelectOverrideHostTest) { { // No valid load balancer context. auto host_map = std::make_shared(); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, nullptr), nullptr, - false); + expect_result(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, nullptr), nullptr, + false, Status::Success); } { @@ -172,8 +175,8 @@ TEST(HostUtilityTest, SelectOverrideHostTest) { EXPECT_CALL(context, overrideHostToSelect()) .WillOnce(Return(OptRef())); auto host_map = std::make_shared(); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, &context), nullptr, - false); + expect_result(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, &context), nullptr, + false, Status::Success); } // Test overriding host in strict and non-strict mode. @@ -183,8 +186,8 @@ TEST(HostUtilityTest, SelectOverrideHostTest) { LoadBalancerContext::OverrideHost override_host{"1.2.3.4", strict_mode}; EXPECT_CALL(context, overrideHostToSelect()) .WillOnce(Return(OptRef(override_host))); - expect_helper(HostUtility::selectOverrideHost(nullptr, AllStatuses, &context), nullptr, - strict_mode); + expect_result(HostUtility::selectOverrideHost(nullptr, AllStatuses, &context), nullptr, + strict_mode, Status::NotFound); } { @@ -193,8 +196,8 @@ TEST(HostUtilityTest, SelectOverrideHostTest) { EXPECT_CALL(context, overrideHostToSelect()) .WillOnce(Return(OptRef(override_host))); auto host_map = std::make_shared(); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, &context), nullptr, - strict_mode); + expect_result(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, &context), nullptr, + strict_mode, Status::NotFound); } { auto mock_host = std::make_shared>(); @@ -208,21 +211,21 @@ TEST(HostUtilityTest, SelectOverrideHostTest) { auto host_map = std::make_shared(); host_map->insert({"1.2.3.4", mock_host}); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), UnhealthyStatus, &context), - mock_host, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, &context), - mock_host, strict_mode); - - expect_helper(HostUtility::selectOverrideHost(host_map.get(), HealthyStatus, &context), - nullptr, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), DegradedStatus, &context), - nullptr, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), TimeoutStatus, &context), - nullptr, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), DrainingStatus, &context), - nullptr, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), UnknownStatus, &context), - nullptr, strict_mode); + expect_result(HostUtility::selectOverrideHost(host_map.get(), UnhealthyStatus, &context), + mock_host, strict_mode, Status::Success); + expect_result(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, &context), + mock_host, strict_mode, Status::Success); + + expect_result(HostUtility::selectOverrideHost(host_map.get(), HealthyStatus, &context), + nullptr, strict_mode, Status::Unhealthy); + expect_result(HostUtility::selectOverrideHost(host_map.get(), DegradedStatus, &context), + nullptr, strict_mode, Status::Unhealthy); + expect_result(HostUtility::selectOverrideHost(host_map.get(), TimeoutStatus, &context), + nullptr, strict_mode, Status::Unhealthy); + expect_result(HostUtility::selectOverrideHost(host_map.get(), DrainingStatus, &context), + nullptr, strict_mode, Status::Unhealthy); + expect_result(HostUtility::selectOverrideHost(host_map.get(), UnknownStatus, &context), + nullptr, strict_mode, Status::Unhealthy); } { auto mock_host = std::make_shared>(); @@ -236,21 +239,21 @@ TEST(HostUtilityTest, SelectOverrideHostTest) { auto host_map = std::make_shared(); host_map->insert({"1.2.3.4", mock_host}); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), DegradedStatus, &context), - mock_host, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, &context), - mock_host, strict_mode); - - expect_helper(HostUtility::selectOverrideHost(host_map.get(), HealthyStatus, &context), - nullptr, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), UnhealthyStatus, &context), - nullptr, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), TimeoutStatus, &context), - nullptr, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), DrainingStatus, &context), - nullptr, strict_mode); - expect_helper(HostUtility::selectOverrideHost(host_map.get(), UnknownStatus, &context), - nullptr, strict_mode); + expect_result(HostUtility::selectOverrideHost(host_map.get(), DegradedStatus, &context), + mock_host, strict_mode, Status::Success); + expect_result(HostUtility::selectOverrideHost(host_map.get(), AllStatuses, &context), + mock_host, strict_mode, Status::Success); + + expect_result(HostUtility::selectOverrideHost(host_map.get(), HealthyStatus, &context), + nullptr, strict_mode, Status::Unhealthy); + expect_result(HostUtility::selectOverrideHost(host_map.get(), UnhealthyStatus, &context), + nullptr, strict_mode, Status::Unhealthy); + expect_result(HostUtility::selectOverrideHost(host_map.get(), TimeoutStatus, &context), + nullptr, strict_mode, Status::Unhealthy); + expect_result(HostUtility::selectOverrideHost(host_map.get(), DrainingStatus, &context), + nullptr, strict_mode, Status::Unhealthy); + expect_result(HostUtility::selectOverrideHost(host_map.get(), UnknownStatus, &context), + nullptr, strict_mode, Status::Unhealthy); } } } diff --git a/test/extensions/filters/http/stateful_session/stateful_session_test.cc b/test/extensions/filters/http/stateful_session/stateful_session_test.cc index 981087b9a2baa..91900af5dab83 100644 --- a/test/extensions/filters/http/stateful_session/stateful_session_test.cc +++ b/test/extensions/filters/http/stateful_session/stateful_session_test.cc @@ -1,4 +1,3 @@ -#include #include "source/extensions/filters/http/stateful_session/stateful_session.h" #include "source/server/generic_factory_context.h" @@ -409,6 +408,96 @@ stat_prefix: "test" EXPECT_EQ(0, context_.scope().counterFromString("stateful_session.test.no_session").value()); } +TEST_F(StatefulSessionTest, OverrideHostWithCustomStatus) { + const std::string strict_config = R"EOF( +session_state: + name: envoy.http.stateful_session.mock + typed_config: + "@type": type.googleapis.com/google.protobuf.Struct +strict: true +stat_prefix: "test" +status_on_strict_destination_not_found: 421 +)EOF"; + + initialize(strict_config); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/"}, {":method", "GET"}, {":authority", "test.com"}}; + + auto raw_session_state = new testing::NiceMock(); + EXPECT_CALL(*factory_, create(_)) + .WillOnce(Return(testing::ByMove(std::unique_ptr(raw_session_state)))); + EXPECT_CALL(*raw_session_state, upstreamAddress()) + .WillOnce(Return(absl::make_optional("127.0.0.1:8080"))); + + Upstream::LoadBalancerContext::OverrideHost captured_host; + EXPECT_CALL(decoder_callbacks_, setUpstreamOverrideHost(_)) + .WillOnce(testing::SaveArg<0>(&captured_host)); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); + + // Verify the override host is set with the custom status code. + EXPECT_EQ("127.0.0.1:8080", captured_host.host); + EXPECT_TRUE(captured_host.strict); + EXPECT_EQ(Http::Code::MisdirectedRequest, captured_host.status_on_strict_destination_not_found); +} + +TEST_F(StatefulSessionTest, OverrideHostWithDefaultStatus) { + const std::string strict_config = R"EOF( +session_state: + name: envoy.http.stateful_session.mock + typed_config: + "@type": type.googleapis.com/google.protobuf.Struct +strict: true +stat_prefix: "test" +)EOF"; + + initialize(strict_config); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/"}, {":method", "GET"}, {":authority", "test.com"}}; + + auto raw_session_state = new testing::NiceMock(); + EXPECT_CALL(*factory_, create(_)) + .WillOnce(Return(testing::ByMove(std::unique_ptr(raw_session_state)))); + EXPECT_CALL(*raw_session_state, upstreamAddress()) + .WillOnce(Return(absl::make_optional("127.0.0.1:8080"))); + + Upstream::LoadBalancerContext::OverrideHost captured_host; + EXPECT_CALL(decoder_callbacks_, setUpstreamOverrideHost(_)) + .WillOnce(testing::SaveArg<0>(&captured_host)); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); + + // Verify the override host is set with the default status code (503). + EXPECT_EQ("127.0.0.1:8080", captured_host.host); + EXPECT_TRUE(captured_host.strict); + EXPECT_EQ(Http::Code::ServiceUnavailable, captured_host.status_on_strict_destination_not_found); +} + +TEST_F(StatefulSessionTest, StatusOnStrictDestinationNotFoundIgnoredWithoutStrict) { + const std::string config_yaml = R"EOF( +session_state: + name: envoy.http.stateful_session.mock + typed_config: + "@type": type.googleapis.com/google.protobuf.Struct +strict: false +status_on_strict_destination_not_found: 421 +)EOF"; + + Http::MockSessionStateFactoryConfig config_factory; + Registry::InjectFactory registration(config_factory); + auto mock_factory = std::make_shared>(); + EXPECT_CALL(config_factory, createSessionStateFactory(_, _)).WillOnce(Return(mock_factory)); + + ProtoConfig proto_config; + TestUtility::loadFromYaml(config_yaml, proto_config); + Envoy::Server::GenericFactoryContextImpl generic_context(context_); + + // When strict is false, status_on_strict_destination_not_found is ignored and defaults to 503. + StatefulSessionConfig config(proto_config, generic_context, "", context_.scope()); + EXPECT_FALSE(config.isStrict()); + EXPECT_EQ(Http::Code::ServiceUnavailable, config.statusOnMissingStrictDestination()); +} + } // namespace } // namespace StatefulSession } // namespace HttpFilters From 0c53bebc052ca1ff850186a1a43db44b2ef0e21d Mon Sep 17 00:00:00 2001 From: Rudrakh Panigrahi Date: Tue, 14 Apr 2026 18:26:18 +0530 Subject: [PATCH 2/2] address literal comment Signed-off-by: Rudrakh Panigrahi --- .../filters/http/stateful_session/v3/stateful_session.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/http/stateful_session/v3/stateful_session.proto b/api/envoy/extensions/filters/http/stateful_session/v3/stateful_session.proto index 9639055b8f3a0..a61d9124000cf 100644 --- a/api/envoy/extensions/filters/http/stateful_session/v3/stateful_session.proto +++ b/api/envoy/extensions/filters/http/stateful_session/v3/stateful_session.proto @@ -44,7 +44,7 @@ message StatefulSession { // The HTTP status code to return when ``strict`` mode is enabled and the requested destination // is not found in the set of available endpoints. This does not apply when the destination exists // but is unhealthy. This field has no effect when ``strict`` is set to ``false`` and will be - // ignored. Defaults to 503 (Service Unavailable) if not specified or set to 0. + // ignored. Defaults to ``503`` (Service Unavailable) if not specified or set to ``0``. uint32 status_on_strict_destination_not_found = 4; }