Skip to content

Commit f903bb1

Browse files
Bencheng21Bencheng21
authored
Merge pull request #97 from ConductorOne/INC300
[INC-300] baton-github: optimize Grants() for org Resource
2 parents 206fc0d + ad3faeb commit f903bb1

File tree

4 files changed

+113
-93
lines changed

4 files changed

+113
-93
lines changed

‎pkg/connector/org.go‎

Lines changed: 56 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -187,72 +187,78 @@ func (o *orgResourceType) Grants(
187187
return nil, "", nil, err
188188
}
189189

190-
opts := github.ListMembersOptions{
191-
ListOptions: github.ListOptions{
192-
Page: page,
193-
PerPage: maxPageSize,
194-
},
195-
}
190+
var (
191+
reqAnnos annotations.Annotations
192+
pageToken string
193+
rv = []*v2.Grant{}
194+
)
196195

197-
orgName, err := o.orgCache.GetOrgName(ctx, resource.Id)
198-
if err != nil {
199-
return nil, "", nil, err
200-
}
196+
switch rId := bag.ResourceTypeID(); rId {
197+
case resourceTypeOrg.Id:
198+
bag.Pop()
199+
bag.Push(pagination.PageState{
200+
ResourceTypeID: orgRoleAdmin,
201+
})
202+
bag.Push(pagination.PageState{
203+
ResourceTypeID: orgRoleMember,
204+
})
205+
case orgRoleAdmin, orgRoleMember:
201206

202-
users, resp, err := o.client.Organizations.ListMembers(ctx, orgName, &opts)
203-
if err != nil {
204-
if isNotFoundError(resp) {
205-
return nil, "", nil, uhttp.WrapErrors(codes.NotFound, fmt.Sprintf("org: %s not found", orgName))
207+
orgName, err := o.orgCache.GetOrgName(ctx, resource.Id)
208+
if err != nil {
209+
return nil, "", nil, err
206210
}
207-
errMsg := "github-connectorv2: failed to list org members"
208-
if isRatelimited(resp) {
209-
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
211+
opts := github.ListMembersOptions{
212+
Role: rId,
213+
ListOptions: github.ListOptions{
214+
Page: page,
215+
PerPage: maxPageSize,
216+
},
210217
}
211-
return nil, "", nil, fmt.Errorf("%s: %w", errMsg, err)
212-
}
213-
214-
nextPage, reqAnnos, err := parseResp(resp)
215-
if err != nil {
216-
return nil, "", nil, fmt.Errorf("github-connectorv2: failed to parse response: %w", err)
217-
}
218-
219-
pageToken, err := bag.NextToken(nextPage)
220-
if err != nil {
221-
return nil, "", nil, err
222-
}
223-
224-
var rv []*v2.Grant
225-
for _, user := range users {
226-
membership, _, err := o.client.Organizations.GetOrgMembership(ctx, user.GetLogin(), orgName)
218+
users, resp, err := o.client.Organizations.ListMembers(ctx, orgName, &opts)
227219
if err != nil {
228-
return nil, "", nil, fmt.Errorf("github-connectorv2: failed to get org memberships for user: %w", err)
220+
if isNotFoundError(resp) {
221+
return nil, "", nil, uhttp.WrapErrors(codes.NotFound, fmt.Sprintf("org: %s not found", orgName))
222+
}
223+
errMsg := "github-connectorv2: failed to list org members"
224+
if isRatelimited(resp) {
225+
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
226+
}
227+
return nil, "", nil, fmt.Errorf("%s: %w", errMsg, err)
229228
}
230-
if membership.GetState() == "pending" {
231-
continue
229+
230+
var nextPage string
231+
nextPage, reqAnnos, err = parseResp(resp)
232+
if err != nil {
233+
return nil, "", nil, fmt.Errorf("github-connectorv2: failed to parse response: %w", err)
232234
}
233235

234-
ur, err := userResource(ctx, user, user.GetEmail(), nil)
236+
err = bag.Next(nextPage)
235237
if err != nil {
236238
return nil, "", nil, err
237239
}
238240

239-
roleName := strings.ToLower(membership.GetRole())
240-
switch roleName {
241-
case orgRoleAdmin:
242-
rv = append(rv, o.orgRoleGrant(orgRoleAdmin, resource, ur.Id, user.GetID()))
243-
rv = append(rv, o.orgRoleGrant(orgRoleMember, resource, ur.Id, user.GetID()))
244-
245-
case orgRoleMember:
246-
rv = append(rv, o.orgRoleGrant(orgRoleMember, resource, ur.Id, user.GetID()))
241+
for _, user := range users {
242+
ur, err := userResource(ctx, user, user.GetEmail(), nil)
243+
if err != nil {
244+
return nil, "", nil, err
245+
}
247246

248-
default:
249-
ctxzap.Extract(ctx).Warn("Unknown GitHub Role Name",
250-
zap.String("role_name", roleName),
251-
zap.String("github_username", user.GetLogin()),
252-
)
247+
if rId == orgRoleAdmin {
248+
rv = append(rv, o.orgRoleGrant(orgRoleMember, resource, ur.Id, user.GetID()))
249+
}
250+
rv = append(rv, o.orgRoleGrant(rId, resource, ur.Id, user.GetID()))
253251
}
252+
default:
253+
ctxzap.Extract(ctx).Warn("Unknown GitHub Role Name",
254+
zap.String("role_name", rId),
255+
)
254256
}
255257

258+
pageToken, err = bag.Marshal()
259+
if err != nil {
260+
return nil, "", nil, err
261+
}
256262
return rv, pageToken, reqAnnos, nil
257263
}
258264

‎pkg/connector/org_test.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,10 @@ func TestOrganization(t *testing.T) {
3737
require.Nil(t, err)
3838
require.Empty(t, grantAnnotations)
3939

40-
grants, nextToken, grantsAnnotations, err := client.Grants(ctx, organization, &pagination.Token{})
40+
_, nextToken, grantsAnnotations, err := client.Grants(ctx, organization, &pagination.Token{})
4141
require.Nil(t, err)
4242
test.AssertHasRatelimitAnnotations(t, grantsAnnotations)
43-
require.Equal(t, "", nextToken)
44-
require.Len(t, grants, 2)
43+
require.Equal(t, "{\"states\":[{\"type\":\"admin\"}],\"current_state\":{\"type\":\"member\"}}", nextToken)
4544

4645
grant := v2.Grant{
4746
Entitlement: &entitlement,

‎pkg/connector/team.go‎

Lines changed: 53 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -174,56 +174,72 @@ func (o *teamResourceType) Grants(ctx context.Context, resource *v2.Resource, pT
174174
return nil, "", nil, err
175175
}
176176

177-
opts := github.TeamListTeamMembersOptions{
178-
ListOptions: github.ListOptions{
179-
Page: page,
180-
PerPage: maxPageSize,
181-
},
182-
}
183-
184-
users, resp, err := o.client.Teams.ListTeamMembersByID(ctx, org.GetID(), githubID, &opts)
185-
if err != nil {
186-
if isNotFoundError(resp) {
187-
return nil, "", nil, uhttp.WrapErrors(codes.NotFound, fmt.Sprintf("org: %d not found", org.GetID()))
188-
}
189-
if isRatelimited(resp) {
190-
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
177+
var (
178+
reqAnnos annotations.Annotations
179+
pageToken string
180+
rv = []*v2.Grant{}
181+
)
182+
switch rId := bag.ResourceTypeID(); rId {
183+
case resourceTypeTeam.Id:
184+
bag.Pop()
185+
bag.Push(pagination.PageState{
186+
ResourceTypeID: teamRoleMember,
187+
})
188+
bag.Push(pagination.PageState{
189+
ResourceTypeID: teamRoleMaintainer,
190+
})
191+
case teamRoleMember, teamRoleMaintainer:
192+
opts := github.TeamListTeamMembersOptions{
193+
ListOptions: github.ListOptions{
194+
Page: page,
195+
PerPage: maxPageSize,
196+
},
197+
Role: rId,
191198
}
192-
return nil, "", nil, fmt.Errorf("github-connectorv2: failed to fetch team members: %w", err)
193-
}
194-
195-
nextPage, reqAnnos, err := parseResp(resp)
196-
if err != nil {
197-
return nil, "", nil, fmt.Errorf("github-connectorv2: failed to parse response: %w", err)
198-
}
199-
200-
pageToken, err := bag.NextToken(nextPage)
201-
if err != nil {
202-
return nil, "", nil, err
203-
}
204199

205-
var rv []*v2.Grant
206-
for _, user := range users {
207-
membership, _, err := o.client.Teams.GetTeamMembershipByID(ctx, org.GetID(), githubID, user.GetLogin())
200+
users, resp, err := o.client.Teams.ListTeamMembersByID(ctx, org.GetID(), githubID, &opts)
208201
if err != nil {
209202
if isNotFoundError(resp) {
210-
return nil, "", nil, uhttp.WrapErrors(codes.NotFound, fmt.Sprintf("user: %s not found", user.GetLogin()))
203+
return nil, "", nil, uhttp.WrapErrors(codes.NotFound, fmt.Sprintf("org: %d not found", org.GetID()))
204+
}
205+
if isRatelimited(resp) {
206+
return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err)
211207
}
212-
return nil, "", nil, fmt.Errorf("github-connectorv2: failed to get team membership for user: %w", err)
208+
return nil, "", nil, fmt.Errorf("github-connectorv2: failed to fetch team members: %w", err)
209+
}
210+
211+
var nextPage string
212+
nextPage, reqAnnos, err = parseResp(resp)
213+
if err != nil {
214+
return nil, "", nil, fmt.Errorf("github-connectorv2: failed to parse response: %w", err)
213215
}
214216

215-
ur, err := userResource(ctx, user, user.GetEmail(), nil)
217+
err = bag.Next(nextPage)
216218
if err != nil {
217219
return nil, "", nil, err
218220
}
219221

220-
rv = append(rv, grant.NewGrant(resource, membership.GetRole(), ur.Id,
221-
grant.WithAnnotation(&v2.V1Identifier{
222-
Id: fmt.Sprintf("team-grant:%s:%d:%s", resource.Id.Resource, user.GetID(), membership.GetRole()),
223-
}),
224-
))
222+
for _, user := range users {
223+
ur, err := userResource(ctx, user, user.GetEmail(), nil)
224+
if err != nil {
225+
return nil, "", nil, err
226+
}
227+
rv = append(rv, grant.NewGrant(resource, rId, ur.Id,
228+
grant.WithAnnotation(&v2.V1Identifier{
229+
Id: fmt.Sprintf("team-grant:%s:%d:%s", resource.Id.Resource, user.GetID(), rId),
230+
}),
231+
))
232+
}
233+
default:
234+
ctxzap.Extract(ctx).Warn("Unknown GitHub Role Name",
235+
zap.String("role_name", rId),
236+
)
225237
}
226238

239+
pageToken, err = bag.Marshal()
240+
if err != nil {
241+
return nil, "", nil, err
242+
}
227243
return rv, pageToken, reqAnnos, nil
228244
}
229245

‎pkg/connector/team_test.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,10 @@ func TestTeam(t *testing.T) {
3939
require.Nil(t, err)
4040
require.Empty(t, grantAnnotations)
4141

42-
grants, nextToken, grantsAnnotations, err := client.Grants(ctx, team, &pagination.Token{})
42+
_, nextToken, grantsAnnotations, err := client.Grants(ctx, team, &pagination.Token{})
4343
require.Nil(t, err)
4444
test.AssertHasRatelimitAnnotations(t, grantsAnnotations)
45-
require.Equal(t, "", nextToken)
46-
require.Len(t, grants, 1)
45+
require.Equal(t, "{\"states\":[{\"type\":\"member\"}],\"current_state\":{\"type\":\"maintainer\"}}", nextToken)
4746

4847
grant := v2.Grant{
4948
Entitlement: &entitlement,

0 commit comments

Comments
 (0)