Skip to content

Commit

Permalink
health check: add configurable authority to gRPC health checker (envo…
Browse files Browse the repository at this point in the history
…yproxy#5275)

This change allows user to configure custom authority headers to be
sent with gRPC health checks. It defaults to the name of the cluster,
thus maintaining backward compatibility.

Signed-off-by: Yuan Liu <[email protected]>
  • Loading branch information
yuan-stripe authored and mattklein123 committed Dec 14, 2018
1 parent c751197 commit 6af566f
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 48 deletions.
5 changes: 5 additions & 0 deletions api/envoy/api/v2/core/health_check.proto
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ message HealthCheck {
// message. See `gRPC health-checking overview
// <https://github.com/grpc/grpc/blob/master/doc/health-checking.md>`_ for more information.
string service_name = 1;

// The value of the :authority header in the gRPC health check request. If
// left empty (default value), the name of the cluster this health check is associated
// with will be used.
string authority = 2;
}

// Custom health check.
Expand Down
2 changes: 2 additions & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ Version history
* ext-authz: added support for providing per route config - optionally disable the filter and provide context extensions.
* fault: removed integer percentage support.
* health check: Added :ref: 'logging health check failure events <envoy_api_field_core.HealthCheck.always_log_health_check_failures>'.
* health check: added ability to set :ref:`authority header value
<envoy_api_field_core.HealthCheck.GrpcHealthCheck.authority>` for gRPC health check.
* http: Added HTTP/2 WebSocket proxying via :ref:`extended CONNECT <envoy_api_field_core.Http2ProtocolOptions.allow_connect>`
* http: added limits to the number and length of header modifications in all fields request_headers_to_add and response_headers_to_add. These limits are very high and should only be used as a last-resort safeguard.
* http: added support for a :ref:`request timeout <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.request_timeout>`. The timeout is disabled by default.
Expand Down
15 changes: 11 additions & 4 deletions source/common/upstream/health_checker_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,10 @@ GrpcHealthCheckerImpl::GrpcHealthCheckerImpl(const Cluster& cluster,
if (!config.grpc_health_check().service_name().empty()) {
service_name_ = config.grpc_health_check().service_name();
}

if (!config.grpc_health_check().authority().empty()) {
authority_value_ = config.grpc_health_check().authority();
}
}

GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::GrpcActiveHealthCheckSession(
Expand Down Expand Up @@ -488,17 +492,20 @@ void GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::onInterval() {
request_encoder_ = &client_->newStream(*this);
request_encoder_->getStream().addCallbacks(*this);

auto headers_message = Grpc::Common::prepareHeaders(
parent_.cluster_.info()->name(), parent_.service_method_.service()->full_name(),
parent_.service_method_.name(), absl::nullopt);
const std::string& authority = parent_.authority_value_.has_value()
? parent_.authority_value_.value()
: parent_.cluster_.info()->name();
auto headers_message =
Grpc::Common::prepareHeaders(authority, parent_.service_method_.service()->full_name(),
parent_.service_method_.name(), absl::nullopt);
headers_message->headers().insertUserAgent().value().setReference(
Http::Headers::get().UserAgentValues.EnvoyHealthChecker);
Router::FilterUtility::setUpstreamScheme(headers_message->headers(), *parent_.cluster_.info());

request_encoder_->encodeHeaders(headers_message->headers(), false);

grpc::health::v1::HealthCheckRequest request;
if (parent_.service_name_) {
if (parent_.service_name_.has_value()) {
request.set_service(parent_.service_name_.value());
}

Expand Down
1 change: 1 addition & 0 deletions source/common/upstream/health_checker_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ class GrpcHealthCheckerImpl : public HealthCheckerImplBase {

const Protobuf::MethodDescriptor& service_method_;
absl::optional<std::string> service_name_;
absl::optional<std::string> authority_value_;
};

/**
Expand Down
104 changes: 60 additions & 44 deletions test/common/upstream/health_checker_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2444,9 +2444,12 @@ class GrpcHealthCheckerImplTestBase {
});
}

void setupServiceNameHC() {
void setupServiceNameHC(const absl::optional<std::string>& authority) {
auto config = createGrpcHealthCheckConfig();
config.mutable_grpc_health_check()->set_service_name("service");
if (authority.has_value()) {
config.mutable_grpc_health_check()->set_authority(authority.value());
}
health_checker_.reset(new TestGrpcHealthCheckerImpl(*cluster_, config, dispatcher_, runtime_,
random_,
HealthCheckEventLoggerPtr(event_logger_)));
Expand Down Expand Up @@ -2629,6 +2632,56 @@ class GrpcHealthCheckerImplTestBase {
}
}

void testSingleHostSuccess(const absl::optional<std::string>& authority) {
std::string expected_host = cluster_->info_->name();
if (authority.has_value()) {
expected_host = authority.value();
}

setupServiceNameHC(authority);

cluster_->prioritySet().getMockHostSet(0)->hosts_ = {
makeTestHost(cluster_->info_, "tcp://127.0.0.1:80")};
cluster_->info_->stats().upstream_cx_total_.inc();

expectSessionCreate();
expectHealthcheckStart(0);

EXPECT_CALL(test_sessions_[0]->request_encoder_, encodeHeaders(_, false))
.WillOnce(Invoke([&](const Http::HeaderMap& headers, bool) {
EXPECT_EQ(Http::Headers::get().ContentTypeValues.Grpc,
headers.ContentType()->value().c_str());
EXPECT_EQ(std::string("/grpc.health.v1.Health/Check"), headers.Path()->value().c_str());
EXPECT_EQ(Http::Headers::get().SchemeValues.Http, headers.Scheme()->value().c_str());
EXPECT_NE(nullptr, headers.Method());
EXPECT_EQ(expected_host, headers.Host()->value().c_str());
}));
EXPECT_CALL(test_sessions_[0]->request_encoder_, encodeData(_, true))
.WillOnce(Invoke([&](Buffer::Instance& data, bool) {
std::vector<Grpc::Frame> decoded_frames;
Grpc::Decoder decoder;
ASSERT_TRUE(decoder.decode(data, decoded_frames));
ASSERT_EQ(1U, decoded_frames.size());
auto& frame = decoded_frames[0];
Buffer::ZeroCopyInputStreamImpl stream(std::move(frame.data_));
grpc::health::v1::HealthCheckRequest request;
ASSERT_TRUE(request.ParseFromZeroCopyStream(&stream));
EXPECT_EQ("service", request.service());
}));
health_checker_->start();

EXPECT_CALL(runtime_.snapshot_, getInteger("health_check.max_interval", _));
EXPECT_CALL(runtime_.snapshot_, getInteger("health_check.min_interval", _))
.WillOnce(Return(45000));
expectHealthcheckStop(0, 45000);

// Host state should not be changed (remains healty).
EXPECT_CALL(*this, onHostStatus(cluster_->prioritySet().getMockHostSet(0)->hosts_[0],
HealthTransition::Unchanged));
respondServiceStatus(0, grpc::health::v1::HealthCheckResponse::SERVING);
expectHostHealthy(true);
}

MOCK_METHOD2(onHostStatus, void(HostSharedPtr host, HealthTransition changed_state));

std::shared_ptr<MockCluster> cluster_;
Expand All @@ -2645,54 +2698,17 @@ class GrpcHealthCheckerImplTestBase {
class GrpcHealthCheckerImplTest : public GrpcHealthCheckerImplTestBase, public testing::Test {};

// Test single host check success.
TEST_F(GrpcHealthCheckerImplTest, Success) {
setupServiceNameHC();
TEST_F(GrpcHealthCheckerImplTest, Success) { testSingleHostSuccess(absl::nullopt); }

cluster_->prioritySet().getMockHostSet(0)->hosts_ = {
makeTestHost(cluster_->info_, "tcp://127.0.0.1:80")};
cluster_->info_->stats().upstream_cx_total_.inc();

expectSessionCreate();
expectHealthcheckStart(0);

EXPECT_CALL(test_sessions_[0]->request_encoder_, encodeHeaders(_, false))
.WillOnce(Invoke([&](const Http::HeaderMap& headers, bool) {
EXPECT_EQ(Http::Headers::get().ContentTypeValues.Grpc,
headers.ContentType()->value().c_str());
EXPECT_EQ(std::string("/grpc.health.v1.Health/Check"), headers.Path()->value().c_str());
EXPECT_EQ(Http::Headers::get().SchemeValues.Http, headers.Scheme()->value().c_str());
EXPECT_NE(nullptr, headers.Method());
EXPECT_NE(nullptr, headers.Host());
}));
EXPECT_CALL(test_sessions_[0]->request_encoder_, encodeData(_, true))
.WillOnce(Invoke([&](Buffer::Instance& data, bool) {
std::vector<Grpc::Frame> decoded_frames;
Grpc::Decoder decoder;
ASSERT_TRUE(decoder.decode(data, decoded_frames));
ASSERT_EQ(1U, decoded_frames.size());
auto& frame = decoded_frames[0];
Buffer::ZeroCopyInputStreamImpl stream(std::move(frame.data_));
grpc::health::v1::HealthCheckRequest request;
ASSERT_TRUE(request.ParseFromZeroCopyStream(&stream));
EXPECT_EQ("service", request.service());
}));
health_checker_->start();

EXPECT_CALL(runtime_.snapshot_, getInteger("health_check.max_interval", _));
EXPECT_CALL(runtime_.snapshot_, getInteger("health_check.min_interval", _))
.WillOnce(Return(45000));
expectHealthcheckStop(0, 45000);

// Host state should not be changed (remains healty).
EXPECT_CALL(*this, onHostStatus(cluster_->prioritySet().getMockHostSet(0)->hosts_[0],
HealthTransition::Unchanged));
respondServiceStatus(0, grpc::health::v1::HealthCheckResponse::SERVING);
expectHostHealthy(true);
// Test single host check success with custom authority.
TEST_F(GrpcHealthCheckerImplTest, SuccessWithCustomAuthority) {
const std::string authority = "www.envoyproxy.io";
testSingleHostSuccess(authority);
}

// Test host check success when gRPC response payload is split between several incoming data chunks.
TEST_F(GrpcHealthCheckerImplTest, SuccessResponseSplitBetweenChunks) {
setupServiceNameHC();
setupServiceNameHC(absl::nullopt);
expectSingleHealthcheck(HealthTransition::Unchanged);

auto response_headers = std::make_unique<Http::TestHeaderMapImpl>(
Expand Down

0 comments on commit 6af566f

Please sign in to comment.