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
istio · Aug 10, 2018 · 5d09f41 · 5d09f41
1 parent adea4d0
commit 5d09f41
Show file tree

Hide file tree

Showing 10 changed files with 130 additions and 10 deletions.
diff --git 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<std::string, std::string> *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
@@ -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
@@ -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"
 	}
-]
+]
diff --git 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
@@ -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<std::string, std::string>* 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
@@ -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<std::string, std::string>* 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
@@ -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<std::string, std::string> 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
@@ -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,15 +573,15 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithoutAuthnFilter) {
   EXPECT_CALL(mock_data, GetRequestHeaders())
       .WillOnce(Invoke([]() -> std::map<std::string, std::string> {
         std::map<std::string, std::string> map;
-        map["path"] = "/books";
+        map["path"] = "/books?a=b&c=d";
         map["host"] = "localhost";
         return map;
       }));
   EXPECT_CALL(mock_data, FindHeaderByType(_, _))
       .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<std::string, std::string> *map) -> bool {
+        (*map)["a"] = "b";
+        (*map)["c"] = "d";
+        return true;
+      }));
+
   RequestContext request;
   AttributesBuilder builder(&request);
   builder.ExtractCheckAttributes(&mock_data);
@@ -590,15 +644,15 @@ TEST(AttributesBuilderTest, TestCheckAttributes) {
   EXPECT_CALL(mock_data, GetRequestHeaders())
       .WillOnce(Invoke([]() -> std::map<std::string, std::string> {
         std::map<std::string, std::string> map;
-        map["path"] = "/books";
+        map["path"] = "/books?a=b&c=d";
         map["host"] = "localhost";
         return map;
       }));
   EXPECT_CALL(mock_data, FindHeaderByType(_, _))
       .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<std::string, std::string> *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
@@ -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<std::string, std::string> *));
 };
 
 // The mock object for HeaderUpdate interface.

diff --git 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";