Add two new attributes: request.url_path and request.queries

istio · Jul 3, 2018 · b47be78 · b47be78
1 parent f592fc1
commit b47be78
Show file tree

Hide file tree

Showing 8 changed files with 86 additions and 6 deletions.
diff --git a/include/istio/control/http/check_data.h b/include/istio/control/http/check_data.h
@@ -86,6 +86,12 @@ class CheckData {
   // If the request has authentication result in header, parses data into the
   // output result; returns true if success. Otherwise, returns false.
   virtual bool GetAuthenticationResult(istio::authn::Result *result) const = 0;
+
+  // Get request url path, which strips query part from the http path header.
+  virtual std::string GetUrlPath() const = 0;
+
+  // Get request queries with string map format.
+  virtual std::map<std::string, std::string> GetRequestQueries() 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
@@ -34,6 +34,8 @@ struct AttributeName {
   static const char kRequestPath[];
   static const char kRequestReferer[];
   static const char kRequestScheme[];
+  static const char kRequestUrlPath[];
+  static const char kRequestQueries[];
   static const char kRequestBodySize[];
   // Total size of request received, including request headers, body, and
   // trailers.

diff --git a/src/envoy/http/mixer/check_data.cc b/src/envoy/http/mixer/check_data.cc
@@ -200,6 +200,24 @@ bool CheckData::GetAuthenticationResult(istio::authn::Result* result) const {
   return Utils::Authentication::FetchResultFromHeader(headers_, result);
 }
 
+std::string CheckData::GetUrlPath() const {
+  if (headers_.Path()) {
+    const HeaderString& path = headers_.Path()->value();
+    const char* query_start = Utility::findQueryStringStart(path);
+    if (query_start != nullptr) {
+      return std::string(path.c_str(), query_start - path.c_str());
+    }
+  }
+  return "";
+}
+
+std::map<std::string, std::string> CheckData::GetRequestQueries() const {
+  if (headers_.Path()) {
+    return Utility::parseQueryString(headers_.Path()->value().c_str());
+  }
+  return std::map<std::string, std::string>();
+}
+
 }  // namespace Mixer
 }  // namespace Http
 }  // namespace Envoy
diff --git a/src/envoy/http/mixer/check_data.h b/src/envoy/http/mixer/check_data.h
@@ -59,6 +59,10 @@ class CheckData : public ::istio::control::http::CheckData,
 
   bool GetAuthenticationResult(istio::authn::Result* result) const override;
 
+  std::string GetUrlPath() const override;
+
+  std::map<std::string, std::string> GetRequestQueries() const override;
+
  private:
   const HeaderMap& headers_;
   const Network::Connection* connection_;

diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc
@@ -169,6 +169,11 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData *check_data) {
     }
   }
   builder.AddString(utils::AttributeName::kContextProtocol, protocol);
+
+  builder.AddStringMap(utils::AttributeName::kRequestQueries,
+                       check_data->GetRequestQueries());
+  builder.AddString(utils::AttributeName::kRequestUrlPath,
+                    check_data->GetUrlPath());
 }
 
 void AttributesBuilder::ForwardAttributes(const Attributes &forward_attributes,

diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc
@@ -28,8 +28,8 @@ using ::google::protobuf::util::MessageDifferencer;
 using ::istio::mixer::v1::Attributes;
 using ::istio::mixer::v1::Attributes_StringMap;
 
-using ::testing::Invoke;
 using ::testing::_;
+using ::testing::Invoke;
 
 namespace istio {
 namespace control {
@@ -53,7 +53,7 @@ attributes {
       }
       entries {
         key: "path"
-        value: "/books"
+        value: "/books?a=b&c=d"
       }
     }
   }
@@ -66,10 +66,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.queries"
+  value {
+    string_map_value {
+      entries {
+        key: "a"
+        value: "b"
+      }
+      entries {
+        key: "c"
+        value: "d"
+      }
+    }
+  }
+}
 attributes {
   key: "request.scheme"
   value {
@@ -289,15 +310,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";
@@ -318,6 +339,16 @@ TEST(AttributesBuilderTest, TestCheckAttributes) {
         (*payload)["exp"] = "5112754205";
         return true;
       }));
+  EXPECT_CALL(mock_data, GetUrlPath()).WillOnce(Invoke([]() -> std::string {
+    return "/books";
+  }));
+  EXPECT_CALL(mock_data, GetRequestQueries())
+      .WillOnce(Invoke([]() -> std::map<std::string, std::string> {
+        std::map<std::string, std::string> map;
+        map["a"] = "b";
+        map["c"] = "d";
+        return map;
+      }));
 
   RequestContext request;
   AttributesBuilder builder(&request);
@@ -344,22 +375,32 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) {
   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";
               return true;
             }
             return false;
           }));
+  EXPECT_CALL(mock_data, GetUrlPath()).WillOnce(Invoke([]() -> std::string {
+    return "/books";
+  }));
+  EXPECT_CALL(mock_data, GetRequestQueries())
+      .WillOnce(Invoke([]() -> std::map<std::string, std::string> {
+        std::map<std::string, std::string> map;
+        map["a"] = "b";
+        map["c"] = "d";
+        return map;
+      }));
   EXPECT_CALL(mock_data, GetAuthenticationResult(_))
       .WillOnce(Invoke([](istio::authn::Result *result) -> bool {
         result->set_principal("thisisiss/thisissub");

diff --git a/src/istio/control/http/mock_check_data.h b/src/istio/control/http/mock_check_data.h
@@ -44,6 +44,8 @@ class MockCheckData : public CheckData {
   MOCK_CONST_METHOD1(GetAuthenticationResult,
                      bool(istio::authn::Result *result));
   MOCK_CONST_METHOD0(IsMutualTLS, bool());
+  MOCK_CONST_METHOD0(GetUrlPath, std::string());
+  MOCK_CONST_METHOD0(GetRequestQueries, 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
@@ -28,6 +28,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::kRequestQueries[] = "request.queries";
 const char AttributeName::kRequestBodySize[] = "request.size";
 const char AttributeName::kRequestTotalSize[] = "request.total_size";
 const char AttributeName::kRequestTime[] = "request.time";