diff --git a/lib/services/service_brokers/v2/client.rb b/lib/services/service_brokers/v2/client.rb index e80cf656810..edeccc96f09 100644 --- a/lib/services/service_brokers/v2/client.rb +++ b/lib/services/service_brokers/v2/client.rb @@ -312,13 +312,13 @@ def fetch_and_handle_service_binding_last_operation(service_binding, user_guid: end def fetch_service_instance(instance, user_guid: nil) - path = service_instance_resource_path(instance) + path = fetch_service_instance_path(instance) response = @http_client.get(path, user_guid:) @response_parser.parse_fetch_service_instance(path, response).deep_symbolize_keys end def fetch_service_binding(service_binding, user_guid: nil) - path = service_binding_resource_path(service_binding.guid, service_binding.service_instance_guid) + path = fetch_service_binding_path(service_binding) response = @http_client.get(path, user_guid:) @response_parser.parse_fetch_service_binding(path, response).deep_symbolize_keys end @@ -407,12 +407,28 @@ def service_binding_last_operation_path(service_binding) "#{service_binding_resource_path(service_binding.guid, service_binding.service_instance_guid)}/last_operation?#{query_params.to_query}" end + def fetch_service_binding_path(service_binding) + query_params = { + 'service_id' => service_binding.service_instance.service.broker_provided_id, + 'plan_id' => service_binding.service_instance.service_plan.broker_provided_id + } + "#{service_binding_resource_path(service_binding.guid, service_binding.service_instance_guid)}?#{query_params.to_query}" + end + def service_instance_resource_path(instance, opts={}) path = "/v2/service_instances/#{instance.guid}" path += '?accepts_incomplete=true' if opts[:accepts_incomplete] path end + def fetch_service_instance_path(instance) + query_params = { + 'service_id' => instance.service.broker_provided_id, + 'plan_id' => instance.service_plan.broker_provided_id + } + "#{service_instance_resource_path(instance)}?#{query_params.to_query}" + end + def hashified_public_annotations(annotations) public_annotations = [] annotations.each do |annotation| diff --git a/spec/acceptance/broker_api_compatibility/broker_api_v2.14_spec.rb b/spec/acceptance/broker_api_compatibility/broker_api_v2.14_spec.rb index f97c0143955..e12a6b860d0 100644 --- a/spec/acceptance/broker_api_compatibility/broker_api_v2.14_spec.rb +++ b/spec/acceptance/broker_api_compatibility/broker_api_v2.14_spec.rb @@ -173,12 +173,14 @@ end context 'when the last operation is successful' do + let(:fetch_binding_query) { 'plan_id=plan1-guid-here&service_id=service-guid-here' } + it 'fetches the service binding details' do stub_async_last_operation async_bind_service(status: 202) service_binding = VCAP::CloudController::ServiceBinding.find(guid: @binding_guid) - stub_request(:get, service_binding_url(service_binding)).to_return(status: 200, body: '{"credentials": {"foo": true}}') + stub_request(:get, service_binding_url(service_binding, fetch_binding_query)).to_return(status: 200, body: '{"credentials": {"foo": true}}') Delayed::Worker.new.work_off @@ -199,7 +201,7 @@ context 'is invalid' do it 'set the last operation status to failed and does not perform orphan mitigation' do - stub_request(:get, service_binding_url(service_binding)).to_return(status: 200, body: 'invalid-response') + stub_request(:get, service_binding_url(service_binding, fetch_binding_query)).to_return(status: 200, body: 'invalid-response') Delayed::Worker.new.work_off @@ -210,7 +212,7 @@ context 'is not 200' do it 'set the last operation status to failed and does not perform orphan mitigation' do - stub_request(:get, service_binding_url(service_binding)).to_return(status: 204, body: '{}') + stub_request(:get, service_binding_url(service_binding, fetch_binding_query)).to_return(status: 204, body: '{}') Delayed::Worker.new.work_off @@ -221,7 +223,7 @@ context 'times out' do it 'set the last operation status to failed and does not perform orphan mitigation' do - stub_request(:get, service_binding_url(service_binding)).to_timeout + stub_request(:get, service_binding_url(service_binding, fetch_binding_query)).to_timeout Delayed::Worker.new.work_off diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb index 68835e7765c..6ccb84e750f 100644 --- a/spec/request/service_credential_bindings_spec.rb +++ b/spec/request/service_credential_bindings_spec.rb @@ -1095,6 +1095,7 @@ def check_filtered_bindings(*bindings) expect( a_request(:get, broker_binding_url). with( + query: { service_id: instance.service.broker_provided_id, plan_id: instance.service_plan.broker_provided_id }, headers: { 'X-Broker-Api-Originating-Identity' => "cloudfoundry #{encoded_user_guid}" } ) ).to have_been_made.once @@ -1157,6 +1158,7 @@ def check_filtered_bindings(*bindings) expect( a_request(:get, broker_binding_url). with( + query: { service_id: instance.service.broker_provided_id, plan_id: instance.service_plan.broker_provided_id }, headers: { 'X-Broker-Api-Originating-Identity' => "cloudfoundry #{encoded_user_guid}" } ) ).to have_been_made.once diff --git a/spec/request/service_instances_spec.rb b/spec/request/service_instances_spec.rb index b9c69073434..129c2155144 100644 --- a/spec/request/service_instances_spec.rb +++ b/spec/request/service_instances_spec.rb @@ -641,6 +641,7 @@ def check_filtered_instances(*instances) encoded_user_guid = Base64.strict_encode64("{\"user_id\":\"#{user.guid}\"}") expect(a_request(:get, "#{instance.service_broker.broker_url}/v2/service_instances/#{instance.guid}"). with( + query: { service_id: service_plan.service.unique_id, plan_id: service_plan.unique_id }, headers: { 'X-Broker-Api-Originating-Identity' => "cloudfoundry #{encoded_user_guid}" } )).to have_been_made.once end @@ -1515,6 +1516,7 @@ def check_filtered_instances(*instances) to_return(status: 200, body: { state: 'succeeded' }.to_json, headers: {}) stub_request(:get, "#{instance.service.service_broker.broker_url}/v2/service_instances/#{instance.guid}"). + with(query: { service_id: service_plan.service.unique_id, plan_id: service_plan.unique_id }). to_return(status: 200, body: { dashboard_url: }.to_json) execute_all_jobs(expected_successes: 1, expected_failures: 0) @@ -2121,6 +2123,7 @@ def check_filtered_instances(*instances) to_return(status: last_operation_status_code, body: last_operation_response.to_json, headers: {}) stub_request(:get, "#{service_instance.service_broker.broker_url}/v2/service_instances/#{service_instance.guid}"). + with(query: { service_id: service_instance.service_plan.service.unique_id, plan_id: service_instance.service_plan.unique_id }). to_return(status: 200, body: { dashboard_url: }.to_json) end diff --git a/spec/request/service_route_bindings_spec.rb b/spec/request/service_route_bindings_spec.rb index af2cde52cc0..cf0dc3e5a73 100644 --- a/spec/request/service_route_bindings_spec.rb +++ b/spec/request/service_route_bindings_spec.rb @@ -844,9 +844,11 @@ let(:fetch_binding_body) do { route_service_url: } end + let(:fetch_binding_query) { { service_id: service_instance.service.broker_provided_id, plan_id: service_instance.service_plan.broker_provided_id } } before do stub_request(:get, broker_bind_url). + with(query: fetch_binding_query). to_return(status: fetch_binding_status_code, body: fetch_binding_body.to_json, headers: {}) end @@ -856,6 +858,7 @@ encoded_user_guid = Base64.strict_encode64("{\"user_id\":\"#{user.guid}\"}") expect( a_request(:get, broker_bind_url).with( + query: fetch_binding_query, headers: { 'X-Broker-Api-Originating-Identity' => "cloudfoundry #{encoded_user_guid}" } ) ).to have_been_made.once @@ -1634,12 +1637,14 @@ context 'managed service instances' do let(:broker_base_url) { service_instance.service_broker.broker_url } let(:broker_fetch_binding_url) { "#{broker_base_url}/v2/service_instances/#{service_instance.guid}/service_bindings/#{binding.guid}" } + let(:broker_fetch_binding_query) { { service_id: service_instance.service.broker_provided_id, plan_id: service_instance.service_plan.broker_provided_id } } let(:broker_status_code) { 200 } let(:broker_response) { { parameters: { abra: 'kadabra', kadabra: 'alakazan' } } } let(:parameters_response) { { code: 200, response_object: broker_response[:parameters] } } before do stub_request(:get, broker_fetch_binding_url). + with(query: broker_fetch_binding_query). to_return(status: broker_status_code, body: broker_response.to_json, headers: {}) end @@ -1679,6 +1684,7 @@ expect( a_request(:get, broker_fetch_binding_url). with( + query: broker_fetch_binding_query, headers: { 'X-Broker-Api-Originating-Identity' => "cloudfoundry #{encoded_user_guid}" } ) ).to have_been_made.once @@ -1719,6 +1725,7 @@ context 'when the broker returns params with mixed data types' do before do stub_request(:get, broker_fetch_binding_url). + with(query: broker_fetch_binding_query). to_return(status: broker_status_code, body: "{\"parameters\":#{parameters_mixed_data_types_as_json_string}}") end diff --git a/spec/support/shared_examples/request/bindings_create.rb b/spec/support/shared_examples/request/bindings_create.rb index 6539bda433c..bf73a274766 100644 --- a/spec/support/shared_examples/request/bindings_create.rb +++ b/spec/support/shared_examples/request/bindings_create.rb @@ -255,6 +255,12 @@ let(:fetch_binding_status_code) { 200 } let(:credentials) { { password: 'foo' } } let(:parameters) { { foo: 'bar', another_foo: 'another_bar' } } + let(:broker_fetch_binding_query) do + { + service_id: service_instance.service_plan.service.unique_id, + plan_id: service_instance.service_plan.unique_id + } + end let(:app_binding_attributes) do if check_app @@ -277,6 +283,7 @@ before do stub_request(:get, broker_bind_url). + with(query: broker_fetch_binding_query). to_return(status: fetch_binding_status_code, body: fetch_binding_body.to_json, headers: {}) end @@ -286,6 +293,7 @@ encoded_user_guid = Base64.strict_encode64("{\"user_id\":\"#{user.guid}\"}") expect( a_request(:get, broker_bind_url).with( + query: broker_fetch_binding_query, headers: { 'X-Broker-Api-Originating-Identity' => "cloudfoundry #{encoded_user_guid}" } ) ).to have_been_made.once diff --git a/spec/unit/lib/services/service_brokers/v2/client_spec.rb b/spec/unit/lib/services/service_brokers/v2/client_spec.rb index 209834eb7f0..6ab67037449 100644 --- a/spec/unit/lib/services/service_brokers/v2/client_spec.rb +++ b/spec/unit/lib/services/service_brokers/v2/client_spec.rb @@ -2275,10 +2275,15 @@ module VCAP::Services::ServiceBrokers::V2 allow(http_client).to receive(:get).and_return(broker_response) end - it 'makes a get request with the correct path' do + it 'makes a get request with the correct path including service_id and plan_id' do client.fetch_service_binding(binding) + + service_id = binding.service_instance.service.broker_provided_id + plan_id = binding.service_instance.service_plan.broker_provided_id + query_params = "?plan_id=#{plan_id}&service_id=#{service_id}" + expect(http_client).to have_received(:get).with( - "/v2/service_instances/#{binding.service_instance.guid}/service_bindings/#{binding.guid}", + "/v2/service_instances/#{binding.service_instance.guid}/service_bindings/#{binding.guid}#{query_params}", { user_guid: nil } ) end @@ -2293,8 +2298,13 @@ module VCAP::Services::ServiceBrokers::V2 it 'makes a request with the correct user_guid' do client.fetch_service_binding(binding, user_guid:) + + service_id = binding.service_instance.service.broker_provided_id + plan_id = binding.service_instance.service_plan.broker_provided_id + query_params = "?plan_id=#{plan_id}&service_id=#{service_id}" + expect(http_client).to have_received(:get).with( - "/v2/service_instances/#{binding.service_instance.guid}/service_bindings/#{binding.guid}", + "/v2/service_instances/#{binding.service_instance.guid}/service_bindings/#{binding.guid}#{query_params}", { user_guid: } ) end @@ -2309,10 +2319,15 @@ module VCAP::Services::ServiceBrokers::V2 allow(http_client).to receive(:get).and_return(broker_response) end - it 'makes a get request with the correct path' do + it 'makes a get request with the correct path including service_id and plan_id' do client.fetch_service_instance(instance) + + service_id = instance.service.broker_provided_id + plan_id = instance.service_plan.broker_provided_id + query_params = "?plan_id=#{plan_id}&service_id=#{service_id}" + expect(http_client).to have_received(:get).with( - "/v2/service_instances/#{instance.guid}", + "/v2/service_instances/#{instance.guid}#{query_params}", { user_guid: nil } ) end @@ -2327,8 +2342,13 @@ module VCAP::Services::ServiceBrokers::V2 it 'makes a request with the correct user_guid' do client.fetch_service_instance(instance, user_guid:) + + service_id = instance.service.broker_provided_id + plan_id = instance.service_plan.broker_provided_id + query_params = "?plan_id=#{plan_id}&service_id=#{service_id}" + expect(http_client).to have_received(:get).with( - "/v2/service_instances/#{instance.guid}", + "/v2/service_instances/#{instance.guid}#{query_params}", { user_guid: } ) end