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

Deny request if no conversion host is configured #455

Merged
merged 1 commit into from
Nov 5, 2018
Merged

Deny request if no conversion host is configured #455

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
4 changes: 2 additions & 2 deletions ...ageIQ/Transformation/StateMachines/TransformationPlanRequestApproval.class/__class__.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ object:
condition:
on_entry: validate_request
on_exit:
on_error: pending_request
on_error: deny_request
max_retries: '100'
max_time:
- field:
Expand All @@ -68,6 +68,6 @@ object:
condition:
on_entry: approve_request
on_exit:
on_error: pending_request
on_error: deny_request
max_retries: '100'
max_time:
5 changes: 5 additions & 0 deletions ...rmation/StateMachines/TransformationPlanRequestApproval.class/__methods__/deny_request.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
request = $evm.root['miq_request']
message = $evm.object['reason']
$evm.log('info', "Request denied because of #{message}")
request.message = message
request.deny('admin', msg = message)
0 ...al.class/__methods__/pending_request.yaml → ...roval.class/__methods__/deny_request.yaml
View file
Open in desktop
File renamed without changes.
10 changes: 0 additions & 10 deletions ...tion/StateMachines/TransformationPlanRequestApproval.class/__methods__/pending_request.rb
View file
Open in desktop

This file was deleted.

14 changes: 9 additions & 5 deletions ...ion/StateMachines/TransformationPlanRequestApproval.class/__methods__/validate_request.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#
# Description: Placeholder for service request validation
#
prov = $evm.root['miq_request']
prov.source_vms.each { |vm| prov.approve_vm(vm) if prov.validate_vm(vm) }
request = $evm.root['miq_request']

unless request.validate_conversion_hosts
$evm.object['reason'] = 'No conversion host configured'
request.message = 'No conversion host configured'
exit MIQ_ABORT
Copy link
Member

Choose a reason for hiding this comment

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

@tinaafitz @mkanoor Should this remain exit or is it better to use raise?

Copy link
Contributor

Choose a reason for hiding this comment

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

@gmcculloug It doesn't hurt to have the exit there, we solved the spec problem with the approach listed here

manageiq-content/spec/content/automate/ManageIQ/System/CommonMethods/Utils.class/__methods__/log_object_spec.rb

Line 81 in 166a3df

end.to raise_error(SystemExit) do |error|

Which allows us to test different exit codes. So if there are specs for this change it should follow a similar pattern if the method is being tested directly outside of the DRb and Automate Engine framework.

end

request.source_vms.each { |vm| request.approve_vm(vm) if request.validate_vm(vm) }
Copy link
Member

Choose a reason for hiding this comment

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

This should eventually move to the backend, but not in this PR.