From 5d09f41a26db4d1846bf69748b5788a8913b4c76 Mon Sep 17 00:00:00 2001 From: Pengyuan Bian <34738376+bianpengyuan@users.noreply.github.com> Date: Thu, 9 Aug 2018 18:32:30 -0700 Subject: [PATCH] Add two new attributes: request.url_path and request.queries (#1837) * Add two new attributes: request.url_path and request.queries * Update api in repositories.bzl --- include/istio/control/http/check_data.h | 9 +++ include/istio/utils/attribute_names.h | 2 + istio.deps | 4 +- repositories.bzl | 3 +- src/envoy/http/mixer/check_data.cc | 23 ++++++ src/envoy/http/mixer/check_data.h | 5 ++ src/istio/control/http/attributes_builder.cc | 10 +++ .../control/http/attributes_builder_test.cc | 79 +++++++++++++++++-- src/istio/control/http/mock_check_data.h | 3 + src/istio/utils/attribute_names.cc | 2 + 10 files changed, 130 insertions(+), 10 deletions(-) diff --git a/include/istio/control/http/check_data.h b/include/istio/control/http/check_data.h index 10889524518..a05211888c1 100644 --- a/include/istio/control/http/check_data.h +++ b/include/istio/control/http/check_data.h @@ -84,6 +84,15 @@ class CheckData { // Returns a pointer to the authentication result from request info dynamic // metadata, if available. Otherwise, returns nullptr. virtual const ::google::protobuf::Struct *GetAuthenticationResult() const = 0; + + // Get request url path, which strips query part from the http path header. + // Return true if url path is found, otherwise return false. + virtual bool GetUrlPath(std::string *url_path) const = 0; + + // Get request queries with string map format. Return true if query params are + // found, otherwise return false. + virtual bool GetRequestQueryParams( + std::map *query_params) const = 0; }; // An interfact to update request HTTP headers with Istio attributes. diff --git a/include/istio/utils/attribute_names.h b/include/istio/utils/attribute_names.h index 90bafc9f8d3..84e7415837d 100644 --- a/include/istio/utils/attribute_names.h +++ b/include/istio/utils/attribute_names.h @@ -35,6 +35,8 @@ struct AttributeName { static const char kRequestPath[]; static const char kRequestReferer[]; static const char kRequestScheme[]; + static const char kRequestUrlPath[]; + static const char kRequestQueryParams[]; static const char kRequestBodySize[]; // Total size of request received, including request headers, body, and // trailers. diff --git a/istio.deps b/istio.deps index 52e53ec1b35..b8489db2c0d 100644 --- a/istio.deps +++ b/istio.deps @@ -4,7 +4,7 @@ "name": "ISTIO_API", "repoName": "api", "file": "repositories.bzl", - "lastStableSHA": "123b0a79a4dba5bd653dcac09cf731b8002e5341" + "lastStableSHA": "85f06ac32da4744449da69643bf9d4e149e14892" }, { "_comment": "", @@ -13,4 +13,4 @@ "file": "WORKSPACE", "lastStableSHA": "346059548e135199eb0b7f0006f3ef19e173bf79" } -] \ No newline at end of file +] diff --git a/repositories.bzl b/repositories.bzl index facc92a915f..4be45a190fd 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -113,7 +113,7 @@ cc_library( actual = "@googletest_git//:googletest_prod", ) -ISTIO_API = "123b0a79a4dba5bd653dcac09cf731b8002e5341" +ISTIO_API = "85f06ac32da4744449da69643bf9d4e149e14892" def mixerapi_repositories(bind=True): BUILD = """ @@ -168,6 +168,7 @@ cc_proto_library( srcs = glob( ["envoy/config/filter/http/authn/v2alpha1/*.proto", "authentication/v1alpha1/*.proto", + "common/v1alpha1/*.proto", ], ), default_runtime = "//external:protobuf", diff --git a/src/envoy/http/mixer/check_data.cc b/src/envoy/http/mixer/check_data.cc index 88f2f455e7f..0cc991e1381 100644 --- a/src/envoy/http/mixer/check_data.cc +++ b/src/envoy/http/mixer/check_data.cc @@ -171,6 +171,29 @@ const ::google::protobuf::Struct* CheckData::GetAuthenticationResult() const { return Utils::Authentication::GetResultFromMetadata(metadata_); } +bool CheckData::GetUrlPath(std::string* url_path) const { + if (!headers_.Path()) { + return false; + } + const HeaderString& path = headers_.Path()->value(); + const char* query_start = Utility::findQueryStringStart(path); + if (query_start != nullptr) { + *url_path = std::string(path.c_str(), query_start - path.c_str()); + } else { + *url_path = std::string(path.c_str(), path.size()); + } + return true; +} + +bool CheckData::GetRequestQueryParams( + std::map* query_params) const { + if (!headers_.Path()) { + return false; + } + *query_params = Utility::parseQueryString(headers_.Path()->value().c_str()); + return true; +} + } // namespace Mixer } // namespace Http } // namespace Envoy diff --git a/src/envoy/http/mixer/check_data.h b/src/envoy/http/mixer/check_data.h index ed5cce45509..88a36ad41f9 100644 --- a/src/envoy/http/mixer/check_data.h +++ b/src/envoy/http/mixer/check_data.h @@ -62,6 +62,11 @@ class CheckData : public ::istio::control::http::CheckData, const ::google::protobuf::Struct* GetAuthenticationResult() const override; + bool GetUrlPath(std::string* url_path) const override; + + bool GetRequestQueryParams( + std::map* query_params) const override; + private: const HeaderMap& headers_; const envoy::api::v2::core::Metadata& metadata_; diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index deb23eacd98..d7e326eff61 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -66,6 +66,16 @@ void AttributesBuilder::ExtractRequestHeaderAttributes(CheckData *check_data) { builder.AddString(it.name, it.default_value); } } + + std::string query_path; + if (check_data->GetUrlPath(&query_path)) { + builder.AddString(utils::AttributeName::kRequestUrlPath, query_path); + } + + std::map query_map; + if (check_data->GetRequestQueryParams(&query_map) && query_map.size() > 0) { + builder.AddStringMap(utils::AttributeName::kRequestQueryParams, query_map); + } } void AttributesBuilder::ExtractAuthAttributes(CheckData *check_data) { diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 9c87a5de992..d0f6bfa00ec 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -85,7 +85,7 @@ attributes { } entries { key: "path" - value: "/books" + value: "/books?a=b&c=d" } } } @@ -99,7 +99,22 @@ attributes { attributes { key: "request.path" value { - string_value: "/books" + string_value: "/books?a=b&c=d" + } +} +attributes { + key: "request.query_params" + value { + string_map_value { + entries { + key: "a" + value: "b" + } + entries { + key: "c" + value: "d" + } + } } } attributes { @@ -115,6 +130,12 @@ attributes { } } } +attributes { + key: "request.url_path" + value { + string_value: "/books" + } +} )"; const char kCheckAttributes[] = R"( @@ -134,7 +155,7 @@ attributes { } entries { key: "path" - value: "/books" + value: "/books?a=b&c=d" } } } @@ -147,10 +168,31 @@ attributes { } attributes { key: "request.path" + value { + string_value: "/books?a=b&c=d" + } +} +attributes { + key: "request.url_path" value { string_value: "/books" } } +attributes { + key: "request.query_params" + value { + string_map_value { + entries { + key: "a" + value: "b" + } + entries { + key: "c" + value: "d" + } + } + } +} attributes { key: "request.scheme" value { @@ -531,7 +573,7 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithoutAuthnFilter) { EXPECT_CALL(mock_data, GetRequestHeaders()) .WillOnce(Invoke([]() -> std::map { std::map map; - map["path"] = "/books"; + map["path"] = "/books?a=b&c=d"; map["host"] = "localhost"; return map; })); @@ -539,7 +581,7 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithoutAuthnFilter) { .WillRepeatedly(Invoke( [](CheckData::HeaderType header_type, std::string *value) -> bool { if (header_type == CheckData::HEADER_PATH) { - *value = "/books"; + *value = "/books?a=b&c=d"; return true; } else if (header_type == CheckData::HEADER_HOST) { *value = "localhost"; @@ -550,6 +592,18 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithoutAuthnFilter) { EXPECT_CALL(mock_data, GetAuthenticationResult()) .WillOnce(testing::Return(nullptr)); + EXPECT_CALL(mock_data, GetUrlPath(_)) + .WillOnce(Invoke([](std::string *path) -> bool { + *path = "/books"; + return true; + })); + EXPECT_CALL(mock_data, GetRequestQueryParams(_)) + .WillOnce(Invoke([](std::map *map) -> bool { + (*map)["a"] = "b"; + (*map)["c"] = "d"; + return true; + })); + RequestContext request; AttributesBuilder builder(&request); builder.ExtractCheckAttributes(&mock_data); @@ -590,7 +644,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, GetRequestHeaders()) .WillOnce(Invoke([]() -> std::map { std::map map; - map["path"] = "/books"; + map["path"] = "/books?a=b&c=d"; map["host"] = "localhost"; return map; })); @@ -598,7 +652,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { .WillRepeatedly(Invoke( [](CheckData::HeaderType header_type, std::string *value) -> bool { if (header_type == CheckData::HEADER_PATH) { - *value = "/books"; + *value = "/books?a=b&c=d"; return true; } else if (header_type == CheckData::HEADER_HOST) { *value = "localhost"; @@ -612,6 +666,17 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, GetAuthenticationResult()) .WillOnce(testing::Return(&authn_result)); + EXPECT_CALL(mock_data, GetUrlPath(_)) + .WillOnce(Invoke([](std::string *path) -> bool { + *path = "/books"; + return true; + })); + EXPECT_CALL(mock_data, GetRequestQueryParams(_)) + .WillOnce(Invoke([](std::map *map) -> bool { + (*map)["a"] = "b"; + (*map)["c"] = "d"; + return true; + })); RequestContext request; AttributesBuilder builder(&request); diff --git a/src/istio/control/http/mock_check_data.h b/src/istio/control/http/mock_check_data.h index f85e1487c3b..106ac78f28e 100644 --- a/src/istio/control/http/mock_check_data.h +++ b/src/istio/control/http/mock_check_data.h @@ -46,6 +46,9 @@ class MockCheckData : public CheckData { const ::google::protobuf::Struct *()); MOCK_CONST_METHOD0(IsMutualTLS, bool()); MOCK_CONST_METHOD1(GetRequestedServerName, bool(std::string *name)); + MOCK_CONST_METHOD1(GetUrlPath, bool(std::string *)); + MOCK_CONST_METHOD1(GetRequestQueryParams, + bool(std::map *)); }; // The mock object for HeaderUpdate interface. diff --git a/src/istio/utils/attribute_names.cc b/src/istio/utils/attribute_names.cc index 3a9677fd147..d42e0666b38 100644 --- a/src/istio/utils/attribute_names.cc +++ b/src/istio/utils/attribute_names.cc @@ -29,6 +29,8 @@ const char AttributeName::kRequestMethod[] = "request.method"; const char AttributeName::kRequestPath[] = "request.path"; const char AttributeName::kRequestReferer[] = "request.referer"; const char AttributeName::kRequestScheme[] = "request.scheme"; +const char AttributeName::kRequestUrlPath[] = "request.url_path"; +const char AttributeName::kRequestQueryParams[] = "request.query_params"; const char AttributeName::kRequestBodySize[] = "request.size"; const char AttributeName::kRequestTotalSize[] = "request.total_size"; const char AttributeName::kRequestTime[] = "request.time";