diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index ef773abe13bb..50a7d122c7d9 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -53,6 +53,12 @@ Minor Behavior Changes * stats: the fake symbol table implemention has been removed from the binary, and the option `--use-fake-symbol-table` is now a no-op with a warning. * thrift_proxy: special characters {'\0', '\r', '\n'} will be stripped from thrift headers. * watchdog: replaced single watchdog with separate watchdog configuration for worker threads and for the main thread configured via :ref:`Watchdogs`. It works with :ref:`watchdog` by having the worker thread and main thread watchdogs have same config. +* build: the Alpine based debug images are no longer built in CI, use Ubuntu based images instead. +* cluster manager: the cluster which can't extract secret entity by SDS to be warming and never activate. This feature is disabled by default and is controlled by runtime guard `envoy.reloadable_features.cluster_keep_warming_no_secret_entity`. +* ext_authz filter: disable `envoy.reloadable_features.ext_authz_measure_timeout_on_check_created` by default. +* ext_authz filter: the deprecated field :ref:`use_alpha ` is no longer supported and cannot be set anymore. +* grpc_web filter: if a `grpc-accept-encoding` header is present it's passed as-is to the upstream and if it isn't `grpc-accept-encoding:identity` is sent instead. The header was always overwriten with `grpc-accept-encoding:identity,deflate,gzip` before. +* watchdog: the watchdog action :ref:`abort_action ` is now the default action to terminate the process if watchdog kill / multikill is enabled. Bug Fixes --------- diff --git a/include/envoy/network/transport_socket.h b/include/envoy/network/transport_socket.h index fe054ce2f16d..63169d3624e7 100644 --- a/include/envoy/network/transport_socket.h +++ b/include/envoy/network/transport_socket.h @@ -226,6 +226,11 @@ class TransportSocketFactory { */ virtual TransportSocketPtr createTransportSocket(TransportSocketOptionsSharedPtr options) const PURE; + + /** + * Check whether matched transport socket which required to use secret information is available. + */ + virtual bool isReady() const PURE; }; using TransportSocketFactoryPtr = std::unique_ptr; diff --git a/include/envoy/ssl/context_config.h b/include/envoy/ssl/context_config.h index 675bddde27de..91bdddf57a36 100644 --- a/include/envoy/ssl/context_config.h +++ b/include/envoy/ssl/context_config.h @@ -111,6 +111,11 @@ class ClientContextConfig : public virtual ContextConfig { * for names. */ virtual const std::string& signingAlgorithmsForTest() const PURE; + + /** + * Check whether TLS certificate entity and certificate validation context entity is available + */ + virtual bool isSecretReady() const PURE; }; using ClientContextConfigPtr = std::unique_ptr; diff --git a/source/common/network/raw_buffer_socket.cc b/source/common/network/raw_buffer_socket.cc index c539add2f71a..69f9d7095313 100644 --- a/source/common/network/raw_buffer_socket.cc +++ b/source/common/network/raw_buffer_socket.cc @@ -93,5 +93,7 @@ RawBufferSocketFactory::createTransportSocket(TransportSocketOptionsSharedPtr) c } bool RawBufferSocketFactory::implementsSecureTransport() const { return false; } + +bool RawBufferSocketFactory::isReady() const { return true; } } // namespace Network } // namespace Envoy diff --git a/source/common/network/raw_buffer_socket.h b/source/common/network/raw_buffer_socket.h index fe87bbeda605..9fb37b31613d 100644 --- a/source/common/network/raw_buffer_socket.h +++ b/source/common/network/raw_buffer_socket.h @@ -32,6 +32,7 @@ class RawBufferSocketFactory : public TransportSocketFactory { // Network::TransportSocketFactory TransportSocketPtr createTransportSocket(TransportSocketOptionsSharedPtr options) const override; bool implementsSecureTransport() const override; + bool isReady() const override; }; } // namespace Network diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 56a7eb8189cb..b8a495f0a827 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -111,6 +111,8 @@ constexpr const char* disabled_runtime_features[] = { "envoy.reloadable_features.new_tcp_connection_pool", // Sentinel and test flag. "envoy.reloadable_features.test_feature_false", + // The cluster which can't extract secret entity by SDS to be warming and never activate. + "envoy.reloadable_features.cluster_keep_warming_no_secret_entity", }; RuntimeFeatures::RuntimeFeatures() { diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 80a9ef9ac070..924c0a6feb07 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -422,6 +422,21 @@ void ClusterManagerImpl::onClusterInit(Cluster& cluster) { // We have a situation that clusters will be immediately active, such as static and primary // cluster. So we must have this prevention logic here. if (cluster_data != warming_clusters_.end()) { + Network::TransportSocketFactory& factory = + cluster.info()->transportSocketMatcher().resolve(&cluster.info()->metadata()).factory_; + // If there is no secret entity, currently supports only TLS Certificate and Validation + // Context, when it failed to extract them via SDS, it will fail to change cluster status from + // warming to active. In current implementation, there is no strategy to activate clusters + // which failed to initialize at once. + // TODO(shikugawa): To implement to be available by keeping warming after no-available secret + // entity behavior occurred. And remove + // `envoy.reloadable_features.cluster_keep_warming_no_secret_entity` runtime feature flag. + const bool keep_warming_enabled = Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.cluster_keep_warming_no_secret_entity"); + if (!factory.isReady() && keep_warming_enabled) { + ENVOY_LOG(warn, "Failed to activate {}", cluster.info()->name()); + return; + } clusterWarmingToActive(cluster.info()->name()); updateClusterCounts(); } diff --git a/source/extensions/quic_listeners/quiche/quic_transport_socket_factory.h b/source/extensions/quic_listeners/quiche/quic_transport_socket_factory.h index 2ada9e2de17b..b974a36725d1 100644 --- a/source/extensions/quic_listeners/quiche/quic_transport_socket_factory.h +++ b/source/extensions/quic_listeners/quiche/quic_transport_socket_factory.h @@ -24,6 +24,7 @@ class QuicTransportSocketFactoryBase : public Network::TransportSocketFactory { NOT_REACHED_GCOVR_EXCL_LINE; } bool implementsSecureTransport() const override { return true; } + bool isReady() const override { return true; } }; // TODO(danzh): when implement ProofSource, examine of it's necessary to diff --git a/source/extensions/transport_sockets/alts/tsi_socket.cc b/source/extensions/transport_sockets/alts/tsi_socket.cc index 0fe5b752ceca..7ba6b2cab798 100644 --- a/source/extensions/transport_sockets/alts/tsi_socket.cc +++ b/source/extensions/transport_sockets/alts/tsi_socket.cc @@ -261,6 +261,7 @@ TsiSocketFactory::createTransportSocket(Network::TransportSocketOptionsSharedPtr return std::make_unique(handshaker_factory_, handshake_validator_); } +bool TsiSocketFactory::isReady() const { return true; } } // namespace Alts } // namespace TransportSockets } // namespace Extensions diff --git a/source/extensions/transport_sockets/alts/tsi_socket.h b/source/extensions/transport_sockets/alts/tsi_socket.h index 0acba405022d..529e316c95ff 100644 --- a/source/extensions/transport_sockets/alts/tsi_socket.h +++ b/source/extensions/transport_sockets/alts/tsi_socket.h @@ -100,6 +100,7 @@ class TsiSocketFactory : public Network::TransportSocketFactory { bool implementsSecureTransport() const override; Network::TransportSocketPtr createTransportSocket(Network::TransportSocketOptionsSharedPtr options) const override; + bool isReady() const override; private: HandshakerFactory handshaker_factory_; diff --git a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc index 3d4f716421e7..ac2716c96d72 100644 --- a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc @@ -123,7 +123,11 @@ bool UpstreamProxyProtocolSocketFactory::implementsSecureTransport() const { return transport_socket_factory_->implementsSecureTransport(); } +bool UpstreamProxyProtocolSocketFactory::isReady() const { + return transport_socket_factory_->isReady(); +} + } // namespace ProxyProtocol } // namespace TransportSockets } // namespace Extensions -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.h b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.h index 4a191ebf539d..cc7fcee7e79a 100644 --- a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.h +++ b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.h @@ -49,6 +49,7 @@ class UpstreamProxyProtocolSocketFactory : public Network::TransportSocketFactor Network::TransportSocketPtr createTransportSocket(Network::TransportSocketOptionsSharedPtr options) const override; bool implementsSecureTransport() const override; + bool isReady() const override; private: Network::TransportSocketFactoryPtr transport_socket_factory_; diff --git a/source/extensions/transport_sockets/tap/tap.cc b/source/extensions/transport_sockets/tap/tap.cc index 7674ba6b584d..5b8d83fc8186 100644 --- a/source/extensions/transport_sockets/tap/tap.cc +++ b/source/extensions/transport_sockets/tap/tap.cc @@ -66,6 +66,8 @@ bool TapSocketFactory::implementsSecureTransport() const { return transport_socket_factory_->implementsSecureTransport(); } +bool TapSocketFactory::isReady() const { return transport_socket_factory_->isReady(); } + } // namespace Tap } // namespace TransportSockets } // namespace Extensions diff --git a/source/extensions/transport_sockets/tap/tap.h b/source/extensions/transport_sockets/tap/tap.h index 33156b705153..c2d91e1f571a 100644 --- a/source/extensions/transport_sockets/tap/tap.h +++ b/source/extensions/transport_sockets/tap/tap.h @@ -41,6 +41,7 @@ class TapSocketFactory : public Network::TransportSocketFactory, Network::TransportSocketPtr createTransportSocket(Network::TransportSocketOptionsSharedPtr options) const override; bool implementsSecureTransport() const override; + bool isReady() const override; private: Network::TransportSocketFactoryPtr transport_socket_factory_; diff --git a/source/extensions/transport_sockets/tls/context_config_impl.cc b/source/extensions/transport_sockets/tls/context_config_impl.cc index 546489232e74..7e6a59d85919 100644 --- a/source/extensions/transport_sockets/tls/context_config_impl.cc +++ b/source/extensions/transport_sockets/tls/context_config_impl.cc @@ -169,14 +169,14 @@ ContextConfigImpl::ContextConfigImpl( const std::string& default_cipher_suites, const std::string& default_curves, Server::Configuration::TransportSocketFactoryContext& factory_context) : api_(factory_context.api()), + tls_certificate_providers_(getTlsCertificateConfigProviders(config, factory_context)), + certificate_validation_context_provider_( + getCertificateValidationContextConfigProvider(config, factory_context, &default_cvc_)), alpn_protocols_(RepeatedPtrUtil::join(config.alpn_protocols(), ",")), cipher_suites_(StringUtil::nonEmptyStringOrDefault( RepeatedPtrUtil::join(config.tls_params().cipher_suites(), ":"), default_cipher_suites)), ecdh_curves_(StringUtil::nonEmptyStringOrDefault( RepeatedPtrUtil::join(config.tls_params().ecdh_curves(), ":"), default_curves)), - tls_certificate_providers_(getTlsCertificateConfigProviders(config, factory_context)), - certificate_validation_context_provider_( - getCertificateValidationContextConfigProvider(config, factory_context, &default_cvc_)), min_protocol_version_(tlsVersionFromProto(config.tls_params().tls_minimum_protocol_version(), default_min_protocol_version)), max_protocol_version_(tlsVersionFromProto(config.tls_params().tls_maximum_protocol_version(), @@ -375,6 +375,15 @@ ClientContextConfigImpl::ClientContextConfigImpl( } } +bool ClientContextConfigImpl::isSecretReady() const { + for (const auto& provider : tls_certificate_providers_) { + if (provider->secret() == nullptr) { + return false; + } + } + return certificate_validation_context_provider_->secret() != nullptr; +} + const unsigned ServerContextConfigImpl::DEFAULT_MIN_VERSION = TLS1_VERSION; const unsigned ServerContextConfigImpl::DEFAULT_MAX_VERSION = TLS1_3_VERSION; diff --git a/source/extensions/transport_sockets/tls/context_config_impl.h b/source/extensions/transport_sockets/tls/context_config_impl.h index 44c5a8cc619d..d40bb627a51d 100644 --- a/source/extensions/transport_sockets/tls/context_config_impl.h +++ b/source/extensions/transport_sockets/tls/context_config_impl.h @@ -69,6 +69,14 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig { const std::string& default_cipher_suites, const std::string& default_curves, Server::Configuration::TransportSocketFactoryContext& factory_context); Api::Api& api_; + // If certificate validation context type is combined_validation_context. default_cvc_ + // holds a copy of CombinedCertificateValidationContext::default_validation_context. + // Otherwise, default_cvc_ is nullptr. + std::unique_ptr + default_cvc_; + std::vector tls_certificate_providers_; + Secret::CertificateValidationContextConfigProviderSharedPtr + certificate_validation_context_provider_; private: static unsigned tlsVersionFromProto( @@ -81,16 +89,8 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig { std::vector tls_certificate_configs_; Ssl::CertificateValidationContextConfigPtr validation_context_config_; - // If certificate validation context type is combined_validation_context. default_cvc_ - // holds a copy of CombinedCertificateValidationContext::default_validation_context. - // Otherwise, default_cvc_ is nullptr. - std::unique_ptr - default_cvc_; - std::vector tls_certificate_providers_; // Handle for TLS certificate dynamic secret callback. Envoy::Common::CallbackHandle* tc_update_callback_handle_{}; - Secret::CertificateValidationContextConfigProviderSharedPtr - certificate_validation_context_provider_; // Handle for certificate validation context dynamic secret callback. Envoy::Common::CallbackHandle* cvc_update_callback_handle_{}; Envoy::Common::CallbackHandle* cvc_validation_callback_handle_{}; @@ -120,6 +120,7 @@ class ClientContextConfigImpl : public ContextConfigImpl, public Envoy::Ssl::Cli bool allowRenegotiation() const override { return allow_renegotiation_; } size_t maxSessionKeys() const override { return max_session_keys_; } const std::string& signingAlgorithmsForTest() const override { return sigalgs_; } + bool isSecretReady() const override; private: static const unsigned DEFAULT_MIN_VERSION; diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index 485468443096..0bc6dcc400c8 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -362,6 +362,8 @@ void ClientSslSocketFactory::onAddOrUpdateSecret() { stats_.ssl_context_update_by_sds_.inc(); } +bool ClientSslSocketFactory::isReady() const { return config_->isSecretReady(); } + ServerSslSocketFactory::ServerSslSocketFactory(Envoy::Ssl::ServerContextConfigPtr config, Envoy::Ssl::ContextManager& manager, Stats::Scope& stats_scope, @@ -403,6 +405,8 @@ void ServerSslSocketFactory::onAddOrUpdateSecret() { stats_.ssl_context_update_by_sds_.inc(); } +bool ServerSslSocketFactory::isReady() const { return true; } + } // namespace Tls } // namespace TransportSockets } // namespace Extensions diff --git a/source/extensions/transport_sockets/tls/ssl_socket.h b/source/extensions/transport_sockets/tls/ssl_socket.h index c14cb502bed1..82834b133d60 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.h +++ b/source/extensions/transport_sockets/tls/ssl_socket.h @@ -105,10 +105,11 @@ class ClientSslSocketFactory : public Network::TransportSocketFactory, ClientSslSocketFactory(Envoy::Ssl::ClientContextConfigPtr config, Envoy::Ssl::ContextManager& manager, Stats::Scope& stats_scope); + // Network::TransportSocketFactory Network::TransportSocketPtr createTransportSocket(Network::TransportSocketOptionsSharedPtr options) const override; bool implementsSecureTransport() const override; - + bool isReady() const override; // Secret::SecretCallbacks void onAddOrUpdateSecret() override; @@ -132,7 +133,7 @@ class ServerSslSocketFactory : public Network::TransportSocketFactory, Network::TransportSocketPtr createTransportSocket(Network::TransportSocketOptionsSharedPtr options) const override; bool implementsSecureTransport() const override; - + bool isReady() const override; // Secret::SecretCallbacks void onAddOrUpdateSecret() override; diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index b6b19139a7b5..0c9ca2ed6a17 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -46,10 +46,12 @@ envoy_cc_test( "//test/mocks/upstream:health_checker_mocks", "//test/mocks/upstream:load_balancer_context_mock", "//test/mocks/upstream:thread_aware_load_balancer_mocks", + "//test/test_common:test_runtime_lib", "@envoy_api//envoy/admin/v3:pkg_cc_proto", "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", ], ) diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 34c31a7f4f03..34d8403cf5d0 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -3,6 +3,7 @@ #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/cluster/v3/cluster.pb.validate.h" #include "envoy/config/core/v3/base.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/secret.pb.h" #include "test/common/upstream/test_cluster_manager.h" #include "test/mocks/upstream/cds_api.h" @@ -12,6 +13,7 @@ #include "test/mocks/upstream/health_checker.h" #include "test/mocks/upstream/load_balancer_context.h" #include "test/mocks/upstream/thread_aware_load_balancer.h" +#include "test/test_common/test_runtime.h" namespace Envoy { namespace Upstream { @@ -2306,6 +2308,154 @@ TEST_F(ClusterManagerImplTest, DynamicHostRemoveDefaultPriority) { factory_.tls_.shutdownThread(); } +TEST_F(ClusterManagerImplTest, + DynamicAddedAndKeepWarmingWithoutCertificateValidationContextEntity) { + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.reloadable_features.cluster_keep_warming_no_secret_entity", "true"}}); + create(defaultConfig()); + + ReadyWatcher initialized; + EXPECT_CALL(initialized, ready()); + cluster_manager_->setInitializedCb([&]() -> void { initialized.ready(); }); + + std::shared_ptr cluster1(new NiceMock()); + cluster1->info_->name_ = "fake_cluster"; + + auto transport_socket_factory = std::make_unique(); + EXPECT_CALL(*transport_socket_factory, isReady()).WillOnce(Return(false)); + + auto transport_socket_matcher = std::make_unique>( + std::move(transport_socket_factory)); + cluster1->info_->transport_socket_matcher_ = std::move(transport_socket_matcher); + + EXPECT_CALL(factory_, clusterFromProto_(_, _, _, _)) + .WillOnce(Return(std::make_pair(cluster1, nullptr))); + EXPECT_CALL(*cluster1, initializePhase()).Times(0); + EXPECT_CALL(*cluster1, initialize(_)); + EXPECT_TRUE(cluster_manager_->addOrUpdateCluster(defaultStaticCluster("fake_cluster"), "")); + checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 0 /*active*/, 1 /*warming*/); + EXPECT_EQ(1, cluster_manager_->warmingClusterCount()); + EXPECT_EQ(nullptr, cluster_manager_->get("fake_cluster")); + cluster1->initialize_callback_(); + + // Check to be keep warming fake_cluster after callback invoked. + EXPECT_EQ(cluster1->info_, cluster_manager_->get("fake_cluster")->info()); + checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 0 /*active*/, 1 /*warming*/); + EXPECT_EQ(1, cluster_manager_->warmingClusterCount()); + + EXPECT_TRUE(Mock::VerifyAndClearExpectations(cluster1.get())); +} + +TEST_F(ClusterManagerImplTest, + DynamicAddedAndKeepWarmingDisabledWithoutCertificateValidationContextEntity) { + create(defaultConfig()); + + ReadyWatcher initialized; + EXPECT_CALL(initialized, ready()); + cluster_manager_->setInitializedCb([&]() -> void { initialized.ready(); }); + + std::shared_ptr cluster1(new NiceMock()); + cluster1->info_->name_ = "fake_cluster"; + + auto transport_socket_factory = std::make_unique(); + EXPECT_CALL(*transport_socket_factory, isReady()).WillOnce(Return(false)); + + auto transport_socket_matcher = std::make_unique>( + std::move(transport_socket_factory)); + cluster1->info_->transport_socket_matcher_ = std::move(transport_socket_matcher); + + EXPECT_CALL(factory_, clusterFromProto_(_, _, _, _)) + .WillOnce(Return(std::make_pair(cluster1, nullptr))); + EXPECT_CALL(*cluster1, initializePhase()).Times(0); + EXPECT_CALL(*cluster1, initialize(_)); + EXPECT_TRUE(cluster_manager_->addOrUpdateCluster(defaultStaticCluster("fake_cluster"), "")); + checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 0 /*active*/, 1 /*warming*/); + EXPECT_EQ(1, cluster_manager_->warmingClusterCount()); + EXPECT_EQ(nullptr, cluster_manager_->get("fake_cluster")); + cluster1->initialize_callback_(); + + // Check to be keep warming fake_cluster after callback invoked. + EXPECT_EQ(cluster1->info_, cluster_manager_->get("fake_cluster")->info()); + checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 1 /*active*/, 0 /*warming*/); + EXPECT_EQ(0, cluster_manager_->warmingClusterCount()); + + EXPECT_TRUE(Mock::VerifyAndClearExpectations(cluster1.get())); +} + +TEST_F(ClusterManagerImplTest, DynamicAddedAndKeepWarmingWithoutTlsCertificateEntity) { + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.reloadable_features.cluster_keep_warming_no_secret_entity", "true"}}); + create(defaultConfig()); + + ReadyWatcher initialized; + EXPECT_CALL(initialized, ready()); + cluster_manager_->setInitializedCb([&]() -> void { initialized.ready(); }); + + std::shared_ptr cluster1(new NiceMock()); + cluster1->info_->name_ = "fake_cluster"; + + auto transport_socket_factory = std::make_unique(); + EXPECT_CALL(*transport_socket_factory, isReady()).WillOnce(Return(false)); + + auto transport_socket_matcher = std::make_unique>( + std::move(transport_socket_factory)); + cluster1->info_->transport_socket_matcher_ = std::move(transport_socket_matcher); + + EXPECT_CALL(factory_, clusterFromProto_(_, _, _, _)) + .WillOnce(Return(std::make_pair(cluster1, nullptr))); + EXPECT_CALL(*cluster1, initializePhase()).Times(0); + EXPECT_CALL(*cluster1, initialize(_)); + EXPECT_TRUE(cluster_manager_->addOrUpdateCluster(defaultStaticCluster("fake_cluster"), "")); + checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 0 /*active*/, 1 /*warming*/); + EXPECT_EQ(1, cluster_manager_->warmingClusterCount()); + EXPECT_EQ(nullptr, cluster_manager_->get("fake_cluster")); + cluster1->initialize_callback_(); + + // Check to be keep warming fake_cluster after callback invoked. + EXPECT_EQ(cluster1->info_, cluster_manager_->get("fake_cluster")->info()); + checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 0 /*active*/, 1 /*warming*/); + EXPECT_EQ(1, cluster_manager_->warmingClusterCount()); + + EXPECT_TRUE(Mock::VerifyAndClearExpectations(cluster1.get())); +} + +TEST_F(ClusterManagerImplTest, DynamicAddedAndKeepWarmingDisabledWithoutTlsCertificateEntity) { + create(defaultConfig()); + + ReadyWatcher initialized; + EXPECT_CALL(initialized, ready()); + cluster_manager_->setInitializedCb([&]() -> void { initialized.ready(); }); + + std::shared_ptr cluster1(new NiceMock()); + cluster1->info_->name_ = "fake_cluster"; + + auto transport_socket_factory = std::make_unique(); + EXPECT_CALL(*transport_socket_factory, isReady()).WillOnce(Return(false)); + + auto transport_socket_matcher = std::make_unique>( + std::move(transport_socket_factory)); + cluster1->info_->transport_socket_matcher_ = std::move(transport_socket_matcher); + + EXPECT_CALL(factory_, clusterFromProto_(_, _, _, _)) + .WillOnce(Return(std::make_pair(cluster1, nullptr))); + EXPECT_CALL(*cluster1, initializePhase()).Times(0); + EXPECT_CALL(*cluster1, initialize(_)); + EXPECT_TRUE(cluster_manager_->addOrUpdateCluster(defaultStaticCluster("fake_cluster"), "")); + checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 0 /*active*/, 1 /*warming*/); + EXPECT_EQ(1, cluster_manager_->warmingClusterCount()); + EXPECT_EQ(nullptr, cluster_manager_->get("fake_cluster")); + cluster1->initialize_callback_(); + + // Check to be keep warming fake_cluster after callback invoked. + EXPECT_EQ(cluster1->info_, cluster_manager_->get("fake_cluster")->info()); + checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 1 /*active*/, 0 /*warming*/); + EXPECT_EQ(0, cluster_manager_->warmingClusterCount()); + + EXPECT_TRUE(Mock::VerifyAndClearExpectations(cluster1.get())); +} + class MockConnPoolWithDestroy : public Http::ConnectionPool::MockInstance { public: ~MockConnPoolWithDestroy() override { onDestroy(); } diff --git a/test/common/upstream/transport_socket_matcher_test.cc b/test/common/upstream/transport_socket_matcher_test.cc index cfde130d1d1f..61e5ab2cec43 100644 --- a/test/common/upstream/transport_socket_matcher_test.cc +++ b/test/common/upstream/transport_socket_matcher_test.cc @@ -33,6 +33,7 @@ class FakeTransportSocketFactory : public Network::TransportSocketFactory { MOCK_METHOD(bool, implementsSecureTransport, (), (const)); MOCK_METHOD(Network::TransportSocketPtr, createTransportSocket, (Network::TransportSocketOptionsSharedPtr), (const)); + MOCK_METHOD(bool, isReady, (), (const)); FakeTransportSocketFactory(std::string id) : id_(std::move(id)) {} std::string id() const { return id_; } @@ -48,6 +49,7 @@ class FooTransportSocketFactory MOCK_METHOD(bool, implementsSecureTransport, (), (const)); MOCK_METHOD(Network::TransportSocketPtr, createTransportSocket, (Network::TransportSocketOptionsSharedPtr), (const)); + MOCK_METHOD(bool, isReady, (), (const)); Network::TransportSocketFactoryPtr createTransportSocketFactory(const Protobuf::Message& proto, diff --git a/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc b/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc index 953e5999a5fb..32a2281cf0af 100644 --- a/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc @@ -469,6 +469,13 @@ TEST_F(ProxyProtocolSocketFactoryTest, ImplementsSecureTransportCallInnerFactory ASSERT_TRUE(factory_->implementsSecureTransport()); } +// Test isReady calls inner factory +TEST_F(ProxyProtocolSocketFactoryTest, IsReadyCallInnerFactory) { + initialize(); + EXPECT_CALL(*inner_factory_, isReady()).WillOnce(Return(true)); + ASSERT_TRUE(factory_->isReady()); +} + } // namespace } // namespace ProxyProtocol } // namespace TransportSockets diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index 0307ebb2daef..3b361d88beac 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -1604,6 +1604,62 @@ TEST_F(ClientContextConfigImplTest, MissingStaticCertificateValidationContext) { "Unknown static certificate validation context: missing"); } +TEST_F(ClientContextConfigImplTest, ValidationContextEntityNotExist) { + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; + auto* validation_context_sds_secret_config = + tls_context.mutable_common_tls_context()->mutable_validation_context_sds_secret_config(); + validation_context_sds_secret_config->set_name("sds_validation_context"); + auto* config_source = validation_context_sds_secret_config->mutable_sds_config(); + auto* api_config_source = config_source->mutable_api_config_source(); + api_config_source->set_api_type(envoy::config::core::v3::ApiConfigSource::GRPC); + + NiceMock local_info; + Stats::IsolatedStoreImpl stats; + NiceMock init_manager; + NiceMock dispatcher; + EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); + EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); + EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(ReturnRef(init_manager)); + EXPECT_CALL(factory_context_, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); + + ClientContextConfigImpl client_context_config(tls_context, factory_context_); + EXPECT_FALSE(client_context_config.isSecretReady()); + + NiceMock secret_callback; + client_context_config.setSecretUpdateCallback( + [&secret_callback]() { secret_callback.onAddOrUpdateSecret(); }); + client_context_config.setSecretUpdateCallback([]() {}); +} + +TEST_F(ClientContextConfigImplTest, TlsCertificateEntityNotExist) { + envoy::extensions::transport_sockets::tls::v3::SdsSecretConfig secret_config; + secret_config.set_name("sds_tls_certificate"); + auto* config_source = secret_config.mutable_sds_config(); + auto* api_config_source = config_source->mutable_api_config_source(); + api_config_source->set_api_type(envoy::config::core::v3::ApiConfigSource::GRPC); + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; + auto tls_certificate_sds_secret_configs = + tls_context.mutable_common_tls_context()->mutable_tls_certificate_sds_secret_configs(); + *tls_certificate_sds_secret_configs->Add() = secret_config; + + NiceMock local_info; + Stats::IsolatedStoreImpl stats; + NiceMock init_manager; + NiceMock dispatcher; + EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); + EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); + EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(ReturnRef(init_manager)); + EXPECT_CALL(factory_context_, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); + + ClientContextConfigImpl client_context_config(tls_context, factory_context_); + EXPECT_FALSE(client_context_config.isSecretReady()); + + NiceMock secret_callback; + client_context_config.setSecretUpdateCallback( + [&secret_callback]() { secret_callback.onAddOrUpdateSecret(); }); + client_context_config.setSecretUpdateCallback([]() {}); +} + class ServerContextConfigImplTest : public SslCertsTest {}; // Multiple TLS certificates are supported. diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index b4bdb84e5737..c0874d16019f 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -4524,6 +4524,7 @@ TEST_P(SslSocketTest, UpstreamNotReadySslSocket) { EXPECT_FALSE(client_cfg->isReady()); ContextManagerImpl manager(time_system_); + ClientSslSocketFactory client_ssl_socket_factory(std::move(client_cfg), manager, stats_store); auto transport_socket = client_ssl_socket_factory.createTransportSocket(nullptr); EXPECT_EQ(EMPTY_STRING, transport_socket->protocol()); @@ -5729,6 +5730,15 @@ TEST_P(SslSocketTest, TestConnectionFailsOnMultipleCertificatesNonePassOcspPolic testUtil(test_options.setExpectedServerStats("ssl.ocsp_staple_failed").enableOcspStapling()); } +TEST_P(SslSocketTest, ClientSocketFactoryIsReadyTest) { + ContextManagerImpl manager(time_system_); + Stats::TestUtil::TestStore stats_store; + auto client_cfg = std::make_unique>(); + EXPECT_CALL(*client_cfg, isSecretReady()).WillOnce(Return(true)); + ClientSslSocketFactory client_ssl_socket_factory(std::move(client_cfg), manager, stats_store); + EXPECT_TRUE(client_ssl_socket_factory.isReady()); +} + } // namespace Tls } // namespace TransportSockets } // namespace Extensions diff --git a/test/integration/xfcc_integration_test.cc b/test/integration/xfcc_integration_test.cc index 3b299de13c51..7fa5dee96696 100644 --- a/test/integration/xfcc_integration_test.cc +++ b/test/integration/xfcc_integration_test.cc @@ -46,7 +46,7 @@ Network::TransportSocketFactoryPtr XfccIntegrationTest::createClientSslContext(b validation_context: trusted_ca: filename: {{ test_rundir }}/test/config/integration/certs/cacert.pem - match_subject_alt_names: + match_subject_alt_names: exact: "spiffe://lyft.com/backend-team" exact: "lyft.com" exact: "www.lyft.com" @@ -57,7 +57,7 @@ Network::TransportSocketFactoryPtr XfccIntegrationTest::createClientSslContext(b validation_context: trusted_ca: filename: {{ test_rundir }}/test/config/integration/certs/cacert.pem - match_subject_alt_names: + match_subject_alt_names: exact: "spiffe://lyft.com/backend-team" exact: "lyft.com" exact: "www.lyft.com" diff --git a/test/mocks/network/transport_socket.h b/test/mocks/network/transport_socket.h index ee53570c20ac..164e393c23ff 100644 --- a/test/mocks/network/transport_socket.h +++ b/test/mocks/network/transport_socket.h @@ -38,6 +38,7 @@ class MockTransportSocketFactory : public TransportSocketFactory { MOCK_METHOD(bool, implementsSecureTransport, (), (const)); MOCK_METHOD(TransportSocketPtr, createTransportSocket, (TransportSocketOptionsSharedPtr), (const)); + MOCK_METHOD(bool, isReady, (), (const)); }; } // namespace Network diff --git a/test/mocks/ssl/mocks.h b/test/mocks/ssl/mocks.h index 6a5cbe8df649..31da44badabd 100644 --- a/test/mocks/ssl/mocks.h +++ b/test/mocks/ssl/mocks.h @@ -95,6 +95,7 @@ class MockClientContextConfig : public ClientContextConfig { MOCK_METHOD(bool, allowRenegotiation, (), (const)); MOCK_METHOD(size_t, maxSessionKeys, (), (const)); MOCK_METHOD(const std::string&, signingAlgorithmsForTest, (), (const)); + MOCK_METHOD(bool, isSecretReady, (), (const)); }; class MockServerContextConfig : public ServerContextConfig {