Gitlab Source: Backoff from Scan2 which is experimental to legacy pagination API call #4608

kashifkhan0771 · 2025-12-18T09:21:52Z

This PR refactors simplifiedGitlabEnumeration to use the legacy pagination API with keyset pagination instead of Scan2, which is currently in an experimental state. This change is intended as an experiment and does not guarantee a fix for the issue.

Additionally, it introduces retry logic to handle 500-level errors and adds logging to track the number of projects being enumerated.

Related issue on Gitlab: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7516

Checklist:

Tests passing (make test-community)?
Lint passing (make lint this requires golangci-lint)?

This commit rewrite simplifiedGitlabEnumeration to use legacy pagination API call with keyset pagination instead of Scan2 which is currently in experimental state. Note that this doesn't promise to fix this problem it's just a test to check. It also adds a retry logic in case any 500 error occurs. I added some logs as well to keep track of no of projects being enumerated.

martinlocklear

Thanks for looking at this! Some notes:

Definitely need test coverage for this before merging in.
I notice in our go.mod we're overriding the version of the library we're using. Is it possible that some of the issues that we've been seeing could be fixed by newer versions of this library?

martinlocklear · 2025-12-18T16:27:55Z

pkg/sources/gitlab/gitlab.go


 	projectQueryOptions := &gitlab.ListProjectsOptions{
 		ListOptions: listOpts,
 		Membership:  gitlab.Ptr(true),


I don't understand why we have membership set to true. (I'm not saying we shouldn't, just that I don't understand)

From the docs:

Limit by projects that the current user is a member of.

If this flag is potentially causing problems, why add in this limitation? Why not just scan everything that's visible to the current token, member or not?

The reason for adding this is that, if we don’t set membership to true for GitLab Cloud, the scan will enumerate all public GitLab projects. This is different from GitHub, where we explicitly restrict what can be scanned with the following condition:

if len(*githubScanOrgs) == 0 && len(*githubScanRepos) == 0 { return scanMetrics, fmt.Errorf("invalid config: you must specify at least one organization or repository") }

This ensures that GitHub scans are limited to explicitly specified organizations or repositories. In GitLab, however, we don’t currently have an org option. As a result, a scan can run with just a token and attempt to enumerate everything that token has access to.

To prevent this on GitLab Cloud, we set membership=true so that only projects the user is a member of are enumerated. For self-managed GitLab instances, we disable this behavior, allowing the scan to enumerate all projects the token has access to on that instance.

I see two possible approaches:

Add support for organization/group-based scanning in GitLab (similar to GitHub). With that restriction in place, we could remove the membership flag.

Keep the current behavior to avoid unintentionally scanning all public projects on GitLab Cloud.

The second option ensures we don’t accidentally scan the entire set of public GitLab Cloud projects.

Oh! Ok, that makes sense to me - I trust your judgement on the best path forward here.

I did some testing and found that, by default, GitLab keeps the membership flag set to false. To take advantage of it, I reversed some conditions to avoid setting the membership flag for non-GitLab Cloud instances. This could help if the issue was indeed related to the membership flag. I also set the simple flag to true; according to the docs, this returns only the essential fields for each project instead of the full, large JSON objects.

pkg/sources/gitlab/gitlab.go

… next page

shahzadhaider1 · 2025-12-22T06:26:54Z

pkg/sources/gitlab/gitlab.go

