From 505dcd2995d5a73cba7b409b04f87936ec9835c6 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Thu, 27 Oct 2016 17:59:42 -0400 Subject: [PATCH 1/2] Raise an error if a region is not configured for central admin Now when someone tries to do something with an object that belongs to a remote region and hasn't configured the authentication key yet for that region they will get a clear error message. https://bugzilla.redhat.com/show_bug.cgi?id=1389536 --- .../mixins/inter_region_api_method_relay.rb | 17 +++++-- .../inter_region_api_method_relay_spec.rb | 50 ++++++++++++------- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/app/models/mixins/inter_region_api_method_relay.rb b/app/models/mixins/inter_region_api_method_relay.rb index 9920813acf7..3c7c65ee0d2 100644 --- a/app/models/mixins/inter_region_api_method_relay.rb +++ b/app/models/mixins/inter_region_api_method_relay.rb @@ -48,18 +48,25 @@ def api_relay_class_method(method, action = method) end end - def self.api_client_connection_for_region(region) - url = MiqRegion.find_by(:region => region).remote_ws_url + def self.api_client_connection_for_region(region_number) + region = MiqRegion.find_by(:region => region_number) + + unless region.auth_key_configured? + _log.error("Region #{region_number} is not configured for central administration") + raise "Region #{region_number} is not configured for central administration" + end + + url = region.remote_ws_url if url.nil? - _log.error("The remote region [#{region}] does not have a web service address.") - raise "Failed to establish API connection to region #{region}" + _log.error("The remote region [#{region_number}] does not have a web service address.") + raise "Failed to establish API connection to region #{region_number}" end require 'manageiq-api-client' ManageIQ::API::Client.new( :url => url, - :miqtoken => MiqRegion.api_system_auth_token_for_region(region, User.current_userid), + :miqtoken => region.api_system_auth_token(User.current_userid), :ssl => {:verify => false} ) end diff --git a/spec/models/mixins/inter_region_api_method_relay_spec.rb b/spec/models/mixins/inter_region_api_method_relay_spec.rb index 3a52b003aa6..573c910c01a 100644 --- a/spec/models/mixins/inter_region_api_method_relay_spec.rb +++ b/spec/models/mixins/inter_region_api_method_relay_spec.rb @@ -165,38 +165,52 @@ def expect_api_call(expected_action, expected_args = nil) describe ".api_client_connection_for_region" do let!(:server) { EvmSpecHelper.local_miq_server(:has_active_webservices => true) } + let!(:region) { FactoryGirl.create(:miq_region, :region => region_number) } + let(:region_number) { ApplicationRecord.my_region_number } let(:region_seq_start) { ApplicationRecord.rails_sequence_start } let(:request_user) { "test_user" } let(:api_connection) { double("ManageIQ::API::Client connection") } let(:region_auth_token) { double("MiqRegion API auth token") } before do - FactoryGirl.create(:miq_region, :region => ApplicationRecord.my_region_number) + expect(MiqRegion).to receive(:find_by).with(:region => region_number).and_return(region) end - it "opens an api connection to that address when the server has an ip address" do - require "manageiq-api-client" + context "with authentication configured" do + before do + expect(region).to receive(:auth_key_configured?).and_return true + end + + it "opens an api connection to that address when the server has an ip address" do + require "manageiq-api-client" - server.ipaddress = "192.0.2.1" - server.save! + server.ipaddress = "192.0.2.1" + server.save! - expect(User).to receive(:current_userid).and_return(request_user) - expect(MiqRegion).to receive(:api_system_auth_token_for_region) - .with(ApplicationRecord.my_region_number, request_user).and_return(region_auth_token) + expect(User).to receive(:current_userid).and_return(request_user) + expect(region).to receive(:api_system_auth_token).with(request_user).and_return(region_auth_token) - client_connection_hash = { - :url => "https://#{server.ipaddress}", - :miqtoken => region_auth_token, - :ssl => {:verify => false} - } - expect(ManageIQ::API::Client).to receive(:new).with(client_connection_hash).and_return(api_connection) - described_class.api_client_connection_for_region(ApplicationRecord.my_region_number) + client_connection_hash = { + :url => "https://#{server.ipaddress}", + :miqtoken => region_auth_token, + :ssl => {:verify => false} + } + expect(ManageIQ::API::Client).to receive(:new).with(client_connection_hash).and_return(api_connection) + described_class.api_client_connection_for_region(region_number) + end + + it "raises if the server doesn't have an ip address" do + expect { + described_class.api_client_connection_for_region(region_number) + }.to raise_error("Failed to establish API connection to region #{region_number}") + end end - it "raises if the server doesn't have an ip address" do + it "raises without authentication configured" do + expect(region).to receive(:auth_key_configured?).and_return false expect { - described_class.api_client_connection_for_region(ApplicationRecord.my_region_number) - }.to raise_error(RuntimeError) + described_class.api_client_connection_for_region(region_number) + }.to raise_error("Region #{region_number} is not configured for central administration") end end From d08652db76d7d634b79e96d92a8b2e40eba2f91e Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Thu, 27 Oct 2016 18:01:55 -0400 Subject: [PATCH 2/2] Show the new error message when provisioning a VM from a remote template https://bugzilla.redhat.com/show_bug.cgi?id=1389536 --- .../application_controller/miq_request_methods.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/application_controller/miq_request_methods.rb b/app/controllers/application_controller/miq_request_methods.rb index ad495d5f6cf..f51898424cd 100644 --- a/app/controllers/application_controller/miq_request_methods.rb +++ b/app/controllers/application_controller/miq_request_methods.rb @@ -597,7 +597,15 @@ def prov_req_submit id = session[:edit][:req_id] || "new" return unless load_edit("prov_edit__#{id}", "show_list") @edit[:new][:schedule_time] = @edit[:new][:schedule_time].in_time_zone("Etc/UTC") if @edit[:new][:schedule_time] - if @edit[:wf].make_request(@edit[:req_id], @edit[:new]) + + begin + request = @edit[:wf].make_request(@edit[:req_id], @edit[:new]) + rescue => bang + request = false + add_flash(bang.message, :error) + end + + if request @breadcrumbs.pop if @breadcrumbs typ = @edit[:org_controller] case typ