Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Add 'internal' case for repositoryQuery#62266

Merged
pjlast merged 7 commits into
mainfrom
pjlast/61924-ghe-internal-repos-support
May 2, 2024
Merged

Add 'internal' case for repositoryQuery#62266
pjlast merged 7 commits into
mainfrom
pjlast/61924-ghe-internal-repos-support

Conversation

@pjlast

@pjlast pjlast commented Apr 30, 2024

Copy link
Copy Markdown
Contributor

Closes #61924

Docs update: sourcegraph/docs#286

Adds 'internal' as a special case option for the repositoryQuery field on GitHub code host connections. This query will fetch all orgs the user of the PAT belongs to and construct a search query consisting of org:org1 org:org2 ... is:internal.

A search query is used instead of the standard org/repos endpoint, because for a large amount of orgs it is much faster to fetch all of the repositories with the search API than to hit the /repos endpoint for each individual org.

Test plan

Test added for "repositoryQuery": ["internal"]

@cla-bot cla-bot Bot added the cla-signed label Apr 30, 2024
@github-actions github-actions Bot added team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all labels Apr 30, 2024
@pjlast pjlast requested a review from a team April 30, 2024 11:50

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, we talked through this on a call. In a next step, we want to announce that public will change behavior in a future version and that internal should be added as well, when public was used before.

Comment thread internal/repos/github.go Outdated
Comment thread internal/repos/github.go

for _, org := range orgs {
sb.WriteString("org:")
sb.WriteString(org.Login)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I hope those can't contain spaces or colons themselves 😬

Comment thread internal/repos/github.go
// a search query. This leads to much fewer requests to the GitHub API.
func (s *GitHubSource) listInternal(ctx context.Context, results chan *githubResult) {
orgs, err := fetchAll(func(page int) (items []*github.Org, hasNext bool, cost int, err error) {
return s.v3Client.GetAuthenticatedUserOrgs(ctx, page)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm assuming that works with GitHub App installations as well, where only a subset of orgs may be granted access to?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

GitHub App installations are tied to a single org/user. So this wouldn't have worked. I added a special case in case we're dealing with a github app, although it would be more efficient for the admin to add a repositoryQuery with org:org-name is:internal than any options the GitHub App installations API gives us.

Actually, perhaps that's what we should do 🤔 return an error asking them to do that

@mmanela

mmanela commented Apr 30, 2024

Copy link
Copy Markdown
Contributor

@pjlast Can we update changelog too?

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@pjlast pjlast merged commit 710c734 into main May 2, 2024
@pjlast pjlast deleted the pjlast/61924-ghe-internal-repos-support branch May 2, 2024 13:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support internal repos on GHE better without having to individually add orgs

3 participants