From 5c346a9fc37c6f990991d5de868e0591ed2e8788 Mon Sep 17 00:00:00 2001 From: Petri-Johan Last Date: Wed, 14 Jun 2023 16:28:02 +0200 Subject: [PATCH] Cache results of sub-repo perms enabled checks (#53463) --- internal/authz/sub_repo_perms.go | 8 ------ .../search/job/jobutil/sub_repo_perms_job.go | 28 +++++++++++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/internal/authz/sub_repo_perms.go b/internal/authz/sub_repo_perms.go index 0daf5e086e6..20e66c12a52 100644 --- a/internal/authz/sub_repo_perms.go +++ b/internal/authz/sub_repo_perms.go @@ -164,14 +164,6 @@ func canReadPaths(ctx context.Context, checker SubRepoPermissionChecker, repo ap return true, nil } - enabled, err := SubRepoEnabledForRepo(ctx, checker, repo) - if err != nil { - return false, err - } - if !enabled { - return true, nil - } - start := time.Now() var checkPathPermsCount int defer func() { diff --git a/internal/search/job/jobutil/sub_repo_perms_job.go b/internal/search/job/jobutil/sub_repo_perms_job.go index 1e9dcd914b2..a28a4f71766 100644 --- a/internal/search/job/jobutil/sub_repo_perms_job.go +++ b/internal/search/job/jobutil/sub_repo_perms_job.go @@ -8,6 +8,7 @@ import ( "go.opentelemetry.io/otel/attribute" "github.com/sourcegraph/sourcegraph/internal/actor" + "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/authz" "github.com/sourcegraph/sourcegraph/internal/search" "github.com/sourcegraph/sourcegraph/internal/search/job" @@ -84,7 +85,34 @@ func applySubRepoFiltering(ctx context.Context, checker authz.SubRepoPermissionC // Filter matches in place filtered := matches[:0] + subRepoPermsCache := map[api.RepoName]bool{} + errCache := map[api.RepoName]struct{}{} // cache repos that errored + for _, m := range matches { + // If the check errored before, skip the repo + if _, ok := errCache[m.RepoName().Name]; ok { + continue + } + // Skip check if sub-repo perms are disabled for the repository + enabled, ok := subRepoPermsCache[m.RepoName().Name] + if ok && !enabled { + filtered = append(filtered, m) + continue + } + if !ok { + enabled, err := authz.SubRepoEnabledForRepo(ctx, checker, m.RepoName().Name) + if err != nil { + // If an error occurs while checking sub-repo perms, we omit it from the results + logger.Error("Could not determine if sub-repo permissions are enabled for repo, skipping", log.String("repoName", string(m.RepoName().Name))) + errCache[m.RepoName().Name] = struct{}{} + continue + } + subRepoPermsCache[m.RepoName().Name] = enabled // cache the result for this repo name + if !enabled { + filtered = append(filtered, m) + continue + } + } switch mm := m.(type) { case *result.FileMatch: repo := mm.Repo.Name