Skip to content

Commit 0cb851c

Browse files
committed
Add flag enable_api_key_uid_reporting and report unknown when the check
response status is not OK. Verified in local unit testing.
1 parent 8094420 commit 0cb851c

File tree

12 files changed

+135
-9
lines changed

12 files changed

+135
-9
lines changed

WORKSPACE

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ load("@io_bazel_rules_python//python:pip.bzl", "pip_repositories", "pip_import")
8484

8585
pip_import(
8686
name = "grpc_python_dependencies",
87-
requirements = "@com_github_grpc_grpc//:requirements.bazel.txt",
87+
requirements = "//:requirements.bazel.txt",
8888
)
8989

9090
load("@com_github_grpc_grpc//bazel:grpc_deps.bzl", "grpc_deps", "grpc_test_only_deps")

requirements.bazel.txt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# GRPC Python setup requirements
2+
coverage>=4.0
3+
cython==0.28.3
4+
enum34>=1.0.4
5+
protobuf==3.5.0.post1
6+
six>=1.10
7+
wheel>=0.29
8+
futures>=2.2.0
9+
google-auth>=1.0.0
10+
oauth2client==4.1.0
11+
requests>=2.14.2
12+
urllib3>=1.23
13+
chardet==3.0.4
14+
certifi==2017.4.17
15+
idna==2.7
16+
googleapis-common-protos==1.5.5

src/api_manager/proto/server_config.proto

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ message ServiceControlConfig {
128128
// In case of failing to connect to service control service, the requests
129129
// are allowed if this field is true. Default is false.
130130
bool network_fail_open = 16;
131+
132+
// If set to true, reports api_key_uid instead of api_key in ServiceControl
133+
// report.
134+
bool enable_api_key_uid_reporting = 17;
131135
}
132136

133137
// Check aggregator config

