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

network: Enable the Kill Request filter to honor Route-level configur… #14944

Merged
merged 8 commits into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions source/extensions/filters/http/kill_request/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ envoy_cc_library(
"//source/common/http:header_utility_lib",
"//source/common/http:headers_lib",
"//source/common/protobuf:utility_lib",
"//source/extensions/filters/http:well_known_names",
"@envoy_api//envoy/extensions/filters/http/kill_request/v3:pkg_cc_proto",
],
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
#include "extensions/filters/http/kill_request/kill_request_filter.h"

#include <csignal>
#include <string>

#include "common/protobuf/utility.h"

#include "extensions/filters/http/well_known_names.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace KillRequest {

using ::envoy::extensions::filters::http::kill_request::v3::KillRequest;

KillSettings::KillSettings(const KillRequest& kill_request)
: kill_probability_(kill_request.probability()) {}

bool KillRequestFilter::isKillRequestEnabled() {
return ProtobufPercentHelper::evaluateFractionalPercent(kill_request_.probability(),
random_generator_.random());
Expand All @@ -29,6 +37,20 @@ Http::FilterHeadersStatus KillRequestFilter::decodeHeaders(Http::RequestHeaderMa
return Http::FilterHeadersStatus::Continue;
}

// Route-level configuration overrides filter-level configuration.
if (decoder_callbacks_->route() && decoder_callbacks_->route()->routeEntry()) {
const std::string& name = Extensions::HttpFilters::HttpFilterNames::get().KillRequest;
Copy link
Member

Choose a reason for hiding this comment

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

I think we eventually want to match with the actual name used in the filter instantiation, but this hasn't been fixed Envoy-wide to my knowledge (#12575).

const auto* route_entry = decoder_callbacks_->route()->routeEntry();

const auto* per_route_kill_settings =
route_entry->mostSpecificPerFilterConfigTyped<KillSettings>(name);

if (per_route_kill_settings) {
envoy::type::v3::FractionalPercent probability = per_route_kill_settings->getProbability();
Copy link
Member

Choose a reason for hiding this comment

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

Nit: const envoy::type::v3::FractionalPercent probability = = ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

kill_request_.set_allocated_probability(&probability);
}
}

if (is_kill_request && isKillRequestEnabled()) {
// Crash Envoy.
raise(SIGABRT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "envoy/http/filter.h"
#include "envoy/http/header_map.h"

#include "common/http/header_utility.h"
#include "common/http/headers.h"

namespace Envoy {
Expand Down Expand Up @@ -52,7 +53,9 @@ class KillRequestFilter : public Http::StreamFilter, Logger::Loggable<Logger::Id
return Http::FilterTrailersStatus::Continue;
}

void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks&) override {}
void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override {
decoder_callbacks_ = &callbacks;
}

// Http::StreamEncoderFilter
Http::FilterHeadersStatus encode100ContinueHeaders(Http::ResponseHeaderMap&) override {
Expand Down Expand Up @@ -82,8 +85,29 @@ class KillRequestFilter : public Http::StreamFilter, Logger::Loggable<Logger::Id
// equaling true.
bool isKillRequestEnabled();

const envoy::extensions::filters::http::kill_request::v3::KillRequest kill_request_;
envoy::extensions::filters::http::kill_request::v3::KillRequest kill_request_;
Random::RandomGenerator& random_generator_;
Http::StreamDecoderFilterCallbacks* decoder_callbacks_{};
};

/**
* Configuration for fault injection.
*/
class KillSettings : public Router::RouteSpecificFilterConfig {
public:
KillSettings(const envoy::extensions::filters::http::kill_request::v3::KillRequest& kill_request);

const std::vector<Http::HeaderUtility::HeaderDataPtr>& filterHeaders() const {
return kill_request_filter_headers_;
}

const envoy::type::v3::FractionalPercent getProbability() const {
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 a bit weird. Why not just make return type const envoy::type::v3::FractionalPercent& and skip the move?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return std::move(kill_probability_);
}

private:
envoy::type::v3::FractionalPercent kill_probability_;
const std::vector<Http::HeaderUtility::HeaderDataPtr> kill_request_filter_headers_;
};

} // namespace KillRequest
Expand Down
2 changes: 2 additions & 0 deletions test/extensions/filters/http/kill_request/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ envoy_extension_cc_test(
deps = [
"//include/envoy/http:metadata_interface",
"//source/common/buffer:buffer_lib",
"//source/extensions/filters/http:well_known_names",
"//source/extensions/filters/http/kill_request:kill_request_filter_lib",
"//test/mocks:common_lib",
"//test/mocks/http:http_mocks",
"//test/test_common:utility_lib",
"@envoy_api//envoy/extensions/filters/http/kill_request/v3:pkg_cc_proto",
"@envoy_api//envoy/type/v3:pkg_cc_proto",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
#include "common/buffer/buffer_impl.h"

#include "extensions/filters/http/kill_request/kill_request_filter.h"
#include "extensions/filters/http/well_known_names.h"

#include "test/mocks/common.h"
#include "test/mocks/http/mocks.h"
#include "test/test_common/utility.h"

#include "gmock/gmock.h"
Expand All @@ -18,17 +20,23 @@ namespace HttpFilters {
namespace KillRequest {
namespace {

using ::testing::AnyNumber;
using ::testing::Return;

class KillRequestFilterTest : public testing::Test {
protected:
void
setUpTest(const envoy::extensions::filters::http::kill_request::v3::KillRequest& kill_request) {
filter_ = std::make_unique<KillRequestFilter>(kill_request, random_generator_);

filter_->setDecoderFilterCallbacks(decoder_filter_callbacks_);
EXPECT_CALL(decoder_filter_callbacks_.dispatcher_, pushTrackedObject(_)).Times(AnyNumber());
EXPECT_CALL(decoder_filter_callbacks_.dispatcher_, popTrackedObject(_)).Times(AnyNumber());
}

std::unique_ptr<KillRequestFilter> filter_;
testing::NiceMock<Random::MockRandomGenerator> random_generator_;
testing::NiceMock<Http::MockStreamDecoderFilterCallbacks> decoder_filter_callbacks_;
Http::TestRequestHeaderMapImpl request_headers_;
};

Expand Down Expand Up @@ -74,6 +82,42 @@ TEST_F(KillRequestFilterTest, KillRequestDisabledWhenIsKillRequestEnabledReturns
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false));
}

// Kill request should be enabled when isKillRequestEnabled returns true
// from the route level configuration.
TEST_F(KillRequestFilterTest, KillRequestEnabledFromRouteLevelConfiguration) {
envoy::extensions::filters::http::kill_request::v3::KillRequest kill_request;
kill_request.mutable_probability()->set_numerator(0);
setUpTest(kill_request);
request_headers_.addCopy("x-envoy-kill-request", "true");

envoy::extensions::filters::http::kill_request::v3::KillRequest route_level_kill_request;
route_level_kill_request.mutable_probability()->set_numerator(1);
route_level_kill_request.set_kill_request_header("x-custom-kill-request");

KillSettings kill_settings = KillSettings(route_level_kill_request);

ON_CALL(random_generator_, random()).WillByDefault(Return(0));
ON_CALL(decoder_filter_callbacks_.route_->route_entry_,
perFilterConfig(Extensions::HttpFilters::HttpFilterNames::get().KillRequest))
.WillByDefault(Return(&kill_settings));
EXPECT_DEATH(filter_->decodeHeaders(request_headers_, false), "");
}

// Kill request should be disabled when isKillRequestEnabled returns false
// from the route level configuration.
TEST_F(KillRequestFilterTest, KillRequestDisabledRouteLevelConfiguration) {
envoy::extensions::filters::http::kill_request::v3::KillRequest kill_request;
kill_request.mutable_probability()->set_numerator(0);
setUpTest(kill_request);
request_headers_.addCopy("x-envoy-kill-request", "true");

ON_CALL(random_generator_, random()).WillByDefault(Return(0));
ON_CALL(decoder_filter_callbacks_.route_->route_entry_,
perFilterConfig(Extensions::HttpFilters::HttpFilterNames::get().KillRequest))
.WillByDefault(Return(nullptr));
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false));
}

TEST_F(KillRequestFilterTest, KillRequestDisabledWhenHeaderIsMissing) {
envoy::extensions::filters::http::kill_request::v3::KillRequest kill_request;
kill_request.mutable_probability()->set_numerator(100);
Expand Down