fix: add service_id and plan_id to OSBAPI (service) fetch endpoints

Add optional service_id and plan_id query parameters to fetch_service_instance
and fetch_service_binding broker requests, matching the pattern used by
last_operation endpoints.

Enables delegate/router brokers to route fetch requests without falling back
to trying all sub-brokers. While optional per OSBAPI spec, these parameters
are helpful hints that CC already has available. An aim of a platform (like CC)
is to make it easier to write brokers - especially stateless brokers, so low
cost hints like this bring the code in line with the product aims.

- [x] I have reviewed the contributing guide
- [x] I have viewed, signed, and submitted the Contributor License Agreement
- [x] I have made this pull request to the `main` branch
- [x] I have run all the unit tests using `bundle exec rake`
- [ ] I have run CF Acceptance Tests

Author: blgm

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|

spec/request/service_instances_spec.rb

@@ -1515,6 +1515,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 +2122,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
 

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
 

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