Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

route: implementation of HeaderAppendAction #22723

Merged
merged 21 commits into from
Aug 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 9 additions & 4 deletions api/envoy/config/core/v3/base.proto
Original file line number Diff line number Diff line change
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.
htuch marked this conversation as resolved.
Show resolved Hide resolved
// 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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