diff --git a/pkg/connector/api_token.go b/pkg/connector/api_token.go index 713e5e8b..ade4f698 100644 --- a/pkg/connector/api_token.go +++ b/pkg/connector/api_token.go @@ -95,7 +95,7 @@ func (o *apiTokenResourceType) List( }) if err != nil { if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, err } diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 15b40e7e..510fbdc9 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -97,21 +97,24 @@ type GitHub struct { graphqlClient *githubv4.Client hasSAMLEnabled *bool orgCache *orgNameCache + userCache *userDataCache + teamCache *teamDataCache syncSecrets bool enterprises []string } func (gh *GitHub) ResourceSyncers(ctx context.Context) []connectorbuilder.ResourceSyncer { resourceSyncers := []connectorbuilder.ResourceSyncer{ - orgBuilder(gh.client, gh.appClient, gh.orgCache, gh.orgs, gh.syncSecrets), - teamBuilder(gh.client, gh.orgCache), - userBuilder(gh.client, gh.hasSAMLEnabled, gh.graphqlClient, gh.orgCache, gh.orgs), - repositoryBuilder(gh.client, gh.orgCache), - orgRoleBuilder(gh.client, gh.orgCache), + orgBuilder(gh.client, gh.appClient, gh.orgCache, gh.userCache, gh.orgs, gh.syncSecrets), + teamBuilder(gh.client, gh.orgCache, gh.userCache, gh.teamCache), + userBuilder(gh.client, gh.hasSAMLEnabled, gh.graphqlClient, gh.orgCache, gh.userCache, gh.orgs), + repositoryBuilder(gh.client, gh.orgCache, gh.userCache, gh.teamCache), + orgRoleBuilder(gh.client, gh.orgCache, gh.userCache), invitationBuilder(invitationBuilderParams{ - client: gh.client, - orgCache: gh.orgCache, - orgs: gh.orgs, + client: gh.client, + orgCache: gh.orgCache, + userCache: gh.userCache, + orgs: gh.orgs, }), } @@ -120,7 +123,7 @@ func (gh *GitHub) ResourceSyncers(ctx context.Context) []connectorbuilder.Resour } if len(gh.enterprises) > 0 { - resourceSyncers = append(resourceSyncers, enterpriseRoleBuilder(gh.client, gh.customClient, gh.enterprises)) + resourceSyncers = append(resourceSyncers, enterpriseRoleBuilder(gh.client, gh.customClient, gh.userCache, gh.enterprises)) } return resourceSyncers } @@ -314,6 +317,8 @@ func New(ctx context.Context, ghc *cfg.Github, appKey string) (*GitHub, error) { enterprises: ghc.Enterprises, graphqlClient: graphqlClient, orgCache: newOrgNameCache(ghClient), + userCache: newUserDataCache(ghClient), + teamCache: newTeamDataCache(ghClient), syncSecrets: ghc.SyncSecrets, } return gh, nil @@ -454,7 +459,7 @@ func getOrgs(ctx context.Context, client *github.Client, orgs []string) ([]strin orgs, resp, err := client.Organizations.List(ctx, "", &github.ListOptions{Page: page, PerPage: maxPageSize}) if err != nil { if isRatelimited(resp) { - return nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } if isAuthError(resp) { return nil, uhttp.WrapErrors(codes.Unauthenticated, "github-connector: failed to retrieve org", err) diff --git a/pkg/connector/enterprise_role.go b/pkg/connector/enterprise_role.go index 5aa60ab8..5b2c12d2 100644 --- a/pkg/connector/enterprise_role.go +++ b/pkg/connector/enterprise_role.go @@ -22,6 +22,7 @@ type enterpriseRoleResourceType struct { resourceType *v2.ResourceType client *github.Client customClient *customclient.Client + userCache *userDataCache enterprises []string roleUsersCache map[string][]string mu *sync.Mutex @@ -139,10 +140,10 @@ func (o *enterpriseRoleResourceType) Grants( ret := []*v2.Grant{} for _, userLogin := range cache[resource.Id.Resource] { - user, resp, err := o.client.Users.Get(ctx, userLogin) + user, resp, err := o.userCache.GetUserByLogin(ctx, userLogin) if err != nil { if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("baton-github: error getting user %s: %w", userLogin, err) } @@ -162,11 +163,12 @@ func (o *enterpriseRoleResourceType) Grants( return ret, "", nil, nil } -func enterpriseRoleBuilder(client *github.Client, customClient *customclient.Client, enterprises []string) *enterpriseRoleResourceType { +func enterpriseRoleBuilder(client *github.Client, customClient *customclient.Client, userCache *userDataCache, enterprises []string) *enterpriseRoleResourceType { return &enterpriseRoleResourceType{ resourceType: resourceTypeEnterpriseRole, client: client, customClient: customClient, + userCache: userCache, enterprises: enterprises, roleUsersCache: make(map[string][]string), mu: &sync.Mutex{}, diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index 8c7c82da..e1f4ff30 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -67,6 +67,115 @@ func newOrgNameCache(c *github.Client) *orgNameCache { } } +type userDataCache struct { + sync.RWMutex + c *github.Client + usersById map[int64]*github.User + usersByLogin map[string]*github.User +} + +func (u *userDataCache) GetUser(ctx context.Context, userID int64) (*github.User, *github.Response, error) { + u.RLock() + if user, ok := u.usersById[userID]; ok { + u.RUnlock() + return user, nil, nil + } + u.RUnlock() + + u.Lock() + defer u.Unlock() + + if user, ok := u.usersById[userID]; ok { + return user, nil, nil + } + + user, resp, err := u.c.Users.GetByID(ctx, userID) + if err != nil { + return nil, resp, err + } + + u.usersById[userID] = user + if user.Login != nil { + u.usersByLogin[*user.Login] = user + } + + return user, resp, nil +} + +func (u *userDataCache) GetUserByLogin(ctx context.Context, login string) (*github.User, *github.Response, error) { + u.RLock() + if user, ok := u.usersByLogin[login]; ok { + u.RUnlock() + return user, nil, nil + } + u.RUnlock() + + u.Lock() + defer u.Unlock() + + if user, ok := u.usersByLogin[login]; ok { + return user, nil, nil + } + + user, resp, err := u.c.Users.Get(ctx, login) + if err != nil { + return nil, resp, err + } + + u.usersByLogin[login] = user + if user.ID != nil { + u.usersById[*user.ID] = user + } + + return user, resp, nil +} + +func newUserDataCache(c *github.Client) *userDataCache { + return &userDataCache{ + c: c, + usersById: make(map[int64]*github.User), + usersByLogin: make(map[string]*github.User), + } +} + +type teamDataCache struct { + sync.RWMutex + c *github.Client + teams map[int64]*github.Team +} + +func (t *teamDataCache) GetTeam(ctx context.Context, orgID int64, teamID int64) (*github.Team, *github.Response, error) { + t.RLock() + if team, ok := t.teams[teamID]; ok { + t.RUnlock() + return team, nil, nil + } + t.RUnlock() + + t.Lock() + defer t.Unlock() + + if team, ok := t.teams[teamID]; ok { + return team, nil, nil + } + + team, resp, err := t.c.Teams.GetTeamByID(ctx, orgID, teamID) //nolint:staticcheck // TODO: migrate to GetTeamBySlug + if err != nil { + return nil, resp, err + } + + t.teams[teamID] = team + + return team, resp, nil +} + +func newTeamDataCache(c *github.Client) *teamDataCache { + return &teamDataCache{ + c: c, + teams: make(map[int64]*github.Team), + } +} + func v1AnnotationsForResourceType(resourceTypeID string) annotations.Annotations { annos := annotations.Annotations{} annos.Update(&v2.V1Identifier{ diff --git a/pkg/connector/invitation.go b/pkg/connector/invitation.go index d4cae673..c5342c33 100644 --- a/pkg/connector/invitation.go +++ b/pkg/connector/invitation.go @@ -40,9 +40,10 @@ func invitationToUserResource(invitation *github.Invitation) (*v2.Resource, erro } type invitationResourceType struct { - client *github.Client - orgCache *orgNameCache - orgs []string + client *github.Client + orgCache *orgNameCache + userCache *userDataCache + orgs []string } var _ connectorbuilder.AccountManager = &invitationResourceType{} @@ -225,15 +226,17 @@ func getCreateUserParams(accountInfo *v2.AccountInfo) (*createUserParams, error) } type invitationBuilderParams struct { - client *github.Client - orgCache *orgNameCache - orgs []string + client *github.Client + orgCache *orgNameCache + userCache *userDataCache + orgs []string } func invitationBuilder(p invitationBuilderParams) *invitationResourceType { return &invitationResourceType{ - client: p.client, - orgCache: p.orgCache, - orgs: p.orgs, + client: p.client, + orgCache: p.orgCache, + userCache: p.userCache, + orgs: p.orgs, } } diff --git a/pkg/connector/org.go b/pkg/connector/org.go index cc2eec7e..ec0a8991 100644 --- a/pkg/connector/org.go +++ b/pkg/connector/org.go @@ -38,6 +38,7 @@ type orgResourceType struct { appClient *github.Client orgs map[string]struct{} orgCache *orgNameCache + userCache *userDataCache syncSecrets bool } @@ -103,7 +104,7 @@ func (o *orgResourceType) List( orgs, resp, err := o.client.Organizations.List(ctx, "", opts) if err != nil { if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("github-connector: failed to fetch org: %w", err) } @@ -131,7 +132,7 @@ func (o *orgResourceType) List( } if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, err } @@ -229,7 +230,7 @@ func (o *orgResourceType) Grants( } errMsg := "github-connectorv2: failed to list org members" if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("%s: %w", errMsg, err) } @@ -294,7 +295,7 @@ func (o *orgResourceType) Grant(ctx context.Context, principal *v2.Resource, en return nil, err } - user, _, err := o.client.Users.GetByID(ctx, principalID) + user, _, err := o.userCache.GetUser(ctx, principalID) if err != nil { return nil, fmt.Errorf("github-connectorv2: failed to get user: %w", err) } @@ -386,7 +387,7 @@ func (o *orgResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotati return nil, err } - user, _, err := o.client.Users.GetByID(ctx, principalID) + user, _, err := o.userCache.GetUser(ctx, principalID) if err != nil { return nil, fmt.Errorf("github-connectorv2: failed to get user: %w", err) } @@ -416,7 +417,7 @@ func (o *orgResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotati return nil, nil } -func orgBuilder(client, appClient *github.Client, orgCache *orgNameCache, orgs []string, syncSecrets bool) *orgResourceType { +func orgBuilder(client, appClient *github.Client, orgCache *orgNameCache, userCache *userDataCache, orgs []string, syncSecrets bool) *orgResourceType { orgMap := make(map[string]struct{}) for _, o := range orgs { @@ -429,6 +430,7 @@ func orgBuilder(client, appClient *github.Client, orgCache *orgNameCache, orgs [ client: client, appClient: appClient, orgCache: orgCache, + userCache: userCache, syncSecrets: syncSecrets, } } diff --git a/pkg/connector/org_role.go b/pkg/connector/org_role.go index f6450c0b..2d3c1787 100644 --- a/pkg/connector/org_role.go +++ b/pkg/connector/org_role.go @@ -40,6 +40,7 @@ type orgRoleResourceType struct { resourceType *v2.ResourceType client *github.Client orgCache *orgNameCache + userCache *userDataCache } func orgRoleResource( @@ -91,7 +92,7 @@ func (o *orgRoleResourceType) List( return nil, "", nil, nil } if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("failed to list organization roles: %w", err) } @@ -228,7 +229,7 @@ func (o *orgRoleResourceType) Grants( return nil, pageToken, nil, nil } if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("failed to list role teams: %w", err) } @@ -325,7 +326,7 @@ func (o *orgRoleResourceType) Grant(ctx context.Context, principal *v2.Resource, return nil, fmt.Errorf("github-connectorv2: invalid entitlement ID: %s", entitlement.Id) } - user, _, err := o.client.Users.GetByID(ctx, userID) + user, _, err := o.userCache.GetUser(ctx, userID) if err != nil { return nil, fmt.Errorf("failed to get user: %w", err) } @@ -400,7 +401,7 @@ func (o *orgRoleResourceType) Revoke(ctx context.Context, grant *v2.Grant) (anno return nil, fmt.Errorf("invalid user ID: %w", err) } - user, _, err := o.client.Users.GetByID(ctx, userID) + user, _, err := o.userCache.GetUser(ctx, userID) if err != nil { return nil, fmt.Errorf("failed to get user: %w", err) } @@ -423,10 +424,11 @@ func (o *orgRoleResourceType) Revoke(ctx context.Context, grant *v2.Grant) (anno return nil, nil } -func orgRoleBuilder(client *github.Client, orgCache *orgNameCache) *orgRoleResourceType { +func orgRoleBuilder(client *github.Client, orgCache *orgNameCache, userCache *userDataCache) *orgRoleResourceType { return &orgRoleResourceType{ resourceType: resourceTypeOrgRole, client: client, orgCache: orgCache, + userCache: userCache, } } diff --git a/pkg/connector/org_role_test.go b/pkg/connector/org_role_test.go index 3fc34ae4..bd292d75 100644 --- a/pkg/connector/org_role_test.go +++ b/pkg/connector/org_role_test.go @@ -24,7 +24,8 @@ func TestOrgRole(t *testing.T) { githubClient := github.NewClient(mgh.Server()) cache := newOrgNameCache(githubClient) - client := orgRoleBuilder(githubClient, cache) + userCache := newUserDataCache(githubClient) + client := orgRoleBuilder(githubClient, cache, userCache) organization, _ := organizationResource(ctx, githubOrganization, nil, true) roleResource, _ := orgRoleResource(ctx, &OrganizationRole{ @@ -89,7 +90,8 @@ func TestOrgRole(t *testing.T) { githubClient := github.NewClient(mockGithub.Server()) cache := newOrgNameCache(githubClient) - client := orgRoleBuilder(githubClient, cache) + userCache := newUserDataCache(githubClient) + client := orgRoleBuilder(githubClient, cache, userCache) organization, _ := organizationResource(ctx, githubOrganization, nil, true) diff --git a/pkg/connector/org_test.go b/pkg/connector/org_test.go index bb1a27da..34b17847 100644 --- a/pkg/connector/org_test.go +++ b/pkg/connector/org_test.go @@ -23,7 +23,8 @@ func TestOrganization(t *testing.T) { githubClient := github.NewClient(mgh.Server()) cache := newOrgNameCache(githubClient) - client := orgBuilder(githubClient, nil, cache, nil, false) + userCache := newUserDataCache(githubClient) + client := orgBuilder(githubClient, nil, cache, userCache, nil, false) organization, _ := organizationResource(ctx, githubOrganization, nil, false) user, _ := userResource(ctx, githubUser, *githubUser.Email, nil) diff --git a/pkg/connector/repository.go b/pkg/connector/repository.go index 12aeaed4..fb775f1d 100644 --- a/pkg/connector/repository.go +++ b/pkg/connector/repository.go @@ -60,6 +60,8 @@ type repositoryResourceType struct { resourceType *v2.ResourceType client *github.Client orgCache *orgNameCache + userCache *userDataCache + teamCache *teamDataCache } func (o *repositoryResourceType) ResourceType(_ context.Context) *v2.ResourceType { @@ -182,7 +184,7 @@ func (o *repositoryResourceType) Grants( return nil, "", nil, uhttp.WrapErrors(codes.NotFound, fmt.Sprintf("repo: %s not found", resource.DisplayName)) } if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("github-connector: failed to list repos: %w", err) } @@ -238,7 +240,7 @@ func (o *repositoryResourceType) Grants( } if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("github-connector: failed to list repos: %w", err) } @@ -319,7 +321,7 @@ func (o *repositoryResourceType) Grant(ctx context.Context, principal *v2.Resour switch principal.Id.ResourceType { case resourceTypeUser.Id: - user, _, err := o.client.Users.GetByID(ctx, principalID) + user, _, err := o.userCache.GetUser(ctx, principalID) if err != nil { return nil, fmt.Errorf("github-connectorv2: failed to get user: %w", err) } @@ -336,7 +338,7 @@ func (o *repositoryResourceType) Grant(ctx context.Context, principal *v2.Resour return nil, fmt.Errorf("github-connectorv2: failed to add user to a repository: %w", e) } case resourceTypeTeam.Id: - team, _, err := o.client.Teams.GetTeamByID(ctx, org.GetID(), principalID) //nolint:staticcheck // TODO: migrate to GetTeamBySlug + team, _, err := o.teamCache.GetTeam(ctx, org.GetID(), principalID) if err != nil { return nil, fmt.Errorf("github-connectorv2: failed to get team: %w", err) } @@ -384,7 +386,7 @@ func (o *repositoryResourceType) Revoke(ctx context.Context, grant *v2.Grant) (a switch principal.Id.ResourceType { case resourceTypeUser.Id: - user, _, err := o.client.Users.GetByID(ctx, principalID) + user, _, err := o.userCache.GetUser(ctx, principalID) if err != nil { return nil, fmt.Errorf("github-connectorv2: failed to get user: %w", err) } @@ -394,7 +396,7 @@ func (o *repositoryResourceType) Revoke(ctx context.Context, grant *v2.Grant) (a return nil, fmt.Errorf("github-connectorv2: failed to remove user from repo: %w", e) } case resourceTypeTeam.Id: - team, _, err := o.client.Teams.GetTeamByID(ctx, org.GetID(), principalID) //nolint:staticcheck // TODO: migrate to GetTeamBySlug + team, _, err := o.teamCache.GetTeam(ctx, org.GetID(), principalID) if err != nil { return nil, fmt.Errorf("github-connectorv2: failed to get team: %w", err) } @@ -415,11 +417,13 @@ func (o *repositoryResourceType) Revoke(ctx context.Context, grant *v2.Grant) (a return nil, nil } -func repositoryBuilder(client *github.Client, orgCache *orgNameCache) *repositoryResourceType { +func repositoryBuilder(client *github.Client, orgCache *orgNameCache, userCache *userDataCache, teamCache *teamDataCache) *repositoryResourceType { return &repositoryResourceType{ resourceType: resourceTypeRepository, client: client, orgCache: orgCache, + userCache: userCache, + teamCache: teamCache, } } diff --git a/pkg/connector/repository_test.go b/pkg/connector/repository_test.go index b6770bac..f939818e 100644 --- a/pkg/connector/repository_test.go +++ b/pkg/connector/repository_test.go @@ -24,7 +24,9 @@ func TestRepository(t *testing.T) { githubClient := github.NewClient(mgh.Server()) cache := newOrgNameCache(githubClient) - client := repositoryBuilder(githubClient, cache) + userCache := newUserDataCache(githubClient) + teamCache := newTeamDataCache(githubClient) + client := repositoryBuilder(githubClient, cache, userCache, teamCache) organization, _ := organizationResource(ctx, githubOrganization, nil, false) repository, _ := repositoryResource(ctx, githubRepository, organization.Id) diff --git a/pkg/connector/team.go b/pkg/connector/team.go index 88a13b03..a839aa34 100644 --- a/pkg/connector/team.go +++ b/pkg/connector/team.go @@ -60,6 +60,8 @@ type teamResourceType struct { resourceType *v2.ResourceType client *github.Client orgCache *orgNameCache + userCache *userDataCache + teamCache *teamDataCache } func (o *teamResourceType) ResourceType(_ context.Context) *v2.ResourceType { @@ -96,7 +98,7 @@ func (o *teamResourceType) List(ctx context.Context, parentID *v2.ResourceId, pt teams, resp, err := o.client.Teams.ListTeams(ctx, orgName, opts) if err != nil { if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("github-connector: failed to list teams: %w", err) } @@ -107,10 +109,10 @@ func (o *teamResourceType) List(ctx context.Context, parentID *v2.ResourceId, pt } for _, team := range teams { - fullTeam, resp, err := o.client.Teams.GetTeamByID(ctx, orgID, team.GetID()) //nolint:staticcheck // TODO: migrate to GetTeamBySlug + fullTeam, resp, err := o.teamCache.GetTeam(ctx, orgID, team.GetID()) if err != nil { if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, err } @@ -173,7 +175,7 @@ func (o *teamResourceType) Grants(ctx context.Context, resource *v2.Resource, pT org, resp, err := o.client.Organizations.GetByID(ctx, orgID) if err != nil { if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, err } @@ -212,7 +214,7 @@ func (o *teamResourceType) Grants(ctx context.Context, resource *v2.Resource, pT return nil, "", nil, uhttp.WrapErrors(codes.NotFound, fmt.Sprintf("org: %d not found", org.GetID())) } if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("github-connectorv2: failed to fetch team members: %w", err) } @@ -302,7 +304,7 @@ func (o *teamResourceType) Grant(ctx context.Context, principal *v2.Resource, en return nil, err } - user, _, err := o.client.Users.GetByID(ctx, userId) + user, _, err := o.userCache.GetUser(ctx, userId) if err != nil { return nil, fmt.Errorf("github-connectorv2: failed to get user %d, err: %w", userId, err) } @@ -362,7 +364,7 @@ func (o *teamResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotat return nil, err } - user, _, err := o.client.Users.GetByID(ctx, userId) + user, _, err := o.userCache.GetUser(ctx, userId) if err != nil { return nil, fmt.Errorf("github-connectorv2: failed to get user %d, err: %w", userId, err) } @@ -374,10 +376,12 @@ func (o *teamResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotat return nil, nil } -func teamBuilder(client *github.Client, orgCache *orgNameCache) *teamResourceType { +func teamBuilder(client *github.Client, orgCache *orgNameCache, userCache *userDataCache, teamCache *teamDataCache) *teamResourceType { return &teamResourceType{ resourceType: resourceTypeTeam, client: client, orgCache: orgCache, + userCache: userCache, + teamCache: teamCache, } } diff --git a/pkg/connector/team_test.go b/pkg/connector/team_test.go index d739b529..61e02c57 100644 --- a/pkg/connector/team_test.go +++ b/pkg/connector/team_test.go @@ -24,7 +24,9 @@ func TestTeam(t *testing.T) { githubClient := github.NewClient(mgh.Server()) cache := newOrgNameCache(githubClient) - client := teamBuilder(githubClient, cache) + userCache := newUserDataCache(githubClient) + teamCache := newTeamDataCache(githubClient) + client := teamBuilder(githubClient, cache, userCache, teamCache) organization, _ := organizationResource(ctx, githubOrganization, nil, false) team, _ := teamResource(githubTeam, organization.Id) diff --git a/pkg/connector/user.go b/pkg/connector/user.go index 98d9f887..2835ccb2 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -94,6 +94,7 @@ type userResourceType struct { graphqlClient *githubv4.Client hasSAMLEnabled *bool orgCache *orgNameCache + userCache *userDataCache orgs []string } @@ -134,7 +135,7 @@ func (o *userResourceType) List(ctx context.Context, parentID *v2.ResourceId, pt users, resp, err := o.client.Organizations.ListMembers(ctx, orgName, &opts) if err != nil { if isRatelimited(resp) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp.Response, err) } return nil, "", nil, fmt.Errorf("github-connector: ListMembers failed: %w", err) } @@ -157,10 +158,10 @@ func (o *userResourceType) List(ctx context.Context, parentID *v2.ResourceId, pt q := listUsersQuery{} rv := make([]*v2.Resource, 0, len(users)) for _, user := range users { - u, res, err := o.client.Users.GetByID(ctx, user.GetID()) + u, res, err := o.userCache.GetUser(ctx, user.GetID()) if err != nil { if isRatelimited(res) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, res.Response, err) } // This undocumented API can return 404 for some users. If this fails it means we won't get some of their details like email if res == nil || res.StatusCode != http.StatusNotFound { @@ -251,7 +252,7 @@ func (o *userResourceType) Delete(ctx context.Context, resourceId *v2.ResourceId return nil, fmt.Errorf("baton-github: invalid invitation id") } - u, _, err := o.client.Users.GetByID(ctx, userID) + u, _, err := o.userCache.GetUser(ctx, userID) if err != nil { return nil, fmt.Errorf("baton-github: invalid userID") } @@ -281,13 +282,14 @@ func (o *userResourceType) Delete(ctx context.Context, resourceId *v2.ResourceId return annotations, nil } -func userBuilder(client *github.Client, hasSAMLEnabled *bool, graphqlClient *githubv4.Client, orgCache *orgNameCache, orgs []string) *userResourceType { +func userBuilder(client *github.Client, hasSAMLEnabled *bool, graphqlClient *githubv4.Client, orgCache *orgNameCache, userCache *userDataCache, orgs []string) *userResourceType { return &userResourceType{ resourceType: resourceTypeUser, client: client, graphqlClient: graphqlClient, hasSAMLEnabled: hasSAMLEnabled, orgCache: orgCache, + userCache: userCache, orgs: orgs, } } diff --git a/pkg/connector/user_test.go b/pkg/connector/user_test.go index 09e970e7..456c2e26 100644 --- a/pkg/connector/user_test.go +++ b/pkg/connector/user_test.go @@ -44,11 +44,13 @@ func TestUsersList(t *testing.T) { githubClient := github.NewClient(mgh.Server()) graphQLClient := mocks.MockGraphQL() cache := newOrgNameCache(githubClient) + userCache := newUserDataCache(githubClient) client := userBuilder( githubClient, testCase.hasSamlEnabled, graphQLClient, cache, + userCache, []string{organization.DisplayName}, )