From 47a9e32983c1aebade42d9cfd31ce9eaf364c92e Mon Sep 17 00:00:00 2001 From: Aparna Karve Date: Mon, 15 May 2017 15:37:31 -0700 Subject: [PATCH 1/9] Add support for CloudVolume Delete --- config/api.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/config/api.yml b/config/api.yml index 595863cd348..69e439b2156 100644 --- a/config/api.yml +++ b/config/api.yml @@ -468,7 +468,7 @@ :identifier: cloud_volume :options: - :collection - :verbs: *gp + :verbs: *gpppd :klass: CloudVolume :collection_actions: :get: @@ -477,10 +477,17 @@ :post: - :name: query :identifier: cloud_volume_show_list + - :name: delete + :identifier: cloud_volume_delete :resource_actions: :get: - :name: read :identifier: cloud_volume + :post: + - :name: query + :identifier: cloud_volume_show_list + - :name: delete + :identifier: cloud_volume_delete :clusters: :description: Clusters :identifier: ems_cluster From 25fe7551b8ba01c4cfc9e4b5a3b8ece5ec9d1d61 Mon Sep 17 00:00:00 2001 From: Aparna Karve Date: Mon, 15 May 2017 19:24:57 -0700 Subject: [PATCH 2/9] Add delete_resource method that does queued delete of cloud volumes --- app/controllers/api/cloud_volumes_controller.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/controllers/api/cloud_volumes_controller.rb b/app/controllers/api/cloud_volumes_controller.rb index c434b97b25e..6b54db6b42c 100644 --- a/app/controllers/api/cloud_volumes_controller.rb +++ b/app/controllers/api/cloud_volumes_controller.rb @@ -1,4 +1,11 @@ module Api class CloudVolumesController < BaseController + def delete_resource(type, id, _data) + cloud_volume = resource_search(id, type, collection_class(:cloud_volumes)) + task_id = cloud_volume.delete_volume_queue(User.current_user) + action_result(true, "Deleting Cloud Volume #{cloud_volume.name}", :task_id => task_id) + rescue => err + action_result(false, err.to_s) + end end end From 813a8468671e1e1dddb45960e87a8653e6abb894 Mon Sep 17 00:00:00 2001 From: Aparna Karve Date: Mon, 15 May 2017 19:28:07 -0700 Subject: [PATCH 3/9] Spec for multiple cloud volume deletes through POST --- spec/requests/api/cloud_volumes_spec.rb | 29 +++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/spec/requests/api/cloud_volumes_spec.rb b/spec/requests/api/cloud_volumes_spec.rb index 13415461d26..7cbae94f5aa 100644 --- a/spec/requests/api/cloud_volumes_spec.rb +++ b/spec/requests/api/cloud_volumes_spec.rb @@ -40,4 +40,33 @@ "id" => cloud_volume.id ) end + + it 'can delete cloud volumes through POST' do + zone = FactoryGirl.create(:zone, :name => "api_zone") + aws = FactoryGirl.create(:ems_amazon, :zone => zone) + + cloud_volume1 = FactoryGirl.create(:cloud_volume, :ext_management_system => aws, :name => "CloudVolume1") + cloud_volume2 = FactoryGirl.create(:cloud_volume, :ext_management_system => aws, :name => "CloudVolume2") + + api_basic_authorize collection_action_identifier(:cloud_volumes, :delete, :post) + + expected = { + 'results' => [ + a_hash_including( + 'success' => true, + 'message' => a_string_including('Deleting Cloud Volume CloudVolume1'), + 'task_id' => a_kind_of(Numeric) + ), + a_hash_including( + 'success' => true, + 'message' => a_string_including('Deleting Cloud Volume CloudVolume2'), + 'task_id' => a_kind_of(Numeric) + ) + ] + } + run_post(cloud_volumes_url, :action => 'delete', :resources => [{ 'id' => cloud_volume1.id }, { 'id' => cloud_volume2.id }]) + + expect(response.parsed_body).to include(expected) + expect(response).to have_http_status(:ok) + end end From b65d5fb39c9dc61b69dacee77903a1c12ae130d3 Mon Sep 17 00:00:00 2001 From: Aparna Karve Date: Tue, 16 May 2017 11:16:27 -0700 Subject: [PATCH 4/9] Add RBAC spec that checks for `have_http_status(:forbidden)` --- spec/requests/api/cloud_volumes_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/requests/api/cloud_volumes_spec.rb b/spec/requests/api/cloud_volumes_spec.rb index 7cbae94f5aa..176ed8b5413 100644 --- a/spec/requests/api/cloud_volumes_spec.rb +++ b/spec/requests/api/cloud_volumes_spec.rb @@ -41,6 +41,14 @@ ) end + it "rejects delete request without appropriate role" do + api_basic_authorize + + run_post(cloud_volumes_url, :action => 'delete') + + expect(response).to have_http_status(:forbidden) + end + it 'can delete cloud volumes through POST' do zone = FactoryGirl.create(:zone, :name => "api_zone") aws = FactoryGirl.create(:ems_amazon, :zone => zone) From c7551bb82307d90bec60020600ae4c8dbeba97e2 Mon Sep 17 00:00:00 2001 From: Aparna Karve Date: Tue, 16 May 2017 11:21:44 -0700 Subject: [PATCH 5/9] PR feedback - removed :post query --- config/api.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/api.yml b/config/api.yml index 69e439b2156..ae48192e2c9 100644 --- a/config/api.yml +++ b/config/api.yml @@ -484,8 +484,6 @@ - :name: read :identifier: cloud_volume :post: - - :name: query - :identifier: cloud_volume_show_list - :name: delete :identifier: cloud_volume_delete :clusters: From c8541938ffad3e5047950236d66eb15f0322d252 Mon Sep 17 00:00:00 2001 From: Aparna Karve Date: Tue, 16 May 2017 14:35:09 -0700 Subject: [PATCH 6/9] Spec for single Cloud Volume delete --- spec/requests/api/cloud_volumes_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/spec/requests/api/cloud_volumes_spec.rb b/spec/requests/api/cloud_volumes_spec.rb index 176ed8b5413..d679fda0eee 100644 --- a/spec/requests/api/cloud_volumes_spec.rb +++ b/spec/requests/api/cloud_volumes_spec.rb @@ -49,6 +49,26 @@ expect(response).to have_http_status(:forbidden) end + it "can delete a single cloud volume" do + zone = FactoryGirl.create(:zone, :name => "api_zone") + aws = FactoryGirl.create(:ems_amazon, :zone => zone) + + cloud_volume1 = FactoryGirl.create(:cloud_volume, :ext_management_system => aws, :name => "CloudVolume1") + + api_basic_authorize action_identifier(:cloud_volumes, :delete, :resource_actions, :post) + + run_post(cloud_volumes_url(cloud_volume1.id), :action => "delete") + + expected = { + 'message' => 'Deleting Cloud Volume CloudVolume1', + 'success' => true, + 'task_id' => a_kind_of(Numeric) + } + + expect(response.parsed_body).to include(expected) + expect(response).to have_http_status(:ok) + end + it 'can delete cloud volumes through POST' do zone = FactoryGirl.create(:zone, :name => "api_zone") aws = FactoryGirl.create(:ems_amazon, :zone => zone) From 2006899e63e87545761aec14cfba2ec3d3ddea00 Mon Sep 17 00:00:00 2001 From: Aparna Karve Date: Wed, 17 May 2017 12:04:53 -0700 Subject: [PATCH 7/9] changed verbs to `gpd` and added specs using `run_delete` --- .../api/cloud_volumes_controller.rb | 2 +- config/api.yml | 8 +++- spec/requests/api/cloud_volumes_spec.rb | 46 +++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/cloud_volumes_controller.rb b/app/controllers/api/cloud_volumes_controller.rb index 6b54db6b42c..e5a6eb7bee5 100644 --- a/app/controllers/api/cloud_volumes_controller.rb +++ b/app/controllers/api/cloud_volumes_controller.rb @@ -1,6 +1,6 @@ module Api class CloudVolumesController < BaseController - def delete_resource(type, id, _data) + def delete_resource(type, id, _data = {}) cloud_volume = resource_search(id, type, collection_class(:cloud_volumes)) task_id = cloud_volume.delete_volume_queue(User.current_user) action_result(true, "Deleting Cloud Volume #{cloud_volume.name}", :task_id => task_id) diff --git a/config/api.yml b/config/api.yml index ae48192e2c9..5d76f2fbb58 100644 --- a/config/api.yml +++ b/config/api.yml @@ -468,7 +468,7 @@ :identifier: cloud_volume :options: - :collection - :verbs: *gpppd + :verbs: *gpd :klass: CloudVolume :collection_actions: :get: @@ -479,6 +479,9 @@ :identifier: cloud_volume_show_list - :name: delete :identifier: cloud_volume_delete + :delete: + - :name: delete + :identifier: cloud_volume_delete :resource_actions: :get: - :name: read @@ -486,6 +489,9 @@ :post: - :name: delete :identifier: cloud_volume_delete + :delete: + - :name: delete + :identifier: cloud_volume_delete :clusters: :description: Clusters :identifier: ems_cluster diff --git a/spec/requests/api/cloud_volumes_spec.rb b/spec/requests/api/cloud_volumes_spec.rb index d679fda0eee..07e6146c3d2 100644 --- a/spec/requests/api/cloud_volumes_spec.rb +++ b/spec/requests/api/cloud_volumes_spec.rb @@ -69,6 +69,52 @@ expect(response).to have_http_status(:ok) end + it "can delete a cloud volume with DELETE as a resource action" do + zone = FactoryGirl.create(:zone, :name => "api_zone") + aws = FactoryGirl.create(:ems_amazon, :zone => zone) + + cloud_volume1 = FactoryGirl.create(:cloud_volume, :ext_management_system => aws, :name => "CloudVolume1") + + api_basic_authorize action_identifier(:cloud_volumes, :delete, :resource_actions, :delete) + + run_delete cloud_volumes_url(cloud_volume1.id) + + expect(response).to have_http_status(:no_content) + end + + it "can delete a cloud volume with DELETE as a collection action" do + zone = FactoryGirl.create(:zone, :name => "api_zone") + aws = FactoryGirl.create(:ems_amazon, :zone => zone) + + cloud_volume1 = FactoryGirl.create(:cloud_volume, :ext_management_system => aws, :name => "CloudVolume1") + + api_basic_authorize action_identifier(:cloud_volumes, :delete, :collection_actions, :delete) + + run_delete(cloud_volumes_url, :resources => [{ 'id' => cloud_volume1.id }]) + + expect(response).to have_http_status(:no_content) + end + + it "rejects delete request with DELETE as a resource action without appropriate role" do + cloud_volume = FactoryGirl.create(:cloud_volume) + + api_basic_authorize + + run_delete cloud_volumes_url(cloud_volume.id) + + expect(response).to have_http_status(:forbidden) + end + + it "rejects delete request with DELETE as a collection action without appropriate role" do + cloud_volume = FactoryGirl.create(:cloud_volume) + + api_basic_authorize + + run_delete(cloud_volumes_url, :resources => [{ 'id' => cloud_volume.id }]) + + expect(response).to have_http_status(:forbidden) + end + it 'can delete cloud volumes through POST' do zone = FactoryGirl.create(:zone, :name => "api_zone") aws = FactoryGirl.create(:ems_amazon, :zone => zone) From 4c1bbf854251462c9495df98639684d09427f7ac Mon Sep 17 00:00:00 2001 From: Aparna Karve Date: Wed, 17 May 2017 15:55:20 -0700 Subject: [PATCH 8/9] Prefer a_collection_containing_exactly() over [] --- spec/requests/api/cloud_volumes_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/api/cloud_volumes_spec.rb b/spec/requests/api/cloud_volumes_spec.rb index 07e6146c3d2..db3397a6a21 100644 --- a/spec/requests/api/cloud_volumes_spec.rb +++ b/spec/requests/api/cloud_volumes_spec.rb @@ -125,7 +125,7 @@ api_basic_authorize collection_action_identifier(:cloud_volumes, :delete, :post) expected = { - 'results' => [ + 'results' => a_collection_containing_exactly( a_hash_including( 'success' => true, 'message' => a_string_including('Deleting Cloud Volume CloudVolume1'), @@ -136,7 +136,7 @@ 'message' => a_string_including('Deleting Cloud Volume CloudVolume2'), 'task_id' => a_kind_of(Numeric) ) - ] + ) } run_post(cloud_volumes_url, :action => 'delete', :resources => [{ 'id' => cloud_volume1.id }, { 'id' => cloud_volume2.id }]) From 6ca849f1e1ccf698dd4bff40395a926e6835129f Mon Sep 17 00:00:00 2001 From: Aparna Karve Date: Fri, 19 May 2017 10:14:44 -0700 Subject: [PATCH 9/9] Removed DELETE method on the collection and the related specs --- config/api.yml | 3 --- spec/requests/api/cloud_volumes_spec.rb | 23 ----------------------- 2 files changed, 26 deletions(-) diff --git a/config/api.yml b/config/api.yml index 5d76f2fbb58..82a798f9d80 100644 --- a/config/api.yml +++ b/config/api.yml @@ -479,9 +479,6 @@ :identifier: cloud_volume_show_list - :name: delete :identifier: cloud_volume_delete - :delete: - - :name: delete - :identifier: cloud_volume_delete :resource_actions: :get: - :name: read diff --git a/spec/requests/api/cloud_volumes_spec.rb b/spec/requests/api/cloud_volumes_spec.rb index db3397a6a21..1e8dc950554 100644 --- a/spec/requests/api/cloud_volumes_spec.rb +++ b/spec/requests/api/cloud_volumes_spec.rb @@ -82,19 +82,6 @@ expect(response).to have_http_status(:no_content) end - it "can delete a cloud volume with DELETE as a collection action" do - zone = FactoryGirl.create(:zone, :name => "api_zone") - aws = FactoryGirl.create(:ems_amazon, :zone => zone) - - cloud_volume1 = FactoryGirl.create(:cloud_volume, :ext_management_system => aws, :name => "CloudVolume1") - - api_basic_authorize action_identifier(:cloud_volumes, :delete, :collection_actions, :delete) - - run_delete(cloud_volumes_url, :resources => [{ 'id' => cloud_volume1.id }]) - - expect(response).to have_http_status(:no_content) - end - it "rejects delete request with DELETE as a resource action without appropriate role" do cloud_volume = FactoryGirl.create(:cloud_volume) @@ -105,16 +92,6 @@ expect(response).to have_http_status(:forbidden) end - it "rejects delete request with DELETE as a collection action without appropriate role" do - cloud_volume = FactoryGirl.create(:cloud_volume) - - api_basic_authorize - - run_delete(cloud_volumes_url, :resources => [{ 'id' => cloud_volume.id }]) - - expect(response).to have_http_status(:forbidden) - end - it 'can delete cloud volumes through POST' do zone = FactoryGirl.create(:zone, :name => "api_zone") aws = FactoryGirl.create(:ems_amazon, :zone => zone)