Skip to content

Commit

Permalink
router: Add ability of custom headers to rely on per-request data (#4219
Browse files Browse the repository at this point in the history
)

Description: This PR adds an per-request state object of type FilterState (was DynamicMetadata) to the RequestInfo and makes objects of type StringAccessor stored in that object available to custom headers.

Risk Level: Low (only adding functionality)
Testing: Unit tests.
Docs Changes: Added documentation for the %PER_REQUEST_STATE% variable for custom headers.
Release Notes: Added %PER_REQUEST_STATE% to variables available for custom headers.

Fixes #3559 #151
  • Loading branch information
curiouserrandy authored and zuercher committed Aug 28, 2018
1 parent 68d20b4 commit 329e591
Show file tree
Hide file tree
Showing 23 changed files with 474 additions and 196 deletions.
6 changes: 6 additions & 0 deletions docs/root/configuration/http_conn_man/headers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,12 @@ Supported variable names are:
namespace and key(s) are specified as a JSON array of strings. Finally, percent symbols in the
parameters **do not** need to be escaped by doubling them.

%PER_REQUEST_STATE(reverse.dns.data.name)%
Populates the header with values set on the request info perRequestState() object. To be
usable in custom request/response headers, these values must be of type
Envoy::Router::StringAccessor. These values should be named in standard reverse DNS style,
identifying the organization that created the value and ending in a unique name for the data.

%START_TIME%
Request start time. START_TIME can be customized with specifiers as specified in
:ref:`access log format rules<config_access_log_format_start_time>`.
Expand Down
6 changes: 3 additions & 3 deletions include/envoy/request_info/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ envoy_cc_library(
hdrs = ["request_info.h"],
external_deps = ["abseil_optional"],
deps = [
":dynamic_metadata_interface",
":filter_state_interface",
"//include/envoy/common:time_interface",
"//include/envoy/http:protocol_interface",
"//include/envoy/upstream:upstream_interface",
],
)

