Prevent unsupported classes from being added

app/models/ext_management_system.rb

@@ -101,8 +101,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)) }
 
@@ -828,6 +828,10 @@ def allow_targeted_refresh?
 
   private
 
+  def validate_ems_type
+    errors.add(:base, "emstype #{self.class.name} is not supported for create") unless ExtManagementSystem.supported_types_and_descriptions_hash.key?(emstype)
+  end
+
   def disable!
     _log.info("Disabling EMS [#{name}] id [#{id}].")
     update!(:enabled => false)

spec/factories/ext_management_system.rb

@@ -1,5 +1,6 @@
 FactoryBot.define do
-  factory :ext_management_system do
+  factory :ext_management_system,
+          :class   => "ManageIQ::Providers::Vmware::InfraManager" do
     sequence(:name)      { |n| "ems_#{seq_padded_for_sorting(n)}" }
     sequence(:hostname)  { |n| "ems-#{seq_padded_for_sorting(n)}" }
     sequence(:ipaddress) { |n| ip_from_seq(n) }
@@ -56,35 +57,32 @@
   end
 
   # Intermediate classes
-
+  # using leaf classes...
   factory :ems_infra,
           :aliases => ["manageiq/providers/infra_manager"],
-          :class   => "ManageIQ::Providers::InfraManager",
+          :class   => "ManageIQ::Providers::Vmware::InfraManager",
           :parent  => :ext_management_system
 
   factory :ems_physical_infra,
           :aliases => ["manageiq/providers/physical_infra_manager"],
-          :class   => "ManageIQ::Providers::PhysicalInfraManager",
+          :class   => "ManageIQ::Providers::Redfish::PhysicalInfraManager",
           :parent  => :ext_management_system
 
-  factory :ems_cloud,
+  factory(:ems_cloud,
           :aliases => ["manageiq/providers/cloud_manager"],
-          :class   => "ManageIQ::Providers::CloudManager",
-          :parent  => :ext_management_system
-
-  factory :ems_datawarehouse,
-          :aliases => ["manageiq/providers/datawarehouse_manager"],
-          :class   => "ManageIQ::Providers::DatawarehouseManager",
-          :parent  => :ext_management_system
+          :class   => "ManageIQ::Providers::Amazon::CloudManager",
+          :parent  => :ext_management_system) do
+    provider_region { "us-east-1" }
+  end
 
   factory :ems_network,
           :aliases => ["manageiq/providers/network_manager"],
-          :class   => "ManageIQ::Providers::NetworkManager",
+          :class   => "ManageIQ::Providers::Openstack::NetworkManager",
           :parent  => :ext_management_system
 
   factory :ems_storage,
           :aliases => ["manageiq/providers/storage_manager"],
-          :class   => "ManageIQ::Providers::StorageManager",
+          :class   => "ManageIQ::Providers::StorageManager::SwiftManager",
           :parent  => :ext_management_system
 
   factory :ems_cinder,
@@ -99,39 +97,34 @@
 
   factory :ems_container,
           :aliases => ["manageiq/providers/container_manager"],
-          :class   => "ManageIQ::Providers::ContainerManager",
-          :parent  => :ext_management_system
-
-  factory :ems_middleware,
-          :aliases => ["manageiq/providers/middleware_manager"],
-          :class   => "ManageIQ::Providers::MiddlewareManager",
+          :class   => "ManageIQ::Providers::Openshift::ContainerManager",
           :parent  => :ext_management_system
 
   factory :configuration_manager,
           :aliases => ["manageiq/providers/configuration_manager"],
-          :class   => "ManageIQ::Providers::ConfigurationManager",
+          :class   => "ManageIQ::Providers::Foreman::ConfigurationManager",
           :parent  => :ext_management_system
 
   # Automation managers
 
   factory :automation_manager,
           :aliases => ["manageiq/providers/automation_manager"],
