From 68059344f8f1fef010e1114a463b123234f4c75f Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Wed, 6 Aug 2025 02:04:32 -0600 Subject: [PATCH] Unified storage: Remove unifiedStorageSearchPermissionFiltering feature flag (#109216) * Unified storage: Remove unifiedStorageSearchPermissionFiltering ff * unit test --- e2e/dashboards-search-suite/mode0.ini | 1 - e2e/dashboards-search-suite/mode1.ini | 1 - .../mode2-legacy-search-api.ini | 1 - e2e/dashboards-search-suite/mode2.ini | 1 - e2e/dashboards-search-suite/mode3.ini | 1 - e2e/dashboards-search-suite/mode4.ini | 1 - e2e/dashboards-search-suite/mode5.ini | 1 - .../src/types/featureToggles.gen.ts | 5 ---- pkg/registry/apis/dashboard/search.go | 5 ---- pkg/registry/apis/dashboard/search_test.go | 26 +++---------------- pkg/server/search_server_distributor_test.go | 2 +- pkg/services/featuremgmt/registry.go | 9 ------- pkg/services/featuremgmt/toggles-gitlog.csv | 1 - pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 --- pkg/services/featuremgmt/toggles_gen.json | 15 ----------- pkg/storage/unified/README.md | 6 ----- pkg/storage/unified/search/bleve.go | 2 +- .../unified/search/bleve_integration_test.go | 4 +-- .../unified/search/bleve_search_test.go | 2 +- pkg/storage/unified/search/bleve_test.go | 4 +-- .../unified/sql/test/benchmark_test.go | 2 +- .../unified/sql/test/integration_test.go | 2 +- 23 files changed, 12 insertions(+), 85 deletions(-) diff --git a/e2e/dashboards-search-suite/mode0.ini b/e2e/dashboards-search-suite/mode0.ini index e8f0a768c77..5bb45c0fceb 100644 --- a/e2e/dashboards-search-suite/mode0.ini +++ b/e2e/dashboards-search-suite/mode0.ini @@ -5,7 +5,6 @@ unifiedStorageSearch = true unifiedStorageSearchUI = true grafanaAPIServerWithExperimentalAPIs = true unifiedStorageSearchSprinkles = true -unifiedStorageSearchPermissionFiltering = true [unified_storage.folders.folder.grafana.app] dualWriterMode = 0 diff --git a/e2e/dashboards-search-suite/mode1.ini b/e2e/dashboards-search-suite/mode1.ini index 5beecc31f65..0a3a6463c03 100644 --- a/e2e/dashboards-search-suite/mode1.ini +++ b/e2e/dashboards-search-suite/mode1.ini @@ -5,7 +5,6 @@ unifiedStorageSearch = true unifiedStorageSearchUI = true grafanaAPIServerWithExperimentalAPIs = true unifiedStorageSearchSprinkles = true -unifiedStorageSearchPermissionFiltering = true [unified_storage.folders.folder.grafana.app] dualWriterMode = 1 diff --git a/e2e/dashboards-search-suite/mode2-legacy-search-api.ini b/e2e/dashboards-search-suite/mode2-legacy-search-api.ini index 45b16de4034..b034996f8cb 100644 --- a/e2e/dashboards-search-suite/mode2-legacy-search-api.ini +++ b/e2e/dashboards-search-suite/mode2-legacy-search-api.ini @@ -5,7 +5,6 @@ unifiedStorageSearch = true unifiedStorageSearchUI = false grafanaAPIServerWithExperimentalAPIs = true unifiedStorageSearchSprinkles = true -unifiedStorageSearchPermissionFiltering = true [unified_storage.folders.folder.grafana.app] dualWriterMode = 2 diff --git a/e2e/dashboards-search-suite/mode2.ini b/e2e/dashboards-search-suite/mode2.ini index 55538ee82e3..83c15652063 100644 --- a/e2e/dashboards-search-suite/mode2.ini +++ b/e2e/dashboards-search-suite/mode2.ini @@ -5,7 +5,6 @@ unifiedStorageSearch = true unifiedStorageSearchUI = true grafanaAPIServerWithExperimentalAPIs = true unifiedStorageSearchSprinkles = true -unifiedStorageSearchPermissionFiltering = true [unified_storage.folders.folder.grafana.app] dualWriterMode = 2 diff --git a/e2e/dashboards-search-suite/mode3.ini b/e2e/dashboards-search-suite/mode3.ini index 5955dfcd72a..b9c02681dd8 100644 --- a/e2e/dashboards-search-suite/mode3.ini +++ b/e2e/dashboards-search-suite/mode3.ini @@ -5,7 +5,6 @@ unifiedStorageSearch = true unifiedStorageSearchUI = true grafanaAPIServerWithExperimentalAPIs = true unifiedStorageSearchSprinkles = true -unifiedStorageSearchPermissionFiltering = true [unified_storage.folders.folder.grafana.app] dualWriterMode = 3 diff --git a/e2e/dashboards-search-suite/mode4.ini b/e2e/dashboards-search-suite/mode4.ini index e0f6f6d892d..db8796c572f 100644 --- a/e2e/dashboards-search-suite/mode4.ini +++ b/e2e/dashboards-search-suite/mode4.ini @@ -5,7 +5,6 @@ unifiedStorageSearch = true unifiedStorageSearchUI = true grafanaAPIServerWithExperimentalAPIs = true unifiedStorageSearchSprinkles = true -unifiedStorageSearchPermissionFiltering = true [unified_storage.folders.folder.grafana.app] dualWriterMode = 4 diff --git a/e2e/dashboards-search-suite/mode5.ini b/e2e/dashboards-search-suite/mode5.ini index 8722cb091eb..596fe7cb80c 100644 --- a/e2e/dashboards-search-suite/mode5.ini +++ b/e2e/dashboards-search-suite/mode5.ini @@ -5,7 +5,6 @@ unifiedStorageSearch = true unifiedStorageSearchUI = true grafanaAPIServerWithExperimentalAPIs = true unifiedStorageSearchSprinkles = true -unifiedStorageSearchPermissionFiltering = true [unified_storage.folders.folder.grafana.app] dualWriterMode = 5 diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 7198f33660e..cb400b4e5fa 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -679,11 +679,6 @@ export interface FeatureToggles { */ unifiedStorageSearchSprinkles?: boolean; /** - * Enable permission filtering on unified storage search - * @default true - */ - unifiedStorageSearchPermissionFiltering?: boolean; - /** * Pick the dual write mode from database configs */ managedDualWriter?: boolean; diff --git a/pkg/registry/apis/dashboard/search.go b/pkg/registry/apis/dashboard/search.go index 4252502e9ef..e00cca791d2 100644 --- a/pkg/registry/apis/dashboard/search.go +++ b/pkg/registry/apis/dashboard/search.go @@ -406,11 +406,6 @@ func asResourceKey(ns string, k string) (*resourcepb.ResourceKey, error) { } func (s *SearchHandler) getDashboardsUIDsSharedWithUser(ctx context.Context, user identity.Requester) ([]string, error) { - if !s.features.IsEnabledGlobally(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering) { - s.log.Warn("Tried to search for 'sharedwithme' dashboards with ", featuremgmt.FlagUnifiedStorageSearchPermissionFiltering, " disabled") - return []string{}, nil - } - // gets dashboards that the user was granted read access to permissions := user.GetPermissions() dashboardPermissions := permissions[dashboards.ActionDashboardsRead] diff --git a/pkg/registry/apis/dashboard/search_test.go b/pkg/registry/apis/dashboard/search_test.go index 31203b57efd..bf15cd7cc14 100644 --- a/pkg/registry/apis/dashboard/search_test.go +++ b/pkg/registry/apis/dashboard/search_test.go @@ -341,30 +341,10 @@ func TestSearchHandler(t *testing.T) { } func TestSearchHandlerSharedDashboards(t *testing.T) { - t.Run("should bail out if FlagUnifiedStorageSearchPermissionFiltering is not enabled globally", func(t *testing.T) { - mockClient := &MockClient{} - - features := featuremgmt.WithFeatures() - searchHandler := SearchHandler{ - log: log.New("test", "test"), - client: mockClient, - tracer: tracing.NewNoopTracerService(), - features: features, - } - rr := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/search?folder=sharedwithme", nil) - req.Header.Add("content-type", "application/json") - req = req.WithContext(identity.WithRequester(req.Context(), &user.SignedInUser{Namespace: "test"})) - - searchHandler.DoSearch(rr, req) - - assert.Equal(t, mockClient.CallCount, 0) - }) - t.Run("should return empty result without searching if user does not have shared dashboards", func(t *testing.T) { mockClient := &MockClient{} - features := featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering) + features := featuremgmt.WithFeatures() searchHandler := SearchHandler{ log: log.New("test", "test"), client: mockClient, @@ -451,7 +431,7 @@ func TestSearchHandlerSharedDashboards(t *testing.T) { MockResponses: []*resourcepb.ResourceSearchResponse{mockResponse1, mockResponse2}, } - features := featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering) + features := featuremgmt.WithFeatures() searchHandler := SearchHandler{ log: log.New("test", "test"), client: mockClient, @@ -571,7 +551,7 @@ func TestSearchHandlerSharedDashboards(t *testing.T) { MockResponses: []*resourcepb.ResourceSearchResponse{mockResponse1, mockResponse2, mockResponse3}, } - features := featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering) + features := featuremgmt.WithFeatures() searchHandler := SearchHandler{ log: log.New("test", "test"), client: mockClient, diff --git a/pkg/server/search_server_distributor_test.go b/pkg/server/search_server_distributor_test.go index 1fce76d8936..303fb9302a9 100644 --- a/pkg/server/search_server_distributor_test.go +++ b/pkg/server/search_server_distributor_test.go @@ -195,7 +195,7 @@ func TestIntegrationDistributor(t *testing.T) { } func getBaselineResponse[Req any, Resp any](t *testing.T, req *Req, fn func(ctx context.Context, req *Req) (*Resp, error)) *Resp { - ctx := context.Background() + ctx := identity.WithServiceIdentityContext(context.Background(), 1) baselineRes, err := fn(ctx, req) require.NoError(t, err) return baselineRes diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index c36496ca97e..ed5d527799e 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1169,15 +1169,6 @@ var ( HideFromDocs: true, HideFromAdminPage: true, }, - { - Name: "unifiedStorageSearchPermissionFiltering", - Description: "Enable permission filtering on unified storage search", - Stage: FeatureStageGeneralAvailability, - Owner: grafanaSearchAndStorageSquad, - Expression: "true", - HideFromDocs: true, - HideFromAdminPage: true, - }, { Name: "managedDualWriter", Description: "Pick the dual write mode from database configs", diff --git a/pkg/services/featuremgmt/toggles-gitlog.csv b/pkg/services/featuremgmt/toggles-gitlog.csv index 767f1dd2539..6c49dec2438 100644 --- a/pkg/services/featuremgmt/toggles-gitlog.csv +++ b/pkg/services/featuremgmt/toggles-gitlog.csv @@ -421,7 +421,6 @@ queryLibraryDashboards,2025-01-14T11:01:15Z,2025-02-14T16:39:22Z,740cd22fe51a354 elasticsearchImprovedParsing,2025-01-15T17:05:54Z,,bab55a4cb84f2ba57838f96a492ab9aa7f307957,Adam Yeats grafanaAdvisor,2025-01-20T10:08:00Z,,c1364d6be6f552203ba786f17a89664304b89247,Andres Martinez Gotor datasourceConnectionsTab,2025-01-21T17:39:48Z,,97d8f68b705f9949493079d1833abfe80e7b48f3,Syerikjan Kh -unifiedStorageSearchPermissionFiltering,2025-01-22T11:38:37Z,,dd483fc17fa4a2931848e3574cfc31ea6f6530d9,owensmallwood alertingSaveStateCompressed,2025-01-27T17:47:33Z,,cb43f4b6962fca18655b3ba634adeb4d59dc89df,Alexander Akhmetov fetchRulesUsingPost,2025-01-29T12:17:44Z,,1444051b65af0de6c412a12132083135c7730414,Fayzal Ghantiwala templateVariablesUsesCombobox,2025-01-31T09:53:13Z,,7190bfb0ca675fc1b3b5d7ddf7e0ee9d1c9ca3d7,Tobias Skarhed diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 9900d133c55..8af78162673 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -152,7 +152,6 @@ 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,GA,@grafana/search-and-storage,false,false,false managedDualWriter,experimental,@grafana/search-and-storage,false,false,false pluginsSriChecks,GA,@grafana/plugins-platform-backend,false,false,false unifiedStorageBigObjectsSupport,experimental,@grafana/search-and-storage,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index ae29df4a555..3a745490f8d 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -619,10 +619,6 @@ const ( // Enable sprinkles on unified storage search FlagUnifiedStorageSearchSprinkles = "unifiedStorageSearchSprinkles" - // FlagUnifiedStorageSearchPermissionFiltering - // Enable permission filtering on unified storage search - FlagUnifiedStorageSearchPermissionFiltering = "unifiedStorageSearchPermissionFiltering" - // FlagManagedDualWriter // Pick the dual write mode from database configs FlagManagedDualWriter = "managedDualWriter" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 4e18aefb96c..68fc984f930 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -3225,21 +3225,6 @@ "hideFromDocs": true } }, - { - "metadata": { - "name": "unifiedStorageSearchPermissionFiltering", - "resourceVersion": "1753448760331", - "creationTimestamp": "2025-01-22T11:38:37Z" - }, - "spec": { - "description": "Enable permission filtering on unified storage search", - "stage": "GA", - "codeowner": "@grafana/search-and-storage", - "hideFromAdminPage": true, - "hideFromDocs": true, - "expression": "true" - } - }, { "metadata": { "name": "unifiedStorageSearchSprinkles", diff --git a/pkg/storage/unified/README.md b/pkg/storage/unified/README.md index 608f51c062d..f60b6c937db 100644 --- a/pkg/storage/unified/README.md +++ b/pkg/storage/unified/README.md @@ -263,8 +263,6 @@ unifiedStorageSearchUI = true unifiedStorageSearch = true ; (optional) Allows you to sort dashboards by usage insights fields when using enterprise ; unifiedStorageSearchSprinkles = true -; (optional) Will skip search results filter based on user permissions -; unifiedStorageSearchPermissionFiltering = false ``` The dashboard search page has been set up to search unified storage. Additionally, all legacy search calls (e.g. `/api/search`) will go to @@ -871,7 +869,6 @@ Unified Search requires several feature flags to be enabled depending on the des |--------------|---------|-------|--------------| | `unifiedStorageSearch` | Core search functionality | Experimental | Search API servers, indexing | | `unifiedStorageSearchUI` | Frontend search interface | Experimental | Grafana UI search | -| `unifiedStorageSearchPermissionFiltering` | User permission filtering | GA | Access control in search results | | `unifiedStorageSearchSprinkles` | Usage insights integration | Experimental | Dashboard usage sorting (Enterprise) | | `unifiedStorageSearchDualReaderEnabled` | Shadow traffic to unified search | Experimental | Shadow traffic during migration | @@ -887,9 +884,6 @@ unifiedStorageSearch = true ; Enable search UI (required for frontend) unifiedStorageSearchUI = true -; Enable permission filtering (recommended) -unifiedStorageSearchPermissionFiltering = true - ; Enable shadow traffic during migration (optional) unifiedStorageSearchDualReaderEnabled = true diff --git a/pkg/storage/unified/search/bleve.go b/pkg/storage/unified/search/bleve.go index 2626ab36f1d..1e0d19e2e4f 100644 --- a/pkg/storage/unified/search/bleve.go +++ b/pkg/storage/unified/search/bleve.go @@ -961,7 +961,7 @@ func (b *bleveIndex) toBleveSearchRequest(ctx context.Context, req *resourcepb.R searchrequest.Query = bleve.NewConjunctionQuery(queries...) // AND } - 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")) diff --git a/pkg/storage/unified/search/bleve_integration_test.go b/pkg/storage/unified/search/bleve_integration_test.go index f76f89255e0..043b7642dac 100644 --- a/pkg/storage/unified/search/bleve_integration_test.go +++ b/pkg/storage/unified/search/bleve_integration_test.go @@ -21,7 +21,7 @@ func TestBleveSearchBackend(t *testing.T) { backend, err := NewBleveBackend(BleveOptions{ Root: tempDir, FileThreshold: 5, - }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering), nil) + }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(), nil) require.NoError(t, err) require.NotNil(t, backend) @@ -44,7 +44,7 @@ func TestSearchBackendBenchmark(t *testing.T) { // Create a new bleve backend backend, err := NewBleveBackend(BleveOptions{ Root: tempDir, - }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering), nil) + }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(), nil) require.NoError(t, err) require.NotNil(t, backend) diff --git a/pkg/storage/unified/search/bleve_search_test.go b/pkg/storage/unified/search/bleve_search_test.go index 0680026e2af..e5bfc81a338 100644 --- a/pkg/storage/unified/search/bleve_search_test.go +++ b/pkg/storage/unified/search/bleve_search_test.go @@ -533,7 +533,7 @@ func newTestDashboardsIndex(t TB, threshold int64, size int64, batchSize int64, Root: tmpdir, FileThreshold: threshold, // use in-memory for tests BatchSize: int(batchSize), - }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering), nil) + }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(), nil) require.NoError(t, err) rv := int64(10) diff --git a/pkg/storage/unified/search/bleve_test.go b/pkg/storage/unified/search/bleve_test.go index 821ce6de0c0..a391efde3a1 100644 --- a/pkg/storage/unified/search/bleve_test.go +++ b/pkg/storage/unified/search/bleve_test.go @@ -47,7 +47,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(), featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering), nil) + }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(), nil) require.NoError(t, err) rv := int64(10) @@ -750,7 +750,7 @@ func setupBleveBackend(t *testing.T, fileThreshold int, cacheTTL time.Duration, Root: dir, FileThreshold: int64(fileThreshold), IndexCacheTTL: cacheTTL, - }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering), metrics) + }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(), metrics) require.NoError(t, err) require.NotNil(t, backend) t.Cleanup(backend.closeAllIndexes) diff --git a/pkg/storage/unified/sql/test/benchmark_test.go b/pkg/storage/unified/sql/test/benchmark_test.go index 4740d7f5858..585776fd7bb 100644 --- a/pkg/storage/unified/sql/test/benchmark_test.go +++ b/pkg/storage/unified/sql/test/benchmark_test.go @@ -71,7 +71,7 @@ func TestIntegrationBenchmarkResourceServer(t *testing.T) { // Create a new bleve backend search, err := search.NewBleveBackend(search.BleveOptions{ Root: tempDir, - }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering), nil) + }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(), nil) require.NoError(t, err) require.NotNil(t, search) diff --git a/pkg/storage/unified/sql/test/integration_test.go b/pkg/storage/unified/sql/test/integration_test.go index 7da9798033e..c03915164f8 100644 --- a/pkg/storage/unified/sql/test/integration_test.go +++ b/pkg/storage/unified/sql/test/integration_test.go @@ -117,7 +117,7 @@ func TestIntegrationSearchAndStorage(t *testing.T) { search, err := search.NewBleveBackend(search.BleveOptions{ FileThreshold: 0, Root: tempDir, - }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(featuremgmt.FlagUnifiedStorageSearchPermissionFiltering), nil) + }, tracing.NewNoopTracerService(), featuremgmt.WithFeatures(), nil) require.NoError(t, err) require.NotNil(t, search)