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

[Backport 5.2] Sync GitHub internal repos during user perms sync#57386

Merged
keegancsmith merged 3 commits into
5.2from
backport-56677-to-5.2
Oct 16, 2023
Merged

[Backport 5.2] Sync GitHub internal repos during user perms sync#57386
keegancsmith merged 3 commits into
5.2from
backport-56677-to-5.2

Conversation

@sourcegraph-release-bot

@sourcegraph-release-bot sourcegraph-release-bot commented Oct 5, 2023

Copy link
Copy Markdown
Collaborator

Closes #56648

Sourcegraph now explicitly fetches internal repositories for organizations a user belongs to. This used to be only done when groups caching is enabled, but it leads to a lot of confusion for customers.

This may increase the number of requests made by user permission syncs. If that is the case, groups caching should be enabled.

As part of this work I decided the refactor the GitHub user permissions fetching code, which was very contrived. It's a lot easier to reason about what should be happening now.

Test plan

VCR tests updated to account for extra API calls. Local validations done as well.


Backport d271395 from #56677

Preview 🤩

Preview Link

@sourcegraph-bot

sourcegraph-bot commented Oct 5, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 5775b5e...bb4ab41.

Notify File(s)
@eseliger internal/batches/sources/testdata/golden/GithubSource_GetFork_success_with_existing_changeset_and_existing_fork
internal/batches/sources/testdata/golden/GithubSource_GetFork_success_with_existing_changeset_and_new_fork
internal/batches/sources/testdata/golden/GithubSource_GetFork_success_with_new_changeset_and_existing_fork
internal/batches/sources/testdata/golden/GithubSource_GetFork_success_with_new_changeset_and_new_fork
internal/extsvc/github/common.go
internal/extsvc/github/testdata/golden/ListRepositoriesForSearch
internal/extsvc/github/testdata/golden/SearchRepos-Enterprise-narrow-query-enterprise
internal/extsvc/github/testdata/golden/TestGetRepository/cached-response/first_run
internal/extsvc/github/testdata/golden/TestGetRepository/cached-response/second_run
internal/extsvc/github/testdata/golden/TestGetRepository/forked_repo
internal/extsvc/github/testdata/golden/TestV3Client_Fork_success_sourcegraph-testing
internal/extsvc/github/testdata/golden/TestV3Client_Fork_success_user
internal/extsvc/github/v3.go
internal/extsvc/github/v3_test.go
internal/extsvc/github/v4.go
internal/repos/github.go
internal/repos/github_test.go
@sourcegraph/delivery doc/admin/external_service/github.md
@unknwon cmd/frontend/internal/auth/githuboauth/session.go
cmd/repo-updater/internal/authz/BUILD.bazel
cmd/repo-updater/internal/authz/integration_test.go
cmd/repo-updater/internal/authz/testdata/vcr/TestIntegration_GitHubInternalRepositories.yaml
cmd/repo-updater/internal/authz/testdata/vcr/TestIntegration_GitHubPermissions/repo-centric/groups-enabled.yaml
cmd/repo-updater/internal/authz/testdata/vcr/TestIntegration_GitHubPermissions/repo-centric/no-groups.yaml
cmd/repo-updater/internal/authz/testdata/vcr/TestIntegration_GitHubPermissions/user-centric/groups-enabled.yaml
cmd/repo-updater/internal/authz/testdata/vcr/TestIntegration_GitHubPermissions/user-centric/no-groups.yaml
internal/authz/providers/github/BUILD.bazel
internal/authz/providers/github/BUILD.bazel
internal/authz/providers/github/authz.go
internal/authz/providers/github/authz.go
internal/authz/providers/github/client.go
internal/authz/providers/github/client.go
internal/authz/providers/github/github.go
internal/authz/providers/github/github.go
internal/authz/providers/github/github_test.go
internal/authz/providers/github/github_test.go
internal/authz/providers/github/mocks_test.go
internal/authz/providers/github/mocks_test.go

@sourcegraph-bot

sourcegraph-bot commented Oct 5, 2023

Copy link
Copy Markdown
Contributor

📖 Storybook live preview

Comment thread CHANGELOG.md Outdated

### Added

- Added two new authorization configuration options to GitHub code host connections: "markInternalReposAsPublic" and "syncInternalRepoPermissions". Setting "markInternalReposAsPublic" to true is useful for organizations that have a large amount of internal repositories that everyone on the instance should be able to access, removing the need to have permissions to access these repositories. Setting "syncInternalRepoPermissions" to true adds an additional step to user permission syncs that explicitly checks for internal repositories. However, this could lead to longer user permission sync times. [#56677](https://github.com/sourcegraph/sourcegraph/pull/56677)

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 your changelog entry is not going under 5.2.1. Are you sure this was meant to be backported?

@eseliger eseliger removed the team/iam label Oct 9, 2023
@keegancsmith keegancsmith enabled auto-merge (squash) October 16, 2023 12:31
@keegancsmith keegancsmith added the backport/bugfix Standard patches to fix bugs label Oct 16, 2023
@keegancsmith keegancsmith merged commit 23ca773 into 5.2 Oct 16, 2023
@keegancsmith keegancsmith deleted the backport-56677-to-5.2 branch October 16, 2023 14:27
@varungandhi-src varungandhi-src mentioned this pull request Jan 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants