Merge pull request #16237 from gmcculloug/remove_legacy_provision_naming

Remove legacy naming code from provisioning
(cherry picked from commit 8636cc7)

https://bugzilla.redhat.com/show_bug.cgi?id=1539752

app/models/miq_provision/naming.rb

@@ -1,40 +1,26 @@
 module MiqProvision::Naming
   extend ActiveSupport::Concern
 
-  NAME_VIA_AUTOMATE = true
   NAME_SEQUENCE_REGEX = /\$n\{(\d+)\}/
-  SOURCE_IDENTIFIER = "provisioning"  # a unique name for the source column in custom_attributes table
+  SOURCE_IDENTIFIER = "provisioning".freeze # a unique name for the source column in custom_attributes table
 
   module ClassMethods
     def get_next_vm_name(prov_obj, determine_index = true)
-      unresolved_vm_name = nil
+      prov_obj.save
+      attrs = {'request' => 'UI_PROVISION_INFO', 'message' => 'get_vmname'}
+      MiqAeEngine.set_automation_attributes_from_objects([prov_obj.get_user], attrs)
+      ws = MiqAeEngine.resolve_automation_object("REQUEST", prov_obj.get_user, attrs, :vmdb_object => prov_obj)
 
-      if NAME_VIA_AUTOMATE == true
-        prov_obj.save
-        attrs = {'request' => 'UI_PROVISION_INFO', 'message' => 'get_vmname'}
-        MiqAeEngine.set_automation_attributes_from_objects([prov_obj.get_user], attrs)
-        ws = MiqAeEngine.resolve_automation_object("REQUEST", prov_obj.get_user, attrs, :vmdb_object => prov_obj)
-
-        unresolved_vm_name = ws.root("vmname")
-        prov_obj.reload
-      end
-
-      if unresolved_vm_name.blank?
-        options        = prov_obj.options
-        options[:tags] = prov_obj.get_tags
-
-        load ApplicationRecord::FIXTURE_DIR.join("miq_provision_naming.rb")
-        unresolved_vm_name = MiqProvisionNaming.naming(options)
-      end
+      unresolved_vm_name = ws.root("vmname")
+      prov_obj.reload
 
       # Check if we need to force a unique target name
       if prov_obj.get_option(:miq_force_unique_name) == true && unresolved_vm_name !~ NAME_SEQUENCE_REGEX
         unresolved_vm_name += '$n{4}'
         _log.info "Forced unique provision name to #{unresolved_vm_name} for #{prov_obj.class}:#{prov_obj.id}"
       end
 
-      vm_name = get_vm_full_name(unresolved_vm_name, prov_obj, determine_index)
-      vm_name
+      get_vm_full_name(unresolved_vm_name, prov_obj, determine_index)
     end
 
     def get_vm_full_name(unresolved_vm_name, prov_obj, determine_index)
@@ -52,7 +38,7 @@ def get_vm_full_name(unresolved_vm_name, prov_obj, determine_index)
       index_length = name[:index_length]
       loop do
         next_number = MiqRegion.my_region.next_naming_sequence(unresolved_vm_name, SOURCE_IDENTIFIER)
-        idx_str  = "%0#{index_length}d" % next_number
+        idx_str = "%0#{index_length}d" % next_number
         if idx_str.length > index_length
           index_length += 1
           unresolved_vm_name = "#{name[:prefix]}$n{#{index_length}}#{name[:suffix]}"

config/brakeman.ignore

@@ -40,26 +40,6 @@
       "confidence": "Medium",
       "note": "Temporarily skipped, found in new brakeman version"
     },
