Skip to content

Commit

Permalink
tenant: introduce systemtenant
Browse files Browse the repository at this point in the history
Some tasks, such as loading shards, require priviledged access on
startup. Here I introduce a systemtenant we can use for these things.

This is motivated by bug where the symbol sidebar in multi-tenant node
wouldn't work because ranked shards were not loaded correctly which in
turn caused `SelectRepoSet` to return 0 shards always.

Test plan:
- added unit test
- manual testing: symbol sidebar works now
  • Loading branch information
stefanhengl committed Nov 21, 2024
1 parent c52a9cd commit 6bbab33
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 1 deletion.
5 changes: 5 additions & 0 deletions internal/tenant/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package tenant

import (
"context"

"github.com/sourcegraph/zoekt/internal/tenant/systemtenant"
)

// EqualsID returns true if the tenant ID in the context matches the
Expand All @@ -10,6 +12,9 @@ func EqualsID(ctx context.Context, id int) bool {
if !EnforceTenant() {
return true
}
if systemtenant.Is(ctx) {
return true
}
t, err := FromContext(ctx)
if err != nil {
return false
Expand Down
33 changes: 33 additions & 0 deletions internal/tenant/systemtenant/systemtenant.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Package systemtenant contains function to mark a context as allowed to
// access shards across all tenants. This must only be used for tasks that are
// not request specific.
package systemtenant

import (
"context"
"fmt"

"github.com/sourcegraph/zoekt/internal/tenant/internal/tenanttype"
)

type contextKey int

const systemTenantKey contextKey = iota

// With marks a ctx to be allowed to access shards across all tenants. This MUST
// NOT BE USED on the user request path.
func With(ctx context.Context) (context.Context, error) {
// We don't want to allow setting the system tenant on a context that already
// has a user tenant set.
if _, ok := tenanttype.GetTenant(ctx); ok {
return nil, fmt.Errorf("tenant context already set")
}
return context.WithValue(ctx, systemTenantKey, systemTenantKey), nil
}

// Is returns true if the context has been marked to allow queries across all
// tenants.
func Is(ctx context.Context) bool {
_, ok := ctx.Value(systemTenantKey).(contextKey)
return ok
}
17 changes: 17 additions & 0 deletions internal/tenant/systemtenant/systemtenant_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package systemtenant

import (
"context"
"testing"

"github.com/stretchr/testify/require"
)

func TestSystemtenantRoundtrip(t *testing.T) {
if Is(context.Background()) {
t.Fatal()
}
ctx, err := With(context.Background())
require.NoError(t, err)
require.True(t, Is(ctx))
}
7 changes: 6 additions & 1 deletion shards/shards.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"go.uber.org/atomic"

"github.com/sourcegraph/zoekt"
"github.com/sourcegraph/zoekt/internal/tenant/systemtenant"
"github.com/sourcegraph/zoekt/query"
"github.com/sourcegraph/zoekt/trace"
)
Expand Down Expand Up @@ -1064,7 +1065,11 @@ func (s *shardedSearcher) getLoaded() loaded {

func mkRankedShard(s zoekt.Searcher) *rankedShard {
q := query.Const{Value: true}
result, err := s.List(context.Background(), &q, nil)
ctx, err := systemtenant.With(context.Background())
if err != nil {
return &rankedShard{Searcher: s}
}
result, err := s.List(ctx, &q, nil)
if err != nil {
return &rankedShard{Searcher: s}
}
Expand Down

0 comments on commit 6bbab33

Please sign in to comment.