sds: keep warming when dynamic inserted cluster can't be extracted secret entity

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;

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<ClientContextConfig>;

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<TransportSocketMatcher>;

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

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; }
 };
 

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() {

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();
   }

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

source/common/upstream/transport_socket_match_impl.h

@@ -39,6 +39,7 @@ class TransportSocketMatcherImpl : public Logger::Loggable<Logger::Id::upstream>
       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);

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; }
 };
 

source/extensions/transport_sockets/alts/tsi_socket.cc

@@ -261,6 +261,7 @@ TsiSocketFactory::createTransportSocket(Network::TransportSocketOptionsSharedPtr
   return std::make_unique<TsiSocket>(handshaker_factory_, handshake_validator_);
 }
 
+bool TsiSocketFactory::isReady() const { return true; }
 } // namespace Alts
 } // namespace TransportSockets
 } // namespace Extensions

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_;

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
+} // namespace Envoy

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:

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();
 }

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:

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;
 

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<envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext>
+      default_cvc_;
+  std::vector<Secret::TlsCertificateConfigProviderSharedPtr> 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<Ssl::TlsCertificateConfigImpl> 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<envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext>
-      default_cvc_;
-  std::vector<Secret::TlsCertificateConfigProviderSharedPtr> 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;

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

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

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",
     ],
 )