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

router: Add ability of custom headers to rely on per-request data #4219

Merged
merged 22 commits into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion docs/root/configuration/http_conn_man/headers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,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 All @@ -502,4 +508,4 @@ Supported variable names are:
- header:
key: "x-request-start"
value: "%START_TIME(%s.%3f)%"
append: true
append: true
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 @@ -303,6 +303,15 @@ class RequestInfo {
* the same key overriding existing.
*/
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;
};

} // namespace RequestInfo
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_; }

const SystemTime start_time_;
const MonotonicTime start_time_monotonic_;

Expand All @@ -197,6 +201,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 @@ -224,6 +224,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 @@ -232,3 +235,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_; }

SystemTime start_time_;
MonotonicTime start_time_monotonic_;

Expand All @@ -178,6 +184,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_{};
};

} // namespace Envoy
6 changes: 3 additions & 3 deletions test/common/request_info/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ load(
envoy_package()

envoy_cc_test(
name = "dynamic_metadata_impl_test",
srcs = ["dynamic_metadata_impl_test.cc"],
name = "filter_state_impl_test",
srcs = ["filter_state_impl_test.cc"],
deps = [
"//source/common/request_info:dynamic_metadata_lib",
"//source/common/request_info:filter_state_lib",
"//test/test_common:utility_lib",
],
)
Expand Down
Loading