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

Edit VMs API #14623

Merged
merged 2 commits into from
Apr 5, 2017
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
47 changes: 46 additions & 1 deletion app/controllers/api/vms_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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|
Expand All @@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

👍 this way we control what gets assigned.

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
Expand Down
2 changes: 0 additions & 2 deletions config/api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2265,7 +2265,6 @@
:disabled: true
- :name: edit
:identifier: vm_edit
:disabled: true
- :name: add_lifecycle_event
:identifier: vm_edit
- :name: add_event
Expand Down Expand Up @@ -2313,7 +2312,6 @@
:post:
- :name: edit
:identifier: vm_edit
:disabled: true
- :name: add_lifecycle_event
:identifier: vm_edit
- :name: add_event
Expand Down
86 changes: 86 additions & 0 deletions spec/requests/api/vms_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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") }
Expand Down