diff --git a/app/models/miq_provision/naming.rb b/app/models/miq_provision/naming.rb index 96bd1894425..7c06ded370f 100644 --- a/app/models/miq_provision/naming.rb +++ b/app/models/miq_provision/naming.rb @@ -1,31 +1,18 @@ 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 @@ -33,8 +20,7 @@ def get_next_vm_name(prov_obj, determine_index = true) _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]}" diff --git a/config/brakeman.ignore b/config/brakeman.ignore index c1558d0ff96..5b4aa4b62c8 100644 --- a/config/brakeman.ignore +++ b/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, diff --git a/db/fixtures/miq_provision_naming.rb b/db/fixtures/miq_provision_naming.rb deleted file mode 100644 index 53a4daea71a..00000000000 --- a/db/fixtures/miq_provision_naming.rb +++ /dev/null @@ -1,28 +0,0 @@ -module MiqProvisionNaming - def self.naming(options) - vm_name = options[:vm_name].to_s.strip - number_of_vms_being_provisioned = options[:number_of_vms] - number_of_vms_being_provisioned = number_of_vms_being_provisioned.first if number_of_vms_being_provisioned.kind_of?(Array) - tags = options[:tags] - - # Construct VM name here - # Use $n{#} to indicate a sequence number where # is the 0-char padding. - # Example: $n{3} would result in "001" added to the vm name - # Reference tags by the category name - # Example - Add selected environment to vm name: "#{tags['environment']}v#{vm_name}$n{3}" - # Result for dev environment with "miq" entered as the vm name: devmiq001 - - # Sample: - # Create random VM name - # "miq_vm_#{rand(20000)}" - - # Default naming: - # Single VM: Pass name from dialog through without modifying - # Multi-VM : Append 3 digit sequence number to the end of the name from the dialog - if number_of_vms_being_provisioned == 1 - vm_name.to_s - else - "#{vm_name}$n{3}" - end - end -end diff --git a/spec/models/miq_provision_spec.rb b/spec/models/miq_provision_spec.rb index a909e317556..3ac5bde3c0d 100644 --- a/spec/models/miq_provision_spec.rb +++ b/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) diff --git a/spec/models/miq_request_spec.rb b/spec/models/miq_request_spec.rb index 514f16037ee..087afac2526 100644 --- a/spec/models/miq_request_spec.rb +++ b/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,