-
Notifications
You must be signed in to change notification settings - Fork 988
Add support for space-supporter role for set-space-role and unset-space-role #2176
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
Conversation
unset-space-role This also makes both commands case-insensitive for all roles Still need to update help text, will ask in PR thread for requested help text #2175
|
We have created an issue in Pivotal Tracker to manage this: https://www.pivotaltracker.com/story/show/178832672 The labels on this github issue will be updated when the story is started. |
|
@monamohebbi I have requested your review on this PR, once that's done it should be good to merge! |
monamohebbi
left a comment
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.
Looks good! I'm happy with the help text you have and don't think we should get to deep into bikeshedding, but let me know your thoughts on my suggestions. But could you update the test for whatever text you end up using?
Co-authored-by: Juan Diego Gonzalez <gojuan@vmware.com>
monamohebbi
left a comment
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!
| func convertSpaceRoleType(givenRole flag.SpaceRole) (constant.RoleType, error) { | ||
| switch givenRole.Role { | ||
| case "SpaceAuditor": | ||
| switch strings.ToLower(givenRole.Role) { |
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.
Love that!
a-b
left a comment
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
Does this PR modify CLI v6 or v7?
v8!
Description of the Change
Adds support for space-supporter role on set-space-role and unset-space-role, it also makes role entry for these commands both case insensitive.
Help text needs to be modified, will ask in comments of this PR for what the folks working on RBAC want it to say
This will throw an error if you run it on any foundation not set up for space-supporter, which is most, so the command needs to be listed as beta and subject to change
Applicable Issues
#2175