Skip to content

Commit

Permalink
Prevent unsupported classes from being added
Browse files Browse the repository at this point in the history
  • Loading branch information
d-m-u committed Jun 12, 2019
1 parent 36304a8 commit dd80db2
Show file tree
Hide file tree
Showing 15 changed files with 71 additions and 51 deletions.
2 changes: 2 additions & 0 deletions app/models/ems_refresh/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module Manager
module ClassMethods
unless respond_to?(:ems_type)
def ems_type
ActiveSupport::Deprecation.warn("ems_type is required.", caller[1..-1])
#{}"vmwarews"
end
end
end
Expand Down
10 changes: 9 additions & 1 deletion app/models/ext_management_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ def self.supported_for_create?
!reflections.include?("parent_manager")
end

def self.ems_type_supported_for_create?
ExtManagementSystem.supported_types_and_descriptions_hash.key?(ems_type)
end

belongs_to :provider
has_many :child_managers, :class_name => 'ExtManagementSystem', :foreign_key => 'parent_ems_id'

Expand Down Expand Up @@ -101,8 +105,8 @@ def self.supported_for_create?
validates :name, :presence => true, :uniqueness => {:scope => [:tenant_id]}
validates :hostname, :presence => true, :if => :hostname_required?
validate :hostname_uniqueness_valid?, :hostname_format_valid?, :if => :hostname_required?

validate :validate_ems_enabled_when_zone_changed?, :validate_zone_not_maintenance_when_ems_enabled?
validate :validate_ems_type, :on => :create

scope :with_eligible_manager_types, ->(eligible_types) { where(:type => Array(eligible_types).collect(&:to_s)) }

Expand Down Expand Up @@ -828,6 +832,10 @@ def allow_targeted_refresh?

private

def validate_ems_type
errors.add :base, "emstype #{self.class.ems_type} is unsupported" unless self.class.ems_type_supported_for_create?
end

def disable!
_log.info("Disabling EMS [#{name}] id [#{id}].")
update!(:enabled => false)
Expand Down
24 changes: 12 additions & 12 deletions spec/lib/miq_preloader_spec.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
describe MiqPreloader do
describe ".preload" do
it "preloads once from an object" do
ems = FactoryBot.create(:ems_infra)
ems = FactoryBot.create(:ems_vmware)
expect(ems.vms).not_to be_loaded
expect { preload(ems, :vms) }.to match_query_limit_of(1)
expect(ems.vms).to be_loaded
expect { preload(ems, :vms) }.to match_query_limit_of(0)
end

it "preloads from an array" do
emses = FactoryBot.create_list(:ems_infra, 2)
emses = FactoryBot.create_list(:ems_vmware, 2)
expect { preload(emses, :vms) }.to match_query_limit_of(1)
expect(emses[0].vms).to be_loaded
end

it "preloads from an association" do
ems = FactoryBot.create(:ems_infra)
ems = FactoryBot.create(:ems_vmware)
FactoryBot.create_list(:vm, 2, :ext_management_system => ems)

emses = ExtManagementSystem.all
Expand All @@ -29,7 +29,7 @@ def preload(*args)

describe ".preload_and_map" do
it "preloads from an object" do
ems = FactoryBot.create(:ems_infra)
ems = FactoryBot.create(:ems_vmware)
FactoryBot.create_list(:vm, 2, :ext_management_system => ems)

vms = nil
Expand All @@ -38,7 +38,7 @@ def preload(*args)
end

it "preloads from an association" do
ems = FactoryBot.create(:ems_infra)
ems = FactoryBot.create(:ems_vmware)
FactoryBot.create_list(:vm, 2, :ext_management_system => ems)

emses = ExtManagementSystem.all
Expand All @@ -52,7 +52,7 @@ def preload_and_map(*args)

describe ".preload_and_scope" do
it "preloads (object).has_many" do
ems = FactoryBot.create(:ems_infra)
ems = FactoryBot.create(:ems_vmware)
FactoryBot.create_list(:vm, 2, :ext_management_system => ems)

vms = nil
Expand All @@ -61,23 +61,23 @@ def preload_and_map(*args)
end

