Skip to content
Closed
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 README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ baton resources
- Organization roles
- Invitations (users invited to an organization who have not accepted yet)
- GitHub Apps (installed in organizations, synced as non-human identities)
- Enterprise roles, only when `--enterprises` is set, and only on GitHub Enterprise Cloud — `github.com` or a `ghe.com` data-residency host. A personal access token syncs every role; a GitHub App syncs the built-in Enterprise Owner role and can also grant and revoke it, with the app installed on both the enterprise account and the organization
- Enterprise roles, only when `--enterprises` is set, and only on GitHub Enterprise Cloud — `github.com` or a `ghe.com` data-residency host. A personal access token syncs every role; a GitHub App syncs the built-in Enterprise Owner role and can also grant and revoke it, with the app installed on both the enterprise account and the organization. When this type is being synced, Owners, pending Owner invitees, and — with a token — anyone who holds an enterprise role are also synced as users, even when they are not members of the configured organization
- Enterprise licenses, only when `--enterprises` is set. The API behind them requires a personal access token; under GitHub App authentication the type answers 403, so leave it disabled on the app path

By default, `baton-github` will sync information from any organizations that the provided credential has Administrator permissions on. You can specify exactly which organizations you would like to sync using the `--orgs` flag.
Expand Down
18 changes: 15 additions & 3 deletions docs/docs-info.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,23 @@ C1 has no pending state for a grant, so an invitation nobody has accepted and an

An invitation sent to someone who is not a member of the enterprise is therefore invisible to the sync, and that is not hypothetical: an owner invitation can be addressed to any GitHub user. It is not enough that C1 has already synced the principal either — outside collaborators reach C1 through repository access without being enterprise members, so a grant to one of them would create an invitation on GitHub that no later sync could read, and C1 would drop the grant while it stayed live. Grant therefore checks membership first and rejects a non-member with `InvalidArgument` rather than creating state it cannot read back. The check is one filtered `Enterprise.members` lookup, and it compares the returned logins because that argument is a search rather than an exact match.

The cost of resolving invitations scales with the enterprise, not with the organization: every sync walks the whole enterprise membership, one page of 100 per request, and asks one aliased batch per page. An enterprise of N members therefore adds roughly N/50 GraphQL requests per sync — about 1,000 for 50,000 members. Memory is unaffected, since the walk streams through the page token, and none of it happens unless the capability is enabled.
The cost of resolving invitations scales with the enterprise, not with the organization: every sync walks the whole enterprise membership, one page of 100 per request, and asks one aliased batch per page. An enterprise of N members therefore adds roughly N/50 GraphQL requests per sync — about 1,000 for 50,000 members. The user sync walks those same owner and member pages once, so the principals exist as users; the grant sync walks them again. Memory is unaffected, since the walk streams through the page token, and none of it happens unless `--enterprises` is set.

### Owner grants can reference a user the sync did not emit
### Users named by an enterprise-role grant are synced

The `user` resource type is populated from the members of the **configured organization**, while `Organization.enterpriseOwners` returns owners of the whole **enterprise account** and the invitation candidates come from `Enterprise.members`, which spans every organization in it. An owner who belongs to a different organization in the same enterprise therefore produces a grant whose principal this sync never created. Widening the user sync is out of scope here — it would change the connector's user population for every deployment — so the grant is emitted and this limitation is recorded instead.
Organization members still come from the configured organization. When `--enterprises` is set **and the `enterprise_role` type is being synced**, the user sync also emits every principal an enterprise-role grant names who is not in that member list. The second condition is not automatic on a hosted tenant: `enterprise_role` is absent from the published capability set, so C1 leaves it out of a new connector's default selection and an admin enables it by hand, as described above. Until then there are no enterprise-role grants to resolve and the user population is unchanged.

Under GitHub App authentication the principals are the enterprise's Owners and the enterprise members with a pending Owner invitation; an enterprise member with neither is named by no grant and is left out.

