Sds backport (#281)

* cluster manager: avoid immediate activation for dynamic inserted cluster when initialize (envoyproxy#12783)

Signed-off-by: Shikugawa <rei@tetrate.io>

* sds: keep warming when dynamic inserted cluster can't be extracted secret entity (envoyproxy#13344)

This PR highly depends on envoyproxy#12783. Changed to keep warming if dynamic inserted clusters (when initialize doesn't finished) failed to extract TLS certificate and certificate validation context. They shouldn't be indicated as ACTIVE cluster.
Risk Level: Mid
Testing: Unit
Docs Changes:
Release Notes: Added
Fixes envoyproxy#11120, future work: envoyproxy#13777

Signed-off-by: Shikugawa <rei@tetrate.io>

Co-authored-by: Rei Shimizu <rei@tetrate.io>

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<envoy_v3_api_field_config.bootstrap.v3.Bootstrap.watchdogs>`. It works with :ref:`watchdog<envoy_v3_api_field_config.bootstrap.v3.Bootstrap.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 <envoy_api_field_config.filter.http.ext_authz.v2.ExtAuthz.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 <envoy_v3_api_msg_watchdog.v3alpha.AbortActionConfig>` is now the default action to terminate the process if watchdog kill / multikill is enabled.
 
 Bug Fixes
 ---------

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

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

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;
 };
 
 } // namespace Network

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

source/common/upstream/cluster_manager_impl.cc

@@ -417,7 +417,31 @@ void ClusterManagerImpl::onClusterInit(Cluster& cluster) {
   // been setup for cross-thread updates to avoid needless updates during initialization. The order
   // of operations here is important. We start by initializing the thread aware load balancer if
   // needed. This must happen first so cluster updates are heard first by the load balancer.
-  auto cluster_data = active_clusters_.find(cluster.info()->name());
+  // Also, it assures that all of clusters which this function is called should be always active.
+  auto cluster_data = warming_clusters_.find(cluster.info()->name());
+  // 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();
+  }
+  cluster_data = active_clusters_.find(cluster.info()->name());
+
   if (cluster_data->second->thread_aware_lb_ != nullptr) {
     cluster_data->second->thread_aware_lb_->initialize();
   }
@@ -587,17 +611,6 @@ bool ClusterManagerImpl::addOrUpdateCluster(const envoy::config::cluster::v3::Cl
       // The following init manager remove call is a NOP in the case we are already initialized.
       // It's just kept here to avoid additional logic.
       init_helper_.removeCluster(*existing_active_cluster->second->cluster_);
-    } else {
-      // Validate that warming clusters are not added to the init_helper_.
-      // NOTE: This loop is compiled out in optimized builds.
-      for (const std::list<Cluster*>& cluster_list :
-           {std::cref(init_helper_.primary_init_clusters_),
-            std::cref(init_helper_.secondary_init_clusters_)}) {
-        ASSERT(!std::any_of(cluster_list.begin(), cluster_list.end(),
-                            [&existing_warming_cluster](Cluster* cluster) {
-                              return existing_warming_cluster->second->cluster_.get() == cluster;
-                            }));
-      }
     }
     cm_stats_.cluster_modified_.inc();
   } else {
@@ -614,40 +627,39 @@ bool ClusterManagerImpl::addOrUpdateCluster(const envoy::config::cluster::v3::Cl
   //       the future we may decide to undergo a refactor to unify the logic but the effort/risk to
   //       do that right now does not seem worth it given that the logic is generally pretty clean
   //       and easy to understand.
-  const bool use_active_map =
-      init_helper_.state() != ClusterManagerInitHelper::State::AllClustersInitialized;
-  loadCluster(cluster, version_info, true, use_active_map ? active_clusters_ : warming_clusters_);
-
-  if (use_active_map) {
+  const bool all_clusters_initialized =
+      init_helper_.state() == ClusterManagerInitHelper::State::AllClustersInitialized;
+  loadCluster(cluster, version_info, true, warming_clusters_);
+  auto& cluster_entry = warming_clusters_.at(cluster_name);
+  if (!all_clusters_initialized) {
     ENVOY_LOG(debug, "add/update cluster {} during init", cluster_name);
-    auto& cluster_entry = active_clusters_.at(cluster_name);
     createOrUpdateThreadLocalCluster(*cluster_entry);
     init_helper_.addCluster(*cluster_entry->cluster_);
   } else {
-    auto& cluster_entry = warming_clusters_.at(cluster_name);
     ENVOY_LOG(debug, "add/update cluster {} starting warming", cluster_name);
     cluster_entry->cluster_->initialize([this, cluster_name] {
-      auto warming_it = warming_clusters_.find(cluster_name);
-      auto& cluster_entry = *warming_it->second;
-
-      // If the cluster is being updated, we need to cancel any pending merged updates.
-      // Otherwise, applyUpdates() will fire with a dangling cluster reference.
-      updates_map_.erase(cluster_name);
-
-      active_clusters_[cluster_name] = std::move(warming_it->second);
-      warming_clusters_.erase(warming_it);
-
       ENVOY_LOG(debug, "warming cluster {} complete", cluster_name);
-      createOrUpdateThreadLocalCluster(cluster_entry);
-      onClusterInit(*cluster_entry.cluster_);
-      updateClusterCounts();
+      auto state_changed_cluster_entry = warming_clusters_.find(cluster_name);
+      createOrUpdateThreadLocalCluster(*state_changed_cluster_entry->second);
+      onClusterInit(*state_changed_cluster_entry->second->cluster_);
     });
   }
 
-  updateClusterCounts();
   return true;
 }
 
+void ClusterManagerImpl::clusterWarmingToActive(const std::string& cluster_name) {
+  auto warming_it = warming_clusters_.find(cluster_name);
+  ASSERT(warming_it != warming_clusters_.end());
+
+  // If the cluster is being updated, we need to cancel any pending merged updates.
+  // Otherwise, applyUpdates() will fire with a dangling cluster reference.
+  updates_map_.erase(cluster_name);
+
+  active_clusters_[cluster_name] = std::move(warming_it->second);
+  warming_clusters_.erase(warming_it);
+}
+
 void ClusterManagerImpl::createOrUpdateThreadLocalCluster(ClusterData& cluster) {
   tls_->runOnAllThreads([new_cluster = cluster.cluster_->info(),
                          thread_aware_lb_factory = cluster.loadBalancerFactory()](
@@ -702,6 +714,7 @@ bool ClusterManagerImpl::removeCluster(const std::string& cluster_name) {
   if (existing_warming_cluster != warming_clusters_.end() &&
       existing_warming_cluster->second->added_via_api_) {
     removed = true;
+    init_helper_.removeCluster(*existing_warming_cluster->second->cluster_);
     warming_clusters_.erase(existing_warming_cluster);
     ENVOY_LOG(info, "removing warming cluster {}", cluster_name);
   }
@@ -804,7 +817,9 @@ void ClusterManagerImpl::updateClusterCounts() {
   // Once cluster is warmed up, CDS is resumed, and ACK is sent to ADS, providing a
   // signal to ADS to proceed with RDS updates.
   // If we're in the middle of shutting down (ads_mux_ already gone) then this is irrelevant.
-  if (ads_mux_) {
+  const bool all_clusters_initialized =
+      init_helper_.state() == ClusterManagerInitHelper::State::AllClustersInitialized;
+  if (all_clusters_initialized && ads_mux_) {
     const auto type_urls = Config::getAllVersionTypeUrls<envoy::config::cluster::v3::Cluster>();
     const uint64_t previous_warming = cm_stats_.warming_clusters_.value();
     if (previous_warming == 0 && !warming_clusters_.empty()) {

source/common/upstream/cluster_manager_impl.h

@@ -482,6 +482,7 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable<Logger::Id::u
   void onClusterInit(Cluster& cluster);
   void postThreadLocalHealthFailure(const HostSharedPtr& host);
   void updateClusterCounts();
+  void clusterWarmingToActive(const std::string& cluster_name);
   void maybePrefetch(ThreadLocalClusterManagerImpl::ClusterEntryPtr& cluster_entry,
                      std::function<ConnectionPool::Instance*()> prefetch_pool);
 

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

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

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

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;
 
 private:
   Network::TransportSocketFactoryPtr transport_socket_factory_;

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

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

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;
 

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

@@ -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

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;