diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 816b41c0c959..72ede7e05608 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -25,6 +25,9 @@ bug_fixes: change: | Fix a RELEASE_ASSERT when using :ref:`auto_sni ` if the downstream request ``:authority`` was longer than 255 characters. +- area: http + change: | + Fix a crash when reloading the HTTP Connection Manager via ECDS. removed_config_or_runtime: diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index 6730cfb1d322..6e060f375c9f 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -545,5 +545,7 @@ class ConnectionManagerConfig { */ virtual bool addProxyProtocolConnectionState() const PURE; }; + +using ConnectionManagerConfigSharedPtr = std::shared_ptr; } // namespace Http } // namespace Envoy diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 00ad942ecf92..311f32ceaeb9 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -98,7 +98,7 @@ ConnectionManagerImpl::generateListenerStats(const std::string& prefix, Stats::S return {CONN_MAN_LISTENER_STATS(POOL_COUNTER_PREFIX(scope, prefix))}; } -ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfig& config, +ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfigSharedPtr config, const Network::DrainDecision& drain_close, Random::RandomGenerator& random_generator, Http::Context& http_context, Runtime::Loader& runtime, @@ -106,12 +106,12 @@ ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfig& config, Upstream::ClusterManager& cluster_manager, Server::OverloadManager& overload_manager, TimeSource& time_source) - : config_(config), stats_(config_.stats()), + : config_(std::move(config)), stats_(config_->stats()), conn_length_(new Stats::HistogramCompletableTimespanImpl( stats_.named_.downstream_cx_length_ms_, time_source)), drain_close_(drain_close), user_agent_(http_context.userAgentContext()), random_generator_(random_generator), runtime_(runtime), local_info_(local_info), - cluster_manager_(cluster_manager), listener_stats_(config_.listenerStats()), + cluster_manager_(cluster_manager), listener_stats_(config_->listenerStats()), overload_manager_(overload_manager), overload_state_(overload_manager.getThreadLocalOverloadState()), accept_new_http_stream_( @@ -124,8 +124,8 @@ ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfig& config, overload_state_.getState(Server::OverloadActionNames::get().DisableHttpKeepAlive)), time_source_(time_source), proxy_name_(StreamInfo::ProxyStatusUtils::makeProxyName( /*node_id=*/local_info_.node().id(), - /*server_name=*/config_.serverName(), - /*proxy_status_config=*/config_.proxyStatusConfig())), + /*server_name=*/config_->serverName(), + /*proxy_status_config=*/config_->proxyStatusConfig())), max_requests_during_dispatch_( runtime_.snapshot().getInteger(ConnectionManagerImpl::MaxRequestsPerIoCycle, UINT32_MAX)), refresh_rtt_after_request_( @@ -162,7 +162,7 @@ void ConnectionManagerImpl::initializeReadFilterCallbacks(Network::ReadFilterCal read_callbacks_->connection().addConnectionCallbacks(*this); - if (config_.addProxyProtocolConnectionState() && + if (config_->addProxyProtocolConnectionState() && !read_callbacks_->connection() .streamInfo() .filterState() @@ -176,20 +176,20 @@ void ConnectionManagerImpl::initializeReadFilterCallbacks(Network::ReadFilterCal StreamInfo::FilterState::LifeSpan::Connection); } - if (config_.idleTimeout()) { + if (config_->idleTimeout()) { connection_idle_timer_ = dispatcher_->createScaledTimer(Event::ScaledTimerType::HttpDownstreamIdleConnectionTimeout, [this]() -> void { onIdleTimeout(); }); - connection_idle_timer_->enableTimer(config_.idleTimeout().value()); + connection_idle_timer_->enableTimer(config_->idleTimeout().value()); } - if (config_.maxConnectionDuration()) { + if (config_->maxConnectionDuration()) { connection_duration_timer_ = dispatcher_->createTimer([this]() -> void { onConnectionDurationTimeout(); }); - connection_duration_timer_->enableTimer(config_.maxConnectionDuration().value()); + connection_duration_timer_->enableTimer(config_->maxConnectionDuration().value()); } - read_callbacks_->connection().setDelayedCloseTimeout(config_.delayedCloseTimeout()); + read_callbacks_->connection().setDelayedCloseTimeout(config_->delayedCloseTimeout()); read_callbacks_->connection().setConnectionStats( {stats_.named_.downstream_cx_rx_bytes_total_, stats_.named_.downstream_cx_rx_bytes_buffered_, @@ -377,7 +377,7 @@ void ConnectionManagerImpl::doDeferredStreamDestroy(ActiveStream& stream) { } if (connection_idle_timer_ && streams_.empty()) { - connection_idle_timer_->enableTimer(config_.idleTimeout().value()); + connection_idle_timer_->enableTimer(config_->idleTimeout().value()); } maybeDrainDueToPrematureResets(); } @@ -412,8 +412,8 @@ RequestDecoder& ConnectionManagerImpl::newStream(ResponseEncoder& response_encod *this, response_encoder.getStream().bufferLimit(), std::move(downstream_stream_account)); accumulated_requests_++; - if (config_.maxRequestsPerConnection() > 0 && - accumulated_requests_ >= config_.maxRequestsPerConnection()) { + if (config_->maxRequestsPerConnection() > 0 && + accumulated_requests_ >= config_->maxRequestsPerConnection()) { if (codec_->protocol() < Protocol::Http2) { new_stream->filter_manager_.streamInfo().setShouldDrainConnectionUponCompletion(true); // Prevent erroneous debug log of closing due to incoming connection close header. @@ -462,7 +462,7 @@ void ConnectionManagerImpl::handleCodecOverloadError(absl::string_view error) { void ConnectionManagerImpl::createCodec(Buffer::Instance& data) { ASSERT(!codec_); - codec_ = config_.createCodec(read_callbacks_->connection(), data, *this, overload_manager_); + codec_ = config_->createCodec(read_callbacks_->connection(), data, *this, overload_manager_); switch (codec_->protocol()) { case Protocol::Http3: @@ -772,16 +772,16 @@ ConnectionManagerImpl::HttpStreamIdProviderImpl::toStringView() const { if (parent_.request_headers_ == nullptr) { return {}; } - ASSERT(parent_.connection_manager_.config_.requestIDExtension() != nullptr); - return parent_.connection_manager_.config_.requestIDExtension()->get(*parent_.request_headers_); + ASSERT(parent_.connection_manager_.config_->requestIDExtension() != nullptr); + return parent_.connection_manager_.config_->requestIDExtension()->get(*parent_.request_headers_); } absl::optional ConnectionManagerImpl::HttpStreamIdProviderImpl::toInteger() const { if (parent_.request_headers_ == nullptr) { return {}; } - ASSERT(parent_.connection_manager_.config_.requestIDExtension() != nullptr); - return parent_.connection_manager_.config_.requestIDExtension()->getInteger( + ASSERT(parent_.connection_manager_.config_->requestIDExtension() != nullptr); + return parent_.connection_manager_.config_->requestIDExtension()->getInteger( *parent_.request_headers_); } @@ -789,34 +789,34 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect uint32_t buffer_limit, Buffer::BufferMemoryAccountSharedPtr account) : connection_manager_(connection_manager), - connection_manager_tracing_config_(connection_manager_.config_.tracingConfig() == nullptr + connection_manager_tracing_config_(connection_manager_.config_->tracingConfig() == nullptr ? absl::nullopt : makeOptRef( - *connection_manager_.config_.tracingConfig())), + *connection_manager_.config_->tracingConfig())), stream_id_(connection_manager.random_generator_.random()), filter_manager_( *this, *connection_manager_.dispatcher_, connection_manager_.read_callbacks_->connection(), stream_id_, std::move(account), - connection_manager_.config_.proxy100Continue(), buffer_limit, - connection_manager_.config_.filterFactory(), connection_manager_.config_.localReply(), + connection_manager_.config_->proxy100Continue(), buffer_limit, + connection_manager_.config_->filterFactory(), connection_manager_.config_->localReply(), connection_manager_.codec_->protocol(), connection_manager_.timeSource(), connection_manager_.read_callbacks_->connection().streamInfo().filterState(), StreamInfo::FilterState::LifeSpan::Connection, connection_manager_.overload_manager_), request_response_timespan_(new Stats::HistogramCompletableTimespanImpl( connection_manager_.stats_.named_.downstream_rq_time_, connection_manager_.timeSource())), header_validator_( - connection_manager.config_.makeHeaderValidator(connection_manager.codec_->protocol())) { - ASSERT(!connection_manager.config_.isRoutable() || - ((connection_manager.config_.routeConfigProvider() == nullptr && - connection_manager.config_.scopedRouteConfigProvider() != nullptr && - connection_manager.config_.scopeKeyBuilder().has_value()) || - (connection_manager.config_.routeConfigProvider() != nullptr && - connection_manager.config_.scopedRouteConfigProvider() == nullptr && - !connection_manager.config_.scopeKeyBuilder().has_value())), + connection_manager.config_->makeHeaderValidator(connection_manager.codec_->protocol())) { + ASSERT(!connection_manager.config_->isRoutable() || + ((connection_manager.config_->routeConfigProvider() == nullptr && + connection_manager.config_->scopedRouteConfigProvider() != nullptr && + connection_manager.config_->scopeKeyBuilder().has_value()) || + (connection_manager.config_->routeConfigProvider() != nullptr && + connection_manager.config_->scopedRouteConfigProvider() == nullptr && + !connection_manager.config_->scopeKeyBuilder().has_value())), "Either routeConfigProvider or (scopedRouteConfigProvider and scopeKeyBuilder) should be " "set in " "ConnectionManagerImpl."); - for (const AccessLog::InstanceSharedPtr& access_log : connection_manager_.config_.accessLogs()) { + for (const AccessLog::InstanceSharedPtr& access_log : connection_manager_.config_->accessLogs()) { filter_manager_.addAccessLogHandler(access_log); } @@ -827,16 +827,16 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect static const std::string route_factory = "envoy.route_config_update_requester.default"; auto factory = Envoy::Config::Utility::getFactoryByName(route_factory); - if (connection_manager_.config_.isRoutable() && - connection_manager.config_.routeConfigProvider() != nullptr && factory) { - route_config_update_requester_ = - factory->createRouteConfigUpdateRequester(connection_manager.config_.routeConfigProvider()); - } else if (connection_manager_.config_.isRoutable() && - connection_manager.config_.scopedRouteConfigProvider() != nullptr && - connection_manager.config_.scopeKeyBuilder().has_value() && factory) { + if (connection_manager_.config_->isRoutable() && + connection_manager.config_->routeConfigProvider() != nullptr && factory) { route_config_update_requester_ = factory->createRouteConfigUpdateRequester( - connection_manager.config_.scopedRouteConfigProvider(), - connection_manager.config_.scopeKeyBuilder()); + connection_manager.config_->routeConfigProvider()); + } else if (connection_manager_.config_->isRoutable() && + connection_manager.config_->scopedRouteConfigProvider() != nullptr && + connection_manager.config_->scopeKeyBuilder().has_value() && factory) { + route_config_update_requester_ = factory->createRouteConfigUpdateRequester( + connection_manager.config_->scopedRouteConfigProvider(), + connection_manager.config_->scopeKeyBuilder()); } ScopeTrackerScopeState scope(this, connection_manager_.read_callbacks_->connection().dispatcher()); @@ -851,38 +851,38 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect connection_manager_.stats_.named_.downstream_rq_http1_total_.inc(); } - if (connection_manager_.config_.streamIdleTimeout().count()) { - idle_timeout_ms_ = connection_manager_.config_.streamIdleTimeout(); + if (connection_manager_.config_->streamIdleTimeout().count()) { + idle_timeout_ms_ = connection_manager_.config_->streamIdleTimeout(); stream_idle_timer_ = connection_manager_.dispatcher_->createScaledTimer( Event::ScaledTimerType::HttpDownstreamIdleStreamTimeout, [this]() -> void { onIdleTimeout(); }); resetIdleTimer(); } - if (connection_manager_.config_.requestTimeout().count()) { - std::chrono::milliseconds request_timeout = connection_manager_.config_.requestTimeout(); + if (connection_manager_.config_->requestTimeout().count()) { + std::chrono::milliseconds request_timeout = connection_manager_.config_->requestTimeout(); request_timer_ = connection_manager.dispatcher_->createTimer([this]() -> void { onRequestTimeout(); }); request_timer_->enableTimer(request_timeout, this); } - if (connection_manager_.config_.requestHeadersTimeout().count()) { + if (connection_manager_.config_->requestHeadersTimeout().count()) { std::chrono::milliseconds request_headers_timeout = - connection_manager_.config_.requestHeadersTimeout(); + connection_manager_.config_->requestHeadersTimeout(); request_header_timer_ = connection_manager.dispatcher_->createTimer([this]() -> void { onRequestHeaderTimeout(); }); request_header_timer_->enableTimer(request_headers_timeout, this); } - const auto max_stream_duration = connection_manager_.config_.maxStreamDuration(); + const auto max_stream_duration = connection_manager_.config_->maxStreamDuration(); if (max_stream_duration.has_value() && max_stream_duration.value().count()) { max_stream_duration_timer_ = connection_manager.dispatcher_->createTimer( [this]() -> void { onStreamMaxDurationReached(); }); - max_stream_duration_timer_->enableTimer(connection_manager_.config_.maxStreamDuration().value(), - this); + max_stream_duration_timer_->enableTimer( + connection_manager_.config_->maxStreamDuration().value(), this); } - if (connection_manager_.config_.accessLogFlushInterval().has_value()) { + if (connection_manager_.config_->accessLogFlushInterval().has_value()) { access_log_flush_timer_ = connection_manager.dispatcher_->createTimer([this]() -> void { // If the request is complete, we've already done the stream-end access-log, and shouldn't // do the periodic log. @@ -910,7 +910,7 @@ void ConnectionManagerImpl::ActiveStream::completeRequest() { connection_manager_.stats_.named_.downstream_rq_active_.dec(); if (filter_manager_.streamInfo().healthCheck()) { - connection_manager_.config_.tracingStats().health_check_.inc(); + connection_manager_.config_->tracingStats().health_check_.inc(); } if (active_span_) { @@ -1187,17 +1187,17 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPt } // We need to snap snapped_route_config_ here as it's used in mutateRequestHeaders later. - if (connection_manager_.config_.isRoutable()) { - if (connection_manager_.config_.routeConfigProvider() != nullptr) { - snapped_route_config_ = connection_manager_.config_.routeConfigProvider()->configCast(); - } else if (connection_manager_.config_.scopedRouteConfigProvider() != nullptr && - connection_manager_.config_.scopeKeyBuilder().has_value()) { + if (connection_manager_.config_->isRoutable()) { + if (connection_manager_.config_->routeConfigProvider() != nullptr) { + snapped_route_config_ = connection_manager_.config_->routeConfigProvider()->configCast(); + } else if (connection_manager_.config_->scopedRouteConfigProvider() != nullptr && + connection_manager_.config_->scopeKeyBuilder().has_value()) { snapped_scoped_routes_config_ = - connection_manager_.config_.scopedRouteConfigProvider()->config(); + connection_manager_.config_->scopedRouteConfigProvider()->config(); snapScopedRouteConfig(); } } else { - snapped_route_config_ = connection_manager_.config_.routeConfigProvider()->configCast(); + snapped_route_config_ = connection_manager_.config_->routeConfigProvider()->configCast(); } // Drop new requests when overloaded as soon as we have decoded the headers. @@ -1215,7 +1215,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPt sendLocalReply( Http::Code::ServiceUnavailable, "envoy overloaded", [this](Http::ResponseHeaderMap& headers) { - if (connection_manager_.config_.appendLocalOverload()) { + if (connection_manager_.config_->appendLocalOverload()) { headers.addReference(Http::Headers::get().EnvoyLocalOverloaded, Http::Headers::get().EnvoyOverloadedValues.True); } @@ -1224,7 +1224,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPt return; } - if (!connection_manager_.config_.proxy100Continue() && request_headers_->Expect() && + if (!connection_manager_.config_->proxy100Continue() && request_headers_->Expect() && // The Expect field-value is case-insensitive. // https://tools.ietf.org/html/rfc7231#section-5.1.1 absl::EqualsIgnoreCase((request_headers_->Expect()->value().getStringView()), @@ -1291,7 +1291,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPt // In UHV mode path normalization is done in the UHV // Path sanitization should happen before any path access other than the above sanity check. const auto action = - ConnectionManagerUtility::maybeNormalizePath(*request_headers_, connection_manager_.config_); + ConnectionManagerUtility::maybeNormalizePath(*request_headers_, *connection_manager_.config_); // gRPC requests are rejected if Envoy is configured to redirect post-normalization. This is // because gRPC clients do not support redirect. if (action == ConnectionManagerUtility::NormalizePathAction::Reject || @@ -1316,7 +1316,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPt ASSERT(action == ConnectionManagerUtility::NormalizePathAction::Continue); #endif auto optional_port = ConnectionManagerUtility::maybeNormalizeHost( - *request_headers_, connection_manager_.config_, localPort()); + *request_headers_, *connection_manager_.config_, localPort()); if (optional_port.has_value() && requestWasConnect(request_headers_, connection_manager_.codec_->protocol())) { filter_manager_.streamInfo().filterState()->setData( @@ -1329,7 +1329,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPt // Modify the downstream remote address depending on configuration and headers. const auto mutate_result = ConnectionManagerUtility::mutateRequestHeaders( *request_headers_, connection_manager_.read_callbacks_->connection(), - connection_manager_.config_, *snapped_route_config_, connection_manager_.local_info_, + *connection_manager_.config_, *snapped_route_config_, connection_manager_.local_info_, filter_manager_.streamInfo()); // IP detection failed, reject the request. @@ -1352,7 +1352,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPt if (!state_.is_internally_created_) { // Only mutate tracing headers on first pass. filter_manager_.streamInfo().setTraceReason( ConnectionManagerUtility::mutateTracingRequestHeader( - *request_headers_, connection_manager_.runtime_, connection_manager_.config_, + *request_headers_, connection_manager_.runtime_, *connection_manager_.config_, cached_route_.value().get())); } @@ -1360,7 +1360,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPt const bool upgrade_rejected = filter_manager_.createFilterChain() == false; - if (connection_manager_.config_.flushAccessLogOnNewRequest()) { + if (connection_manager_.config_->flushAccessLogOnNewRequest()) { filter_manager_.log(AccessLog::AccessLogType::DownstreamStart); } @@ -1402,7 +1402,7 @@ void ConnectionManagerImpl::ActiveStream::traceRequest() { const Tracing::Decision tracing_decision = Tracing::TracerUtility::shouldTraceRequest(filter_manager_.streamInfo()); ConnectionManagerImpl::chargeTracingStats(tracing_decision.reason, - connection_manager_.config_.tracingStats()); + connection_manager_.config_->tracingStats()); Tracing::HttpTraceContext trace_context(*request_headers_); active_span_ = connection_manager_.tracer().startSpan( @@ -1512,14 +1512,14 @@ void ConnectionManagerImpl::startDrainSequence() { drain_state_ = DrainState::Draining; codec_->shutdownNotice(); drain_timer_ = dispatcher_->createTimer([this]() -> void { onDrainTimeout(); }); - drain_timer_->enableTimer(config_.drainTimeout()); + drain_timer_->enableTimer(config_->drainTimeout()); } void ConnectionManagerImpl::ActiveStream::snapScopedRouteConfig() { // NOTE: if a RDS subscription hasn't got a RouteConfiguration back, a Router::NullConfigImpl is // returned, in that case we let it pass. auto scope_key = - connection_manager_.config_.scopeKeyBuilder()->computeScopeKey(*request_headers_); + connection_manager_.config_->scopeKeyBuilder()->computeScopeKey(*request_headers_); snapped_route_config_ = snapped_scoped_routes_config_->getRouteConfig(scope_key); if (snapped_route_config_ == nullptr) { ENVOY_STREAM_LOG(trace, "can't find SRDS scope.", *this); @@ -1554,7 +1554,7 @@ void ConnectionManagerImpl::ActiveStream::refreshDurationTimeout() { } else { // Fall back to HCM config. If no HCM duration limit exists, disable // timers set by any prior route configuration. - const auto max_stream_duration = connection_manager_.config_.maxStreamDuration(); + const auto max_stream_duration = connection_manager_.config_->maxStreamDuration(); if (max_stream_duration.has_value() && max_stream_duration.value().count()) { timeout = max_stream_duration.value(); } else { @@ -1627,9 +1627,9 @@ void ConnectionManagerImpl::ActiveStream::refreshCachedRoute(const Router::Route Router::RouteConstSharedPtr route; if (request_headers_ != nullptr) { - if (connection_manager_.config_.isRoutable() && - connection_manager_.config_.scopedRouteConfigProvider() != nullptr && - connection_manager_.config_.scopeKeyBuilder().has_value()) { + if (connection_manager_.config_->isRoutable() && + connection_manager_.config_->scopedRouteConfigProvider() != nullptr && + connection_manager_.config_->scopeKeyBuilder().has_value()) { // NOTE: re-select scope as well in case the scope key header has been changed by a filter. snapScopedRouteConfig(); } @@ -1679,8 +1679,8 @@ void ConnectionManagerImpl::ActiveStream::requestRouteConfigUpdate( } absl::optional ConnectionManagerImpl::ActiveStream::routeConfig() { - if (connection_manager_.config_.routeConfigProvider() != nullptr) { - return {connection_manager_.config_.routeConfigProvider()->configCast()}; + if (connection_manager_.config_->routeConfigProvider() != nullptr) { + return {connection_manager_.config_->routeConfigProvider()->configCast()}; } return {}; } @@ -1697,7 +1697,7 @@ void ConnectionManagerImpl::ActiveStream::encode1xxHeaders(ResponseHeaderMap& re // Strip the T-E headers etc. Defer other header additions as well as drain-close logic to the // continuation headers. ConnectionManagerUtility::mutateResponseHeaders( - response_headers, request_headers_.get(), connection_manager_.config_, EMPTY_STRING, + response_headers, request_headers_.get(), *connection_manager_.config_, EMPTY_STRING, filter_manager_.streamInfo(), connection_manager_.proxy_name_, connection_manager_.clear_hop_by_hop_response_headers_); @@ -1716,20 +1716,20 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ResponseHeaderMap& heade // We want to preserve the original date header, but we add a date header if it is absent if (!headers.Date()) { - connection_manager_.config_.dateProvider().setDateHeader(headers); + connection_manager_.config_->dateProvider().setDateHeader(headers); } // Following setReference() is safe because serverName() is constant for the life of the // listener. - const auto transformation = connection_manager_.config_.serverHeaderTransformation(); + const auto transformation = connection_manager_.config_->serverHeaderTransformation(); if (transformation == ConnectionManagerConfig::HttpConnectionManagerProto::OVERWRITE || (transformation == ConnectionManagerConfig::HttpConnectionManagerProto::APPEND_IF_ABSENT && headers.Server() == nullptr)) { - headers.setReferenceServer(connection_manager_.config_.serverName()); + headers.setReferenceServer(connection_manager_.config_->serverName()); } ConnectionManagerUtility::mutateResponseHeaders( - headers, request_headers_.get(), connection_manager_.config_, - connection_manager_.config_.via(), filter_manager_.streamInfo(), + headers, request_headers_.get(), *connection_manager_.config_, + connection_manager_.config_->via(), filter_manager_.streamInfo(), connection_manager_.proxy_name_, connection_manager_.clear_hop_by_hop_response_headers_); bool drain_connection_due_to_overload = false; @@ -1831,7 +1831,7 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ResponseHeaderMap& heade chargeStats(headers); if (state_.is_tunneling_ && - connection_manager_.config_.flushAccessLogOnTunnelSuccessfullyEstablished()) { + connection_manager_.config_->flushAccessLogOnTunnelSuccessfullyEstablished()) { filter_manager_.log(AccessLog::AccessLogType::DownstreamTunnelSuccessfullyEstablished); } ENVOY_STREAM_LOG(debug, "encoding headers via codec (end_stream={}):\n{}", *this, end_stream, @@ -2104,9 +2104,9 @@ void ConnectionManagerImpl::ActiveStream::refreshIdleTimeout() { } void ConnectionManagerImpl::ActiveStream::refreshAccessLogFlushTimer() { - if (connection_manager_.config_.accessLogFlushInterval().has_value()) { + if (connection_manager_.config_->accessLogFlushInterval().has_value()) { access_log_flush_timer_->enableTimer( - connection_manager_.config_.accessLogFlushInterval().value(), this); + connection_manager_.config_->accessLogFlushInterval().value(), this); } } diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index 63c3c7a8b857..5f546ed9be3d 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -64,7 +64,8 @@ class ConnectionManagerImpl : Logger::Loggable, public Network::ConnectionCallbacks, public Http::ApiListener { public: - ConnectionManagerImpl(ConnectionManagerConfig& config, const Network::DrainDecision& drain_close, + ConnectionManagerImpl(ConnectionManagerConfigSharedPtr config, + const Network::DrainDecision& drain_close, Random::RandomGenerator& random_generator, Http::Context& http_context, Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info, Upstream::ClusterManager& cluster_manager, @@ -566,7 +567,7 @@ class ConnectionManagerImpl : Logger::Loggable, void onConnectionDurationTimeout(); void onDrainTimeout(); void startDrainSequence(); - Tracing::Tracer& tracer() { return *config_.tracer(); } + Tracing::Tracer& tracer() { return *config_->tracer(); } void handleCodecErrorImpl(absl::string_view error, absl::string_view details, StreamInfo::CoreResponseFlag response_flag); void handleCodecError(absl::string_view error); @@ -589,7 +590,7 @@ class ConnectionManagerImpl : Logger::Loggable, enum class DrainState { NotDraining, Draining, Closing }; - ConnectionManagerConfig& config_; + ConnectionManagerConfigSharedPtr config_; ConnectionManagerStats& stats_; // We store a reference here to avoid an extra stats() call on // the config in the hot path. ServerConnectionPtr codec_; diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 1bfa9b129523..c3ae4b7ed3d7 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -295,7 +295,7 @@ HttpConnectionManagerFilterConfigFactory::createFilterFactoryFromProtoAndHopByHo auto& server_context = context.serverFactoryContext(); auto hcm = std::make_shared( - *filter_config, context.drainDecision(), server_context.api().randomGenerator(), + filter_config, context.drainDecision(), server_context.api().randomGenerator(), server_context.httpContext(), server_context.runtime(), server_context.localInfo(), server_context.clusterManager(), server_context.overloadManager(), server_context.mainThreadDispatcher().timeSource()); @@ -852,7 +852,7 @@ HttpConnectionManagerFactory::createHttpConnectionManagerFactoryFromProto( auto& server_context = context.serverFactoryContext(); auto conn_manager = std::make_unique( - *filter_config, context.drainDecision(), server_context.api().randomGenerator(), + filter_config, context.drainDecision(), server_context.api().randomGenerator(), server_context.httpContext(), server_context.runtime(), server_context.localInfo(), server_context.clusterManager(), server_context.overloadManager(), server_context.mainThreadDispatcher().timeSource()); diff --git a/source/server/admin/admin.cc b/source/server/admin/admin.cc index e712a3e1b5ea..c41aca81f5d0 100644 --- a/source/server/admin/admin.cc +++ b/source/server/admin/admin.cc @@ -288,9 +288,9 @@ bool AdminImpl::createNetworkFilterChain(Network::Connection& connection, // Pass in the null overload manager so that the admin interface is accessible even when Envoy // is overloaded. connection.addReadFilter(Network::ReadFilterSharedPtr{new Http::ConnectionManagerImpl( - *this, server_.drainManager(), server_.api().randomGenerator(), server_.httpContext(), - server_.runtime(), server_.localInfo(), server_.clusterManager(), null_overload_manager_, - server_.timeSource())}); + shared_from_this(), server_.drainManager(), server_.api().randomGenerator(), + server_.httpContext(), server_.runtime(), server_.localInfo(), server_.clusterManager(), + null_overload_manager_, server_.timeSource())}); return true; } diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index ef0cde79927b..085826847b3d 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -67,6 +67,7 @@ class AdminImpl : public Admin, public Network::FilterChainFactory, public Http::FilterChainFactory, public Http::ConnectionManagerConfig, + public std::enable_shared_from_this, Logger::Loggable { public: AdminImpl(const std::string& profile_path, Server::Instance& server, diff --git a/source/server/server.cc b/source/server/server.cc index a6edd0bf380a..e7e1ffb49a51 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -606,7 +606,7 @@ absl::Status InstanceBase::initializeOrThrow(Network::Address::InstanceConstShar OptRef config_tracker; #ifdef ENVOY_ADMIN_FUNCTIONALITY - admin_ = std::make_unique(initial_config.admin().profilePath(), *this, + admin_ = std::make_shared(initial_config.admin().profilePath(), *this, initial_config.admin().ignoreGlobalConnLimit()); config_tracker = admin_->getConfigTracker(); diff --git a/source/server/server.h b/source/server/server.h index ae9b1ea95bc0..1b0bd9f8259b 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -379,7 +379,7 @@ class InstanceBase : Logger::Loggable, std::unique_ptr ssl_context_manager_; Event::DispatcherPtr dispatcher_; AccessLog::AccessLogManagerImpl access_log_manager_; - std::unique_ptr admin_; + std::shared_ptr admin_; Singleton::ManagerPtr singleton_manager_; Network::ConnectionHandlerPtr handler_; std::unique_ptr runtime_; diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index cf11c952f996..13275d0380b7 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -605,7 +605,7 @@ DEFINE_PROTO_FUZZER(const test::common::http::ConnManagerImplTestCase& input) { return; } - FuzzConfig config(input.forward_client_cert()); + std::shared_ptr config = std::make_shared(input.forward_client_cert()); NiceMock drain_close; NiceMock random; Stats::SymbolTableImpl symbol_table; @@ -630,7 +630,7 @@ DEFINE_PROTO_FUZZER(const test::common::http::ConnManagerImplTestCase& input) { std::make_shared("0.0.0.0")); ConnectionManagerImpl conn_manager(config, drain_close, random, http_context, runtime, local_info, - cluster_manager, overload_manager, config.time_system_); + cluster_manager, overload_manager, config->time_system_); conn_manager.initializeReadFilterCallbacks(filter_callbacks); std::vector streams; @@ -645,7 +645,7 @@ DEFINE_PROTO_FUZZER(const test::common::http::ConnManagerImplTestCase& input) { switch (action.action_selector_case()) { case test::common::http::Action::kNewStream: { streams.emplace_back(new FuzzStream( - conn_manager, config, + conn_manager, *config, Fuzz::fromHeaders(action.new_stream().request_headers(), /* ignore_headers =*/{}, {":authority"}), action.new_stream().status(), action.new_stream().end_stream())); diff --git a/test/common/http/conn_manager_impl_test_base.cc b/test/common/http/conn_manager_impl_test_base.cc index 663c0eddee4b..bdad9c1c95fb 100644 --- a/test/common/http/conn_manager_impl_test_base.cc +++ b/test/common/http/conn_manager_impl_test_base.cc @@ -12,6 +12,137 @@ using testing::Return; namespace Envoy { namespace Http { +// This is a hack that allows getting a shared_ptr to the config, without the config +// having a lifetime controlled by a shared_ptr. +// +// This is here to avoid an enormous change in the HCM tests to make +// `HttpConnectionManagerImplMixin` be a separate object owned by a shared_ptr instead of a mixin. +class ConnectionManagerConfigProxyObject : public ConnectionManagerConfig { +public: + ConnectionManagerConfigProxyObject(ConnectionManagerConfig& parent) : parent_(parent) {} + const RequestIDExtensionSharedPtr& requestIDExtension() override { + return parent_.requestIDExtension(); + } + const std::list& accessLogs() override { + return parent_.accessLogs(); + } + const absl::optional& accessLogFlushInterval() override { + return parent_.accessLogFlushInterval(); + } + bool flushAccessLogOnNewRequest() override { return parent_.flushAccessLogOnNewRequest(); } + bool flushAccessLogOnTunnelSuccessfullyEstablished() const override { + return parent_.flushAccessLogOnTunnelSuccessfullyEstablished(); + } + ServerConnectionPtr createCodec(Network::Connection& connection, const Buffer::Instance& data, + ServerConnectionCallbacks& callbacks, + Server::OverloadManager& overload_manager) override { + return parent_.createCodec(connection, data, callbacks, overload_manager); + } + DateProvider& dateProvider() override { return parent_.dateProvider(); } + std::chrono::milliseconds drainTimeout() const override { return parent_.drainTimeout(); } + FilterChainFactory& filterFactory() override { return parent_.filterFactory(); } + bool generateRequestId() const override { return parent_.generateRequestId(); } + bool preserveExternalRequestId() const override { return parent_.preserveExternalRequestId(); } + bool alwaysSetRequestIdInResponse() const override { + return parent_.alwaysSetRequestIdInResponse(); + } + absl::optional idleTimeout() const override { + return parent_.idleTimeout(); + } + bool isRoutable() const override { return parent_.isRoutable(); } + absl::optional maxConnectionDuration() const override { + return parent_.maxConnectionDuration(); + } + uint32_t maxRequestHeadersKb() const override { return parent_.maxRequestHeadersKb(); } + uint32_t maxRequestHeadersCount() const override { return parent_.maxRequestHeadersCount(); } + std::chrono::milliseconds streamIdleTimeout() const override { + return parent_.streamIdleTimeout(); + } + std::chrono::milliseconds requestTimeout() const override { return parent_.requestTimeout(); } + std::chrono::milliseconds requestHeadersTimeout() const override { + return parent_.requestHeadersTimeout(); + } + std::chrono::milliseconds delayedCloseTimeout() const override { + return parent_.delayedCloseTimeout(); + } + absl::optional maxStreamDuration() const override { + return parent_.maxStreamDuration(); + } + Router::RouteConfigProvider* routeConfigProvider() override { + return parent_.routeConfigProvider(); + } + Config::ConfigProvider* scopedRouteConfigProvider() override { + return parent_.scopedRouteConfigProvider(); + } + OptRef scopeKeyBuilder() override { + return parent_.scopeKeyBuilder(); + } + const std::string& serverName() const override { return parent_.serverName(); } + HttpConnectionManagerProto::ServerHeaderTransformation + serverHeaderTransformation() const override { + return parent_.serverHeaderTransformation(); + } + const absl::optional& schemeToSet() const override { return parent_.schemeToSet(); } + ConnectionManagerStats& stats() override { return parent_.stats(); } + ConnectionManagerTracingStats& tracingStats() override { return parent_.tracingStats(); } + bool useRemoteAddress() const override { return parent_.useRemoteAddress(); } + const InternalAddressConfig& internalAddressConfig() const override { + return parent_.internalAddressConfig(); + } + uint32_t xffNumTrustedHops() const override { return parent_.xffNumTrustedHops(); } + bool skipXffAppend() const override { return parent_.skipXffAppend(); } + const std::string& via() const override { return parent_.via(); } + ForwardClientCertType forwardClientCert() const override { return parent_.forwardClientCert(); } + const std::vector& setCurrentClientCertDetails() const override { + return parent_.setCurrentClientCertDetails(); + } + const Network::Address::Instance& localAddress() override { return parent_.localAddress(); } + const absl::optional& userAgent() override { return parent_.userAgent(); } + Tracing::TracerSharedPtr tracer() override { return parent_.tracer(); } + const TracingConnectionManagerConfig* tracingConfig() override { return parent_.tracingConfig(); } + ConnectionManagerListenerStats& listenerStats() override { return parent_.listenerStats(); } + bool proxy100Continue() const override { return parent_.proxy100Continue(); } + bool streamErrorOnInvalidHttpMessaging() const override { + return parent_.streamErrorOnInvalidHttpMessaging(); + } + const Http::Http1Settings& http1Settings() const override { return parent_.http1Settings(); } + bool shouldNormalizePath() const override { return parent_.shouldNormalizePath(); } + bool shouldMergeSlashes() const override { return parent_.shouldMergeSlashes(); } + StripPortType stripPortType() const override { return parent_.stripPortType(); } + envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction + headersWithUnderscoresAction() const override { + return parent_.headersWithUnderscoresAction(); + } + const LocalReply::LocalReply& localReply() const override { return parent_.localReply(); } + envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager:: + PathWithEscapedSlashesAction + pathWithEscapedSlashesAction() const override { + return parent_.pathWithEscapedSlashesAction(); + } + const std::vector& originalIpDetectionExtensions() const override { + return parent_.originalIpDetectionExtensions(); + } + const std::vector& earlyHeaderMutationExtensions() const override { + return parent_.earlyHeaderMutationExtensions(); + } + bool shouldStripTrailingHostDot() const override { return parent_.shouldStripTrailingHostDot(); } + uint64_t maxRequestsPerConnection() const override { return parent_.maxRequestsPerConnection(); } + const HttpConnectionManagerProto::ProxyStatusConfig* proxyStatusConfig() const override { + return parent_.proxyStatusConfig(); + } + ServerHeaderValidatorPtr makeHeaderValidator(Protocol protocol) override { + return parent_.makeHeaderValidator(protocol); + } + bool appendXForwardedPort() const override { return parent_.appendXForwardedPort(); } + bool appendLocalOverload() const override { return parent_.appendLocalOverload(); } + bool addProxyProtocolConnectionState() const override { + return parent_.addProxyProtocolConnectionState(); + } + +private: + ConnectionManagerConfig& parent_; +}; + HttpConnectionManagerImplMixin::HttpConnectionManagerImplMixin() : fake_stats_(*symbol_table_), http_context_(fake_stats_.symbolTable()), access_log_path_("dummy_path"), @@ -76,8 +207,9 @@ void HttpConnectionManagerImplMixin::setup(bool ssl, const std::string& server_n filter_callbacks_.connection_.stream_info_.downstream_connection_info_provider_->setSslConnection( ssl_connection_); conn_manager_ = std::make_unique( - *this, drain_close_, random_, http_context_, runtime_, local_info_, cluster_manager_, - overload_manager_, test_time_.timeSystem()); + std::make_shared(*this), drain_close_, random_, + http_context_, runtime_, local_info_, cluster_manager_, overload_manager_, + test_time_.timeSystem()); conn_manager_->initializeReadFilterCallbacks(filter_callbacks_); diff --git a/test/common/http/hcm_router_fuzz_test.cc b/test/common/http/hcm_router_fuzz_test.cc index 372e391d25a2..7e0628aa4434 100644 --- a/test/common/http/hcm_router_fuzz_test.cc +++ b/test/common/http/hcm_router_fuzz_test.cc @@ -498,7 +498,7 @@ class FuzzConfig : public HttpConnectionManagerImplMixin { class Harness { public: - Harness() : hcm_config_(Protobuf::RepeatedPtrField{}) { + Harness() : hcm_config_(std::make_shared(Protobuf::RepeatedPtrField{})) { ON_CALL(filter_callbacks_.connection_, close(_, _)).WillByDefault(InvokeWithoutArgs([this]() { closed_ = true; })); @@ -506,12 +506,12 @@ class Harness { void fuzz(const FuzzCase& input) { hcm_ = std::make_unique( - hcm_config_, drain_close_, random_, hcm_config_.http_context_, runtime_, local_info_, - hcm_config_.cm_, overload_manager_, hcm_config_.time_system_); + hcm_config_, drain_close_, random_, hcm_config_->http_context_, runtime_, local_info_, + hcm_config_->cm_, overload_manager_, hcm_config_->time_system_); hcm_->initializeReadFilterCallbacks(filter_callbacks_); Buffer::OwnedImpl data; hcm_->onData(data, false); - FuzzClusterManager& cluster_manager = hcm_config_.getFuzzClusterManager(); + FuzzClusterManager& cluster_manager = hcm_config_->getFuzzClusterManager(); for (const auto& action : input.actions()) { if (closed_) { @@ -520,7 +520,7 @@ class Harness { switch (action.action_case()) { case ActionCase::kAdvanceTime: { const auto& a = action.advance_time(); - hcm_config_.time_system_.timeSystem().advanceTimeWait( + hcm_config_->time_system_.timeSystem().advanceTimeWait( std::chrono::milliseconds(a.milliseconds())); break; } @@ -582,7 +582,7 @@ class Harness { } private: - FuzzConfig hcm_config_; + std::shared_ptr hcm_config_; NiceMock drain_close_; NiceMock random_; NiceMock runtime_;