Under a personal access token the set is far larger, and deliberately so. That path reads every enterprise role out of consumed licenses, and `Member` is the role GitHub reports for every license-consuming account, so the role sync emits a grant for each of them and each one needs a user. The principal set is therefore the whole licensed enterprise, not its administrators.

The resource ID is the GitHub database ID the grant already uses. These users gain no organization membership; like every other user this connector emits, they carry no parent resource. On the App path the enterprise query supplies the database ID, so an account whose lookup answers 404 is still emitted from that ID and its login. The token path has only a login to go on, so it cannot name the account and the sync fails instead, which is what the enterprise-role grant sync already does with the same lookup.

Each principal costs one `GET /orgs/{org}/members/{login}` per organization this sync emits members for, until one answers that it holds the account. That check is what keeps the walk from overwriting a member page: those pages resolve a user's SAML identity, the resource store keeps the last write, and emitting the account again from the enterprise would drop it. Which organizations count is decided by the same rule the organization sync applies — the ones the credential administers — because treating a principal as covered by an organization this sync leaves out would drop the user and put the grant back in the state this walk repairs. A configured list costs nothing to resolve. Without `--orgs` the connector reads the organizations once per page of enterprise principals, which is one walk of `/user/orgs` plus one membership read per organization.

The connector keeps no state between pages. Every configured organization runs the walk at the end of its members and repeating it emits the same records, so the result does not depend on which organization ran it or on state surviving a resumed sync. The cost of that choice is that a deployment naming several organizations walks the enterprise once per organization; GitHub App authentication serves one, so it walks once.

Budget for the token path before enabling `enterprise_role` on a large enterprise. Its principal set is the whole licensed enterprise, and each member costs the membership checks above plus one `GET /users/{login}`, on top of the one the role sync already spends per login. On an enterprise of tens of thousands of seats that is six figures of REST calls against a 5,000 per hour limit. The App path does not have this shape: its principal set is the Owners and the pending Owner invitees.

---

Expand Down
25 changes: 20 additions & 5 deletions pkg/connector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,26 @@ type GitHub struct {
enterprises []string
newEnterpriseRoleClients enterpriseClientProvider
syncLastActivity bool
connectorOpts *cli.ConnectorOpts
}

// willSyncResourceType is true when the type is in this sync. Nil opts, which
// is how the capabilities prototype is built, means no filter, so every
// advertised type is in scope.
func (gh *GitHub) willSyncResourceType(resourceTypeID string) bool {
if gh.connectorOpts == nil {
return true
}

return gh.connectorOpts.WillSyncResourceType(resourceTypeID)
}

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),
UserBuilder(gh.client, gh.graphqlClient, gh.orgCache, gh.orgs, gh.customClient, gh.enterprises,
gh.newEnterpriseRoleClients, gh.willSyncResourceType(resourceTypeEnterpriseRole.Id)),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low-confidence] The enterprise_role gate is resolved once, at construction, from cli.ConnectorOpts.SyncResourceTypeIDs. That field only carries the local --sync-resource-types flag (or the lambda effective config). In self-hosted daemon mode (--client-id), the per-task selection from C1 is applied later, inside the SDK syncer (c1api/full_sync.go: c.task.GetSyncFull().GetSyncResourceTypeIds() -> sdkSync.WithSyncResourceTypes), and never reaches ConnectorOpts. So SyncFilterIsExplicit() is false, WillSyncResourceType("enterprise_role") returns true, and with --enterprises set the user syncer walks the enterprise on every sync even when the admin has left enterprise_role deselected. On the PAT path that walk covers the whole licensed enterprise: one IsMember per org plus one Users.Get per seat, which the new docs put at six figures of REST calls on a large enterprise. It also adds users that no grant references. This contradicts the PR description's claim that the walk runs only when C1 selects the type. Please either derive the decision from something that reflects the actual sync, or make the walk opt-in explicitly (for example a config flag). At minimum, document that daemon-mode deployments always run the walk. Also confirm that hosted lambda deployments actually populate sync-resource-types from the UI selection; otherwise the same applies there.

