Skip to content
Open
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, hasSAMLEnabled *bool, orgCache *orgNameCache) *apiTokenResourceType {
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.

is hasSamlEnabled useless today?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removed in 9a926e8. It was actually dead in apiTokenBuilder even before this PR (accepted but never stored or used). On the user side, it's replaced by the per-org samlStates map and checkOrgSAML

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.

Just to be clear have we recently introduced something the customer will have to change in order to continue to have their sync succeed? If so we cannot ship without customer success's help.

func apiTokenBuilder(client *github.Client, orgCache *orgNameCache) *apiTokenResourceType {
return &apiTokenResourceType{
resourceType: resourceTypeApiToken,
client: client,
Expand Down
57 changes: 41 additions & 16 deletions pkg/connector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,24 +94,32 @@ var (
)

type GitHub struct {
orgs []string
client *github.Client
appClient *github.Client
customClient *customclient.Client
instanceURL string
graphqlClient *githubv4.Client
hasSAMLEnabled *bool
orgCache *orgNameCache
syncSecrets bool
omitArchivedRepositories bool
enterprises []string
orgs []string
client *github.Client
appClient *github.Client
customClient *customclient.Client
instanceURL string
graphqlClient *githubv4.Client
orgCache *orgNameCache
syncSecrets bool
omitArchivedRepositories bool
enterprises []string
enterpriseLicensesAvailable bool // set during Validate; guards enterprise role sync and SAML enrichment
}

func (gh *GitHub) ResourceSyncers(ctx context.Context) []connectorbuilder.ResourceSyncerV2 {
// Only pass enterprises to builders that need them if the consumed-licenses
// API is confirmed accessible. Otherwise, skip enterprise SAML enrichment
// and enterprise role sync gracefully.
var activeEnterprises []string
if gh.enterpriseLicensesAvailable {
activeEnterprises = gh.enterprises
}

resourceSyncers := []connectorbuilder.ResourceSyncerV2{
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, gh.customClient, gh.enterprises),
userBuilder(gh.client, gh.graphqlClient, gh.orgCache, gh.orgs, gh.customClient, activeEnterprises),
repositoryBuilder(gh.client, gh.orgCache, gh.omitArchivedRepositories),
orgRoleBuilder(gh.client, gh.orgCache),
invitationBuilder(invitationBuilderParams{
Expand All @@ -122,11 +130,11 @@ func (gh *GitHub) ResourceSyncers(ctx context.Context) []connectorbuilder.Resour
}

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

if len(gh.enterprises) > 0 {
resourceSyncers = append(resourceSyncers, enterpriseRoleBuilder(gh.client, gh.customClient, gh.enterprises))
if len(activeEnterprises) > 0 {
resourceSyncers = append(resourceSyncers, enterpriseRoleBuilder(gh.client, gh.customClient, activeEnterprises))
}
return resourceSyncers
}
Expand Down Expand Up @@ -210,9 +218,13 @@ func (gh *GitHub) Validate(ctx context.Context) (annotations.Annotations, error)
}

if len(gh.enterprises) > 0 {
l := ctxzap.Extract(ctx)
_, _, err := gh.customClient.ListEnterpriseConsumedLicenses(ctx, gh.enterprises[0], 1)
if err != nil {
return nil, uhttp.WrapErrors(codes.PermissionDenied, "github-connector: failed to access enterprise licenses", err)
l.Warn("failed to access enterprise consumed licenses — enterprise SAML email enrichment and enterprise role sync will be skipped",
zap.Error(err))
} else {
gh.enterpriseLicensesAvailable = true
}
}
return nil, nil
Expand All @@ -228,6 +240,19 @@ func (gh *GitHub) validateAppCredentials(ctx context.Context) (annotations.Annot
if err != nil {
return nil, err
}

if len(gh.enterprises) > 0 {
l := ctxzap.Extract(ctx)
_, _, err := gh.customClient.ListEnterpriseConsumedLicenses(ctx, gh.enterprises[0], 1)
if err != nil {
l.Warn("failed to access enterprise consumed licenses — enterprise SAML email enrichment and enterprise role sync will be skipped"+
" (GitHub App installations cannot access this endpoint — use a PAT with enterprise admin scope)",
zap.Error(err))
} else {
gh.enterpriseLicensesAvailable = true
}
}

return nil, nil
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/connector/enterprise_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ func (o *enterpriseRoleResourceType) getRoleUsersCache(ctx context.Context) (map

func (o *enterpriseRoleResourceType) fillCache(ctx context.Context) error {
for _, enterprise := range o.enterprises {
page := 0
// GitHub's consumed-licenses API is 1-indexed; page 0 is undocumented
// and may return the same results as page 1, causing duplicates.
page := 1
continuePagination := true
for continuePagination {
consumedLicenses, _, err := o.customClient.ListEnterpriseConsumedLicenses(ctx, enterprise, page)
Expand Down
3 changes: 2 additions & 1 deletion pkg/connector/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ type listUsersQuery struct {
type hasSAMLQuery struct {
Organization struct {
SamlIdentityProvider struct {
Id string
Id string
SsoUrl githubv4.String
}
} `graphql:"organization(login: $orgLoginName)"`
}
Expand Down
Loading
Loading