Adding Azure role definitions and assignments clients #48632
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
mvbrock
added
the
no-changelog
lib/cloud/azure/roleassignments.go
Outdated
| type RoleAssignmentsClient interface { | ||
| ListRoleAssignments(ctx context.Context, scope string) ([]*armauthorization.RoleAssignment, error) | ||
| } | ||
|
|
||
| type roleAssignmentsClient struct { | ||
| cli *armauthorization.RoleAssignmentsClient | ||
| } | ||
|
|
||
| func (c *roleAssignmentsClient) ListRoleAssignments(ctx context.Context, scope string) ([]*armauthorization.RoleAssignment, error) { | ||
| pager := c.cli.NewListForScopePager(scope, nil) | ||
| roleDefs := make([]*armauthorization.RoleAssignment, 0, 128) | ||
| for pager.More() { | ||
| page, err := pager.NextPage(ctx) | ||
| if err != nil { | ||
| return nil, trace.Wrap(err) | ||
| } | ||
| roleDefs = append(roleDefs, page.Value...) | ||
| } | ||
| return roleDefs, nil | ||
| } | ||
|
|
||
| func NewRoleAssignmentsClient(subscription string, cred azcore.TokenCredential, options *arm.ClientOptions) (RoleAssignmentsClient, error) { |
There was a problem hiding this comment.
This use of interfaces is a bit of an anti-pattern. It's generally recommended to define the interface at the consumer, and not the producer. See https://go.dev/wiki/CodeReviewComments#interfaces.
There was a problem hiding this comment.
Makes sense. I can remove the interface entirely, rename roleAssignmentsClient to the be the concrete type, and (eventually) create the interface at the consumer. This kind of makes the PR an anti-pattern, in that I'm defining an interface and an implementation without a consumer, aka not driven by business logic. I might actually just close this PR and incorporate the new clients into a PR with the consumer of the functionality (and define the interface in the consumer).
There was a problem hiding this comment.
Sounds good to me. I think we've come to realize that these cloud clients interfaces cause problems and result in bloating dependency trees. As we've started to migrate things to use aws-sdk-go-v2, previous consumers of the aws cloud client interface have opted to forgo that and define a smaller local interface instead.
lib/cloud/azure/roledefinitions.go
Outdated
| type RoleDefinitionsClient interface { | ||
| ListRoleDefinitions(ctx context.Context, scope string) ([]*armauthorization.RoleDefinition, error) | ||
| } | ||
|
|
||
| type roleDefinitionsClient struct { | ||
| cli *armauthorization.RoleDefinitionsClient | ||
| } | ||
|
|
||
| func (c *roleDefinitionsClient) ListRoleDefinitions(ctx context.Context, scope string) ([]*armauthorization.RoleDefinition, error) { | ||
| pager := c.cli.NewListPager(scope, nil) | ||
| roleDefs := make([]*armauthorization.RoleDefinition, 0, 128) | ||
| for pager.More() { | ||
| page, err := pager.NextPage(ctx) | ||
| if err != nil { | ||
| return nil, trace.Wrap(err) | ||
| } | ||
| roleDefs = append(roleDefs, page.Value...) | ||
| } | ||
| return roleDefs, nil | ||
| } | ||
|
|
||
| func NewRoleDefinitionsClient(subscription string, cred azcore.TokenCredential, options *arm.ClientOptions) (RoleDefinitionsClient, error) { |
There was a problem hiding this comment.
This use of interfaces is a bit of an anti-pattern. It's generally recommended to define the interface at the consumer, and not the producer. See https://go.dev/wiki/CodeReviewComments#interfaces.
|
Closing per convo with @rosstimothy about defining interfaces from the consumer and adding functionality based on both business requirements and interface specs. |
Part of of https://github.com/gravitational/access-graph/issues/640 to provide visibility of Azure resource in the Access Graph, and originating from the Azure integration POC branch master...mvbrock/azure-integration-poc. This PR adds the Azure role definitions and assignments clients to the cloud library, in order to build access graph relationships for Azure identities.