Unified Storage: Search permissions put behind feature flag (#99340)

* add feature flag

* puts search permission filtering behind a feature flag

* fixes issue with doc match id. When the match is for an in-memory index, the internal id is a string (this is what we expected). However, when its a file-based index, the internal id is a binary encoded int64 that point to something internally. So to get the id, we need to use ExternalID() instead of relying on the indexInternalID to be the correct format.

* adds debug log

* update comment

* formatting

packages/grafana-data/src/types/featureToggles.gen.ts

@@ -218,6 +218,7 @@ export interface FeatureToggles {
   rolePickerDrawer?: boolean;
   unifiedStorageSearch?: boolean;
   unifiedStorageSearchSprinkles?: boolean;
+  unifiedStorageSearchPermissionFiltering?: boolean;
   pluginsSriChecks?: boolean;
   unifiedStorageBigObjectsSupport?: boolean;
   timeRangeProvider?: boolean;

pkg/services/featuremgmt/registry.go

@@ -1507,6 +1507,14 @@ var (
 			HideFromDocs:      true,
 			HideFromAdminPage: true,
 		},
+		{
+			Name:              "unifiedStorageSearchPermissionFiltering",
+			Description:       "Enable permission filtering on unified storage search",
+			Stage:             FeatureStageExperimental,
+			Owner:             grafanaSearchAndStorageSquad,
+			HideFromDocs:      true,
+			HideFromAdminPage: true,
+		},
 		{
 			Name:        "pluginsSriChecks",
 			Description: "Enables SRI checks for plugin assets",

pkg/services/featuremgmt/toggles_gen.csv

@@ -199,6 +199,7 @@ useSessionStorageForRedirection,GA,@grafana/identity-access-team,false,false,fal
 rolePickerDrawer,experimental,@grafana/identity-access-team,false,false,false
 unifiedStorageSearch,experimental,@grafana/search-and-storage,false,false,false
 unifiedStorageSearchSprinkles,experimental,@grafana/search-and-storage,false,false,false
+unifiedStorageSearchPermissionFiltering,experimental,@grafana/search-and-storage,false,false,false
 pluginsSriChecks,experimental,@grafana/plugins-platform-backend,false,false,false
 unifiedStorageBigObjectsSupport,experimental,@grafana/search-and-storage,false,false,false
 timeRangeProvider,experimental,@grafana/grafana-frontend-platform,false,false,false

pkg/services/featuremgmt/toggles_gen.go

@@ -807,6 +807,10 @@ const (
 	// Enable sprinkles on unified storage search
 	FlagUnifiedStorageSearchSprinkles = "unifiedStorageSearchSprinkles"
 
+	// FlagUnifiedStorageSearchPermissionFiltering
+	// Enable permission filtering on unified storage search
+	FlagUnifiedStorageSearchPermissionFiltering = "unifiedStorageSearchPermissionFiltering"
+
 	// FlagPluginsSriChecks
 	// Enables SRI checks for plugin assets
 	FlagPluginsSriChecks = "pluginsSriChecks"

pkg/services/featuremgmt/toggles_gen.json

@@ -3882,6 +3882,20 @@
         "hideFromDocs": true
       }
     },
+    {
+      "metadata": {
+        "name": "unifiedStorageSearchPermissionFiltering",
+        "resourceVersion": "1737489629408",
+        "creationTimestamp": "2025-01-21T20:00:29Z"
+      },
+      "spec": {
+        "description": "Enable permission filtering on unified storage search",
+        "stage": "experimental",
+        "codeowner": "@grafana/search-and-storage",
+        "hideFromAdminPage": true,
+        "hideFromDocs": true
+      }
+    },
     {
       "metadata": {
         "name": "unifiedStorageSearchSprinkles",

pkg/storage/unified/search/bleve.go

@@ -18,6 +18,7 @@ import (
 	"github.com/blevesearch/bleve/v2/search/query"
 	bleveSearch "github.com/blevesearch/bleve/v2/search/searcher"
 	index "github.com/blevesearch/bleve_index_api"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"go.opentelemetry.io/otel/trace"
 	"k8s.io/apimachinery/pkg/selection"
 
@@ -53,9 +54,11 @@ type bleveBackend struct {
 	// cache info
 	cache   map[resource.NamespacedResource]*bleveIndex
 	cacheMu sync.RWMutex
+
+	features featuremgmt.FeatureToggles
 }
 
-func NewBleveBackend(opts BleveOptions, tracer trace.Tracer) (*bleveBackend, error) {
+func NewBleveBackend(opts BleveOptions, tracer trace.Tracer, features featuremgmt.FeatureToggles) (*bleveBackend, error) {
 	if opts.Root == "" {
 		return nil, fmt.Errorf("bleve backend missing root folder configuration")
 	}
@@ -68,11 +71,12 @@ func NewBleveBackend(opts BleveOptions, tracer trace.Tracer) (*bleveBackend, err
 	}
 
 	return &bleveBackend{
-		log:    slog.Default().With("logger", "bleve-backend"),
+		log:      slog.Default().With("logger", "bleve-backend"),
-		tracer: tracer,
+		tracer:   tracer,
-		cache:  make(map[resource.NamespacedResource]*bleveIndex),
+		cache:    make(map[resource.NamespacedResource]*bleveIndex),
-		opts:   opts,
+		opts:     opts,
-		start:  time.Now(),
+		start:    time.Now(),
+		features: features,
 	}, nil
 }
 
@@ -172,6 +176,7 @@ func (b *bleveBackend) BuildIndex(ctx context.Context,
 		batchSize: b.opts.BatchSize,
 		fields:    fields,
 		standard:  resource.StandardSearchFields(),
+		features:  b.features,
 	}
 
 	idx.allFields, err = getAllFields(idx.standard, fields)
@@ -243,6 +248,8 @@ type bleveIndex struct {
 	// only valid in single thread
 	batch     *bleve.Batch
 	batchSize int // ??? not totally sure the units here
+
+	features featuremgmt.FeatureToggles
 }
 
 // Write implements resource.DocumentIndex.
@@ -579,8 +586,7 @@ func (b *bleveIndex) toBleveSearchRequest(ctx context.Context, req *resource.Res
 		searchrequest.Query = bleve.NewConjunctionQuery(queries...) // AND
 	}
 
-	// Can we remove this? Is access ever nil?
+	if access != nil && b.features.IsEnabledGlobally(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering) {
-	if access != nil {
 		auth, ok := authlib.AuthInfoFrom(ctx)
 		if !ok {
 			return nil, resource.AsErrorResult(fmt.Errorf("missing auth info"))
@@ -883,13 +889,20 @@ func (q *permissionScopedQuery) Searcher(ctx context.Context, i index.IndexReade
 	}
 
 	filteringSearcher := bleveSearch.NewFilteringSearcher(ctx, searcher, func(d *search.DocumentMatch) bool {
-		// The internal ID has the format: <namespace>/<group>/<resourceType>/<name>
+		// The doc ID has the format: <namespace>/<group>/<resourceType>/<name>
-		// Only the internal ID is present on the document match here. Need to use the dvReader for any other fields.
+		// IndexInternalID will be the same as the doc ID when using an in-memory index, but when using a file-based
-		id := string(d.IndexInternalID)
+		// index it becomes a binary encoded number that has some other internal meaning. Using ExternalID() will get the
-		parts := strings.Split(id, "/")
+		// correct doc ID regardless of the index type.
+		d.ID, err = i.ExternalID(d.IndexInternalID)
+		if err != nil {
+			q.log.Debug("Error getting external ID", "error", err)
+			return false
+		}
+
+		parts := strings.Split(d.ID, "/")
 		// Exclude doc if id isn't expected format
 		if len(parts) != 4 {
-			q.log.Debug("Unexpected document ID format", "id", id)
+			q.log.Debug("Unexpected document ID format", "id", d.ID)
 			return false
 		}
 		ns := parts[0]

pkg/storage/unified/search/bleve_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -40,7 +41,7 @@ func TestBleveBackend(t *testing.T) {
 	backend, err := NewBleveBackend(BleveOptions{
 		Root:          tmpdir,
 		FileThreshold: 5, // with more than 5 items we create a file on disk
-	}, tracing.NewNoopTracerService())
+	}, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering))
 	require.NoError(t, err)
 
 	// AVOID NPE in test

pkg/storage/unified/sql/server.go

@@ -70,7 +70,8 @@ func NewResourceServer(ctx context.Context, db infraDB.DB, cfg *setting.Cfg,
 			Root:          root,
 			FileThreshold: int64(cfg.IndexFileThreshold), // fewer than X items will use a memory index
 			BatchSize:     cfg.IndexMaxBatchSize,         // This is the batch size for how many objects to add to the index at once
-		}, tracer)
+		}, tracer, features)
+
 		if err != nil {
 			return nil, err
 		}