diff --git a/.gitignore b/.gitignore index 1282e8293..6938aa837 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,9 @@ bin/ *.pyc node_modules/ src/go/proto/ +# IDE +.clwb/ +.ijwb/ # C++ coverage generated/ diff --git a/WORKSPACE b/WORKSPACE index f59150c1f..b367db64e 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -39,9 +39,9 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") # 3) Check if envoy_build_config/extensions_build_config.bzl is up-to-date. # Try to match it with the one in source/extensions and comment out unneeded extensions. -ENVOY_SHA1 = "dcd329a2e95b54f754b17aceca3f72724294b502" # v1.22.0 2022-04-19 +ENVOY_SHA1 = "ce49c7f65668a22b80d1e83c35d170741bb8d46a" # v1.23.0 2022-07-15 -ENVOY_SHA256 = "ed455b6a8b7058a243e42df608317d9d1864c1ec618705cc52d747357b3433ed" +ENVOY_SHA256 = "33975319b86087ad1bd22162c0bc8cf573b6be4aae9c53897dee148e955eb27c" http_archive( name = "envoy", @@ -80,6 +80,14 @@ load("@envoy//bazel:repositories_extra.bzl", "envoy_dependencies_extra") envoy_dependencies_extra() +load("@envoy//bazel:python_dependencies.bzl", "envoy_python_dependencies") + +envoy_python_dependencies() + +load("@base_pip3//:requirements.bzl", "install_deps") + +install_deps() + load("@envoy//bazel:dependency_imports.bzl", "envoy_dependency_imports") envoy_dependency_imports() diff --git a/src/envoy/http/backend_auth/filter.cc b/src/envoy/http/backend_auth/filter.cc index d8dd2e59c..6d3a9d281 100644 --- a/src/envoy/http/backend_auth/filter.cc +++ b/src/envoy/http/backend_auth/filter.cc @@ -62,7 +62,7 @@ FilterHeadersStatus Filter::decodeHeaders(RequestHeaderMap& headers, bool) { const auto* per_route = ::Envoy::Http::Utility::resolveMostSpecificPerFilterConfig< - PerRouteFilterConfig>(kFilterName, route); + PerRouteFilterConfig>(decoder_callbacks_); if (per_route == nullptr) { ENVOY_LOG(debug, "no per-route config"); config_->stats().allowed_by_auth_not_required_.inc(); diff --git a/src/envoy/http/backend_auth/filter_test.cc b/src/envoy/http/backend_auth/filter_test.cc index 2e01eed03..4d1706970 100644 --- a/src/envoy/http/backend_auth/filter_test.cc +++ b/src/envoy/http/backend_auth/filter_test.cc @@ -67,10 +67,9 @@ class BackendAuthFilterTest : public ::testing::Test { auto per_route = std::make_shared(per_route_cfg); EXPECT_CALL(mock_decoder_callbacks_, route()) .WillRepeatedly(Return(mock_route_)); - EXPECT_CALL(*mock_route_, mostSpecificPerFilterConfig(kFilterName)) - .WillRepeatedly( - Invoke([per_route](const std::string&) - -> const Envoy::Router::RouteSpecificFilterConfig* { + EXPECT_CALL(mock_decoder_callbacks_, mostSpecificPerFilterConfig()) + .WillRepeatedly(Invoke( + [per_route]() -> const Envoy::Router::RouteSpecificFilterConfig* { return per_route.get(); })); } @@ -102,7 +101,7 @@ TEST_F(BackendAuthFilterTest, NotPerRouteConfigAllowed) { {":path", "/books/1"}}; EXPECT_CALL(mock_decoder_callbacks_, route()) .WillRepeatedly(Return(mock_route_)); - EXPECT_CALL(*mock_route_, mostSpecificPerFilterConfig(kFilterName)) + EXPECT_CALL(mock_decoder_callbacks_, mostSpecificPerFilterConfig()) .WillRepeatedly(Return(nullptr)); Envoy::Http::FilterHeadersStatus status = diff --git a/src/envoy/http/path_rewrite/filter.cc b/src/envoy/http/path_rewrite/filter.cc index ddcf57c1a..7d0263b0b 100644 --- a/src/envoy/http/path_rewrite/filter.cc +++ b/src/envoy/http/path_rewrite/filter.cc @@ -88,7 +88,7 @@ FilterHeadersStatus Filter::decodeHeaders(RequestHeaderMap& headers, bool) { const auto* per_route = ::Envoy::Http::Utility::resolveMostSpecificPerFilterConfig< - PerRouteFilterConfig>(kFilterName, route); + PerRouteFilterConfig>(decoder_callbacks_); if (per_route == nullptr) { ENVOY_LOG(debug, "no per-route config, request is passed through unmodified"); diff --git a/src/envoy/http/path_rewrite/filter_test.cc b/src/envoy/http/path_rewrite/filter_test.cc index f7234866a..2564d69ee 100644 --- a/src/envoy/http/path_rewrite/filter_test.cc +++ b/src/envoy/http/path_rewrite/filter_test.cc @@ -147,7 +147,7 @@ TEST_F(FilterTest, NotPerRouteConfigNotChanged) { {":path", "/books/1"}}; EXPECT_CALL(mock_decoder_callbacks_, route()) .WillRepeatedly(Return(mock_route_)); - EXPECT_CALL(*mock_route_, mostSpecificPerFilterConfig(kFilterName)) + EXPECT_CALL(mock_decoder_callbacks_, mostSpecificPerFilterConfig()) .WillRepeatedly(Return(nullptr)); Envoy::Http::FilterHeadersStatus status = @@ -171,7 +171,7 @@ TEST_F(FilterTest, RejectedByMismatchUrlTemplate) { {":path", "/books/1"}}; EXPECT_CALL(mock_decoder_callbacks_, route()) .WillRepeatedly(Return(mock_route_)); - EXPECT_CALL(*mock_route_, mostSpecificPerFilterConfig(kFilterName)) + EXPECT_CALL(mock_decoder_callbacks_, mostSpecificPerFilterConfig()) .WillRepeatedly(Return(per_route_config_.get())); // Mismatch @@ -210,7 +210,7 @@ TEST_F(FilterTest, PathUpdated) { {":path", "/books/1"}}; EXPECT_CALL(mock_decoder_callbacks_, route()) .WillRepeatedly(Return(mock_route_)); - EXPECT_CALL(*mock_route_, mostSpecificPerFilterConfig(kFilterName)) + EXPECT_CALL(mock_decoder_callbacks_, mostSpecificPerFilterConfig()) .WillRepeatedly(Return(per_route_config_.get())); // path rewrite ok diff --git a/src/envoy/http/service_control/filter.cc b/src/envoy/http/service_control/filter.cc index 25617d98b..79f67c1ab 100644 --- a/src/envoy/http/service_control/filter.cc +++ b/src/envoy/http/service_control/filter.cc @@ -69,8 +69,7 @@ Envoy::Http::FilterHeadersStatus ServiceControlFilter::decodeHeaders( return Envoy::Http::FilterHeadersStatus::Continue; } - handler_ = - factory_.createHandler(headers, decoder_callbacks_->streamInfo(), stats_); + handler_ = factory_.createHandler(headers, decoder_callbacks_, stats_); handler_->fillFilterState(*decoder_callbacks_->streamInfo().filterState()); state_ = Calling; stopped_ = false; @@ -144,11 +143,12 @@ void ServiceControlFilter::log( const Envoy::Http::RequestHeaderMap* request_headers, const Envoy::Http::ResponseHeaderMap* response_headers, const Envoy::Http::ResponseTrailerMap* response_trailers, - const Envoy::StreamInfo::StreamInfo& stream_info) { + const Envoy::StreamInfo::StreamInfo&) { ENVOY_LOG(debug, "Called ServiceControl Filter : {}", __func__); if (!handler_) { if (!request_headers) return; - handler_ = factory_.createHandler(*request_headers, stream_info, stats_); + handler_ = + factory_.createHandler(*request_headers, decoder_callbacks_, stats_); } Envoy::Tracing::Span& parent_span = decoder_callbacks_->activeSpan(); diff --git a/src/envoy/http/service_control/handler.h b/src/envoy/http/service_control/handler.h index dc463906b..c1b70e259 100644 --- a/src/envoy/http/service_control/handler.h +++ b/src/envoy/http/service_control/handler.h @@ -66,7 +66,7 @@ class ServiceControlHandlerFactory { virtual ServiceControlHandlerPtr createHandler( const Envoy::Http::RequestHeaderMap& headers, - const Envoy::StreamInfo::StreamInfo& stream_info, + Envoy::Http::StreamDecoderFilterCallbacks* decoder_callbacks, ServiceControlFilterStats& filter_stats) const PURE; }; diff --git a/src/envoy/http/service_control/handler_impl.cc b/src/envoy/http/service_control/handler_impl.cc index ca2ff41eb..d6a955275 100644 --- a/src/envoy/http/service_control/handler_impl.cc +++ b/src/envoy/http/service_control/handler_impl.cc @@ -63,11 +63,12 @@ constexpr char JwtPayloadAudiencePath[] = "aud"; ServiceControlHandlerImpl::ServiceControlHandlerImpl( const Envoy::Http::RequestHeaderMap& headers, - const Envoy::StreamInfo::StreamInfo& stream_info, const std::string& uuid, - const FilterConfigParser& cfg_parser, Envoy::TimeSource& time_source, - ServiceControlFilterStats& filter_stats) + Envoy::Http::StreamDecoderFilterCallbacks* decoder_callbacks, + const std::string& uuid, const FilterConfigParser& cfg_parser, + Envoy::TimeSource& time_source, ServiceControlFilterStats& filter_stats) : cfg_parser_(cfg_parser), - stream_info_(stream_info), + stream_info_(decoder_callbacks->streamInfo()), + decoder_callbacks_(decoder_callbacks), time_source_(time_source), uuid_(uuid), request_header_size_(headers.byteSize()), @@ -82,7 +83,7 @@ ServiceControlHandlerImpl::ServiceControlHandlerImpl( http_method_ = std::string(utils::readHeaderEntry(headers.Method())); path_ = std::string(utils::readHeaderEntry(headers.Path())); - const auto operation = getOperationFromPerRoute(stream_info_); + const auto operation = getOperationFromPerRoute(); if (!operation.empty()) { require_ctx_ = cfg_parser_.find_requirement(operation); if (!require_ctx_) { @@ -107,17 +108,10 @@ ServiceControlHandlerImpl::ServiceControlHandlerImpl( ServiceControlHandlerImpl::~ServiceControlHandlerImpl() {} -absl::string_view ServiceControlHandlerImpl::getOperationFromPerRoute( - const Envoy::StreamInfo::StreamInfo& stream_info) { - if (stream_info_.route() == nullptr || - stream_info_.route()->routeEntry() == nullptr) { - ENVOY_LOG(debug, "No route entry"); - return Envoy::EMPTY_STRING; - } - +absl::string_view ServiceControlHandlerImpl::getOperationFromPerRoute() { const auto* per_route = ::Envoy::Http::Utility::resolveMostSpecificPerFilterConfig< - PerRouteFilterConfig>(kFilterName, stream_info.route()); + PerRouteFilterConfig>(decoder_callbacks_); if (per_route == nullptr) { ENVOY_LOG(debug, "no per-route config"); return Envoy::EMPTY_STRING; diff --git a/src/envoy/http/service_control/handler_impl.h b/src/envoy/http/service_control/handler_impl.h index 9737043f7..8ca8cefa3 100644 --- a/src/envoy/http/service_control/handler_impl.h +++ b/src/envoy/http/service_control/handler_impl.h @@ -20,6 +20,7 @@ #include "envoy/buffer/buffer.h" #include "envoy/common/random_generator.h" #include "envoy/common/time.h" +#include "envoy/http/filter.h" #include "envoy/http/header_map.h" #include "envoy/http/query_params.h" #include "envoy/runtime/runtime.h" @@ -42,12 +43,11 @@ class ServiceControlHandlerImpl : public Envoy::Logger::Loggable, public ServiceControlHandler { public: - ServiceControlHandlerImpl(const Envoy::Http::RequestHeaderMap& headers, - const Envoy::StreamInfo::StreamInfo& stream_info, - const std::string& uuid, - const FilterConfigParser& cfg_parser, - Envoy::TimeSource& timeSource, - ServiceControlFilterStats& filter_stats); + ServiceControlHandlerImpl( + const Envoy::Http::RequestHeaderMap& headers, + Envoy::Http::StreamDecoderFilterCallbacks* decoder_callbacks, + const std::string& uuid, const FilterConfigParser& cfg_parser, + Envoy::TimeSource& timeSource, ServiceControlFilterStats& filter_stats); ~ServiceControlHandlerImpl() override; void callCheck(Envoy::Http::RequestHeaderMap& headers, @@ -64,8 +64,7 @@ class ServiceControlHandlerImpl void onDestroy() override; private: - absl::string_view getOperationFromPerRoute( - const Envoy::StreamInfo::StreamInfo& stream_info); + absl::string_view getOperationFromPerRoute(); void callQuota(); @@ -106,6 +105,8 @@ class ServiceControlHandlerImpl // The metadata for the request const Envoy::StreamInfo::StreamInfo& stream_info_; + Envoy::Http::StreamDecoderFilterCallbacks* decoder_callbacks_; + // timeSource Envoy::TimeSource& time_source_; @@ -152,10 +153,10 @@ class ServiceControlHandlerFactoryImpl : public ServiceControlHandlerFactory { ServiceControlHandlerPtr createHandler( const Envoy::Http::RequestHeaderMap& headers, - const Envoy::StreamInfo::StreamInfo& stream_info, + Envoy::Http::StreamDecoderFilterCallbacks* decoder_callbacks, ServiceControlFilterStats& filter_stats) const override { return std::make_unique( - headers, stream_info, random_.uuid(), cfg_parser_, time_source_, + headers, decoder_callbacks, random_.uuid(), cfg_parser_, time_source_, filter_stats); } diff --git a/src/envoy/http/service_control/handler_impl_test.cc b/src/envoy/http/service_control/handler_impl_test.cc index 4b50188cd..5ce0bdaa3 100644 --- a/src/envoy/http/service_control/handler_impl_test.cc +++ b/src/envoy/http/service_control/handler_impl_test.cc @@ -34,6 +34,7 @@ namespace http_filters { namespace service_control { namespace { +using Envoy::Http::MockStreamDecoderFilterCallbacks; using Envoy::Http::TestRequestHeaderMapImpl; using Envoy::Http::TestRequestTrailerMapImpl; using Envoy::Http::TestResponseHeaderMapImpl; @@ -201,12 +202,9 @@ class HandlerTest : public ::testing::Test { per_route_cfg; per_route_cfg.set_operation_name(operation); auto per_route = std::make_shared(per_route_cfg); - mock_route_ = std::make_shared>(); - EXPECT_CALL(mock_stream_info_, route()).WillRepeatedly(Return(mock_route_)); - EXPECT_CALL(*mock_route_, mostSpecificPerFilterConfig(kFilterName)) - .WillRepeatedly( - Invoke([per_route](const std::string&) - -> const Envoy::Router::RouteSpecificFilterConfig* { + EXPECT_CALL(mock_decoder_callbacks_, mostSpecificPerFilterConfig()) + .WillRepeatedly(Invoke( + [per_route]() -> const Envoy::Router::RouteSpecificFilterConfig* { return per_route.get(); })); } @@ -215,7 +213,7 @@ class HandlerTest : public ::testing::Test { ServiceControlFilterStats stats_; testing::NiceMock mock_check_done_callback_; - testing::NiceMock mock_stream_info_; + testing::NiceMock mock_decoder_callbacks_; std::shared_ptr> mock_route_; testing::NiceMock mock_call_factory_; Envoy::Event::SimulatedTimeSystem test_time_; @@ -351,11 +349,13 @@ TEST_F(HandlerTest, HandlerNoOperationFound) { // Test: If no operation is found, the request will be passed through without // check and report should be called. - // Note: The operation is set in mock_stream_info_.filter_state_. This test - // should not set that value. + // Note: The operation is set in + // mock_decoder_callbacks_.stream_info_.filter_state_. This test should not + // set that value. TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/echo"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); EXPECT_CALL(mock_check_done_callback_, onCheckDone(OkStatus(), "")); EXPECT_CALL(*mock_call_, callCheck(_, _, _)).Times(0); @@ -380,7 +380,7 @@ TEST_F(HandlerTest, HandlerMissingHeaders) { // Note: This test builds off of `HandlerNoOperationFound` to keep mocks // simple - ServiceControlHandlerImpl handler(req_headers_, mock_stream_info_, + ServiceControlHandlerImpl handler(req_headers_, &mock_decoder_callbacks_, "test-uuid", *cfg_parser_, test_time_, stats_); @@ -407,8 +407,9 @@ TEST_F(HandlerTest, HandlerNoRequirementMatched) { // passed through without check and report should be called. setPerRouteOperation("bad-operation-name"); TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/echo"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); EXPECT_CALL(mock_check_done_callback_, onCheckDone(OkStatus(), "")); EXPECT_CALL(*mock_call_, callCheck(_, _, _)).Times(0); handler.callCheck(headers, mock_span_, mock_check_done_callback_); @@ -430,8 +431,9 @@ TEST_F(HandlerTest, HandlerCheckNotNeeded) { TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/echo"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); EXPECT_CALL(*mock_call_, callCheck(_, _, _)).Times(0); EXPECT_CALL(*mock_call_, callQuota(_, _)).Times(0); @@ -458,7 +460,7 @@ TEST_F(HandlerTest, RequestHeaderSizeWithModificationInUpstream) { TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(request_headers, mock_stream_info_, + ServiceControlHandlerImpl handler(request_headers, &mock_decoder_callbacks_, "test-uuid", *cfg_parser_, test_time_, stats_); EXPECT_CALL(mock_check_done_callback_, onCheckDone(OkStatus(), "")); @@ -485,8 +487,9 @@ TEST_F(HandlerTest, HandlerCheckMissingApiKey) { TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); Status bad_status = Status(StatusCode::kUnauthenticated, "Method doesn't allow unregistered callers (callers without " @@ -525,8 +528,9 @@ TEST_F(HandlerTest, HandlerSuccessfulCheckSyncWithApiKeyRestrictionFields) { {"x-android-cert", "cert-123"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); CheckResponseInfo response_info; CheckRequestInfo expected_check_info; @@ -564,8 +568,9 @@ TEST_F(HandlerTest, HandlerSuccessfulCheckSyncWithoutApiKeyRestrictionFields) { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); CheckResponseInfo response_info; CheckRequestInfo expected_check_info; @@ -598,8 +603,9 @@ TEST_F(HandlerTest, HandlerSuccessfulQuotaSync) { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); CheckResponseInfo response_info; EXPECT_CALL(*mock_call_, callCheck(_, _, _)) @@ -636,8 +642,9 @@ TEST_F(HandlerTest, HandlerCallQuotaWithoutCheck) { {":path", "/echo?key=foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); // Check is not called. EXPECT_CALL(*mock_call_, callCheck(_, _, _)).Times(0); @@ -670,8 +677,9 @@ TEST_F(HandlerTest, HandlerFailCheckSync) { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); Status bad_status = Status(StatusCode::kPermissionDenied, "test bad status returned from service control"); @@ -695,7 +703,8 @@ TEST_F(HandlerTest, HandlerFailCheckSync) { onCheckDone(bad_status, expected_rc_detail)); handler.callCheck(headers, mock_span_, mock_check_done_callback_); - mock_stream_info_.response_code_details_ = expected_rc_detail; + mock_decoder_callbacks_.stream_info_.response_code_details_ = + expected_rc_detail; ReportRequestInfo expected_report_info; initExpectedReportInfo(expected_report_info); expected_report_info.status = bad_status; @@ -715,16 +724,19 @@ TEST_F(HandlerTest, FillFilterState) { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); - handler.fillFilterState(*mock_stream_info_.filter_state_); + handler.fillFilterState(*mock_decoder_callbacks_.stream_info_.filter_state_); - EXPECT_EQ(utils::getStringFilterState(*mock_stream_info_.filter_state_, - utils::kFilterStateApiKey), + EXPECT_EQ(utils::getStringFilterState( + *mock_decoder_callbacks_.stream_info_.filter_state_, + utils::kFilterStateApiKey), "foobar"); - EXPECT_EQ(utils::getStringFilterState(*mock_stream_info_.filter_state_, - utils::kFilterStateApiMethod), + EXPECT_EQ(utils::getStringFilterState( + *mock_decoder_callbacks_.stream_info_.filter_state_, + utils::kFilterStateApiMethod), "get_header_key"); } @@ -736,8 +748,9 @@ TEST_F(HandlerTest, HandlerFailQuotaSync) { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); CheckResponseInfo response_info; EXPECT_CALL(*mock_call_, callCheck(_, _, _)) @@ -776,8 +789,9 @@ TEST_F(HandlerTest, HandlerSuccessfulCheckAsync) { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); CheckResponseInfo response_info; @@ -818,8 +832,9 @@ TEST_F(HandlerTest, HandlerSuccessfulQuotaAsync) { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); CheckResponseInfo response_info; EXPECT_CALL(*mock_call_, callCheck(_, _, _)) @@ -868,8 +883,9 @@ TEST_F(HandlerTest, HandlerFailCheckAsync) { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); CheckResponseInfo response_info; response_info.error = {"API_KEY_INVALID", false, @@ -919,8 +935,9 @@ TEST_F(HandlerTest, HandlerFailQuotaAsync) { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; TestResponseHeaderMapImpl response_headers{ {"content-type", "application/grpc"}}; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); CheckResponseInfo response_info; EXPECT_CALL(*mock_call_, callCheck(_, _, _)) @@ -977,8 +994,9 @@ TEST_F(HandlerTest, HandlerCancelFuncResetOnDone) { MockFunction mock_cancel; CancelFunc cancel_fn = mock_cancel.AsStdFunction(); - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); EXPECT_CALL(*mock_call_, callCheck(_, _, _)) .WillOnce(Invoke([&stored_on_done, cancel_fn](const CheckRequestInfo&, Envoy::Tracing::Span&, @@ -1003,8 +1021,9 @@ TEST_F(HandlerTest, HandlerCancelFuncCalledOnDestroy) { MockFunction mock_cancel; CancelFunc cancel_fn = mock_cancel.AsStdFunction(); - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); EXPECT_CALL(*mock_call_, callCheck(_, _, _)) .WillOnce( Invoke([cancel_fn](const CheckRequestInfo&, Envoy::Tracing::Span&, @@ -1024,8 +1043,9 @@ TEST_F(HandlerTest, HandlerCancelFuncNotCalledOnDestroyForSyncOnDone) { MockFunction mock_cancel; CancelFunc cancel_fn = mock_cancel.AsStdFunction(); - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); EXPECT_CALL(*mock_call_, callCheck(_, _, _)) .WillOnce( Invoke([cancel_fn](const CheckRequestInfo&, Envoy::Tracing::Span&, @@ -1053,8 +1073,9 @@ TEST_F(HandlerTest, HandlerReportWithoutCheck) { {"content-type", "application/grpc"}}; CheckDoneFunc stored_on_done; CheckResponseInfo response_info; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); ReportRequestInfo expected_report_info; initExpectedReportInfo(expected_report_info); @@ -1077,8 +1098,9 @@ TEST_F(HandlerTest, HandlerReportWithTrace) { {"content-type", "application/grpc"}}; CheckDoneFunc stored_on_done; CheckResponseInfo response_info; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); ReportRequestInfo expected_report_info; initExpectedReportInfo(expected_report_info); @@ -1097,7 +1119,7 @@ class HandlerReportStatusTest : public HandlerTest { TestResponseHeaderMapImpl response_headers, TestResponseTrailerMapImpl response_trailers, absl::optional expected_grpc_status) { - EXPECT_CALL(mock_stream_info_, responseCode()) + EXPECT_CALL(mock_decoder_callbacks_.stream_info_, responseCode()) .WillRepeatedly(Return(http_response_code)); setPerRouteOperation("get_header_key"); @@ -1105,8 +1127,9 @@ class HandlerReportStatusTest : public HandlerTest { {":method", "GET"}, {":path", "/echo"}, {"x-api-key", "foobar"}}; CheckDoneFunc stored_on_done; CheckResponseInfo response_info; - ServiceControlHandlerImpl handler(headers, mock_stream_info_, "test-uuid", - *cfg_parser_, test_time_, stats_); + ServiceControlHandlerImpl handler(headers, &mock_decoder_callbacks_, + "test-uuid", *cfg_parser_, test_time_, + stats_); ReportRequestInfo expected_report_info; initExpectedReportInfo(expected_report_info); diff --git a/src/envoy/http/service_control/handler_utils.cc b/src/envoy/http/service_control/handler_utils.cc index 301ea3215..854052be2 100644 --- a/src/envoy/http/service_control/handler_utils.cc +++ b/src/envoy/http/service_control/handler_utils.cc @@ -210,7 +210,7 @@ void fillLatency(const Envoy::StreamInfo::StreamInfo& stream_info, // overhead latency and over-reporting backend latency. if (!start && stream_info.responseCodeDetails() && (stream_info.responseCodeDetails().value() == - Envoy::StreamInfo::ResponseCodeDetails::get().UpstreamTimeout || + Envoy::StreamInfo::ResponseCodeDetails::get().ResponseTimeout || stream_info.responseCodeDetails().value() == Envoy::StreamInfo::ResponseCodeDetails::get() .UpstreamPerTryTimeout)) { diff --git a/src/envoy/http/service_control/http_call.cc b/src/envoy/http/service_control/http_call.cc index b33aac918..4bfe0ada6 100644 --- a/src/envoy/http/service_control/http_call.cc +++ b/src/envoy/http/service_control/http_call.cc @@ -207,7 +207,7 @@ class HttpCallImpl : public HttpCall, request_span_->setTag(Envoy::Tracing::Tags::get().HttpMethod, "POST"); Envoy::Http::RequestMessagePtr message = prepareHeaders(token); - request_span_->injectContext(message->headers()); + request_span_->injectContext(message->headers(), nullptr); ENVOY_LOG(debug, "http call from [uri = {}]: start", uri_); const auto thread_local_cluster = diff --git a/src/envoy/http/service_control/mocks.h b/src/envoy/http/service_control/mocks.h index cb8bb8b86..b06046f80 100644 --- a/src/envoy/http/service_control/mocks.h +++ b/src/envoy/http/service_control/mocks.h @@ -14,6 +14,7 @@ #pragma once +#include "envoy/http/filter.h" #include "envoy/http/header_map.h" #include "gmock/gmock.h" #include "src/envoy/http/service_control/handler.h" @@ -49,7 +50,7 @@ class MockServiceControlHandlerFactory : public ServiceControlHandlerFactory { public: MOCK_METHOD(ServiceControlHandlerPtr, createHandler, (const Envoy::Http::RequestHeaderMap& headers, - const Envoy::StreamInfo::StreamInfo& stream_info, + Envoy::Http::StreamDecoderFilterCallbacks* decoder_callbacks, ServiceControlFilterStats& filter_stats), (const, override)); }; diff --git a/src/envoy/token/token_subscriber.cc b/src/envoy/token/token_subscriber.cc index a48fbc14b..2897d8e25 100644 --- a/src/envoy/token/token_subscriber.cc +++ b/src/envoy/token/token_subscriber.cc @@ -143,19 +143,19 @@ void TokenSubscriber::refresh() { void TokenSubscriber::processResponse( Envoy::Http::ResponseMessagePtr&& response) { - try { - const uint64_t status_code = - Envoy::Http::Utility::getResponseStatus(response->headers()); - - if (status_code != Envoy::enumToInt(Envoy::Http::Code::OK)) { - ENVOY_LOG(error, "{}: failed: {}", debug_name_, status_code); - handleFailResponse(); - return; - } - } catch (const Envoy::EnvoyException& e) { + auto status = + Envoy::Http::Utility::getResponseStatusOrNullopt(response->headers()); + if (!status.has_value()) { // This occurs if the status header is missing. // Catch the exception to prevent unwinding and skipping cleanup. - ENVOY_LOG(error, "{}: failed: {}", debug_name_, e.what()); + ENVOY_LOG(error, "{}: failed: No status in headers", debug_name_); + handleFailResponse(); + return; + } + + const uint64_t status_code = status.value(); + if (status_code != Envoy::enumToInt(Envoy::Http::Code::OK)) { + ENVOY_LOG(error, "{}: failed: {}", debug_name_, status_code); handleFailResponse(); return; } diff --git a/src/envoy/token/token_subscriber_test.cc b/src/envoy/token/token_subscriber_test.cc index b0a676a7b..8d91c65dd 100644 --- a/src/envoy/token/token_subscriber_test.cc +++ b/src/envoy/token/token_subscriber_test.cc @@ -18,7 +18,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "source/common/http/message_impl.h" -#include "source/common/tracing/http_tracer_impl.h" #include "src/envoy/token/mocks.h" #include "test/mocks/init/mocks.h" #include "test/mocks/server/mocks.h"