envoy_cc_library(
name = "dynamic_metadata_interface",
hdrs = ["dynamic_metadata.h"],
name = "filter_state_interface",
hdrs = ["filter_state.h"],
external_deps = ["abseil_optional"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@
namespace Envoy {
namespace RequestInfo {

class DynamicMetadata {
class FilterState {
public:
class Object {
public:
virtual ~Object(){};
};

virtual ~DynamicMetadata(){};
virtual ~FilterState(){};

/**
* @param data_name the name of the data being set.
* @param data an owning pointer to the data to be stored.
* Note that it is an error to call setData() twice with the same data_name; this is to
* enforce a single authoritative source for each piece of data stored in DynamicMetadata.
* enforce a single authoritative source for each piece of data stored in FilterState.
*/
virtual void setData(absl::string_view data_name, std::unique_ptr<Object>&& data) PURE;

Expand Down
11 changes: 10 additions & 1 deletion include/envoy/request_info/request_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include "envoy/common/pure.h"
#include "envoy/common/time.h"
#include "envoy/http/protocol.h"
#include "envoy/request_info/dynamic_metadata.h"
#include "envoy/request_info/filter_state.h"
#include "envoy/upstream/upstream.h"

#include "absl/types/optional.h"
Expand Down Expand Up @@ -304,6 +304,15 @@ class RequestInfo {
*/
virtual void setDynamicMetadata(const std::string& name, const ProtobufWkt::Struct& value) PURE;

/**
* Object on which filters can share data on a per-request basis.
* Only one filter can produce a named data object, but it may be
* consumed by many other objects.
* @return the per-request state associated with this request.
*/
virtual FilterState& perRequestState() PURE;
virtual const FilterState& perRequestState() const PURE;

/**
* @param SNI value requested
*/
Expand Down
9 changes: 9 additions & 0 deletions include/envoy/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,12 @@ envoy_cc_library(
hdrs = ["shadow_writer.h"],
deps = ["//include/envoy/http:message_interface"],
)

envoy_cc_library(
name = "string_accessor_interface",
hdrs = ["string_accessor.h"],
external_deps = ["abseil_optional"],
deps = [
"//include/envoy/request_info:filter_state_interface",
],
)
25 changes: 25 additions & 0 deletions include/envoy/router/string_accessor.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#pragma once

#include "envoy/common/pure.h"
#include "envoy/request_info/filter_state.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Router {

/**
* Contains a string in a form which is usable with FilterState and
* allows lazy evaluation if needed. All values meant to be accessible to the
* custom request/response header mechanism must use this type.
*/
class StringAccessor : public RequestInfo::FilterState::Object {
public:
/**
* @return the string the accessor represents.
*/
virtual absl::string_view asString() const PURE;
};

} // namespace Router
} // namespace Envoy
9 changes: 5 additions & 4 deletions source/common/request_info/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,18 @@ envoy_cc_library(
name = "request_info_lib",
hdrs = ["request_info_impl.h"],
deps = [
":filter_state_lib",
"//include/envoy/request_info:request_info_interface",
"//source/common/common:assert_lib",
],
)

envoy_cc_library(
name = "dynamic_metadata_lib",
srcs = ["dynamic_metadata_impl.cc"],
hdrs = ["dynamic_metadata_impl.h"],
name = "filter_state_lib",
srcs = ["filter_state_impl.cc"],
hdrs = ["filter_state_impl.h"],
deps = [
"//include/envoy/request_info:dynamic_metadata_interface",
"//include/envoy/request_info:filter_state_interface",
],
)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,33 +1,32 @@
#include "common/request_info/dynamic_metadata_impl.h"
#include "common/request_info/filter_state_impl.h"

#include "envoy/common/exception.h"

namespace Envoy {
namespace RequestInfo {

void DynamicMetadataImpl::setData(absl::string_view data_name, std::unique_ptr<Object>&& data) {
void FilterStateImpl::setData(absl::string_view data_name, std::unique_ptr<Object>&& data) {
// TODO(Google): Remove string conversion when fixed internally.
const std::string name(data_name);
if (data_storage_.find(name) != data_storage_.end()) {
throw EnvoyException("DynamicMetadata::setData<T> called twice with same name.");
throw EnvoyException("FilterState::setData<T> called twice with same name.");
}
// absl::string_view will not convert to std::string without an explicit case; see
// https://github.com/abseil/abseil-cpp/blob/master/absl/strings/string_view.h#L328
data_storage_[name] = std::move(data);
}

bool DynamicMetadataImpl::hasDataWithName(absl::string_view data_name) const {
bool FilterStateImpl::hasDataWithName(absl::string_view data_name) const {
// TODO(Google): Remove string conversion when fixed internally.
return data_storage_.count(std::string(data_name)) > 0;
}

const DynamicMetadata::Object*
DynamicMetadataImpl::getDataGeneric(absl::string_view data_name) const {
const FilterState::Object* FilterStateImpl::getDataGeneric(absl::string_view data_name) const {
// TODO(Google): Remove string conversion when fixed internally.
const auto& it = data_storage_.find(std::string(data_name));

if (it == data_storage_.end()) {
throw EnvoyException("DynamicMetadata::getData<T> called for unknown data name.");
throw EnvoyException("FilterState::getData<T> called for unknown data name.");
}
return it->second.get();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@

#include <map>

#include "envoy/request_info/dynamic_metadata.h"
#include "envoy/request_info/filter_state.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace RequestInfo {

class DynamicMetadataImpl : public DynamicMetadata {
class FilterStateImpl : public FilterState {
public:
// DynamicMetadata
// FilterState
void setData(absl::string_view data_name, std::unique_ptr<Object>&& data) override;
bool hasDataWithName(absl::string_view) const override;
const Object* getDataGeneric(absl::string_view data_name) const override;
Expand Down
5 changes: 5 additions & 0 deletions source/common/request_info/request_info_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "envoy/request_info/request_info.h"

#include "common/common/assert.h"
#include "common/request_info/filter_state_impl.h"

namespace Envoy {
namespace RequestInfo {
Expand Down Expand Up @@ -178,6 +179,9 @@ struct RequestInfoImpl : public RequestInfo {
(*metadata_.mutable_filter_metadata())[name].MergeFrom(value);
};

FilterState& perRequestState() override { return per_request_state_; }
const FilterState& perRequestState() const override { return per_request_state_; }

void setRequestedServerName(absl::string_view requested_server_name) override {
requested_server_name_ = std::string(requested_server_name);
}
Expand All @@ -203,6 +207,7 @@ struct RequestInfoImpl : public RequestInfo {
bool hc_request_{};
const Router::RouteEntry* route_entry_{};
envoy::api::v2::core::Metadata metadata_{};
FilterStateImpl per_request_state_{};

private:
uint64_t bytes_received_{};
Expand Down
11 changes: 11 additions & 0 deletions source/common/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ envoy_cc_library(
hdrs = ["header_formatter.h"],
external_deps = ["abseil_optional"],
deps = [
"//include/envoy/request_info:filter_state_interface",
"//include/envoy/request_info:request_info_interface",
"//include/envoy/router:string_accessor_interface",
"//source/common/access_log:access_log_formatter_lib",
"//source/common/common:minimal_logger_lib",
"//source/common/common:utility_lib",
Expand All @@ -233,3 +236,11 @@ envoy_cc_library(
"//source/common/json:json_loader_lib",
],
)

envoy_cc_library(
name = "string_accessor_lib",
hdrs = ["string_accessor_impl.h"],
deps = [
"//include/envoy/router:string_accessor_interface",
],
)
51 changes: 51 additions & 0 deletions source/common/router/header_formatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include <string>

#include "envoy/router/string_accessor.h"

#include "common/access_log/access_log_formatter.h"
#include "common/common/fmt.h"
#include "common/common/logger.h"
Expand Down Expand Up @@ -32,6 +34,13 @@ std::string formatUpstreamMetadataParseException(absl::string_view params,
params, reason);
}

std::string formatPerRequestStateParseException(absl::string_view params) {
return fmt::format("Invalid header configuration. Expected format "
"PER_REQUEST_STATE(<data_name>), actual format "
"PER_REQUEST_STATE{}",
params);
}

// Parses the parameters for UPSTREAM_METADATA and returns a function suitable for accessing the
// specified metadata from an RequestInfo::RequestInfo. Expects a string formatted as:
// (["a", "b", "c"])
Expand Down Expand Up @@ -114,6 +123,45 @@ parseUpstreamMetadataField(absl::string_view params_str) {
};
}

// Parses the parameters for PER_REQUEST_STATE and returns a function suitable for accessing the
// specified metadata from an RequestInfo::RequestInfo. Expects a string formatted as:
// (<state_name>)
// The state name is expected to be in reverse DNS format, though this is not enforced by
// this function.
std::function<std::string(const Envoy::RequestInfo::RequestInfo&)>
parsePerRequestStateField(absl::string_view param_str) {
absl::string_view modified_param_str = StringUtil::trim(param_str);
if (modified_param_str.empty() || modified_param_str.front() != '(' ||
modified_param_str.back() != ')') {
throw EnvoyException(formatPerRequestStateParseException(param_str));
}
modified_param_str = modified_param_str.substr(1, modified_param_str.size() - 2); // trim parens
if (modified_param_str.size() == 0) {
throw EnvoyException(formatPerRequestStateParseException(param_str));
}

std::string param(modified_param_str);
return [param](const Envoy::RequestInfo::RequestInfo& request_info) -> std::string {
const Envoy::RequestInfo::FilterState& per_request_state = request_info.perRequestState();

// No such value means don't output anything.
if (!per_request_state.hasDataWithName(param)) {
return std::string();
}

// Value exists but isn't string accessible is a contract violation; throw an error.
if (!per_request_state.hasData<StringAccessor>(param)) {
ENVOY_LOG_MISC(debug,
"Invalid header information: PER_REQUEST_STATE value \"{}\" "
"exists but is not string accessible",
param);
return std::string();
}

return std::string(per_request_state.getData<StringAccessor>(param).asString());
};
}

} // namespace

RequestInfoHeaderFormatter::RequestInfoHeaderFormatter(absl::string_view field_name, bool append)
Expand Down Expand Up @@ -155,6 +203,9 @@ RequestInfoHeaderFormatter::RequestInfoHeaderFormatter(absl::string_view field_n
} else if (field_name.find("UPSTREAM_METADATA") == 0) {
field_extractor_ =
parseUpstreamMetadataField(field_name.substr(STATIC_STRLEN("UPSTREAM_METADATA")));
} else if (field_name.find("PER_REQUEST_STATE") == 0) {
field_extractor_ =
parsePerRequestStateField(field_name.substr(STATIC_STRLEN("PER_REQUEST_STATE")));
} else {
throw EnvoyException(fmt::format("field '{}' not supported as custom header", field_name));
}
Expand Down
20 changes: 20 additions & 0 deletions source/common/router/string_accessor_impl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#pragma once

#include "envoy/router/string_accessor.h"

namespace Envoy {
namespace Router {

class StringAccessorImpl : public StringAccessor {
public:
StringAccessorImpl(absl::string_view value) : value_(value) {}

// StringAccessor
absl::string_view asString() const override { return value_; }

private:
std::string value_;
};

} // namespace Router
} // namespace Envoy
7 changes: 7 additions & 0 deletions test/common/access_log/test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "envoy/request_info/request_info.h"

#include "common/common/assert.h"
#include "common/request_info/filter_state_impl.h"

namespace Envoy {

Expand Down Expand Up @@ -156,6 +157,11 @@ class TestRequestInfo : public RequestInfo::RequestInfo {
(*metadata_.mutable_filter_metadata())[name].MergeFrom(value);
};

const Envoy::RequestInfo::FilterState& perRequestState() const override {
return per_request_state_;
}
Envoy::RequestInfo::FilterState& perRequestState() override { return per_request_state_; }

void setRequestedServerName(const absl::string_view requested_server_name) override {
requested_server_name_ = std::string(requested_server_name);
}
Expand Down Expand Up @@ -184,6 +190,7 @@ class TestRequestInfo : public RequestInfo::RequestInfo {
Network::Address::InstanceConstSharedPtr downstream_remote_address_;
const Router::RouteEntry* route_entry_{};
envoy::api::v2::core::Metadata metadata_{};
Envoy::RequestInfo::FilterStateImpl per_request_state_{};
std::string requested_server_name_;
};

Expand Down
Loading

0 comments on commit 329e591

Please sign in to comment.