From ba9c83c70e9bfc290f8f7402460044fb4f339a1d Mon Sep 17 00:00:00 2001 From: Xuyang Tao Date: Thu, 19 Sep 2019 16:28:29 -0700 Subject: [PATCH 1/8] metadata: add fetch failure status timeout The metadata fetch won't fetch after the first failure and set a failure time window, after which the proxy can fecth again. --- src/api_manager/auth/service_account_token.h | 13 ++ src/api_manager/fetch_metadata.cc | 15 +- src/nginx/t/metadata_fecth_fail.t | 219 +++++++++++++++++++ 3 files changed, 243 insertions(+), 4 deletions(-) create mode 100644 src/nginx/t/metadata_fecth_fail.t diff --git a/src/api_manager/auth/service_account_token.h b/src/api_manager/auth/service_account_token.h index 0102de292..7265e6b02 100644 --- a/src/api_manager/auth/service_account_token.h +++ b/src/api_manager/auth/service_account_token.h @@ -54,6 +54,16 @@ class ServiceAccountToken { access_token_.set_token(token, expiration); } + // Set the last failed fetch time + void set_last_failed_fetch_time(std::chrono::system_clock::time_point time) { + last_failed_fetch_time_ = time; + } + + // Get the last failed fetch time + std::chrono::system_clock::time_point last_failed_fetch_time() const { + return last_failed_fetch_time_; + } + // The access token from metadata server response in JSON format: // { // "access_token":" ... ", @@ -154,6 +164,9 @@ class ServiceAccountToken { // Fetching state FetchState state_; + + // The time of last failed fetch + std::chrono::system_clock::time_point last_failed_fetch_time_; }; } // namespace auth diff --git a/src/api_manager/fetch_metadata.cc b/src/api_manager/fetch_metadata.cc index df972e52e..448b19b0a 100644 --- a/src/api_manager/fetch_metadata.cc +++ b/src/api_manager/fetch_metadata.cc @@ -19,6 +19,7 @@ #include "include/api_manager/http_request.h" #include "src/api_manager/auth/lib/auth_token.h" +using std::chrono::system_clock; using ::google::api_manager::utils::Status; using ::google::protobuf::util::error::Code; @@ -38,7 +39,8 @@ const char kMetadataInstanceIdentityToken[] = // The maximum lifetime of a cache token. Unit: seconds. // Token expired in 1 hour, reduce 100 seconds for grace buffer. const int kInstanceIdentityTokenExpiration = 3500; - +// Time window (in seconds) of failure status after a failed fetch. +const int kFailureStatusWindow = 5; // Initial metadata fetch timeout (1s) const int kMetadataFetchTimeout = 1000; // Maximum number of retries to fetch token from metadata @@ -116,9 +118,13 @@ void GlobalFetchServiceAccountToken( } break; case auth::ServiceAccountToken::FAILED: - // permanent failure - continuation(Status(Code::INTERNAL, kFailedTokenFetch)); - return; + // If the current time doesn't get the time window of failure status, + // it will return kFailedTokenFetch directly. + if (system_clock::now() - token->last_failed_fetch_time() < std::chrono::seconds(kFailureStatusWindow)) { + continuation(Status(Code::INTERNAL, kFailedTokenFetch)); + return; + } + break; case auth::ServiceAccountToken::NONE: default: env->LogDebug("Need to fetch service account token"); @@ -132,6 +138,7 @@ void GlobalFetchServiceAccountToken( // fetch failed if (!status.ok()) { env->LogDebug("Failed to fetch service account token"); + token->set_last_failed_fetch_time(system_clock::now()); token->set_state(auth::ServiceAccountToken::FAILED); continuation(Status(Code::INTERNAL, kFailedTokenFetch)); return; diff --git a/src/nginx/t/metadata_fecth_fail.t b/src/nginx/t/metadata_fecth_fail.t new file mode 100644 index 000000000..742e375bb --- /dev/null +++ b/src/nginx/t/metadata_fecth_fail.t @@ -0,0 +1,219 @@ +# 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. +# +################################################################################ +# +use strict; +use warnings; + +################################################################################ + +use src::nginx::t::ApiManager; # Must be first (sets up import path to the Nginx test module) +use src::nginx::t::HttpServer; +use src::nginx::t::ServiceControl; +use Test::Nginx; # Imports Nginx's test module +use Test::More; # And the test framework + +################################################################################ + +# Port assignments +my $NginxPort = ApiManager::pick_port(); +my $BackendPort = ApiManager::pick_port(); +my $ServiceControlPort = ApiManager::pick_port(); +my $MetadataPort = ApiManager::pick_port(); + +my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(16); + +my $config = ApiManager::get_bookstore_service_config . <<"EOF"; +control { + environment: "http://127.0.0.1:${ServiceControlPort}" +} +EOF + +$t->write_file('service.pb.txt', $config); +ApiManager::write_file_expand($t, 'nginx.conf', <<"EOF"); +%%TEST_GLOBALS%% +daemon off; +events { worker_connections 32; } +http { + %%TEST_GLOBALS_HTTP%% + server_tokens off; + endpoints { + metadata_server http://127.0.0.1:${MetadataPort}; + } + server { + listen 127.0.0.1:${NginxPort}; + server_name localhost; + location / { + endpoints { + api service.pb.txt; + %%TEST_CONFIG%% + on; + } + proxy_pass http://127.0.0.1:${BackendPort}; + } + } +} +EOF + +sub no_check_call { + my (@requests) = @_; + foreach my $r (@requests) { + if ($r->{path} =~ qr/:check$/) { + return 0; + } + } + return 1; +} + +sub test_metadata { + my ($sleep, $wantReqHeader, $wantReqBody) = @_; + my $report_done = 'report_done'; + my $backend_log = 'backend.log'; + my $servicecontrol_log = 'servicecontrol.log'; + + $t->run_daemon(\&backends, $t, $BackendPort, $backend_log); + $t->run_daemon(\&servicecontrol, $t, $ServiceControlPort, $servicecontrol_log, $report_done); + $t->run_daemon(\&metadata, $t, $MetadataPort, 'metadata.log'); + + is($t->waitforsocket("127.0.0.1:${BackendPort}"), 1, 'bookstore socket ready.'); + is($t->waitforsocket("127.0.0.1:${ServiceControlPort}"), 1, 'Service control socket ready.'); + is($t->waitforsocket("127.0.0.1:${MetadataPort}"), 1, 'Metadata socket ready.'); + + $t->run(); + + ################################################################################ + + my $shelves1 = ApiManager::http_get($NginxPort, '/shelves'); + is($t->waitforfile("$t->{_testdir}/${report_done}"), 1, 'Report succeeded'); + + my ($shelves_headers1, $shelves_body1) = split /\r\n\r\n/, $shelves1, 2; + like($shelves_headers1, qr/HTTP\/1\.1 500 Internal Server Error/, '/shelves returned HTTP 500.'); + like($shelves_body1, qr/Failed to fetch service account token/, 'Proxy failed in fetch service account token'); + + ################################################################################ + # if no sleep, the service account token still doesn't get out of last failed + # fetch so it will fail. + if ($sleep) { + sleep 5; + } + my $shelves2 = ApiManager::http_get($NginxPort, '/shelves'); + is($t->waitforfile("$t->{_testdir}/${report_done}"), 1, 'Report succeeded'); + + $t->stop(); + $t->stop_daemons(); + + my ($shelves_headers2, $shelves_body2) = split /\r\n\r\n/, $shelves2, 2; + like($shelves_headers2, $wantReqHeader, '/shelves returned HTTP 500.'); + like($shelves_body2, $wantReqBody, 'Proxy failed in fetch service account token'); +} +# Fail the first request by failed fetch and do the second request right away, which +# also get failed since the failed fetch status doesn't expire. +test_metadata(0, qr/HTTP\/1\.1 500 Internal Server Error/, qr/Failed to fetch service account token/); +# Fail the first request by failed fetch and do the second request after sleeping +# 5s , which will get the token and. +test_metadata(1, qr/HTTP\/1.1 401 Unauthorized/, qr/Method doesn't allow unregistered callers/); + +################################################################################ + +sub checkfile { + if (-e $_[0]) {return 1;} + else {return 0;} +} + +sub backends { + my ($t, $port, $file) = @_; + my $server = HttpServer->new($port, $t->testdir() . '/' . $file) + or die "Can't create test server socket: $!\n"; + local $SIG{PIPE} = 'IGNORE'; + $server->run(); +} + +sub metadata { + my ($t, $port, $file) = @_; + my $server = HttpServer->new($port, $t->testdir() . '/' . $file) + or die "Can't create test server socket: $!\n"; + my $request_count = 0; + + local $SIG{PIPE} = 'IGNORE'; + $server->on_sub('GET', '/computeMetadata/v1/instance/service-accounts/default/token', sub { + my ($headers, $body, $client) = @_; + $request_count++; + # The retry time is 5 and it would be 6 times for the first failed fetch. + if ($request_count < 7) { + return; + } + # Only the 7th request will get the token. + print $client <<'EOF'; +HTTP/1.1 200 OK +Metadata-Flavor: Google +Content-Type: application/json + +{ + "access_token":"ya29.7gFRTEGmovWacYDnQIpC9X9Qp8cH0sgQyWVrZaB1Eg1WoAhQMSG4L2rtaHk1", + "expires_in":200, + "token_type":"Bearer" +} +EOF + + }); + + $server->run(); +} + +################################################################################ + +sub servicecontrol { + my ($t, $port, $file, $done) = @_; + + # Save requests (last argument). + my $server = HttpServer->new($ServiceControlPort, $t->testdir() . '/' . $file) + or die "Can't create test server socket: $!\n"; + local $SIG{PIPE} = 'IGNORE'; + + $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:check', sub { + my ($headers, $body, $client) = @_; + print $client <<'EOF'; +HTTP/1.1 200 OK +Content-Type: application/json +Connection: close + +EOF + }); + + $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:report', sub { + my ($headers, $body, $client) = @_; + print $client <<'EOF'; +HTTP/1.1 200 OK +Content-Type: application/json +Connection: close + +EOF + $t->write_file($done, ':report done'); + }); + + $server->run(); +} + +################################################################################ From 7bf82e7b01188e37c78dfb16176205698e4777d1 Mon Sep 17 00:00:00 2001 From: Xuyang Tao Date: Thu, 19 Sep 2019 16:28:29 -0700 Subject: [PATCH 2/8] metadata: add fetch failure status timeout The metadata fetch won't fetch after the first failure and set a failure time window, after which the proxy can fecth again. --- src/api_manager/auth/service_account_token.h | 13 ++ src/api_manager/fetch_metadata.cc | 15 +- src/nginx/t/metadata_fecth_fail.t | 219 +++++++++++++++++++ 3 files changed, 243 insertions(+), 4 deletions(-) create mode 100644 src/nginx/t/metadata_fecth_fail.t diff --git a/src/api_manager/auth/service_account_token.h b/src/api_manager/auth/service_account_token.h index 0102de292..7265e6b02 100644 --- a/src/api_manager/auth/service_account_token.h +++ b/src/api_manager/auth/service_account_token.h @@ -54,6 +54,16 @@ class ServiceAccountToken { access_token_.set_token(token, expiration); } + // Set the last failed fetch time + void set_last_failed_fetch_time(std::chrono::system_clock::time_point time) { + last_failed_fetch_time_ = time; + } + + // Get the last failed fetch time + std::chrono::system_clock::time_point last_failed_fetch_time() const { + return last_failed_fetch_time_; + } + // The access token from metadata server response in JSON format: // { // "access_token":" ... ", @@ -154,6 +164,9 @@ class ServiceAccountToken { // Fetching state FetchState state_; + + // The time of last failed fetch + std::chrono::system_clock::time_point last_failed_fetch_time_; }; } // namespace auth diff --git a/src/api_manager/fetch_metadata.cc b/src/api_manager/fetch_metadata.cc index 55877b9f3..66903a936 100644 --- a/src/api_manager/fetch_metadata.cc +++ b/src/api_manager/fetch_metadata.cc @@ -19,6 +19,7 @@ #include "include/api_manager/http_request.h" #include "src/api_manager/auth/lib/auth_token.h" +using std::chrono::system_clock; using ::google::api_manager::utils::Status; using ::google::protobuf::util::error::Code; @@ -39,7 +40,8 @@ const char kMetadataInstanceIdentityToken[] = // The maximum lifetime of a cache token. Unit: seconds. // Token expired in 1 hour, reduce 100 seconds for grace buffer. const int kInstanceIdentityTokenExpiration = 3500; - +// Time window (in seconds) of failure status after a failed fetch. +const int kFailureStatusWindow = 5; // Initial metadata fetch timeout (1s) const int kMetadataFetchTimeout = 1000; // Maximum number of retries to fetch token from metadata @@ -117,9 +119,13 @@ void GlobalFetchServiceAccountToken( } break; case auth::ServiceAccountToken::FAILED: - // permanent failure - continuation(Status(Code::INTERNAL, kFailedTokenFetch)); - return; + // If the current time doesn't get the time window of failure status, + // it will return kFailedTokenFetch directly. + if (system_clock::now() - token->last_failed_fetch_time() < std::chrono::seconds(kFailureStatusWindow)) { + continuation(Status(Code::INTERNAL, kFailedTokenFetch)); + return; + } + break; case auth::ServiceAccountToken::NONE: default: env->LogDebug("Need to fetch service account token"); @@ -133,6 +139,7 @@ void GlobalFetchServiceAccountToken( // fetch failed if (!status.ok()) { env->LogDebug("Failed to fetch service account token"); + token->set_last_failed_fetch_time(system_clock::now()); token->set_state(auth::ServiceAccountToken::FAILED); continuation(Status(Code::INTERNAL, kFailedTokenFetch)); return; diff --git a/src/nginx/t/metadata_fecth_fail.t b/src/nginx/t/metadata_fecth_fail.t new file mode 100644 index 000000000..742e375bb --- /dev/null +++ b/src/nginx/t/metadata_fecth_fail.t @@ -0,0 +1,219 @@ +# 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. +# +################################################################################ +# +use strict; +use warnings; + +################################################################################ + +use src::nginx::t::ApiManager; # Must be first (sets up import path to the Nginx test module) +use src::nginx::t::HttpServer; +use src::nginx::t::ServiceControl; +use Test::Nginx; # Imports Nginx's test module +use Test::More; # And the test framework + +################################################################################ + +# Port assignments +my $NginxPort = ApiManager::pick_port(); +my $BackendPort = ApiManager::pick_port(); +my $ServiceControlPort = ApiManager::pick_port(); +my $MetadataPort = ApiManager::pick_port(); + +my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(16); + +my $config = ApiManager::get_bookstore_service_config . <<"EOF"; +control { + environment: "http://127.0.0.1:${ServiceControlPort}" +} +EOF + +$t->write_file('service.pb.txt', $config); +ApiManager::write_file_expand($t, 'nginx.conf', <<"EOF"); +%%TEST_GLOBALS%% +daemon off; +events { worker_connections 32; } +http { + %%TEST_GLOBALS_HTTP%% + server_tokens off; + endpoints { + metadata_server http://127.0.0.1:${MetadataPort}; + } + server { + listen 127.0.0.1:${NginxPort}; + server_name localhost; + location / { + endpoints { + api service.pb.txt; + %%TEST_CONFIG%% + on; + } + proxy_pass http://127.0.0.1:${BackendPort}; + } + } +} +EOF + +sub no_check_call { + my (@requests) = @_; + foreach my $r (@requests) { + if ($r->{path} =~ qr/:check$/) { + return 0; + } + } + return 1; +} + +sub test_metadata { + my ($sleep, $wantReqHeader, $wantReqBody) = @_; + my $report_done = 'report_done'; + my $backend_log = 'backend.log'; + my $servicecontrol_log = 'servicecontrol.log'; + + $t->run_daemon(\&backends, $t, $BackendPort, $backend_log); + $t->run_daemon(\&servicecontrol, $t, $ServiceControlPort, $servicecontrol_log, $report_done); + $t->run_daemon(\&metadata, $t, $MetadataPort, 'metadata.log'); + + is($t->waitforsocket("127.0.0.1:${BackendPort}"), 1, 'bookstore socket ready.'); + is($t->waitforsocket("127.0.0.1:${ServiceControlPort}"), 1, 'Service control socket ready.'); + is($t->waitforsocket("127.0.0.1:${MetadataPort}"), 1, 'Metadata socket ready.'); + + $t->run(); + + ################################################################################ + + my $shelves1 = ApiManager::http_get($NginxPort, '/shelves'); + is($t->waitforfile("$t->{_testdir}/${report_done}"), 1, 'Report succeeded'); + + my ($shelves_headers1, $shelves_body1) = split /\r\n\r\n/, $shelves1, 2; + like($shelves_headers1, qr/HTTP\/1\.1 500 Internal Server Error/, '/shelves returned HTTP 500.'); + like($shelves_body1, qr/Failed to fetch service account token/, 'Proxy failed in fetch service account token'); + + ################################################################################ + # if no sleep, the service account token still doesn't get out of last failed + # fetch so it will fail. + if ($sleep) { + sleep 5; + } + my $shelves2 = ApiManager::http_get($NginxPort, '/shelves'); + is($t->waitforfile("$t->{_testdir}/${report_done}"), 1, 'Report succeeded'); + + $t->stop(); + $t->stop_daemons(); + + my ($shelves_headers2, $shelves_body2) = split /\r\n\r\n/, $shelves2, 2; + like($shelves_headers2, $wantReqHeader, '/shelves returned HTTP 500.'); + like($shelves_body2, $wantReqBody, 'Proxy failed in fetch service account token'); +} +# Fail the first request by failed fetch and do the second request right away, which +# also get failed since the failed fetch status doesn't expire. +test_metadata(0, qr/HTTP\/1\.1 500 Internal Server Error/, qr/Failed to fetch service account token/); +# Fail the first request by failed fetch and do the second request after sleeping +# 5s , which will get the token and. +test_metadata(1, qr/HTTP\/1.1 401 Unauthorized/, qr/Method doesn't allow unregistered callers/); + +################################################################################ + +sub checkfile { + if (-e $_[0]) {return 1;} + else {return 0;} +} + +sub backends { + my ($t, $port, $file) = @_; + my $server = HttpServer->new($port, $t->testdir() . '/' . $file) + or die "Can't create test server socket: $!\n"; + local $SIG{PIPE} = 'IGNORE'; + $server->run(); +} + +sub metadata { + my ($t, $port, $file) = @_; + my $server = HttpServer->new($port, $t->testdir() . '/' . $file) + or die "Can't create test server socket: $!\n"; + my $request_count = 0; + + local $SIG{PIPE} = 'IGNORE'; + $server->on_sub('GET', '/computeMetadata/v1/instance/service-accounts/default/token', sub { + my ($headers, $body, $client) = @_; + $request_count++; + # The retry time is 5 and it would be 6 times for the first failed fetch. + if ($request_count < 7) { + return; + } + # Only the 7th request will get the token. + print $client <<'EOF'; +HTTP/1.1 200 OK +Metadata-Flavor: Google +Content-Type: application/json + +{ + "access_token":"ya29.7gFRTEGmovWacYDnQIpC9X9Qp8cH0sgQyWVrZaB1Eg1WoAhQMSG4L2rtaHk1", + "expires_in":200, + "token_type":"Bearer" +} +EOF + + }); + + $server->run(); +} + +################################################################################ + +sub servicecontrol { + my ($t, $port, $file, $done) = @_; + + # Save requests (last argument). + my $server = HttpServer->new($ServiceControlPort, $t->testdir() . '/' . $file) + or die "Can't create test server socket: $!\n"; + local $SIG{PIPE} = 'IGNORE'; + + $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:check', sub { + my ($headers, $body, $client) = @_; + print $client <<'EOF'; +HTTP/1.1 200 OK +Content-Type: application/json +Connection: close + +EOF + }); + + $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:report', sub { + my ($headers, $body, $client) = @_; + print $client <<'EOF'; +HTTP/1.1 200 OK +Content-Type: application/json +Connection: close + +EOF + $t->write_file($done, ':report done'); + }); + + $server->run(); +} + +################################################################################ From b76eae609a74af231c7751126926767f530d7129 Mon Sep 17 00:00:00 2001 From: Xuyang Tao Date: Thu, 19 Sep 2019 20:00:47 -0700 Subject: [PATCH 3/8] fix format; optimize t test --- src/api_manager/fetch_metadata.cc | 5 +- src/nginx/t/BUILD | 1 + ...ata_fecth_fail.t => metadata_fetch_fail.t} | 80 ++----------------- 3 files changed, 12 insertions(+), 74 deletions(-) rename src/nginx/t/{metadata_fecth_fail.t => metadata_fetch_fail.t} (69%) diff --git a/src/api_manager/fetch_metadata.cc b/src/api_manager/fetch_metadata.cc index 66903a936..21411806d 100644 --- a/src/api_manager/fetch_metadata.cc +++ b/src/api_manager/fetch_metadata.cc @@ -19,9 +19,9 @@ #include "include/api_manager/http_request.h" #include "src/api_manager/auth/lib/auth_token.h" -using std::chrono::system_clock; using ::google::api_manager::utils::Status; using ::google::protobuf::util::error::Code; +using std::chrono::system_clock; namespace google { namespace api_manager { @@ -121,7 +121,8 @@ void GlobalFetchServiceAccountToken( case auth::ServiceAccountToken::FAILED: // If the current time doesn't get the time window of failure status, // it will return kFailedTokenFetch directly. - if (system_clock::now() - token->last_failed_fetch_time() < std::chrono::seconds(kFailureStatusWindow)) { + if (system_clock::now() - token->last_failed_fetch_time() < + std::chrono::seconds(kFailureStatusWindow)) { continuation(Status(Code::INTERNAL, kFailedTokenFetch)); return; } diff --git a/src/nginx/t/BUILD b/src/nginx/t/BUILD index 02a531540..edde3c063 100644 --- a/src/nginx/t/BUILD +++ b/src/nginx/t/BUILD @@ -220,6 +220,7 @@ nginx_suite( "init_service_configs_single.t", "metadata.t", "metadata_fail.t", + "metadata_fetch_fail.t", "metadata_timeout.t", "metadata_token.t", "multiple_apis.t", diff --git a/src/nginx/t/metadata_fecth_fail.t b/src/nginx/t/metadata_fetch_fail.t similarity index 69% rename from src/nginx/t/metadata_fecth_fail.t rename to src/nginx/t/metadata_fetch_fail.t index 742e375bb..a47dce911 100644 --- a/src/nginx/t/metadata_fecth_fail.t +++ b/src/nginx/t/metadata_fetch_fail.t @@ -43,7 +43,7 @@ my $BackendPort = ApiManager::pick_port(); my $ServiceControlPort = ApiManager::pick_port(); my $MetadataPort = ApiManager::pick_port(); -my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(16); +my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(8); my $config = ApiManager::get_bookstore_service_config . <<"EOF"; control { @@ -77,28 +77,14 @@ http { } EOF -sub no_check_call { - my (@requests) = @_; - foreach my $r (@requests) { - if ($r->{path} =~ qr/:check$/) { - return 0; - } - } - return 1; -} sub test_metadata { - my ($sleep, $wantReqHeader, $wantReqBody) = @_; - my $report_done = 'report_done'; + my ($sleepLength, $wantReqHeader, $wantReqBody) = @_; my $backend_log = 'backend.log'; - my $servicecontrol_log = 'servicecontrol.log'; $t->run_daemon(\&backends, $t, $BackendPort, $backend_log); - $t->run_daemon(\&servicecontrol, $t, $ServiceControlPort, $servicecontrol_log, $report_done); $t->run_daemon(\&metadata, $t, $MetadataPort, 'metadata.log'); - is($t->waitforsocket("127.0.0.1:${BackendPort}"), 1, 'bookstore socket ready.'); - is($t->waitforsocket("127.0.0.1:${ServiceControlPort}"), 1, 'Service control socket ready.'); is($t->waitforsocket("127.0.0.1:${MetadataPort}"), 1, 'Metadata socket ready.'); $t->run(); @@ -106,50 +92,34 @@ sub test_metadata { ################################################################################ my $shelves1 = ApiManager::http_get($NginxPort, '/shelves'); - is($t->waitforfile("$t->{_testdir}/${report_done}"), 1, 'Report succeeded'); my ($shelves_headers1, $shelves_body1) = split /\r\n\r\n/, $shelves1, 2; like($shelves_headers1, qr/HTTP\/1\.1 500 Internal Server Error/, '/shelves returned HTTP 500.'); - like($shelves_body1, qr/Failed to fetch service account token/, 'Proxy failed in fetch service account token'); + like($shelves_body1, qr/Failed to fetch service account token/, 'Returned Failure Status'); ################################################################################ # if no sleep, the service account token still doesn't get out of last failed # fetch so it will fail. - if ($sleep) { - sleep 5; - } + sleep $sleepLength; + my $shelves2 = ApiManager::http_get($NginxPort, '/shelves'); - is($t->waitforfile("$t->{_testdir}/${report_done}"), 1, 'Report succeeded'); $t->stop(); $t->stop_daemons(); my ($shelves_headers2, $shelves_body2) = split /\r\n\r\n/, $shelves2, 2; like($shelves_headers2, $wantReqHeader, '/shelves returned HTTP 500.'); - like($shelves_body2, $wantReqBody, 'Proxy failed in fetch service account token'); + like($shelves_body2, $wantReqBody, 'Returned Failure Status'); } # Fail the first request by failed fetch and do the second request right away, which # also get failed since the failed fetch status doesn't expire. -test_metadata(0, qr/HTTP\/1\.1 500 Internal Server Error/, qr/Failed to fetch service account token/); +test_metadata(2, qr/HTTP\/1\.1 500 Internal Server Error/, qr/Failed to fetch service account token/); # Fail the first request by failed fetch and do the second request after sleeping # 5s , which will get the token and. -test_metadata(1, qr/HTTP\/1.1 401 Unauthorized/, qr/Method doesn't allow unregistered callers/); +test_metadata(7, qr/HTTP\/1.1 401 Unauthorized/, qr/Method doesn't allow unregistered callers/); ################################################################################ -sub checkfile { - if (-e $_[0]) {return 1;} - else {return 0;} -} - -sub backends { - my ($t, $port, $file) = @_; - my $server = HttpServer->new($port, $t->testdir() . '/' . $file) - or die "Can't create test server socket: $!\n"; - local $SIG{PIPE} = 'IGNORE'; - $server->run(); -} - sub metadata { my ($t, $port, $file) = @_; my $server = HttpServer->new($port, $t->testdir() . '/' . $file) @@ -183,37 +153,3 @@ EOF } ################################################################################ - -sub servicecontrol { - my ($t, $port, $file, $done) = @_; - - # Save requests (last argument). - my $server = HttpServer->new($ServiceControlPort, $t->testdir() . '/' . $file) - or die "Can't create test server socket: $!\n"; - local $SIG{PIPE} = 'IGNORE'; - - $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:check', sub { - my ($headers, $body, $client) = @_; - print $client <<'EOF'; -HTTP/1.1 200 OK -Content-Type: application/json -Connection: close - -EOF - }); - - $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:report', sub { - my ($headers, $body, $client) = @_; - print $client <<'EOF'; -HTTP/1.1 200 OK -Content-Type: application/json -Connection: close - -EOF - $t->write_file($done, ':report done'); - }); - - $server->run(); -} - -################################################################################ From 35807b9a16cd994350b2d15cb9e51ff23c47499c Mon Sep 17 00:00:00 2001 From: Xuyang Tao Date: Thu, 19 Sep 2019 20:24:06 -0700 Subject: [PATCH 4/8] rebase --- src/api_manager/fetch_metadata.cc | 4 ++-- src/nginx/t/metadata_fetch_fail.t | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/api_manager/fetch_metadata.cc b/src/api_manager/fetch_metadata.cc index 21411806d..af0feb9d3 100644 --- a/src/api_manager/fetch_metadata.cc +++ b/src/api_manager/fetch_metadata.cc @@ -119,8 +119,8 @@ void GlobalFetchServiceAccountToken( } break; case auth::ServiceAccountToken::FAILED: - // If the current time doesn't get the time window of failure status, - // it will return kFailedTokenFetch directly. + // If the current time doesn't get out of the time window of failure + // status, it will return kFailedTokenFetch directly. if (system_clock::now() - token->last_failed_fetch_time() < std::chrono::seconds(kFailureStatusWindow)) { continuation(Status(Code::INTERNAL, kFailedTokenFetch)); diff --git a/src/nginx/t/metadata_fetch_fail.t b/src/nginx/t/metadata_fetch_fail.t index a47dce911..4c9aa1197 100644 --- a/src/nginx/t/metadata_fetch_fail.t +++ b/src/nginx/t/metadata_fetch_fail.t @@ -139,7 +139,6 @@ sub metadata { HTTP/1.1 200 OK Metadata-Flavor: Google Content-Type: application/json - { "access_token":"ya29.7gFRTEGmovWacYDnQIpC9X9Qp8cH0sgQyWVrZaB1Eg1WoAhQMSG4L2rtaHk1", "expires_in":200, @@ -152,4 +151,4 @@ EOF $server->run(); } -################################################################################ +################################################################################ \ No newline at end of file From ac7b4310452820d25093e1c6023c859b4394823e Mon Sep 17 00:00:00 2001 From: Xuyang Tao Date: Thu, 19 Sep 2019 20:25:52 -0700 Subject: [PATCH 5/8] patch --- src/nginx/t/metadata_fecth_fail.t | 219 ------------------------------ 1 file changed, 219 deletions(-) delete mode 100644 src/nginx/t/metadata_fecth_fail.t diff --git a/src/nginx/t/metadata_fecth_fail.t b/src/nginx/t/metadata_fecth_fail.t deleted file mode 100644 index 742e375bb..000000000 --- a/src/nginx/t/metadata_fecth_fail.t +++ /dev/null @@ -1,219 +0,0 @@ -# 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. -# -################################################################################ -# -use strict; -use warnings; - -################################################################################ - -use src::nginx::t::ApiManager; # Must be first (sets up import path to the Nginx test module) -use src::nginx::t::HttpServer; -use src::nginx::t::ServiceControl; -use Test::Nginx; # Imports Nginx's test module -use Test::More; # And the test framework - -################################################################################ - -# Port assignments -my $NginxPort = ApiManager::pick_port(); -my $BackendPort = ApiManager::pick_port(); -my $ServiceControlPort = ApiManager::pick_port(); -my $MetadataPort = ApiManager::pick_port(); - -my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(16); - -my $config = ApiManager::get_bookstore_service_config . <<"EOF"; -control { - environment: "http://127.0.0.1:${ServiceControlPort}" -} -EOF - -$t->write_file('service.pb.txt', $config); -ApiManager::write_file_expand($t, 'nginx.conf', <<"EOF"); -%%TEST_GLOBALS%% -daemon off; -events { worker_connections 32; } -http { - %%TEST_GLOBALS_HTTP%% - server_tokens off; - endpoints { - metadata_server http://127.0.0.1:${MetadataPort}; - } - server { - listen 127.0.0.1:${NginxPort}; - server_name localhost; - location / { - endpoints { - api service.pb.txt; - %%TEST_CONFIG%% - on; - } - proxy_pass http://127.0.0.1:${BackendPort}; - } - } -} -EOF - -sub no_check_call { - my (@requests) = @_; - foreach my $r (@requests) { - if ($r->{path} =~ qr/:check$/) { - return 0; - } - } - return 1; -} - -sub test_metadata { - my ($sleep, $wantReqHeader, $wantReqBody) = @_; - my $report_done = 'report_done'; - my $backend_log = 'backend.log'; - my $servicecontrol_log = 'servicecontrol.log'; - - $t->run_daemon(\&backends, $t, $BackendPort, $backend_log); - $t->run_daemon(\&servicecontrol, $t, $ServiceControlPort, $servicecontrol_log, $report_done); - $t->run_daemon(\&metadata, $t, $MetadataPort, 'metadata.log'); - - is($t->waitforsocket("127.0.0.1:${BackendPort}"), 1, 'bookstore socket ready.'); - is($t->waitforsocket("127.0.0.1:${ServiceControlPort}"), 1, 'Service control socket ready.'); - is($t->waitforsocket("127.0.0.1:${MetadataPort}"), 1, 'Metadata socket ready.'); - - $t->run(); - - ################################################################################ - - my $shelves1 = ApiManager::http_get($NginxPort, '/shelves'); - is($t->waitforfile("$t->{_testdir}/${report_done}"), 1, 'Report succeeded'); - - my ($shelves_headers1, $shelves_body1) = split /\r\n\r\n/, $shelves1, 2; - like($shelves_headers1, qr/HTTP\/1\.1 500 Internal Server Error/, '/shelves returned HTTP 500.'); - like($shelves_body1, qr/Failed to fetch service account token/, 'Proxy failed in fetch service account token'); - - ################################################################################ - # if no sleep, the service account token still doesn't get out of last failed - # fetch so it will fail. - if ($sleep) { - sleep 5; - } - my $shelves2 = ApiManager::http_get($NginxPort, '/shelves'); - is($t->waitforfile("$t->{_testdir}/${report_done}"), 1, 'Report succeeded'); - - $t->stop(); - $t->stop_daemons(); - - my ($shelves_headers2, $shelves_body2) = split /\r\n\r\n/, $shelves2, 2; - like($shelves_headers2, $wantReqHeader, '/shelves returned HTTP 500.'); - like($shelves_body2, $wantReqBody, 'Proxy failed in fetch service account token'); -} -# Fail the first request by failed fetch and do the second request right away, which -# also get failed since the failed fetch status doesn't expire. -test_metadata(0, qr/HTTP\/1\.1 500 Internal Server Error/, qr/Failed to fetch service account token/); -# Fail the first request by failed fetch and do the second request after sleeping -# 5s , which will get the token and. -test_metadata(1, qr/HTTP\/1.1 401 Unauthorized/, qr/Method doesn't allow unregistered callers/); - -################################################################################ - -sub checkfile { - if (-e $_[0]) {return 1;} - else {return 0;} -} - -sub backends { - my ($t, $port, $file) = @_; - my $server = HttpServer->new($port, $t->testdir() . '/' . $file) - or die "Can't create test server socket: $!\n"; - local $SIG{PIPE} = 'IGNORE'; - $server->run(); -} - -sub metadata { - my ($t, $port, $file) = @_; - my $server = HttpServer->new($port, $t->testdir() . '/' . $file) - or die "Can't create test server socket: $!\n"; - my $request_count = 0; - - local $SIG{PIPE} = 'IGNORE'; - $server->on_sub('GET', '/computeMetadata/v1/instance/service-accounts/default/token', sub { - my ($headers, $body, $client) = @_; - $request_count++; - # The retry time is 5 and it would be 6 times for the first failed fetch. - if ($request_count < 7) { - return; - } - # Only the 7th request will get the token. - print $client <<'EOF'; -HTTP/1.1 200 OK -Metadata-Flavor: Google -Content-Type: application/json - -{ - "access_token":"ya29.7gFRTEGmovWacYDnQIpC9X9Qp8cH0sgQyWVrZaB1Eg1WoAhQMSG4L2rtaHk1", - "expires_in":200, - "token_type":"Bearer" -} -EOF - - }); - - $server->run(); -} - -################################################################################ - -sub servicecontrol { - my ($t, $port, $file, $done) = @_; - - # Save requests (last argument). - my $server = HttpServer->new($ServiceControlPort, $t->testdir() . '/' . $file) - or die "Can't create test server socket: $!\n"; - local $SIG{PIPE} = 'IGNORE'; - - $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:check', sub { - my ($headers, $body, $client) = @_; - print $client <<'EOF'; -HTTP/1.1 200 OK -Content-Type: application/json -Connection: close - -EOF - }); - - $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:report', sub { - my ($headers, $body, $client) = @_; - print $client <<'EOF'; -HTTP/1.1 200 OK -Content-Type: application/json -Connection: close - -EOF - $t->write_file($done, ':report done'); - }); - - $server->run(); -} - -################################################################################ From 22fb488e018ce1fa0f5dc7a04825d9bf5719373b Mon Sep 17 00:00:00 2001 From: Xuyang Tao Date: Thu, 19 Sep 2019 20:32:02 -0700 Subject: [PATCH 6/8] patch --- src/nginx/t/metadata_fetch_fail.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nginx/t/metadata_fetch_fail.t b/src/nginx/t/metadata_fetch_fail.t index 4c9aa1197..69bd1f52a 100644 --- a/src/nginx/t/metadata_fetch_fail.t +++ b/src/nginx/t/metadata_fetch_fail.t @@ -115,7 +115,7 @@ sub test_metadata { # also get failed since the failed fetch status doesn't expire. test_metadata(2, qr/HTTP\/1\.1 500 Internal Server Error/, qr/Failed to fetch service account token/); # Fail the first request by failed fetch and do the second request after sleeping -# 5s , which will get the token and. +# 5s , which will get the token. test_metadata(7, qr/HTTP\/1.1 401 Unauthorized/, qr/Method doesn't allow unregistered callers/); ################################################################################ From e978dd87937fca376f6f316fc72f283cccb20679 Mon Sep 17 00:00:00 2001 From: Xuyang Tao Date: Thu, 19 Sep 2019 20:33:41 -0700 Subject: [PATCH 7/8] patch --- src/nginx/t/metadata_fetch_fail.t | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nginx/t/metadata_fetch_fail.t b/src/nginx/t/metadata_fetch_fail.t index 69bd1f52a..b5eb19762 100644 --- a/src/nginx/t/metadata_fetch_fail.t +++ b/src/nginx/t/metadata_fetch_fail.t @@ -111,12 +111,12 @@ sub test_metadata { like($shelves_headers2, $wantReqHeader, '/shelves returned HTTP 500.'); like($shelves_body2, $wantReqBody, 'Returned Failure Status'); } -# Fail the first request by failed fetch and do the second request right away, which +# Fail the first request by failed fetch and do the second request after 2s, which # also get failed since the failed fetch status doesn't expire. test_metadata(2, qr/HTTP\/1\.1 500 Internal Server Error/, qr/Failed to fetch service account token/); # Fail the first request by failed fetch and do the second request after sleeping -# 5s , which will get the token. -test_metadata(7, qr/HTTP\/1.1 401 Unauthorized/, qr/Method doesn't allow unregistered callers/); +# 6s , which will get the token. +test_metadata(6, qr/HTTP\/1.1 401 Unauthorized/, qr/Method doesn't allow unregistered callers/); ################################################################################ From f5a2598e753f8e89baf23508683dd6e99a37731a Mon Sep 17 00:00:00 2001 From: Xuyang Tao Date: Fri, 20 Sep 2019 09:12:37 -0700 Subject: [PATCH 8/8] patch --- src/nginx/t/metadata_fetch_fail.t | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/nginx/t/metadata_fetch_fail.t b/src/nginx/t/metadata_fetch_fail.t index b5eb19762..715781b46 100644 --- a/src/nginx/t/metadata_fetch_fail.t +++ b/src/nginx/t/metadata_fetch_fail.t @@ -84,7 +84,6 @@ sub test_metadata { $t->run_daemon(\&backends, $t, $BackendPort, $backend_log); $t->run_daemon(\&metadata, $t, $MetadataPort, 'metadata.log'); - is($t->waitforsocket("127.0.0.1:${MetadataPort}"), 1, 'Metadata socket ready.'); $t->run(); @@ -108,8 +107,8 @@ sub test_metadata { $t->stop_daemons(); my ($shelves_headers2, $shelves_body2) = split /\r\n\r\n/, $shelves2, 2; - like($shelves_headers2, $wantReqHeader, '/shelves returned HTTP 500.'); - like($shelves_body2, $wantReqBody, 'Returned Failure Status'); + like($shelves_headers2, $wantReqHeader, 'Got expected request header'); + like($shelves_body2, $wantReqBody, 'Got expected request body'); } # Fail the first request by failed fetch and do the second request after 2s, which # also get failed since the failed fetch status doesn't expire. @@ -130,22 +129,24 @@ sub metadata { $server->on_sub('GET', '/computeMetadata/v1/instance/service-accounts/default/token', sub { my ($headers, $body, $client) = @_; $request_count++; + # The retry time is 5 and it would be 6 times for the first failed fetch. if ($request_count < 7) { return; } + # Only the 7th request will get the token. print $client <<'EOF'; HTTP/1.1 200 OK Metadata-Flavor: Google Content-Type: application/json + { "access_token":"ya29.7gFRTEGmovWacYDnQIpC9X9Qp8cH0sgQyWVrZaB1Eg1WoAhQMSG4L2rtaHk1", "expires_in":200, "token_type":"Bearer" } EOF - }); $server->run();