-          :class   => "ManageIQ::Providers::AutomationManager",
+          :class   => "ManageIQ::Providers::AnsibleTower::AutomationManager",
           :parent  => :ext_management_system
 
   factory :external_automation_manager,
           :aliases => ["manageiq/providers/external_automation_manager"],
-          :class   => "ManageIQ::Providers::ExternalAutomationManager",
+          :class   => "ManageIQ::Providers::AnsibleTower::AutomationManager",
           :parent  => :automation_manager
 
   factory :embedded_automation_manager,
           :aliases => ["manageiq/providers/embedded_automation_manager"],
-          :class   => "ManageIQ::Providers::EmbeddedAutomationManager",
+          :class   => "ManageIQ::Providers::EmbeddedAnsible::AutomationManager",
           :parent  => :automation_manager
 
   factory :provisioning_manager,
           :aliases => ["manageiq/providers/provisioning_manager"],
-          :class   => "ManageIQ::Providers::ProvisioningManager",
+          :class   => "ManageIQ::Providers::Foreman::ProvisioningManager",
           :parent  => :ext_management_system
 
   # Leaf classes for ems_infra
@@ -373,18 +366,4 @@
           :aliases => ["manageiq/providers/foreman/provisioning_manager"],
           :class   => "ManageIQ::Providers::Foreman::ProvisioningManager",
           :parent  => :provisioning_manager
-
-  # Leaf classes for middleware_manager
-
-  factory :ems_hawkular,
-          :aliases => ["manageiq/providers/hawkular/middleware_manager"],
-          :class   => "ManageIQ::Providers::Hawkular::MiddlewareManager",
-          :parent  => :ems_middleware
-
-  # Leaf classes for datawarehouse_manager
-
-  factory :ems_hawkular_datawarehouse,
-          :aliases => ["manageiq/providers/hawkular/datawarehouse_manager"],
-          :class   => "ManageIQ::Providers::Hawkular::DatawarehouseManager",
-          :parent  => :ems_datawarehouse
 end

spec/models/ext_management_system_spec.rb

@@ -94,6 +94,16 @@
     expect(described_class.with_eligible_manager_types(r.class).count).to eq(1)
   end
 
+  it "validates type" do
+    v = FactoryBot.create(:ems_vmware)
+    e = FactoryBot.create(:ext_management_system)
+    s = FactoryBot.create(:ems_storage)
+
+    expect([v.valid?, v.emstype]).to eq([true, 'vmwarews'])
+    expect([e.valid?, e.emstype]).to eq([true, 'vmwarews'])
+    expect([s.valid?, s.emstype]).to eq([true, 'swift'])
+  end
+
   context "#ipaddress / #ipaddress=" do
     it "will delegate to the default endpoint" do
       ems = FactoryBot.build(:ems_vmware, :ipaddress => "1.2.3.4")

spec/models/manageiq/providers/cloud_manager/auth_key_pair_spec.rb

@@ -9,7 +9,6 @@
 
     # TODO(maufart): do we have any special approach to test module methods separately?
     it 'forces implement methods' do
-      expect { subject.class.create_key_pair ems.id, {} }.to raise_error NotImplementedError
       expect { subject.delete_key_pair }.to raise_error NotImplementedError
     end
   end

spec/models/manageiq/providers/cloud_manager/provision_workflow_spec.rb

@@ -3,7 +3,7 @@
 
   let(:admin) { FactoryBot.create(:user_with_group) }
   let(:ems) { FactoryBot.create(:ems_cloud) }
-  let(:network_manager) { FactoryBot.create(:ems_network, :parent_ems_id => ems.id) }
+  let(:network_manager) { ems.network_manager }
   let(:template) { FactoryBot.create(:miq_template, :name => "template", :ext_management_system => ems) }
 
   let(:cloud_init_template) { FactoryBot.create(:customization_template_cloud_init) }
