diff --git a/CHANGELOG.md b/CHANGELOG.md index 14197ade719..efde10c4699 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ Unreleased Changes ------------------ +* Issue - Aws::InstanceProfileCredentials - Expanded retry logic in the + `Aws::InstanceProfileCredentials` class. Previously this class could + raise JSON parse error if the instance metadata service responded but + with an invalid JSON document. These errors will be retried 3 times now + with backoff. + + See [related GitHub issue #913](https://github.com/aws/aws-sdk-ruby/issues/913). + * Feature - Aws::EC2 - Implemented `Aws::EC2::VpcPeeringConnection#exists?`. See [related GitHub issue #916](https://github.com/aws/aws-sdk-ruby/issues/916). diff --git a/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb b/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb index 90c187220c9..2d20c6ed0cc 100644 --- a/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb +++ b/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb @@ -1,3 +1,4 @@ +require 'json' require 'time' require 'net/http' @@ -15,7 +16,7 @@ class Non200Response < RuntimeError; end # is not present, no responding or some other non-recoverable # error. # @api private - FAILURES = [ + NETWORK_ERRORS = [ Errno::EHOSTUNREACH, Errno::ECONNREFUSED, Errno::EHOSTDOWN, @@ -65,31 +66,33 @@ def backoff(backoff) end def refresh - c = Json.load(get_credentials) - @credentials = Credentials.new( - c['AccessKeyId'], - c['SecretAccessKey'], - c['Token'] - ) - @expiration = c['Expiration'] ? Time.parse(c['Expiration']) : nil + # Retry loading credentials up to 3 times is the instance metadata + # service is responding but is returning invalid JSON documents + # in response to the GET profile credentials call. + retry_errors([JSON::ParserError], max_retries: 3) do + c = JSON.parse(get_credentials.to_s) + @credentials = Credentials.new( + c['AccessKeyId'], + c['SecretAccessKey'], + c['Token'] + ) + @expiration = c['Expiration'] ? Time.parse(c['Expiration']) : nil + end end def get_credentials - failed_attempts = 0 + # Retry loading credentials a configurable number of times if + # the instance metadata service is not responding. begin - open_connection do |conn| - path = '/latest/meta-data/iam/security-credentials/' - profile_name = http_get(conn, path).lines.first.strip - http_get(conn, path + profile_name) - end - rescue *FAILURES - if failed_attempts < @retries - @backoff.call(failed_attempts) - failed_attempts += 1 - retry - else - '{}' + retry_errors(NETWORK_ERRORS, max_retries: @retries) do + open_connection do |conn| + path = '/latest/meta-data/iam/security-credentials/' + profile_name = http_get(conn, path).lines.first.strip + http_get(conn, path + profile_name) + end end + rescue + '{}' end end @@ -111,5 +114,21 @@ def http_get(connection, path) end end + def retry_errors(error_classes, options = {}, &block) + max_retries = options[:max_retries] + retries = 0 + begin + yield + rescue *error_classes => error + if retries < max_retries + @backoff.call(retries) + retries += 1 + retry + else + raise + end + end + end + end end diff --git a/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb b/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb index 5b351224dc7..154c760ca6b 100644 --- a/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb +++ b/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb @@ -88,6 +88,36 @@ module Aws expect(c.expiration.to_s).to eq(expiration2.to_s) end + it 'retries if get profile response is invalid JSON' do + stub_request(:get, "http://169.254.169.254#{path}"). + to_return(:status => 500). + to_return(:status => 200, :body => "profile-name\n") + stub_request(:get, "http://169.254.169.254#{path}profile-name"). + to_return(:status => 200, :body => ' '). + to_return(:status => 200, :body => ''). + to_return(:status => 200, :body => '{'). + to_return(:status => 200, :body => resp2) + c = InstanceProfileCredentials.new(backoff:0) + expect(c.credentials.access_key_id).to eq('akid-2') + expect(c.credentials.secret_access_key).to eq('secret-2') + expect(c.credentials.session_token).to eq('session-token-2') + expect(c.expiration.to_s).to eq(expiration2.to_s) + end + + it 'retries invalid JSON exactly 3 times' do + stub_request(:get, "http://169.254.169.254#{path}"). + to_return(:status => 500). + to_return(:status => 200, :body => "profile-name\n") + stub_request(:get, "http://169.254.169.254#{path}profile-name"). + to_return(:status => 200, :body => ''). + to_return(:status => 200, :body => ' '). + to_return(:status => 200, :body => '{'). + to_return(:status => 200, :body => ' ') + expect { + InstanceProfileCredentials.new(backoff:0) + }.to raise_error(JSON::ParserError) + end + describe 'auto refreshing' do # expire in 4 minutes