-
Notifications
You must be signed in to change notification settings - Fork 897
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
Create a physical infrastructure user group #17840
Conversation
Checked commits skovic/manageiq@3d5f991~...a3ecd11 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@miq-bot add_label providers/physical |
@miq-bot add_label enhancement |
@agrare Could you take a look at this PR when you get a chance? Thanks |
@gtanzillo can you help review the new role? |
- :name: EvmRole-physical_infrastructure | ||
:read_only: true | ||
:miq_product_feature_identifiers: | ||
- everything |
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.
Are you sure we need to give users in a group having this role "everything"? That is the equivalent of a super administrator. Is there a subset of features that these users would need instead of everything?
Since the purpose of this is mainly to give the group a custom dashboard, perhaps it would be good enough to copy the EvmRole-administrator
role for this group. Or, maybe you don't need a new role, just a new group that has the new dashboard. What do you think?
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.
@gtanzillo We wanted a usergroup whose members have the same access to everything in the UI that the admin user has but with a default dashboard containing our widgets. Is there a better way to do this?
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.
@skovic I think this is fine if you need the group to have everything
. I was thinking that you could just use an existing role with the new group. But, I noticed that the role maps are one to one in naming between group and role, by convention. We might as well keep it that way.
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.
LGTM 👍
- :name: EvmRole-physical_infrastructure | ||
:read_only: true | ||
:miq_product_feature_identifiers: | ||
- everything |
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.
@skovic I think this is fine if you need the group to have everything
. I was thinking that you could just use an existing role with the new group. But, I noticed that the role maps are one to one in naming between group and role, by convention. We might as well keep it that way.
This PR adds a new physical infrastructure user group and a default dashboard for the new group. The default dashboard contains a few physical server widgets.