+		apiClient, err := gitlab.NewOAuthClient(
+			s.token,
+			gitlab.WithBaseURL(s.url),
+			gitlab.WithCustomRetryWaitMinMax(time.Second, 5*time.Second),


Seems like these are used in quite a few places. It'll be better to create constants for min and max.

time.Second is already a constant from time and I don't like the idea of making 5 only a constant here. Maybe I am wrong but usually we use time like this n*time.Second.

pkg/sources/gitlab/gitlab.go

shahzadhaider1 · 2025-12-24T10:16:44Z

pkg/sources/gitlab/gitlab.go

+		// Fetch a page of projects from the GitLab API using the current query options.
+		projects, resp, err := apiClient.Projects.ListProjects(projectQueryOptions, requestOptions...)
+		if err != nil {
+			err = fmt.Errorf("received error on listing projects, you might not have permissions to do that: %w", err)


totally optional thing:

Suggested change

err = fmt.Errorf("received error on listing projects, you might not have permissions to do that: %w", err)

err = fmt.Errorf("received an error while listing projects; this may be due to insufficient permissions: %w", err)

martinlocklear

I think we still need to add unit testing before we merge this.

martinlocklear · 2026-01-05T19:54:34Z

pkg/sources/gitlab/gitlab.go


 	projectQueryOptions := &gitlab.ListProjectsOptions{
 		ListOptions: listOpts,
 		Membership:  gitlab.Ptr(true),


Oh! Ok, that makes sense to me - I trust your judgement on the best path forward here.

Reversed the gitlab cloud logic to add membership flag, so that we use the default false for non gitlab.com instances. This can help if the issue really was membership flag as mentioned in some gitlab issues. Also added simple flag in list projects to get only minimal fields in response instead of big json response for each project. Added test case as well.

kashifkhan0771 · 2026-01-06T07:33:39Z

I think we still need to add unit testing before we merge this.

Added the test case

camgunz

LGTM! If you want you could add a couple more checks in the test, like I'm pretty sure some other sources check chunk count and so on. But up to you--I know you've done a lot of local testing w/ this.

camgunz · 2026-01-08T14:39:39Z

@martinlocklear

I notice in our go.mod we're overriding the version of the library we're using. Is it possible that some of the issues that we've been seeing could be fixed by newer versions of this library?

I just had a look, and while the changelog doesn't go as far back as our pinned version, I didn't see any bug fix that would've affected us. I made INS-249 to do this some day.

kashifkhan0771 · 2026-01-08T15:05:12Z

I notice in our go.mod we're overriding the version of the library we're using. Is it possible that some of the issues that we've been seeing could be fixed by newer versions of this library?

Somehow I missed this comment. The reason we did that was the latest version at that time was v1.134.0 but it deprecated existing Auth methods, also the Scan2 API was experimental so we pinned to version to avoid any new updates which accidentally break the API.

As Charlie said there isn't any bug fix in later versions, so I would prefer to update that in a separate PR.

martinlocklear · 2026-01-08T14:47:48Z

pkg/sources/gitlab/gitlab_integration_test.go

+	// use simplified gitlab enumeration
+	feature.UseSimplifiedGitlabEnumeration.Store(true)
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)


Just use the testing context, I think it's t.Context()

martinlocklear · 2026-01-08T14:49:53Z

pkg/sources/gitlab/gitlab_integration_test.go

+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {


Why even are we building this test table and then building a test runner? Let's just write a test. If we want to make our test setup and breakdown reusable, then we can convert this to a test suite and use SetupSuite or (better) SetupTest for that purpose.

martinlocklear · 2026-01-08T15:28:55Z

pkg/sources/gitlab/gitlab_integration_test.go

 	}
 }
+
+func TestSource_Chunks_SimplifiedGitlabEnumeration(t *testing.T) {


Can we (in comments) describe what the purpose of this test is? Like, what would it tell us if it was failing, and what does it give us confidence in when it's passing.

martinlocklear · 2026-01-08T15:31:10Z

Some non-blocking comments about the new test

kashifkhan0771 requested a review from a team December 18, 2025 09:21

kashifkhan0771 requested a review from a team as a code owner December 18, 2025 09:21

kashifkhan0771 changed the title ~~Backoff from Scan2 which is experimental to legacy pagination API call~~ Gitlab Source: Backoff from Scan2 which is experimental to legacy pagination API call Dec 18, 2025

martinlocklear requested changes Dec 18, 2025

View reviewed changes

implemented builtin retry mechanism for gitlab and proper handling of…

815ac8b

… next page

kashifkhan0771 force-pushed the fix/ins-191 branch from 25a7098 to 815ac8b Compare December 19, 2025 10:41

Merge branch 'main' into fix/ins-191

4696e76

kashifkhan0771 force-pushed the fix/ins-191 branch from 2e0dc27 to 4696e76 Compare December 22, 2025 09:16

shahzadhaider1 reviewed Dec 24, 2025

View reviewed changes

fixed basic auth

2caa3d3

shahzadhaider1 approved these changes Dec 26, 2025

View reviewed changes

martinlocklear requested changes Jan 5, 2026

View reviewed changes

kashifkhan0771 requested a review from martinlocklear January 6, 2026 07:45

kashifkhan0771 added 2 commits January 6, 2026 20:33

Merge branch 'main' into fix/ins-191

99228dc

Merge branch 'main' into fix/ins-191

afabb72

camgunz approved these changes Jan 8, 2026

View reviewed changes

martinlocklear reviewed Jan 8, 2026

View reviewed changes

	err = fmt.Errorf("received error on listing projects, you might not have permissions to do that: %w", err)
	err = fmt.Errorf("received an error while listing projects; this may be due to insufficient permissions: %w", err)

Gitlab Source: Backoff from Scan2 which is experimental to legacy pagination API call #4608

Are you sure you want to change the base?

Gitlab Source: Backoff from Scan2 which is experimental to legacy pagination API call #4608

Conversation

kashifkhan0771 commented Dec 18, 2025 • edited by shahzadhaider1 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist:

Uh oh!

martinlocklear left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martinlocklear left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kashifkhan0771 commented Jan 6, 2026

Uh oh!

camgunz left a comment

Choose a reason for hiding this comment

Uh oh!

camgunz commented Jan 8, 2026 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kashifkhan0771 commented Jan 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martinlocklear commented Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kashifkhan0771 commented Dec 18, 2025 •

edited by shahzadhaider1

Loading

camgunz commented Jan 8, 2026 •

edited by atlassian bot

Loading