From d19096f4b046e56e15f59fb7828c51fd5ee1ed6a Mon Sep 17 00:00:00 2001
From: Trevor Rowe <trevorrowe@gmail.com>
Date: Tue, 1 Sep 2015 14:08:15 -0700
Subject: [PATCH] Expanded retry logic in Aws::InstanceProfileCredentials.

Fixes #913.
---
 CHANGELOG.md                                  |  8 +++
 .../instance_profile_credentials.rb           | 61 ++++++++++++-------
 .../aws/instance_profile_credentials_spec.rb  | 30 +++++++++
 3 files changed, 78 insertions(+), 21 deletions(-)

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