review

Signed-off-by: Kuat Yessenov <[email protected]>
kyessenov · Sep 29, 2023 · afe20ce · afe20ce
1 parent 664648a
commit afe20ce
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 34 deletions.
diff --git a/api/envoy/extensions/filters/common/set_filter_state/v3/rule.proto b/api/envoy/extensions/filters/common/set_filter_state/v3/rule.proto
@@ -29,8 +29,13 @@ message Rule {
     TRANSITIVE = 2;
   }
 
-  // Filter state key.
-  string key = 1 [(validate.rules).string = {min_len: 1}];
+  oneof key {
+    option (validate.required) = true;
+
+    // Filter state object key. The key is expected to be registered via an object factory, see
+    // :ref:`the well-known filter state keys <well_known_filter_state>`.
+    string object_key = 1 [(validate.rules).string = {min_len: 1}];
+  }
 
   oneof value {
     option (validate.required) = true;

diff --git a/api/envoy/extensions/filters/http/set_filter_state/v3/set_filter_state.proto b/api/envoy/extensions/filters/http/set_filter_state/v3/set_filter_state.proto
@@ -21,6 +21,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#extension: envoy.filters.http.set_filter_state]
 
 message Config {
-  // A sequence of the filter state updates to apply.
-  repeated common.set_filter_state.v3.Rule rules = 1;
+  // A sequence of the filter state updates to apply on request headers events.
+  repeated common.set_filter_state.v3.Rule on_request_headers = 1;
 }
diff --git a/api/envoy/extensions/filters/network/set_filter_state/v3/set_filter_state.proto b/api/envoy/extensions/filters/network/set_filter_state/v3/set_filter_state.proto
@@ -21,6 +21,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#extension: envoy.filters.network.set_filter_state]
 
 message Config {
-  // A sequence of the filter state updates to apply.
-  repeated common.set_filter_state.v3.Rule rules = 1;
+  // A sequence of the filter state updates to apply on the new connection events.
+  repeated common.set_filter_state.v3.Rule on_new_connection = 1;
 }
diff --git a/docs/root/configuration/http/http_filters/set_filter_state.rst b/docs/root/configuration/http/http_filters/set_filter_state.rst
@@ -1,7 +1,7 @@
 .. _config_http_filters_set_filter_state:
 
-Envoy Set-Filter-State HTTP Filter
-==================================
+Set-Filter-State HTTP Filter
+============================
 * This filter should be configured with the type URL ``type.googleapis.com/envoy.extensions.filters.http.set_filter_state.v3.Config``.
 * :ref:`v3 API reference <envoy_v3_api_msg_extensions.filters.http.set_filter_state.v3.Config>`
 
@@ -20,8 +20,8 @@ address:
 .. validated-code-block:: yaml
   :type-name: envoy.extensions.filters.http.set_filter_state.v3.Config
 
-  rules:
-  - key: envoy.network.transport_socket.original_dst_address
+  on_request_headers:
+  - object_key: envoy.network.transport_socket.original_dst_address
     format_string:
       text_format_source:
         inline_string: "%REQ(:AUTHORITY)%"
@@ -35,13 +35,13 @@ filter addresses on the upstream :ref:`internal listener connection
 .. validated-code-block:: yaml
   :type-name: envoy.extensions.filters.http.set_filter_state.v3.Config
 
-  rules:
-  - key: envoy.filters.listener.original_dst.local_ip
+  on_request_headers:
+  - object_key: envoy.filters.listener.original_dst.local_ip
     format_string:
       text_format_source:
         inline_string: "%REQ(:AUTHORITY)%"
     shared_with_upstream: ONCE
-  - key: envoy.filters.listener.original_dst.remote_ip
+  - object_key: envoy.filters.listener.original_dst.remote_ip
     format_string:
       text_format_source:
         inline_string: "%DOWNSTREAM_REMOTE_ADDRESS%"

diff --git a/docs/root/configuration/listeners/network_filters/set_filter_state.rst b/docs/root/configuration/listeners/network_filters/set_filter_state.rst
@@ -1,7 +1,7 @@
 .. _config_network_filters_set_filter_state:
 
-Envoy Set-Filter-State Network Filter
-=====================================
+Set-Filter-State Network Filter
+===============================
 * This filter should be configured with the type URL ``type.googleapis.com/envoy.extensions.filters.network.set_filter_state.v3.Config``.
 * :ref:`v3 API reference <envoy_v3_api_msg_extensions.filters.network.set_filter_state.v3.Config>`
 
@@ -18,8 +18,8 @@ A sample filter configuration that propagates the downstream SNI as the upstream
 .. validated-code-block:: yaml
   :type-name: envoy.extensions.filters.http.set_filter_state.v3.Config
 
-  rules:
-  - key: envoy.network.upstream_server_name
+  on_new_connection:
+  - object_key: envoy.network.upstream_server_name
     format_string:
       text_format_source:
         inline_string: "%REQUESTED_SERVER_NAME%"

diff --git a/source/extensions/filters/common/set_filter_state/filter_config.cc b/source/extensions/filters/common/set_filter_state/filter_config.cc
@@ -16,7 +16,7 @@ Config::parse(const Protobuf::RepeatedPtrField<
   rules.reserve(proto_rules.size());
   for (const auto& proto_rule : proto_rules) {
     Rule rule;
-    rule.key_ = proto_rule.key();
+    rule.key_ = proto_rule.object_key();
     rule.factory_ =
         Registry::FactoryRegistry<StreamInfo::FilterState::ObjectFactory>::getFactory(rule.key_);
     if (rule.factory_ == nullptr) {

diff --git a/source/extensions/filters/http/set_filter_state/config.cc b/source/extensions/filters/http/set_filter_state/config.cc
@@ -26,7 +26,7 @@ Http::FilterFactoryCb SetFilterStateConfig::createFilterFactoryFromProtoTyped(
     const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
     const std::string&, Server::Configuration::FactoryContext& context) {
   auto filter_config(std::make_shared<Filters::Common::SetFilterState::Config>(
-      proto_config.rules(), StreamInfo::FilterState::LifeSpan::FilterChain, context));
+      proto_config.on_request_headers(), StreamInfo::FilterState::LifeSpan::FilterChain, context));
   return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void {
     callbacks.addStreamDecoderFilter(
         Http::StreamDecoderFilterSharedPtr{new SetFilterState(filter_config)});
@@ -37,7 +37,7 @@ Http::FilterFactoryCb SetFilterStateConfig::createFilterFactoryFromProtoWithServ
     const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
     const std::string&, Server::Configuration::ServerFactoryContext& context) {
   auto filter_config(std::make_shared<Filters::Common::SetFilterState::Config>(
-      proto_config.rules(), StreamInfo::FilterState::LifeSpan::FilterChain, context));
+      proto_config.on_request_headers(), StreamInfo::FilterState::LifeSpan::FilterChain, context));
 
   return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void {
     callbacks.addStreamDecoderFilter(

diff --git a/source/extensions/filters/network/set_filter_state/config.cc b/source/extensions/filters/network/set_filter_state/config.cc
@@ -34,7 +34,7 @@ class SetFilterStateConfigFactory
       const envoy::extensions::filters::network::set_filter_state::v3::Config& proto_config,
       Server::Configuration::FactoryContext& context) override {
     auto filter_config(std::make_shared<Filters::Common::SetFilterState::Config>(
-        proto_config.rules(), StreamInfo::FilterState::LifeSpan::Connection, context));
+        proto_config.on_new_connection(), StreamInfo::FilterState::LifeSpan::Connection, context));
     return [filter_config](Network::FilterManager& filter_manager) -> void {
       filter_manager.addReadFilter(std::make_shared<SetFilterState>(filter_config));
     };

diff --git a/test/extensions/filters/common/set_filter_state/filter_config_test.cc b/test/extensions/filters/common/set_filter_state/filter_config_test.cc
@@ -64,7 +64,7 @@ class ConfigTest : public testing::Test {
 
 TEST_F(ConfigTest, SetValue) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: "XXX"
@@ -79,7 +79,7 @@ TEST_F(ConfigTest, SetValue) {
 
 TEST_F(ConfigTest, SetValueConnection) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: "XXX"
@@ -95,7 +95,7 @@ TEST_F(ConfigTest, SetValueConnection) {
 
 TEST_F(ConfigTest, UpdateValue) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: "XXX"
@@ -112,7 +112,7 @@ TEST_F(ConfigTest, UpdateValue) {
 
 TEST_F(ConfigTest, SetValueFromHeader) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: "%REQ(test-header)%"
@@ -127,19 +127,19 @@ TEST_F(ConfigTest, SetValueFromHeader) {
 
 TEST_F(ConfigTest, MultipleRules) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: "XXX"
   )YAML",
               R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: "YYY"
   )YAML",
               R"YAML(
-    key: bar
+    object_key: bar
     format_string:
       text_format_source:
         inline_string: "ZZZ"
@@ -156,7 +156,7 @@ TEST_F(ConfigTest, MultipleRules) {
 
 TEST_F(ConfigTest, BadValue) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: "BAD_VALUE"
@@ -169,7 +169,7 @@ TEST_F(ConfigTest, BadValue) {
 
 TEST_F(ConfigTest, MissingKey) {
   EXPECT_THROW_WITH_MESSAGE(initialize({R"YAML(
-    key: unknown_key
+    object_key: unknown_key
     format_string:
       text_format_source:
         inline_string: "XXX"
@@ -179,7 +179,7 @@ TEST_F(ConfigTest, MissingKey) {
 
 TEST_F(ConfigTest, EmptyValue) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: ""
@@ -194,7 +194,7 @@ TEST_F(ConfigTest, EmptyValue) {
 
 TEST_F(ConfigTest, EmptyValueSkip) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: ""
@@ -208,7 +208,7 @@ TEST_F(ConfigTest, EmptyValueSkip) {
 
 TEST_F(ConfigTest, SetValueUpstreamSharedOnce) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: "XXX"
@@ -229,7 +229,7 @@ TEST_F(ConfigTest, SetValueUpstreamSharedOnce) {
 
 TEST_F(ConfigTest, SetValueUpstreamSharedTransitive) {
   initialize({R"YAML(
-    key: foo
+    object_key: foo
     format_string:
       text_format_source:
         inline_string: "XXX"