Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

RBAC assignment methods #167

Merged
merged 7 commits into from
Nov 6, 2023
Merged

Conversation

aarongodin
Copy link
Contributor

@aarongodin aarongodin commented Oct 25, 2023

Addresses one component of https://github.com/grafana/identity-access-team/issues/424

This adds a set of methods on the client for interacting with the RBAC assignment through the /api/access-control/:resource endpoints.

One noteworthy change is that, to make all resources share the same logic for calling the API, I added an abstraction in resource_permissions.go. The methods in service_account_permissions.go were the only ones interacting with the desired endpoints. As such, I felt it was acceptable to make a breaking change by deleting the methods in that file and replacing them with ones that follow the same naming scheme as the other resources.

Additionally, I made all methods accept either an id or a uid through an interface called ResourceIdent. Since our endpoints allow for both, it seems best to have a generic way to pass these values as they can be either int64 or string.

@CLAassistant
Copy link

CLAassistant commented Oct 25, 2023

CLA assistant check
All committers have signed the CLA.

@aarongodin
Copy link
Contributor Author

I added testify to the go.mod in the project and it seems to have broke the golangci-lint command.

Copy link
Contributor

@IevaVasiljeva IevaVasiljeva left a comment

Choose a reason for hiding this comment

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

Nice work! I've left a few minor comments, but I think it's also good to merge as is after the imports are sorted out.

Regarding dependency management - I wonder if we need to update Go version 🤔 1.14 seems pretty old, and I think I've had similar issues in the past and bumping up the version helped.

I think it's fine to change the code of service_account_permissions.go, I don't think it would have been widely used. But we do need to make sure to change the service account permission resource of Terraform accordingly. We should also add deprecation notices to the current permission endpoints for folders etc (but that can be done in another PR).

resource_permissions.go Outdated Show resolved Hide resolved
dashboard_permissions.go Outdated Show resolved Hide resolved
dashboard_permissions.go Outdated Show resolved Hide resolved
dashboard_permissions.go Outdated Show resolved Hide resolved
resource_permissions_test.go Show resolved Hide resolved
@aarongodin aarongodin force-pushed the feat/resource-permissions-methods branch from 18c4431 to fdea1ff Compare November 1, 2023 16:19
Copy link
Contributor

@IevaVasiljeva IevaVasiljeva left a comment

Choose a reason for hiding this comment

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

Thanks for all the responses! All looks good to me, so I'd be happy with this being merged now. What do you think?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants