Skip to content

Conversation

@JenGoldstrich
Copy link
Contributor

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

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
@cf-gitbot
Copy link

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 monamohebbi linked an issue Jul 13, 2021 that may be closed by this pull request
@JenGoldstrich JenGoldstrich marked this pull request as ready for review July 14, 2021 16:47
@JenGoldstrich
Copy link
Contributor Author

@monamohebbi I have requested your review on this PR, once that's done it should be good to merge!

Copy link
Contributor

@monamohebbi monamohebbi left a 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>
Copy link
Contributor

@monamohebbi monamohebbi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@JenGoldstrich JenGoldstrich changed the title Add support for space-supporter role for set-space-role and Add support for space-supporter role for set-space-role and unset-space-role Jul 16, 2021
@JenGoldstrich JenGoldstrich requested a review from a-b July 16, 2021 18:02
func convertSpaceRoleType(givenRole flag.SpaceRole) (constant.RoleType, error) {
switch givenRole.Role {
case "SpaceAuditor":
switch strings.ToLower(givenRole.Role) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love that!

Copy link
Member

@a-b a-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@a-b a-b merged commit 9c07860 into master Jul 16, 2021
@a-b a-b deleted the space-role-supporter branch July 16, 2021 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow CF CLI users ability to assign Space Supporter role to users

5 participants