RepositoryBuilder(gh.client, gh.orgCache, gh.omitArchivedRepositories, gh.directCollaboratorsOnly),
OrgRoleBuilder(gh.client, gh.orgCache),
InvitationBuilder(InvitationBuilderParams{
Expand Down Expand Up @@ -329,21 +342,21 @@ func NewLambdaConnector(ctx context.Context, ghc *cfg.Github, cliOpts *cli.Conne
err error
)
if group == cfg.GithubAppGroup {
cb, err = newWithGithubApp(ctx, ghc)
cb, err = newWithGithubApp(ctx, ghc, cliOpts)
if err != nil {
return nil, nil, err
}
return cb, nil, nil
}

cb, err = newWithGithubPAT(ctx, ghc)
cb, err = newWithGithubPAT(ctx, ghc, cliOpts)
if err != nil {
return nil, nil, err
}
return cb, nil, nil
}

func newWithGithubPAT(ctx context.Context, ghc *cfg.Github) (*GitHub, error) {
func newWithGithubPAT(ctx context.Context, ghc *cfg.Github, connectorOpts *cli.ConnectorOpts) (*GitHub, error) {
ts := oauth2.StaticTokenSource(
&oauth2.Token{AccessToken: ghc.Token},
)
Expand All @@ -367,6 +380,7 @@ func newWithGithubPAT(ctx context.Context, ghc *cfg.Github) (*GitHub, error) {
omitArchivedRepositories: ghc.OmitArchivedRepositories,
directCollaboratorsOnly: ghc.DirectCollaboratorsOnly,
syncLastActivity: ghc.SyncLastActivity,
connectorOpts: connectorOpts,
}, nil
}

Expand All @@ -384,7 +398,7 @@ func appPrivateKeyPEM(ghc *cfg.Github) (string, error) {
return "", errors.New("github app authentication requires either --app-privatekey or --app-privatekey-path")
}

func newWithGithubApp(ctx context.Context, ghc *cfg.Github) (*GitHub, error) {
func newWithGithubApp(ctx context.Context, ghc *cfg.Github, connectorOpts *cli.ConnectorOpts) (*GitHub, error) {
enterprises := distinctEnterprises(ghc.Enterprises)
privateKey, err := appPrivateKeyPEM(ghc)
if err != nil {
Expand Down Expand Up @@ -485,6 +499,7 @@ func newWithGithubApp(ctx context.Context, ghc *cfg.Github) (*GitHub, error) {
omitArchivedRepositories: ghc.OmitArchivedRepositories,
directCollaboratorsOnly: ghc.DirectCollaboratorsOnly,
syncLastActivity: ghc.SyncLastActivity,
connectorOpts: connectorOpts,
}
return gh, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/connector/default_capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (d *DefaultCapabilitiesBuilder) ResourceSyncers(_ context.Context) []connec
return []connectorbuilder.ResourceSyncerV2{
OrgBuilder(nil, nil, nil, nil, false),
TeamBuilder(nil, nil, false),
UserBuilder(nil, nil, nil, nil, nil, nil),
UserBuilder(nil, nil, nil, nil, nil, nil, nil, false),
RepositoryBuilder(nil, nil, false, false),
OrgRoleBuilder(nil, nil),
InvitationBuilder(InvitationBuilderParams{}),
Expand Down
5 changes: 1 addition & 4 deletions pkg/connector/enterprise_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,10 +421,7 @@ func (o *enterpriseRoleResourceType) appGrants(
bag.Push(pagination.PageState{ResourceTypeID: enterpriseOwnersPhase})
}

var after *githubv4.String
if cursor := bag.PageToken(); cursor != "" {
after = githubv4.NewString(githubv4.String(cursor))
}
after := graphqlCursor(bag.PageToken())

var (
ret []*v2.Grant
Expand Down
Loading
Loading