diff --git a/app/controllers/api/vms_controller.rb b/app/controllers/api/vms_controller.rb index cfa5dd38481..f8f7ed5663a 100644 --- a/app/controllers/api/vms_controller.rb +++ b/app/controllers/api/vms_controller.rb @@ -8,6 +8,9 @@ class VmsController < BaseController include Subcollections::Software include Subcollections::Snapshots + VALID_EDIT_ATTRS = %w(description child_resources parent_resource).freeze + RELATIONSHIP_COLLECTIONS = [:vms, :templates].freeze + def start_resource(type, id = nil, _data = nil) raise BadRequestError, "Must specify an id for starting a #{type} resource" unless id @@ -86,6 +89,18 @@ def shelve_offload_resource(type, id = nil, _data = nil) end end + def edit_resource(type, id, data) + attrs = validate_edit_data(data) + parent, children = build_parent_children(data) + resource_search(id, type, collection_class(type)).tap do |vm| + vm.update_attributes!(attrs) + vm.replace_children(children) + vm.set_parent(parent) + end + rescue => err + raise BadRequestError, "Cannot edit VM - #{err}" + end + def delete_resource(type, id = nil, _data = nil) raise BadRequestError, "Must specify an id for deleting a #{type} resource" unless id @@ -216,7 +231,7 @@ def request_console_resource(type, id = nil, data = nil) # protocol = data["protocol"] || "mks" # However, there are different entitlements for the different protocol as per miq_product_feature, # so we may go for different action, i.e. request_console_vnc - #protocol = "mks" + # protocol = "mks" protocol = data["protocol"] || "vnc" api_action(type, id) do |klass| @@ -231,6 +246,36 @@ def request_console_resource(type, id = nil, data = nil) private + def validate_edit_data(data) + invalid_keys = data.keys - VALID_EDIT_ATTRS - valid_custom_attrs + raise BadRequestError, "Cannot edit values #{invalid_keys.join(', ')}" if invalid_keys.present? + data.except('parent_resource', 'child_resources') + end + + def build_parent_children(data) + children = if data.key?('child_resources') + data['child_resources'].collect do |child| + fetch_relationship(child['href']) + end + end + + parent = if data.key?('parent_resource') + fetch_relationship(data['parent_resource']['href']) + end + + [parent, Array(children)] + end + + def fetch_relationship(href) + collection, id = parse_href(href) + raise "Invalid relationship type #{collection}" unless RELATIONSHIP_COLLECTIONS.include?(collection) + resource_search(id, collection, collection_class(collection)) + end + + def valid_custom_attrs + Vm.virtual_attribute_names.select { |name| name =~ /custom_\d/ } + end + def vm_ident(vm) "VM id:#{vm.id} name:'#{vm.name}'" end diff --git a/config/api.yml b/config/api.yml index 8013c62e512..0946c424028 100644 --- a/config/api.yml +++ b/config/api.yml @@ -2273,7 +2273,6 @@ :disabled: true - :name: edit :identifier: vm_edit - :disabled: true - :name: add_lifecycle_event :identifier: vm_edit - :name: add_event @@ -2321,7 +2320,6 @@ :post: - :name: edit :identifier: vm_edit - :disabled: true - :name: add_lifecycle_event :identifier: vm_edit - :name: add_event diff --git a/spec/requests/api/vms_spec.rb b/spec/requests/api/vms_spec.rb index be0a19a0396..a5c18365605 100644 --- a/spec/requests/api/vms_spec.rb +++ b/spec/requests/api/vms_spec.rb @@ -28,6 +28,92 @@ def update_raw_power_state(state, *vms) vms.each { |vm| vm.update_attributes!(:raw_power_state => state) } end + context 'Vm edit' do + let(:new_vms) { FactoryGirl.create_list(:vm_openstack, 2) } + + before do + vm.set_child(vm_openstack) + vm.set_parent(vm_openstack1) + end + + it 'cannot edit a VM without an appropriate role' do + api_basic_authorize + + run_post(vms_url(vm.id), :action => 'edit') + + expect(response).to have_http_status(:forbidden) + end + + it 'can edit a VM with an appropriate role' do + api_basic_authorize collection_action_identifier(:vms, :edit) + children = new_vms.collect do |vm| + { 'href' => vms_url(vm.id) } + end + + run_post(vms_url(vm.id), :action => 'edit', + :description => 'bar', + :child_resources => children, + :custom_1 => 'foobar', + :custom_9 => 'fizzbuzz', + :parent_resource => { :href => vms_url(vm_openstack2.id) }) + + expected = { + 'description' => 'bar' + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include(expected) + expect(vm.reload.children).to match_array(new_vms) + expect(vm.parent).to eq(vm_openstack2) + expect(vm.custom_1).to eq('foobar') + expect(vm.custom_9).to eq('fizzbuzz') + end + + it 'only allows edit of custom_1, description, parent, and children' do + api_basic_authorize collection_action_identifier(:vms, :edit) + + run_post(vms_url(vm.id), :action => 'edit', :name => 'foo', :autostart => true, :power_state => 'off') + + expected = { + 'error' => a_hash_including( + 'kind' => 'bad_request', + 'message' => 'Cannot edit VM - Cannot edit values name, autostart, power_state' + ) + } + expect(response).to have_http_status(:bad_request) + expect(response.parsed_body).to include(expected) + end + + it 'can edit multiple vms' do + api_basic_authorize collection_action_identifier(:vms, :edit) + + run_post(vms_url, :action => 'edit', :resources => [{ :id => vm.id, :description => 'foo' }, { :id => vm_openstack.id, :description => 'bar'}]) + + expected = { + 'results' => [ + a_hash_including('description' => 'foo'), + a_hash_including('description' => 'bar') + ] + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include(expected) + end + + it 'requires a valid child/parent relationship ' do + api_basic_authorize collection_action_identifier(:vms, :edit) + + run_post(vms_url(vm.id), :action => 'edit', :parent_resource => { :href => users_url(10) }) + + expected = { + 'error' => a_hash_including( + 'kind' => 'bad_request', + 'message' => 'Cannot edit VM - Invalid relationship type users' + ) + } + expect(response).to have_http_status(:bad_request) + expect(response.parsed_body).to include(expected) + end + end + context "Vm accounts subcollection" do let(:acct1) { FactoryGirl.create(:account, :vm_or_template_id => vm.id, :name => "John") } let(:acct2) { FactoryGirl.create(:account, :vm_or_template_id => vm.id, :name => "Jane") }