diff --git a/api/envoy/extensions/router/cluster_specifiers/dynamic_modules/v3/dynamic_modules.proto b/api/envoy/extensions/router/cluster_specifiers/dynamic_modules/v3/dynamic_modules.proto index 4f42c503fa3b..3d1186c3a3bb 100644 --- a/api/envoy/extensions/router/cluster_specifiers/dynamic_modules/v3/dynamic_modules.proto +++ b/api/envoy/extensions/router/cluster_specifiers/dynamic_modules/v3/dynamic_modules.proto @@ -29,8 +29,8 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // freely by the module on each request. // // Each entry is built and validated when the cluster specifier is configured, so an invalid entry -// is rejected at configuration load rather than on the request path. At least one property must be -// specified. +// is rejected at configuration load rather than on the request path. Every entry must replace at +// least one property, so an entry that builds to no replacement is rejected as well. message RouteActionOverride { // Retry policy replacing the retry policy of the matched route. If not specified, the retry // policy of the matched route is used, so an entry cannot be used to remove a retry policy that @@ -43,19 +43,22 @@ message RouteActionOverride { // :ref:`RouteAction.metadata_match // `. If not specified, or if there // is no ``envoy.lb`` entry, the criteria of the matched route are used, so an entry cannot be used - // to remove the criteria that the matched route configures. + // to remove the criteria that the matched route configures. An entry whose only property is a + // ``metadata_match`` without an ``envoy.lb`` entry therefore replaces nothing and is rejected. config.core.v3.Metadata metadata_match = 2; // Request mirroring policies replacing the request mirroring policies of the matched route. If // not specified, the request mirroring policies of the matched route are used, so an entry cannot - // be used to turn off mirroring that the matched route configures. + // be used to turn off mirroring that the matched route configures. Statically named mirror + // clusters are checked against the cluster manager when :ref:`validate_clusters + // ` is enabled. repeated config.route.v3.RouteAction.RequestMirrorPolicy request_mirror_policies = 3; } // Configuration for the Dynamic Modules Cluster Specifier. This cluster specifier allows loading // shared object files via ``dlopen`` to select the upstream cluster for a request, and to replace -// the timeout, idle timeout, priority, request body buffer limit, retry policy, metadata match -// criteria and request mirroring policies of the matched route. +// the timeout, idle timeout, priority, request body buffer limit, cluster not found response code, +// retry policy, metadata match criteria and request mirroring policies of the matched route. // // A module can be loaded by multiple cluster specifiers. It is loaded only once and shared across // multiple cluster specifier instances. The module is invoked while the route is being @@ -112,8 +115,8 @@ message DynamicModuleClusterSpecifier { google.protobuf.Any specifier_config = 3; // Route action overrides that the module may select by name for a request. If not specified, the - // module can only select the cluster, the timeout, the idle timeout, the priority and the request - // body buffer limit. + // module can only select the cluster, the timeout, the idle timeout, the priority, the request + // body buffer limit and the cluster not found response code. map route_action_overrides = 4 [(validate.rules).map = {keys {string {min_len: 1}}}]; } diff --git a/changelogs/current/new_features/dynamic_modules__added-a-cluster-specifier-extension.rst b/changelogs/current/new_features/dynamic_modules__added-a-cluster-specifier-extension.rst index 12585bfa8fda..2346f42364ca 100644 --- a/changelogs/current/new_features/dynamic_modules__added-a-cluster-specifier-extension.rst +++ b/changelogs/current/new_features/dynamic_modules__added-a-cluster-specifier-extension.rst @@ -1,7 +1,8 @@ Added a dynamic modules cluster specifier extension (``envoy.router.cluster_specifier_plugin.dynamic_modules``) that lets a dynamic module select the upstream cluster for a request and replace the timeout, idle timeout, priority, request body buffer -limit, retry policy, metadata match criteria and request mirroring policies of the matched route. +limit, cluster not found response code, retry policy, metadata match criteria and request mirroring +policies of the matched route. The selection context exposes the request headers, stream info attributes, dynamic metadata, the route name, and the random value Envoy generated for cluster selection, and the module is invoked again whenever a filter refreshes the route cluster. The Rust SDK exposes this through the diff --git a/docs/root/configuration/http/cluster_specifier/dynamic_modules.rst b/docs/root/configuration/http/cluster_specifier/dynamic_modules.rst index 1c8021edbe69..d3d151a6fc08 100644 --- a/docs/root/configuration/http/cluster_specifier/dynamic_modules.rst +++ b/docs/root/configuration/http/cluster_specifier/dynamic_modules.rst @@ -9,8 +9,8 @@ Overview The :ref:`DynamicModuleClusterSpecifier ` configuration specifies a cluster specifier backed by a :ref:`dynamic module `. The module selects the upstream cluster for a request and may replace the timeout, idle timeout, -priority, request body buffer limit, retry policy, metadata match criteria and request mirroring -policies of the matched route. +priority, request body buffer limit, cluster not found response code, retry policy, metadata match +criteria and request mirroring policies of the matched route. The module is invoked while the route is being resolved, so its selection is visible to the router without clearing the route cache. It is invoked again whenever a filter calls @@ -22,8 +22,12 @@ The properties that are built from other extensions are declared as named :ref:`route action overrides `. Each override is built and validated once when the cluster specifier is configured, so an invalid override is rejected at configuration load rather than on the request path, and the module selects -one by name. Selecting a name that is not declared leaves the route action properties of the matched -route in effect and logs a warning. +one by name. An override that replaces no property is rejected too, so a module can rely on a +declared name changing something. Selecting a name that is not declared leaves the route action +properties of the matched route in effect and logs a warning. When +:ref:`validate_clusters ` +is enabled, the statically named mirror clusters of every override are checked against the cluster +manager. Not every property can be changed once the request is under way, because Envoy reads each of them at a different point: @@ -36,6 +40,8 @@ a different point: * The idle timeout and the request body buffer limit are read while the route is resolved, and the timeout, the retry policy and the request mirroring policies are read before the first upstream attempt, so only the first selection applies to them. +* The cluster not found response code is read only when the selected cluster does not exist, which + ends the request, so the selection that named the missing cluster is the one that applies. Configuration ------------- diff --git a/source/extensions/dynamic_modules/abi/abi.h b/source/extensions/dynamic_modules/abi/abi.h index 9e2ed5e74ce2..40b9ce33fb10 100644 --- a/source/extensions/dynamic_modules/abi/abi.h +++ b/source/extensions/dynamic_modules/abi/abi.h @@ -14890,6 +14890,36 @@ bool envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata( envoy_dynamic_module_type_module_buffer filter_name, envoy_dynamic_module_type_module_buffer path, envoy_dynamic_module_type_envoy_buffer* result); +/** + * envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_number is called by the + * module to get a number value from dynamic metadata by filter name and key path. + * + * @param context_envoy_ptr is the pointer to the cluster selection context. + * @param filter_name is the filter namespace in dynamic metadata. + * @param path is the key path within the filter namespace, which may be nested with dots. + * @param result receives the number value. It is left untouched when this returns false. + * @return true if a number value exists at the path, false otherwise. + */ +bool envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_number( + envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr context_envoy_ptr, + envoy_dynamic_module_type_module_buffer filter_name, + envoy_dynamic_module_type_module_buffer path, double* result); + +/** + * envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_bool is called by the module + * to get a boolean value from dynamic metadata by filter name and key path. + * + * @param context_envoy_ptr is the pointer to the cluster selection context. + * @param filter_name is the filter namespace in dynamic metadata. + * @param path is the key path within the filter namespace, which may be nested with dots. + * @param result receives the boolean value. It is left untouched when this returns false. + * @return true if a boolean value exists at the path, false otherwise. + */ +bool envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_bool( + envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr context_envoy_ptr, + envoy_dynamic_module_type_module_buffer filter_name, + envoy_dynamic_module_type_module_buffer path, bool* result); + /** * envoy_dynamic_module_callback_cluster_specifier_get_route_name is called by the module to get the * name of the matched route. @@ -14986,6 +15016,22 @@ void envoy_dynamic_module_callback_cluster_specifier_set_priority( envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr context_envoy_ptr, envoy_dynamic_module_type_resource_priority priority); +/** + * envoy_dynamic_module_callback_cluster_specifier_set_cluster_not_found_response_code sets the + * status code Envoy replies with when the selected cluster does not exist, replacing the one of the + * matched route. A module that derives cluster names from the request can use this to distinguish a + * name that resolves to nothing from an upstream that is unavailable. + * + * @param context_envoy_ptr is the pointer to the cluster selection context. + * @param status_code is the HTTP status code to reply with. Only codes in the range [200, 600) are + * accepted, because the code is used to terminate the request. + * @return true if the status code was accepted, false when it is out of range, in which case the + * call changes nothing. + */ +bool envoy_dynamic_module_callback_cluster_specifier_set_cluster_not_found_response_code( + envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr context_envoy_ptr, + uint32_t status_code); + /** * envoy_dynamic_module_callback_cluster_specifier_set_route_action_override selects one of the * route action overrides declared in the cluster specifier configuration. The selected override diff --git a/source/extensions/dynamic_modules/abi_impl.cc b/source/extensions/dynamic_modules/abi_impl.cc index 6f64f63cffe1..bb0316008a9f 100644 --- a/source/extensions/dynamic_modules/abi_impl.cc +++ b/source/extensions/dynamic_modules/abi_impl.cc @@ -2836,6 +2836,24 @@ __attribute__((weak)) bool envoy_dynamic_module_callback_cluster_specifier_get_d return false; } +__attribute__((weak)) bool +envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_number( + envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr, + envoy_dynamic_module_type_module_buffer, envoy_dynamic_module_type_module_buffer, double*) { + IS_ENVOY_BUG("envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_number: not " + "implemented in this context"); + return false; +} + +__attribute__((weak)) bool +envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_bool( + envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr, + envoy_dynamic_module_type_module_buffer, envoy_dynamic_module_type_module_buffer, bool*) { + IS_ENVOY_BUG("envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_bool: not " + "implemented in this context"); + return false; +} + __attribute__((weak)) uint64_t envoy_dynamic_module_callback_cluster_specifier_get_random_value( envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr) { IS_ENVOY_BUG("envoy_dynamic_module_callback_cluster_specifier_get_random_value: not implemented " @@ -2897,6 +2915,14 @@ __attribute__((weak)) void envoy_dynamic_module_callback_cluster_specifier_set_p "context"); } +__attribute__((weak)) bool +envoy_dynamic_module_callback_cluster_specifier_set_cluster_not_found_response_code( + envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr, uint32_t) { + IS_ENVOY_BUG("envoy_dynamic_module_callback_cluster_specifier_set_cluster_not_found_response_" + "code: not implemented in this context"); + return false; +} + __attribute__((weak)) void envoy_dynamic_module_callback_cluster_specifier_set_request_body_buffer_limit( envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr, uint64_t) { diff --git a/source/extensions/dynamic_modules/sdk/rust/src/cluster_specifier.rs b/source/extensions/dynamic_modules/sdk/rust/src/cluster_specifier.rs index 993736050194..22ff5de43a24 100644 --- a/source/extensions/dynamic_modules/sdk/rust/src/cluster_specifier.rs +++ b/source/extensions/dynamic_modules/sdk/rust/src/cluster_specifier.rs @@ -225,6 +225,49 @@ impl ClusterSpecifierContext { } } + /// Get a number value from dynamic metadata. + /// + /// The arguments are the same as [`Self::get_dynamic_metadata`]. Only number values are returned. + pub fn get_dynamic_metadata_number(&self, filter_name: &str, path: &str) -> Option { + let filter_buf = crate::str_to_module_buffer(filter_name); + let path_buf = crate::str_to_module_buffer(path); + let mut result: f64 = 0.0; + if unsafe { + abi::envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_number( + self.envoy_ptr, + filter_buf, + path_buf, + &mut result, + ) + } { + Some(result) + } else { + None + } + } + + /// Get a boolean value from dynamic metadata. + /// + /// The arguments are the same as [`Self::get_dynamic_metadata`]. Only boolean values are + /// returned. + pub fn get_dynamic_metadata_bool(&self, filter_name: &str, path: &str) -> Option { + let filter_buf = crate::str_to_module_buffer(filter_name); + let path_buf = crate::str_to_module_buffer(path); + let mut result: bool = false; + if unsafe { + abi::envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_bool( + self.envoy_ptr, + filter_buf, + path_buf, + &mut result, + ) + } { + Some(result) + } else { + None + } + } + /// Get the name of the matched route. /// /// This returns `None` when the route has no name. @@ -318,6 +361,22 @@ impl ClusterSpecifierContext { } } + /// Set the status code Envoy replies with when the selected cluster does not exist. + /// + /// This replaces the cluster-not-found response code of the matched route, letting a module that + /// derives cluster names from the request distinguish a name that resolves to nothing from an + /// upstream that is unavailable. Returns `false` when the status code is outside the range + /// [200, 600), in which case the call changes nothing. + #[must_use] + pub fn set_cluster_not_found_response_code(&mut self, status_code: u32) -> bool { + unsafe { + abi::envoy_dynamic_module_callback_cluster_specifier_set_cluster_not_found_response_code( + self.envoy_ptr, + status_code, + ) + } + } + /// Select one of the route action overrides declared in the cluster specifier configuration. /// /// The selected override replaces the retry policy, the subset load balancing metadata match diff --git a/source/extensions/dynamic_modules/sdk/rust/src/lib_test.rs b/source/extensions/dynamic_modules/sdk/rust/src/lib_test.rs index 8707e049638c..29f3d576c443 100644 --- a/source/extensions/dynamic_modules/sdk/rust/src/lib_test.rs +++ b/source/extensions/dynamic_modules/sdk/rust/src/lib_test.rs @@ -8783,6 +8783,7 @@ struct StubSpecifierSelection { idle_timeout_ms: Option, request_body_buffer_limit: Option, priority: Option, + cluster_not_found_response_code: Option, route_action_override: Option, } @@ -8793,6 +8794,7 @@ static STUB_SPECIFIER_SELECTION: std::sync::Mutex = idle_timeout_ms: None, request_body_buffer_limit: None, priority: None, + cluster_not_found_response_code: None, route_action_override: None, }); @@ -8927,6 +8929,34 @@ pub extern "C" fn envoy_dynamic_module_callback_cluster_specifier_get_dynamic_me unsafe { stub_specifier_result("shard-a", result) } } +#[no_mangle] +pub extern "C" fn envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_number( + _context_envoy_ptr: abi::envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr, + _filter_name: abi::envoy_dynamic_module_type_module_buffer, + _path: abi::envoy_dynamic_module_type_module_buffer, + result: *mut f64, +) -> bool { + if !STUB_SPECIFIER_STATE_PRESENT.load(std::sync::atomic::Ordering::SeqCst) { + return false; + } + unsafe { *result = 0.25 }; + true +} + +#[no_mangle] +pub extern "C" fn envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_bool( + _context_envoy_ptr: abi::envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr, + _filter_name: abi::envoy_dynamic_module_type_module_buffer, + _path: abi::envoy_dynamic_module_type_module_buffer, + result: *mut bool, +) -> bool { + if !STUB_SPECIFIER_STATE_PRESENT.load(std::sync::atomic::Ordering::SeqCst) { + return false; + } + unsafe { *result = true }; + true +} + #[no_mangle] pub extern "C" fn envoy_dynamic_module_callback_cluster_specifier_get_route_name( _context_envoy_ptr: abi::envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr, @@ -8986,6 +9016,21 @@ pub extern "C" fn envoy_dynamic_module_callback_cluster_specifier_set_priority( STUB_SPECIFIER_SELECTION.lock().unwrap().priority = Some(priority); } +#[no_mangle] +pub extern "C" fn envoy_dynamic_module_callback_cluster_specifier_set_cluster_not_found_response_code( + _context_envoy_ptr: abi::envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr, + status_code: u32, +) -> bool { + if !(200..600).contains(&status_code) { + return false; + } + STUB_SPECIFIER_SELECTION + .lock() + .unwrap() + .cluster_not_found_response_code = Some(status_code); + true +} + #[no_mangle] pub extern "C" fn envoy_dynamic_module_callback_cluster_specifier_set_route_action_override( _context_envoy_ptr: abi::envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr, @@ -9209,6 +9254,14 @@ fn test_cluster_specifier_context_reads_request_state() { .unwrap() .as_slice() ); + assert_eq!( + Some(0.25), + ctx.get_dynamic_metadata_number("envoy.test", "weight") + ); + assert_eq!( + Some(true), + ctx.get_dynamic_metadata_bool("envoy.test", "canary") + ); assert_eq!( STUB_SPECIFIER_ROUTE_NAME.as_bytes(), ctx.route_name().unwrap().as_slice() @@ -9238,6 +9291,12 @@ fn test_cluster_specifier_context_reads_request_state() { .get_attribute_bool(abi::envoy_dynamic_module_type_attribute_id::HealthCheck) .is_none()); assert!(ctx.get_dynamic_metadata("envoy.test", "shard").is_none()); + assert!(ctx + .get_dynamic_metadata_number("envoy.test", "weight") + .is_none()); + assert!(ctx + .get_dynamic_metadata_bool("envoy.test", "canary") + .is_none()); assert!(ctx.route_name().is_none()); STUB_SPECIFIER_STATE_PRESENT.store(true, std::sync::atomic::Ordering::SeqCst); } @@ -9254,6 +9313,7 @@ fn test_cluster_specifier_context_records_selection() { ctx.set_idle_timeout(std::time::Duration::from_millis(4321)); ctx.set_request_body_buffer_limit(9999); ctx.set_priority(ResourcePriority::High); + assert!(ctx.set_cluster_not_found_response_code(404)); assert!(ctx.set_route_action_override(STUB_SPECIFIER_OVERRIDE_NAME)); { @@ -9266,6 +9326,7 @@ fn test_cluster_specifier_context_records_selection() { Some(abi::envoy_dynamic_module_type_resource_priority::High), selection.priority ); + assert_eq!(Some(404), selection.cluster_not_found_response_code); assert_eq!( Some(STUB_SPECIFIER_OVERRIDE_NAME.to_string()), selection.route_action_override @@ -9275,8 +9336,10 @@ fn test_cluster_specifier_context_records_selection() { // Sub-millisecond precision is truncated, and the default priority maps to its own ABI value. ctx.set_timeout(std::time::Duration::from_micros(1500)); ctx.set_priority(ResourcePriority::Default); - // An override that the configuration does not declare is reported as not found. + // An override that the configuration does not declare is reported as not found, and so is a + // status code that is not an HTTP response code. assert!(!ctx.set_route_action_override("unknown")); + assert!(!ctx.set_cluster_not_found_response_code(600)); { let selection = STUB_SPECIFIER_SELECTION.lock().unwrap(); @@ -9285,6 +9348,7 @@ fn test_cluster_specifier_context_records_selection() { Some(abi::envoy_dynamic_module_type_resource_priority::Default), selection.priority ); + assert_eq!(Some(404), selection.cluster_not_found_response_code); assert_eq!( Some(STUB_SPECIFIER_OVERRIDE_NAME.to_string()), selection.route_action_override diff --git a/source/extensions/router/cluster_specifiers/dynamic_modules/BUILD b/source/extensions/router/cluster_specifiers/dynamic_modules/BUILD index 5050021efff4..c0a92c58204a 100644 --- a/source/extensions/router/cluster_specifiers/dynamic_modules/BUILD +++ b/source/extensions/router/cluster_specifiers/dynamic_modules/BUILD @@ -20,8 +20,10 @@ envoy_cc_library( hdrs = ["cluster_specifier.h"], deps = [ "//envoy/common:exception_lib", + "//envoy/http:codes_interface", "//envoy/router:cluster_specifier_plugin_interface", "//envoy/server:factory_context_interface", + "//envoy/upstream:cluster_manager_interface", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", "//source/common/common:statusor_lib", diff --git a/source/extensions/router/cluster_specifiers/dynamic_modules/abi_impl.cc b/source/extensions/router/cluster_specifiers/dynamic_modules/abi_impl.cc index c5c628387f0c..f1aef2e598e2 100644 --- a/source/extensions/router/cluster_specifiers/dynamic_modules/abi_impl.cc +++ b/source/extensions/router/cluster_specifiers/dynamic_modules/abi_impl.cc @@ -89,6 +89,22 @@ bool envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata( return ContextAccessor::getDynamicMetadata(context->stream_info, filter_name, path, result); } +bool envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_number( + envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr context_envoy_ptr, + envoy_dynamic_module_type_module_buffer filter_name, + envoy_dynamic_module_type_module_buffer path, double* result) { + auto* context = clusterSpecifierContext(context_envoy_ptr); + return ContextAccessor::getDynamicMetadataNumber(context->stream_info, filter_name, path, result); +} + +bool envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_bool( + envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr context_envoy_ptr, + envoy_dynamic_module_type_module_buffer filter_name, + envoy_dynamic_module_type_module_buffer path, bool* result) { + auto* context = clusterSpecifierContext(context_envoy_ptr); + return ContextAccessor::getDynamicMetadataBool(context->stream_info, filter_name, path, result); +} + bool envoy_dynamic_module_callback_cluster_specifier_get_route_name( envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr context_envoy_ptr, envoy_dynamic_module_type_envoy_buffer* result) { @@ -142,6 +158,24 @@ void envoy_dynamic_module_callback_cluster_specifier_set_priority( : Upstream::ResourcePriority::Default; } +bool envoy_dynamic_module_callback_cluster_specifier_set_cluster_not_found_response_code( + envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr context_envoy_ptr, + uint32_t status_code) { + auto* context = clusterSpecifierContext(context_envoy_ptr); + // The code is only used if the selected cluster turns out to be missing, so an invalid one would + // otherwise surface as a malformed response far from this call. Reject it here instead. + if (status_code < 200 || status_code >= 600) { + ENVOY_LOG_EVERY_POW_2_TO_LOGGER( + Envoy::Logger::Registry::getLog(Envoy::Logger::Id::dynamic_modules), warn, + "dynamic module set the out of range cluster not found response code {}, so the response " + "code of the matched route stays in effect", + status_code); + return false; + } + context->selection.cluster_not_found_response_code = static_cast(status_code); + return true; +} + bool envoy_dynamic_module_callback_cluster_specifier_set_route_action_override( envoy_dynamic_module_type_cluster_specifier_context_envoy_ptr context_envoy_ptr, envoy_dynamic_module_type_module_buffer name) { diff --git a/source/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier.cc b/source/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier.cc index 612c6815ed9f..c57372c2d1bc 100644 --- a/source/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier.cc +++ b/source/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier.cc @@ -24,11 +24,6 @@ using RouteActionOverrideProto = absl::StatusOr buildRouteActionOverride(const RouteActionOverrideProto& proto_override, Server::Configuration::ServerFactoryContext& context) { - if (!proto_override.has_retry_policy() && !proto_override.has_metadata_match() && - proto_override.request_mirror_policies().empty()) { - return absl::InvalidArgumentError( - "Route action override must specify at least one property to replace"); - } RouteActionOverride entry; if (proto_override.has_retry_policy()) { auto policy_or_error = Envoy::Router::RetryPolicyImpl::create( @@ -50,6 +45,14 @@ buildRouteActionOverride(const RouteActionOverrideProto& proto_override, RETURN_IF_NOT_OK_REF(policy_or_error.status()); entry.shadow_policies.push_back(std::move(policy_or_error.value())); } + // Validate what was built rather than what was configured. A metadata_match without an envoy.lb + // entry contributes nothing, so a populated looking configuration can still build an override + // that replaces no property, which set_route_action_override would then accept as a decision. + if (entry.retry_policy == nullptr && entry.metadata_match_criteria == nullptr && + entry.shadow_policies.empty()) { + return absl::InvalidArgumentError( + "Route action override must replace at least one route action property"); + } return entry; } @@ -77,6 +80,23 @@ DynamicModuleClusterSpecifierConfig::routeActionOverride(absl::string_view name) return it != route_action_overrides_.end() ? &it->second : nullptr; } +absl::Status DynamicModuleClusterSpecifierConfig::validateClusters( + const Upstream::ClusterManager& cluster_manager) const { + for (const auto& [name, override_entry] : route_action_overrides_) { + for (const auto& shadow_policy : override_entry.shadow_policies) { + // A policy that names its cluster through a request header resolves it per request, so only a + // statically named cluster can be checked here. + if (!shadow_policy->cluster().empty() && + !cluster_manager.hasCluster(shadow_policy->cluster())) { + return absl::InvalidArgumentError( + fmt::format("route action override '{}': unknown shadow cluster '{}'", name, + shadow_policy->cluster())); + } + } + } + return absl::OkStatus(); +} + absl::StatusOr newDynamicModuleClusterSpecifierConfig(const DynamicModuleClusterSpecifierProto& proto_config, Extensions::DynamicModules::DynamicModulePtr dynamic_module, diff --git a/source/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier.h b/source/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier.h index 6568b06afefa..bc9f509ade64 100644 --- a/source/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier.h +++ b/source/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier.h @@ -7,8 +7,10 @@ #include #include "envoy/extensions/router/cluster_specifiers/dynamic_modules/v3/dynamic_modules.pb.h" +#include "envoy/http/codes.h" #include "envoy/router/cluster_specifier_plugin.h" #include "envoy/server/factory_context.h" +#include "envoy/upstream/cluster_manager.h" #include "source/common/common/logger.h" #include "source/common/common/statusor.h" @@ -72,6 +74,14 @@ class DynamicModuleClusterSpecifierConfig { */ const RouteActionOverride* routeActionOverride(absl::string_view name) const; + /** + * Validates the statically named mirror clusters of every route action override. The route level + * check only walks the mirror policies of the matched route, which never include these. + * @param cluster_manager the cluster manager to look the clusters up in. + * @return an error naming the first override that mirrors to an unknown cluster. + */ + absl::Status validateClusters(const Upstream::ClusterManager& cluster_manager) const; + // The corresponding in-module cluster specifier configuration. envoy_dynamic_module_type_cluster_specifier_config_module_ptr in_module_config_{nullptr}; @@ -124,6 +134,7 @@ struct ClusterSpecifierSelection { std::optional idle_timeout; std::optional request_body_buffer_limit; std::optional priority; + std::optional cluster_not_found_response_code; // Points into the override map of the cluster specifier configuration, which is immutable after // construction. const RouteActionOverride* route_action_override{nullptr}; @@ -165,6 +176,11 @@ class DynamicModuleRouteEntry : public Envoy::Router::DelegatingRouteEntry, return selection_.priority.has_value() ? *selection_.priority : DelegatingRouteEntry::priority(); } + Http::Code clusterNotFoundResponseCode() const override { + return selection_.cluster_not_found_response_code.has_value() + ? *selection_.cluster_not_found_response_code + : DelegatingRouteEntry::clusterNotFoundResponseCode(); + } const Envoy::Router::RetryPolicyConstSharedPtr& retryPolicy() const override { const RouteActionOverride* entry = selection_.route_action_override; return entry != nullptr && entry->retry_policy != nullptr ? entry->retry_policy @@ -215,6 +231,9 @@ class DynamicModuleClusterSpecifierPlugin : public Envoy::Router::ClusterSpecifi : config_(std::move(config)) {} // Router::ClusterSpecifierPlugin + absl::Status validateClusters(const Upstream::ClusterManager& cluster_manager) const override { + return config_->validateClusters(cluster_manager); + } Envoy::Router::RouteConstSharedPtr route(Envoy::Router::RouteEntryAndRouteConstSharedPtr parent, const Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo& stream_info, diff --git a/test/extensions/dynamic_modules/abi_impl_test.cc b/test/extensions/dynamic_modules/abi_impl_test.cc index e961fdeebc8b..5011de280642 100644 --- a/test/extensions/dynamic_modules/abi_impl_test.cc +++ b/test/extensions/dynamic_modules/abi_impl_test.cc @@ -1798,6 +1798,12 @@ WEAK_STUB(ClusterSpecifierGetAttributeString, WEAK_STUB(ClusterSpecifierGetDynamicMetadata, envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata( nullptr, {nullptr, 0}, {nullptr, 0}, nullptr)) +WEAK_STUB(ClusterSpecifierGetDynamicMetadataBool, + envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_bool( + nullptr, {nullptr, 0}, {nullptr, 0}, nullptr)) +WEAK_STUB(ClusterSpecifierGetDynamicMetadataNumber, + envoy_dynamic_module_callback_cluster_specifier_get_dynamic_metadata_number( + nullptr, {nullptr, 0}, {nullptr, 0}, nullptr)) WEAK_STUB(ClusterSpecifierGetRandomValue, envoy_dynamic_module_callback_cluster_specifier_get_random_value(nullptr)) WEAK_STUB(ClusterSpecifierGetRequestHeaderValue, @@ -1811,6 +1817,9 @@ WEAK_STUB(ClusterSpecifierGetRouteName, envoy_dynamic_module_callback_cluster_specifier_get_route_name(nullptr, nullptr)) WEAK_STUB(ClusterSpecifierSetClusterName, envoy_dynamic_module_callback_cluster_specifier_set_cluster_name(nullptr, {nullptr, 0})) +WEAK_STUB(ClusterSpecifierSetClusterNotFoundResponseCode, + envoy_dynamic_module_callback_cluster_specifier_set_cluster_not_found_response_code( + nullptr, 503)) WEAK_STUB(ClusterSpecifierSetIdleTimeout, envoy_dynamic_module_callback_cluster_specifier_set_idle_timeout(nullptr, 0)) WEAK_STUB(ClusterSpecifierSetPriority, diff --git a/test/extensions/dynamic_modules/test_data/rust/cluster_specifier_integration_test.rs b/test/extensions/dynamic_modules/test_data/rust/cluster_specifier_integration_test.rs index 572e956eecd3..3e071e9e426c 100644 --- a/test/extensions/dynamic_modules/test_data/rust/cluster_specifier_integration_test.rs +++ b/test/extensions/dynamic_modules/test_data/rust/cluster_specifier_integration_test.rs @@ -16,6 +16,7 @@ //! `x-idle-timeout-ms` the stream idle timeout to set, in milliseconds. //! `x-buffer-limit` the request body buffer limit to set, in bytes. //! `x-priority` `high` or `default`, the upstream resource priority to set. +//! `x-not-found-code` the status code to reply with when the selected cluster does not exist. //! `x-echo` the name of a read accessor whose value replaces the cluster name, so that //! a test can assert on what the module read across the ABI boundary. //! `x-retry-cluster` the cluster to route to once the first upstream attempt has been made, so @@ -107,6 +108,12 @@ fn read_echoed_value(ctx: &ClusterSpecifierContext, name: &[u8]) -> String { b"dynamic-metadata" => { buffer_to_string_or_absent(ctx.get_dynamic_metadata("envoy.test", "shard")) }, + b"dynamic-metadata-number" => ctx + .get_dynamic_metadata_number("envoy.test", "weight") + .map_or_else(|| ABSENT.to_owned(), |value| value.to_string()), + b"dynamic-metadata-bool" => ctx + .get_dynamic_metadata_bool("envoy.test", "enabled") + .map_or_else(|| ABSENT.to_owned(), |value| value.to_string()), b"route-name" => buffer_to_string_or_absent(ctx.route_name()), b"random-value" => ctx.random_value().to_string(), _ => ABSENT.to_owned(), @@ -152,6 +159,10 @@ impl ClusterSpecifierConfig for TestClusterSpecifierConfig { if let Some(limit) = read_u64_header(ctx, "x-buffer-limit") { ctx.set_request_body_buffer_limit(limit); } + if let Some(code) = read_u64_header(ctx, "x-not-found-code") { + // Tests also pass out of range codes, so a rejection is expected here. + let _ = ctx.set_cluster_not_found_response_code(u32::try_from(code).unwrap_or(u32::MAX)); + } let priority = ctx .get_request_header("x-priority") .map(|buffer| match buffer.as_slice() { diff --git a/test/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier_test.cc b/test/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier_test.cc index 02146d6cb1c0..2340348317b4 100644 --- a/test/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier_test.cc +++ b/test/extensions/router/cluster_specifiers/dynamic_modules/cluster_specifier_test.cc @@ -207,7 +207,68 @@ TEST_F(DynamicModuleClusterSpecifierFactoryTest, EmptyOverrideRejected) { (*proto_config.mutable_route_action_overrides())["empty"] = {}; EXPECT_THROW_WITH_REGEX(factory_.createClusterSpecifierPlugin(proto_config, context_), EnvoyException, - "Route action override must specify at least one property to replace"); + "Route action override must replace at least one route action property"); +} + +// Only the envoy.lb entry of a metadata match is used, so an override whose only property is a +// metadata match without one replaces nothing even though the proto looks populated. +TEST_F(DynamicModuleClusterSpecifierFactoryTest, OverrideWithNonLoadBalancingMetadataOnlyRejected) { + auto proto_config = protoConfig("cluster_specifier_no_op", "test_cluster_specifier"); + auto& entry = (*proto_config.mutable_route_action_overrides())["hollow"]; + Protobuf::Struct metadata; + (*metadata.mutable_fields())["shard"] = ValueUtil::stringValue("a"); + (*entry.mutable_metadata_match()->mutable_filter_metadata())["envoy.not_lb"] = metadata; + EXPECT_THROW_WITH_REGEX(factory_.createClusterSpecifierPlugin(proto_config, context_), + EnvoyException, + "Route action override must replace at least one route action property"); +} + +// The route level cluster validation only walks the mirror policies of the matched route, so the +// plugin has to validate the ones its overrides carry. +TEST_F(DynamicModuleClusterSpecifierFactoryTest, UnknownMirrorClusterInOverrideRejected) { + auto proto_config = protoConfig("cluster_specifier_no_op", "test_cluster_specifier"); + (*proto_config.mutable_route_action_overrides())["mirror"] + .add_request_mirror_policies() + ->set_cluster("absent-cluster"); + auto plugin = factory_.createClusterSpecifierPlugin(proto_config, context_); + EXPECT_CALL(context_.cluster_manager_, hasCluster(absl::string_view("absent-cluster"))) + .WillOnce(Return(false)); + EXPECT_THAT(plugin->validateClusters(context_.cluster_manager_).message(), + testing::HasSubstr("route action override 'mirror': unknown shadow cluster " + "'absent-cluster'")); +} + +TEST_F(DynamicModuleClusterSpecifierFactoryTest, KnownMirrorClusterInOverrideAccepted) { + auto proto_config = protoConfig("cluster_specifier_no_op", "test_cluster_specifier"); + (*proto_config.mutable_route_action_overrides())["mirror"] + .add_request_mirror_policies() + ->set_cluster("present-cluster"); + auto plugin = factory_.createClusterSpecifierPlugin(proto_config, context_); + EXPECT_CALL(context_.cluster_manager_, hasCluster(absl::string_view("present-cluster"))) + .WillOnce(Return(true)); + EXPECT_TRUE(plugin->validateClusters(context_.cluster_manager_).ok()); +} + +// A mirror policy that names its cluster through a header resolves it per request, so there is +// nothing to look up at configuration load. +TEST_F(DynamicModuleClusterSpecifierFactoryTest, HeaderNamedMirrorClusterNotValidated) { + auto proto_config = protoConfig("cluster_specifier_no_op", "test_cluster_specifier"); + (*proto_config.mutable_route_action_overrides())["mirror"] + .add_request_mirror_policies() + ->set_cluster_header("x-mirror"); + auto plugin = factory_.createClusterSpecifierPlugin(proto_config, context_); + EXPECT_CALL(context_.cluster_manager_, hasCluster(testing::_)).Times(0); + EXPECT_TRUE(plugin->validateClusters(context_.cluster_manager_).ok()); +} + +// An override without mirror policies has no cluster to validate. +TEST_F(DynamicModuleClusterSpecifierFactoryTest, OverrideWithoutMirrorPolicyNotValidated) { + auto proto_config = protoConfig("cluster_specifier_no_op", "test_cluster_specifier"); + (*proto_config.mutable_route_action_overrides())["retry"].mutable_retry_policy()->set_retry_on( + "5xx"); + auto plugin = factory_.createClusterSpecifierPlugin(proto_config, context_); + EXPECT_CALL(context_.cluster_manager_, hasCluster(testing::_)).Times(0); + EXPECT_TRUE(plugin->validateClusters(context_.cluster_manager_).ok()); } // An override key must not be empty. @@ -487,6 +548,53 @@ TEST_F(DynamicModuleClusterSpecifierTest, DefaultPriorityReplaced) { EXPECT_EQ(Upstream::ResourcePriority::Default, resolveRouteEntry(headers)->priority()); } +TEST_F(DynamicModuleClusterSpecifierTest, ClusterNotFoundResponseCodeDelegatesByDefault) { + setUpPlugin(); + EXPECT_CALL(parent_->route_entry_, clusterNotFoundResponseCode()) + .WillRepeatedly(Return(Http::Code::InternalServerError)); + Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"env", "prod"}}; + EXPECT_EQ(Http::Code::InternalServerError, + resolveRouteEntry(headers)->clusterNotFoundResponseCode()); +} + +TEST_F(DynamicModuleClusterSpecifierTest, ClusterNotFoundResponseCodeReplaced) { + setUpPlugin(); + EXPECT_CALL(parent_->route_entry_, clusterNotFoundResponseCode()).Times(0); + Http::TestRequestHeaderMapImpl headers{ + {":path", "/"}, {"env", "prod"}, {"x-not-found-code", "404"}}; + EXPECT_EQ(Http::Code::NotFound, resolveRouteEntry(headers)->clusterNotFoundResponseCode()); +} + +// The code is only read once the selected cluster turns out to be missing, so an invalid one would +// otherwise surface as a malformed response far from the module call that set it. +TEST_F(DynamicModuleClusterSpecifierTest, OutOfRangeClusterNotFoundResponseCodeRejected) { + setUpPlugin(); + EXPECT_CALL(parent_->route_entry_, clusterNotFoundResponseCode()) + .WillRepeatedly(Return(Http::Code::ServiceUnavailable)); + Http::TestRequestHeaderMapImpl headers{ + {":path", "/"}, {"env", "prod"}, {"x-not-found-code", "max"}}; + EXPECT_LOG_CONTAINS("warn", "out of range cluster not found response code", { + EXPECT_EQ(Http::Code::ServiceUnavailable, + resolveRouteEntry(headers)->clusterNotFoundResponseCode()); + }); +} + +// A refresh replaces the whole decision, so a response code an earlier call set goes back to the +// matched route. +TEST_F(DynamicModuleClusterSpecifierTest, RefreshWithoutResponseCodeRevertsToMatchedRoute) { + setUpPlugin(); + ON_CALL(parent_->route_entry_, clusterNotFoundResponseCode()) + .WillByDefault(Return(Http::Code::ServiceUnavailable)); + Http::TestRequestHeaderMapImpl headers{ + {":path", "/"}, {"env", "prod"}, {"x-not-found-code", "404"}}; + const auto* entry = resolveRouteEntry(headers); + EXPECT_EQ(Http::Code::NotFound, entry->clusterNotFoundResponseCode()); + + headers.remove(Http::LowerCaseString("x-not-found-code")); + entry->refreshRouteCluster(headers, stream_info_); + EXPECT_EQ(Http::Code::ServiceUnavailable, entry->clusterNotFoundResponseCode()); +} + // Tests that assert the values the module read through the context callbacks. The module echoes the // value named by the x-echo header into the cluster name, so every read is observable. class DynamicModuleClusterSpecifierReadTest : public DynamicModuleClusterSpecifierTest { @@ -551,6 +659,31 @@ TEST_F(DynamicModuleClusterSpecifierReadTest, AbsentDynamicMetadataReadable) { setUpPlugin(); Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"env", "prod"}}; EXPECT_EQ("absent", echoedValue(headers, "dynamic-metadata")); + EXPECT_EQ("absent", echoedValue(headers, "dynamic-metadata-number")); + EXPECT_EQ("absent", echoedValue(headers, "dynamic-metadata-bool")); +} + +TEST_F(DynamicModuleClusterSpecifierReadTest, NumberAndBoolDynamicMetadataReadable) { + setUpPlugin(); + Protobuf::Struct metadata; + (*metadata.mutable_fields())["weight"] = ValueUtil::numberValue(7); + (*metadata.mutable_fields())["enabled"] = ValueUtil::boolValue(true); + (*stream_info_.metadata_.mutable_filter_metadata())["envoy.test"] = metadata; + Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"env", "prod"}}; + EXPECT_EQ("7", echoedValue(headers, "dynamic-metadata-number")); + EXPECT_EQ("true", echoedValue(headers, "dynamic-metadata-bool")); +} + +// A value of the wrong type is not returned, so a module cannot read a string as a number. +TEST_F(DynamicModuleClusterSpecifierReadTest, MistypedDynamicMetadataNotReadable) { + setUpPlugin(); + Protobuf::Struct metadata; + (*metadata.mutable_fields())["weight"] = ValueUtil::stringValue("7"); + (*metadata.mutable_fields())["enabled"] = ValueUtil::stringValue("true"); + (*stream_info_.metadata_.mutable_filter_metadata())["envoy.test"] = metadata; + Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"env", "prod"}}; + EXPECT_EQ("absent", echoedValue(headers, "dynamic-metadata-number")); + EXPECT_EQ("absent", echoedValue(headers, "dynamic-metadata-bool")); } TEST_F(DynamicModuleClusterSpecifierReadTest, RouteNameReadable) { @@ -670,10 +803,14 @@ TEST_F(DynamicModuleRouteActionOverrideTest, RefreshWithoutOverrideDelegatesToMa } // Metadata under a namespace other than envoy.lb is ignored, matching RouteAction.metadata_match. +// The retry policy makes the override replace something, since one that replaces nothing is +// rejected at configuration load. TEST_F(DynamicModuleClusterSpecifierTest, NonLoadBalancingMetadataIgnored) { setUpPlugin(R"EOF( route_action_overrides: other: + retry_policy: + num_retries: 7 metadata_match: filter_metadata: envoy.other: @@ -681,7 +818,10 @@ TEST_F(DynamicModuleClusterSpecifierTest, NonLoadBalancingMetadataIgnored) { )EOF"); const auto* matched_route_criteria = giveMatchedRouteMetadataMatchCriteria(); Http::TestRequestHeaderMapImpl headers{{":path", "/"}, {"env", "prod"}, {"x-override", "other"}}; - EXPECT_EQ(matched_route_criteria, resolveRouteEntry(headers)->metadataMatchCriteria()); + const auto* entry = resolveRouteEntry(headers); + EXPECT_EQ(matched_route_criteria, entry->metadataMatchCriteria()); + // The retry policy still comes from the override, so the entry itself was selected. + EXPECT_EQ(7, entry->retryPolicy()->numRetries()); } } // namespace diff --git a/test/extensions/router/cluster_specifiers/dynamic_modules/integration_test.cc b/test/extensions/router/cluster_specifiers/dynamic_modules/integration_test.cc index 83a656ab38b0..34a2348c7ec0 100644 --- a/test/extensions/router/cluster_specifiers/dynamic_modules/integration_test.cc +++ b/test/extensions/router/cluster_specifiers/dynamic_modules/integration_test.cc @@ -43,8 +43,9 @@ class DynamicModuleClusterSpecifierIntegrationTest } config_helper_.addConfigModifier( - [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) { + [mirror_override = mirror_override_]( + envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { const std::string specifier_yaml = R"EOF( dynamic_module_config: name: cluster_specifier_integration_test @@ -58,8 +59,6 @@ specifier_name: test_cluster_specifier filter_metadata: envoy.lb: version: canary - request_mirror_policies: - - cluster: mirror-cluster stable: metadata_match: filter_metadata: @@ -73,6 +72,11 @@ specifier_name: test_cluster_specifier )EOF"; DynamicModuleClusterSpecifierProto specifier_config; TestUtility::loadFromYaml(specifier_yaml, specifier_config); + if (mirror_override) { + (*specifier_config.mutable_route_action_overrides())["canary"] + .add_request_mirror_policies() + ->set_cluster("mirror-cluster"); + } auto* route_action = hcm.mutable_route_config() ->mutable_virtual_hosts(0) @@ -138,6 +142,14 @@ fallback_policy: NO_FALLBACK }); } + // Declares the request mirroring policy of the canary override together with the cluster it + // names. A statically named mirror cluster has to exist when the route configuration is + // validated, so the two cannot be configured independently. + void configureMirrorOverride() { + addClusterCopy("mirror-cluster"); + mirror_override_ = true; + } + // Adds a copy of the upstream cluster under a second name, so that a test can tell which of two // clusters served a request rather than relying on the only cluster in the configuration. void addClusterCopy(const std::string& name) { @@ -176,6 +188,8 @@ fallback_policy: NO_FALLBACK const auto values = headers.get(Http::LowerCaseString(HostVersionHeader)); return values.empty() ? "" : std::string(values[0]->value().getStringView()); } + + bool mirror_override_{false}; }; INSTANTIATE_TEST_SUITE_P(IpVersions, DynamicModuleClusterSpecifierIntegrationTest, @@ -242,6 +256,21 @@ TEST_P(DynamicModuleClusterSpecifierIntegrationTest, UnknownClusterFailsRequest) EXPECT_EQ(1, test_server_->counter("http.config_test.no_cluster")->value()); } +// The cluster not found response code the module selected replaces the one of the matched route, so +// a module that derives cluster names can tell a name that resolves to nothing apart from an +// upstream that is unavailable. +TEST_P(DynamicModuleClusterSpecifierIntegrationTest, SelectedClusterNotFoundResponseCodeIsUsed) { + setupTest(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto response = codec_client_->makeHeaderOnlyRequest( + requestHeaders({{"env", "nonexistent"}, {"x-not-found-code", "404"}})); + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("404", response->headers().getStatusValue()); + EXPECT_EQ(1, test_server_->counter("http.config_test.no_cluster")->value()); +} + // Without a decision from the module the matched route has no cluster, so the request fails. TEST_P(DynamicModuleClusterSpecifierIntegrationTest, NoDecisionFailsRequest) { setupTest(); @@ -338,7 +367,7 @@ TEST_P(DynamicModuleClusterSpecifierIntegrationTest, NoOverrideFindsNoLoadBalanc // The request mirroring policies of the selected override reach the router, so the mirror cluster // receives a copy of the request. TEST_P(DynamicModuleClusterSpecifierIntegrationTest, OverrideMirrorsRequest) { - addClusterCopy("mirror-cluster"); + configureMirrorOverride(); setupTest(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -352,7 +381,7 @@ TEST_P(DynamicModuleClusterSpecifierIntegrationTest, OverrideMirrorsRequest) { // Without an override the mirroring policies of the matched route are in effect, and the matched // route has none. TEST_P(DynamicModuleClusterSpecifierIntegrationTest, NoOverrideDoesNotMirrorRequest) { - addClusterCopy("mirror-cluster"); + configureMirrorOverride(); setupTest(); codec_client_ = makeHttpConnection(lookupPort("http"));