src/api_manager/service_control/aggregated.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,16 @@ void Aggregated::Check(
375375
TRACE(trace_span) << "Check returned with status: " << status.ToString();
376376
CheckResponseInfo response_info;
377377
if (status.ok()) {
378+
bool enableApiKeyUid = server_config_
379+
&& server_config_->has_service_control_config()
380+
&& server_config_->service_control_config()
381+
.enable_api_key_uid_reporting();
378382
Status status = Proto::ConvertCheckResponse(
379-
*response, service_control_proto_.service_name(), &response_info);
383+
*response, service_control_proto_.service_name(), &response_info,
384+
enableApiKeyUid);
380385
on_done(status, response_info);
381386
} else {
387+
response_info.is_network_failure = true;
382388
// All 5xx Http status codes are treated as network failure.
383389
// If network_fail_open is true, it is OK to proceed with these errors.
384390
if (network_fail_open_ && StatusCodeIs5xxHttpCode(status.error_code())) {

src/api_manager/service_control/info.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,14 @@ struct CheckResponseInfo {
8787
// The api key uid of the request.
8888
std::string api_key_uid;
8989

90+
// The check has failed because of network failure.
91+
bool is_network_failure;
92+
9093
// By default api_key is valid and service is activated.
9194
// They only set to false by the check response from server.
92-
CheckResponseInfo() : is_api_key_valid(true), service_is_activated(true) {}
95+
CheckResponseInfo() : is_api_key_valid(true),
96+
service_is_activated(true),
97+
is_network_failure(false) {}
9398
};
9499

95100
struct QuotaRequestInfo : public OperationInfo {

src/api_manager/service_control/proto.cc

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
//
1515
////////////////////////////////////////////////////////////////////////////////
1616
//
17+
#include "absl/strings/str_cat.h"
18+
1719
#include "src/api_manager/service_control/proto.h"
1820

1921
#include <functional>
@@ -544,11 +546,20 @@ const char kServiceAgentPrefix[] = "ESP/";
544546
Status set_credential_id(const SupportedLabel& l, const ReportRequestInfo& info,
545547
Map<std::string, std::string>* labels) {
546548
// The rule to set /credential_id is:
547-
// 1) If api_key is available, set it as apiKey:API-KEY
549+
// 1) If api_key is available.
550+
// 1] if CheckRequest has RPC error, set it as "apikey:UNKNOWN".
551+
// 2] if CheckRequest status is OK(RPC successful):
552+
// if api_key_uid present, set it as api_key_uid.
553+
// else, set it as "apikey:<api_key>".
548554
// 2) If auth issuer and audience both are available, set it as:
549555
// jwtAuth:issuer=base64(issuer)&audience=base64(audience)
550556
if (!info.api_key.empty()) {
551-
(*labels)[l.name] = absl::StrCat(kApiKeyPrefix, info.check_response_info.api_key_uid.empty() ? info.api_key : info.check_response_info.api_key_uid);
557+
if (info.check_response_info.is_network_failure) {
558+
(*labels)[l.name] = absl::StrCat(kApiKeyPrefix, "UNKNOWN");
559+
} else {
560+
(*labels)[l.name] = info.check_response_info.api_key_uid.empty() ? absl::StrCat(kApiKeyPrefix, info.api_key)
561+
: info.check_response_info.api_key_uid;
562+
}
552563
} else if (!info.auth_issuer.empty()) {
553564
// If auth is used, auth_issuer should NOT be empty since it is required.
554565
char* base64_issuer = auth::esp_base64_encode(
@@ -1386,14 +1397,17 @@ Status Proto::ConvertAllocateQuotaResponse(
13861397

13871398
Status Proto::ConvertCheckResponse(const CheckResponse& check_response,
13881399
const std::string& service_name,
1389-
CheckResponseInfo* check_response_info) {
1400+
CheckResponseInfo* check_response_info,
1401+
bool enable_api_key_uid) {
13901402
if (check_response.check_info().consumer_info().project_number() > 0) {
13911403
// Store project id to check_response_info
13921404
check_response_info->consumer_project_id = std::to_string(
13931405
check_response.check_info().consumer_info().project_number());
13941406
}
13951407

1396-
check_response_info->api_key_uid = check_response.check_info().api_key_uid();
1408+
if (enable_api_key_uid) {
1409+
check_response_info->api_key_uid = check_response.check_info().api_key_uid();
1410+
}
13971411

13981412
if (check_response.check_errors().size() == 0) {
13991413
return Status::OK;

src/api_manager/service_control/proto.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ class Proto final {
7575
// failures.
7676
static utils::Status ConvertCheckResponse(
7777
const ::google::api::servicecontrol::v1::CheckResponse& response,
78-
const std::string& service_name, CheckResponseInfo* check_response_info);
78+
const std::string& service_name, CheckResponseInfo* check_response_info,
79+
bool enable_api_key_uid = false);
7980

8081
static utils::Status ConvertAllocateQuotaResponse(
8182
const ::google::api::servicecontrol::v1::AllocateQuotaResponse& response,

src/api_manager/service_control/proto_test.cc

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,31 @@ TEST_F(ProtoTest, CredentailIdApiKeyTest) {
385385
"apikey:api_key_x");
386386
}
387387

388+
TEST_F(ProtoTest, CredentailIdApiKeyUidTest) {
389+
ReportRequestInfo info;
390+
FillOperationInfo(&info);
391+
info.check_response_info.api_key_uid = "apikey:fake_uid";
392+
393+
gasv1::ReportRequest request;
394+
ASSERT_TRUE(scp_.FillReportRequest(info, &request).ok());
395+
396+
ASSERT_EQ(request.operations(0).labels().at("/credential_id"),
397+
"apikey:fake_uid");
398+
}
399+
400+
TEST_F(ProtoTest, CredentailIdApiKeyUidUnknownTest) {
401+
ReportRequestInfo info;
402+
FillOperationInfo(&info);
403+
info.check_response_info.is_network_failure = true;
404+
info.check_response_info.api_key_uid = "apikey:fake_uid";
405+
406+
gasv1::ReportRequest request;
407+
ASSERT_TRUE(scp_.FillReportRequest(info, &request).ok());
408+
409+
ASSERT_EQ(request.operations(0).labels().at("/credential_id"),
410+
"apikey:UNKNOWN");
411+
}
412+
388413
TEST_F(ProtoTest, CredentailIdIssuerOnlyTest) {
389414
ReportRequestInfo info;
390415
FillOperationInfo(&info);

start_esp/server-auto.conf.template

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ service_control_config {
6565
%if service_control_network_fail_open:
6666
network_fail_open: true
6767
% endif
68+
% if enable_api_key_uid_reporting:
69+
enable_api_key_uid_reporting: true
70+
% endif
6871
}
6972
% endif
7073
% if jwks_cache_duration_in_s or redirect_authorization_url:

start_esp/start_esp.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,8 @@ def write_server_config_template(server_config_path, args):
209209
grpc_backend_ssl_credentials=args.grpc_backend_ssl_credentials,
210210
jwks_cache_duration_in_s=args.jwks_cache_duration_in_s,
211211
redirect_authorization_url=args.enable_jwt_authorization_url_redirect,
212-
rollout_fetch_throttle_window_in_s=args.rollout_fetch_throttle_window_in_s)
212+
rollout_fetch_throttle_window_in_s=args.rollout_fetch_throttle_window_in_s,
213+
enable_api_key_uid_reporting=args.enable_api_key_uid_reporting)
213214

214215
server_config_file = server_config_path
215216
if server_config_file.endswith('/'):
@@ -951,6 +952,10 @@ def make_argparser():
951952
to call at the same time to exceed the quota, the calling time is throttled within
952953
a window. This flag specifies the throttle window in seconds. Default is 5 minutes.
953954
If number of ESP instances for a service is big, please increase this number.''')
955+
956+
parser.add_argument('--enable_api_key_uid_reporting', action='store_true',
957+
help='''If set to true, reports api_key_uid instead of api_key in
958+
ServiceControl report.''')
954959

955960
return parser
956961

start_esp/test/start_esp_test.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,5 +371,10 @@ def test_enable_backend_routing_conflicts_with_single_dash_flag(self):
371371
return_code = os.system(config_generator)
372372
self.assertEqual(return_code >> 8, 3)
373373

374+
def test_enable_api_key_uid_reporting(self):
375+
expected_config_file = "./start_esp/test/testdata/expected_enable_api_key_uid_reporting.json"
376+
config_generator = self.basic_config_generator + " --enable_api_key_uid_reporting"
377+
self.run_test_with_expectation(expected_config_file, self.generated_server_config_file, config_generator)
378+
374379
if __name__ == '__main__':
375380
unittest.main()
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Auto-generated by start_esp
2+
# Copyright (C) Extensible Service Proxy Authors
3+
# All rights reserved.
4+
#
5+
# Redistribution and use in source and binary forms, with or without
6+
# modification, are permitted provided that the following conditions
7+
# are met:
8+
# 1. Redistributions of source code must retain the above copyright
9+
# notice, this list of conditions and the following disclaimer.
10+
# 2. Redistributions in binary form must reproduce the above copyright
11+
# notice, this list of conditions and the following disclaimer in the
12+
# documentation and/or other materials provided with the distribution.
13+
#
14+
# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
15+
# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
16+
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
17+
# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
18+
# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
19+
# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
20+
# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
21+
# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
22+
# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
23+
# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
24+
# SUCH DAMAGE.
25+
service_config_rollout {
26+
traffic_percentages {
27+
key: "./start_esp/test/testdata/test_service_config_1.json"
28+
value: 100
29+
}
30+
}
31+
service_management_config {
32+
url: "https://servicemanagement.googleapis.com"
33+
}
34+
service_control_config {
35+
network_fail_open: true
36+
enable_api_key_uid_reporting: true
37+
}
38+
experimental {
39+
disable_log_status: true
40+
}
41+
rollout_strategy: "None"
42+

0 commit comments

Comments
 (0)