Skip to content

Expose request_logs as a read-only subcollection of all request types - #1331

Open
jrafanie wants to merge 1 commit into
ManageIQ:masterfrom
jrafanie:add-request-log-subcollection
Open

jrafanie wants to merge 1 commit into
ManageIQ:masterfrom
jrafanie:add-request-log-subcollection

Conversation

@jrafanie

@jrafanie jrafanie commented Jun 25, 2026 •

Copy link
Copy Markdown
Member

Request logs are recorded for every MiqRequest (e.g. approval and
automate events) but could not be retrieved through the API, so clients
had no way to see why a request was stalled or failed.

Authorize with the existing miq_request_show_logs product feature, the
same one the classic UI checks before rendering the Request Logs
section, so the API and UI agree on who can see request logs. This
matters for custom roles that were granted miq_request_show without the
Logs feature.

The parent MiqRequest lookup is already RBAC-filtered before
request_logs_query_resource runs, so scoping by resource_id is enough.
Added to requests, service_requests, provision_requests and
automation_requests for consistency with request_tasks.

Comment thread config/api.yml
:klass: AutomationRequest
:subcollections:
- :request_tasks
- :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

@miq-bot miq-bot added the wip label Jun 25, 2026
Comment thread config/api.yml Outdated
:subcollection_actions:
:get:
- :name: read
:identifier: miq_request_show

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.

I need to double check what the classic UI and (if the service UI has it, which it might not) uses in order to determine who can see request logs... the current logic is to show request logs for people who can see the request.

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.

will change this to miq_request_show_logs, which is what the UI uses. Service UI doesn't seem to have this at all so if it adds it, it can also use this same feature or add it's own.

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.

done

@jrafanie
jrafanie force-pushed the add-request-log-subcollection branch 2 times, most recently from 38c6975 to 595101f Compare October 21, 2026 19:32
@jrafanie jrafanie removed the wip label Oct 21, 2026
@jrafanie jrafanie changed the title [WIP] Expose request_logs as a read-only subcollection of all request types Expose request_logs as a read-only subcollection of all request types Oct 21, 2026
@jrafanie

Copy link
Copy Markdown
Member Author

@prashantsrivastava6, can you give this a try and report any results? I changed the product feature to use miq_request_show_logs, which is what the UI uses to show logs.

@jrafanie

jrafanie commented Nov 6, 2026

Copy link
Copy Markdown
Member Author

@Fryguy do you want to get this in, I think it's all ready to go? We can get feedback when it lands.

Request logs are recorded for every MiqRequest (e.g. approval and
automate events) but could not be retrieved through the API, so clients
had no way to see why a request was stalled or failed.

Authorize with the existing miq_request_show_logs product feature, the
same one the classic UI checks before rendering the Request Logs
section, so the API and UI agree on who can see request logs. This
matters for custom roles that were granted miq_request_show without the
Logs feature.

The parent MiqRequest lookup is already RBAC-filtered before
request_logs_query_resource runs, so scoping by resource_id is enough.
Added to requests, service_requests, provision_requests and
automation_requests for consistency with request_tasks.
@jrafanie
jrafanie force-pushed the add-request-log-subcollection branch from 595101f to 84b32b8 Compare November 6, 2026 16:19
@jrafanie

jrafanie commented Nov 6, 2026

Copy link
Copy Markdown
Member Author

Now, it's ready for review

@miq-bot

miq-bot commented Nov 6, 2026

Copy link
Copy Markdown
Member

Checked commit jrafanie@84b32b8 with ruby 3.3.12, rubocop 1.91.0, haml-lint 0.78.0, and yamllint 1.37.1
6 files checked, 0 offenses detected
Everything looks fine. 🏆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants