v3(services): delete a route binding **sync** (cloudfoundry#1868)

[#174191028](https://www.pivotaltracker.com/story/show/174191028)

Co-authored-by: Derik Evangelista <devangelista@pivotal.io>

Author: blgm

app/actions/service_route_binding_delete.rb

@@ -0,0 +1,58 @@
+module VCAP::CloudController
+  module V3
+    class ServiceRouteBindingDelete
+      class UnprocessableDelete < StandardError; end
+
+      RequiresAsync = Class.new.freeze
+      DeleteComplete = Class.new.freeze
+      DeleteInProgress = Struct.new(:operation).freeze
+
+      def initialize(service_event_repository)
+        @service_event_repository = service_event_repository
+      end
+
+      def delete(binding, async_allowed:)
+        return RequiresAsync unless async_allowed || binding.service_instance.user_provided_instance?
+
+        operation_in_progress! if binding.service_instance.operation_in_progress?
+
+        result = send_unbind_to_broker(binding)
+        unsupported! unless result == DeleteComplete
+
+        record_audit_event(binding)
+        binding.destroy
+        binding.notify_diego
+
+        DeleteComplete
+      end
+
+      private
+
+      attr_reader :service_event_repository
+
+      def send_unbind_to_broker(binding)
+        client = VCAP::Services::ServiceClientProvider.provide(instance: binding.service_instance)
+        details = client.unbind(binding, accepts_incomplete: false)
+        details[:async] ? DeleteInProgress.new(details[:operation]) : DeleteComplete
+      rescue => err
+        raise UnprocessableDelete.new("Service broker failed to delete service binding for instance #{binding.service_instance.name}: #{err.message}")
+      end
+
+      def record_audit_event(binding)
+        service_event_repository.record_service_instance_event(
+          :unbind_route,
+          binding.service_instance,
+          { route_guid: binding.route.guid },
+        )
+      end
+
+      def operation_in_progress!
+        raise UnprocessableDelete.new('There is an operation in progress for the service instance')
+      end
+
+      def unsupported!
+        raise UnprocessableDelete.new('async unbind not supported yet')
+      end
+    end
+  end
+end

app/controllers/v3/service_route_bindings_controller.rb

@@ -2,7 +2,9 @@
 require 'messages/service_route_binding_show_message'
 require 'messages/service_route_bindings_list_message'
 require 'actions/service_route_binding_create'
+require 'actions/service_route_binding_delete'
 require 'jobs/v3/create_route_binding_job'
+require 'jobs/v3/delete_route_binding_job'
 require 'presenters/v3/paginated_list_presenter'
 require 'presenters/v3/service_route_binding_presenter'
 require 'fetchers/route_binding_list_fetcher'
@@ -59,6 +61,21 @@ def index
     )
   end
 
+  def destroy
+    route_binding = RouteBinding.first(guid: hashed_params[:guid])
+    route_binding_not_found! unless route_binding && can_read_space?(route_binding.route.space)
+
+    action = V3::ServiceRouteBindingDelete.new(service_event_repository)
+    result = action.delete(route_binding, async_allowed: false)
+
+    if result == V3::ServiceRouteBindingDelete::RequiresAsync
+      pollable_job_guid = enqueue_unbind_job(route_binding.guid)
+      head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}")
+    else
+      head :no_content
+    end
+  end
+
   private
 
   AVAILABLE_DECORATORS = [
@@ -94,6 +111,15 @@ def enqueue_bind_job(binding_guid, parameters)
     pollable_job.guid
   end
 
+  def enqueue_unbind_job(binding_guid)
+    bind_job = VCAP::CloudController::V3::DeleteRouteBindingJob.new(
+      binding_guid,
+      user_audit_info: user_audit_info,
+    )
+    pollable_job = Jobs::Enqueuer.new(bind_job, queue: Jobs::Queues.generic).enqueue_pollable
+    pollable_job.guid
+  end
+
   def fetch_route_bindings(message)
     fetcher = RouteBindingListFetcher.new
     if permission_queryer.can_read_globally?

app/jobs/v3/delete_route_binding_job.rb

@@ -0,0 +1,60 @@
+require 'jobs/reoccurring_job'
+require 'actions/service_route_binding_create'
+require 'cloud_controller/errors/api_error'
+
+module VCAP::CloudController
+  module V3
+    class DeleteRouteBindingJob < VCAP::CloudController::Jobs::CCJob
+      def initialize(binding_guid, user_audit_info:)
+        super()
+        @binding_guid = binding_guid
+        @user_audit_info = user_audit_info
+      end
+
+      def operation
+        :unbind
+      end
+
+      def operation_type
+        'delete'
+      end
+
+      def max_attempts
+        1
+      end
+
+      def display_name
+        'service_route_bindings.delete'
+      end
+
+      def resource_guid
+        @binding_guid
+      end
+
+      def resource_type
+        'service_route_binding'
+      end
+
+      def perform
+        binding = route_binding
+        gone! unless binding
+
+        service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithUserActor.new(@user_audit_info)
+        action = V3::ServiceRouteBindingDelete.new(service_event_repository)
+        action.delete(binding, async_allowed: true)
+      rescue => e
+        raise CloudController::Errors::ApiError.new_from_details('UnableToPerform', 'unbind', e.message)
+      end
+
+      private
+
+      def route_binding
+        RouteBinding.first(guid: resource_guid)
+      end
+
+      def gone!
+        raise CloudController::Errors::ApiError.new_from_details('ResourceNotFound', "The binding could not be found: #{resource_guid}")
+      end
+    end
+  end
+end

config/routes.rb

@@ -197,7 +197,7 @@
   # service_route_bindings
   resources :service_route_bindings,
     param: :guid,
-    only: [:show, :create, :index]
+    only: [:show, :create, :index, :destroy]
 
   # service_brokers
   get '/service_brokers', to: 'service_brokers#index'

spec/request/service_route_bindings_spec.rb

@@ -218,7 +218,7 @@
         get "/v3/jobs/#{job.guid}", nil, space_dev_headers
 
         expect(last_response).to have_status_code(200)
-        expect(parsed_response['guid']). to eq(job.guid)
+        expect(parsed_response['guid']).to eq(job.guid)
       end
 
       describe 'the pollable job' do
@@ -650,11 +650,11 @@
         [
           expected_json(
             binding_guid: route_binding_1.guid,
-              route_service_url: route_service_url,
-              service_instance_guid: service_instance_1.guid,
-              route_guid: route.guid,
-              last_operation_type: 'create',
-              last_operation_state: 'successful',
+            route_service_url: route_service_url,
+            service_instance_guid: service_instance_1.guid,
+            route_guid: route.guid,
+            last_operation_type: 'create',
+            last_operation_state: 'successful',
           ),
           expected_json(
             binding_guid: route_binding_2.guid,
@@ -867,6 +867,149 @@
     end
   end
 
+  describe 'DELETE /v3/service_route_bindings/:guid' do
+    let(:api_call) { lambda { |user_headers| delete "/v3/service_route_bindings/#{guid}", nil, user_headers } }
+
+    context 'route binding exists' do
+      let(:route) { VCAP::CloudController::Route.make(space: space) }
+      let(:binding) do
+        VCAP::CloudController::RouteBinding.new.save_with_new_operation(
+          { service_instance: service_instance, route: route, route_service_url: route_service_url },
+          { type: 'create', state: 'successful' }
+        )
+      end
+      let(:guid) { binding.guid }
+
+      context 'user-provided service instance' do
+        let(:service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make(space: space, route_service_url: route_service_url) }
+
+        let(:expected_codes_and_responses) { responses_for_space_restricted_delete_endpoint }
+        let(:db_check) {
+          lambda do
+            expect(VCAP::CloudController::RouteBinding.all).to be_empty
+          end
+        }
+
+        it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS
+      end
+
+      context 'managed service instance' do
+        let(:service_offering) { VCAP::CloudController::Service.make(requires: ['route_forwarding']) }
+        let(:service_plan) { VCAP::CloudController::ServicePlan.make(service: service_offering) }
+        let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, service_plan: service_plan) }
+
+        let(:expected_codes_and_responses) { responses_for_space_restricted_async_delete_endpoint }
+        let(:db_check) { lambda {} }
+        let(:job) { VCAP::CloudController::PollableJobModel.last }
+
+        it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS
+
+        it 'responds with a job resource' do
+          api_call.call(space_dev_headers)
+          expect(last_response).to have_status_code(202)
+          expect(last_response.headers['Location']).to end_with("/v3/jobs/#{job.guid}")
+
+          expect(job.state).to eq(VCAP::CloudController::PollableJobModel::PROCESSING_STATE)
+          expect(job.operation).to eq('service_route_bindings.delete')
+          expect(job.resource_guid).to eq(binding.guid)
+          expect(job.resource_type).to eq('service_route_binding')
+
+          get "/v3/jobs/#{job.guid}", nil, space_dev_headers
+
+          expect(last_response).to have_status_code(200)
+          expect(parsed_response['guid']).to eq(job.guid)
+        end
+
+        describe 'the pollable job' do
+          let(:broker_base_url) { service_instance.service_broker.broker_url }
+          let(:broker_unbind_url) { "#{broker_base_url}/v2/service_instances/#{service_instance.guid}/service_bindings/#{binding.guid}" }
+          let(:route_service_url) { 'https://route_service_url.com' }
+          let(:broker_status_code) { 200 }
+          let(:broker_response) { {} }
+          let(:query) do
+            {
+              service_id: service_instance.service_plan.service.unique_id,
+              plan_id: service_instance.service_plan.unique_id,
+            }
+          end
+
+          before do
+            api_call.call(space_dev_headers)
+            expect(last_response).to have_status_code(202)
+
+            stub_request(:delete, broker_unbind_url).
+              with(query: query).
+              to_return(status: broker_status_code, body: broker_response.to_json, headers: {})
+          end
+
+          it 'sends an unbind request with the right arguments to the service broker' do
+            execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+            expect(
+              a_request(:delete, broker_unbind_url).
+                with(
+                  query: query,
+                )
+            ).to have_been_made.once
+          end
+
+          context 'when the unbind completes synchronously' do
+            it 'removes the binding' do
+              execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+              expect(VCAP::CloudController::RouteBinding.all).to be_empty
+            end
+
+            it 'completes the job' do
+              execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::COMPLETE_STATE)
+            end
+          end
+
+          context 'when the broker returns a failure' do
+            let(:broker_status_code) { 418 }
+            let(:broker_response) { 'nope' }
+
+            it 'does not remove the binding' do
+              execute_all_jobs(expected_successes: 0, expected_failures: 1)
+
+              expect(VCAP::CloudController::RouteBinding.all).not_to be_empty
+            end
+
+            it 'puts the error details in the job' do
+              execute_all_jobs(expected_successes: 0, expected_failures: 1)
+
+              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::FAILED_STATE)
+              expect(job.cf_api_error).not_to be_nil
+              error = YAML.safe_load(job.cf_api_error)
+              expect(error['errors'].first['code']).to eq(10009)
+              expect(error['errors'].first['detail']).
+                to include('The service broker rejected the request. Status Code: 418 I\'m a Teapot, Body: "nope"')
+            end
+          end
+        end
+      end
+    end
+
+    context 'no route binding' do
+      let(:guid) { 'no-such-route-binding' }
+
+      it 'fails with the correct error' do
+        api_call.call(space_dev_headers)
+
+        expect(last_response).to have_status_code(404)
+        expect(parsed_response['errors']).to include(
+          include({
+            'detail' => 'Service route binding not found',
+            'title' => 'CF-ResourceNotFound',
+            'code' => 10010,
+          })
+        )
+      end
+    end
+  end
+
   let(:user) { VCAP::CloudController::User.make }
   let(:org) { VCAP::CloudController::Organization.make }
   let(:space) { VCAP::CloudController::Space.make(organization: org) }
@@ -921,7 +1064,7 @@ def bind_service_to_route(service_instance, route)
     route_service_url = service_instance.route_service_url
     VCAP::CloudController::RouteBinding.new.save_with_new_operation(
       { service_instance: service_instance, route: route, route_service_url: route_service_url },
-        { type: 'create', state: 'successful' }
+      { type: 'create', state: 'successful' }
     )
   end
 end

spec/support/space_restricted_response_generators.rb

@@ -21,4 +21,24 @@ def responses_for_space_restricted_single_endpoint(
       end
     end
   end
+
+  def responses_for_space_restricted_delete_endpoint(
+    permitted_roles: SpaceRestrictedResponseGenerators.default_permitted_roles
+  )
+    Hash.new(code: 404).tap do |h|
+      permitted_roles.each do |role|
+        h[role] = { code: 204 }
+      end
+    end
+  end
+
+  def responses_for_space_restricted_async_delete_endpoint(
+    permitted_roles: SpaceRestrictedResponseGenerators.default_permitted_roles
+  )
+    Hash.new(code: 404).tap do |h|
+      permitted_roles.each do |role|
+        h[role] = { code: 202 }
+      end
+    end
+  end
 end