it "preloads (object.all).has_many" do
FactoryBot.create_list(:vm, 2, :ext_management_system => FactoryBot.create(:ems_infra))
FactoryBot.create(:template, :ext_management_system => FactoryBot.create(:ems_infra))
FactoryBot.create_list(:vm, 2, :ext_management_system => FactoryBot.create(:ems_vmware))
FactoryBot.create(:template, :ext_management_system => FactoryBot.create(:ems_vmware))

vms = nil
expect { vms = preload_and_scope(ExtManagementSystem.all, :vms_and_templates) }.to match_query_limit_of(0)
expect { expect(vms.count).to eq(3) }.to match_query_limit_of(1)
end

it "respects scopes (object.all).has_many {with scope}" do
FactoryBot.create_list(:vm, 2, :ext_management_system => FactoryBot.create(:ems_infra))
FactoryBot.create(:template, :ext_management_system => FactoryBot.create(:ems_infra))
FactoryBot.create_list(:vm, 2, :ext_management_system => FactoryBot.create(:ems_vmware))
FactoryBot.create(:template, :ext_management_system => FactoryBot.create(:ems_vmware))

expect { expect(preload_and_scope(ExtManagementSystem.all, :vms).count).to eq(2) }.to match_query_limit_of(1)
end

it "preloads (object.all).has_many.belongs_to" do
ems = FactoryBot.create(:ems_infra)
ems = FactoryBot.create(:ems_vmware)
FactoryBot.create_list(:vm, 2,
:ext_management_system => ems,
:host => FactoryBot.create(:host, :ext_management_system => ems))
Expand All @@ -92,7 +92,7 @@ def preload_and_map(*args)
end

