From 3f4a28659be342af7c255dfce0bb73d4a3604884 Mon Sep 17 00:00:00 2001 From: Jillian Tullo Date: Thu, 30 Mar 2017 11:41:28 -0400 Subject: [PATCH] remove resources from service https://www.pivotaltracker.com/story/show/142775271 --- app/controllers/api/services_controller.rb | 33 +++++-- config/api.yml | 4 + spec/requests/api/custom_actions_spec.rb | 4 +- spec/requests/api/services_spec.rb | 108 +++++++++++++++++++++ 4 files changed, 139 insertions(+), 10 deletions(-) diff --git a/app/controllers/api/services_controller.rb b/app/controllers/api/services_controller.rb index 3b25a1f9923..0036b64a963 100644 --- a/app/controllers/api/services_controller.rb +++ b/app/controllers/api/services_controller.rb @@ -22,17 +22,23 @@ def add_resource_resource(type, id, data) raise "Must specify a service href or id to add_resource to" unless id svc = resource_search(id, type, collection_class(type)) - resource_href = data.fetch_path("resource", "href") - raise "Must specify a resource reference" unless resource_href - - resource_type, resource_id = parse_href(resource_href) - raise "Invalid resource href specified #{resource_href}" unless resource_type && resource_id - - resource = resource_search(resource_id, resource_type, collection_class(resource_type)) + resource_type, resource = validate_resource(data) raise "Cannot assign #{resource_type} to #{service_ident(svc)}" unless resource.respond_to? :add_to_service resource.add_to_service(svc) - action_result(true, "Assigned resource #{resource_type} id:#{resource_id} to #{service_ident(svc)}") + action_result(true, "Assigned resource #{resource_type} id:#{resource.id} to #{service_ident(svc)}") + rescue => err + action_result(false, err.to_s) + end + + def remove_resource_resource(type, id, data) + raise 'Must specify a resource to remove_resource from' unless id + svc = resource_search(id, type, collection_class(type)) + + resource_type, resource = validate_resource(data) + + svc.remove_resource(resource) + action_result(true, "Unassigned resource #{resource_type} id:#{resource.id} from #{service_ident(svc)}") rescue => err action_result(false, err.to_s) end @@ -103,6 +109,17 @@ def suspend_resource(type, id = nil, _data = nil) private + def validate_resource(data) + resource_href = data.fetch_path("resource", "href") + raise "Must specify a resource reference" unless resource_href + + resource_type, resource_id = parse_href(resource_href) + raise "Invalid resource href specified #{resource_href}" unless resource_type && resource_id + + resource = resource_search(resource_id, resource_type, collection_class(resource_type)) + [resource_type, resource] + end + def build_service_attributes(data) attributes = data.dup attributes['job_template'] = fetch_configuration_script(data['job_template']) if data['job_template'] diff --git a/config/api.yml b/config/api.yml index 98a4392c59b..2cfd401cc20 100644 --- a/config/api.yml +++ b/config/api.yml @@ -1931,6 +1931,8 @@ :identifier: service_tag - :name: add_resource :identifier: service_edit + - :name: remove_resource + :identifier: service_edit :resource_actions: :get: - :name: read @@ -1956,6 +1958,8 @@ :identifier: service_delete - :name: add_resource :identifier: service_edit + - :name: remove_resource + :identifier: service_edit :delete: - :name: delete :identifier: service_delete diff --git a/spec/requests/api/custom_actions_spec.rb b/spec/requests/api/custom_actions_spec.rb index 1d9f1d9ac17..08a3994489a 100644 --- a/spec/requests/api/custom_actions_spec.rb +++ b/spec/requests/api/custom_actions_spec.rb @@ -75,7 +75,7 @@ def expect_result_to_have_custom_actions_hash run_get services_url(svc1.id) expect_result_to_have_keys(%w(id href actions)) - expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit add_resource)) + expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit add_resource remove_resource)) end end @@ -91,7 +91,7 @@ def expect_result_to_have_custom_actions_hash run_get services_url(svc1.id) expect_result_to_have_keys(%w(id href actions)) - expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit button1 button2 button3 add_resource)) + expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit button1 button2 button3 add_resource remove_resource)) end it "supports the custom_actions attribute" do diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index ea018cc157c..9f1895f74b9 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -820,4 +820,112 @@ def expect_svc_with_vms expect(response).to have_http_status(:forbidden) end end + + describe 'remove_resource' do + let(:vm1) { FactoryGirl.create(:vm_vmware) } + let(:vm2) { FactoryGirl.create(:vm_vmware) } + + before do + svc.add_resource(vm1) + svc1.add_resource(vm2) + end + + it 'cannot remove vms from services without an appropriate role' do + api_basic_authorize + + run_post(services_url, 'action' => 'remove_resource') + + expect(response).to have_http_status(:forbidden) + end + + it 'can remove vms from multiple services by href with an appropriate role' do + api_basic_authorize collection_action_identifier(:services, :remove_resource) + request = { + 'action' => 'remove_resource', + 'resources' => [ + { 'href' => services_url(svc.id), 'resource' => { 'href' => vms_url(vm1.id)} }, + { 'href' => services_url(svc1.id), 'resource' => { 'href' => vms_url(vm2.id)} } + ] + } + + run_post(services_url, request) + + expected = { + 'results' => [ + { 'success' => true, 'message' => "Unassigned resource vms id:#{vm1.id} from Service id:#{svc.id} name:'#{svc.name}'" }, + { 'success' => true, 'message' => "Unassigned resource vms id:#{vm2.id} from Service id:#{svc1.id} name:'#{svc1.name}'" } + ] + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + expect(svc.reload.service_resources).to eq([]) + expect(svc1.reload.service_resources).to eq([]) + end + + it 'requires a service id to be specified' do + api_basic_authorize collection_action_identifier(:services, :remove_resource) + request = { + 'action' => 'remove_resource', + 'resources' => [ + { 'href' => services_url, 'resource' => { 'href' => vms_url(vm1.id)} } + ] + } + + run_post(services_url, request) + + expected = { + 'results' => [ + { 'success' => false, 'message' => 'Must specify a resource to remove_resource from' } + ] + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + end + + it 'requires that a resource be specified' do + api_basic_authorize collection_action_identifier(:services, :remove_resource) + request = { + 'action' => 'remove_resource', + 'resources' => [ + { 'href' => services_url(svc.id), 'resource' => {} } + ] + } + + run_post(services_url, request) + + expected = { + 'results' => [ + { 'success' => false, 'message' => 'Must specify a resource reference' } + ] + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + end + + it 'cannot remove a vm from a service without an appropriate role' do + api_basic_authorize + + run_post(services_url(svc.id), 'action' => 'remove_resource') + + expect(response).to have_http_status(:forbidden) + end + + it 'can remove a vm from a service by href with an appropriate role' do + api_basic_authorize collection_action_identifier(:services, :remove_resource) + request = { + 'action' => 'remove_resource', + 'resource' => { 'resource' => {'href' => vms_url(vm1.id)} } + } + + run_post(services_url(svc.id), request) + + expected = { + 'success' => true, + 'message' => "Unassigned resource vms id:#{vm1.id} from Service id:#{svc.id} name:'#{svc.name}'" + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + expect(svc.reload.service_resources).to eq([]) + end + end end