From 30382fc6cd138728aa67807c6c31550f62fb39ad Mon Sep 17 00:00:00 2001 From: Dhi Aurrahman Date: Tue, 10 Sep 2019 07:15:28 +0700 Subject: [PATCH] ext_authz: Check for cluster before sending HTTP request (#8144) Signed-off-by: Dhi Aurrahman --- .../common/ext_authz/ext_authz_http_impl.cc | 17 ++++++++++++++--- .../ext_authz/ext_authz_http_impl_test.cc | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc index b40a927fdbfb..b49960bf511a 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc +++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc @@ -202,9 +202,20 @@ void RawHttpClientImpl::check(RequestCallbacks& callbacks, std::make_unique(request.attributes().request().http().body()); } - request_ = cm_.httpAsyncClientForCluster(config_->cluster()) - .send(std::move(message), *this, - Http::AsyncClient::RequestOptions().setTimeout(config_->timeout())); + const std::string& cluster = config_->cluster(); + + // It's possible that the cluster specified in the filter configuration no longer exists due to a + // CDS removal. + if (cm_.get(cluster) == nullptr) { + // TODO(dio): Add stats and tracing related to this. + ENVOY_LOG(debug, "ext_authz cluster '{}' does not exist", cluster); + callbacks_->onComplete(std::make_unique(errorResponse())); + callbacks_ = nullptr; + } else { + request_ = cm_.httpAsyncClientForCluster(cluster).send( + std::move(message), *this, + Http::AsyncClient::RequestOptions().setTimeout(config_->timeout())); + } } void RawHttpClientImpl::onSuccess(Http::MessagePtr&& message) { diff --git a/test/extensions/filters/common/ext_authz/ext_authz_http_impl_test.cc b/test/extensions/filters/common/ext_authz/ext_authz_http_impl_test.cc index e8d40a655a3f..6941c1b3624d 100644 --- a/test/extensions/filters/common/ext_authz/ext_authz_http_impl_test.cc +++ b/test/extensions/filters/common/ext_authz/ext_authz_http_impl_test.cc @@ -16,6 +16,8 @@ using testing::_; using testing::AllOf; +using testing::Eq; +using testing::InSequence; using testing::Invoke; using testing::Return; using testing::ReturnRef; @@ -374,6 +376,18 @@ TEST_F(ExtAuthzHttpClientTest, CancelledAuthorizationRequest) { client_.cancel(); } +// Test the client when the configured cluster is missing/removed. +TEST_F(ExtAuthzHttpClientTest, NoCluster) { + InSequence s; + + EXPECT_CALL(cm_, get(Eq("ext_authz"))).WillOnce(Return(nullptr)); + EXPECT_CALL(cm_, httpAsyncClientForCluster("ext_authz")).Times(0); + EXPECT_CALL(request_callbacks_, + onComplete_(WhenDynamicCastTo(AuthzErrorResponse(CheckStatus::Error)))); + client_.check(request_callbacks_, envoy::service::auth::v2::CheckRequest{}, + Tracing::NullSpan::instance()); +} + } // namespace } // namespace ExtAuthz } // namespace Common