diff --git a/include/envoy/network/transport_socket.h b/include/envoy/network/transport_socket.h index 4244e62477670..a008831a0d23d 100644 --- a/include/envoy/network/transport_socket.h +++ b/include/envoy/network/transport_socket.h @@ -231,6 +231,11 @@ class TransportSocketFactory { createTransportSocket(TransportSocketOptionsSharedPtr options) const PURE; /** + * Check whether matched transport socket which required to use secret information is available. + */ + virtual bool isReady() const PURE; + + /* * @return bool whether the transport socket will use proxy protocol options. */ virtual bool usesProxyProtocolOptions() const PURE; diff --git a/include/envoy/ssl/context_config.h b/include/envoy/ssl/context_config.h index 675bddde27def..91bdddf57a366 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/include/envoy/upstream/host_description.h b/include/envoy/upstream/host_description.h index bd0714bae455f..d8f6a4a50a37e 100644 --- a/include/envoy/upstream/host_description.h +++ b/include/envoy/upstream/host_description.h @@ -182,6 +182,11 @@ class TransportSocketMatcher { * @return the match information of the transport socket selected. */ virtual MatchData resolve(const envoy::config::core::v3::Metadata* metadata) const PURE; + + /** + * Check if all transport socket factories are ready. + */ + virtual bool factoriesReady() const PURE; }; using TransportSocketMatcherPtr = std::unique_ptr; diff --git a/source/common/network/raw_buffer_socket.cc b/source/common/network/raw_buffer_socket.cc index c539add2f71ad..69f9d70953135 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 9a17fe35516b4..70ea55ad5c7db 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; bool usesProxyProtocolOptions() const override { return false; } }; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 64ce93c8be1c9..9bf4fa0546201 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -112,7 +112,8 @@ constexpr const char* disabled_runtime_features[] = { "envoy.reloadable_features.test_feature_false", // gRPC Timeout header is missing (#13580) "envoy.reloadable_features.ext_authz_measure_timeout_on_check_created", - + // When a cluster can't extract secret entity by SDS, keep it 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 6eaef38a901b2..7618a6581d191 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -431,6 +431,19 @@ 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()) { + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.cluster_keep_warming_no_secret_entity") && + !cluster.info()->transportSocketMatcher().factoriesReady()) { + // If `envoy.reloadable_features.cluster_keep_warming_no_secret_entity` is enabled, + // when a cluster depends on a SDS secret but the secret entity is not ready, instead of + // marking it active immediately, keep it warming until the next CDS update. This means + // Envoy will not advertise itself in ready state so it won't get traffic in deployments + // with readiness probes that checks the state. + // TODO(lizan): #13777/#13952 In long term we want to fix this behavior with init manager + // to keep clusters in warming state until Envoy get SDS response. + ENVOY_LOG(warn, "Failed to activate {} due to no secret entity", cluster.info()->name()); + return; + } clusterWarmingToActive(cluster.info()->name()); updateClusterCounts(); } diff --git a/source/common/upstream/transport_socket_match_impl.cc b/source/common/upstream/transport_socket_match_impl.cc index 7d425dd4a53c7..596fa3d69f397 100644 --- a/source/common/upstream/transport_socket_match_impl.cc +++ b/source/common/upstream/transport_socket_match_impl.cc @@ -48,5 +48,14 @@ TransportSocketMatcherImpl::resolve(const envoy::config::core::v3::Metadata* met return MatchData(*default_match_.factory, default_match_.stats, default_match_.name); } +bool TransportSocketMatcherImpl::factoriesReady() const { + for (const auto& match : matches_) { + if (!match.factory->isReady()) { + return false; + } + } + return default_match_.factory->isReady(); +} + } // namespace Upstream } // namespace Envoy diff --git a/source/common/upstream/transport_socket_match_impl.h b/source/common/upstream/transport_socket_match_impl.h index f3fb2ab173e40..12cafb2a4fe50 100644 --- a/source/common/upstream/transport_socket_match_impl.h +++ b/source/common/upstream/transport_socket_match_impl.h @@ -39,6 +39,7 @@ class TransportSocketMatcherImpl : public Logger::Loggable Network::TransportSocketFactoryPtr& default_factory, Stats::Scope& stats_scope); MatchData resolve(const envoy::config::core::v3::Metadata* metadata) const override; + bool factoriesReady() const override; protected: TransportSocketMatchStats generateStats(const std::string& prefix); 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 142489b39d9b5..33607151bbf05 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; } bool usesProxyProtocolOptions() const override { return false; } }; diff --git a/source/extensions/transport_sockets/alts/tsi_socket.cc b/source/extensions/transport_sockets/alts/tsi_socket.cc index 0fe5b752ceca3..7ba6b2cab798f 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 a4a7423bd927b..febe94fc83f8f 100644 --- a/source/extensions/transport_sockets/alts/tsi_socket.h +++ b/source/extensions/transport_sockets/alts/tsi_socket.h @@ -101,6 +101,7 @@ class TsiSocketFactory : public Network::TransportSocketFactory { bool usesProxyProtocolOptions() const override { return false; } 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 3d4f716421e72..ac2716c96d728 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 c4c9a80f629e9..3a4a768e557c5 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; bool usesProxyProtocolOptions() const override { return true; } private: diff --git a/source/extensions/transport_sockets/tap/tap.cc b/source/extensions/transport_sockets/tap/tap.cc index 2f58c23703ce7..972a1d8ffc38b 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(); } + bool TapSocketFactory::usesProxyProtocolOptions() const { return transport_socket_factory_->usesProxyProtocolOptions(); } diff --git a/source/extensions/transport_sockets/tap/tap.h b/source/extensions/transport_sockets/tap/tap.h index 2971c3e846ba6..3d37ae059e406 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; bool usesProxyProtocolOptions() const override; private: diff --git a/source/extensions/transport_sockets/tls/context_config_impl.cc b/source/extensions/transport_sockets/tls/context_config_impl.cc index 348a02d6dd9ef..ee7f0564a9b8e 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(), @@ -367,6 +367,16 @@ ClientContextConfigImpl::ClientContextConfigImpl( } } +bool ClientContextConfigImpl::isSecretReady() const { + for (const auto& provider : tls_certificate_providers_) { + if (provider == nullptr || provider->secret() == nullptr) { + return false; + } + } + return certificate_validation_context_provider_ == nullptr || + 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 44c5a8cc619d0..d40bb627a51de 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 50b2d27926e21..9d0c088155229 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -383,6 +383,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, @@ -424,6 +426,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 4c5f38e0fb143..d42d2591e5c28 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.h +++ b/source/extensions/transport_sockets/tls/ssl_socket.h @@ -106,9 +106,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; bool usesProxyProtocolOptions() const override { return false; } // Secret::SecretCallbacks @@ -134,6 +136,7 @@ class ServerSslSocketFactory : public Network::TransportSocketFactory, Network::TransportSocketPtr createTransportSocket(Network::TransportSocketOptionsSharedPtr options) const override; bool implementsSecureTransport() const override; + bool isReady() const override; bool usesProxyProtocolOptions() const override { return false; } // Secret::SecretCallbacks diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index a793dbe3ab4ab..6eb4257dd878b 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 155e2b6288577..41a1f01e1a493 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 { @@ -2405,6 +2407,136 @@ 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_matcher = std::make_unique>( + std::make_unique()); + EXPECT_CALL(*transport_socket_matcher, factoriesReady()).WillOnce(Return(false)); + 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"; + + 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_matcher = std::make_unique>( + std::make_unique()); + EXPECT_CALL(*transport_socket_matcher, factoriesReady()).WillOnce(Return(false)); + 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"; + + 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 b5e495c8e5928..7bb4f7711e5fd 100644 --- a/test/common/upstream/transport_socket_matcher_test.cc +++ b/test/common/upstream/transport_socket_matcher_test.cc @@ -34,6 +34,7 @@ class FakeTransportSocketFactory : public Network::TransportSocketFactory { MOCK_METHOD(bool, usesProxyProtocolOptions, (), (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_; } @@ -50,6 +51,7 @@ class FooTransportSocketFactory MOCK_METHOD(bool, usesProxyProtocolOptions, (), (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 16f56b4f94895..eda57ac595089 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 49445e4ec616d..b48b4f2733304 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -1648,6 +1648,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 b265710464f10..1c853038c082a 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -4750,6 +4750,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()); @@ -5959,6 +5960,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 3b299de13c513..7fa5dee966964 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 ed8fa15b7be36..9dd8a817e6e78 100644 --- a/test/mocks/network/transport_socket.h +++ b/test/mocks/network/transport_socket.h @@ -39,6 +39,7 @@ class MockTransportSocketFactory : public TransportSocketFactory { MOCK_METHOD(bool, usesProxyProtocolOptions, (), (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 3cc0ac4e7d559..8fafd921cc48f 100644 --- a/test/mocks/ssl/mocks.h +++ b/test/mocks/ssl/mocks.h @@ -97,6 +97,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 { diff --git a/test/mocks/upstream/transport_socket_match.h b/test/mocks/upstream/transport_socket_match.h index 16c1fd9fec204..62f35e9b4223e 100644 --- a/test/mocks/upstream/transport_socket_match.h +++ b/test/mocks/upstream/transport_socket_match.h @@ -19,6 +19,7 @@ class MockTransportSocketMatcher : public TransportSocketMatcher { ~MockTransportSocketMatcher() override; MOCK_METHOD(TransportSocketMatcher::MatchData, resolve, (const envoy::config::core::v3::Metadata*), (const)); + MOCK_METHOD(bool, factoriesReady, (), (const)); private: Network::TransportSocketFactoryPtr socket_factory_; diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index 13526f0eebb2c..59c88182d5384 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -62,7 +62,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/tracers:96.0" "source/extensions/tracers/opencensus:91.2" "source/extensions/tracers/xray:94.0" -"source/extensions/transport_sockets:95.3" +"source/extensions/transport_sockets:95.2" "source/extensions/transport_sockets/tap:95.6" "source/extensions/transport_sockets/tls:94.2" "source/extensions/transport_sockets/tls/ocsp:95.3"