diff --git a/pkg/sources/github/github.go b/pkg/sources/github/github.go index cf0b8fc79..f94488972 100644 --- a/pkg/sources/github/github.go +++ b/pkg/sources/github/github.go @@ -382,7 +382,9 @@ func (s *Source) Chunks(ctx context.Context, chunksChan chan *sources.Chunk, tar // s.memberCache, s.totalRepoSize, s.orgsCache, and s.repos. Additionally, // repositories and gists are reported to the provided UnitReporter. func (s *Source) Enumerate(ctx context.Context, reporter sources.UnitReporter) error { - seenUnits := make(map[sources.SourceUnit]struct{}) + // Use SourceUnitID (URL) as the deduplication key instead of the full struct + // to handle cases where the same repo might be reported with slightly different URL formats + seenUnitIDs := make(map[string]struct{}) // Wrapper reporter to deduplicate and filter found units. dedupeReporter := sources.VisitorReporter{ VisitUnit: func(ctx context.Context, su sources.SourceUnit) error { @@ -391,11 +393,12 @@ func (s *Source) Enumerate(ctx context.Context, reporter sources.UnitReporter) e if !s.filteredRepoCache.Exists(name) { return ctx.Err() } - // Only report a unit once. - if _, ok := seenUnits[su]; ok { + // Only report a unit once, using the SourceUnitID as the deduplication key. + unitID, _ := su.SourceUnitID() + if _, ok := seenUnitIDs[unitID]; ok { return ctx.Err() } - seenUnits[su] = struct{}{} + seenUnitIDs[unitID] = struct{}{} return reporter.UnitOk(ctx, su) }, VisitErr: reporter.UnitErr, diff --git a/pkg/sources/github/github_test.go b/pkg/sources/github/github_test.go index 53e20d6a2..0468b2c73 100644 --- a/pkg/sources/github/github_test.go +++ b/pkg/sources/github/github_test.go @@ -14,6 +14,7 @@ import ( "slices" "strconv" "strings" + "sync/atomic" "testing" "time" @@ -1103,3 +1104,100 @@ func noopReporter() sources.UnitReporter { }, } } + +func TestEnumerateDeduplication(t *testing.T) { + // Test that repositories are not reported multiple times when + // they appear in both explicit repos and organization enumeration + ctx := context.Background() + + // Set up mocks for the API calls + gock.New("https://api.github.com"). + Get("/user"). + Reply(200). + JSON(map[string]string{"login": "test-user"}) + + // Mock the explicit repo fetch + gock.New("https://api.github.com"). + Get("/repos/test-org/repo1"). + Reply(200). + JSON(map[string]any{ + "full_name": "test-org/repo1", + "clone_url": "https://github.com/test-org/repo1.git", + "size": 100, + }) + + gock.New("https://api.github.com"). + Get("/repos/test-org/repo2"). + Reply(200). + JSON(map[string]any{ + "full_name": "test-org/repo2", + "clone_url": "https://github.com/test-org/repo2.git", + "size": 100, + }) + + // Mock the org repos endpoint - returns the same repos + var orgReposCalled atomic.Bool + gock.New("https://api.github.com"). + Get("/orgs/test-org/repos"). + MatchParam("per_page", "100"). + AddMatcher(gock.MatchFunc(func(req *http.Request, greq *gock.Request) (bool, error) { + orgReposCalled.Store(true) + return true, nil + })). + Reply(200). + JSON([]map[string]any{ + { + "full_name": "test-org/repo1", + "clone_url": "https://github.com/test-org/repo1.git", + "size": 100, + }, + { + "full_name": "test-org/repo2", + "clone_url": "https://github.com/test-org/repo2.git", + "size": 100, + }, + }) + + // Create a source with both explicit repos and organization specified + // The repos provided explicitly are ALSO in the organization + source := initTestSource(&sourcespb.GitHub{ + Credential: &sourcespb.GitHub_Token{ + Token: "test-token", + }, + Repositories: []string{ + "test-org/repo1", + "test-org/repo2", + }, + Organizations: []string{ + "test-org", // This org contains the same repos + }, + }) + + // Track reported units to verify deduplication + reportedUnits := make(map[string]int) + countingReporter := sources.VisitorReporter{ + VisitUnit: func(ctx context.Context, unit sources.SourceUnit) error { + unitID, _ := unit.SourceUnitID() + reportedUnits[unitID]++ + return nil + }, + } + + // Test the Enumerate method + err := source.Enumerate(ctx, countingReporter) + require.NoError(t, err) + + // Check if there were any unmatched HTTP requests + if gock.HasUnmatchedRequest() { + t.Errorf("Had unmatched HTTP requests: %v", gock.GetUnmatchedRequests()) + } + + // Verify that each repo was only reported ONCE despite appearing in both explicit and org enumeration + repo1URL := "https://github.com/test-org/repo1.git" + repo2URL := "https://github.com/test-org/repo2.git" + + assert.Equal(t, 1, reportedUnits[repo1URL], "repo1 should be reported exactly once, not %d times", reportedUnits[repo1URL]) + assert.Equal(t, 1, reportedUnits[repo2URL], "repo2 should be reported exactly once, not %d times", reportedUnits[repo2URL]) + assert.Len(t, reportedUnits, 2, "Should have exactly 2 unique repositories reported") + assert.True(t, orgReposCalled.Load(), "Organization repos endpoint should have been called") +}