Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise an error if a region is not configured for central admin #12264

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 12 additions & 5 deletions app/models/mixins/inter_region_api_method_relay.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 32 additions & 18 deletions spec/models/mixins/inter_region_api_method_relay_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you even need this line? Doesn't 168 and 169 take care of that naturally?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so too, but the object returned from FactoryGirl.create and the one returned by MiqRegion.find_by are different instances so I can't use the region defined by the let for expects without doing this.

If that's not what's going on, I agree, I would much rather not have this here 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh when I remove this, specs fail because unless you have a region file it'll be region 0 (invalid), makes sense I suppose. ¯_(ツ)_/¯

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

Expand Down