Correct the shared_ptr pattern for filter config (#22436)

* Correct the shared_ptr pattern for filter config Signed-off-by: Tianyu Xia <[email protected]>
envoyproxy · Aug 10, 2022 · ff11af7 · ff11af7
1 parent 5db5651
commit ff11af7
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 22 deletions.
diff --git a/source/extensions/filters/http/gcp_authn/filter_config.cc b/source/extensions/filters/http/gcp_authn/filter_config.cc
@@ -20,10 +20,15 @@ Http::FilterFactoryCb GcpAuthnFilterFactory::createFilterFactoryFromProtoTyped(
   if (PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.cache_config(), cache_size, 0) > 0) {
     token_cache = std::make_shared<TokenCache>(config.cache_config(), context);
   }
-  return [config, stats_prefix, &context, token_cache = std::move(token_cache)](
-             Http::FilterChainFactoryCallbacks& callbacks) -> void {
+  FilterConfigSharedPtr filter_config =
+      std::make_shared<envoy::extensions::filters::http::gcp_authn::v3::GcpAuthnFilterConfig>(
+          config);
+
+  return [config, stats_prefix, &context, token_cache = std::move(token_cache),
+          filter_config =
+              std::move(filter_config)](Http::FilterChainFactoryCallbacks& callbacks) -> void {
     callbacks.addStreamFilter(std::make_shared<GcpAuthnFilter>(
-        config, context, stats_prefix,
+        filter_config, context, stats_prefix,
         (token_cache != nullptr) ? &token_cache->tls.get()->cache() : nullptr));
   };
 }

diff --git a/source/extensions/filters/http/gcp_authn/gcp_authn_filter.h b/source/extensions/filters/http/gcp_authn/gcp_authn_filter.h
@@ -32,8 +32,8 @@ struct GcpAuthnFilterStats {
   ALL_GCP_AUTHN_FILTER_STATS(GENERATE_COUNTER_STRUCT)
 };
 
-using FilterConfigProtoSharedPtr =
-    std::shared_ptr<envoy::extensions::filters::http::gcp_authn::v3::GcpAuthnFilterConfig>;
+using FilterConfigSharedPtr =
+    std::shared_ptr<const envoy::extensions::filters::http::gcp_authn::v3::GcpAuthnFilterConfig>;
 
 class GcpAuthnFilter : public Http::PassThroughFilter,
                        public RequestCallbacks,
@@ -44,14 +44,11 @@ class GcpAuthnFilter : public Http::PassThroughFilter,
   // it or has completed.
   enum class State { NotStarted, Calling, Complete };
 
-  GcpAuthnFilter(
-      const envoy::extensions::filters::http::gcp_authn::v3::GcpAuthnFilterConfig& config,
-      Server::Configuration::FactoryContext& context, const std::string& stats_prefix,
-      TokenCacheImpl<JwtToken>* token_cache)
-      : filter_config_(
-            std::make_shared<envoy::extensions::filters::http::gcp_authn::v3::GcpAuthnFilterConfig>(
-                config)),
-        context_(context), client_(std::make_unique<GcpAuthnClient>(*filter_config_, context_)),
+  GcpAuthnFilter(FilterConfigSharedPtr filter_config,
+                 Server::Configuration::FactoryContext& context, const std::string& stats_prefix,
+                 TokenCacheImpl<JwtToken>* token_cache)
+      : filter_config_(std::move(filter_config)), context_(context),
+        client_(std::make_unique<GcpAuthnClient>(*filter_config_, context_)),
         stats_(generateStats(stats_prefix, context_.scope())), jwt_token_cache_(token_cache) {}
 
   Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers,
@@ -70,7 +67,7 @@ class GcpAuthnFilter : public Http::PassThroughFilter,
     return {ALL_GCP_AUTHN_FILTER_STATS(POOL_COUNTER_PREFIX(scope, stats_prefix))};
   }
 
-  FilterConfigProtoSharedPtr filter_config_;
+  FilterConfigSharedPtr filter_config_;
   Server::Configuration::FactoryContext& context_;
   std::unique_ptr<GcpAuthnClient> client_;
   Http::StreamDecoderFilterCallbacks* decoder_callbacks_{};

diff --git a/test/extensions/filters/http/gcp_authn/gcp_authn_filter_test.cc b/test/extensions/filters/http/gcp_authn/gcp_authn_filter_test.cc
@@ -42,6 +42,14 @@ constexpr char DefaultConfig[] = R"EOF(
 
 class GcpAuthnFilterTest : public testing::Test {
 public:
+  GcpAuthnFilterTest() {
+    // Initialize the default configuration.
+    TestUtility::loadFromYaml(DefaultConfig, config_);
+    filter_config_ =
+        std::make_shared<envoy::extensions::filters::http::gcp_authn::v3::GcpAuthnFilterConfig>(
+            config_);
+  }
+
   void setupMockObjects() {
     EXPECT_CALL(context_.cluster_manager_, getThreadLocalCluster(_))
         .WillRepeatedly(Return(&thread_local_cluster_));
@@ -58,7 +66,7 @@ class GcpAuthnFilterTest : public testing::Test {
   }
 
   void setupFilterAndCallback() {
-    filter_ = std::make_unique<GcpAuthnFilter>(config_, context_, "stats", nullptr);
+    filter_ = std::make_unique<GcpAuthnFilter>(filter_config_, context_, "stats", nullptr);
     filter_->setDecoderFilterCallbacks(decoder_callbacks_);
   }
 
@@ -77,14 +85,15 @@ class GcpAuthnFilterTest : public testing::Test {
     ON_CALL(*cluster_info_, metadata()).WillByDefault(testing::ReturnRef(metadata_));
   }
 
-  void createClient(const std::string& config_str = DefaultConfig) {
-    TestUtility::loadFromYaml(config_str, config_);
-    client_ = std::make_unique<GcpAuthnClient>(config_, context_);
-  }
-  void createClient(const GcpAuthnFilterConfig& config) {
-    client_ = std::make_unique<GcpAuthnClient>(config, context_);
+  void overrideConfig(const GcpAuthnFilterConfig& config) {
+    config_ = config;
+    filter_config_ =
+        std::make_shared<envoy::extensions::filters::http::gcp_authn::v3::GcpAuthnFilterConfig>(
+            config);
   }
 
+  void createClient() { client_ = std::make_unique<GcpAuthnClient>(config_, context_); }
+
   NiceMock<MockFactoryContext> context_;
   NiceMock<MockThreadLocalCluster> thread_local_cluster_;
   std::shared_ptr<NiceMock<Upstream::MockClusterInfo>> cluster_info_;
@@ -101,6 +110,7 @@ class GcpAuthnFilterTest : public testing::Test {
   std::unique_ptr<GcpAuthnClient> client_;
   std::unique_ptr<GcpAuthnFilter> filter_;
   GcpAuthnFilterConfig config_;
+  FilterConfigSharedPtr filter_config_;
   Http::TestRequestHeaderMapImpl default_headers_{
       {":method", "GET"}, {":path", "/"}, {":scheme", "http"}, {":authority", "host"}};
   envoy::config::core::v3::Metadata metadata_;
@@ -152,7 +162,8 @@ TEST_F(GcpAuthnFilterTest, NoCluster) {
   EXPECT_CALL(request_callbacks_, onComplete(/*response_ptr=*/nullptr));
   GcpAuthnFilterConfig config;
   TestUtility::loadFromYaml(no_cluster_config, config);
-  createClient(config);
+  overrideConfig(config);
+  createClient();
   client_->fetchToken(request_callbacks_, buildRequest(config.http_uri().uri()));
 }