Add flag enable_api_key_uid_reporting and report unknown when the che…

…ck response status is not OK. (#865)

* Add flag enable_api_key_uid_reporting and report unknown when the check
response status is not OK.

Verified in local unit testing.

* address comments

* address comments

WORKSPACE

@@ -84,7 +84,7 @@ load("@io_bazel_rules_python//python:pip.bzl", "pip_repositories", "pip_import")
 
 pip_import(
     name = "grpc_python_dependencies",
-    requirements = "@com_github_grpc_grpc//:requirements.bazel.txt",
+    requirements = "//:requirements.bazel.txt",
 )
 
 load("@com_github_grpc_grpc//bazel:grpc_deps.bzl", "grpc_deps", "grpc_test_only_deps")

requirements.bazel.txt

@@ -0,0 +1,18 @@
+# GRPC Python setup requirements
+# TODO(kangda): dynamically update the build dependency instead of
+# using a static file.
+coverage>=4.0
+cython==0.28.3
+enum34>=1.0.4
+protobuf==3.5.0.post1
+six>=1.10
+wheel>=0.29
+futures>=2.2.0
+google-auth>=1.0.0
+oauth2client==4.1.0
+requests>=2.14.2
+urllib3>=1.23
+chardet==3.0.4
+certifi==2017.4.17
+idna==2.7
+googleapis-common-protos==1.5.5

src/api_manager/proto/server_config.proto

@@ -128,6 +128,10 @@ message ServiceControlConfig {
   // In case of failing to connect to service control service, the requests
   // are allowed if this field is true.  Default is false.
   bool network_fail_open = 16;
+
+  // If set to true, reports api_key_uid instead of api_key in ServiceControl
+  // report.
+  bool enable_api_key_uid_reporting = 17;
 }
 
 // Check aggregator config

src/api_manager/service_control/aggregated.cc

@@ -375,10 +375,16 @@ void Aggregated::Check(
     TRACE(trace_span) << "Check returned with status: " << status.ToString();
     CheckResponseInfo response_info;
     if (status.ok()) {
+      bool enableApiKeyUid = server_config_
+        && server_config_->has_service_control_config()
+        && server_config_->service_control_config()
+                            .enable_api_key_uid_reporting();
       Status status = Proto::ConvertCheckResponse(
-          *response, service_control_proto_.service_name(), &response_info);
+          *response, service_control_proto_.service_name(), &response_info,
+           enableApiKeyUid);
       on_done(status, response_info);
     } else {
+      response_info.is_network_failure = true;
       // All 5xx Http status codes are treated as network failure.
       // If network_fail_open is true, it is OK to proceed with these errors.
       if (network_fail_open_ && StatusCodeIs5xxHttpCode(status.error_code())) {

src/api_manager/service_control/info.h

@@ -87,9 +87,14 @@ struct CheckResponseInfo {
   // The api key uid of the request.
   std::string api_key_uid;
 
+  // The check has failed because of network failure.
+  bool is_network_failure;
+
   // By default api_key is valid and service is activated.
   // They only set to false by the check response from server.
-  CheckResponseInfo() : is_api_key_valid(true), service_is_activated(true) {}
+  CheckResponseInfo() : is_api_key_valid(true),
+                        service_is_activated(true), 
+                        is_network_failure(false) {}
 };
 
 struct QuotaRequestInfo : public OperationInfo {

src/api_manager/service_control/proto.cc

@@ -14,6 +14,8 @@
 //
 ////////////////////////////////////////////////////////////////////////////////
 //
+#include "absl/strings/str_cat.h"
+
 #include "src/api_manager/service_control/proto.h"
 
 #include <functional>
@@ -544,11 +546,20 @@ const char kServiceAgentPrefix[] = "ESP/";
 Status set_credential_id(const SupportedLabel& l, const ReportRequestInfo& info,
                          Map<std::string, std::string>* labels) {
   // The rule to set /credential_id is:
-  // 1) If api_key is available, set it as apiKey:API-KEY
+  // 1) If api_key is available.
+  //    1] if CheckRequest has RPC error, set it as "apikey:UNKNOWN".
+  //    2] if CheckRequest status is OK(RPC successful):
+  //          if api_key_uid present, set it as api_key_uid.
+  //                            else, set it as "apikey:<api_key>".
   // 2) If auth issuer and audience both are available, set it as:
   //    jwtAuth:issuer=base64(issuer)&audience=base64(audience)
   if (!info.api_key.empty()) {
-    (*labels)[l.name] = absl::StrCat(kApiKeyPrefix, info.check_response_info.api_key_uid.empty() ? info.api_key : info.check_response_info.api_key_uid);
+    if (info.check_response_info.is_network_failure) {
+      (*labels)[l.name] = absl::StrCat(kApiKeyPrefix, "UNKNOWN");
+    } else  {
+      (*labels)[l.name] = info.check_response_info.api_key_uid.empty() ? absl::StrCat(kApiKeyPrefix, info.api_key)
+                      : info.check_response_info.api_key_uid;
+    }
   } else if (!info.auth_issuer.empty()) {
     // If auth is used, auth_issuer should NOT be empty since it is required.
     char* base64_issuer = auth::esp_base64_encode(
@@ -1386,14 +1397,17 @@ Status Proto::ConvertAllocateQuotaResponse(
 
 Status Proto::ConvertCheckResponse(const CheckResponse& check_response,
                                    const std::string& service_name,
-                                   CheckResponseInfo* check_response_info) {
+                                   CheckResponseInfo* check_response_info,
+                                   bool enable_api_key_uid) {
   if (check_response.check_info().consumer_info().project_number() > 0) {
     // Store project id to check_response_info
     check_response_info->consumer_project_id = std::to_string(
         check_response.check_info().consumer_info().project_number());
   }
 
-  check_response_info->api_key_uid = check_response.check_info().api_key_uid();
+  if (enable_api_key_uid) {
+    check_response_info->api_key_uid = check_response.check_info().api_key_uid();
+  }
 
   if (check_response.check_errors().size() == 0) {
     return Status::OK;

src/api_manager/service_control/proto.h

@@ -75,7 +75,8 @@ class Proto final {
   // failures.
   static utils::Status ConvertCheckResponse(
       const ::google::api::servicecontrol::v1::CheckResponse& response,
-      const std::string& service_name, CheckResponseInfo* check_response_info);
+      const std::string& service_name, CheckResponseInfo* check_response_info,
+      bool enable_api_key_uid = false);
 
   static utils::Status ConvertAllocateQuotaResponse(
       const ::google::api::servicecontrol::v1::AllocateQuotaResponse& response,

src/api_manager/service_control/proto_test.cc

@@ -374,7 +374,7 @@ TEST_F(ProtoTest, FillReportRequestEmptyOptionalTest) {
   ASSERT_EQ(expected_text, text);
 }
 
-TEST_F(ProtoTest, CredentailIdApiKeyTest) {
+TEST_F(ProtoTest, CredentialIdApiKeyTest) {
   ReportRequestInfo info;
   FillOperationInfo(&info);
 
@@ -385,7 +385,32 @@ TEST_F(ProtoTest, CredentailIdApiKeyTest) {
             "apikey:api_key_x");
 }
 
-TEST_F(ProtoTest, CredentailIdIssuerOnlyTest) {
+TEST_F(ProtoTest, CredentialIdApiKeyUidTest) {
+  ReportRequestInfo info;
+  FillOperationInfo(&info);
+  info.check_response_info.api_key_uid = "apikey:fake_uid";
+
+  gasv1::ReportRequest request;
+  ASSERT_TRUE(scp_.FillReportRequest(info, &request).ok());
+
+  ASSERT_EQ(request.operations(0).labels().at("/credential_id"),
+            "apikey:fake_uid");
+}
+
+TEST_F(ProtoTest, CredentialIdApiKeyUidUnknownTest) {
+  ReportRequestInfo info;
+  FillOperationInfo(&info);
+  info.check_response_info.is_network_failure = true;
+  info.check_response_info.api_key_uid = "apikey:fake_uid";
+
+  gasv1::ReportRequest request;
+  ASSERT_TRUE(scp_.FillReportRequest(info, &request).ok());
+
+  ASSERT_EQ(request.operations(0).labels().at("/credential_id"),
+            "apikey:UNKNOWN");
+}
+
+TEST_F(ProtoTest, CredentialIdIssuerOnlyTest) {
   ReportRequestInfo info;
   FillOperationInfo(&info);
   info.api_key = "";
@@ -398,7 +423,7 @@ TEST_F(ProtoTest, CredentailIdIssuerOnlyTest) {
             "jwtauth:issuer=YXV0aC1pc3N1ZXI");
 }
 
-TEST_F(ProtoTest, CredentailIdIssuerAudienceTest) {
+TEST_F(ProtoTest, CredentialIdIssuerAudienceTest) {
   ReportRequestInfo info;
   FillOperationInfo(&info);
   info.api_key = "";

start_esp/server-auto.conf.template

@@ -65,6 +65,9 @@ service_control_config {
   %if service_control_network_fail_open:
   network_fail_open: true
   % endif
+  % if enable_api_key_uid_reporting:
+  enable_api_key_uid_reporting: true
+% endif
 }
 % endif
 % if jwks_cache_duration_in_s or redirect_authorization_url:

start_esp/start_esp.py

@@ -209,7 +209,8 @@ def write_server_config_template(server_config_path, args):
                 grpc_backend_ssl_credentials=args.grpc_backend_ssl_credentials,
                 jwks_cache_duration_in_s=args.jwks_cache_duration_in_s,
                 redirect_authorization_url=args.enable_jwt_authorization_url_redirect,
-                rollout_fetch_throttle_window_in_s=args.rollout_fetch_throttle_window_in_s)
+                rollout_fetch_throttle_window_in_s=args.rollout_fetch_throttle_window_in_s,
+                enable_api_key_uid_reporting=args.enable_api_key_uid_reporting)
 
         server_config_file = server_config_path
         if server_config_file.endswith('/'):
@@ -951,6 +952,10 @@ def make_argparser():
         to call at the same time to exceed the quota, the calling time is throttled within
         a window. This flag specifies the throttle window in seconds. Default is 5 minutes.
         If number of ESP instances for a service is big, please increase this number.''')
+
+    parser.add_argument('--enable_api_key_uid_reporting', action='store_true',
+                        help='''If set to true, reports api_key_uid instead of api_key in
+                        ServiceControl report.''')
 
     return parser
 

start_esp/test/start_esp_test.py

@@ -371,5 +371,10 @@ def test_enable_backend_routing_conflicts_with_single_dash_flag(self):
             return_code = os.system(config_generator)
             self.assertEqual(return_code >> 8, 3)
 
+    def test_enable_api_key_uid_reporting(self):
+        expected_config_file = "./start_esp/test/testdata/expected_enable_api_key_uid_reporting.json"
+        config_generator = self.basic_config_generator + " --enable_api_key_uid_reporting"
+        self.run_test_with_expectation(expected_config_file, self.generated_server_config_file, config_generator)
+
 if __name__ == '__main__':
     unittest.main()

start_esp/test/testdata/expected_enable_api_key_uid_reporting.json

@@ -0,0 +1,42 @@
+# Auto-generated by start_esp
+# Copyright (C) Extensible Service Proxy Authors
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+#    notice, this list of conditions and the following disclaimer in the
+#    documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+service_config_rollout {
+    traffic_percentages {
+	key: "./start_esp/test/testdata/test_service_config_1.json"
+	value: 100
+    }
+}
+service_management_config {
+    url: "https://servicemanagement.googleapis.com"
+}
+service_control_config {
+    network_fail_open: true
+    enable_api_key_uid_reporting: true
+}
+experimental {
+    disable_log_status: true
+}
+rollout_strategy: "None"
+