Refactor SecretManagerImpl to templatize findOrCreate method.

source/common/secret/sds_api.h

@@ -68,14 +68,18 @@ class SdsApi : public Init::Target,
   Cleanup clean_up_;
 };
 
-typedef std::shared_ptr<SdsApi> SdsApiSharedPtr;
+class TlsCertificateSdsApi;
+class CertificateValidationContextSdsApi;
+typedef std::shared_ptr<TlsCertificateSdsApi> TlsCertificateSdsApiSharedPtr;
+typedef std::shared_ptr<CertificateValidationContextSdsApi>
+    CertificateValidationContextSdsApiSharedPtr;
 
 /**
  * TlsCertificateSdsApi implementation maintains and updates dynamic TLS certificate secrets.
  */
 class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider {
 public:
-  static SdsApiSharedPtr
+  static TlsCertificateSdsApiSharedPtr
   create(Server::Configuration::TransportSocketFactoryContext& secret_provider_context,
          const envoy::api::v2::core::ConfigSource& sds_config, const std::string& sds_config_name,
          std::function<void()> destructor_cb) {
@@ -119,7 +123,7 @@ class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider
 class CertificateValidationContextSdsApi : public SdsApi,
                                            public CertificateValidationContextConfigProvider {
 public:
-  static SdsApiSharedPtr
+  static CertificateValidationContextSdsApiSharedPtr
   create(Server::Configuration::TransportSocketFactoryContext& secret_provider_context,
          const envoy::api::v2::core::ConfigSource& sds_config, const std::string& sds_config_name,
          std::function<void()> destructor_cb) {

source/common/secret/secret_manager_impl.cc

@@ -64,59 +64,19 @@ SecretManagerImpl::createInlineCertificateValidationContextProvider(
       certificate_validation_context);
 }
 
-void SecretManagerImpl::removeDynamicSecretProvider(const std::string& map_key) {
-  ENVOY_LOG(debug, "Unregister secret provider. hash key: {}", map_key);
-
-  auto num_deleted = dynamic_secret_providers_.erase(map_key);
-  ASSERT(num_deleted == 1, "");
-}
-
-SdsApiSharedPtr SecretManagerImpl::findOrCreate(
-    const envoy::api::v2::core::ConfigSource& sds_config_source, const std::string& config_name,
-    std::function<SdsApiSharedPtr(std::function<void()> unregister_secret_provider)> create_fn) {
-  const std::string map_key = sds_config_source.SerializeAsString() + config_name;
-
-  SdsApiSharedPtr secret_provider = dynamic_secret_providers_[map_key].lock();
-  if (!secret_provider) {
-    // SdsApi is owned by ListenerImpl and ClusterInfo which are destroyed before
-    // SecretManagerImpl. It is safe to invoke this callback at the destructor of SdsApi.
-    std::function<void()> unregister_secret_provider = [map_key, this]() {
-      removeDynamicSecretProvider(map_key);
-    };
-
-    secret_provider = create_fn(unregister_secret_provider);
-    dynamic_secret_providers_[map_key] = secret_provider;
-  }
-  return secret_provider;
-}
-
 TlsCertificateConfigProviderSharedPtr SecretManagerImpl::findOrCreateTlsCertificateProvider(
     const envoy::api::v2::core::ConfigSource& sds_config_source, const std::string& config_name,
     Server::Configuration::TransportSocketFactoryContext& secret_provider_context) {
-  auto create_fn = [&secret_provider_context, &sds_config_source, &config_name](
-                       std::function<void()> unregister_secret_provider) -> SdsApiSharedPtr {
-    ASSERT(secret_provider_context.initManager() != nullptr);
-    return TlsCertificateSdsApi::create(secret_provider_context, sds_config_source, config_name,
-                                        unregister_secret_provider);
-  };
-  SdsApiSharedPtr secret_provider = findOrCreate(sds_config_source, config_name, create_fn);
-
-  return std::dynamic_pointer_cast<TlsCertificateConfigProvider>(secret_provider);
+  return certificate_providers_.findOrCreate(sds_config_source, config_name,
+                                             secret_provider_context);
 }
 
 CertificateValidationContextConfigProviderSharedPtr
 SecretManagerImpl::findOrCreateCertificateValidationContextProvider(
     const envoy::api::v2::core::ConfigSource& sds_config_source, const std::string& config_name,
     Server::Configuration::TransportSocketFactoryContext& secret_provider_context) {
-  auto create_fn = [&secret_provider_context, &sds_config_source, &config_name](
-                       std::function<void()> unregister_secret_provider) -> SdsApiSharedPtr {
-    ASSERT(secret_provider_context.initManager() != nullptr);
-    return CertificateValidationContextSdsApi::create(secret_provider_context, sds_config_source,
-                                                      config_name, unregister_secret_provider);
-  };
-  SdsApiSharedPtr secret_provider = findOrCreate(sds_config_source, config_name, create_fn);
-
-  return std::dynamic_pointer_cast<CertificateValidationContextConfigProvider>(secret_provider);
+  return validation_context_providers_.findOrCreate(sds_config_source, config_name,
+                                                    secret_provider_context);
 }
 
 } // namespace Secret

source/common/secret/secret_manager_impl.h

@@ -14,7 +14,7 @@
 namespace Envoy {
 namespace Secret {
 
-class SecretManagerImpl : public SecretManager, Logger::Loggable<Logger::Id::secret> {
+class SecretManagerImpl : public SecretManager {
 public:
   void addStaticSecret(const envoy::api::v2::auth::Secret& secret) override;
 
@@ -42,12 +42,42 @@ class SecretManagerImpl : public SecretManager, Logger::Loggable<Logger::Id::sec
       Server::Configuration::TransportSocketFactoryContext& secret_provider_context) override;
 
 private:
-  // Removes dynamic secret provider which has been deleted.
-  void removeDynamicSecretProvider(const std::string& map_key);
-  // Finds or creates SdsApi object.
-  SdsApiSharedPtr findOrCreate(
-      const envoy::api::v2::core::ConfigSource& sds_config_source, const std::string& config_name,
-      std::function<SdsApiSharedPtr(std::function<void()> unregister_secret_provider)> create_fn);
+  template <class SecretType>
+  class DynamicSecretProviders : public Logger::Loggable<Logger::Id::secret> {
+  public:
+    // Finds or creates SdsApi object.
+    std::shared_ptr<SecretType>
+    findOrCreate(const envoy::api::v2::core::ConfigSource& sds_config_source,
+                 const std::string& config_name,
+                 Server::Configuration::TransportSocketFactoryContext& secret_provider_context) {
+      const std::string map_key = sds_config_source.SerializeAsString() + config_name;
+
+      std::shared_ptr<SecretType> secret_provider = dynamic_secret_providers_[map_key].lock();
+      if (!secret_provider) {
+        // SdsApi is owned by ListenerImpl and ClusterInfo which are destroyed before
+        // SecretManagerImpl. It is safe to invoke this callback at the destructor of SdsApi.
+        std::function<void()> unregister_secret_provider = [map_key, this]() {
+          removeDynamicSecretProvider(map_key);
+        };
+        ASSERT(secret_provider_context.initManager() != nullptr);
+        secret_provider = SecretType::create(secret_provider_context, sds_config_source,
+                                             config_name, unregister_secret_provider);
+        dynamic_secret_providers_[map_key] = secret_provider;
+      }
+      return secret_provider;
+    }
+
+  private:
+    // Removes dynamic secret provider which has been deleted.
+    void removeDynamicSecretProvider(const std::string& map_key) {
+      ENVOY_LOG(debug, "Unregister secret provider. hash key: {}", map_key);
+
+      auto num_deleted = dynamic_secret_providers_.erase(map_key);
+      ASSERT(num_deleted == 1, "");
+    }
+
+    std::unordered_map<std::string, std::weak_ptr<SecretType>> dynamic_secret_providers_;
+  };
 
   // Manages pairs of secret name and TlsCertificateConfigProviderSharedPtr.
   std::unordered_map<std::string, TlsCertificateConfigProviderSharedPtr>
@@ -58,7 +88,8 @@ class SecretManagerImpl : public SecretManager, Logger::Loggable<Logger::Id::sec
       static_certificate_validation_context_providers_;
 
   // map hash code of SDS config source and SdsApi object.
-  std::unordered_map<std::string, std::weak_ptr<SdsApi>> dynamic_secret_providers_;
+  DynamicSecretProviders<TlsCertificateSdsApi> certificate_providers_;
+  DynamicSecretProviders<CertificateValidationContextSdsApi> validation_context_providers_;
 };
 
 } // namespace Secret