it "preloads (object.all).belongs_to.has_many" do
ems = FactoryBot.create(:ems_infra)
ems = FactoryBot.create(:ems_vmware)
host = FactoryBot.create(:host, :ext_management_system => ems)
FactoryBot.create_list(:vm, 2,
:ext_management_system => ems,
Expand Down
2 changes: 1 addition & 1 deletion spec/models/conversion_host/configurations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
end

context "queuing configuration requests" do
let(:ext_management_system) { FactoryBot.create(:ext_management_system) }
let(:ext_management_system) { FactoryBot.create(:ems_vmware) }
let(:vm) { FactoryBot.create(:vm_openstack, :ext_management_system => ext_management_system) }
let(:expected_task_action) { "Configuring a conversion_host: operation=#{op} resource=(name: #{vm.name} type: #{vm.class.name} id: #{vm.id})" }

Expand Down
34 changes: 22 additions & 12 deletions spec/models/ext_management_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,16 @@
expect(ems.total_storages).to eq 2
end

context "supported_for_create?" do
it "base class is unsupported" do
expect { FactoryBot.create(:ext_management_system) }.to raise_error(ActiveRecord::RecordInvalid)
end

it "supported class is supported" do
expect(FactoryBot.create(:ems_vmware).class.supported_for_create?).to eq(true)
end
end

context "#hostname / #hostname=" do
it "will delegate to the default endpoint" do
ems = FactoryBot.build(:ems_vmware, :hostname => "example.org")
Expand Down Expand Up @@ -547,15 +557,15 @@

it 'is allowed when enabled' do
zone = FactoryGirl.create(:zone)
ems = FactoryGirl.create(:ext_management_system, :zone => Zone.default_zone)
ems = FactoryGirl.create(:ems_vmware, :zone => Zone.default_zone)

ems.zone = zone
expect(ems.save).to eq(true)
end

it 'is denied when disabled' do
zone = FactoryGirl.create(:zone)
ems = FactoryGirl.create(:ext_management_system, :zone => Zone.default_zone, :enabled => false)
ems = FactoryGirl.create(:ems_vmware, :zone => Zone.default_zone, :enabled => false)

ems.zone = zone
expect(ems.save).to eq(false)
Expand All @@ -565,7 +575,7 @@
it 'to maintenance is not possible when provider enabled' do
zone_visible = FactoryGirl.create(:zone)

ems = FactoryGirl.create(:ext_management_system, :zone => zone_visible, :enabled => true)
ems = FactoryGirl.create(:ems_vmware, :zone => zone_visible, :enabled => true)

ems.zone = Zone.maintenance_zone
expect(ems.save).to eq(false)
Expand All @@ -575,7 +585,7 @@

context "destroy" do
it "destroys an ems with no active workers" do
ems = FactoryBot.create(:ext_management_system)
ems = FactoryBot.create(:ems_vmware)
ems.destroy
expect(ExtManagementSystem.count).to eq(0)
end
Expand All @@ -590,8 +600,8 @@
end

context ".destroy_queue" do
let(:ems) { FactoryBot.create(:ext_management_system, :zone => zone) }
let(:ems2) { FactoryBot.create(:ext_management_system, :zone => zone) }
let(:ems) { FactoryBot.create(:ems_vmware, :zone => zone) }
let(:ems2) { FactoryBot.create(:ems_vmware, :zone => zone) }
let(:server) { EvmSpecHelper.local_miq_server }
let(:zone) { server.zone }

Expand All @@ -604,7 +614,7 @@
end

context "#destroy_queue" do
let(:ems) { FactoryBot.create(:ext_management_system, :zone => zone) }
let(:ems) { FactoryBot.create(:ems_vmware, :zone => zone) }
let(:server) { EvmSpecHelper.local_miq_server }
let(:zone) { server.zone }

Expand Down Expand Up @@ -651,15 +661,15 @@ def deliver_queue_message(queue_message = MiqQueue.order(:id).first)

context "virtual column :supports_block_storage" do
it "returns true if block storage is supported" do
ems = FactoryBot.create(:ext_management_system)
ems = FactoryBot.create(:ems_vmware)
allow(ems).to receive(:supports_block_storage).and_return(true)
expect(ems.supports_block_storage).to eq(true)
end
end

context "virtual column :supports_cloud_object_store_container_create" do
it "returns true if cloud_object_store_container_create is supported" do
ems = FactoryBot.create(:ext_management_system)
ems = FactoryBot.create(:ems_vmware)
allow(ems).to receive(:supports_cloud_object_store_container_create).and_return(true)
expect(ems.supports_cloud_object_store_container_create).to eq(true)
end
Expand All @@ -677,9 +687,9 @@ def deliver_queue_message(queue_message = MiqQueue.order(:id).first)
describe ".inventory_status" do
it "works with infra providers" do
ems = FactoryBot.create(:ems_infra)
host = FactoryBot.create(:host, :ext_management_system => ems)
FactoryBot.create(:vm_infra, :ext_management_system => ems, :host => host)
FactoryBot.create(:vm_infra, :ext_management_system => ems, :host => host)
host = FactoryBot.create(:host, :ems_vmware => ems)
FactoryBot.create(:vm_infra, :ems_vmware => ems, :host => host)
FactoryBot.create(:vm_infra, :ems_vmware => ems, :host => host)

result = ExtManagementSystem.inventory_status
expect(result.size).to eq(2)
Expand Down
8 changes: 4 additions & 4 deletions spec/models/host_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@

before do
EvmSpecHelper.create_guid_miq_server_zone
@ems = FactoryBot.create(:ext_management_system, :tenant => FactoryBot.create(:tenant))
@ems = FactoryBot.create(:ems_vmware, :tenant => FactoryBot.create(:tenant))
@host = FactoryBot.create(:host, :ems_id => @ems.id)
end

Expand Down Expand Up @@ -328,7 +328,7 @@ def assert_remote_credentials_validated
context "#tenant_identity" do
let(:admin) { FactoryBot.create(:user_with_group, :userid => "admin") }
let(:tenant) { FactoryBot.create(:tenant) }
let(:ems) { FactoryBot.create(:ext_management_system, :tenant => tenant) }
let(:ems) { FactoryBot.create(:ems_vmware, :tenant => tenant) }
before { admin }

subject { @host.tenant_identity }
Expand All @@ -351,7 +351,7 @@ def assert_remote_credentials_validated
end

describe "#disconnect_ems" do
let(:ems) { FactoryBot.build(:ext_management_system) }
let(:ems) { FactoryBot.build(:ems_vmware) }
let(:host) do
FactoryBot.build(:host,
:ext_management_system => ems,
Expand All @@ -364,7 +364,7 @@ def assert_remote_credentials_validated
end

it "doesnt clear the wrong ems" do
host.disconnect_ems(FactoryBot.build(:ext_management_system))
host.disconnect_ems(FactoryBot.build(:ems_vmware))
expect(host.ext_management_system).not_to be_nil
expect(host.ems_cluster).not_to be_nil
end
Expand Down
6 changes: 3 additions & 3 deletions spec/models/manageiq/providers/base_manager/refresher_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
describe ManageIQ::Providers::BaseManager::Refresher do
context "#initialize" do
let(:ems1) { FactoryBot.create(:ext_management_system) }
let(:ems2) { FactoryBot.create(:ext_management_system) }
let(:ems1) { FactoryBot.create(:ems_vmware) }
let(:ems2) { FactoryBot.create(:ems_vmware) }
let(:vm1) { FactoryBot.create(:vm, :ext_management_system => ems1) }
let(:vm2) { FactoryBot.create(:vm, :ext_management_system => ems2) }

Expand All @@ -13,7 +13,7 @@

context "#refresh" do
context "#preprocess_targets" do
let(:ems) { FactoryBot.create(:ext_management_system) }
let(:ems) { FactoryBot.create(:ems_vmware) }
let(:vm) { FactoryBot.create(:vm, :ext_management_system => ems) }
let(:lots_of_vms) do
num_targets = Settings.ems_refresh.full_refresh_threshold + 1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
describe ManageIQ::Providers::CloudManager::Provision do
context "Cloning" do
let(:provider) { FactoryBot.create(:ems_cloud) }
let(:provider) { FactoryBot.create(:ems_vmware_cloud) }
let(:template) { FactoryBot.create(:template_cloud, :ext_management_system => provider) }
let(:vm) { FactoryBot.create(:vm_cloud, :ext_management_system => provider, :ems_ref => "vm_1") }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
include Spec::Support::WorkflowHelper

let(:admin) { FactoryBot.create(:user_with_group) }
let(:ems) { FactoryBot.create(:ems_cloud) }
let(:ems) { FactoryBot.create(:ems_vmware_cloud) }
let(:network_manager) { FactoryBot.create(:ems_network, :parent_ems_id => ems.id) }
let(:template) { FactoryBot.create(:miq_template, :name => "template", :ext_management_system => ems) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
describe ManageIQ::Providers::Inventory::Persister::Builder do
before :each do
@zone = FactoryBot.create(:zone)
@ems = FactoryBot.create(:ems_cloud,
@ems = FactoryBot.create(:ems_vmware_cloud,
:zone => @zone,
:network_manager => FactoryBot.create(:ems_network, :zone => @zone))
:network_manager => FactoryBot.create(:ems_amazon_network, :zone => @zone))
@persister = create_persister
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
before do
@miq_server = EvmSpecHelper.local_miq_server
@zone = @miq_server.zone
ems = FactoryBot.create(:ext_management_system, :zone => @zone)
ems = FactoryBot.create(:ems_vmware, :zone => @zone)
@stack = FactoryBot.create(:orchestration_stack, :ext_management_system => ems)
end

Expand Down
6 changes: 3 additions & 3 deletions spec/models/service_template_orchestration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
let(:ra2) { FactoryBot.create(:resource_action, :action => 'Retirement') }
let(:service_dialog) { FactoryBot.create(:dialog) }
let(:template) { FactoryBot.create(:orchestration_template) }
let(:manager) { FactoryBot.create(:ext_management_system) }
let(:manager) { FactoryBot.create(:ems_vmware) }
let(:catalog_item_options) do
{
:name => 'Orchestration Template',
Expand Down Expand Up @@ -168,7 +168,7 @@

describe '#update_catalog_item' do
let(:new_template) { FactoryBot.create(:orchestration_template) }
let(:new_manager) { FactoryBot.create(:ext_management_system) }
let(:new_manager) { FactoryBot.create(:ems_vmware) }
let(:updated_catalog_item_options) do
{
:name => 'Updated Orchestration Template',
Expand Down Expand Up @@ -231,7 +231,7 @@
describe '#config_info' do
it 'returns the correct format' do
template = FactoryBot.create(:orchestration_template)
manager = FactoryBot.create(:ext_management_system)
manager = FactoryBot.create(:ems_vmware)
service_template = FactoryBot.create(:service_template_orchestration,
:orchestration_template => template,
:orchestration_manager => manager)
Expand Down
Loading

0 comments on commit dd80db2

Please sign in to comment.