From b287b5ac3ac7638c6d3901b289dbb2b0dfcc9dee Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Thu, 9 May 2019 08:19:54 -0400 Subject: [PATCH 1/4] Explicitly search authentications for v2v. --- app/models/conversion_host.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 4712310939d..869dd0faa8d 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -248,6 +248,7 @@ def resource_supports_conversion_host # def find_credentials(auth_type = 'v2v') authentication = authentication_type(auth_type) + authentication ||= authentications.detect { |a| a.authtype == auth_type } if authentication.blank? res = resource.respond_to?(:authentication_type) ? resource : resource.ext_management_system From 1ad0c51335d83ce998600eb65adf163f89e27609 Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Thu, 9 May 2019 12:03:58 -0400 Subject: [PATCH 2/4] Add explicit tests for the find_credentials method, plus add a helper factory. --- spec/factories/authentication.rb | 4 +++ spec/models/conversion_host_spec.rb | 48 +++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/spec/factories/authentication.rb b/spec/factories/authentication.rb index ac2b5f1b79e..db6a181d164 100644 --- a/spec/factories/authentication.rb +++ b/spec/factories/authentication.rb @@ -35,6 +35,10 @@ status { "SomeMockedStatus" } end + factory :authentication_v2v, :parent => :authentication_ssh_keypair do + authtype { "v2v" } + end + factory :authentication_redhat_metric, :parent => :authentication do authtype { "metrics" } end diff --git a/spec/models/conversion_host_spec.rb b/spec/models/conversion_host_spec.rb index b543ba0d091..ef3669d6cc8 100644 --- a/spec/models/conversion_host_spec.rb +++ b/spec/models/conversion_host_spec.rb @@ -351,17 +351,53 @@ context "authentication associations" do let(:vm) { FactoryBot.create(:vm_openstack) } + let(:ems) { FactoryBot.create(:ems_redhat, :zone => FactoryBot.create(:zone), :api_version => '4.2.4') } + let(:host) { FactoryBot.create(:host_redhat, :ext_management_system => ems) } + let(:conversion_host_vm) { FactoryBot.create(:conversion_host, :resource => vm) } - let(:auth_authkey) { FactoryBot.create(:authentication_ssh_keypair, :resource => conversion_host_vm) } + let(:conversion_host_host) { FactoryBot.create(:conversion_host, :resource => host) } + + let(:auth_keypair) { FactoryBot.create(:authentication_ssh_keypair, :resource => conversion_host_vm) } + let(:auth_v2v) { FactoryBot.create(:authentication_v2v, :resource => conversion_host_host) } - it "finds associated authentications" do - expect(conversion_host_vm.authentications).to contain_exactly(auth_authkey) + it "finds associated ssh_keypair authentications" do + expect(conversion_host_vm.authentications).to contain_exactly(auth_keypair) + end + + it "finds associated v2v authentications" do + expect(conversion_host_host.authentications).to contain_exactly(auth_v2v) end it "allows a resource to add an authentication" do - auth_authkey2 = FactoryBot.create(:authentication_ssh_keypair) - conversion_host_vm.authentications << auth_authkey2 - expect(conversion_host_vm.authentications).to contain_exactly(auth_authkey, auth_authkey2) + auth_keypair2 = FactoryBot.create(:authentication_ssh_keypair) + conversion_host_vm.authentications << auth_keypair2 + expect(conversion_host_vm.authentications).to contain_exactly(auth_keypair, auth_keypair2) + end + end + + context "find_credentials" do + let(:auth_v2v) { FactoryBot.create(:authentication_v2v, :resource => conversion_host_vm) } + let(:ems_redhat) { FactoryBot.create(:ems_redhat, :zone => FactoryBot.create(:zone), :api_version => '4.2.4') } + let(:ems_openstack) { FactoryBot.create(:ems_openstack, :zone => FactoryBot.create(:zone)) } + let(:auth_default) { FactoryBot.create(:authentication) } + let(:auth_v2v) { FactoryBot.create(:authentication_v2v) } + + let(:host) { FactoryBot.create(:host_redhat, :ext_management_system => ems_redhat) } + let(:vm) { FactoryBot.create(:vm_openstack, :ext_management_system => ems_openstack) } + + let(:conversion_host_vm) { FactoryBot.create(:conversion_host, :resource => vm) } + let(:conversion_host_host) { FactoryBot.create(:conversion_host, :resource => host) } + + it "finds the v2v credentials as expected when associated directly with the conversion host" do + conversion_host_vm.authentications << auth_v2v + expect(conversion_host_vm.send(:find_credentials)).to eq(auth_v2v) + end + + it "finds the credentials associated with the resource if credentials cannot be found for the conversion host" do + vm.ext_management_system.authentications << auth_default + host.authentications << auth_default + expect(conversion_host_vm.send(:find_credentials)).to eq(auth_default) + expect(conversion_host_host.send(:find_credentials)).to eq(auth_default) end end From d3cd9a8787506f5040beb85228525bbf6118c10d Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Thu, 9 May 2019 12:10:06 -0400 Subject: [PATCH 3/4] Just use detect in find_credentials. --- app/models/conversion_host.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 869dd0faa8d..b8a0cedc328 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -247,8 +247,7 @@ def resource_supports_conversion_host # with the resource using the 'ssh_keypair' auth type, and finally 'default'. # def find_credentials(auth_type = 'v2v') - authentication = authentication_type(auth_type) - authentication ||= authentications.detect { |a| a.authtype == auth_type } + authentication = authentications.detect { |a| a.authtype == auth_type } if authentication.blank? res = resource.respond_to?(:authentication_type) ? resource : resource.ext_management_system From ea45831ca71f9053a6dbe9c39762d8795e1fc686 Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Fri, 10 May 2019 08:44:13 -0400 Subject: [PATCH 4/4] Set the default value for auth_type to 'v2v' for the ansible_playbook method. --- app/models/conversion_host.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index b8a0cedc328..9da57ae7fcc 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -307,7 +307,7 @@ def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm # +extra_vars+ option should be a hash of key/value pairs which, if present, # will be passed to the '-e' flag. # - def ansible_playbook(playbook, extra_vars = {}, miq_task_id = nil, auth_type = nil) + def ansible_playbook(playbook, extra_vars = {}, miq_task_id = nil, auth_type = 'v2v') task = MiqTask.find(miq_task_id) if miq_task_id.present? host = hostname || ipaddress