diff --git a/src/envoy/http/mixer/filter.cc b/src/envoy/http/mixer/filter.cc index 788e7b228ac..3de446898ca 100644 --- a/src/envoy/http/mixer/filter.cc +++ b/src/envoy/http/mixer/filter.cc @@ -157,11 +157,7 @@ void Filter::completeCheck(const CheckResponseInfo& info) { route_directive_ = info.route_directive; - // set UAEX access log flag - if (!status.ok()) { - decoder_callbacks_->streamInfo().setResponseFlag( - StreamInfo::ResponseFlag::UnauthorizedExternalService); - } + Utils::CheckResponseInfoToStreamInfo(info, decoder_callbacks_->streamInfo()); // handle direct response from the route directive if (route_directive_.direct_response_code() != 0) { diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc index fe2976191b8..9cb94a25efc 100644 --- a/src/envoy/tcp/mixer/filter.cc +++ b/src/envoy/tcp/mixer/filter.cc @@ -61,9 +61,8 @@ void Filter::callCheck() { state_ = State::Calling; filter_callbacks_->connection().readDisable(true); calling_check_ = true; - cancel_check_ = handler_->Check(this, [this](const CheckResponseInfo &info) { - completeCheck(info.response_status); - }); + cancel_check_ = handler_->Check( + this, [this](const CheckResponseInfo &info) { completeCheck(info); }); calling_check_ = false; } @@ -138,13 +137,18 @@ Network::FilterStatus Filter::onNewConnection() { return Network::FilterStatus::Continue; } -void Filter::completeCheck(const Status &status) { +void Filter::completeCheck(const CheckResponseInfo &info) { + const auto &status = info.response_status; ENVOY_LOG(debug, "Called tcp filter completeCheck: {}", status.ToString()); cancel_check_ = nullptr; if (state_ == State::Closed) { return; } state_ = State::Completed; + + Utils::CheckResponseInfoToStreamInfo( + info, filter_callbacks_->connection().streamInfo()); + filter_callbacks_->connection().readDisable(false); if (!status.ok()) { diff --git a/src/envoy/tcp/mixer/filter.h b/src/envoy/tcp/mixer/filter.h index b957c2c239f..b9bfb97369c 100644 --- a/src/envoy/tcp/mixer/filter.h +++ b/src/envoy/tcp/mixer/filter.h @@ -19,6 +19,7 @@ #include "envoy/network/connection.h" #include "envoy/network/filter.h" #include "google/protobuf/struct.pb.h" +#include "include/istio/mixerclient/check_response.h" #include "src/envoy/tcp/mixer/control.h" namespace Envoy { @@ -80,7 +81,7 @@ class Filter : public Network::Filter, void callCheck(); // Called when Check is done. - void completeCheck(const ::google::protobuf::util::Status &status); + void completeCheck(const ::istio::mixerclient::CheckResponseInfo &info); // Cancel the pending Check call. void cancelCheck(); diff --git a/src/envoy/utils/BUILD b/src/envoy/utils/BUILD index ed9418a13fd..3415ef32633 100644 --- a/src/envoy/utils/BUILD +++ b/src/envoy/utils/BUILD @@ -89,6 +89,7 @@ envoy_cc_test( repository = "@envoy", deps = [ ":utils_lib", + "@envoy//test/mocks/stream_info:stream_info_mocks", "@envoy//test/test_common:utility_lib", ], ) diff --git a/src/envoy/utils/utils.cc b/src/envoy/utils/utils.cc index 6197cacedf5..0b7d95bcb9c 100644 --- a/src/envoy/utils/utils.cc +++ b/src/envoy/utils/utils.cc @@ -139,5 +139,21 @@ Status ParseJsonMessage(const std::string& json, Message* output) { return ::google::protobuf::util::JsonStringToMessage(json, output, options); } +void CheckResponseInfoToStreamInfo( + const istio::mixerclient::CheckResponseInfo& check_response, + StreamInfo::StreamInfo& stream_info) { + static std::string metadata_key = "istio.mixer"; + + if (!check_response.response_status.ok()) { + stream_info.setResponseFlag( + StreamInfo::ResponseFlag::UnauthorizedExternalService); + ProtobufWkt::Struct metadata; + auto& fields = *metadata.mutable_fields(); + fields["status"].set_string_value( + check_response.response_status.ToString()); + stream_info.setDynamicMetadata(metadata_key, metadata); + } +} + } // namespace Utils } // namespace Envoy diff --git a/src/envoy/utils/utils.h b/src/envoy/utils/utils.h index 6ffde52c3c9..3fa4aac5b21 100644 --- a/src/envoy/utils/utils.h +++ b/src/envoy/utils/utils.h @@ -21,6 +21,7 @@ #include "envoy/http/header_map.h" #include "envoy/network/connection.h" #include "google/protobuf/util/json_util.h" +#include "include/istio/mixerclient/check_response.h" namespace Envoy { namespace Utils { @@ -52,5 +53,10 @@ bool GetRequestedServerName(const Network::Connection* connection, ::google::protobuf::util::Status ParseJsonMessage( const std::string& json, ::google::protobuf::Message* output); +// Add result of check to envoy stream info to allow better logging. +void CheckResponseInfoToStreamInfo( + const istio::mixerclient::CheckResponseInfo& check_response, + StreamInfo::StreamInfo& stream_info); + } // namespace Utils } // namespace Envoy diff --git a/src/envoy/utils/utils_test.cc b/src/envoy/utils/utils_test.cc index 8d38b995e77..acd2bb4f9de 100644 --- a/src/envoy/utils/utils_test.cc +++ b/src/envoy/utils/utils_test.cc @@ -15,12 +15,14 @@ #include "src/envoy/utils/utils.h" #include "mixer/v1/config/client/client_config.pb.h" +#include "test/mocks/stream_info/mocks.h" #include "test/test_common/utility.h" -using Envoy::Utils::ParseJsonMessage; - namespace { +using Envoy::Utils::CheckResponseInfoToStreamInfo; +using Envoy::Utils::ParseJsonMessage; + TEST(UtilsTest, ParseNormalMessage) { std::string config_str = R"({ "default_destination_service": "service.svc.cluster.local", @@ -44,4 +46,25 @@ TEST(UtilsTest, ParseMessageWithUnknownField) { EXPECT_EQ(http_config.default_destination_service(), "service.svc.cluster.local"); } + +TEST(UtilsTest, CheckResponseInfoToStreamInfo) { + ::istio::mixerclient::CheckResponseInfo + check_response; // by default status is unknown + Envoy::StreamInfo::MockStreamInfo mock_stream_info; + + EXPECT_CALL( + mock_stream_info, + setResponseFlag( + Envoy::StreamInfo::ResponseFlag::UnauthorizedExternalService)); + EXPECT_CALL(mock_stream_info, setDynamicMetadata(_, _)) + .WillOnce(Invoke( + [](const std::string& key, const Envoy::ProtobufWkt::Struct& value) { + EXPECT_EQ("istio.mixer", key); + EXPECT_EQ(1, value.fields().count("status")); + EXPECT_EQ("UNKNOWN", value.fields().at("status").string_value()); + })); + + CheckResponseInfoToStreamInfo(check_response, mock_stream_info); +} + } // namespace