diff --git a/WORKSPACE b/WORKSPACE index 460479c677d..0cc27fae06b 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -30,7 +30,7 @@ bind( ) # When updating envoy sha manually please update the sha in istio.deps file also -ENVOY_SHA = "c41fa7118e69a0872074d7a685a62331c5d5c17e" +ENVOY_SHA = "74de08a0d4d31bd466639d25d681df5d290bb770" http_archive( name = "envoy", diff --git a/include/istio/control/http/report_data.h b/include/istio/control/http/report_data.h index 11b1e7e9f59..8cf7326fe0f 100644 --- a/include/istio/control/http/report_data.h +++ b/include/istio/control/http/report_data.h @@ -19,6 +19,8 @@ #include #include +#include "google/protobuf/struct.pb.h" + namespace istio { namespace control { namespace http { @@ -42,27 +44,32 @@ class ReportData { int response_code; std::string response_flags; }; - virtual void GetReportInfo(ReportInfo* info) const = 0; + virtual void GetReportInfo(ReportInfo *info) const = 0; // Get destination ip/port. - virtual bool GetDestinationIpPort(std::string* ip, int* port) const = 0; + virtual bool GetDestinationIpPort(std::string *ip, int *port) const = 0; // Get Rbac attributes. struct RbacReportInfo { std::string permissive_resp_code; std::string permissive_policy_id; }; - virtual bool GetRbacReportInfo(RbacReportInfo* report_info) const = 0; + virtual bool GetRbacReportInfo(RbacReportInfo *report_info) const = 0; // Get upstream host UID. This value overrides the value in the report bag. - virtual bool GetDestinationUID(std::string* uid) const = 0; + virtual bool GetDestinationUID(std::string *uid) const = 0; // gRPC status info struct GrpcStatus { std::string status; std::string message; }; - virtual bool GetGrpcStatus(GrpcStatus* status) const = 0; + virtual bool GetGrpcStatus(GrpcStatus *status) const = 0; + + // Get dynamic metadata generated by Envoy filters. + // Useful for logging info generated by custom codecs. + virtual const ::google::protobuf::Map + &GetDynamicFilterState() const = 0; }; } // namespace http diff --git a/include/istio/control/tcp/report_data.h b/include/istio/control/tcp/report_data.h index 7535b5904ba..91a6b7241c0 100644 --- a/include/istio/control/tcp/report_data.h +++ b/include/istio/control/tcp/report_data.h @@ -19,6 +19,8 @@ #include #include +#include "google/protobuf/struct.pb.h" + namespace istio { namespace control { namespace tcp { @@ -30,7 +32,7 @@ class ReportData { virtual ~ReportData() {} // Get upstream tcp connection IP and port. IP is returned in format of bytes. - virtual bool GetDestinationIpPort(std::string* ip, int* port) const = 0; + virtual bool GetDestinationIpPort(std::string *ip, int *port) const = 0; // Get additional report data. struct ReportInfo { @@ -38,10 +40,10 @@ class ReportData { uint64_t received_bytes; std::chrono::nanoseconds duration; }; - virtual void GetReportInfo(ReportInfo* info) const = 0; + virtual void GetReportInfo(ReportInfo *info) const = 0; // Get upstream host UID. This value overrides the value in the report bag. - virtual bool GetDestinationUID(std::string* uid) const = 0; + virtual bool GetDestinationUID(std::string *uid) const = 0; // ConnectionEvent is used to indicates the tcp connection event in Report // call. @@ -50,6 +52,12 @@ class ReportData { CLOSE, CONTINUE, }; + + // Get dynamic metadata generated by Envoy filters. + // Useful for logging info generated by custom codecs. + virtual const ::google::protobuf::Map<::std::string, + ::google::protobuf::Struct> + &GetDynamicFilterState() const = 0; }; } // namespace tcp diff --git a/include/istio/utils/attributes_builder.h b/include/istio/utils/attributes_builder.h index 82e71e89747..c48d872775c 100644 --- a/include/istio/utils/attributes_builder.h +++ b/include/istio/utils/attributes_builder.h @@ -32,32 +32,32 @@ namespace utils { // .Add("key2", value2); class AttributesBuilder { public: - AttributesBuilder(::istio::mixer::v1::Attributes* attributes) + AttributesBuilder(::istio::mixer::v1::Attributes *attributes) : attributes_(attributes) {} - void AddString(const std::string& key, const std::string& str) { + void AddString(const std::string &key, const std::string &str) { (*attributes_->mutable_attributes())[key].set_string_value(str); } - void AddBytes(const std::string& key, const std::string& bytes) { + void AddBytes(const std::string &key, const std::string &bytes) { (*attributes_->mutable_attributes())[key].set_bytes_value(bytes); } - void AddInt64(const std::string& key, int64_t value) { + void AddInt64(const std::string &key, int64_t value) { (*attributes_->mutable_attributes())[key].set_int64_value(value); } - void AddDouble(const std::string& key, double value) { + void AddDouble(const std::string &key, double value) { (*attributes_->mutable_attributes())[key].set_double_value(value); } - void AddBool(const std::string& key, bool value) { + void AddBool(const std::string &key, bool value) { (*attributes_->mutable_attributes())[key].set_bool_value(value); } void AddTimestamp( - const std::string& key, - const std::chrono::time_point& value) { + const std::string &key, + const std::chrono::time_point &value) { auto time_stamp = (*attributes_->mutable_attributes())[key].mutable_timestamp_value(); long long nanos = std::chrono::duration_cast( @@ -67,16 +67,16 @@ class AttributesBuilder { time_stamp->set_nanos(nanos % 1000000000); } - void AddDuration(const std::string& key, - const std::chrono::nanoseconds& value) { + void AddDuration(const std::string &key, + const std::chrono::nanoseconds &value) { auto duration = (*attributes_->mutable_attributes())[key].mutable_duration_value(); duration->set_seconds(value.count() / 1000000000); duration->set_nanos(value.count() % 1000000000); } - void AddStringMap(const std::string& key, - const std::map& string_map) { + void AddStringMap(const std::string &key, + const std::map &string_map) { if (string_map.size() == 0) { return; } @@ -84,13 +84,13 @@ class AttributesBuilder { .mutable_string_map_value() ->mutable_entries(); entries->clear(); - for (const auto& map_it : string_map) { + for (const auto &map_it : string_map) { (*entries)[map_it.first] = map_it.second; } } - void AddProtoStructStringMap(const std::string& key, - const google::protobuf::Struct& struct_map) { + void AddProtoStructStringMap(const std::string &key, + const google::protobuf::Struct &struct_map) { if (struct_map.fields().empty()) { return; } @@ -98,7 +98,7 @@ class AttributesBuilder { .mutable_string_map_value() ->mutable_entries(); entries->clear(); - for (const auto& field : struct_map.fields()) { + for (const auto &field : struct_map.fields()) { // Ignore all fields that are not string or string list. switch (field.second.kind_case()) { case google::protobuf::Value::kStringValue: @@ -122,15 +122,37 @@ class AttributesBuilder { break; } } + + if (entries->empty()) { + attributes_->mutable_attributes()->erase(key); + } + } + + // Serializes all the keys in a map and builds attributes. + // for example, foo.bar.com: struct {str:abc, list:[c,d,e]} will be emitted as + // foo.bar.com: string_map[str:abc, list: c,d,e] + // Only extracts strings and lists. + // TODO: add the ability to pack bools and nums as strings and recurse down + // the struct. + void FlattenMapOfStringToStruct( + const ::google::protobuf::Map<::std::string, ::google::protobuf::Struct> + &filter_state) { + if (filter_state.empty()) { + return; + } + + for (const auto &filter : filter_state) { + AddProtoStructStringMap(filter.first, filter.second); + } } - bool HasAttribute(const std::string& key) const { - const auto& attrs_map = attributes_->attributes(); + bool HasAttribute(const std::string &key) const { + const auto &attrs_map = attributes_->attributes(); return attrs_map.find(key) != attrs_map.end(); } private: - ::istio::mixer::v1::Attributes* attributes_; + ::istio::mixer::v1::Attributes *attributes_; }; } // namespace utils diff --git a/istio.deps b/istio.deps index 78a04f1aa6c..654d778d445 100644 --- a/istio.deps +++ b/istio.deps @@ -11,6 +11,6 @@ "name": "ENVOY_SHA", "repoName": "envoyproxy/envoy", "file": "WORKSPACE", - "lastStableSHA": "c41fa7118e69a0872074d7a685a62331c5d5c17e" + "lastStableSHA": "74de08a0d4d31bd466639d25d681df5d290bb770" } ] diff --git a/src/envoy/http/mixer/report_data.h b/src/envoy/http/mixer/report_data.h index 161316a4e02..7d09992012d 100644 --- a/src/envoy/http/mixer/report_data.h +++ b/src/envoy/http/mixer/report_data.h @@ -20,6 +20,7 @@ #include "envoy/http/header_map.h" #include "envoy/stream_info/stream_info.h" #include "extensions/filters/http/well_known_names.h" +#include "google/protobuf/struct.pb.h" #include "include/istio/control/http/controller.h" #include "src/envoy/utils/utils.h" @@ -156,6 +157,12 @@ class ReportData : public ::istio::control::http::ReportData, return !report_info->permissive_resp_code.empty() || !report_info->permissive_policy_id.empty(); } + + // Get attributes generated by http filters + const ::google::protobuf::Map + &GetDynamicFilterState() const override { + return info_.dynamicMetadata().filter_metadata(); + } }; } // namespace Mixer diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc index 105db9d19b6..5043a44d7f2 100644 --- a/src/envoy/tcp/mixer/filter.cc +++ b/src/envoy/tcp/mixer/filter.cc @@ -24,7 +24,7 @@ namespace Envoy { namespace Tcp { namespace Mixer { -Filter::Filter(Control& control) : control_(control) { +Filter::Filter(Control &control) : control_(control) { ENVOY_LOG(debug, "Called tcp filter: {}", __func__); } @@ -34,7 +34,7 @@ Filter::~Filter() { } void Filter::initializeReadFilterCallbacks( - Network::ReadFilterCallbacks& callbacks) { + Network::ReadFilterCallbacks &callbacks) { ENVOY_LOG(debug, "Called tcp filter: {}", __func__); filter_callbacks_ = &callbacks; filter_callbacks_->connection().addConnectionCallbacks(*this); @@ -60,14 +60,14 @@ void Filter::callCheck() { state_ = State::Calling; filter_callbacks_->connection().readDisable(true); calling_check_ = true; - cancel_check_ = handler_->Check(this, [this](const CheckResponseInfo& info) { + cancel_check_ = handler_->Check(this, [this](const CheckResponseInfo &info) { completeCheck(info.response_status); }); calling_check_ = false; } // Network::ReadFilter -Network::FilterStatus Filter::onData(Buffer::Instance& data, bool) { +Network::FilterStatus Filter::onData(Buffer::Instance &data, bool) { if (state_ == State::NotStarted) { // By waiting to invoke the callCheck() at onData(), the call to Mixer // will have sufficient SSL information to fill the check Request. @@ -83,7 +83,7 @@ Network::FilterStatus Filter::onData(Buffer::Instance& data, bool) { } // Network::WriteFilter -Network::FilterStatus Filter::onWrite(Buffer::Instance& data, bool) { +Network::FilterStatus Filter::onWrite(Buffer::Instance &data, bool) { ENVOY_CONN_LOG(debug, "Called tcp filter onWrite bytes: {}", filter_callbacks_->connection(), data.length()); send_bytes_ += data.length(); @@ -101,7 +101,7 @@ Network::FilterStatus Filter::onNewConnection() { return Network::FilterStatus::Continue; } -void Filter::completeCheck(const Status& status) { +void Filter::completeCheck(const Status &status) { ENVOY_LOG(debug, "Called tcp filter completeCheck: {}", status.ToString()); cancel_check_ = nullptr; if (state_ == State::Closed) { @@ -146,11 +146,11 @@ void Filter::onEvent(Network::ConnectionEvent event) { } } -bool Filter::GetSourceIpPort(std::string* str_ip, int* port) const { +bool Filter::GetSourceIpPort(std::string *str_ip, int *port) const { return Utils::GetIpPort(filter_callbacks_->connection().remoteAddress()->ip(), str_ip, port); } -bool Filter::GetPrincipal(bool peer, std::string* user) const { +bool Filter::GetPrincipal(bool peer, std::string *user) const { return Utils::GetPrincipal(&filter_callbacks_->connection(), peer, user); } @@ -158,11 +158,11 @@ bool Filter::IsMutualTLS() const { return Utils::IsMutualTLS(&filter_callbacks_->connection()); } -bool Filter::GetRequestedServerName(std::string* name) const { +bool Filter::GetRequestedServerName(std::string *name) const { return Utils::GetRequestedServerName(&filter_callbacks_->connection(), name); } -bool Filter::GetDestinationIpPort(std::string* str_ip, int* port) const { +bool Filter::GetDestinationIpPort(std::string *str_ip, int *port) const { if (filter_callbacks_->upstreamHost() && filter_callbacks_->upstreamHost()->address()) { return Utils::GetIpPort(filter_callbacks_->upstreamHost()->address()->ip(), @@ -170,15 +170,22 @@ bool Filter::GetDestinationIpPort(std::string* str_ip, int* port) const { } return false; } -bool Filter::GetDestinationUID(std::string* uid) const { +bool Filter::GetDestinationUID(std::string *uid) const { if (filter_callbacks_->upstreamHost()) { return Utils::GetDestinationUID( *filter_callbacks_->upstreamHost()->metadata(), uid); } return false; } +const ::google::protobuf::Map + &Filter::GetDynamicFilterState() const { + return filter_callbacks_->connection() + .streamInfo() + .dynamicMetadata() + .filter_metadata(); +} void Filter::GetReportInfo( - ::istio::control::tcp::ReportData::ReportInfo* data) const { + ::istio::control::tcp::ReportData::ReportInfo *data) const { data->received_bytes = received_bytes_; data->send_bytes = send_bytes_; data->duration = std::chrono::duration_cast( diff --git a/src/envoy/tcp/mixer/filter.h b/src/envoy/tcp/mixer/filter.h index a06daa9f852..8bd13ca27d2 100644 --- a/src/envoy/tcp/mixer/filter.h +++ b/src/envoy/tcp/mixer/filter.h @@ -18,6 +18,7 @@ #include "common/common/logger.h" #include "envoy/network/connection.h" #include "envoy/network/filter.h" +#include "google/protobuf/struct.pb.h" #include "src/envoy/tcp/mixer/control.h" namespace Envoy { @@ -30,17 +31,17 @@ class Filter : public Network::Filter, public ::istio::control::tcp::ReportData, public Logger::Loggable { public: - Filter(Control& control); + Filter(Control &control); ~Filter(); void initializeReadFilterCallbacks( - Network::ReadFilterCallbacks& callbacks) override; + Network::ReadFilterCallbacks &callbacks) override; // Network::ReadFilter - Network::FilterStatus onData(Buffer::Instance& data, bool) override; + Network::FilterStatus onData(Buffer::Instance &data, bool) override; // Network::WriteFilter - Network::FilterStatus onWrite(Buffer::Instance& data, bool) override; + Network::FilterStatus onWrite(Buffer::Instance &data, bool) override; Network::FilterStatus onNewConnection() override; // Network::ConnectionCallbacks @@ -50,16 +51,18 @@ class Filter : public Network::Filter, void onBelowWriteBufferLowWatermark() override {} // CheckData virtual functions. - bool GetSourceIpPort(std::string* str_ip, int* port) const override; - bool GetPrincipal(bool peer, std::string* user) const override; + bool GetSourceIpPort(std::string *str_ip, int *port) const override; + bool GetPrincipal(bool peer, std::string *user) const override; bool IsMutualTLS() const override; - bool GetRequestedServerName(std::string* name) const override; + bool GetRequestedServerName(std::string *name) const override; // ReportData virtual functions. - bool GetDestinationIpPort(std::string* str_ip, int* port) const override; - bool GetDestinationUID(std::string* uid) const override; + bool GetDestinationIpPort(std::string *str_ip, int *port) const override; + bool GetDestinationUID(std::string *uid) const override; + const ::google::protobuf::Map + &GetDynamicFilterState() const override; void GetReportInfo( - ::istio::control::tcp::ReportData::ReportInfo* data) const override; + ::istio::control::tcp::ReportData::ReportInfo *data) const override; std::string GetConnectionId() const override; private: @@ -72,7 +75,7 @@ class Filter : public Network::Filter, void callCheck(); // Called when Check is done. - void completeCheck(const ::google::protobuf::util::Status& status); + void completeCheck(const ::google::protobuf::util::Status &status); // Cancel the pending Check call. void cancelCheck(); @@ -80,11 +83,11 @@ class Filter : public Network::Filter, // the cancel check istio::mixerclient::CancelFunc cancel_check_; // the control object. - Control& control_; + Control &control_; // pre-request handler std::unique_ptr<::istio::control::tcp::RequestHandler> handler_; // filter callback - Network::ReadFilterCallbacks* filter_callbacks_{}; + Network::ReadFilterCallbacks *filter_callbacks_{}; // state State state_{State::NotStarted}; // calling_check diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index 2c0babcbabf..5e4e69fdd68 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -252,6 +252,8 @@ void AttributesBuilder::ExtractReportAttributes(ReportData *report_data) { rbac_info.permissive_policy_id); } } + + builder.FlattenMapOfStringToStruct(report_data->GetDynamicFilterState()); } } // namespace http diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index f14c32cb98d..31c14b947e9 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -31,6 +31,7 @@ using ::istio::mixer::v1::Attributes_StringMap; using ::testing::_; using ::testing::Invoke; +using ::testing::ReturnRef; namespace istio { namespace control { @@ -406,6 +407,21 @@ attributes { string_value: "policy-foo" } } +attributes { + key: "foo.bar.com" + value { + string_map_value { + entries { + key: "str" + value: "abc" + } + entries { + key: "list" + value: "a,b,c" + } + } + } +} )"; constexpr char kAuthenticationResultStruct[] = R"( @@ -710,6 +726,23 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { TEST(AttributesBuilderTest, TestReportAttributes) { ::testing::StrictMock mock_data; + + ::google::protobuf::Map + filter_metadata; + ::google::protobuf::Struct struct_obj; + ::google::protobuf::Value strval, numval, boolval, listval; + strval.set_string_value("abc"); + (*struct_obj.mutable_fields())["str"] = strval; + numval.set_number_value(12.3); + (*struct_obj.mutable_fields())["num"] = numval; + boolval.set_bool_value(true); + (*struct_obj.mutable_fields())["bool"] = boolval; + listval.mutable_list_value()->add_values()->set_string_value("a"); + listval.mutable_list_value()->add_values()->set_string_value("b"); + listval.mutable_list_value()->add_values()->set_string_value("c"); + (*struct_obj.mutable_fields())["list"] = listval; + filter_metadata["foo.bar.com"] = struct_obj; + EXPECT_CALL(mock_data, GetDestinationIpPort(_, _)) .WillOnce(Invoke([](std::string *ip, int *port) -> bool { *ip = "1.2.3.4"; @@ -750,6 +783,8 @@ TEST(AttributesBuilderTest, TestReportAttributes) { report_info->permissive_policy_id = "policy-foo"; return true; })); + EXPECT_CALL(mock_data, GetDynamicFilterState()) + .WillOnce(ReturnRef(filter_metadata)); RequestContext request; AttributesBuilder builder(&request); @@ -774,6 +809,23 @@ TEST(AttributesBuilderTest, TestReportAttributes) { TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) { ::testing::StrictMock mock_data; + + ::google::protobuf::Map + filter_metadata; + ::google::protobuf::Struct struct_obj; + ::google::protobuf::Value strval, numval, boolval, listval; + strval.set_string_value("abc"); + (*struct_obj.mutable_fields())["str"] = strval; + numval.set_number_value(12.3); + (*struct_obj.mutable_fields())["num"] = numval; + boolval.set_bool_value(true); + (*struct_obj.mutable_fields())["bool"] = boolval; + listval.mutable_list_value()->add_values()->set_string_value("a"); + listval.mutable_list_value()->add_values()->set_string_value("b"); + listval.mutable_list_value()->add_values()->set_string_value("c"); + (*struct_obj.mutable_fields())["list"] = listval; + filter_metadata["foo.bar.com"] = struct_obj; + EXPECT_CALL(mock_data, GetDestinationIpPort(_, _)) .WillOnce(Invoke([](std::string *ip, int *port) -> bool { *ip = "2.3.4.5"; @@ -805,6 +857,8 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) { report_info->permissive_policy_id = "policy-foo"; return true; })); + EXPECT_CALL(mock_data, GetDynamicFilterState()) + .WillOnce(ReturnRef(filter_metadata)); RequestContext request; SetDestinationIp(&request, "1.2.3.4"); diff --git a/src/istio/control/http/mock_report_data.h b/src/istio/control/http/mock_report_data.h index 423a1b6ef4f..64c0402ef9b 100644 --- a/src/istio/control/http/mock_report_data.h +++ b/src/istio/control/http/mock_report_data.h @@ -27,11 +27,15 @@ namespace http { class MockReportData : public ReportData { public: MOCK_CONST_METHOD0(GetResponseHeaders, std::map()); - MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo* info)); - MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string* ip, int* port)); - MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string* ip)); - MOCK_CONST_METHOD1(GetGrpcStatus, bool(GrpcStatus* status)); - MOCK_CONST_METHOD1(GetRbacReportInfo, bool(RbacReportInfo* info)); + MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo *info)); + MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string *ip, int *port)); + MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string *ip)); + MOCK_CONST_METHOD1(GetGrpcStatus, bool(GrpcStatus *status)); + MOCK_CONST_METHOD1(GetRbacReportInfo, bool(RbacReportInfo *info)); + MOCK_CONST_METHOD0( + GetDynamicFilterState, + const ::google::protobuf::Map + &()); }; } // namespace http diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc index eb5700f3dbe..4fc87aa12f0 100644 --- a/src/istio/control/http/request_handler_impl_test.cc +++ b/src/istio/control/http/request_handler_impl_test.cc @@ -38,6 +38,7 @@ using ::istio::utils::LocalAttributes; using ::testing::_; using ::testing::Invoke; +using ::testing::ReturnRef; namespace istio { namespace control { @@ -129,15 +130,15 @@ class RequestHandlerImplTest : public ::testing::Test { public: void SetUp() { SetUpMockController(kDefaultClientConfig); } - void SetUpMockController(const std::string& config_text) { + void SetUpMockController(const std::string &config_text) { SetUpMockController(config_text, kLocalInbound, kLocalOutbound, kLocalForward); } - void SetUpMockController(const std::string& config_text, - const std::string& local_inbound_attributes, - const std::string& local_outbound_attributes, - const std::string& local_forward_attributes) { + void SetUpMockController(const std::string &config_text, + const std::string &local_inbound_attributes, + const std::string &local_outbound_attributes, + const std::string &local_forward_attributes) { ASSERT_TRUE(TextFormat::ParseFromString(config_text, &client_config_)); LocalAttributes la; @@ -158,19 +159,19 @@ class RequestHandlerImplTest : public ::testing::Test { std::unique_ptr(new ControllerImpl(client_context_)); } - void SetServiceConfig(const std::string& name, const ServiceConfig& config) { + void SetServiceConfig(const std::string &name, const ServiceConfig &config) { (*client_config_.mutable_service_configs())[name] = config; } - void ApplyPerRouteConfig(const ServiceConfig& service_config, - Controller::PerRouteConfig* per_route) { + void ApplyPerRouteConfig(const ServiceConfig &service_config, + Controller::PerRouteConfig *per_route) { per_route->service_config_id = "1111"; controller_->AddServiceConfig(per_route->service_config_id, service_config); } std::shared_ptr client_context_; HttpClientConfig client_config_; - ::testing::NiceMock* mock_client_; + ::testing::NiceMock *mock_client_; std::unique_ptr controller_; }; @@ -210,7 +211,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheckReport) { auto handler = controller_->CreateRequestHandler(per_route); handler->Check(&mock_data, &mock_header, nullptr, - [](const CheckResponseInfo& info) { + [](const CheckResponseInfo &info) { EXPECT_TRUE(info.response_status.ok()); }); } @@ -232,7 +233,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) { auto handler = controller_->CreateRequestHandler(per_route); handler->Check(&mock_data, &mock_header, nullptr, - [](const CheckResponseInfo& info) { + [](const CheckResponseInfo &info) { EXPECT_TRUE(info.response_status.ok()); }); } @@ -245,8 +246,8 @@ TEST_F(RequestHandlerImplTest, TestPerRouteAttributes) { // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) - .WillOnce(Invoke([](const Attributes& attributes, - const std::vector& quotas, + .WillOnce(Invoke([](const Attributes &attributes, + const std::vector "as, TransportCheckFunc transport, CheckDoneFunc on_done) -> CancelFunc { auto map = attributes.attributes(); @@ -273,8 +274,8 @@ TEST_F(RequestHandlerImplTest, TestDefaultRouteAttributes) { // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) - .WillOnce(Invoke([](const Attributes& attributes, - const std::vector& quotas, + .WillOnce(Invoke([](const Attributes &attributes, + const std::vector "as, TransportCheckFunc transport, CheckDoneFunc on_done) -> CancelFunc { auto map = attributes.attributes(); @@ -285,7 +286,7 @@ TEST_F(RequestHandlerImplTest, TestDefaultRouteAttributes) { // Attribute is forwarded: route override EXPECT_CALL(mock_header, AddIstioAttributes(_)) - .WillOnce(Invoke([](const std::string& data) { + .WillOnce(Invoke([](const std::string &data) { Attributes forwarded_attr; EXPECT_TRUE(forwarded_attr.ParseFromString(data)); auto map = forwarded_attr.attributes(); @@ -312,8 +313,8 @@ TEST_F(RequestHandlerImplTest, TestRouteAttributes) { // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) - .WillOnce(Invoke([](const Attributes& attributes, - const std::vector& quotas, + .WillOnce(Invoke([](const Attributes &attributes, + const std::vector "as, TransportCheckFunc transport, CheckDoneFunc on_done) -> CancelFunc { auto map = attributes.attributes(); @@ -324,7 +325,7 @@ TEST_F(RequestHandlerImplTest, TestRouteAttributes) { // Attribute is forwarded: global EXPECT_CALL(mock_header, AddIstioAttributes(_)) - .WillOnce(Invoke([](const std::string& data) { + .WillOnce(Invoke([](const std::string &data) { Attributes forwarded_attr; EXPECT_TRUE(forwarded_attr.ParseFromString(data)); auto map = forwarded_attr.attributes(); @@ -343,8 +344,8 @@ TEST_F(RequestHandlerImplTest, TestPerRouteQuota) { // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) - .WillOnce(Invoke([](const Attributes& attributes, - const std::vector& quotas, + .WillOnce(Invoke([](const Attributes &attributes, + const std::vector "as, TransportCheckFunc transport, CheckDoneFunc on_done) -> CancelFunc { auto map = attributes.attributes(); @@ -371,7 +372,7 @@ TEST_F(RequestHandlerImplTest, TestPerRouteApiSpec) { ::testing::NiceMock mock_header; EXPECT_CALL(mock_data, FindHeaderByType(_, _)) .WillRepeatedly( - Invoke([](CheckData::HeaderType type, std::string* value) -> bool { + Invoke([](CheckData::HeaderType type, std::string *value) -> bool { if (type == CheckData::HEADER_PATH) { *value = "/books/120"; return true; @@ -385,8 +386,8 @@ TEST_F(RequestHandlerImplTest, TestPerRouteApiSpec) { // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) - .WillOnce(Invoke([](const Attributes& attributes, - const std::vector& quotas, + .WillOnce(Invoke([](const Attributes &attributes, + const std::vector "as, TransportCheckFunc transport, CheckDoneFunc on_done) -> CancelFunc { auto map = attributes.attributes(); @@ -435,7 +436,7 @@ TEST_F(RequestHandlerImplTest, TestDefaultApiKey) { ::testing::NiceMock mock_header; EXPECT_CALL(mock_data, FindQueryParameter(_, _)) .WillRepeatedly( - Invoke([](const std::string& name, std::string* value) -> bool { + Invoke([](const std::string &name, std::string *value) -> bool { if (name == "key") { *value = "test-api-key"; return true; @@ -445,8 +446,8 @@ TEST_F(RequestHandlerImplTest, TestDefaultApiKey) { // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) - .WillOnce(Invoke([](const Attributes& attributes, - const std::vector& quotas, + .WillOnce(Invoke([](const Attributes &attributes, + const std::vector "as, TransportCheckFunc transport, CheckDoneFunc on_done) -> CancelFunc { auto map = attributes.attributes(); @@ -464,9 +465,14 @@ TEST_F(RequestHandlerImplTest, TestDefaultApiKey) { TEST_F(RequestHandlerImplTest, TestHandlerReport) { ::testing::NiceMock mock_check; ::testing::NiceMock mock_report; + ::google::protobuf::Map + filter_metadata; EXPECT_CALL(mock_check, GetSourceIpPort(_, _)).Times(1); EXPECT_CALL(mock_report, GetResponseHeaders()).Times(1); EXPECT_CALL(mock_report, GetReportInfo(_)).Times(1); + EXPECT_CALL(mock_report, GetDynamicFilterState()) + .Times(1) + .WillOnce(ReturnRef(filter_metadata)); // Report should be called. EXPECT_CALL(*mock_client_, Report(_)).Times(1); @@ -485,6 +491,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledReport) { EXPECT_CALL(mock_check, GetSourceIpPort(_, _)).Times(0); EXPECT_CALL(mock_report, GetResponseHeaders()).Times(0); EXPECT_CALL(mock_report, GetReportInfo(_)).Times(0); + EXPECT_CALL(mock_report, GetDynamicFilterState()).Times(0); // Report should NOT be called. EXPECT_CALL(*mock_client_, Report(_)).Times(0); @@ -509,7 +516,7 @@ TEST_F(RequestHandlerImplTest, TestEmptyConfig) { // Attributes is forwarded. EXPECT_CALL(mock_header, AddIstioAttributes(_)) - .WillOnce(Invoke([](const std::string& data) { + .WillOnce(Invoke([](const std::string &data) { Attributes forwarded_attr; EXPECT_TRUE(forwarded_attr.ParseFromString(data)); auto map = forwarded_attr.attributes(); @@ -522,6 +529,7 @@ TEST_F(RequestHandlerImplTest, TestEmptyConfig) { ::testing::NiceMock mock_report; EXPECT_CALL(mock_report, GetResponseHeaders()).Times(0); EXPECT_CALL(mock_report, GetReportInfo(_)).Times(0); + EXPECT_CALL(mock_report, GetDynamicFilterState()).Times(0); // Report should NOT be called. EXPECT_CALL(*mock_client_, Report(_)).Times(0); @@ -529,7 +537,7 @@ TEST_F(RequestHandlerImplTest, TestEmptyConfig) { Controller::PerRouteConfig config; auto handler = controller_->CreateRequestHandler(config); handler->Check(&mock_check, &mock_header, nullptr, - [](const CheckResponseInfo& info) { + [](const CheckResponseInfo &info) { EXPECT_TRUE(info.response_status.ok()); }); handler->Report(&mock_check, &mock_report); diff --git a/src/istio/control/tcp/attributes_builder.cc b/src/istio/control/tcp/attributes_builder.cc index 47e2dfa2c9c..0c8876e7923 100644 --- a/src/istio/control/tcp/attributes_builder.cc +++ b/src/istio/control/tcp/attributes_builder.cc @@ -29,7 +29,7 @@ const std::string kConnectionContinue("continue"); const std::string kConnectionClose("close"); } // namespace -void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { +void AttributesBuilder::ExtractCheckAttributes(CheckData *check_data) { utils::AttributesBuilder builder(request_->attributes); std::string source_ip; @@ -81,8 +81,8 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { } void AttributesBuilder::ExtractReportAttributes( - ReportData* report_data, ReportData::ConnectionEvent event, - ReportData::ReportInfo* last_report_info) { + ReportData *report_data, ReportData::ConnectionEvent event, + ReportData::ReportInfo *last_report_info) { utils::AttributesBuilder builder(request_->attributes); ReportData::ReportInfo info; @@ -132,6 +132,8 @@ void AttributesBuilder::ExtractReportAttributes( builder.AddString(utils::AttributeName::kDestinationUID, uid); } + builder.FlattenMapOfStringToStruct(report_data->GetDynamicFilterState()); + builder.AddTimestamp(utils::AttributeName::kContextTime, std::chrono::system_clock::now()); } diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index 1ce0afe0aa5..897b7652d0e 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -30,6 +30,7 @@ using ::google::protobuf::util::MessageDifferencer; using ::testing::_; using ::testing::Invoke; using ::testing::Return; +using ::testing::ReturnRef; namespace istio { namespace control { @@ -162,6 +163,21 @@ attributes { string_value: "pod1.ns2" } } +attributes { + key: "foo.bar.com" + value { + string_map_value { + entries { + key: "str" + value: "abc" + } + entries { + key: "list" + value: "a,b,c" + } + } + } +} )"; const char kReportAttributes[] = R"( @@ -240,6 +256,21 @@ attributes { string_value: "pod1.ns2" } } +attributes { + key: "foo.bar.com" + value { + string_map_value { + entries { + key: "str" + value: "abc" + } + entries { + key: "list" + value: "a,b,c" + } + } + } +} )"; const char kDeltaOneReportAttributes[] = R"( @@ -298,6 +329,21 @@ attributes { string_value: "pod1.ns2" } } +attributes { + key: "foo.bar.com" + value { + string_map_value { + entries { + key: "str" + value: "abc" + } + entries { + key: "list" + value: "a,b,c" + } + } + } +} )"; const char kDeltaTwoReportAttributes[] = R"( @@ -356,9 +402,24 @@ attributes { string_value: "pod1.ns2" } } +attributes { + key: "foo.bar.com" + value { + string_map_value { + entries { + key: "str" + value: "abc" + } + entries { + key: "list" + value: "a,b,c" + } + } + } +} )"; -void ClearContextTime(RequestContext* request) { +void ClearContextTime(RequestContext *request) { // Override timestamp with - utils::AttributesBuilder builder(request->attributes); std::chrono::time_point time0; @@ -368,7 +429,7 @@ void ClearContextTime(RequestContext* request) { TEST(AttributesBuilderTest, TestCheckAttributes) { ::testing::NiceMock mock_data; EXPECT_CALL(mock_data, GetSourceIpPort(_, _)) - .WillOnce(Invoke([](std::string* ip, int* port) -> bool { + .WillOnce(Invoke([](std::string *ip, int *port) -> bool { *ip = "1.2.3.4"; *port = 8080; return true; @@ -377,7 +438,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { return true; })); EXPECT_CALL(mock_data, GetPrincipal(_, _)) - .WillRepeatedly(Invoke([](bool peer, std::string* user) -> bool { + .WillRepeatedly(Invoke([](bool peer, std::string *user) -> bool { if (peer) { *user = "cluster.local/sa/test_user/ns/ns_ns/"; } else { @@ -387,7 +448,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { })); EXPECT_CALL(mock_data, GetConnectionId()).WillOnce(Return("1234-5")); EXPECT_CALL(mock_data, GetRequestedServerName(_)) - .WillOnce(Invoke([](std::string* name) -> bool { + .WillOnce(Invoke([](std::string *name) -> bool { *name = "www.google.com"; return true; })); @@ -410,37 +471,57 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { TEST(AttributesBuilderTest, TestReportAttributes) { ::testing::NiceMock mock_data; + + ::google::protobuf::Map + filter_metadata; + ::google::protobuf::Struct struct_obj; + ::google::protobuf::Value strval, numval, boolval, listval; + strval.set_string_value("abc"); + (*struct_obj.mutable_fields())["str"] = strval; + numval.set_number_value(12.3); + (*struct_obj.mutable_fields())["num"] = numval; + boolval.set_bool_value(true); + (*struct_obj.mutable_fields())["bool"] = boolval; + listval.mutable_list_value()->add_values()->set_string_value("a"); + listval.mutable_list_value()->add_values()->set_string_value("b"); + listval.mutable_list_value()->add_values()->set_string_value("c"); + (*struct_obj.mutable_fields())["list"] = listval; + filter_metadata["foo.bar.com"] = struct_obj; + EXPECT_CALL(mock_data, GetDestinationIpPort(_, _)) .Times(4) - .WillRepeatedly(Invoke([](std::string* ip, int* port) -> bool { + .WillRepeatedly(Invoke([](std::string *ip, int *port) -> bool { *ip = "1.2.3.4"; *port = 8080; return true; })); EXPECT_CALL(mock_data, GetDestinationUID(_)) .Times(4) - .WillRepeatedly(Invoke([](std::string* uid) -> bool { + .WillRepeatedly(Invoke([](std::string *uid) -> bool { *uid = "pod1.ns2"; return true; })); + EXPECT_CALL(mock_data, GetDynamicFilterState()) + .Times(4) + .WillRepeatedly(ReturnRef(filter_metadata)); EXPECT_CALL(mock_data, GetReportInfo(_)) .Times(4) - .WillOnce(Invoke([](ReportData::ReportInfo* info) { + .WillOnce(Invoke([](ReportData::ReportInfo *info) { info->received_bytes = 0; info->send_bytes = 0; info->duration = std::chrono::nanoseconds(1); })) - .WillOnce(Invoke([](ReportData::ReportInfo* info) { + .WillOnce(Invoke([](ReportData::ReportInfo *info) { info->received_bytes = 100; info->send_bytes = 200; info->duration = std::chrono::nanoseconds(2); })) - .WillOnce(Invoke([](ReportData::ReportInfo* info) { + .WillOnce(Invoke([](ReportData::ReportInfo *info) { info->received_bytes = 201; info->send_bytes = 404; info->duration = std::chrono::nanoseconds(3); })) - .WillOnce(Invoke([](ReportData::ReportInfo* info) { + .WillOnce(Invoke([](ReportData::ReportInfo *info) { info->received_bytes = 345; info->send_bytes = 678; info->duration = std::chrono::nanoseconds(4); diff --git a/src/istio/control/tcp/mock_report_data.h b/src/istio/control/tcp/mock_report_data.h index 65c18492c82..85c749ff360 100644 --- a/src/istio/control/tcp/mock_report_data.h +++ b/src/istio/control/tcp/mock_report_data.h @@ -26,9 +26,13 @@ namespace tcp { // The mock object for ReportData interface. class MockReportData : public ReportData { public: - MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string* ip, int* port)); - MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string*)); - MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo* info)); + MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string *ip, int *port)); + MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string *)); + MOCK_CONST_METHOD0( + GetDynamicFilterState, + const ::google::protobuf::Map + &()); + MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo *info)); }; } // namespace tcp diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc index eb8b9559c94..c27763b3bc4 100644 --- a/src/istio/control/tcp/request_handler_impl_test.cc +++ b/src/istio/control/tcp/request_handler_impl_test.cc @@ -36,6 +36,7 @@ using ::istio::utils::LocalAttributes; using ::testing::_; using ::testing::Invoke; +using ::testing::ReturnRef; namespace istio { namespace control { @@ -97,7 +98,7 @@ class RequestHandlerImplTest : public ::testing::Test { std::shared_ptr client_context_; TcpClientConfig client_config_; - ::testing::NiceMock* mock_client_; + ::testing::NiceMock *mock_client_; std::unique_ptr controller_; }; @@ -111,7 +112,7 @@ TEST_F(RequestHandlerImplTest, TestHandlerDisabledCheck) { client_config_.set_disable_check_calls(true); auto handler = controller_->CreateRequestHandler(); - handler->Check(&mock_data, [](const CheckResponseInfo& info) { + handler->Check(&mock_data, [](const CheckResponseInfo &info) { EXPECT_TRUE(info.response_status.ok()); }); } @@ -123,8 +124,8 @@ TEST_F(RequestHandlerImplTest, TestHandlerCheck) { // Check should be called. EXPECT_CALL(*mock_client_, Check(_, _, _, _)) - .WillOnce(Invoke([](const Attributes& attributes, - const std::vector& quotas, + .WillOnce(Invoke([](const Attributes &attributes, + const std::vector "as, TransportCheckFunc transport, CheckDoneFunc on_done) -> CancelFunc { auto map = attributes.attributes(); @@ -141,9 +142,14 @@ TEST_F(RequestHandlerImplTest, TestHandlerCheck) { TEST_F(RequestHandlerImplTest, TestHandlerReport) { ::testing::NiceMock mock_data; + ::google::protobuf::Map + filter_metadata; EXPECT_CALL(mock_data, GetDestinationIpPort(_, _)).Times(1); EXPECT_CALL(mock_data, GetDestinationUID(_)).Times(1); EXPECT_CALL(mock_data, GetReportInfo(_)).Times(1); + EXPECT_CALL(mock_data, GetDynamicFilterState()) + .Times(1) + .WillOnce(ReturnRef(filter_metadata)); // Report should be called. EXPECT_CALL(*mock_client_, Report(_)).Times(1); diff --git a/src/istio/utils/utils.h b/src/istio/utils/utils.h index 063c18ca402..0e27aa82d27 100644 --- a/src/istio/utils/utils.h +++ b/src/istio/utils/utils.h @@ -16,6 +16,7 @@ #pragma once #include +#include "google/protobuf/struct.pb.h" namespace istio { namespace utils { @@ -23,6 +24,5 @@ namespace utils { // Get source.namespace attribute from principal. bool GetSourceNamespace(const std::string& principal, std::string* source_namespace); - } // namespace utils } // namespace istio