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

Add options to configure internal repo handling for GitLab#57858

Merged
pjlast merged 17 commits into
mainfrom
pjlast/57804-default-permissions-gitlab-internal-repos
Oct 31, 2023
Merged

Add options to configure internal repo handling for GitLab#57858
pjlast merged 17 commits into
mainfrom
pjlast/57804-default-permissions-gitlab-internal-repos

Conversation

@pjlast

@pjlast pjlast commented Oct 24, 2023

Copy link
Copy Markdown
Contributor

Closes #57804

This PR adds an option to GitLab code host connections called markInternalReposAsPublic, which sets the visibility of all internal repos added by this code host connection to true, and an option to the GitLab auth provider called syncInternalRepoPermissions. syncInternalRepoPermissions are true by default, and can be set to false to optimise systems where markInternalReposAsPublic is set to true.

Test plan

Unit tests updated. Set up and tested locally as well.

@cla-bot cla-bot Bot added the cla-signed label Oct 24, 2023
@github-actions github-actions Bot added the team/source Tickets under the purview of Source - the one Source to graph it all label Oct 24, 2023
@pjlast pjlast requested a review from a team October 25, 2023 12:07
@pjlast pjlast marked this pull request as ready for review October 25, 2023 12:07
@sourcegraph-bot

sourcegraph-bot commented Oct 25, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 17e8f2d...114d762.

Notify File(s)
@eseliger internal/repos/gitlab.go
internal/repos/gitlab_test.go
@unknwon cmd/repo-updater/internal/authz/integration_test.go
internal/authz/providers/authz_test.go
internal/authz/providers/authz_test.go
internal/authz/providers/gitlab/authz.go
internal/authz/providers/gitlab/authz.go
internal/authz/providers/gitlab/oauth.go
internal/authz/providers/gitlab/oauth.go
internal/authz/providers/gitlab/oauth_test.go
internal/authz/providers/gitlab/oauth_test.go
internal/authz/providers/gitlab/sudo.go
internal/authz/providers/gitlab/sudo.go
internal/authz/providers/gitlab/sudo_test.go
internal/authz/providers/gitlab/sudo_test.go

Comment thread doc/admin/external_service/gitlab.md Outdated
}
```

If you would like internal repositories to remain private, but you're experiencing issues where user permission syncs aren't granting access to internal repositories, you can add the following field instead:

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.

when would I experience these issues and how do I notice that I do?

Comment on lines +241 to +247
repoVisibility := []string{"private"}
if listInternalRepos {
repoVisibility = append(repoVisibility, "internal")
}

// This method is meant to return only private or internal projects
for _, visibility := range []string{"private", "internal"} {
for _, visibility := range repoVisibility {

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.

This changes the default here, correct? Would this cause any trouble to existing instances?

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.

Yeah I'm still thinking about this a bit. I'm flip-flopping a bit between what would be the better option.

I don't mind changing the default behaviour too much, as long as we mention it in the changelog. And the behavioural change won't be particularly destructive.

I'd just like to settle on a default behaviour that would work for most cases. Which might be: private internal repos with syncing enabled 🤔 will let it stew a bit

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.

We could probably just condense the settings to the single markInternalReposAsPublic one? Since if they're public, you don't need permission syncs 🤔

Comment thread internal/repos/gitlab.go Outdated
provider: provider,
client: client,
logger: logger,
markInternalReposAsPublic: (c.Authorization != nil) && c.Authorization.MarkInternalReposAsPublic,

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.

hmm do we need that c.Authorization check here?
Is it to cover the case where someone sets it to false, but auth is off?

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.

Authorization can be nil here if it is completely unset, so accessing .MarkInternalReposAsPublic would throw a nil pointer exception

Comment thread internal/repos/gitlab.go Outdated
@pjlast pjlast force-pushed the pjlast/57804-default-permissions-gitlab-internal-repos branch from 4b0c373 to 66f946d Compare October 31, 2023 11:18
@pjlast pjlast merged commit 2ea61e5 into main Oct 31, 2023
@pjlast pjlast deleted the pjlast/57804-default-permissions-gitlab-internal-repos branch October 31, 2023 12:37
sourcegraph-release-bot pushed a commit that referenced this pull request Oct 31, 2023
camdencheek pushed a commit that referenced this pull request Nov 1, 2023
…tLab (#58012)

Add options to configure internal repo handling for GitLab (#57858)

(cherry picked from commit 2ea61e5)

Co-authored-by: Petri-Johan Last <petri.last@sourcegraph.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backport 5.2 cla-signed 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.

Add option to set default permission value for GitLab internal repos (like we do for GitHub)

3 participants