From ca193154b39b9db6eb86a3a8aa61f273af0bc741 Mon Sep 17 00:00:00 2001 From: subencheng Date: Thu, 24 Jul 2025 13:55:22 -0700 Subject: [PATCH 1/6] [INC-300] baton-github: include the error in the returned message --- pkg/connector/connector.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 9f131628..2e908559 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -180,7 +180,7 @@ func (gh *GitHub) Validate(ctx context.Context) (annotations.Annotations, error) membership, _, err := gh.client.Organizations.GetOrgMembership(ctx, "", o) if err != nil { if filterOrgs { - return nil, fmt.Errorf("access token must be an admin on the %s organization", o) + return nil, fmt.Errorf("access token must be an admin on the %s organization: %w", o, err) } continue } @@ -188,7 +188,7 @@ func (gh *GitHub) Validate(ctx context.Context) (annotations.Annotations, error) // Only sync orgs that we are an admin for if strings.ToLower(membership.GetRole()) != orgRoleAdmin { if filterOrgs { - return nil, fmt.Errorf("access token must be an admin on the %s organization", o) + return nil, fmt.Errorf("access token must be an admin on the %s organization: %w", o, err) } continue } From b9eed4029553e4f88f7b88d838bd05918a7ace96 Mon Sep 17 00:00:00 2001 From: subencheng Date: Thu, 24 Jul 2025 13:59:12 -0700 Subject: [PATCH 2/6] now --- pkg/connector/connector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 2e908559..84c9e3b1 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -188,7 +188,7 @@ func (gh *GitHub) Validate(ctx context.Context) (annotations.Annotations, error) // Only sync orgs that we are an admin for if strings.ToLower(membership.GetRole()) != orgRoleAdmin { if filterOrgs { - return nil, fmt.Errorf("access token must be an admin on the %s organization: %w", o, err) + return nil, fmt.Errorf("access token must be an admin on the %s organization", o) } continue } From cdc712e55fc8327b382c99e1ab6f3e24e95e5d27 Mon Sep 17 00:00:00 2001 From: subencheng Date: Thu, 24 Jul 2025 14:48:54 -0700 Subject: [PATCH 3/6] add isRatelimited to All list methods --- pkg/connector/api_token.go | 5 +++++ pkg/connector/connector.go | 4 ++++ pkg/connector/enterprise_role.go | 7 ++++++- pkg/connector/org.go | 7 +++++++ pkg/connector/org_role.go | 10 +++++++++- pkg/connector/repository.go | 7 +++++++ pkg/connector/team.go | 13 +++++++++++-- pkg/connector/user.go | 8 ++++++++ 8 files changed, 57 insertions(+), 4 deletions(-) diff --git a/pkg/connector/api_token.go b/pkg/connector/api_token.go index 601d50ca..713e5e8b 100644 --- a/pkg/connector/api_token.go +++ b/pkg/connector/api_token.go @@ -8,7 +8,9 @@ import ( "github.com/conductorone/baton-sdk/pkg/annotations" "github.com/conductorone/baton-sdk/pkg/pagination" resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/conductorone/baton-sdk/pkg/uhttp" "github.com/google/go-github/v69/github" + "google.golang.org/grpc/codes" ) func apiTokenResource(ctx context.Context, token *github.PersonalAccessToken) (*v2.Resource, error) { @@ -92,6 +94,9 @@ func (o *apiTokenResourceType) List( }, }) if err != nil { + if isRatelimited(resp) { + return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + } return nil, "", nil, err } diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 84c9e3b1..17bf1e07 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -171,6 +171,7 @@ func (gh *GitHub) Validate(ctx context.Context) (annotations.Annotations, error) var err error orgLogins, err = getOrgs(ctx, gh.client, orgLogins) if err != nil { + if err. return nil, err } } @@ -450,6 +451,9 @@ func getOrgs(ctx context.Context, client *github.Client, orgs []string) ([]strin for { orgs, resp, err := client.Organizations.List(ctx, "", &github.ListOptions{Page: page, PerPage: maxPageSize}) if err != nil { + if isRatelimited(resp) { + return nil, nil + } return nil, fmt.Errorf("github-connector: failed to retrieve org: %w", err) } if resp.StatusCode == http.StatusUnauthorized { diff --git a/pkg/connector/enterprise_role.go b/pkg/connector/enterprise_role.go index dff737d6..06ee88c0 100644 --- a/pkg/connector/enterprise_role.go +++ b/pkg/connector/enterprise_role.go @@ -13,7 +13,9 @@ import ( "github.com/conductorone/baton-sdk/pkg/types/entitlement" "github.com/conductorone/baton-sdk/pkg/types/grant" resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/conductorone/baton-sdk/pkg/uhttp" "github.com/google/go-github/v69/github" + "google.golang.org/grpc/codes" ) type enterpriseRoleResourceType struct { @@ -137,8 +139,11 @@ func (o *enterpriseRoleResourceType) Grants( ret := []*v2.Grant{} for _, userLogin := range cache[resource.Id.Resource] { - user, _, err := o.client.Users.Get(ctx, userLogin) + user, resp, err := o.client.Users.Get(ctx, userLogin) if err != nil { + if isRatelimited(resp) { + return ret, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + } return nil, "", nil, fmt.Errorf("baton-github: error getting user %s: %w", userLogin, err) } diff --git a/pkg/connector/org.go b/pkg/connector/org.go index c3614b10..16a41aac 100644 --- a/pkg/connector/org.go +++ b/pkg/connector/org.go @@ -102,6 +102,9 @@ 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, fmt.Errorf("github-connector: failed to fetch org: %w", err) } @@ -126,6 +129,10 @@ func (o *orgResourceType) List( l.Warn("insufficient access to list org membership, skipping org", zap.String("org", org.GetLogin())) continue } + + if isRatelimited(resp) { + return ret, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + } return nil, "", nil, err } diff --git a/pkg/connector/org_role.go b/pkg/connector/org_role.go index 629d9f68..abefd4be 100644 --- a/pkg/connector/org_role.go +++ b/pkg/connector/org_role.go @@ -13,9 +13,11 @@ import ( "github.com/conductorone/baton-sdk/pkg/types/entitlement" "github.com/conductorone/baton-sdk/pkg/types/grant" "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/conductorone/baton-sdk/pkg/uhttp" "github.com/google/go-github/v69/github" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" "go.uber.org/zap" + "google.golang.org/grpc/codes" ) type OrganizationRole struct { @@ -88,6 +90,9 @@ func (o *orgRoleResourceType) List( // Return empty list with no error to indicate we skipped this resource return nil, "", nil, nil } + if isRatelimited(resp) { + uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + } return nil, "", nil, fmt.Errorf("failed to list organization roles: %w", err) } @@ -168,7 +173,7 @@ func (o *orgRoleResourceType) Grants( } users, resp, err := o.client.Organizations.ListUsersAssignedToOrgRole(ctx, orgName, roleID, opts) if err != nil { - if resp != nil && (resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound) { + if isRatelimited(resp) || isNotFoundError(resp) { pageToken, err := bag.NextToken("") if err != nil { return nil, "", nil, err @@ -222,6 +227,9 @@ 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, fmt.Errorf("failed to list role teams: %w", err) } diff --git a/pkg/connector/repository.go b/pkg/connector/repository.go index 3bae644f..12aeaed4 100644 --- a/pkg/connector/repository.go +++ b/pkg/connector/repository.go @@ -181,6 +181,9 @@ func (o *repositoryResourceType) Grants( if isNotFoundError(resp) { 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, fmt.Errorf("github-connector: failed to list repos: %w", err) } @@ -233,6 +236,10 @@ func (o *repositoryResourceType) Grants( if isNotFoundError(resp) { 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, fmt.Errorf("github-connector: failed to list repos: %w", err) } diff --git a/pkg/connector/team.go b/pkg/connector/team.go index 6775bae6..88a13b03 100644 --- a/pkg/connector/team.go +++ b/pkg/connector/team.go @@ -95,6 +95,9 @@ 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, fmt.Errorf("github-connector: failed to list teams: %w", err) } @@ -104,8 +107,11 @@ func (o *teamResourceType) List(ctx context.Context, parentID *v2.ResourceId, pt } for _, team := range teams { - fullTeam, _, err := o.client.Teams.GetTeamByID(ctx, orgID, team.GetID()) //nolint:staticcheck // TODO: migrate to GetTeamBySlug + fullTeam, resp, err := o.client.Teams.GetTeamByID(ctx, orgID, team.GetID()) //nolint:staticcheck // TODO: migrate to GetTeamBySlug if err != nil { + if isRatelimited(resp) { + return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + } return nil, "", nil, err } @@ -164,8 +170,11 @@ func (o *teamResourceType) Grants(ctx context.Context, resource *v2.Resource, pT return nil, "", nil, fmt.Errorf("error fetching orgID from team profile") } - org, _, err := o.client.Organizations.GetByID(ctx, orgID) + 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, err } diff --git a/pkg/connector/user.go b/pkg/connector/user.go index e698c0b2..b05255b4 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -12,10 +12,12 @@ import ( "github.com/conductorone/baton-sdk/pkg/annotations" "github.com/conductorone/baton-sdk/pkg/pagination" "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/conductorone/baton-sdk/pkg/uhttp" "github.com/google/go-github/v69/github" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" "github.com/shurcooL/githubv4" "go.uber.org/zap" + "google.golang.org/grpc/codes" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -131,6 +133,9 @@ 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, fmt.Errorf("github-connector: ListMembers failed: %w", err) } @@ -158,6 +163,9 @@ func (o *userResourceType) List(ctx context.Context, parentID *v2.ResourceId, pt if res == nil || res.StatusCode != http.StatusNotFound { return nil, "", nil, err } + if isRatelimited(resp) { + return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + } l.Error("error fetching user by id", zap.Error(err), zap.Int64("user_id", user.GetID())) u = user } From 4239b2bd5652717d0a0f523e69e728a435b1f4c6 Mon Sep 17 00:00:00 2001 From: subencheng Date: Thu, 24 Jul 2025 14:53:14 -0700 Subject: [PATCH 4/6] more --- pkg/connector/connector.go | 1 - pkg/connector/org_role.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 17bf1e07..681458f4 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -171,7 +171,6 @@ func (gh *GitHub) Validate(ctx context.Context) (annotations.Annotations, error) var err error orgLogins, err = getOrgs(ctx, gh.client, orgLogins) if err != nil { - if err. return nil, err } } diff --git a/pkg/connector/org_role.go b/pkg/connector/org_role.go index abefd4be..d3777104 100644 --- a/pkg/connector/org_role.go +++ b/pkg/connector/org_role.go @@ -173,7 +173,7 @@ func (o *orgRoleResourceType) Grants( } users, resp, err := o.client.Organizations.ListUsersAssignedToOrgRole(ctx, orgName, roleID, opts) if err != nil { - if isRatelimited(resp) || isNotFoundError(resp) { + if resp != nil && (resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound) { pageToken, err := bag.NextToken("") if err != nil { return nil, "", nil, err From 0eaff5ba16786a20dc7801f3a4bbddefd28e41e4 Mon Sep 17 00:00:00 2001 From: subencheng Date: Thu, 24 Jul 2025 15:21:53 -0700 Subject: [PATCH 5/6] address comments --- pkg/connector/connector.go | 2 +- pkg/connector/org.go | 2 +- pkg/connector/org_role.go | 2 +- pkg/connector/user.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 681458f4..efe6f68e 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -451,7 +451,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, nil + return nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) } return nil, fmt.Errorf("github-connector: failed to retrieve org: %w", err) } diff --git a/pkg/connector/org.go b/pkg/connector/org.go index 16a41aac..cc2eec7e 100644 --- a/pkg/connector/org.go +++ b/pkg/connector/org.go @@ -131,7 +131,7 @@ func (o *orgResourceType) List( } if isRatelimited(resp) { - return ret, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) } return nil, "", nil, err } diff --git a/pkg/connector/org_role.go b/pkg/connector/org_role.go index d3777104..f6450c0b 100644 --- a/pkg/connector/org_role.go +++ b/pkg/connector/org_role.go @@ -91,7 +91,7 @@ func (o *orgRoleResourceType) List( return nil, "", nil, nil } if isRatelimited(resp) { - uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) } return nil, "", nil, fmt.Errorf("failed to list organization roles: %w", err) } diff --git a/pkg/connector/user.go b/pkg/connector/user.go index b05255b4..c6b0a80f 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -163,7 +163,7 @@ func (o *userResourceType) List(ctx context.Context, parentID *v2.ResourceId, pt if res == nil || res.StatusCode != http.StatusNotFound { return nil, "", nil, err } - if isRatelimited(resp) { + if isRatelimited(res) { return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) } l.Error("error fetching user by id", zap.Error(err), zap.Int64("user_id", user.GetID())) From 2c8e33f1dfe329822b59eae937afc686f67c133c Mon Sep 17 00:00:00 2001 From: subencheng Date: Thu, 24 Jul 2025 15:29:28 -0700 Subject: [PATCH 6/6] cursor --- pkg/connector/enterprise_role.go | 2 +- pkg/connector/user.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/connector/enterprise_role.go b/pkg/connector/enterprise_role.go index 06ee88c0..7da06449 100644 --- a/pkg/connector/enterprise_role.go +++ b/pkg/connector/enterprise_role.go @@ -142,7 +142,7 @@ func (o *enterpriseRoleResourceType) Grants( user, resp, err := o.client.Users.Get(ctx, userLogin) if err != nil { if isRatelimited(resp) { - return ret, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) + return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) } return nil, "", nil, fmt.Errorf("baton-github: error getting user %s: %w", userLogin, err) } diff --git a/pkg/connector/user.go b/pkg/connector/user.go index c6b0a80f..98d9f887 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -159,13 +159,13 @@ func (o *userResourceType) List(ctx context.Context, parentID *v2.ResourceId, pt for _, user := range users { u, res, err := o.client.Users.GetByID(ctx, user.GetID()) if err != nil { + if isRatelimited(res) { + return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", 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 { return nil, "", nil, err } - if isRatelimited(res) { - return nil, "", nil, uhttp.WrapErrors(codes.Unavailable, "too many requests", err) - } l.Error("error fetching user by id", zap.Error(err), zap.Int64("user_id", user.GetID())) u = user }