Skip to content

Commit

Permalink
route: implementation of HeaderAppendAction (#22723)
Browse files Browse the repository at this point in the history
The HeaderAppendAction was added in the #18246. But the implementation never done. This PR try to complete this feature.

Further work of #18246. To close #22713.

Risk Level: Low.
Testing: Unit Test.

Signed-off-by: wbpcode <wangbaiping@corp.netease.com>
  • Loading branch information
wbpcode committed Aug 26, 2022
1 parent 23a9a68 commit 5183dbf
Show file tree
Hide file tree
Showing 29 changed files with 491 additions and 228 deletions.
13 changes: 9 additions & 4 deletions api/envoy/config/core/v3/base.proto
Expand Up @@ -334,7 +334,7 @@ message HeaderValueOption {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.core.HeaderValueOption";

// [#not-implemented-hide:] Describes the supported actions types for header append action.
// Describes the supported actions types for header append action.
enum HeaderAppendAction {
// This action will append the specified value to the existing values if the header
// already exists. If the header doesn't exist then this will add the header with
Expand All @@ -356,10 +356,15 @@ message HeaderValueOption {

// Should the value be appended? If true (default), the value is appended to
// existing values. Otherwise it replaces any existing values.
google.protobuf.BoolValue append = 2;
// This field is deprecated and please use
// :ref:`append_action <envoy_v3_api_field_config.core.v3.HeaderValueOption.append_action>` as replacement.
google.protobuf.BoolValue append = 2
[deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"];

// [#not-implemented-hide:] Describes the action taken to append/overwrite the given value for an existing header
// or to only add this header if it's absent. Value defaults to :ref:`APPEND_IF_EXISTS_OR_ADD<envoy_v3_api_enum_value_config.core.v3.HeaderValueOption.HeaderAppendAction.APPEND_IF_EXISTS_OR_ADD>`.
// Describes the action taken to append/overwrite the given value for an existing header
// or to only add this header if it's absent.
// Value defaults to :ref:`APPEND_IF_EXISTS_OR_ADD
// <envoy_v3_api_enum_value_config.core.v3.HeaderValueOption.HeaderAppendAction.APPEND_IF_EXISTS_OR_ADD>`.
HeaderAppendAction append_action = 3 [(validate.rules).enum = {defined_only: true}];

// Is the header value allowed to be empty? If false (default), custom headers with empty values are dropped,
Expand Down
4 changes: 4 additions & 0 deletions changelogs/current.yaml
Expand Up @@ -124,3 +124,7 @@ new_features:
allow network filters other than HTTP Connection Manager to be created for QUIC listeners.
deprecated:
- area: http
change: |
deprecated :ref:`append <envoy_v3_api_field_config.core.v3.HeaderValueOption.append>` and please use
:ref:`append_action <envoy_v3_api_field_config.core.v3.HeaderValueOption.append_action>` first.
2 changes: 1 addition & 1 deletion docs/root/configuration/http/http_conn_man/headers.rst
Expand Up @@ -830,7 +830,7 @@ Supported variable names are:
- header:
key: "x-request-start"
value: "%START_TIME(%s.%3f)%"
append: true
append_action: APPEND_IF_EXISTS_OR_ADD
%RESPONSE_FLAGS%
Additional details about the response or connection, if any. Possible values and their meanings
Expand Down
2 changes: 1 addition & 1 deletion docs/root/configuration/http/http_conn_man/local_reply.rst
Expand Up @@ -33,7 +33,7 @@ Example of a LocalReplyConfig
- header:
key: "foo"
value: "bar"
append: false
append_action: OVERWRITE_IF_EXISTS_OR_ADD
status_code: 401
body:
inline_string: "not allowed"
Expand Down
Expand Up @@ -30,7 +30,7 @@ static_resources:
numerator: 100
denominator: HUNDRED
response_headers_to_add:
- append: false
- append_action: OVERWRITE_IF_EXISTS_OR_ADD
header:
key: x-local-rate-limit
value: 'true'
Expand Down
Expand Up @@ -45,7 +45,7 @@ static_resources:
numerator: 100
denominator: HUNDRED
response_headers_to_add:
- append: false
- append_action: OVERWRITE_IF_EXISTS_OR_ADD
header:
key: x-local-rate-limit
value: 'true'
Expand Down
Expand Up @@ -54,7 +54,7 @@ static_resources:
numerator: 100
denominator: HUNDRED
response_headers_to_add:
- append: false
- append_action: OVERWRITE_IF_EXISTS_OR_ADD
header:
key: x-test-rate-limit
value: 'true'
Expand Down
5 changes: 3 additions & 2 deletions envoy/http/header_map.h
Expand Up @@ -690,8 +690,9 @@ using HeaderMapPtr = std::unique_ptr<HeaderMap>;
* Wraps a set of header modifications.
*/
struct HeaderTransforms {
std::vector<std::pair<Http::LowerCaseString, std::string>> headers_to_append;
std::vector<std::pair<Http::LowerCaseString, std::string>> headers_to_overwrite;
std::vector<std::pair<Http::LowerCaseString, std::string>> headers_to_append_or_add;
std::vector<std::pair<Http::LowerCaseString, std::string>> headers_to_overwrite_or_add;
std::vector<std::pair<Http::LowerCaseString, std::string>> headers_to_add_if_absent;
std::vector<Http::LowerCaseString> headers_to_remove;
};

Expand Down
4 changes: 2 additions & 2 deletions examples/local_ratelimit/ratelimit-envoy.yaml
Expand Up @@ -41,7 +41,7 @@ static_resources:
numerator: 100
denominator: HUNDRED
response_headers_to_add:
- append: false
- append_action: OVERWRITE_IF_EXISTS_OR_ADD
header:
key: x-local-rate-limit
value: 'true'
Expand Down Expand Up @@ -90,7 +90,7 @@ static_resources:
numerator: 100
denominator: HUNDRED
response_headers_to_add:
- append: false
- append_action: OVERWRITE_IF_EXISTS_OR_ADD
header:
key: x-local-rate-limit
value: 'true'
Expand Down
5 changes: 3 additions & 2 deletions source/common/grpc/async_client_impl.cc
Expand Up @@ -19,8 +19,9 @@ AsyncClientImpl::AsyncClientImpl(Upstream::ClusterManager& cm,
TimeSource& time_source)
: cm_(cm), remote_cluster_name_(config.envoy_grpc().cluster_name()),
host_name_(config.envoy_grpc().authority()), time_source_(time_source),
metadata_parser_(
Router::HeaderParser::configure(config.initial_metadata(), /*append=*/false)) {}
metadata_parser_(Router::HeaderParser::configure(
config.initial_metadata(),
envoy::config::core::v3::HeaderValueOption::OVERWRITE_IF_EXISTS_OR_ADD)) {}

AsyncClientImpl::~AsyncClientImpl() {
ASSERT(isThreadSafe());
Expand Down
5 changes: 3 additions & 2 deletions source/common/grpc/google_async_client_impl.cc
Expand Up @@ -84,8 +84,9 @@ GoogleAsyncClientImpl::GoogleAsyncClientImpl(Event::Dispatcher& dispatcher,
target_uri_(config.google_grpc().target_uri()), scope_(scope),
per_stream_buffer_limit_bytes_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(
config.google_grpc(), per_stream_buffer_limit_bytes, DefaultBufferLimitBytes)),
metadata_parser_(
Router::HeaderParser::configure(config.initial_metadata(), /*append=*/false)) {
metadata_parser_(Router::HeaderParser::configure(
config.initial_metadata(),
envoy::config::core::v3::HeaderValueOption::OVERWRITE_IF_EXISTS_OR_ADD)) {
// We rebuild the channel each time we construct the channel. It appears that the gRPC library is
// smart enough to do connection pooling and reuse with identical channel args, so this should
// have comparable overhead to what we are doing in Grpc::AsyncClientImpl, i.e. no expensive
Expand Down
14 changes: 9 additions & 5 deletions source/common/router/config_impl.cc
Expand Up @@ -58,11 +58,15 @@ namespace {
constexpr uint32_t DEFAULT_MAX_DIRECT_RESPONSE_BODY_SIZE_BYTES = 4096;

void mergeTransforms(Http::HeaderTransforms& dest, const Http::HeaderTransforms& src) {
dest.headers_to_append.insert(dest.headers_to_append.end(), src.headers_to_append.begin(),
src.headers_to_append.end());
dest.headers_to_overwrite.insert(dest.headers_to_overwrite.end(),
src.headers_to_overwrite.begin(),
src.headers_to_overwrite.end());
dest.headers_to_append_or_add.insert(dest.headers_to_append_or_add.end(),
src.headers_to_append_or_add.begin(),
src.headers_to_append_or_add.end());
dest.headers_to_overwrite_or_add.insert(dest.headers_to_overwrite_or_add.end(),
src.headers_to_overwrite_or_add.begin(),
src.headers_to_overwrite_or_add.end());
dest.headers_to_add_if_absent.insert(dest.headers_to_add_if_absent.end(),
src.headers_to_add_if_absent.begin(),
src.headers_to_add_if_absent.end());
dest.headers_to_remove.insert(dest.headers_to_remove.end(), src.headers_to_remove.begin(),
src.headers_to_remove.end());
}
Expand Down
3 changes: 1 addition & 2 deletions source/common/router/header_formatter.cc
Expand Up @@ -236,8 +236,7 @@ StreamInfoHeaderFormatter::FieldExtractor sslConnectionInfoStringHeaderExtractor

} // namespace

StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_name, bool append)
: append_(append) {
StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_name) {
if (field_name == "PROTOCOL") {
field_extractor_ = [](const Envoy::StreamInfo::StreamInfo& stream_info) {
return Envoy::Formatter::SubstitutionFormatUtils::protocolToStringOrDefault(
Expand Down
22 changes: 5 additions & 17 deletions source/common/router/header_formatter.h
Expand Up @@ -20,12 +20,6 @@ class HeaderFormatter {
virtual ~HeaderFormatter() = default;

virtual const std::string format(const Envoy::StreamInfo::StreamInfo& stream_info) const PURE;

/**
* @return bool indicating whether the formatted header should be appended to the existing
* headers or replace any existing values for the header
*/
virtual bool append() const PURE;
};

using HeaderFormatterPtr = std::unique_ptr<HeaderFormatter>;
Expand All @@ -35,18 +29,16 @@ using HeaderFormatterPtr = std::unique_ptr<HeaderFormatter>;
*/
class StreamInfoHeaderFormatter : public HeaderFormatter {
public:
StreamInfoHeaderFormatter(absl::string_view field_name, bool append);
StreamInfoHeaderFormatter(absl::string_view field_name);

// HeaderFormatter::format
const std::string format(const Envoy::StreamInfo::StreamInfo& stream_info) const override;
bool append() const override { return append_; }

using FieldExtractor = std::function<std::string(const Envoy::StreamInfo::StreamInfo&)>;
using FormatterPtrMap = absl::node_hash_map<std::string, Envoy::Formatter::FormatterPtr>;

private:
FieldExtractor field_extractor_;
const bool append_;

// Maps a string format pattern (including field name and any command operators between
// parenthesis) to the list of FormatterProviderPtrs that are capable of formatting that pattern.
Expand All @@ -61,27 +53,25 @@ class StreamInfoHeaderFormatter : public HeaderFormatter {
*/
class PlainHeaderFormatter : public HeaderFormatter {
public:
PlainHeaderFormatter(const std::string& static_header_value, bool append)
: static_value_(static_header_value), append_(append) {}
PlainHeaderFormatter(const std::string& static_header_value)
: static_value_(static_header_value) {}

// HeaderFormatter::format
const std::string format(const Envoy::StreamInfo::StreamInfo&) const override {
return static_value_;
};
bool append() const override { return append_; }

private:
const std::string static_value_;
const bool append_;
};

/**
* A formatter that produces a value by concatenating the results of multiple HeaderFormatters.
*/
class CompoundHeaderFormatter : public HeaderFormatter {
public:
CompoundHeaderFormatter(std::vector<HeaderFormatterPtr>&& formatters, bool append)
: formatters_(std::move(formatters)), append_(append) {}
CompoundHeaderFormatter(std::vector<HeaderFormatterPtr>&& formatters)
: formatters_(std::move(formatters)) {}

// HeaderFormatter::format
const std::string format(const Envoy::StreamInfo::StreamInfo& stream_info) const override {
Expand All @@ -91,11 +81,9 @@ class CompoundHeaderFormatter : public HeaderFormatter {
}
return buf;
};
bool append() const override { return append_; }

private:
const std::vector<HeaderFormatterPtr> formatters_;
const bool append_;
};

} // namespace Router
Expand Down

0 comments on commit 5183dbf

Please sign in to comment.