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

ext_authz: Allow to set additional prefix for HTTP filter stats #13215

Merged
merged 7 commits into from
Sep 30, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// External Authorization :ref:`configuration overview <config_http_filters_ext_authz>`.
// [#extension: envoy.filters.http.ext_authz]

// [#next-free-field: 13]
// [#next-free-field: 14]
message ExtAuthz {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.filter.http.ext_authz.v2.ExtAuthz";
Expand Down Expand Up @@ -117,6 +117,23 @@ message ExtAuthz {
// When this field is true, Envoy will include the peer X.509 certificate, if available, in the
// :ref:`certificate<envoy_api_field_service.auth.v3.AttributeContext.Peer.certificate>`.
bool include_peer_certificate = 10;

// Optional additional prefix to use when emitting statistics. This allows to distinguish
// emitted statistics between configured *ext_authz* filters in an HTTP filter chain. For example:
//
// .. code-block:: yaml
//
// http_filters:
// - name: envoy.filters.http.ext_authz
// typed_config:
// "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz
// stat_prefix: waf # This emits ext_authz.waf.ok, ext_authz.waf.denied, etc.
// - name: envoy.filters.http.ext_authz
// typed_config:
// "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz
// stat_prefix: blocker # This emits ext_authz.blocker.ok, ext_authz.blocker.denied, etc.
//
string stat_prefix = 13;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is consistent with what we do in HCM, so seems totally fair. I could imagine having a more generic filter wrapper that included a stat prefix, but I think that's total overkill at this point.

}

// Configuration for buffering the request data.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ New Features
* dynamic_forward_proxy: added :ref:`use_tcp_for_dns_lookups<envoy_v3_api_field_extensions.common.dynamic_forward_proxy.v3.DnsCacheConfig.use_tcp_for_dns_lookups>` option to use TCP for DNS lookups in order to match the DNS options for :ref:`Clusters<envoy_v3_api_msg_config.cluster.v3.Cluster>`.
* ext_authz filter: added support for emitting dynamic metadata for both :ref:`HTTP <config_http_filters_ext_authz_dynamic_metadata>` and :ref:`network <config_network_filters_ext_authz_dynamic_metadata>` filters.
The emitted dynamic metadata is set by :ref:`dynamic metadata <envoy_v3_api_field_service.auth.v3.CheckResponse.dynamic_metadata>` field in a returned :ref:`CheckResponse <envoy_v3_api_msg_service.auth.v3.CheckResponse>`.
* ext_authz filter: added :ref:`stat_prefix <envoy_v3_api_field_extensions.filters.http.ext_authz.v3.ExtAuthz.stat_prefix>` as an optional additional prefix for the statistics emitted from `ext_authz` HTTP filter.
* ext_authz filter: added support for letting the authorization server instruct Envoy to remove headers from the original request by setting the new field :ref:`headers_to_remove <envoy_v3_api_field_service.auth.v3.OkHttpResponse.headers_to_remove>` before forwarding it to the upstream.
* ext_authz filter: added support for sending :ref:`raw bytes as request body <envoy_v3_api_field_service.auth.v3.AttributeContext.HttpRequest.raw_body>` of a gRPC check request by setting :ref:`pack_as_bytes <envoy_v3_api_field_extensions.filters.http.ext_authz.v3.BufferSettings.pack_as_bytes>` to true.
* grpc-json: support specifying `response_body` field in for `google.api.HttpBody` message.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 20 additions & 7 deletions source/extensions/filters/http/ext_authz/ext_authz.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,13 @@ class FilterConfig {
metadata_context_namespaces_(config.metadata_context_namespaces().begin(),
config.metadata_context_namespaces().end()),
include_peer_certificate_(config.include_peer_certificate()),
stats_(generateStats(stats_prefix, scope)), ext_authz_ok_(pool_.add("ext_authz.ok")),
ext_authz_denied_(pool_.add("ext_authz.denied")),
ext_authz_error_(pool_.add("ext_authz.error")),
ext_authz_timeout_(pool_.add("ext_authz.timeout")),
ext_authz_failure_mode_allowed_(pool_.add("ext_authz.failure_mode_allowed")) {}
stats_(generateStats(stats_prefix, config.stat_prefix(), scope)),
ext_authz_ok_(pool_.add(createPoolStatName(config.stat_prefix(), "ok"))),
ext_authz_denied_(pool_.add(createPoolStatName(config.stat_prefix(), "denied"))),
ext_authz_error_(pool_.add(createPoolStatName(config.stat_prefix(), "error"))),
ext_authz_timeout_(pool_.add(createPoolStatName(config.stat_prefix(), "timeout"))),
ext_authz_failure_mode_allowed_(
pool_.add(createPoolStatName(config.stat_prefix(), "failure_mode_allowed"))) {}

bool allowPartialMessage() const { return allow_partial_message_; }

Expand Down Expand Up @@ -125,11 +127,22 @@ class FilterConfig {
return Http::Code::Forbidden;
}

ExtAuthzFilterStats generateStats(const std::string& prefix, Stats::Scope& scope) {
const std::string final_prefix = prefix + "ext_authz.";
ExtAuthzFilterStats generateStats(const std::string& prefix,
const std::string& filter_stats_prefix, Stats::Scope& scope) {
const std::string final_prefix = absl::StrCat(prefix, "ext_authz.", filter_stats_prefix);
return {ALL_EXT_AUTHZ_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))};
}

// This generates ext_authz.<optional filter_stats_prefix>.name, for example: ext_authz.waf.ok
// when filter_stats_prefix is "waf", and ext_authz.ok when filter_stats_prefix is empty.
const std::string createPoolStatName(const std::string& filter_stats_prefix,
const std::string& name) {
return absl::StrCat("ext_authz",
filter_stats_prefix.empty() ? EMPTY_STRING
: absl::StrCat(".", filter_stats_prefix),
".", name);
}

const bool allow_partial_message_;
const bool failure_mode_allow_;
const bool clear_route_cache_;
Expand Down
1 change: 1 addition & 0 deletions test/extensions/filters/http/ext_authz/config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ TEST(HttpExtAuthzConfigTest, CorrectProtoGrpc) {

TEST(HttpExtAuthzConfigTest, CorrectProtoHttp) {
std::string yaml = R"EOF(
stat_prefix: "wall"
http_service:
server_uri:
uri: "ext_authz:9000"
Expand Down
51 changes: 51 additions & 0 deletions test/extensions/filters/http/ext_authz/ext_authz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,57 @@ TEST_F(HttpFilterTest, MergeConfig) {
EXPECT_EQ("value", merged_extensions.at("key"));
}

// Test that defining stat_prefix appends an additional prefix to the emitted statistics names.
TEST_F(HttpFilterTest, StatsWithPrefix) {
const std::string stat_prefix = "with_stat_prefix";
const std::string error_counter_name_with_prefix =
absl::StrCat("ext_authz.", stat_prefix, ".error");
const std::string error_counter_name_without_prefix = "ext_authz.error";

InSequence s;

initialize(fmt::format(R"EOF(
stat_prefix: "{}"
grpc_service:
envoy_grpc:
cluster_name: "ext_authz_server"
)EOF",
stat_prefix));

EXPECT_EQ(0U, filter_callbacks_.clusterInfo()
->statsScope()
.counterFromString(error_counter_name_with_prefix)
.value());
EXPECT_EQ(0U, filter_callbacks_.clusterInfo()
->statsScope()
.counterFromString(error_counter_name_without_prefix)
.value());

prepareCheck();
EXPECT_CALL(*client_, check(_, _, _, _, _))
.WillOnce(
WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
request_callbacks_ = &callbacks;
})));
EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark,
filter_->decodeHeaders(request_headers_, false));
EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
EXPECT_CALL(filter_callbacks_, encodeHeaders_(_, _)).Times(1);

Filters::Common::ExtAuthz::Response response{};
response.status = Filters::Common::ExtAuthz::CheckStatus::Error;
request_callbacks_->onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
EXPECT_EQ(1U, filter_callbacks_.clusterInfo()
->statsScope()
.counterFromString(error_counter_name_with_prefix)
.value());
// The one without an additional prefix is not incremented, since it is not "defined".
EXPECT_EQ(0U, filter_callbacks_.clusterInfo()
->statsScope()
.counterFromString(error_counter_name_without_prefix)
.value());
}

// Test when failure_mode_allow is NOT set and the response from the authorization service is Error
// that the request is not allowed to continue.
TEST_F(HttpFilterTest, ErrorFailClose) {
Expand Down