Make ip_tagging exception free (#38460)

Commit Message: Make ip_tagging exception free
Additional Description:
Risk Level: low
Testing: existing ones
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

---------

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

source/extensions/filters/http/ip_tagging/config.cc

@@ -12,16 +12,17 @@ namespace Extensions {
 namespace HttpFilters {
 namespace IpTagging {
 
-Http::FilterFactoryCb IpTaggingFilterFactory::createFilterFactoryFromProtoTyped(
+absl::StatusOr<Http::FilterFactoryCb> IpTaggingFilterFactory::createFilterFactoryFromProtoTyped(
     const envoy::extensions::filters::http::ip_tagging::v3::IPTagging& proto_config,
     const std::string& stat_prefix, Server::Configuration::FactoryContext& context) {
 
-  IpTaggingFilterConfigSharedPtr config(new IpTaggingFilterConfig(
-      proto_config, stat_prefix, context.scope(), context.serverFactoryContext().runtime()));
-
-  return [config](Http::FilterChainFactoryCallbacks& callbacks) -> void {
-    callbacks.addStreamDecoderFilter(std::make_shared<IpTaggingFilter>(config));
-  };
+  absl::StatusOr<IpTaggingFilterConfigSharedPtr> config = IpTaggingFilterConfig::create(
+      proto_config, stat_prefix, context.scope(), context.serverFactoryContext().runtime());
+  RETURN_IF_NOT_OK_REF(config.status());
+  return
+      [config = std::move(config.value())](Http::FilterChainFactoryCallbacks& callbacks) -> void {
+        callbacks.addStreamDecoderFilter(std::make_shared<IpTaggingFilter>(config));
+      };
 }
 
 /**

source/extensions/filters/http/ip_tagging/config.h

@@ -13,13 +13,13 @@ namespace IpTagging {
 /**
  * Config registration for the router filter. @see NamedHttpFilterConfigFactory.
  */
-class IpTaggingFilterFactory
-    : public Common::FactoryBase<envoy::extensions::filters::http::ip_tagging::v3::IPTagging> {
+class IpTaggingFilterFactory : public Common::ExceptionFreeFactoryBase<
+                                   envoy::extensions::filters::http::ip_tagging::v3::IPTagging> {
 public:
-  IpTaggingFilterFactory() : FactoryBase("envoy.filters.http.ip_tagging") {}
+  IpTaggingFilterFactory() : ExceptionFreeFactoryBase("envoy.filters.http.ip_tagging") {}
 
 private:
-  Http::FilterFactoryCb createFilterFactoryFromProtoTyped(
+  absl::StatusOr<Http::FilterFactoryCb> createFilterFactoryFromProtoTyped(
       const envoy::extensions::filters::http::ip_tagging::v3::IPTagging& proto_config,
       const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override;
 };

source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc

@@ -13,9 +13,20 @@ namespace Extensions {
 namespace HttpFilters {
 namespace IpTagging {
 
+absl::StatusOr<IpTaggingFilterConfigSharedPtr> IpTaggingFilterConfig::create(
+    const envoy::extensions::filters::http::ip_tagging::v3::IPTagging& config,
+    const std::string& stat_prefix, Stats::Scope& scope, Runtime::Loader& runtime) {
+  absl::Status creation_status = absl::OkStatus();
+  auto config_ptr = std::shared_ptr<IpTaggingFilterConfig>(
+      new IpTaggingFilterConfig(config, stat_prefix, scope, runtime, creation_status));
+  RETURN_IF_NOT_OK(creation_status);
+  return config_ptr;
+}
+
 IpTaggingFilterConfig::IpTaggingFilterConfig(
     const envoy::extensions::filters::http::ip_tagging::v3::IPTagging& config,
-    const std::string& stat_prefix, Stats::Scope& scope, Runtime::Loader& runtime)
+    const std::string& stat_prefix, Stats::Scope& scope, Runtime::Loader& runtime,
+    absl::Status& creation_status)
     : request_type_(requestTypeEnum(config.request_type())), scope_(scope), runtime_(runtime),
       stat_name_set_(scope.symbolTable().makeSet("IpTagging")),
       stats_prefix_(stat_name_set_->add(stat_prefix + "ip_tagging")),
@@ -32,7 +43,9 @@ IpTaggingFilterConfig::IpTaggingFilterConfig(
   // TODO(ccaraman): Remove size check once file system support is implemented.
   // Work is tracked by issue https://github.com/envoyproxy/envoy/issues/2695.
   if (config.ip_tags().empty()) {
-    throw EnvoyException("HTTP IP Tagging Filter requires ip_tags to be specified.");
+    creation_status =
+        absl::InvalidArgumentError("HTTP IP Tagging Filter requires ip_tags to be specified.");
+    return;
   }
 
   std::vector<std::pair<std::string, std::vector<Network::Address::CidrRange>>> tag_data;
@@ -47,9 +60,10 @@ IpTaggingFilterConfig::IpTaggingFilterConfig(
       if (cidr_or_error.status().ok()) {
         cidr_set.emplace_back(std::move(cidr_or_error.value()));
       } else {
-        throw EnvoyException(
+        creation_status = absl::InvalidArgumentError(
             fmt::format("invalid ip/mask combo '{}/{}' (format is <ip>/<# mask bits>)",
                         entry.address_prefix(), entry.prefix_len().value()));
+        return;
       }
     }
 

source/extensions/filters/http/ip_tagging/ip_tagging_filter.h

@@ -34,10 +34,9 @@ class IpTaggingFilterConfig {
 public:
   using HeaderAction =
       envoy::extensions::filters::http::ip_tagging::v3::IPTagging::IpTagHeader::HeaderAction;
-
-  IpTaggingFilterConfig(const envoy::extensions::filters::http::ip_tagging::v3::IPTagging& config,
-                        const std::string& stat_prefix, Stats::Scope& scope,
-                        Runtime::Loader& runtime);
+  static absl::StatusOr<std::shared_ptr<IpTaggingFilterConfig>>
+  create(const envoy::extensions::filters::http::ip_tagging::v3::IPTagging& config,
+         const std::string& stat_prefix, Stats::Scope& scope, Runtime::Loader& runtime);
 
   Runtime::Loader& runtime() { return runtime_; }
   FilterRequestType requestType() const { return request_type_; }
@@ -58,6 +57,10 @@ class IpTaggingFilterConfig {
   void incTotal() { incCounter(total_); }
 
 private:
+  IpTaggingFilterConfig(const envoy::extensions::filters::http::ip_tagging::v3::IPTagging& config,
+                        const std::string& stat_prefix, Stats::Scope& scope,
+                        Runtime::Loader& runtime, absl::Status& creation_status);
+
   static FilterRequestType requestTypeEnum(
       envoy::extensions::filters::http::ip_tagging::v3::IPTagging::RequestType request_type) {
     switch (request_type) {

test/extensions/filters/http/ip_tagging/ip_tagging_filter_test.cc

@@ -39,16 +39,28 @@ request_type: internal
 
 )EOF";
 
-  void initializeFilter(const std::string& yaml) {
+  void initializeFilter(const std::string& yaml,
+                        absl::optional<std::string> expected_error = absl::nullopt) {
     envoy::extensions::filters::http::ip_tagging::v3::IPTagging config;
     TestUtility::loadFromYaml(yaml, config);
-    config_ =
-        std::make_shared<IpTaggingFilterConfig>(config, "prefix.", *stats_.rootScope(), runtime_);
+    auto config_or =
+        IpTaggingFilterConfig::create(config, "prefix.", *stats_.rootScope(), runtime_);
+    if (expected_error.has_value()) {
+      EXPECT_FALSE(config_or.ok());
+      EXPECT_EQ(expected_error.value(), absl::StrCat(config_or.status()));
+      return;
+    }
+    EXPECT_TRUE(config_or.ok());
+    config_ = std::move(config_or.value());
     filter_ = std::make_unique<IpTaggingFilter>(config_);
     filter_->setDecoderFilterCallbacks(filter_callbacks_);
   }
 
-  ~IpTaggingFilterTest() override { filter_->onDestroy(); }
+  ~IpTaggingFilterTest() override {
+    if (filter_ != nullptr) {
+      filter_->onDestroy();
+    }
+  }
 
   NiceMock<Stats::MockStore> stats_;
   IpTaggingFilterConfigSharedPtr config_;
@@ -437,6 +449,28 @@ TEST_F(IpTaggingFilterTest, ClearRouteCache) {
   EXPECT_FALSE(request_headers.has(Http::Headers::get().EnvoyIpTags));
 }
 
+TEST_F(IpTaggingFilterTest, EmptyIpTags) {
+  const std::string external_request_yaml = R"EOF(
+request_type: external
+)EOF";
+  initializeFilter(external_request_yaml,
+                   "INVALID_ARGUMENT: HTTP IP Tagging Filter requires ip_tags to be specified.");
+}
+
+TEST_F(IpTaggingFilterTest, InvalidCidr) {
+  const std::string external_request_yaml = R"EOF(
+request_type: external
+ip_tags:
+  - ip_tag_name: fooooooo
+    ip_list:
+      - {address_prefix: 12345.12345.12345.12345, prefix_len: 999999}
+)EOF";
+  initializeFilter(
+      external_request_yaml,
+      "INVALID_ARGUMENT: invalid ip/mask combo '12345.12345.12345.12345/999999' (format is "
+      "<ip>/<# mask bits>)");
+}
+
 } // namespace
 } // namespace IpTagging
 } // namespace HttpFilters

tools/code_format/config.yaml

@@ -172,7 +172,6 @@ paths:
     - source/extensions/filters/http/grpc_json_transcoder
     - source/extensions/filters/http/grpc_stats
     - source/extensions/filters/http/health_check
-    - source/extensions/filters/http/ip_tagging
     - source/extensions/filters/http/json_to_metadata
     - source/extensions/filters/http/jwt_authn
     - source/extensions/filters/http/local_ratelimit