diff --git a/api/envoy/extensions/filters/http/rbac/v3/rbac.proto b/api/envoy/extensions/filters/http/rbac/v3/rbac.proto index 649869a255d2..615102f75ef7 100644 --- a/api/envoy/extensions/filters/http/rbac/v3/rbac.proto +++ b/api/envoy/extensions/filters/http/rbac/v3/rbac.proto @@ -27,48 +27,55 @@ message RBAC { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.rbac.v2.RBAC"; - // Specify the RBAC rules to be applied globally. - // If absent, no enforcing RBAC policy will be applied. - // If present and empty, DENY. - // If both rules and matcher are configured, rules will be ignored. + // The primary RBAC policy which will be applied globally, to all the incoming requests. + // + // * If absent, no RBAC enforcement occurs. + // * If set but empty, all requests are denied. + // + // .. note:: + // + // When both ``rules`` and ``matcher`` are configured, ``rules`` will be ignored. + // config.rbac.v3.RBAC rules = 1 [(udpa.annotations.field_migrate).oneof_promotion = "rules_specifier"]; // If specified, rules will emit stats with the given prefix. - // This is useful to distinguish the stat when there are more than 1 RBAC filter configured with - // rules. + // This is useful for distinguishing metrics when multiple RBAC filters are configured. string rules_stat_prefix = 6; - // The match tree to use when resolving RBAC action for incoming requests. Requests do not - // match any matcher will be denied. - // If absent, no enforcing RBAC matcher will be applied. - // If present and empty, deny all requests. + // Match tree for evaluating RBAC actions on incoming requests. Requests not matching any matcher will be denied. + // + // * If absent, no RBAC enforcement occurs. + // * If set but empty, all requests are denied. + // xds.type.matcher.v3.Matcher matcher = 4 [ (udpa.annotations.field_migrate).oneof_promotion = "rules_specifier", (xds.annotations.v3.field_status).work_in_progress = true ]; - // Shadow rules are not enforced by the filter (i.e., returning a 403) - // but will emit stats and logs and can be used for rule testing. - // If absent, no shadow RBAC policy will be applied. - // If both shadow rules and shadow matcher are configured, shadow rules will be ignored. + // Shadow policy for testing RBAC rules without enforcing them. These rules generate stats and logs but do not deny + // requests. If absent, no shadow RBAC policy will be applied. + // + // .. note:: + // + // When both ``shadow_rules`` and ``shadow_matcher`` are configured, ``shadow_rules`` will be ignored. + // config.rbac.v3.RBAC shadow_rules = 2 [(udpa.annotations.field_migrate).oneof_promotion = "shadow_rules_specifier"]; - // The match tree to use for emitting stats and logs which can be used for rule testing for - // incoming requests. // If absent, no shadow matcher will be applied. + // Match tree for testing RBAC rules through stats and logs without enforcing them. + // If absent, no shadow matching occurs. xds.type.matcher.v3.Matcher shadow_matcher = 5 [ (udpa.annotations.field_migrate).oneof_promotion = "shadow_rules_specifier", (xds.annotations.v3.field_status).work_in_progress = true ]; // If specified, shadow rules will emit stats with the given prefix. - // This is useful to distinguish the stat when there are more than 1 RBAC filter configured with - // shadow rules. + // This is useful for distinguishing metrics when multiple RBAC filters use shadow rules. string shadow_rules_stat_prefix = 3; - // If track_per_rule_stats is true, counters will be published for each rule and shadow rule. + // If ``track_per_rule_stats`` is ``true``, counters will be published for each rule and shadow rule. bool track_per_rule_stats = 7; } @@ -78,7 +85,7 @@ message RBACPerRoute { reserved 1; - // Override the global configuration of the filter with this new config. - // If absent, the global RBAC policy will be disabled for this route. + // Per-route specific RBAC configuration that overrides the global RBAC configuration. + // If absent, RBAC policy will be disabled for this route. RBAC rbac = 2; } diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 7b2e3432d28a..1f7e1ec5f62d 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -17,6 +17,10 @@ new_features: change: | Added :ref:`virtualClusterName() ` API to the Stream Info Object to get the name of the virtual cluster matched. +- area: udp_proxy + change: | + Added support for outlier detection in UDP proxy. This change can be temporarily reverted by setting runtime guard + ``envoy.reloadable_features.enable_udp_proxy_outlier_detection`` to ``false``. - area: tcp_proxy change: | Added support for :ref:`backoff_options ` diff --git a/envoy/common/execution_context.h b/envoy/common/execution_context.h index fbe312bdc03e..ee1533fba2cf 100644 --- a/envoy/common/execution_context.h +++ b/envoy/common/execution_context.h @@ -107,9 +107,9 @@ class ScopedExecutionContext : NonCopyable { #define ENVOY_EXECUTION_SCOPE_CAT_(a, b) a##b #define ENVOY_EXECUTION_SCOPE_CAT(a, b) ENVOY_EXECUTION_SCOPE_CAT_(a, b) -// Invoked when |scopedObject| is active from the current line to the end of the current c++ scope. -// |trackedStream| is a OptRef from which a ExecutionContext is extracted. -// |scopedObject| is a pointer to a Envoy::Tracing::Span or a Http::FilterContext. +// Invoked when |scopedObject| is active from the current line to the end of the current C++ scope. +// |trackedStream| is an OptRef from which an ExecutionContext is extracted. +// |scopedObject| is a pointer to an Envoy::Tracing::Span or an Http::FilterContext. #define ENVOY_EXECUTION_SCOPE(trackedStream, scopedObject) \ Envoy::Cleanup ENVOY_EXECUTION_SCOPE_CAT(on_scope_exit_, __LINE__) = \ [execution_context = ExecutionContext::fromStreamInfo(trackedStream), \ diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index b0f9bab96e71..78c376788538 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -39,6 +39,7 @@ RUNTIME_GUARD(envoy_reloadable_features_dns_details); RUNTIME_GUARD(envoy_reloadable_features_dns_nodata_noname_is_success); RUNTIME_GUARD(envoy_reloadable_features_enable_compression_bomb_protection); RUNTIME_GUARD(envoy_reloadable_features_enable_include_histograms); +RUNTIME_GUARD(envoy_reloadable_features_enable_udp_proxy_outlier_detection); RUNTIME_GUARD(envoy_reloadable_features_explicit_internal_address_config); RUNTIME_GUARD(envoy_reloadable_features_ext_proc_timeout_error); RUNTIME_GUARD(envoy_reloadable_features_extend_h3_accept_untrusted); diff --git a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc index 190ab03d8673..b120ad90f71e 100644 --- a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc +++ b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc @@ -929,7 +929,7 @@ void TunnelingConnectionPoolImpl::onPoolFailure(Http::ConnectionPool::PoolFailur // removed by onStreamFailure, which will cause downstream_info_ to be freed. downstream_info_.upstreamInfo()->setUpstreamHost(host); downstream_info_.upstreamInfo()->setUpstreamTransportFailureReason(failure_reason); - callbacks_->onStreamFailure(reason, failure_reason, host); + callbacks_->onStreamFailure(reason, failure_reason, *host); } void TunnelingConnectionPoolImpl::onPoolReady(Http::RequestEncoder& request_encoder, @@ -1032,7 +1032,7 @@ bool UdpProxyFilter::TunnelingActiveSession::createConnectionPool() { void UdpProxyFilter::TunnelingActiveSession::onStreamFailure( ConnectionPool::PoolFailureReason reason, absl::string_view failure_reason, - Upstream::HostDescriptionConstSharedPtr) { + const Upstream::HostDescription& host) { ENVOY_LOG(debug, "Failed to create upstream stream: {}", failure_reason); conn_pool_.reset(); @@ -1045,12 +1045,20 @@ void UdpProxyFilter::TunnelingActiveSession::onStreamFailure( break; case ConnectionPool::PoolFailureReason::Timeout: udp_session_info_.setResponseFlag(StreamInfo::CoreResponseFlag::UpstreamConnectionFailure); + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.enable_udp_proxy_outlier_detection")) { + host.outlierDetector().putResult(Upstream::Outlier::Result::LocalOriginTimeout); + } onUpstreamEvent(Network::ConnectionEvent::RemoteClose); break; case ConnectionPool::PoolFailureReason::RemoteConnectionFailure: if (connecting_) { udp_session_info_.setResponseFlag(StreamInfo::CoreResponseFlag::UpstreamConnectionFailure); } + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.enable_udp_proxy_outlier_detection")) { + host.outlierDetector().putResult(Upstream::Outlier::Result::LocalOriginConnectFailed); + } onUpstreamEvent(Network::ConnectionEvent::RemoteClose); break; } @@ -1058,7 +1066,7 @@ void UdpProxyFilter::TunnelingActiveSession::onStreamFailure( void UdpProxyFilter::TunnelingActiveSession::onStreamReady(StreamInfo::StreamInfo* upstream_info, std::unique_ptr&& upstream, - Upstream::HostDescriptionConstSharedPtr&, + const Upstream::HostDescription& host, const Network::ConnectionInfoProvider&, Ssl::ConnectionInfoConstSharedPtr) { // TODO(ohadvano): save the host description to host_ field. This requires refactoring because @@ -1072,6 +1080,10 @@ void UdpProxyFilter::TunnelingActiveSession::onStreamReady(StreamInfo::StreamInf connecting_ = false; can_send_upstream_ = true; cluster_->cluster_stats_.sess_tunnel_success_.inc(); + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.enable_udp_proxy_outlier_detection")) { + host.outlierDetector().putResult(Upstream::Outlier::Result::LocalOriginConnectSuccessFinal); + } if (filter_.config_->flushAccessLogOnTunnelConnected()) { fillSessionStreamInfo(); diff --git a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.h b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.h index 427f8d9d6c8b..00d856cf334d 100644 --- a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.h +++ b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.h @@ -203,7 +203,7 @@ class HttpStreamCallbacks { * @param ssl_info supplies the ssl information of the upstream connection. */ virtual void onStreamReady(StreamInfo::StreamInfo* info, std::unique_ptr&& upstream, - Upstream::HostDescriptionConstSharedPtr& host, + const Upstream::HostDescription& host, const Network::ConnectionInfoProvider& address_provider, Ssl::ConnectionInfoConstSharedPtr ssl_info) PURE; @@ -216,7 +216,7 @@ class HttpStreamCallbacks { */ virtual void onStreamFailure(ConnectionPool::PoolFailureReason reason, absl::string_view failure_reason, - Upstream::HostDescriptionConstSharedPtr host) PURE; + const Upstream::HostDescription& host) PURE; /** * Called to reset the idle timer. @@ -418,13 +418,13 @@ class TunnelingConnectionPoolImpl : public TunnelingConnectionPool, // TunnelCreationCallbacks void onStreamSuccess(Http::RequestEncoder& request_encoder) override { - callbacks_->onStreamReady(upstream_info_, std::move(upstream_), upstream_host_, + callbacks_->onStreamReady(upstream_info_, std::move(upstream_), *upstream_host_, request_encoder.getStream().connectionInfoProvider(), ssl_info_); } void onStreamFailure() override { callbacks_->onStreamFailure(ConnectionPool::PoolFailureReason::RemoteConnectionFailure, "", - upstream_host_); + *upstream_host_); } // Http::ConnectionPool::Callbacks @@ -754,12 +754,11 @@ class UdpProxyFilter : public Network::UdpListenerReadFilter, // HttpStreamCallbacks void onStreamReady(StreamInfo::StreamInfo*, std::unique_ptr&&, - Upstream::HostDescriptionConstSharedPtr&, - const Network::ConnectionInfoProvider&, + const Upstream::HostDescription&, const Network::ConnectionInfoProvider&, Ssl::ConnectionInfoConstSharedPtr) override; void onStreamFailure(ConnectionPool::PoolFailureReason, absl::string_view, - Upstream::HostDescriptionConstSharedPtr) override; + const Upstream::HostDescription&) override; void resetIdleTimer() override { ActiveSession::resetIdleTimer(); } diff --git a/test/extensions/filters/udp/udp_proxy/mocks.h b/test/extensions/filters/udp/udp_proxy/mocks.h index 62ef8f3da605..64200bebd4c0 100644 --- a/test/extensions/filters/udp/udp_proxy/mocks.h +++ b/test/extensions/filters/udp/udp_proxy/mocks.h @@ -107,12 +107,12 @@ class MockHttpStreamCallbacks : public HttpStreamCallbacks { MOCK_METHOD(void, onStreamReady, (StreamInfo::StreamInfo * info, std::unique_ptr&& upstream, - Upstream::HostDescriptionConstSharedPtr& host, + const Upstream::HostDescription& host, const Network::ConnectionInfoProvider& address_provider, Ssl::ConnectionInfoConstSharedPtr ssl_info)); MOCK_METHOD(void, onStreamFailure, (ConnectionPool::PoolFailureReason reason, absl::string_view failure_reason, - Upstream::HostDescriptionConstSharedPtr host)); + const Upstream::HostDescription& host)); MOCK_METHOD(void, resetIdleTimer, ()); }; diff --git a/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc b/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc index 35e328349a6d..d1249dfb5990 100644 --- a/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc +++ b/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc @@ -453,6 +453,7 @@ use_original_src_ip: true ENVOY_SOCKET_IPV6_TRANSPARENT}; inline static const std::string upstream_ip_address_ = "20.0.0.1:443"; inline static const std::string peer_ip_address_ = "10.0.0.1:1000"; + NiceMock upstream_host_; }; class UdpProxyFilterIpv6Test : public UdpProxyFilterTest { @@ -1878,7 +1879,6 @@ stat_prefix: foo NiceMock stream_info; stream_info.downstream_connection_info_provider_->setConnectionID(0); - Upstream::HostDescriptionConstSharedPtr upstream_host; Network::ConnectionInfoSetterImpl address_provider(nullptr, nullptr); auto session = filter_->createTunnelingSession(); @@ -1899,7 +1899,7 @@ stat_prefix: foo })); session->onNewSession(); - session->onStreamReady(&stream_info, std::unique_ptr{upstream}, upstream_host, + session->onStreamReady(&stream_info, std::unique_ptr{upstream}, upstream_host_, address_provider, nullptr); } @@ -1928,7 +1928,6 @@ stat_prefix: foo NiceMock stream_info; stream_info.downstream_connection_info_provider_->setConnectionID(0); - Upstream::HostDescriptionConstSharedPtr upstream_host; Network::ConnectionInfoSetterImpl address_provider(nullptr, nullptr); auto session = filter_->createTunnelingSession(); @@ -1963,10 +1962,101 @@ stat_prefix: foo })); session->onNewSession(); - session->onStreamReady(&stream_info, std::unique_ptr{upstream}, upstream_host, + session->onStreamReady(&stream_info, std::unique_ptr{upstream}, upstream_host_, address_provider, nullptr); } +TEST_F(UdpProxyFilterTest, TunnelingSessionOutlierDetectionConnectSuccessFinal) { + Event::MockTimer* idle_timer = new Event::MockTimer(&callbacks_.udp_listener_.dispatcher_); + EXPECT_CALL(*idle_timer, enableTimer(_, _)).Times(0); + + setup(readConfig(R"EOF( +stat_prefix: foo +matcher: + on_no_match: + action: + name: route + typed_config: + '@type': type.googleapis.com/envoy.extensions.filters.udp.udp_proxy.v3.Route + cluster: fake_cluster +tunneling_config: + proxy_host: host.com + target_host: host.com + default_target_port: 30 + )EOF"), + true); + + NiceMock stream_info; + stream_info.downstream_connection_info_provider_->setConnectionID(0); + auto* upstream = new NiceMock(); + Network::ConnectionInfoSetterImpl address_provider(nullptr, nullptr); + + auto session = filter_->createTunnelingSession(); + session->onNewSession(); + + EXPECT_CALL(upstream_host_.outlier_detector_, + putResult(Upstream::Outlier::Result::LocalOriginConnectSuccessFinal, _)); + session->onStreamReady(&stream_info, std::unique_ptr{upstream}, upstream_host_, + address_provider, nullptr); +} + +TEST_F(UdpProxyFilterTest, TunnelingSessionOutlierDetectionConnectFailed) { + Event::MockTimer* idle_timer = new Event::MockTimer(&callbacks_.udp_listener_.dispatcher_); + EXPECT_CALL(*idle_timer, enableTimer(_, _)).Times(0); + + setup(readConfig(R"EOF( +stat_prefix: foo +matcher: + on_no_match: + action: + name: route + typed_config: + '@type': type.googleapis.com/envoy.extensions.filters.udp.udp_proxy.v3.Route + cluster: fake_cluster +tunneling_config: + proxy_host: host.com + target_host: host.com + default_target_port: 30 + )EOF"), + true); + + auto session = filter_->createTunnelingSession(); + session->onNewSession(); + + EXPECT_CALL(upstream_host_.outlier_detector_, + putResult(Upstream::Outlier::Result::LocalOriginConnectFailed, _)); + session->onStreamFailure(ConnectionPool::PoolFailureReason::RemoteConnectionFailure, "", + upstream_host_); +} + +TEST_F(UdpProxyFilterTest, TunnelingSessionOutlierDetectionTimeout) { + Event::MockTimer* idle_timer = new Event::MockTimer(&callbacks_.udp_listener_.dispatcher_); + EXPECT_CALL(*idle_timer, enableTimer(_, _)).Times(0); + + setup(readConfig(R"EOF( +stat_prefix: foo +matcher: + on_no_match: + action: + name: route + typed_config: + '@type': type.googleapis.com/envoy.extensions.filters.udp.udp_proxy.v3.Route + cluster: fake_cluster +tunneling_config: + proxy_host: host.com + target_host: host.com + default_target_port: 30 + )EOF"), + true); + + auto session = filter_->createTunnelingSession(); + session->onNewSession(); + + EXPECT_CALL(upstream_host_.outlier_detector_, + putResult(Upstream::Outlier::Result::LocalOriginTimeout, _)); + session->onStreamFailure(ConnectionPool::PoolFailureReason::Timeout, "", upstream_host_); +} + using MockUdpTunnelingConfig = SessionFilters::MockUdpTunnelingConfig; using MockUpstreamTunnelCallbacks = SessionFilters::MockUpstreamTunnelCallbacks; using MockTunnelCreationCallbacks = SessionFilters::MockTunnelCreationCallbacks; @@ -2367,14 +2457,19 @@ TEST_F(TunnelingConnectionPoolImplTest, PoolReady) { TEST_F(TunnelingConnectionPoolImplTest, OnStreamFailure) { setup(); createNewStream(); + pool_->onPoolReady(request_encoder_, upstream_host_, stream_info_, absl::nullopt); + EXPECT_CALL(stream_callbacks_, onStreamFailure(ConnectionPool::PoolFailureReason::RemoteConnectionFailure, "", _)); pool_->onStreamFailure(); + pool_->onDownstreamEvent(Network::ConnectionEvent::LocalClose); } TEST_F(TunnelingConnectionPoolImplTest, OnStreamSuccess) { setup(); createNewStream(); + pool_->onPoolReady(request_encoder_, upstream_host_, stream_info_, absl::nullopt); + EXPECT_CALL(stream_callbacks_, onStreamReady(_, _, _, _, _)); pool_->onStreamSuccess(request_encoder_); } diff --git a/tools/deprecate_guards/deprecate_guards.py b/tools/deprecate_guards/deprecate_guards.py index 0277cd55b543..bd3082046de2 100644 --- a/tools/deprecate_guards/deprecate_guards.py +++ b/tools/deprecate_guards/deprecate_guards.py @@ -180,7 +180,7 @@ async def create_issue(self, title: str, body: str, login: str) -> None: else: self.log.warning(f"Dry run: not creating issue '{title}' for {login}") return - except gidgethub.Exception: + except gidgethub.GitHubException: self.log.warning( f"unable to assign issue {title} to {login}. Add them to the Envoy proxy org" "and assign it their way.") @@ -195,7 +195,7 @@ async def create_issue(self, title: str, body: str, login: str) -> None: self.log.notice(f"Created issue '{title}'{extra_log}") else: self.log.warning(f"Dry run: not creating issue '{title}'{extra_log}") - except gidgethub.Exception as e: + except gidgethub.GitHubException as e: self.log.error("Github error while creating issue.\n{e}") raise DeprecateGuardsError(e)