Adding support for /api/requests creation and edits #11892

abellotti · 2016-10-12T22:51:24Z

Support for creates via POST /api/requests
Support for edits via POST /api/requests/:id - action "edit"

abellotti · 2016-10-12T22:52:19Z

Marked as WIP

TODO's

tire kicking

abellotti · 2016-10-12T22:55:09Z

Basic usage:

POST /api/requests

{
  "options" : {
     "request_type" : "service_reconfigure",
     "src_id" : 4
  },
  "requester" : {
    "user_name" : "admin"
  },
  "auto_approve" : false
}

GET /api/requests/:id

GET /api/requests/:id?expand=tasks

abellotti · 2016-10-12T22:58:42Z

/cc @bdunne @carbonin

WIP, but was able to create a request from API client.

miq.requests.create(:options => { :request_type => "service_reconfigure", :src_id => 4}, :auto_approve => false)

bdunne · 2016-10-13T15:36:08Z

app/controllers/api/requests_controller.rb

+    def create_resource(type, _id, data)
+      assert_id_not_specified(data, type)
+
+      request_type = data.delete("request_type")


:request_type is an attribute on MiqRequest, so it's probably not safe to use that.

bdunne · 2016-10-13T15:37:08Z

app/controllers/api/requests_controller.rb

+    include Subcollections::RequestTasks
+    include Subcollections::Tasks
+
+    def create_resource(type, _id, data)


Do you need _id since it isn't used?

API signature for _resource.

bdunne · 2016-10-13T15:40:20Z

app/controllers/api/requests_controller.rb

+
+      requester = Hash(data.delete("requester"))
+      user = fetch_requester_user(requester)
+      auto_approve = fetch_auto_approve(requester)


The auto_approve flag lives under the requester?

you're right, was using an example from MiqProvisionVirtWorkflow. Moved that puppy out so it is its own parameter now.

bdunne · 2016-10-13T15:42:03Z

app/controllers/api/requests_controller.rb

+      request_type.constantize.create_request(data.symbolize_keys, user, auto_approve)
+    end
+
+    def edit_resource(type, id = nil, data = {})


What is type? I'm assuming STI type, but not sure why we would need that if we have an ID.

collection type, API signature as above.

bdunne · 2016-10-13T15:43:30Z

app/controllers/api/requests_controller.rb

+      raise BadRequestError, "Must specify a id for editing a #{type} resource" unless id
+      request_klass = collection_class(:requests)
+      request = resource_search(id, type, request_klass)
+      user = fetch_requester_user(Hash(data.delete("requester")))


The user updating the request may not be the requester.

bdunne · 2016-10-13T15:45:42Z

config/api.yml

    :collection_actions:
      :get:
      - :name: read
        :identifier: miq_request_show_list
      :post:
      - :name: query
        :identifier: miq_request_show_list
+      - :name: create
+        :identifier: vm_miq_request_new


MiqRequests are not tied to only VMs

yes, this needs to change. Couldn't fine a generic miq_request_new role identifier. Part of the TODO is to support different identifier based on request_type. One option is to open the door here (no role check), but enforce it in the create method based on the type.

done, role identifier per request type now provided by model and used for authorization.

- Support for creates via POST /api/requests - Support for edits via POST /api/requests/:id - action "edit"

- Added mapping of request_type to role identifier in model - Updated default role for creation based on read - Added user authorization based on request_type

miq-bot · 2016-10-19T15:52:43Z

Checked commits abellotti/manageiq@a4a8c11~...a6d6e3e with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 0 offenses detected
Everything looks good. 🏆

carbonin

Thanks @abellotti

carbonin · 2016-10-21T18:17:28Z

Tested this out with the API client gem and it looks good. I think we can un-wip and merge.

bdunne · 2016-10-21T18:17:36Z

Tires have been kicked and everything looks good

Adding support for /api/requests creation and edits (cherry picked from commit aacbf4f)

chessbyte · 2016-10-21T22:45:44Z

Euwe Backport details:

