From 6fcfe7ef90a5404ea25816af4b397f2f8525c214 Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Tue, 26 Sep 2017 22:41:06 +0200 Subject: [PATCH 1/8] add security group subcollection to providers, tenants, and vms --- app/controllers/api/cloud_tenants_controller.rb | 1 + app/controllers/api/providers_controller.rb | 1 + app/controllers/api/subcollections/security_groups.rb | 9 +++++++++ app/controllers/api/vms_controller.rb | 1 + config/api.yml | 4 ++++ 5 files changed, 16 insertions(+) create mode 100644 app/controllers/api/subcollections/security_groups.rb diff --git a/app/controllers/api/cloud_tenants_controller.rb b/app/controllers/api/cloud_tenants_controller.rb index c684e8b341..aa9c718ec7 100644 --- a/app/controllers/api/cloud_tenants_controller.rb +++ b/app/controllers/api/cloud_tenants_controller.rb @@ -1,4 +1,5 @@ module Api class CloudTenantsController < BaseController + include Subcollections::SecurityGroups end end diff --git a/app/controllers/api/providers_controller.rb b/app/controllers/api/providers_controller.rb index b3d9e3a638..4cfdf54553 100644 --- a/app/controllers/api/providers_controller.rb +++ b/app/controllers/api/providers_controller.rb @@ -17,6 +17,7 @@ class ProvidersController < BaseController include Subcollections::CloudTenants include Subcollections::CustomAttributes include Subcollections::LoadBalancers + include Subcollections::SecurityGroups include Subcollections::Vms def create_resource(type, _id, data = {}) diff --git a/app/controllers/api/subcollections/security_groups.rb b/app/controllers/api/subcollections/security_groups.rb new file mode 100644 index 0000000000..15adb1755c --- /dev/null +++ b/app/controllers/api/subcollections/security_groups.rb @@ -0,0 +1,9 @@ +module Api + module Subcollections + module SecurityGroups + def security_groups_query_resource(object) + object.respond_to?(:security_groups) ? object.security_groups : [] + end + end + end +end diff --git a/app/controllers/api/vms_controller.rb b/app/controllers/api/vms_controller.rb index eec9ccab45..913db57192 100644 --- a/app/controllers/api/vms_controller.rb +++ b/app/controllers/api/vms_controller.rb @@ -5,6 +5,7 @@ class VmsController < BaseController include Subcollections::PolicyProfiles include Subcollections::Accounts include Subcollections::CustomAttributes + include Subcollections::SecurityGroups include Subcollections::Software include Subcollections::Snapshots include Subcollections::MetricRollups diff --git a/config/api.yml b/config/api.yml index 18254af80b..207f1f2682 100644 --- a/config/api.yml +++ b/config/api.yml @@ -493,6 +493,8 @@ - :subcollection :verbs: *gp :klass: CloudTenant + :subcollections: + - :security_groups :collection_actions: :get: - :name: read @@ -1607,6 +1609,7 @@ - :cloud_tenants - :custom_attributes - :load_balancers + - :security_groups - :vms :collection_actions: :get: @@ -2650,6 +2653,7 @@ - :policy_profiles - :accounts - :custom_attributes + - :security_groups - :software - :snapshots - :metric_rollups From 7a60ed2ece41c01b5022d1ab2c8abc19307a5b0b Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Tue, 26 Sep 2017 22:53:40 +0200 Subject: [PATCH 2/8] add security group subcollection tests --- spec/requests/providers_spec.rb | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/spec/requests/providers_spec.rb b/spec/requests/providers_spec.rb index 6c2f207794..38f983015d 100644 --- a/spec/requests/providers_spec.rb +++ b/spec/requests/providers_spec.rb @@ -1108,6 +1108,53 @@ def gen_import_request end end + context 'security groups subcollection' do + before do + @provider = FactoryGirl.create(:ems_openstack) + @security_group = FactoryGirl.create(:security_group, :ext_management_system => @provider) + end + + it 'queries all security groups' do + api_basic_authorize subcollection_action_identifier(:providers, :security_groups, :read, :get) + + get(api_provider_security_groups_url(nil, @provider)) + + expected = { + 'resources' => [ + { 'href' => api_provider_security_group_url(nil, @provider, @security_group) } + ] + + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include(expected) + end + + it "will not show a provider's security groups without the appropriate role" do + api_basic_authorize + + get(api_provider_security_groups_url(nil, @provider)) + + expect(response).to have_http_status(:forbidden) + end + + it 'queries a single security group' do + api_basic_authorize action_identifier(:security_groups, :read, :subresource_actions, :get) + + get(api_provider_security_group_url(nil, @provider, @security_group)) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include('id' => @security_group.id.to_s) + end + + it "will not show a provider's security group without the appropriate role" do + api_basic_authorize + + get(api_provider_security_group_url(nil, @provider, @security_group)) + + expect(response).to have_http_status(:forbidden) + end + end + describe 'edit custom_attributes on providers' do context 'provider_class=provider' do let(:generic_provider) { FactoryGirl.create(:provider) } From 293ea1464c903cadfbce371e56bbed19e13eef8b Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Wed, 27 Sep 2017 18:22:24 +0200 Subject: [PATCH 3/8] add test for provider that does not support security groups --- spec/requests/providers_spec.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/spec/requests/providers_spec.rb b/spec/requests/providers_spec.rb index 38f983015d..d81b17cbe2 100644 --- a/spec/requests/providers_spec.rb +++ b/spec/requests/providers_spec.rb @@ -1111,10 +1111,11 @@ def gen_import_request context 'security groups subcollection' do before do @provider = FactoryGirl.create(:ems_openstack) + @infra_provider = FactoryGirl.create(:ems_openstack_infra) @security_group = FactoryGirl.create(:security_group, :ext_management_system => @provider) end - it 'queries all security groups' do + it 'queries all security groups from a provider that responds to security_groups' do api_basic_authorize subcollection_action_identifier(:providers, :security_groups, :read, :get) get(api_provider_security_groups_url(nil, @provider)) @@ -1129,6 +1130,19 @@ def gen_import_request expect(response.parsed_body).to include(expected) end + it 'does not error when querying a provider that does not respond to security_groups' do + api_basic_authorize subcollection_action_identifier(:providers, :security_groups, :read, :get) + + get(api_provider_security_groups_url(nil, @infra_provider)) + + expected = { + 'resources' => [] + + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include(expected) + end + it "will not show a provider's security groups without the appropriate role" do api_basic_authorize From 3b7cefe9f06882965baf1acc645978b60700d12d Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Wed, 27 Sep 2017 19:21:23 +0200 Subject: [PATCH 4/8] corrected spec --- spec/requests/providers_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/providers_spec.rb b/spec/requests/providers_spec.rb index d81b17cbe2..daa12a1f04 100644 --- a/spec/requests/providers_spec.rb +++ b/spec/requests/providers_spec.rb @@ -1110,7 +1110,7 @@ def gen_import_request context 'security groups subcollection' do before do - @provider = FactoryGirl.create(:ems_openstack) + @provider = FactoryGirl.create(:ems_openstack).network_manager @infra_provider = FactoryGirl.create(:ems_openstack_infra) @security_group = FactoryGirl.create(:security_group, :ext_management_system => @provider) end From 331766124ff5f7c6a478d6d3a94854101167f25c Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Wed, 27 Sep 2017 20:11:53 +0200 Subject: [PATCH 5/8] fix spec tests --- config/api.yml | 8 ++++++++ spec/requests/providers_spec.rb | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/config/api.yml b/config/api.yml index 207f1f2682..1933525c9a 100644 --- a/config/api.yml +++ b/config/api.yml @@ -2032,6 +2032,14 @@ :get: - :name: read :identifier: security_group_show + :subcollection_actions: + :get: + - :name: read + :identifier: security_group_show_list + :subresource_actions: + :get: + - :name: read + :identifier: security_group_show :servers: :description: EVM Servers :options: diff --git a/spec/requests/providers_spec.rb b/spec/requests/providers_spec.rb index daa12a1f04..588580dada 100644 --- a/spec/requests/providers_spec.rb +++ b/spec/requests/providers_spec.rb @@ -1130,7 +1130,7 @@ def gen_import_request expect(response.parsed_body).to include(expected) end - it 'does not error when querying a provider that does not respond to security_groups' do + it 'does not error when querying a provider that does not respond to security_groups' do api_basic_authorize subcollection_action_identifier(:providers, :security_groups, :read, :get) get(api_provider_security_groups_url(nil, @infra_provider)) From 9f612b0cc36ae8aba07f1dad93e59f285e1312bb Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Thu, 5 Oct 2017 16:15:10 +0200 Subject: [PATCH 6/8] add additional spec tests for vms and cloud_tenants --- spec/requests/cloud_tenants_spec.rb | 47 +++++++++++++++++++++++++++ spec/requests/vms_spec.rb | 50 +++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/spec/requests/cloud_tenants_spec.rb b/spec/requests/cloud_tenants_spec.rb index b09f31e542..f083a2c43a 100644 --- a/spec/requests/cloud_tenants_spec.rb +++ b/spec/requests/cloud_tenants_spec.rb @@ -46,4 +46,51 @@ expect(response).to have_http_status(:forbidden) end end + + context 'security groups subcollection' do + before do + @cloud_tenant = FactoryGirl.create(:cloud_tenant) + @security_group = FactoryGirl.create(:security_group, :cloud_tenant => @cloud_tenant) + end + + it 'queries all security groups from a cloud tenant' do + api_basic_authorize subcollection_action_identifier(:cloud_tenants, :security_groups, :read, :get) + + get(api_cloud_tenant_security_groups_url(nil, @cloud_tenant)) + + expected = { + 'resources' => [ + { 'href' => api_cloud_tenant_security_group_url(nil, @cloud_tenant, @security_group) } + ] + + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include(expected) + end + + it "will not show a cloud tenant's security groups without the appropriate role" do + api_basic_authorize + + get(api_cloud_tenant_security_groups_url(nil, @cloud_tenant)) + + expect(response).to have_http_status(:forbidden) + end + + it 'queries a single security group' do + api_basic_authorize action_identifier(:security_groups, :read, :subresource_actions, :get) + + get(api_cloud_tenant_security_group_url(nil, @cloud_tenant, @security_group)) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include('id' => @security_group.id.to_s) + end + + it "will not show a cloud tenant's security group without the appropriate role" do + api_basic_authorize + + get(api_cloud_tenant_security_group_url(nil, @cloud_tenant, @security_group)) + + expect(response).to have_http_status(:forbidden) + end + end end diff --git a/spec/requests/vms_spec.rb b/spec/requests/vms_spec.rb index 47a75c7e56..8f3a2e7ee9 100644 --- a/spec/requests/vms_spec.rb +++ b/spec/requests/vms_spec.rb @@ -1642,4 +1642,54 @@ def update_raw_power_state(state, *vms) expect(response).to have_http_status(:forbidden) end end + + context 'security groups subcollection' do + before do + @network_port = FactoryGirl.create(:network_port, :device => vm_openstack) + @security_group = FactoryGirl.create(:security_group, :cloud_tenant => @cloud_tenant) + @network_port_security_group = FactoryGirl.create(:network_port_security_group, + :network_port => @network_port, + :security_group => @security_group) + end + + it 'queries all security groups from a vm' do + api_basic_authorize subcollection_action_identifier(:vms, :security_groups, :read, :get) + + get(api_vm_security_groups_url(nil, vm_openstack)) + + expected = { + 'resources' => [ + { 'href' => api_vm_security_group_url(nil, vm_openstack, @security_group) } + ] + + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include(expected) + end + + it "will not show a vm's security groups without the appropriate role" do + api_basic_authorize + + get(api_vm_security_groups_url(nil, vm_openstack)) + + expect(response).to have_http_status(:forbidden) + end + + it 'queries a single security group' do + api_basic_authorize action_identifier(:security_groups, :read, :subresource_actions, :get) + + get(api_vm_security_group_url(nil, vm_openstack, @security_group)) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include('id' => @security_group.id.to_s) + end + + it "will not show a vm's security group without the appropriate role" do + api_basic_authorize + + get(api_vm_security_group_url(nil, vm_openstack, @security_group)) + + expect(response).to have_http_status(:forbidden) + end + end end From 67e7d14f92d2c1685917b483f3d6f7d53645d065 Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Thu, 5 Oct 2017 20:51:48 +0200 Subject: [PATCH 7/8] style updates --- app/controllers/api/subcollections/security_groups.rb | 2 +- spec/requests/vms_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/subcollections/security_groups.rb b/app/controllers/api/subcollections/security_groups.rb index 15adb1755c..aa2a63b820 100644 --- a/app/controllers/api/subcollections/security_groups.rb +++ b/app/controllers/api/subcollections/security_groups.rb @@ -2,7 +2,7 @@ module Api module Subcollections module SecurityGroups def security_groups_query_resource(object) - object.respond_to?(:security_groups) ? object.security_groups : [] + object.respond_to?(:security_groups) ? object.security_groups : [].freeze end end end diff --git a/spec/requests/vms_spec.rb b/spec/requests/vms_spec.rb index 8f3a2e7ee9..4b46648211 100644 --- a/spec/requests/vms_spec.rb +++ b/spec/requests/vms_spec.rb @@ -1648,7 +1648,7 @@ def update_raw_power_state(state, *vms) @network_port = FactoryGirl.create(:network_port, :device => vm_openstack) @security_group = FactoryGirl.create(:security_group, :cloud_tenant => @cloud_tenant) @network_port_security_group = FactoryGirl.create(:network_port_security_group, - :network_port => @network_port, + :network_port => @network_port, :security_group => @security_group) end From fed2730bde81b8e897524c5a6e906403dc6eb84b Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Thu, 5 Oct 2017 21:27:41 +0200 Subject: [PATCH 8/8] remove string freeze --- app/controllers/api/subcollections/security_groups.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/subcollections/security_groups.rb b/app/controllers/api/subcollections/security_groups.rb index aa2a63b820..15adb1755c 100644 --- a/app/controllers/api/subcollections/security_groups.rb +++ b/app/controllers/api/subcollections/security_groups.rb @@ -2,7 +2,7 @@ module Api module Subcollections module SecurityGroups def security_groups_query_resource(object) - object.respond_to?(:security_groups) ? object.security_groups : [].freeze + object.respond_to?(:security_groups) ? object.security_groups : [] end end end