From 8284f6d6bd217f5cdfddbc309c2e7e2131d06398 Mon Sep 17 00:00:00 2001 From: Jillian Tullo Date: Tue, 20 Feb 2018 13:57:00 -0500 Subject: [PATCH 1/2] Set user when queueing VM actions When VM actions are queued, the user should be set so that it is available as the requester in automate. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1546375 --- app/controllers/api/vms_controller.rb | 28 +++++++++++++++++------ spec/requests/vms_spec.rb | 32 ++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/vms_controller.rb b/app/controllers/api/vms_controller.rb index 4c615d9728..e7b201504f 100644 --- a/app/controllers/api/vms_controller.rb +++ b/app/controllers/api/vms_controller.rb @@ -12,6 +12,7 @@ class VmsController < BaseController VALID_EDIT_ATTRS = %w(description child_resources parent_resource).freeze RELATIONSHIP_COLLECTIONS = %w(vms templates).freeze + DEFAULT_ROLE = 'ems_operations'.freeze def start_resource(type, id = nil, _data = nil) raise BadRequestError, "Must specify an id for starting a #{type} resource" unless id @@ -322,7 +323,7 @@ def validate_vm_for_remote_console(vm, protocol = nil) def start_vm(vm) desc = "#{vm_ident(vm)} starting" - task_id = queue_object_action(vm, desc, :method_name => "start", :role => "ems_operations") + task_id = queue_object_action(vm, desc, queue_options("start")) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -330,7 +331,7 @@ def start_vm(vm) def stop_vm(vm) desc = "#{vm_ident(vm)} stopping" - task_id = queue_object_action(vm, desc, :method_name => "stop", :role => "ems_operations") + task_id = queue_object_action(vm, desc, queue_options("stop")) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -338,7 +339,7 @@ def stop_vm(vm) def suspend_vm(vm) desc = "#{vm_ident(vm)} suspending" - task_id = queue_object_action(vm, desc, :method_name => "suspend", :role => "ems_operations") + task_id = queue_object_action(vm, desc, queue_options("suspend")) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -346,7 +347,7 @@ def suspend_vm(vm) def pause_vm(vm) desc = "#{vm_ident(vm)} pausing" - task_id = queue_object_action(vm, desc, :method_name => "pause", :role => "ems_operations") + task_id = queue_object_action(vm, desc, queue_options("pause")) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -354,7 +355,7 @@ def pause_vm(vm) def shelve_vm(vm) desc = "#{vm_ident(vm)} shelving" - task_id = queue_object_action(vm, desc, :method_name => "shelve", :role => "ems_operations") + task_id = queue_object_action(vm, desc, queue_options("shelve")) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -362,7 +363,7 @@ def shelve_vm(vm) def shelve_offload_vm(vm) desc = "#{vm_ident(vm)} shelve-offloading" - task_id = queue_object_action(vm, desc, :method_name => "shelve_offload", :role => "ems_operations") + task_id = queue_object_action(vm, desc, queue_options("shelve_offload")) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -370,7 +371,7 @@ def shelve_offload_vm(vm) def destroy_vm(vm) desc = "#{vm_ident(vm)} deleting" - task_id = queue_object_action(vm, desc, :method_name => "destroy") + task_id = queue_object_action(vm, desc, queue_options("destroy").except(:role)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -477,5 +478,18 @@ def request_console_vm(vm, protocol) rescue => err action_result(false, err.to_s) end + + def queue_options(method) + current_user = User.current_user + { + :method_name => method, + :role => DEFAULT_ROLE, + :user => { + :user_id => current_user.id, + :group_id => current_user.current_group.id, + :tenant_id => current_user.current_tenant.id + } + } + end end end diff --git a/spec/requests/vms_spec.rb b/spec/requests/vms_spec.rb index 4c5e96f787..50a6e4c665 100644 --- a/spec/requests/vms_spec.rb +++ b/spec/requests/vms_spec.rb @@ -292,7 +292,10 @@ def update_raw_power_state(state, *vms) expect(MiqQueue.where(:class_name => vm.class.name, :instance_id => vm.id, :method_name => "start", - :zone => zone.name).count).to eq(1) + :zone => zone.name, + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "starts multiple vms" do @@ -338,6 +341,10 @@ def update_raw_power_state(state, *vms) post(vm_url, :params => gen_request(:stop)) expect_single_action_result(:success => true, :message => "stopping", :href => api_vm_url(nil, vm), :task => true) + expect(MiqQueue.where(:method_name => "stop", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "stops multiple vms" do @@ -391,6 +398,10 @@ def update_raw_power_state(state, *vms) post(vm_url, :params => gen_request(:suspend)) expect_single_action_result(:success => true, :message => "suspending", :href => api_vm_url(nil, vm), :task => true) + expect(MiqQueue.where(:method_name => "suspend", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "suspends multiple vms" do @@ -444,6 +455,13 @@ def update_raw_power_state(state, *vms) post(vm_url, :params => gen_request(:pause)) expect_single_action_result(:success => true, :message => "pausing", :href => api_vm_url(nil, vm), :task => true) + expect(MiqQueue.where(:class_name => vm.class.name, + :instance_id => vm.id, + :method_name => "pause", + :zone => zone.name, + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "pauses multiple vms" do @@ -517,6 +535,10 @@ def update_raw_power_state(state, *vms) post(vm_openstack_url, :params => gen_request(:shelve)) expect_single_action_result(:success => true, :message => "shelving", :href => api_vm_url(nil, vm_openstack), :task => true) + expect(MiqQueue.where(:method_name => "shelve", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "shelve for a VMWare vm is not supported" do @@ -620,6 +642,10 @@ def update_raw_power_state(state, *vms) expect_single_action_result(:success => true, :message => "shelve-offloading", :href => api_vm_url(nil, vm_openstack)) + expect(MiqQueue.where(:method_name => "shelve_offload", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "shelve_offload for a VMWare vm is not supported" do @@ -677,6 +703,10 @@ def update_raw_power_state(state, *vms) post(vm_url, :params => gen_request(:delete)) expect_single_action_result(:success => true, :message => "deleting", :href => api_vm_url(nil, vm), :task => true) + expect(MiqQueue.where(:method_name => "destroy", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "deletes a vm via a resource DELETE" do From 937ae6ab86a24bdc1939b1b89aeb778541ba3e96 Mon Sep 17 00:00:00 2001 From: Jillian Tullo Date: Wed, 21 Feb 2018 09:35:02 -0500 Subject: [PATCH 2/2] Add user to options when queueing instance actions --- app/controllers/api/base_controller/action.rb | 12 +++++++ app/controllers/api/instances_controller.rb | 18 ++++++----- app/controllers/api/vms_controller.rb | 27 ++++------------ spec/requests/instances_spec.rb | 32 +++++++++++++++++++ 4 files changed, 61 insertions(+), 28 deletions(-) diff --git a/app/controllers/api/base_controller/action.rb b/app/controllers/api/base_controller/action.rb index 4065608d65..17de7e8f6d 100644 --- a/app/controllers/api/base_controller/action.rb +++ b/app/controllers/api/base_controller/action.rb @@ -32,6 +32,18 @@ def queue_object_action(object, summary, options) MiqTask.generic_action_with_callback(task_options, queue_options) end + + def queue_options(method, role = nil) + { + :method_name => method, + :role => role, + :user => { + :user_id => current_user.id, + :group_id => current_user.current_group.id, + :tenant_id => current_user.current_tenant.id + } + } + end end end end diff --git a/app/controllers/api/instances_controller.rb b/app/controllers/api/instances_controller.rb index 47fd332623..d2f94f3df9 100644 --- a/app/controllers/api/instances_controller.rb +++ b/app/controllers/api/instances_controller.rb @@ -5,6 +5,8 @@ class InstancesController < BaseController include Subcollections::SecurityGroups include Subcollections::Snapshots + DEFAULT_ROLE = 'ems_operations'.freeze + def terminate_resource(type, id = nil, _data = nil) raise BadRequestError, "Must specify an id for terminating a #{type} resource" unless id @@ -114,7 +116,7 @@ def instance_ident(instance) def terminate_instance(instance) desc = "#{instance_ident(instance)} terminating" - task_id = queue_object_action(instance, desc, :method_name => "vm_destroy", :role => "ems_operations") + task_id = queue_object_action(instance, desc, queue_options("vm_destroy", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -122,7 +124,7 @@ def terminate_instance(instance) def stop_instance(instance) desc = "#{instance_ident(instance)} stopping" - task_id = queue_object_action(instance, desc, :method_name => "stop", :role => "ems_operations") + task_id = queue_object_action(instance, desc, queue_options("stop", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -130,7 +132,7 @@ def stop_instance(instance) def start_instance(instance) desc = "#{instance_ident(instance)} starting" - task_id = queue_object_action(instance, desc, :method_name => "start", :role => "ems_operations") + task_id = queue_object_action(instance, desc, queue_options("start", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -138,7 +140,7 @@ def start_instance(instance) def pause_instance(instance) desc = "#{instance_ident(instance)} pausing" - task_id = queue_object_action(instance, desc, :method_name => "pause", :role => "ems_operations") + task_id = queue_object_action(instance, desc, queue_options("pause", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -155,7 +157,7 @@ def validate_instance_for_action(instance, action) def suspend_instance(instance) desc = "#{instance_ident(instance)} suspending" - task_id = queue_object_action(instance, desc, :method_name => "suspend", :role => "ems_operations") + task_id = queue_object_action(instance, desc, queue_options("suspend", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -163,7 +165,7 @@ def suspend_instance(instance) def shelve_instance(instance) desc = "#{instance_ident(instance)} shelving" - task_id = queue_object_action(instance, desc, :method_name => "shelve", :role => "ems_operations") + task_id = queue_object_action(instance, desc, queue_options("shelve", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -171,7 +173,7 @@ def shelve_instance(instance) def reboot_guest_instance(instance) desc = "#{instance_ident(instance)} rebooting" - task_id = queue_object_action(instance, desc, :method_name => "reboot_guest", :role => "ems_operations") + task_id = queue_object_action(instance, desc, queue_options("reboot_guest", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -179,7 +181,7 @@ def reboot_guest_instance(instance) def reset_instance(instance) desc = "#{instance_ident(instance)} resetting" - task_id = queue_object_action(instance, desc, :method_name => "reset", :role => "ems_operations") + task_id = queue_object_action(instance, desc, queue_options("reset", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) diff --git a/app/controllers/api/vms_controller.rb b/app/controllers/api/vms_controller.rb index e7b201504f..1c5991cbaf 100644 --- a/app/controllers/api/vms_controller.rb +++ b/app/controllers/api/vms_controller.rb @@ -323,7 +323,7 @@ def validate_vm_for_remote_console(vm, protocol = nil) def start_vm(vm) desc = "#{vm_ident(vm)} starting" - task_id = queue_object_action(vm, desc, queue_options("start")) + task_id = queue_object_action(vm, desc, queue_options("start", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -331,7 +331,7 @@ def start_vm(vm) def stop_vm(vm) desc = "#{vm_ident(vm)} stopping" - task_id = queue_object_action(vm, desc, queue_options("stop")) + task_id = queue_object_action(vm, desc, queue_options("stop", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -339,7 +339,7 @@ def stop_vm(vm) def suspend_vm(vm) desc = "#{vm_ident(vm)} suspending" - task_id = queue_object_action(vm, desc, queue_options("suspend")) + task_id = queue_object_action(vm, desc, queue_options("suspend", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -347,7 +347,7 @@ def suspend_vm(vm) def pause_vm(vm) desc = "#{vm_ident(vm)} pausing" - task_id = queue_object_action(vm, desc, queue_options("pause")) + task_id = queue_object_action(vm, desc, queue_options("pause", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -355,7 +355,7 @@ def pause_vm(vm) def shelve_vm(vm) desc = "#{vm_ident(vm)} shelving" - task_id = queue_object_action(vm, desc, queue_options("shelve")) + task_id = queue_object_action(vm, desc, queue_options("shelve", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -363,7 +363,7 @@ def shelve_vm(vm) def shelve_offload_vm(vm) desc = "#{vm_ident(vm)} shelve-offloading" - task_id = queue_object_action(vm, desc, queue_options("shelve_offload")) + task_id = queue_object_action(vm, desc, queue_options("shelve_offload", DEFAULT_ROLE)) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -371,7 +371,7 @@ def shelve_offload_vm(vm) def destroy_vm(vm) desc = "#{vm_ident(vm)} deleting" - task_id = queue_object_action(vm, desc, queue_options("destroy").except(:role)) + task_id = queue_object_action(vm, desc, queue_options("destroy")) action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) @@ -478,18 +478,5 @@ def request_console_vm(vm, protocol) rescue => err action_result(false, err.to_s) end - - def queue_options(method) - current_user = User.current_user - { - :method_name => method, - :role => DEFAULT_ROLE, - :user => { - :user_id => current_user.id, - :group_id => current_user.current_group.id, - :tenant_id => current_user.current_tenant.id - } - } - end end end diff --git a/spec/requests/instances_spec.rb b/spec/requests/instances_spec.rb index 1e574567aa..ab4a0eff8e 100644 --- a/spec/requests/instances_spec.rb +++ b/spec/requests/instances_spec.rb @@ -61,6 +61,10 @@ def update_raw_power_state(state, *instances) :message => /#{instance.id}.* terminating/i, :href => api_instance_url(nil, instance) ) + expect(MiqQueue.where(:method_name => "vm_destroy", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "terminates multiple valid Instances" do @@ -119,6 +123,10 @@ def update_raw_power_state(state, *instances) post(instance_url, :params => gen_request(:stop)) expect_single_action_result(:success => true, :message => "stopping", :href => api_instance_url(nil, instance), :task => true) + expect(MiqQueue.where(:method_name => "stop", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "stops multiple valid instances" do @@ -163,6 +171,10 @@ def update_raw_power_state(state, *instances) post(instance_url, :params => gen_request(:start)) expect_single_action_result(:success => true, :message => "starting", :href => api_instance_url(nil, instance), :task => true) + expect(MiqQueue.where(:method_name => "start", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "starts multiple instances" do @@ -217,6 +229,10 @@ def update_raw_power_state(state, *instances) post(instance_url, :params => gen_request(:pause)) expect_single_action_result(:success => true, :message => "pausing", :href => api_instance_url(nil, instance), :task => true) + expect(MiqQueue.where(:method_name => "pause", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "pauses multiple instances" do @@ -270,6 +286,10 @@ def update_raw_power_state(state, *instances) post(instance_url, :params => gen_request(:suspend)) expect_single_action_result(:success => true, :message => "suspending", :href => api_instance_url(nil, instance), :task => true) + expect(MiqQueue.where(:method_name => "suspend", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "suspends multiple instances" do @@ -324,6 +344,10 @@ def update_raw_power_state(state, *instances) post(instance_url, :params => gen_request(:shelve)) expect_single_action_result(:success => true, :message => 'shelving', :href => api_instance_url(nil, instance)) + expect(MiqQueue.where(:method_name => "shelve", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "cannot shelve a shelved instance" do @@ -392,6 +416,10 @@ def update_raw_power_state(state, *instances) post(instance_url, :params => gen_request(:reboot_guest)) expect_single_action_result(:success => true, :message => "rebooting", :href => api_instance_url(nil, instance), :task => true) + expect(MiqQueue.where(:method_name => "reboot_guest", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "reboots multiple valid instances" do @@ -436,6 +464,10 @@ def update_raw_power_state(state, *instances) post(instance_url, :params => gen_request(:reset)) expect_single_action_result(:success => true, :message => "resetting", :href => api_instance_url(nil, instance), :task => true) + expect(MiqQueue.where(:method_name => "reset", + :user_id => @user.id, + :group_id => @user.current_group.id, + :tenant_id => @user.current_tenant.id).count).to eq(1) end it "resets multiple valid instances" do