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.

Sign up for GitHub

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

Jump to bottom

Add policy checking for request_host_scan. #14427

Merged
merged 1 commit into from
Mar 30, 2017
Merged

Add policy checking for request_host_scan. #14427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 7 additions & 12 deletions app/models/host.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,22 +1346,17 @@ def first_cat_entry(name)
Classification.first_cat_entry(name, self)
end

# TODO: Rename this to scan_queue and rename scan_from_queue to scan to match
# standard from other places.
def scan(userid = "system", _options = {})
def scan(userid = "system", options = {})
log_target = "#{self.class.name} name: [#{name}], id: [#{id}]"

task = MiqTask.create(:name => "SmartState Analysis for '#{name}' ", :userid => userid)

_log.info("Requesting scan of #{log_target}")
begin
MiqEvent.raise_evm_job_event(self, :type => "scan", :prefix => "request")
rescue => err
_log.warn("Error raising request scan event for #{log_target}: #{err.message}")
return
end
check_policy_prevent(:request_host_scan, :scan_queue, userid, options)
Copy link
Member

Choose a reason for hiding this comment

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

@lfu We already have a scan_from_queue method on the Host model. Can the logic here be reworked so the check_policy_prevent calls that method when the scan is not being stopped by policy? Otherwise we are calling the scan_queue method just to put the scan on the queue.

Copy link
Member Author

Choose a reason for hiding this comment

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

@gmcculloug Should not Host Scan go through the queue waiting for its turn?

Copy link
Member

Choose a reason for hiding this comment

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

The host scan does need to be queued and we already do that with the existing scan_from_queue method.

My question was asking if we could get away without having to call the new scan_queue which then queues the actual scan. Might be too much effort to do this using check_policy_prevent but it seemed worth looking into.

Copy link
Member Author

Choose a reason for hiding this comment

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

@gmcculloug scan_from_queue does not put the host scan on to the queue. It actually does the scan.

Copy link
Member

Choose a reason for hiding this comment

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

@lfu and I discussed my question above and agreed to leave the code as-is for now.

end

def scan_queue(userid = 'system', _options = {})
log_target = "#{self.class.name} name: [#{name}], id: [#{id}]"
_log.info("Queuing scan of #{log_target}")

task = MiqTask.create(:name => "SmartState Analysis for '#{name}' ", :userid => userid)
timeout = ::Settings.host_scan.queue_timeout.to_i_with_method
cb = {:class_name => task.class.name, :instance_id => task.id, :method_name => :queue_callback_on_exceptions, :args => ['Finished']}
MiqQueue.put(
Expand Down
30 changes: 30 additions & 0 deletions spec/models/host_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,4 +622,34 @@ def assert_remote_credentials_validated
expect(Host.non_clustered).to eq([host])
end
end

describe "#scan" do
before do
EvmSpecHelper.create_guid_miq_server_zone
@host = FactoryGirl.create(:host_vmware)
FactoryGirl.create(:miq_event_definition, :name => :request_host_scan)
# admin user is needed to process Events
User.super_admin || FactoryGirl.create(:user_with_group, :userid => "admin")
end

it "policy passes" do
expect_any_instance_of(ManageIQ::Providers::Vmware::InfraManager::Host).to receive(:scan_queue)

allow(MiqAeEngine).to receive_messages(:deliver => ['ok', 'sucess', MiqAeEngine::MiqAeWorkspaceRuntime.new])
@host.scan
status, message, result = MiqQueue.first.deliver
MiqQueue.first.delivered(status, message, result)
end

it "policy prevented" do
expect_any_instance_of(ManageIQ::Providers::Vmware::InfraManager::Host).to_not receive(:scan_queue)

event = {:attributes => {"full_data" => {:policy => {:prevented => true}}}}
allow_any_instance_of(MiqAeEngine::MiqAeWorkspaceRuntime).to receive(:get_obj_from_path).with("/").and_return(:event_stream => event)
allow(MiqAeEngine).to receive_messages(:deliver => ['ok', 'sucess', MiqAeEngine::MiqAeWorkspaceRuntime.new])
@host.scan
status, message, _result = MiqQueue.first.deliver
MiqQueue.first.delivered(status, message, MiqAeEngine::MiqAeWorkspaceRuntime.new)
end
end
end