Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Commit c5373de

Browse files
authored
V3 service credential job should fail when the broker request fails (cloudfoundry#1897)
[#174119280](https://www.pivotaltracker.com/story/show/174119280) * Add audit log for credential binding create in progress * Binding job should fail when the broker request fails * Fix formatting * Fix formatting
1 parent ce99df8 commit c5373de

5 files changed

Lines changed: 69 additions & 10 deletions

File tree

app/actions/service_credential_binding_create.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,18 @@ def complete_binding_and_save(binding, binding_details, last_operation)
5555
event_repository.record_create(binding, @user_audit_info, @audit_hash, manifest_triggered: false)
5656
end
5757

58+
def save_incomplete_binding(binding, broker_operation)
59+
binding.save_with_attributes_and_new_operation(
60+
{},
61+
{
62+
type: 'create',
63+
state: 'in progress',
64+
broker_provided_operation: broker_operation
65+
}
66+
)
67+
event_repository.record_start_create(binding, @user_audit_info, @audit_hash, manifest_triggered: false)
68+
end
69+
5870
def validate!(service_instance, app, volume_mount_services_enabled)
5971
app_is_required! unless app.present?
6072
space_mismatch! unless all_space_guids(service_instance).include? app.space.guid

app/actions/v3/service_binding_create.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def poll(binding)
6464
description: e.message,
6565
}
6666
)
67-
PollingFinished
67+
raise e
6868
end
6969

7070
class BindingNotRetrievable < StandardError; end

spec/request/service_credential_bindings_spec.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,6 +1049,7 @@ def check_filtered_bindings(*bindings)
10491049
let(:plan) { VCAP::CloudController::ServicePlan.make(service: offering) }
10501050
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, service_plan: plan) }
10511051
let(:binding) { VCAP::CloudController::ServiceBinding.last }
1052+
let(:audit) { VCAP::CloudController::Event.last }
10521053
let(:job) { VCAP::CloudController::PollableJobModel.last }
10531054

10541055
it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do
@@ -1286,6 +1287,17 @@ def check_filtered_bindings(*bindings)
12861287
expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE)
12871288
end
12881289

1290+
it 'logs an audit event' do
1291+
execute_all_jobs(expected_successes: 1, expected_failures: 0)
1292+
1293+
event = VCAP::CloudController::Event.find(type: 'audit.service_binding.start_create')
1294+
expect(event).to be
1295+
expect(event.actee).to eq(binding.guid)
1296+
expect(event.data).to include({
1297+
'request' => create_body.with_indifferent_access
1298+
})
1299+
end
1300+
12891301
it 'enqueues the next fetch last operation job' do
12901302
execute_all_jobs(expected_successes: 1, expected_failures: 0)
12911303
expect(Delayed::Job.count).to eq(1)
@@ -1325,6 +1337,28 @@ def check_filtered_bindings(*bindings)
13251337

13261338
expect(job.state).to eq(VCAP::CloudController::PollableJobModel::COMPLETE_STATE)
13271339
end
1340+
1341+
context 'fetching binding fails ' do
1342+
let(:fetch_binding_status_code) { 404 }
1343+
let(:fetch_binding_body) {}
1344+
1345+
it 'fails the job' do
1346+
execute_all_jobs(expected_successes: 0, expected_failures: 1)
1347+
1348+
expect(binding.last_operation.type).to eq('create')
1349+
expect(binding.last_operation.state).to eq('failed')
1350+
expect(binding.last_operation.description).to include('The service broker rejected the request. Status Code: 404 Not Found')
1351+
1352+
expect(job.state).to eq(VCAP::CloudController::PollableJobModel::FAILED_STATE)
1353+
expect(job.cf_api_error).not_to be_nil
1354+
error = YAML.safe_load(job.cf_api_error)
1355+
expect(error['errors'].first).to include({
1356+
'code' => 10009,
1357+
'title' => 'CF-UnableToPerform',
1358+
'detail' => 'bind could not be completed: The service broker rejected the request. Status Code: 404 Not Found, Body: null',
1359+
})
1360+
end
1361+
end
13281362
end
13291363

13301364
it_behaves_like 'binding last operation response handling', 'create'

spec/support/shared_examples/v3_service_binding_create.rb

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,7 @@ class BadError < StandardError; end
160160
it 'should stop polling for other errors' do
161161
allow(broker_client).to receive(:fetch_service_binding_last_operation).and_raise(RuntimeError)
162162

163-
complete = action.poll(binding)
164-
165-
expect(complete).to be_truthy
163+
expect { action.poll(binding) }.to raise_error(RuntimeError)
166164

167165
binding.reload
168166
expect(binding.last_operation.type).to eq('create')
@@ -203,9 +201,7 @@ class BadError < StandardError; end
203201
end
204202

205203
it 'marks the binding as failed' do
206-
complete = action.poll(binding)
207-
208-
expect(complete).to be_truthy
204+
expect { action.poll(binding) }.to raise_error(BadError)
209205

210206
binding.reload
211207
expect(binding.last_operation.type).to eq('create')

spec/unit/actions/service_credential_binding_create_spec.rb

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ module V3
1818
before do
1919
@service_binding_event_repository = Repositories::ServiceBindingEventRepository
2020
allow(@service_binding_event_repository).to receive(:record_create)
21+
allow(@service_binding_event_repository).to receive(:record_start_create)
2122
end
2223

2324
describe '#precursor' do
@@ -174,7 +175,7 @@ module V3
174175
it_behaves_like 'service binding creation', ServiceBinding
175176

176177
describe 'app specific behaviour' do
177-
RSpec.shared_examples 'the credential binding bind' do
178+
RSpec.shared_examples 'the sync credential binding' do
178179
it 'creates and returns the credential binding' do
179180
action.bind(precursor)
180181

@@ -220,7 +221,23 @@ module V3
220221
allow(VCAP::Services::ServiceBrokers::V2::Client).to receive(:new).and_return(broker_client)
221222
end
222223

223-
it_behaves_like 'the credential binding bind'
224+
it_behaves_like 'the sync credential binding'
225+
226+
context 'asynchronous binding' do
227+
let(:broker_provided_operation) { Sham.guid }
228+
let(:bind_async_response) { { async: true, operation: broker_provided_operation } }
229+
let(:broker_client) { instance_double(VCAP::Services::ServiceBrokers::V2::Client, bind: bind_async_response) }
230+
231+
it 'should log audit start_create' do
232+
action.bind(precursor)
233+
expect(@service_binding_event_repository).to have_received(:record_start_create).with(
234+
precursor,
235+
user_audit_info,
236+
audit_hash,
237+
manifest_triggered: false,
238+
)
239+
end
240+
end
224241
end
225242

226243
context 'user-provided service instance' do
@@ -233,7 +250,7 @@ module V3
233250
}
234251
let(:service_instance) { UserProvidedServiceInstance.make(**details) }
235252

236-
it_behaves_like 'the credential binding bind'
253+
it_behaves_like 'the sync credential binding'
237254
end
238255
end
239256
end

0 commit comments

Comments
 (0)