-    {
-      "warning_type": "File Access",
-      "warning_code": 16,
-      "fingerprint": "7db29962886ebaade1d869d329da3fb601de293d121ca29c318410122bf1be40",
-      "check_name": "FileAccess",
-      "message": "Model attribute used in file name",
-      "file": "app/models/miq_provision/naming.rb",
-      "line": 26,
-      "link": "http://brakemanscanner.org/docs/warning_types/file_access/",
-      "code": "load(ApplicationRecord::FIXTURE_DIR.join(\"miq_provision_naming.rb\"))",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "MiqProvision::Naming::ClassMethods",
-        "method": "get_next_vm_name"
-      },
-      "user_input": "ApplicationRecord::FIXTURE_DIR.join(\"miq_provision_naming.rb\")",
-      "confidence": "Weak",
-      "note": "Temporarily skipped, found in new brakeman version"
-    },
     {
       "warning_type": "Command Injection",
       "warning_code": 14,

spec/models/miq_provision_spec.rb

@@ -46,6 +46,7 @@
         end
 
         it "should populate description, target_name and target_hostname" do
+          allow(@vm_prov).to receive(:get_next_vm_name).and_return("test_vm")
           @vm_prov.after_request_task_create
           expect(@vm_prov.description).not_to be_nil
           expect(@vm_prov.get_option(:vm_target_name)).not_to be_nil
@@ -54,9 +55,14 @@
 
         it "should create a valid target_name and hostname" do
           MiqRegion.seed
+          ae_workspace = double("ae_workspace")
+          expect(ae_workspace).to receive(:root).and_return(@target_vm_name)
+          expect(MiqAeEngine).to receive(:resolve_automation_object).and_return(ae_workspace).exactly(3).times
+
           @vm_prov.after_request_task_create
           expect(@vm_prov.get_option(:vm_target_name)).to eq(@target_vm_name)
 
+          expect(ae_workspace).to receive(:root).and_return("#{@target_vm_name}$n{3}").twice
           @vm_prov.options[:number_of_vms] = 2
           @vm_prov.after_request_task_create
           name_001 = "#{@target_vm_name}001"
@@ -80,6 +86,10 @@
           end
 
           it "should advance to next range but based on the existing sequence number for the new range" do
+            ae_workspace = double("ae_workspace")
+            expect(ae_workspace).to receive(:root).and_return("#{@target_vm_name}$n{3}").twice
+            expect(MiqAeEngine).to receive(:resolve_automation_object).and_return(ae_workspace).twice
+
             @vm_prov.options[:number_of_vms] = 2
             @vm_prov.after_request_task_create
             expect(@vm_prov.get_option(:vm_target_name)).to eq("#{@target_vm_name}999")  # 3 digits
@@ -94,8 +104,9 @@
           # Hostname lengths by platform:
           #   Linux   : 63
           #   Windows : 15
-          #                                      1         2         3         4         5         6
-          @vm_prov.options[:vm_name] = '123456789012345678901234567890123456789012345678901234567890123456789'
+          #                        1         2         3         4         5         6
+          long_vm_name = '123456789012345678901234567890123456789012345678901234567890123456789'
+          expect(@vm_prov).to receive(:get_next_vm_name).and_return(long_vm_name).twice
           @vm_prov.after_request_task_create
           expect(@vm_prov.get_option(:vm_target_hostname).length).to eq(15)
 

spec/models/miq_request_spec.rb

@@ -282,6 +282,12 @@
 
   context '#post_create_request_tasks' do
     context 'VM provisioning' do
+      before do
+        ae_workspace = double("ae_workspace")
+        allow(ae_workspace).to receive(:root).and_return("test_vm")
+        allow(MiqAeEngine).to receive(:resolve_automation_object).and_return(ae_workspace)
+      end
+
       let(:description) { 'my original information' }
       let(:template)    { FactoryGirl.create(:template_vmware, :ext_management_system => FactoryGirl.create(:ems_vmware_with_authentication)) }
       let(:request)     { FactoryGirl.build(:miq_provision_request, :requester => fred, :description => description, :src_vm_id => template.id).tap(&:valid?) }
@@ -369,6 +375,9 @@
 
     before do
       allow(MiqRegion).to receive(:my_region).and_return(FactoryGirl.create(:miq_region))
+      ae_workspace = double("ae_workspace")
+      allow(ae_workspace).to receive(:root).and_return("test_vm")
+      allow(MiqAeEngine).to receive(:resolve_automation_object).and_return(ae_workspace)
 
       @options = {
         :src_vm_id     => template.id,