index: use ID based shard names only on multi-tenant instances

index/builder.go

@@ -336,8 +336,9 @@ func (o *Options) shardName(n int) string {
 func (o *Options) shardNameVersion(version, n int) string {
 	var prefix string
 
-	// If tenant enforcement is enabled and we have tenant/repo IDs, use those to generate the prefix
-	if o.RepositoryDescription.TenantID != 0 && o.RepositoryDescription.ID != 0 && tenant.EnforceTenant() {
+	// Sourcegraph specific: We use IDs in shard names on multi-tenant
+	// instances to prevent conflicts.
+	if tenant.UseIDBasedShardNames() {
 		prefix = fmt.Sprintf("%09d_%09d", o.RepositoryDescription.TenantID, o.RepositoryDescription.ID)
 	} else {
 		prefix = o.RepositoryDescription.Name

index/builder_test.go

@@ -1165,3 +1165,36 @@ func TestFileRank(t *testing.T) {
 		})
 	}
 }
+
+func TestOptions_shardName(t *testing.T) {
+	opts := Options{
+		IndexDir: "/data",
+		RepositoryDescription: zoekt.Repository{
+			Name:     "a/b",
+			TenantID: 123,
+			ID:       456,
+		},
+	}
+
+	t.Setenv("WORKSPACES_API_URL", "")
+	if got, want := opts.shardNameVersion(16, 0), "/data/a%2Fb_v16.00000.zoekt"; got != want {
+		t.Fatalf("expected shard name to be repo name based:\ngot:  %q\nwant: %q", got, want)
+	}
+
+	t.Setenv("WORKSPACES_API_URL", "http://example.com")
+	if got, want := opts.shardNameVersion(16, 0), "/data/000000123_000000456_v16.00000.zoekt"; got != want {
+		t.Fatalf("expected shard name to be ID based:\ngot:  %q\nwant: %q", got, want)
+	}
+
+	// If something goes wrong and TenantID and RepoID is unset, we create a
+	// name which won't be visible by any tenant.
+	opts = Options{
+		IndexDir: "/data",
+		RepositoryDescription: zoekt.Repository{
+			Name: "a/b",
+		},
+	}
+	if got, want := opts.shardNameVersion(16, 0), "/data/000000000_000000000_v16.00000.zoekt"; got != want {
+		t.Fatalf("expected shard name to be with no tenant:\ngot:  %q\nwant: %q", got, want)
+	}
+}

internal/tenant/enforcement.go

@@ -1,6 +1,8 @@
 package tenant
 
 import (
+	"os"
+
 	"github.com/sourcegraph/zoekt/internal/tenant/internal/enforcement"
 )
 
@@ -12,3 +14,23 @@ func EnforceTenant() bool {
 		return false
 	}
 }
+
+// UseIDBasedShardNames returns true if the on disk layout of shards should
+// instead use tenant ID and repository IDs in the names instead of the actual
+// repository names.
+//
+// It is possible for repositories to have the same name, but have different
+// content in a multi-tenant setup. As such, this implementation only returns
+// true in those situations.
+//
+// Note: We could migrate all on-disk layout to only be ID based. However,
+// ID's are a Sourcegraph specific feature so we will always need the two code
+// paths. As such we only return true in multitenant setups.
+//
+// This is Sourcegraph specific.
+func UseIDBasedShardNames() bool {
+	// We use the presence of this environment variable to tell if we are in a
+	// multi-tenant setup. This is the same check that is done in the
+	// Sourcegraph monorepo.
+	return os.Getenv("WORKSPACES_API_URL") != ""
+}