diff --git a/app/controllers/v3/roles_controller.rb b/app/controllers/v3/roles_controller.rb index 6d6eae3b184..456f7e009ec 100644 --- a/app/controllers/v3/roles_controller.rb +++ b/app/controllers/v3/roles_controller.rb @@ -58,6 +58,9 @@ def create render status: :created, json: Presenters::V3::RolePresenter.new(role) rescue RoleCreate::Error => e unprocessable!(e) + rescue UaaRateLimited + headers['Retry-After'] = rand(5..20).to_s + raise CloudController::Errors::V3::ApiError.new_from_details('UaaRateLimited') rescue UaaUnavailable raise CloudController::Errors::ApiError.new_from_details('UaaUnavailable') end diff --git a/app/controllers/v3/users_controller.rb b/app/controllers/v3/users_controller.rb index bc6ccb925a4..cd6eb6d30b1 100644 --- a/app/controllers/v3/users_controller.rb +++ b/app/controllers/v3/users_controller.rb @@ -54,6 +54,9 @@ def create else render status: :created, json: Presenters::V3::UserPresenter.new(user, uaa_users: User.uaa_users_info([user.guid])) end + rescue UaaRateLimited + headers['Retry-After'] = rand(5..20).to_s + raise CloudController::Errors::V3::ApiError.new_from_details('UaaRateLimited') rescue VCAP::CloudController::UaaUnavailable raise CloudController::Errors::ApiError.new_from_details('UaaUnavailable') rescue UserCreate::Error => e diff --git a/errors/v3.yml b/errors/v3.yml index 49cdc38a232..3022af70ad4 100644 --- a/errors/v3.yml +++ b/errors/v3.yml @@ -12,3 +12,8 @@ name: ServiceBrokerGone http_code: 422 message: "The service broker was removed before the synchronization completed" + +20008: + name: UaaRateLimited + http_code: 429 + message: "The UAA is currently rate limited. Please try again later" diff --git a/lib/cloud_controller/uaa/errors.rb b/lib/cloud_controller/uaa/errors.rb index b30e0b4152a..0ca47c2faf6 100644 --- a/lib/cloud_controller/uaa/errors.rb +++ b/lib/cloud_controller/uaa/errors.rb @@ -30,4 +30,10 @@ def message 'The UAA returned an unexpected error' end end + + class UaaRateLimited < UaaError + def message + 'The UAA is currently rate limited. Please try again later' + end + end end diff --git a/lib/cloud_controller/uaa/uaa_client.rb b/lib/cloud_controller/uaa/uaa_client.rb index d997a2277db..0708cd09bc1 100644 --- a/lib/cloud_controller/uaa/uaa_client.rb +++ b/lib/cloud_controller/uaa/uaa_client.rb @@ -102,6 +102,14 @@ def create_shadow_user(username, origin) raise e unless e.info['error'] == 'scim_resource_already_exists' { 'id' => e.info['user_id'] } + rescue CF::UAA::BadResponse => e + unless e.message == 'invalid status response: 429' + logger.error("UAA request for creating a user failed: #{e.inspect}") + raise UaaUnavailable + end + + logger.warn("UAA request for creating a user ran into rate limits: #{e.inspect}") + raise UaaRateLimited rescue CF::UAA::UAAError => e logger.error("UAA request for creating a user failed: #{e.inspect}") raise UaaUnavailable diff --git a/spec/request/roles_spec.rb b/spec/request/roles_spec.rb index 75f283e88b3..421765a8052 100644 --- a/spec/request/roles_spec.rb +++ b/spec/request/roles_spec.rb @@ -1044,6 +1044,22 @@ expect(parsed_response).to match_json_response(expected_response) expect(uaa_client).to have_received(:create_shadow_user) end + + context 'when UAA is rate limited' do + before do + allow(uaa_client).to receive(:create_shadow_user).and_raise(VCAP::CloudController::UaaRateLimited) + end + + it 'raises a 429 with a helpful message and Retry-After header' do + post '/v3/roles', params.to_json, admin_header + + expect(last_response).to have_status_code(429) + expect(last_response).to have_error_message( + 'The UAA is currently rate limited. Please try again later' + ) + expect(last_response.headers).to include('Retry-After') + end + end end end end diff --git a/spec/request/users_spec.rb b/spec/request/users_spec.rb index 9997a8bb6ce..bf15263c0ca 100644 --- a/spec/request/users_spec.rb +++ b/spec/request/users_spec.rb @@ -968,6 +968,22 @@ expect(uaa_client).not_to have_received(:users_for_ids) end + context 'when UAA is rate limited' do + before do + allow(uaa_client).to receive(:create_shadow_user).and_raise(VCAP::CloudController::UaaRateLimited) + end + + it 'raises a 429 with a helpful message and Retry-After header' do + post '/v3/users', params.to_json, admin_header + + expect(last_response).to have_status_code(429) + expect(last_response).to have_error_message( + 'The UAA is currently rate limited. Please try again later' + ) + expect(last_response.headers).to include('Retry-After') + end + end + context 'when user already exists in UAA' do before do allow(uaa_client).to receive(:ids_for_usernames_and_origins).and_return([user_guid]) diff --git a/spec/unit/lib/uaa/uaa_client_spec.rb b/spec/unit/lib/uaa/uaa_client_spec.rb index 04a23122020..b54a016fe31 100644 --- a/spec/unit/lib/uaa/uaa_client_spec.rb +++ b/spec/unit/lib/uaa/uaa_client_spec.rb @@ -522,6 +522,38 @@ module VCAP::CloudController expect(mock_logger).to have_received(:error).with("UAA request for creating a user failed: #{uaa_error.inspect}") end end + + context 'when uaa is rate limited' do + let(:uaa_error) { CF::UAA::BadResponse.new('invalid status response: 429') } + let(:mock_logger) { double(:steno_logger, warn: nil) } + + before do + scim = instance_double(CF::UAA::Scim) + allow(scim).to receive(:add).and_raise(uaa_error) + allow(uaa_client).to receive_messages(scim: scim, logger: mock_logger) + end + + it 'logs the error and raises UaaRateLimited' do + expect { uaa_client.create_shadow_user('test-user@idp.local', 'idp.local') }.to raise_error(UaaRateLimited) + expect(mock_logger).to have_received(:warn).with("UAA request for creating a user ran into rate limits: #{uaa_error.inspect}") + end + end + + context 'when a BadResponse is raised' do + let(:uaa_error) { CF::UAA::BadResponse.new('invalid status response: 433') } + let(:mock_logger) { double(:steno_logger, error: nil) } + + before do + scim = instance_double(CF::UAA::Scim) + allow(scim).to receive(:add).and_raise(uaa_error) + allow(uaa_client).to receive_messages(scim: scim, logger: mock_logger) + end + + it 'logs and raises the error' do + expect { uaa_client.create_shadow_user('test-user@idp.local', 'idp.local') }.to raise_error(UaaUnavailable) + expect(mock_logger).to have_received(:error).with("UAA request for creating a user failed: #{uaa_error.inspect}") + end + end end describe '#id_for_username' do