From 97b059c4aa28644682b8856bf2f5559412e9d829 Mon Sep 17 00:00:00 2001 From: Ariane van der Steldt Date: Tue, 5 Nov 2024 14:06:18 +0000 Subject: [PATCH] Move header fields to its own message. Signed-off-by: Ariane van der Steldt --- .../http/ip_tagging/v3/ip_tagging.proto | 69 ++++++++++--------- .../http/ip_tagging/ip_tagging_filter.cc | 13 ++-- .../http/ip_tagging/ip_tagging_filter.h | 3 +- .../http/ip_tagging/ip_tagging_filter_test.cc | 23 ++++--- 4 files changed, 61 insertions(+), 47 deletions(-) diff --git a/api/envoy/extensions/filters/http/ip_tagging/v3/ip_tagging.proto b/api/envoy/extensions/filters/http/ip_tagging/v3/ip_tagging.proto index f98e5a7196bd..35ec83f2c7e1 100644 --- a/api/envoy/extensions/filters/http/ip_tagging/v3/ip_tagging.proto +++ b/api/envoy/extensions/filters/http/ip_tagging/v3/ip_tagging.proto @@ -18,7 +18,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // IP tagging :ref:`configuration overview `. // [#extension: envoy.filters.http.ip_tagging] -// [#next-free-field: 7] +// [#next-free-field: 6] message IPTagging { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.ip_tagging.v2.IPTagging"; @@ -40,21 +40,6 @@ message IPTagging { EXTERNAL = 2; } - // Describes how to apply the tags to the headers. - enum HeaderAction { - // (DEFAULT) The header specified in :ref:`ip_tag_header ` - // will be dropped, before the tags are applied. The incoming header will be "sanitized" regardless of whether the request is internal or external. - // - // Note that the header will be visible unsanitized to any filters that are invoked before the ip-tag-header filter, unless it has an *x-envoy* prefix. - SANITIZE = 0; - - // Tags will be appended to the header specified in - // :ref:`ip_tag_header `. - // - // Please note that this could cause the header to retain values set by the http client regardless of whether the request is internal or external. - APPEND_IF_EXISTS_OR_ADD = 1; - } - // Supplies the IP tag name and the IP address subnets. message IPTag { option (udpa.annotations.versioning).previous_message_type = @@ -68,22 +53,37 @@ message IPTagging { repeated config.core.v3.CidrRange ip_list = 2; } - // Optional header to use for ip-tagging instead of using - // default header ``x-envoy-ip-tags``. - // - // This header will be sanitized based on the config in - // :ref:`ip_tag_header ` - // rather than the defaults for x-envoy prefixed headers. - string ip_tag_header = 5 - [(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME strict: false}]; - - // Control if the header in :ref:`ip_tag_header ` - // will be sanitized, or be appended to. - // - // This is ignored if :ref:`ip_tag_header ` is empty. - // - // Default: *SANITIZE*. - HeaderAction ip_tag_header_action = 6; + // Specify to which header the tags will be written. + message IpTagHeader { + // Header to use for ip-tagging. + // + // This header will be sanitized based on the config in + // :ref:`ip_tag_header ` + // rather than the defaults for x-envoy prefixed headers. + string header = 1 + [(validate.rules).string = {min_len: 1 well_known_regex: HTTP_HEADER_NAME strict: false}]; + + // Describes how to apply the tags to the headers. + enum HeaderAction { + // (DEFAULT) The header specified in :ref:`ip_tag_header ` + // will be dropped, before the tags are applied. The incoming header will be "sanitized" regardless of whether the request is internal or external. + // + // Note that the header will be visible unsanitized to any filters that are invoked before the ip-tag-header filter, unless it has an *x-envoy* prefix. + SANITIZE = 0; + + // Tags will be appended to the header specified in + // :ref:`ip_tag_header `. + // + // Please note that this could cause the header to retain values set by the http client regardless of whether the request is internal or external. + APPEND_IF_EXISTS_OR_ADD = 1; + } + + // Control if the :ref:`header ` + // will be sanitized, or be appended to. + // + // Default: *SANITIZE*. + HeaderAction action = 2; + } // The type of request the filter should apply to. RequestType request_type = 1 [(validate.rules).enum = {defined_only: true}]; @@ -92,4 +92,9 @@ message IPTagging { // Tracked by issue https://github.com/envoyproxy/envoy/issues/2695] // The set of IP tags for the filter. repeated IPTag ip_tags = 4 [(validate.rules).repeated = {min_items: 1}]; + + // Specify to which header the tags will be written. + // + // If left unspecified, the tags will be appended to the ``x-envoy-ip-tags`` header. + optional IpTagHeader ip_tag_header = 5; } diff --git a/source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc b/source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc index c57e2fd4df39..c680230b2e17 100644 --- a/source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc +++ b/source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc @@ -20,8 +20,11 @@ IpTaggingFilterConfig::IpTaggingFilterConfig( stat_name_set_(scope.symbolTable().makeSet("IpTagging")), stats_prefix_(stat_name_set_->add(stat_prefix + "ip_tagging")), no_hit_(stat_name_set_->add("no_hit")), total_(stat_name_set_->add("total")), - unknown_tag_(stat_name_set_->add("unknown_tag.hit")), ip_tag_header_(config.ip_tag_header()), - ip_tag_header_action_(config.ip_tag_header_action()) { + unknown_tag_(stat_name_set_->add("unknown_tag.hit")), + ip_tag_header_(config.has_ip_tag_header() ? config.ip_tag_header().header() : ""), + ip_tag_header_action_(config.has_ip_tag_header() + ? config.ip_tag_header().action() + : HeaderAction::IPTagging_IpTagHeader_HeaderAction_SANITIZE) { // Once loading IP tags from a file system is supported, the restriction on the size // of the set should be removed and observability into what tags are loaded needs @@ -116,7 +119,7 @@ void IpTaggingFilter::applyTags(Http::RequestHeaderMap& headers, if (tags.empty()) { bool maybe_sanitize = - config_->ipTagHeaderAction() == HeaderAction::IPTagging_HeaderAction_SANITIZE; + config_->ipTagHeaderAction() == HeaderAction::IPTagging_IpTagHeader_HeaderAction_SANITIZE; if (header_name.has_value() && maybe_sanitize) { if (headers.remove(header_name.value()) != 0) { // We must clear the route cache in case it held a decision based on the now-removed header. @@ -135,10 +138,10 @@ void IpTaggingFilter::applyTags(Http::RequestHeaderMap& headers, } else { switch (config_->ipTagHeaderAction()) { PANIC_ON_PROTO_ENUM_SENTINEL_VALUES; - case HeaderAction::IPTagging_HeaderAction_SANITIZE: + case HeaderAction::IPTagging_IpTagHeader_HeaderAction_SANITIZE: headers.setCopy(header_name.value(), tags_join); break; - case HeaderAction::IPTagging_HeaderAction_APPEND_IF_EXISTS_OR_ADD: + case HeaderAction::IPTagging_IpTagHeader_HeaderAction_APPEND_IF_EXISTS_OR_ADD: headers.appendCopy(header_name.value(), tags_join); break; } diff --git a/source/extensions/filters/http/ip_tagging/ip_tagging_filter.h b/source/extensions/filters/http/ip_tagging/ip_tagging_filter.h index dd216e381603..f055c962877c 100644 --- a/source/extensions/filters/http/ip_tagging/ip_tagging_filter.h +++ b/source/extensions/filters/http/ip_tagging/ip_tagging_filter.h @@ -32,7 +32,8 @@ enum class FilterRequestType { INTERNAL, EXTERNAL, BOTH }; */ class IpTaggingFilterConfig { public: - using HeaderAction = envoy::extensions::filters::http::ip_tagging::v3::IPTagging::HeaderAction; + 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, diff --git a/test/extensions/filters/http/ip_tagging/ip_tagging_filter_test.cc b/test/extensions/filters/http/ip_tagging/ip_tagging_filter_test.cc index 65da28f8ae50..66890eb0b690 100644 --- a/test/extensions/filters/http/ip_tagging/ip_tagging_filter_test.cc +++ b/test/extensions/filters/http/ip_tagging/ip_tagging_filter_test.cc @@ -195,7 +195,8 @@ TEST_F(IpTaggingFilterTest, AppendEntry) { TEST_F(IpTaggingFilterTest, ReplaceAlternateHeaderWhenActionIsDefaulted) { const std::string internal_request_yaml = R"EOF( request_type: internal -ip_tag_header: x-envoy-optional-header +ip_tag_header: + header: x-envoy-optional-header ip_tags: - ip_tag_name: internal_request_with_optional_header ip_list: @@ -227,8 +228,9 @@ ip_tag_header: x-envoy-optional-header TEST_F(IpTaggingFilterTest, ReplaceAlternateHeader) { const std::string internal_request_yaml = R"EOF( request_type: internal -ip_tag_header: x-envoy-optional-header -ip_tag_header_action: SANITIZE +ip_tag_header: + header: x-envoy-optional-header + action: SANITIZE ip_tags: - ip_tag_name: internal_request_with_optional_header ip_list: @@ -256,8 +258,9 @@ ip_tag_header_action: SANITIZE TEST_F(IpTaggingFilterTest, ClearAlternateHeaderWhenUnmatchedAndSanitized) { const std::string internal_request_yaml = R"EOF( request_type: internal -ip_tag_header: x-envoy-optional-header -ip_tag_header_action: SANITIZE +ip_tag_header: + header: x-envoy-optional-header + action: SANITIZE ip_tags: - ip_tag_name: internal_request_with_optional_header ip_list: @@ -284,8 +287,9 @@ ip_tag_header_action: SANITIZE TEST_F(IpTaggingFilterTest, AppendForwardAlternateHeader) { const std::string internal_request_yaml = R"EOF( request_type: internal -ip_tag_header: x-envoy-optional-header -ip_tag_header_action: APPEND_IF_EXISTS_OR_ADD +ip_tag_header: + header: x-envoy-optional-header + action: APPEND_IF_EXISTS_OR_ADD ip_tags: - ip_tag_name: internal_request_with_optional_header ip_list: @@ -313,8 +317,9 @@ ip_tag_header_action: APPEND_IF_EXISTS_OR_ADD TEST_F(IpTaggingFilterTest, RetainAlternateHeaderWhenUnmatchedAndAppendForwarded) { const std::string internal_request_yaml = R"EOF( request_type: internal -ip_tag_header: x-envoy-optional-header -ip_tag_header_action: APPEND_IF_EXISTS_OR_ADD +ip_tag_header: + header: x-envoy-optional-header + action: APPEND_IF_EXISTS_OR_ADD ip_tags: - ip_tag_name: internal_request_with_optional_header ip_list: