Skip to content
Open
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
1 change: 1 addition & 0 deletions app/controllers/api/automation_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module Api
class AutomationRequestsController < BaseController
include Api::Mixins::ResourceCancel
include Api::Mixins::ResourceApproveDeny
include Subcollections::RequestLogs
include Subcollections::RequestTasks

def create_resource(type, _id, data)
Expand Down
1 change: 1 addition & 0 deletions app/controllers/api/provision_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module Api
class ProvisionRequestsController < BaseController
include Api::Mixins::ResourceCancel
include Api::Mixins::ResourceApproveDeny
include Subcollections::RequestLogs
include Subcollections::RequestTasks

def create_resource(type, _id, data)
Expand Down
1 change: 1 addition & 0 deletions app/controllers/api/requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module Api
class RequestsController < BaseController
include Api::Mixins::ResourceCancel
include Api::Mixins::ResourceApproveDeny
include Subcollections::RequestLogs
include Subcollections::RequestTasks

def create_resource(type, _id, data)
Expand Down
1 change: 1 addition & 0 deletions app/controllers/api/service_requests_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module Api
class ServiceRequestsController < BaseController
include Subcollections::RequestLogs
include Subcollections::RequestTasks
include Api::Mixins::Pictures
include Api::Mixins::ResourceCancel
Expand Down
12 changes: 12 additions & 0 deletions app/controllers/api/subcollections/request_logs.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module Api
module Subcollections
module RequestLogs
def request_logs_query_resource(object)
# object is the parent MiqRequest (or subclass), already RBAC-filtered by
# parent_resource_obj before this method is called, so no additional check is needed.
klass = collection_class(:request_logs)
object ? klass.where(:resource_id => object.id) : {}
end
end
end
end
18 changes: 18 additions & 0 deletions config/api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@
:verbs: *gp
:klass: AutomationRequest
:subcollections:
- :request_logs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alphabetical.

🏃💨

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OH, we added @Fryguy as a test

  1) API configuration (config/api.yml) collections is sorted a-z
     Failure/Error: expect(actual).to eq(expected)

       expected: [:accounts, :actions, :alert_actions, :alert_definition_profiles, :alert_definitions, :alerts, :auth,...mplates, :tenant_groups, :tenants, :time_profiles, :users, :vms, :volume_mappings, :widgets, :zones]
            got: [:accounts, :actions, :alert_actions, :alert_definition_profiles, :alert_definitions, :alerts, :auth,...mplates, :tenant_groups, :tenants, :time_profiles, :users, :vms, :volume_mappings, :widgets, :zones]

       (compared using ==)

       Diff:

       @@ -106,8 +106,8 @@
         :rates,
         :regions,
         :reports,
       - :request_logs,
         :request_tasks,
       + :request_logs,
         :requests,
         :resource_actions,
         :resource_pools,
     # ./spec/lib/api/api_config_spec.rb:10:in `block (3 levels) in <top (required)>'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ugh, I fixed the one but not the other, one second...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok, done

- :request_tasks
:collection_actions:
:get:
Expand Down Expand Up @@ -3000,6 +3001,7 @@
:verbs: *gp
:klass: MiqProvisionRequest
:subcollections:
- :request_logs
- :request_tasks
:collection_actions:
:get:
Expand Down Expand Up @@ -3255,6 +3257,20 @@
:post:
- :name: delete
:identifier: saved_report_delete
:request_logs:
:description: Request Logs
:options:
- :subcollection
:verbs: *g
:klass: RequestLog
:subcollection_actions:
:get:
- :name: read
:identifier: miq_request_show_logs
:subresource_actions:
:get:
- :name: read
:identifier: miq_request_show_logs
:request_tasks:
:description: Request Tasks
:options:
Expand Down Expand Up @@ -3296,6 +3312,7 @@
:verbs: *gp
:klass: MiqRequest
:subcollections:
- :request_logs
- :request_tasks
:collection_actions:
:get:
Expand Down Expand Up @@ -3870,6 +3887,7 @@
:verbs: *gpd
:klass: ServiceTemplateProvisionRequest
:subcollections:
- :request_logs
- :request_tasks
:collection_actions:
:get:
Expand Down
107 changes: 107 additions & 0 deletions spec/requests/request_logs_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
RSpec.describe "Request Logs Subcollection API" do
# request_logs is authorized via miq_request_show_logs, the same feature the classic UI checks
# before displaying request logs. subcollection_action_identifier resolves this correctly.
shared_examples "request_logs subcollection" do |parent_collection, parent_factory, logs_url_helper, log_url_helper|
let(:request) { FactoryBot.create(parent_factory, :requester => @user) }
let(:log) { FactoryBot.create(:request_log, :resource => request) }

context "listing logs" do
it "is forbidden without appropriate role" do
api_basic_authorize

get send(logs_url_helper, nil, request)

expect(response).to have_http_status(:forbidden)
end

it "is forbidden with only miq_request_show" do
api_basic_authorize "miq_request_show"

get send(logs_url_helper, nil, request)

expect(response).to have_http_status(:forbidden)
end

it "returns only logs belonging to the parent request" do
other_request = FactoryBot.create(parent_factory, :requester => @user)
log
other_log = FactoryBot.create(:request_log, :resource => other_request)
api_basic_authorize subcollection_action_identifier(parent_collection, :request_logs, :read, :get)

get send(logs_url_helper, nil, request), :params => {:expand => :resources}

expect(response).to have_http_status(:ok)
returned_ids = response.parsed_body["resources"].pluck("id")
expect(returned_ids).to include(log.id.to_s)
expect(returned_ids).not_to include(other_log.id.to_s)
end

it "returns all logs for the request" do
log1 = FactoryBot.create(:request_log, :resource => request, :message => "first")
log2 = FactoryBot.create(:request_log, :resource => request, :message => "second")
api_basic_authorize subcollection_action_identifier(parent_collection, :request_logs, :read, :get)

get send(logs_url_helper, nil, request), :params => {:expand => :resources}

expect(response).to have_http_status(:ok)
returned_ids = response.parsed_body["resources"].pluck("id")
expect(returned_ids).to match_array([log1.id.to_s, log2.id.to_s])
end
end

context "showing a single log" do
it "is forbidden without appropriate role" do
api_basic_authorize

get send(log_url_helper, nil, request, log)

expect(response).to have_http_status(:forbidden)
end

it "returns the log" do
api_basic_authorize subcollection_action_identifier(parent_collection, :request_logs, :read, :get)

get send(log_url_helper, nil, request, log)

expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(
"id" => log.id.to_s,
"message" => log.message,
"severity" => log.severity
)
end
end
end

context "under /automation_requests" do
include_examples "request_logs subcollection",
:automation_requests,
:automation_request,
:api_automation_request_request_logs_url,
:api_automation_request_request_log_url
end

context "under /provision_requests" do
include_examples "request_logs subcollection",
:provision_requests,
:miq_provision_request,
:api_provision_request_request_logs_url,
:api_provision_request_request_log_url
end

context "under /requests" do
include_examples "request_logs subcollection",
:requests,
:service_template_provision_request,
:api_request_request_logs_url,
:api_request_request_log_url
end

context "under /service_requests" do
include_examples "request_logs subcollection",
:service_requests,
:service_template_provision_request,
:api_service_request_request_logs_url,
:api_service_request_request_log_url
end
end
Loading