$ git log -1
commit 72c9b0a4051baf1b169409f5485bd20b240cdd6d
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Fri Oct 21 14:18:01 2016 -0400

    Merge pull request #11892 from abellotti/api_create_requests

    Adding support for /api/requests creation and edits
    (cherry picked from commit aacbf4f6fb2dc5edbc5ff52f9c8a40b68b415306)

abellotti added enhancement wip api euwe/yes labels Oct 12, 2016

abellotti assigned gtanzillo Oct 12, 2016

bdunne reviewed Oct 13, 2016

View reviewed changes

abellotti force-pushed the api_create_requests branch 4 times, most recently from 112d7ca to 5a30357 Compare October 18, 2016 22:50

abellotti added 8 commits October 19, 2016 11:08

Adding support for /api/requests creation and edits

a4a8c11

- Support for creates via POST /api/requests - Support for edits via POST /api/requests/:id - action "edit"

Moved the auto_approve out of requester into its own parameter.

5ac4b30

Added user authorization of requests creation based on request_type

106da09

- Added mapping of request_type to role identifier in model - Updated default role for creation based on read - Added user authorization based on request_type

Like the create, we need to symbolize keys for the update.

cd19ade

Added first rounds of tests to check validations.

0404abe

Needed to catch possible errors from the request create or update.

d833b7e

Adding requests update and create specs

ead0791

Move to model driven validation and api.yml driven authorization

a6d6e3e

abellotti force-pushed the api_create_requests branch from 5a30357 to a6d6e3e Compare October 19, 2016 15:50

bdunne mentioned this pull request Oct 20, 2016

Regional relay of MiqRequests #12087

Merged

carbonin approved these changes Oct 21, 2016

View reviewed changes

bdunne removed the wip label Oct 21, 2016

bdunne changed the title ~~[WIP] Adding support for /api/requests creation and edits~~ Adding support for /api/requests creation and edits Oct 21, 2016

bdunne merged commit aacbf4f into ManageIQ:master Oct 21, 2016

bdunne added this to the Sprint 48 Ending Oct 24, 2016 milestone Oct 21, 2016

abellotti deleted the api_create_requests branch October 21, 2016 21:34

chessbyte pushed a commit that referenced this pull request Oct 21, 2016

Merge pull request #11892 from abellotti/api_create_requests

72c9b0a

Adding support for /api/requests creation and edits (cherry picked from commit aacbf4f)

chessbyte added euwe/backported and removed euwe/yes labels Oct 21, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for /api/requests creation and edits #11892

Adding support for /api/requests creation and edits #11892

abellotti commented Oct 12, 2016

abellotti commented Oct 12, 2016 •

edited

Loading

abellotti commented Oct 12, 2016 •

edited

Loading

abellotti commented Oct 12, 2016 •

edited

Loading

bdunne Oct 13, 2016

bdunne Oct 13, 2016

abellotti Oct 13, 2016

bdunne Oct 13, 2016

abellotti Oct 13, 2016

bdunne Oct 13, 2016

abellotti Oct 13, 2016

bdunne Oct 13, 2016

bdunne Oct 13, 2016

abellotti Oct 13, 2016

abellotti Oct 13, 2016

miq-bot commented Oct 19, 2016

carbonin left a comment

carbonin commented Oct 21, 2016

bdunne commented Oct 21, 2016

chessbyte commented Oct 21, 2016

Adding support for /api/requests creation and edits #11892

Adding support for /api/requests creation and edits #11892

Conversation

abellotti commented Oct 12, 2016

abellotti commented Oct 12, 2016 • edited Loading

abellotti commented Oct 12, 2016 • edited Loading

abellotti commented Oct 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Oct 19, 2016

carbonin left a comment

Choose a reason for hiding this comment

carbonin commented Oct 21, 2016

bdunne commented Oct 21, 2016

chessbyte commented Oct 21, 2016

abellotti commented Oct 12, 2016 •

edited

Loading

abellotti commented Oct 12, 2016 •

edited

Loading

abellotti commented Oct 12, 2016 •

edited

Loading