Skip to content
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

chore: implement api for creating custom roles #13298

Merged
merged 4 commits into from
May 16, 2024
Merged

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented May 16, 2024

No description provided.

Copy link
Member Author

Emyrk commented May 16, 2024

@Emyrk Emyrk force-pushed the stevenmasley/custom_role_functions branch from ba10185 to ef8c950 Compare May 16, 2024 15:14
@Emyrk Emyrk force-pushed the stevenmasley/custom_role_api branch from ad4023b to c1c3891 Compare May 16, 2024 15:14
Base automatically changed from stevenmasley/custom_role_functions to stevenmasley/custom_site_db May 16, 2024 15:17
@Emyrk Emyrk force-pushed the stevenmasley/custom_site_db branch from 4060c07 to c58a6fd Compare May 16, 2024 15:26
@Emyrk Emyrk force-pushed the stevenmasley/custom_role_api branch from c1c3891 to af25ebb Compare May 16, 2024 15:27
@Emyrk Emyrk force-pushed the stevenmasley/custom_site_db branch from c58a6fd to 88ca559 Compare May 16, 2024 15:30
@Emyrk Emyrk force-pushed the stevenmasley/custom_role_api branch from af25ebb to 8abb57e Compare May 16, 2024 15:30
@Emyrk Emyrk force-pushed the stevenmasley/custom_site_db branch from 88ca559 to 96d1527 Compare May 16, 2024 15:44
@Emyrk Emyrk force-pushed the stevenmasley/custom_role_api branch 3 times, most recently from f1c61bd to e5613b2 Compare May 16, 2024 15:54
coderd/rbac/rolestore/rolestore.go Show resolved Hide resolved
codersdk/roles.go Show resolved Hide resolved
codersdk/roles.go Outdated Show resolved Hide resolved

// UpsertCustomSiteRole will upsert a custom site wide role
func (c *Client) UpsertCustomSiteRole(ctx context.Context, req Role) (Role, error) {
res, err := c.Request(ctx, http.MethodPatch, "/api/v2/users/roles", req)
Copy link
Member

Choose a reason for hiding this comment

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

/api/v2/users/roles seems like a weird place to mount this 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea? That is where the roles api currently is. Any suggestions?

Copy link
Member

@johnstcn johnstcn May 16, 2024

Choose a reason for hiding this comment

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

Wouldn't they be org-scoped?

EDIT: Ah no, they can have user, org, and site perms.

Honestly I'd put them under a separate /api/v2/roles endpoint but I think that's definitely out of scope of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

These roles are site scoped.

There is a /roles under /organizations/{organization}/members/roles for org scoped roles.

coderd/database/db2sdk/db2sdk.go Show resolved Hide resolved
enterprise/coderd/roles_test.go Show resolved Hide resolved
@Emyrk Emyrk force-pushed the stevenmasley/custom_role_api branch from 4259691 to e0cdcdd Compare May 16, 2024 16:26
Comment on lines +63 to +72
// Verify the role exists in the list
// TODO: Turn this assertion back on when the cli api experience is created.
//allRoles, err := tmplAdmin.ListSiteRoles(ctx)
//require.NoError(t, err)
//
//require.True(t, slices.ContainsFunc(allRoles, func(selected codersdk.AssignableRoles) bool {
// return selected.Name == role.Name
//}), "role missing from site role list")
Copy link
Member Author

Choose a reason for hiding this comment

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

To prevent this PR from growing anymore, I am going to push this to the next PR. Essentially, from a UI perspective, our existing role endpoints do not fetch custom roles.

Since there is no UI or cli in this PR and this is already large, I am going to intentionally not solve this here (and this is gated behind an experiment).

@Emyrk Emyrk force-pushed the stevenmasley/custom_site_db branch from 96d1527 to 992c845 Compare May 16, 2024 17:07
@Emyrk Emyrk force-pushed the stevenmasley/custom_role_api branch from 07d531d to 28477ba Compare May 16, 2024 17:07
@Emyrk Emyrk force-pushed the stevenmasley/custom_site_db branch from 992c845 to a6996f6 Compare May 16, 2024 17:09
@Emyrk Emyrk force-pushed the stevenmasley/custom_role_api branch 2 times, most recently from 75e7bd8 to 747fc79 Compare May 16, 2024 17:22
@Emyrk Emyrk requested a review from johnstcn May 16, 2024 17:23
Base automatically changed from stevenmasley/custom_site_db to main May 16, 2024 18:11
@Emyrk Emyrk force-pushed the stevenmasley/custom_role_api branch from 801e054 to 68807ac Compare May 16, 2024 18:14
@Emyrk Emyrk merged commit ad8c314 into main May 16, 2024
34 of 36 checks passed
@Emyrk Emyrk deleted the stevenmasley/custom_role_api branch May 16, 2024 18:47
@github-actions github-actions bot locked and limited conversation to collaborators May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants