Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/connector/api_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (o *apiTokenResourceType) List(
}, nil
}

func apiTokenBuilder(client *github.Client, orgCache *orgNameCache) *apiTokenResourceType {
func APITokenBuilder(client *github.Client, orgCache *orgNameCache) *apiTokenResourceType {
return &apiTokenResourceType{
resourceType: resourceTypeApiToken,
client: client,
Expand Down
18 changes: 9 additions & 9 deletions pkg/connector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,26 +117,26 @@ type GitHub struct {

func (gh *GitHub) ResourceSyncers(ctx context.Context) []connectorbuilder.ResourceSyncerV2 {
resourceSyncers := []connectorbuilder.ResourceSyncerV2{
orgBuilder(gh.client, gh.appClient, gh.orgCache, gh.orgs, gh.syncSecrets),
teamBuilder(gh.client, gh.orgCache, gh.directCollaboratorsOnly),
userBuilder(gh.client, gh.graphqlClient, gh.orgCache, gh.orgs, gh.customClient, gh.enterprises),
repositoryBuilder(gh.client, gh.orgCache, gh.omitArchivedRepositories, gh.directCollaboratorsOnly),
orgRoleBuilder(gh.client, gh.orgCache),
invitationBuilder(invitationBuilderParams{
OrgBuilder(gh.client, gh.appClient, gh.orgCache, gh.orgs, gh.syncSecrets),
TeamBuilder(gh.client, gh.orgCache, gh.directCollaboratorsOnly),
UserBuilder(gh.client, gh.graphqlClient, gh.orgCache, gh.orgs, gh.customClient, gh.enterprises),
RepositoryBuilder(gh.client, gh.orgCache, gh.omitArchivedRepositories, gh.directCollaboratorsOnly),
OrgRoleBuilder(gh.client, gh.orgCache),
InvitationBuilder(InvitationBuilderParams{
client: gh.client,
orgCache: gh.orgCache,
orgs: gh.orgs,
}),
}

if gh.syncSecrets {
resourceSyncers = append(resourceSyncers, apiTokenBuilder(gh.client, gh.orgCache))
resourceSyncers = append(resourceSyncers, APITokenBuilder(gh.client, gh.orgCache))
}

if len(gh.enterprises) > 0 {
resourceSyncers = append(resourceSyncers,
enterpriseRoleBuilder(gh.client, gh.appClient, gh.customClient, gh.enterprises),
licenseBuilder(gh.customClient, gh.enterprises),
EnterpriseRoleBuilder(gh.client, gh.appClient, gh.customClient, gh.enterprises),
LicenseBuilder(gh.customClient, gh.enterprises),
)
}
return resourceSyncers
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/enterprise_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (o *enterpriseRoleResourceType) Grants(
return ret, &resourceSdk.SyncOpResults{}, nil
}

func enterpriseRoleBuilder(client *github.Client, appClient *github.Client, customClient *customclient.Client, enterprises []string) *enterpriseRoleResourceType {
func EnterpriseRoleBuilder(client *github.Client, appClient *github.Client, customClient *customclient.Client, enterprises []string) *enterpriseRoleResourceType {
return &enterpriseRoleResourceType{
resourceType: resourceTypeEnterpriseRole,
client: client,
Expand Down
4 changes: 2 additions & 2 deletions pkg/connector/invitation.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,13 +520,13 @@ func containsLower(s, substr string) bool {
return strings.Contains(strings.ToLower(s), substr)
}

type invitationBuilderParams struct {
type InvitationBuilderParams struct {
client *github.Client
orgCache *orgNameCache
orgs []string
}
Comment on lines +523 to 527
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: InvitationBuilderParams is now exported, but all its fields (client, orgCache, orgs) are still unexported. External packages can reference the type but cannot construct a usable value. Either export the fields or keep the struct unexported.


func invitationBuilder(p invitationBuilderParams) *invitationResourceType {
func InvitationBuilder(p InvitationBuilderParams) *invitationResourceType {
return &invitationResourceType{
client: p.client,
orgCache: p.orgCache,
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/invitation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func TestInvitationListPagination(t *testing.T) {

newBuilder := func(httpClient *http.Client) *invitationResourceType {
gh := github.NewClient(httpClient)
return invitationBuilder(invitationBuilderParams{
return InvitationBuilder(InvitationBuilderParams{
client: gh,
orgCache: newOrgNameCache(gh),
orgs: []string{invitationTestOrgLogin},
Expand Down
14 changes: 1 addition & 13 deletions pkg/connector/license.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ 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/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
"go.uber.org/zap"
)

const (
Expand All @@ -37,23 +35,13 @@ func (l *licenseResourceType) List(
parentID *v2.ResourceId,
opts resourceSdk.SyncOpAttrs,
) ([]*v2.Resource, *resourceSdk.SyncOpResults, error) {
logger := ctxzap.Extract(ctx)

var ret []*v2.Resource
for _, enterprise := range l.enterprises {
// total_seats_purchased and total_seats_consumed are enterprise-wide
// aggregates that the API returns identically on every page, so the
// first page is enough.
consumedLicenses, _, err := l.customClient.ListEnterpriseConsumedLicenses(ctx, enterprise, 1)
if err != nil {
if isPermissionDenied(err) {
logger.Debug("baton-github: enterprise features (--enterprises) require a Personal Access Token. "+
"GitHub App authentication cannot access the consumed-licenses API. "+
"Either switch to PAT auth or remove the --enterprises flag.",
zap.String("enterprise", enterprise),
zap.Error(err))
continue
}
return nil, nil, fmt.Errorf("baton-github: error listing enterprise consumed licenses for %s: %w", enterprise, err)
Comment on lines 44 to 45
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: The removed isPermissionDenied guard provided graceful degradation for GitHub App auth users who configure --enterprises (the consumed-licenses API requires a PAT). Without it, the entire sync will now fail instead of skipping license data. Per R7, consider keeping the graceful degradation or gating this behind a config flag.

}

Expand Down Expand Up @@ -162,7 +150,7 @@ func licenseResource(enterprise string, purchasedSeats, consumedSeats int64, ent
)
}

func licenseBuilder(
func LicenseBuilder(
customClient *customclient.Client,
enterprises []string,
) *licenseResourceType {
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,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, orgs []string, syncSecrets bool) *orgResourceType {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: All exported builder functions (e.g. OrgBuilder, TeamBuilder, UserBuilder, etc.) accept the unexported *orgNameCache parameter, and return unexported concrete types (e.g. *orgResourceType). External callers cannot construct orgNameCache (constructor newOrgNameCache is also unexported) and cannot name the return type. If these are meant to be usable outside the package, the shared dependencies and return types (or their constructors) need to be exported too. If they're only meant for internal use, consider keeping them unexported.

orgMap := make(map[string]struct{})

for _, o := range orgs {
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/org_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ 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) *orgRoleResourceType {
return &orgRoleResourceType{
resourceType: resourceTypeOrgRole,
client: client,
Expand Down
4 changes: 2 additions & 2 deletions pkg/connector/org_role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestOrgRole(t *testing.T) {

githubClient := github.NewClient(mgh.Server())
cache := newOrgNameCache(githubClient)
client := orgRoleBuilder(githubClient, cache)
client := OrgRoleBuilder(githubClient, cache)

organization, _ := organizationResource(ctx, githubOrganization, nil, true)
roleResource, _ := orgRoleResource(ctx, &OrganizationRole{
Expand Down Expand Up @@ -93,7 +93,7 @@ func TestOrgRole(t *testing.T) {

githubClient := github.NewClient(mockGithub.Server())
cache := newOrgNameCache(githubClient)
client := orgRoleBuilder(githubClient, cache)
client := OrgRoleBuilder(githubClient, cache)

organization, _ := organizationResource(ctx, githubOrganization, nil, true)

Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestOrganization(t *testing.T) {

githubClient := github.NewClient(mgh.Server())
cache := newOrgNameCache(githubClient)
client := orgBuilder(githubClient, nil, cache, nil, false)
client := OrgBuilder(githubClient, nil, cache, nil, false)

organization, _ := organizationResource(ctx, githubOrganization, nil, false)
user, _ := userResource(ctx, githubUser, *githubUser.Email, nil)
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ func orgBasePermissionToRepoPermissions(basePerm string) []string {
}
}

func repositoryBuilder(client *github.Client, orgCache *orgNameCache, omitArchivedRepositories bool, directCollaboratorsOnly bool) *repositoryResourceType {
func RepositoryBuilder(client *github.Client, orgCache *orgNameCache, omitArchivedRepositories bool, directCollaboratorsOnly bool) *repositoryResourceType {
return &repositoryResourceType{
resourceType: resourceTypeRepository,
client: client,
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestRepository(t *testing.T) {

githubClient := github.NewClient(mgh.Server())
cache := newOrgNameCache(githubClient)
client := repositoryBuilder(githubClient, cache, false, false)
client := RepositoryBuilder(githubClient, cache, false, false)

organization, _ := organizationResource(ctx, githubOrganization, nil, false)
repository, _ := repositoryResource(ctx, githubRepository, organization.Id)
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ func (o *teamResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotat
return nil, nil
}

func teamBuilder(client *github.Client, orgCache *orgNameCache, directCollaboratorsOnly bool) *teamResourceType {
func TeamBuilder(client *github.Client, orgCache *orgNameCache, directCollaboratorsOnly bool) *teamResourceType {
return &teamResourceType{
resourceType: resourceTypeTeam,
client: client,
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/team_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestTeam(t *testing.T) {

githubClient := github.NewClient(mgh.Server())
cache := newOrgNameCache(githubClient)
client := teamBuilder(githubClient, cache, false)
client := TeamBuilder(githubClient, cache, false)

organization, _ := organizationResource(ctx, githubOrganization, nil, false)
team, _ := teamResource(githubTeam, githubOrganization.GetID(), organization.Id)
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func (u *userResourceType) Delete(ctx context.Context, resourceId *v2.ResourceId
return annotations, nil
}

func userBuilder(client *github.Client, graphqlClient *githubv4.Client, orgCache *orgNameCache, orgs []string, customClient *customclient.Client, enterprises []string) *userResourceType {
func UserBuilder(client *github.Client, graphqlClient *githubv4.Client, orgCache *orgNameCache, orgs []string, customClient *customclient.Client, enterprises []string) *userResourceType {
return &userResourceType{
resourceType: resourceTypeUser,
client: client,
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestUsersList(t *testing.T) {
githubClient := github.NewClient(mgh.Server())
graphQLClient := mocks.MockGraphQL()
cache := newOrgNameCache(githubClient)
client := userBuilder(
client := UserBuilder(
githubClient,
graphQLClient,
cache,
Expand Down
Loading