@@ -189,9 +189,7 @@
 
     context "#allowed_cloud_networks" do
       it "without a zone", :skip_before do
-        network_manager
-
-        expect(workflow.allowed_cloud_networks.length).to be_zero
+        expect(workflow.allowed_cloud_networks.length).to eq(1)
       end
 
       it "with a zone" do

spec/models/manageiq/providers/inventory/persister/builder_spec.rb

@@ -3,10 +3,7 @@
 
 describe ManageIQ::Providers::Inventory::Persister::Builder do
   before :each do
-    @zone = FactoryBot.create(:zone)
-    @ems  = FactoryBot.create(:ems_cloud,
-                               :zone            => @zone,
-                               :network_manager => FactoryBot.create(:ems_network, :zone => @zone))
+    @ems = FactoryBot.create(:ems_cloud)
     @persister = create_persister
   end
 

spec/models/manageiq/providers/inventory/persister/finders_spec.rb

@@ -11,12 +11,7 @@
   ######################################################################################################################
   #
   before do
-    @zone = FactoryBot.create(:zone)
-    @ems  = FactoryBot.create(:ems_cloud,
-                               :zone            => @zone,
-                               :network_manager => FactoryBot.create(:ems_network, :zone => @zone))
-
-    allow(@ems.class).to receive(:ems_type).and_return(:mock)
+    @ems = FactoryBot.create(:ems_cloud)
   end
 
   let(:persister) { create_persister }

spec/models/manageiq/providers/inventory/persister/local_db_finders_spec.rb

@@ -14,10 +14,7 @@
   ######################################################################################################################
   #
   before do
-    @zone = FactoryBot.create(:zone)
-    @ems  = FactoryBot.create(:ems_cloud,
-                               :zone            => @zone,
-                               :network_manager => FactoryBot.create(:ems_network, :zone => @zone))
+    @ems = FactoryBot.create(:ems_cloud)
   end
 
   before do

spec/models/manageiq/providers/inventory/persister/serializing_spec.rb

@@ -11,12 +11,8 @@
   ######################################################################################################################
   #
   before do
-    @zone = FactoryBot.create(:zone)
-    @ems  = FactoryBot.create(:ems_cloud,
-                               :zone            => @zone,
-                               :network_manager => FactoryBot.create(:ems_network, :zone => @zone))
+    @ems = FactoryBot.create(:ems_cloud)
 
-    allow(@ems.class).to receive(:ems_type).and_return(:mock)
     allow(Settings.ems_refresh).to receive(:mock).and_return({})
   end
 

spec/models/manageiq/providers/inventory/persister/targeted_refresh_spec_helper.rb

@@ -7,7 +7,7 @@ def create_persister
   end
 
   def expected_ext_management_systems_count
-    2
+    3
   end
 
   def base_inventory_counts

spec/models/retirement_manager_spec.rb

@@ -2,7 +2,7 @@
   describe "#check" do
     it "with retirement date, runs retirement checks" do
       _, _, zone = EvmSpecHelper.local_guid_miq_server_zone
-      ems = FactoryBot.create(:ems_network, :zone => zone)
+      ems = FactoryBot.create(:ems_openstack_with_authentication, :zone => zone)
 
       orchestration_stack = FactoryBot.create(:orchestration_stack, :retires_on => Time.zone.today + 1.day, :ext_management_system => ems)
       FactoryBot.create(:orchestration_stack, :retired => true)

spec/models/vm_or_template_spec.rb

@@ -558,7 +558,7 @@
 
   context "#supports_terminate?" do
     let(:ems_does_vm_destroy) { FactoryBot.create(:ems_vmware) }
-    let(:ems_doesnot_vm_destroy) { FactoryBot.create(:ems_cloud) }
+    let(:ems_doesnot_vm_destroy) { FactoryBot.create(:ems_storage) }
     let(:host) { FactoryBot.create(:host) }
 
     it "returns true for a VM not terminated" do