Add retirement initiator context #17951
Conversation
|
@miq-bot assign @gmcculloug |
|
@miq-bot add_label gaprindashvili/yes |
|
Tina pointed out that we'll need to adjust this due to the retirement as a request because at https://github.com/ManageIQ/manageiq/blob/master/app/models/mixins/retirement_mixin.rb#L126 we need to know which context is applicable. |
1d42b76 to
5e53746
Compare
|
It's only passing tests cause there were no tests. Travis didn't run on this. It's actually failing everything. Don't be fooled. |
| private | ||
|
|
||
| def find_requester(requester_context, q_options = {}) | ||
| requester_context == "system" ? q_options[:user_id] : User.current_user |
There was a problem hiding this comment.
@d-m-u This could still be nil if the q_options[:userid] is not set
There was a problem hiding this comment.
Yeah, @tinaafitz, I'm not sure how to handle that
| end | ||
|
|
||
| self.class.make_retire_request(self.id) if retirement_due? | ||
| requester = find_requester(initiator_context(retirement_requester)) |
There was a problem hiding this comment.
@d-m-u Since retirement_check is called by the scheduler, the initiator_context here would always be "system".
| private | ||
|
|
||
| def find_requester(requester_context, q_options = {}) | ||
| requester_context == "system" ? q_options[:user_id] : User.current_user |
3b18455 to
4f27777
Compare
|
@miq_bot remove_label wip |
|
@tinaafitz ping |
|
@miq-bot add_label wip |
6cfc8e8 to
47fe9a0
Compare
65aaea8 to
d742ffe
Compare
|
apparently somebody gave you bad advice :( |
d742ffe to
a245c8b
Compare
|
Checked commit https://github.com/d-m-u/manageiq/commit/a245c8bb833117b1be4fe055e0f285a34d1b231d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@bdunne @tinaafitz @gmcculloug @kbrock @mkanoor -- I don't suppose a one of you wants to approve this, by any chance, pretty please? |
|
I'm hoping you can get rid of this line. It has made me very sad for many moons. |
Add retirement initiator context (cherry picked from commit e666425) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1618530
|
Hammer backport details: |
|
Backported to Gaprindashvili via #18106 |
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1618530
If a system retirement goes into retry, there is an opportunity to reset the user because we're just checking the current user. This fixes that by adding a context and setting the user id, group id, and tenant id, which previously were not being set.
The default was set always originally to system. We then update it if there was a requester set. But the code to set the requester is just going off of a current user which can change.
... and ManageIQ/manageiq-schema#288 is related but not required to merge this, it's a later enhancement that'll require subsequent modification of the retirement mixin like what's happening here
related PRs: