-
Notifications
You must be signed in to change notification settings - Fork 896
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
RBAC support for Automate Service Models #12369
Conversation
# See https://github.com/ruby/ruby/blob/trunk/lib/drb/drb.rb#L1658 | ||
# Previously we had used DRb.front but that gets compromised when multiple | ||
# DRb servers are running in the same process. | ||
def self.workspace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fryguy
This is the method that fetches the workspace. Please review
<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush. |
<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush. |
@gmcculloug @Fryguy |
MiqAeEngine::MiqAeWorkspaceRuntime.current || MiqAeEngine::DrbRemoteInvoker.workspace | ||
end | ||
|
||
def workspace_from_drb_thread |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this method get used anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gmcculloug
This was the old method which used the module function front to get to the workspace. I will get rid of it
@@ -60,12 +61,39 @@ def readonly? | |||
@readonly | |||
end | |||
|
|||
def self.current=(ws) | |||
Thread.current.thread_variable_set(:current_workspace, ws) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is thread_variable_set
needed or could you use Thread.current[: current_workspace] = ws
.
Applies to the matching thread_variable_get
below as well.
@@ -168,49 +178,58 @@ module MiqAeServiceSpec | |||
|
|||
context "#create_notification!" do | |||
it "invalid type" do | |||
allow(workspace).to receive(:disable_rbac) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can move this into a before
block for this context.
<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush. |
@Fryguy Any thoughts before merging? |
Checked commits mkanoor/manageiq@8c99cf0~...346be98 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
RBAC support for Automate Service Models (cherry picked from commit b2cd625) https://bugzilla.redhat.com/show_bug.cgi?id=1411477
Euwe backport details:
|
Filters service model objects based on the current user passed into
Automate.
The previous PR was reverted.
Previously we were using DRb.front to access the front object from DRb client threads, this PR uses a thread variable that DRb server sets when dispatching new client request threads.
Links