Populate typed metadata in proxy protocol filter by default (envoypro…

…xy#33146) Fixes envoyproxy#32718 Signed-off-by: Kateryna Nezdolii <[email protected]>
daixiang0 · May 15, 2024 · 87514d5 · 87514d5
1 parent 190f9e0
commit 87514d5
Show file tree

Hide file tree

Showing 17 changed files with 276 additions and 5 deletions.
diff --git a/api/envoy/data/core/v3/tlv_metadata.proto b/api/envoy/data/core/v3/tlv_metadata.proto
@@ -0,0 +1,24 @@
+syntax = "proto3";
+
+package envoy.data.core.v3;
+
+import "udpa/annotations/status.proto";
+
+option java_package = "io.envoyproxy.envoy.data.core.v3";
+option java_outer_classname = "TlvMetadataProto";
+option java_multiple_files = true;
+option go_package = "github.com/envoyproxy/go-control-plane/envoy/data/core/v3;corev3";
+option (udpa.annotations.file_status).package_version_status = ACTIVE;
+
+// [#protodoc-title: Proxy Protocol Filter Typed Metadata]
+// PROXY protocol filter typed metadata.
+
+message TlvsMetadata {
+  // Typed metadata for :ref:`Proxy protocol filter <envoy_v3_api_msg_extensions.filters.listener.proxy_protocol.v3.ProxyProtocol>`, that represents a map of TLVs.
+  // Each entry in the map consists of a key which corresponds to a configured
+  // :ref:`rule key <envoy_v3_api_field_extensions.filters.listener.proxy_protocol.v3.ProxyProtocol.KeyValuePair.key>` and a value (TLV value in bytes).
+  // When runtime flag ``envoy.reloadable_features.use_typed_metadata_in_proxy_protocol_listener`` is enabled,
+  // :ref:`Proxy protocol filter <envoy_v3_api_msg_extensions.filters.listener.proxy_protocol.v3.ProxyProtocol>`
+  // will populate typed metadata and regular metadata. By default filter will populate typed and untyped metadata.
+  map<string, bytes> typed_metadata = 1;
+}
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -16,6 +16,12 @@ behavior_changes:
   change: |
     Changes the default value of ``envoy.reloadable_features.http2_use_oghttp2`` to true. This changes the codec used for HTTP/2
     requests and responses. This behavior can be reverted by setting the feature to false.
+- area: proxy_protocol
+  change: |
+    Populate typed metadata by default in proxy protocol listener. Typed metadata can be consumed as
+    :ref:`TlvsMetadata type <envoy_v3_api_msg_data.core.v3.TlvsMetadata>`.
+    This change can be temporarily disabled by setting the runtime flag
+    ``envoy.reloadable_features.use_typed_metadata_in_proxy_protocol_listener`` to ``false``.
 
 minor_behavior_changes:
 # *Changes that may cause incompatibilities for some users, but should not for most*

diff --git a/docs/root/api-v3/data/core/core.rst b/docs/root/api-v3/data/core/core.rst
@@ -6,3 +6,4 @@ Core data
   :maxdepth: 2
 
   v3/health_check_event.proto
+  v3/tlv_metadata.proto
diff --git a/envoy/network/filter.h b/envoy/network/filter.h
@@ -299,6 +299,14 @@ class ListenerFilterCallbacks {
    */
   virtual void setDynamicMetadata(const std::string& name, const ProtobufWkt::Struct& value) PURE;
 
+  /**
+   * @param name the namespace used in the metadata in reverse DNS format, for example:
+   * envoy.test.my_filter.
+   * @param value of type protobuf any to set on the namespace. A merge will be performed with new
+   * values for the same key overriding existing.
+   */
+  virtual void setDynamicTypedMetadata(const std::string& name, const ProtobufWkt::Any& value) PURE;
+
   /**
    * @return const envoy::config::core::v3::Metadata& the dynamic metadata associated with this
    * connection.

diff --git a/envoy/stream_info/stream_info.h b/envoy/stream_info/stream_info.h
@@ -838,6 +838,13 @@ class StreamInfo {
    */
   virtual void setDynamicMetadata(const std::string& name, const ProtobufWkt::Struct& value) PURE;
 
+  /**
+   * @param name the namespace used in the metadata in reverse DNS format, for example:
+   * envoy.test.my_filter.
+   * @param value of type protobuf any to set on the namespace.
+   */
+  virtual void setDynamicTypedMetadata(const std::string& name, const ProtobufWkt::Any& value) PURE;
+
   /**
    * Object on which filters can share data on a per-request basis. For singleton data objects, only
    * one filter can produce a named data object. List data objects can be updated by multiple

diff --git a/source/common/listener_manager/active_tcp_socket.cc b/source/common/listener_manager/active_tcp_socket.cc
@@ -175,6 +175,11 @@ void ActiveTcpSocket::setDynamicMetadata(const std::string& name,
   stream_info_->setDynamicMetadata(name, value);
 }
 
+void ActiveTcpSocket::setDynamicTypedMetadata(const std::string& name,
+                                              const ProtobufWkt::Any& value) {
+  stream_info_->setDynamicTypedMetadata(name, value);
+}
+
 void ActiveTcpSocket::newConnection() {
   connected_ = true;
 

diff --git a/source/common/listener_manager/active_tcp_socket.h b/source/common/listener_manager/active_tcp_socket.h
@@ -74,6 +74,7 @@ class ActiveTcpSocket : public Network::ListenerFilterManager,
   void startFilterChain() { continueFilterChain(true); }
 
   void setDynamicMetadata(const std::string& name, const ProtobufWkt::Struct& value) override;
+  void setDynamicTypedMetadata(const std::string& name, const ProtobufWkt::Any& value) override;
   envoy::config::core::v3::Metadata& dynamicMetadata() override {
     return stream_info_->dynamicMetadata();
   };

diff --git a/source/common/quic/envoy_quic_server_connection.h b/source/common/quic/envoy_quic_server_connection.h
@@ -63,6 +63,9 @@ class QuicListenerFilterManagerImpl : public Network::QuicListenerFilterManager,
   void setDynamicMetadata(const std::string& name, const ProtobufWkt::Struct& value) override {
     stream_info_.setDynamicMetadata(name, value);
   }
+  void setDynamicTypedMetadata(const std::string& name, const ProtobufWkt::Any& value) override {
+    stream_info_.setDynamicTypedMetadata(name, value);
+  }
   envoy::config::core::v3::Metadata& dynamicMetadata() override {
     return stream_info_.dynamicMetadata();
   };

diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
@@ -101,6 +101,7 @@ RUNTIME_GUARD(envoy_reloadable_features_upstream_allow_connect_with_2xx);
 RUNTIME_GUARD(envoy_reloadable_features_upstream_remote_address_use_connection);
 RUNTIME_GUARD(envoy_reloadable_features_upstream_wait_for_response_headers_before_disabling_read);
 RUNTIME_GUARD(envoy_reloadable_features_use_http3_header_normalisation);
+RUNTIME_GUARD(envoy_reloadable_features_use_typed_metadata_in_proxy_protocol_listener);
 RUNTIME_GUARD(envoy_reloadable_features_validate_connect);
 RUNTIME_GUARD(envoy_reloadable_features_validate_grpc_header_before_log_grpc_status);
 RUNTIME_GUARD(envoy_reloadable_features_validate_upstream_headers);

diff --git a/source/common/stream_info/stream_info_impl.h b/source/common/stream_info/stream_info_impl.h
@@ -292,6 +292,10 @@ struct StreamInfoImpl : public StreamInfo {
     (*metadata_.mutable_filter_metadata())[name].MergeFrom(value);
   };
 
+  void setDynamicTypedMetadata(const std::string& name, const ProtobufWkt::Any& value) override {
+    (*metadata_.mutable_typed_filter_metadata())[name].MergeFrom(value);
+  }
+
   const FilterStateSharedPtr& filterState() override { return filter_state_; }
   const FilterState& filterState() const override { return *filter_state_; }
 

diff --git a/source/extensions/filters/listener/proxy_protocol/BUILD b/source/extensions/filters/listener/proxy_protocol/BUILD
@@ -33,8 +33,11 @@ envoy_cc_library(
         "//source/common/network:address_lib",
         "//source/common/network:proxy_protocol_filter_state_lib",
         "//source/common/network:utility_lib",
+        "//source/common/protobuf:utility_lib",
+        "//source/common/runtime:runtime_features_lib",
         "//source/extensions/common/proxy_protocol:proxy_protocol_header_lib",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
+        "@envoy_api//envoy/data/core/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/filters/listener/proxy_protocol/v3:pkg_cc_proto",
     ],
 )

diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc
@@ -10,6 +10,7 @@
 #include "envoy/common/exception.h"
 #include "envoy/common/platform.h"
 #include "envoy/config/core/v3/proxy_protocol.pb.h"
+#include "envoy/data/core/v3/tlv_metadata.pb.h"
 #include "envoy/event/dispatcher.h"
 #include "envoy/network/listen_socket.h"
 #include "envoy/stats/scope.h"
@@ -26,6 +27,7 @@
 #include "source/common/network/proxy_protocol_filter_state.h"
 #include "source/common/network/utility.h"
 #include "source/common/protobuf/utility.h"
+#include "source/common/runtime/runtime_features.h"
 #include "source/extensions/common/proxy_protocol/proxy_protocol_header.h"
 
 using envoy::config::core::v3::ProxyProtocolConfig;
@@ -533,15 +535,38 @@ bool Filter::parseTlvs(const uint8_t* buf, size_t len) {
     absl::string_view tlv_value(reinterpret_cast<char const*>(buf + idx), tlv_value_length);
     auto key_value_pair = config_->isTlvTypeNeeded(tlv_type);
     if (nullptr != key_value_pair) {
+      std::string metadata_key = key_value_pair->metadata_namespace().empty()
+                                     ? "envoy.filters.listener.proxy_protocol"
+                                     : key_value_pair->metadata_namespace();
+      if (Runtime::runtimeFeatureEnabled(
+              "envoy.reloadable_features.use_typed_metadata_in_proxy_protocol_listener")) {
+        auto& typed_filter_metadata = (*cb_->dynamicMetadata().mutable_typed_filter_metadata());
+
+        const auto typed_proxy_filter_metadata = typed_filter_metadata.find(metadata_key);
+        envoy::data::core::v3::TlvsMetadata tlvs_metadata;
+        auto status = absl::OkStatus();
+        if (typed_proxy_filter_metadata != typed_filter_metadata.end()) {
+          status = MessageUtil::unpackTo(typed_proxy_filter_metadata->second, tlvs_metadata);
+        }
+        if (!status.ok()) {
+          ENVOY_LOG_PERIODIC(warn, std::chrono::seconds(1),
+                             "proxy_protocol: Failed to unpack typed metadata for TLV type ",
+                             tlv_type);
+        } else {
+          Protobuf::BytesValue tlv_byte_value;
+          tlv_byte_value.set_value(tlv_value.data(), tlv_value.size());
+          tlvs_metadata.mutable_typed_metadata()->insert(
+              {key_value_pair->key(), tlv_byte_value.value()});
+          ProtobufWkt::Any typed_metadata;
+          typed_metadata.PackFrom(tlvs_metadata);
+          cb_->setDynamicTypedMetadata(metadata_key, typed_metadata);
+        }
+      }
+      // Always populate untyped metadata for backwards compatibility.
       ProtobufWkt::Value metadata_value;
       // Sanitize any non utf8 characters.
       auto sanitised_tlv_value = MessageUtil::sanitizeUtf8String(tlv_value);
       metadata_value.set_string_value(sanitised_tlv_value.data(), sanitised_tlv_value.size());
-
-      std::string metadata_key = key_value_pair->metadata_namespace().empty()
-                                     ? "envoy.filters.listener.proxy_protocol"
-                                     : key_value_pair->metadata_namespace();
-
       ProtobufWkt::Struct metadata(
           (*cb_->dynamicMetadata().mutable_filter_metadata())[metadata_key]);
       metadata.mutable_fields()->insert({key_value_pair->key(), metadata_value});

diff --git a/test/extensions/filters/listener/proxy_protocol/BUILD b/test/extensions/filters/listener/proxy_protocol/BUILD
@@ -35,9 +35,11 @@ envoy_extension_cc_test(
         "//test/mocks/server:listener_factory_context_mocks",
         "//test/test_common:environment_lib",
         "//test/test_common:network_utility_lib",
+        "//test/test_common:test_runtime_lib",
         "//test/test_common:threadsafe_singleton_injector_lib",
         "//test/test_common:utility_lib",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
+        "@envoy_api//envoy/data/core/v3:pkg_cc_proto",
     ],
 )
 
@@ -81,6 +83,7 @@ envoy_extension_cc_test(
         "//source/extensions/filters/listener/proxy_protocol:config",
         "//source/extensions/filters/network/tcp_proxy:config",
         "//test/integration:http_integration_lib",
+        "//test/test_common:test_runtime_lib",
         "//test/test_common:utility_lib",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",

diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_proto_integration_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_proto_integration_test.cc
@@ -9,6 +9,7 @@
 
 #include "test/test_common/network_utility.h"
 #include "test/test_common/printers.h"
+#include "test/test_common/test_runtime.h"
 #include "test/test_common/utility.h"
 
 #include "fmt/format.h"
Original file line number	Diff line number	Diff line change
Expand Up		@@ -6,3 +6,4 @@ Core data
		:maxdepth: 2

		v3/health_check_event.proto
		v3/tlv_metadata.proto