diff --git a/frontend/src/features/logs/SavedViews.test.ts b/frontend/src/features/logs/SavedViews.test.ts index eeec3a63c..b200b0050 100644 --- a/frontend/src/features/logs/SavedViews.test.ts +++ b/frontend/src/features/logs/SavedViews.test.ts @@ -87,6 +87,12 @@ describe('server saved views', () => { ]); expect(available.map(({ decoded }) => decoded?.dataset)).toEqual(['app', 'app', undefined]); expect(savedLogViewsForDataset([canonical], 'sql')).toEqual([]); + const promql = { + ...classic, + filter_name: 'Request rate', + query: { filter_type: 'promql' as const, filter_query: 'rate(requests_total[5m])' }, + }; + expect(savedLogViewsForDataset([promql], 'app')).toEqual([]); }); it('rejects classic formats and mismatched dataset metadata', () => { const { explorer_state: _state, ...classic } = savedLogViewBody(state, 'Classic'); diff --git a/frontend/src/features/logs/SavedViews.tsx b/frontend/src/features/logs/SavedViews.tsx index 59c4f4899..4b6b87802 100644 --- a/frontend/src/features/logs/SavedViews.tsx +++ b/frontend/src/features/logs/SavedViews.tsx @@ -25,8 +25,11 @@ export function savedLogViewsForDataset(views: SavedLogView[], dataset: string) return decoded.dataset === dataset ? [{ view, decoded }] : []; } catch { // Classic dataset-scoped views remain visible, but canonical views need a - // valid explorer state to establish which dataset they belong to. - return view.stream_name !== 'sql' && view.stream_name === dataset + // valid explorer state to establish which dataset they belong to. Saved + // PromQL queries share the endpoint but are not log views. + return view.query.filter_type !== 'promql' && + view.stream_name !== 'sql' && + view.stream_name === dataset ? [{ view, decoded: undefined }] : []; } diff --git a/frontend/src/features/sql/serverSavedQueries.test.ts b/frontend/src/features/sql/serverSavedQueries.test.ts index 9ecd2ff09..11224ea9e 100644 --- a/frontend/src/features/sql/serverSavedQueries.test.ts +++ b/frontend/src/features/sql/serverSavedQueries.test.ts @@ -92,6 +92,17 @@ const classicSearch: SavedLogView = { }; describe('server saved SQL queries', () => { + it('ignores saved PromQL filters', () => { + const promql: SavedLogView = { + ...classicSearch, + filter_id: '01HPROMQL', + stream_name: 'otel_metrics', + query: { filter_type: 'promql', filter_query: 'sum(rate(http_requests_total[5m]))' }, + }; + expect(savedLogView(promql)).toBe(true); + expect(serverSavedQuery(promql)).toBeUndefined(); + expect(serverSavedQueries([promql])).toEqual([]); + }); it('accepts the real classic fixtures through the client contract', () => { for (const view of [classicSql, classicLogsSql, nativeLogsSql, classicBuilder, classicSearch]) expect(savedLogView(view)).toBe(true); diff --git a/frontend/src/lib/logsContract.test.ts b/frontend/src/lib/logsContract.test.ts index 63040a6d7..0acf36ff4 100644 --- a/frontend/src/lib/logsContract.test.ts +++ b/frontend/src/lib/logsContract.test.ts @@ -106,6 +106,17 @@ describe('saved view HTTP contract', () => { : client.convertLogViewToSql('id'), ).rejects.toBeInstanceOf(ApiError); }); + it('accepts saved PromQL filters in the shared list without failing log views', async () => { + const promql: SavedLogView = { + ...body, + filter_id: 'promql-id', + stream_name: 'otel_metrics', + query: { filter_type: 'promql', filter_query: 'sum(rate(http_requests_total[5m]))' }, + }; + const saved = { ...body, filter_id: 'server-id' }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(json([saved, promql]))); + expect(await createClient({ mode: 'live' }).listLogViews()).toEqual([saved, promql]); + }); it('validates optional explorer state while retaining classic metadata', () => { for (const explorer_state of [undefined, null, 'encoded']) expect(savedLogView({ ...body, explorer_state, classic_metadata: { anything: true } })).toBe( diff --git a/frontend/src/lib/logsContract.ts b/frontend/src/lib/logsContract.ts index 3a04a96b9..43ce54fa5 100644 --- a/frontend/src/lib/logsContract.ts +++ b/frontend/src/lib/logsContract.ts @@ -40,7 +40,7 @@ export function savedLogView(value: unknown): value is SavedLogView { optionalString(value.explorer_state) && object(value.query) && typeof value.query.filter_type === 'string' && - ['filter', 'sql', 'search'].includes(value.query.filter_type) && + ['filter', 'sql', 'search', 'promql'].includes(value.query.filter_type) && optionalString(value.query.filter_query) && (value.query.filter_builder == null || builder(value.query.filter_builder)) && (value.time_filter == null || diff --git a/frontend/src/lib/types.ts b/frontend/src/lib/types.ts index d540b63df..fb55fb0e6 100644 --- a/frontend/src/lib/types.ts +++ b/frontend/src/lib/types.ts @@ -130,7 +130,7 @@ export type SavedLogView = { filter_name: string; stream_name: string; query: { - filter_type: 'filter' | 'sql' | 'search'; + filter_type: 'filter' | 'sql' | 'search' | 'promql'; filter_query?: string | null; filter_builder?: SavedLogViewBuilder | null; }; diff --git a/src/handlers/http/users/filters.rs b/src/handlers/http/users/filters.rs index 6f87e3ee2..a27438aba 100644 --- a/src/handlers/http/users/filters.rs +++ b/src/handlers/http/users/filters.rs @@ -306,7 +306,7 @@ mod tests { assert_eq!(converted, once); converted.stream_name = legacy.stream_name.clone(); assert_eq!(converted, legacy); - for kind in [FilterType::Filter, FilterType::Search] { + for kind in [FilterType::Filter, FilterType::Search, FilterType::Promql] { converted.query.filter_type = kind; let unchanged = converted.clone(); assert!(super::convert_filter_to_sql(&mut converted).is_err()); @@ -333,6 +333,16 @@ mod tests { assert!(validate_stream_name(SQL_FILTER_STREAM_NAME, &FilterType::Search).is_err()); } + #[test] + fn promql_filters_require_a_concrete_dataset_name() { + // PromQL evaluation is scoped to one dataset, and listing authorizes + // the saved filter by that dataset, so the SQL sentinel is not valid. + assert!(validate_stream_name("otel_metrics", &FilterType::Promql).is_ok()); + for stream_name in [SQL_FILTER_STREAM_NAME, "", "team1/serviceA", "../metrics"] { + assert!(validate_stream_name(stream_name, &FilterType::Promql).is_err()); + } + } + #[test] fn rejects_path_like_stream_names() { let stream_name = "team1/serviceA".to_owned(); diff --git a/src/users/filters.rs b/src/users/filters.rs index 7ae5a7b0a..bdddbe8a8 100644 --- a/src/users/filters.rs +++ b/src/users/filters.rs @@ -81,6 +81,7 @@ pub enum FilterType { Filter, SQL, Search, + Promql, } impl FilterType { @@ -89,6 +90,7 @@ impl FilterType { FilterType::Filter => "filter", FilterType::SQL => "sql", FilterType::Search => "search", + FilterType::Promql => "promql", } } } @@ -190,23 +192,26 @@ impl Filters { let query: &str = f.query.filter_query.as_deref().unwrap_or(""); let filter_type = &f.query.filter_type; - // if filter type is SQL, check if the user has access to the dataset based on the query string - // if filter type is search or filter, check if the user has access to the dataset based on the dataset name - if *filter_type == FilterType::SQL { - if (user_auth_for_query(key, query).await).is_ok() { - _filters.push(f.clone()) + match filter_type { + // SQL queries can reference multiple datasets, so authorize the query itself. + FilterType::SQL => { + if (user_auth_for_query(key, query).await).is_ok() { + _filters.push(f.clone()) + } } - } else if *filter_type == FilterType::Search || *filter_type == FilterType::Filter { - let dataset_name = &f.stream_name; - if user_auth_for_datasets( - &permissions, - std::slice::from_ref(dataset_name), - &tenant_id, - ) - .await - .is_ok() - { - _filters.push(f.clone()) + // Other filter types are scoped by the saved filter's dataset. + FilterType::Search | FilterType::Filter | FilterType::Promql => { + let dataset_name = &f.stream_name; + if user_auth_for_datasets( + &permissions, + std::slice::from_ref(dataset_name), + &tenant_id, + ) + .await + .is_ok() + { + _filters.push(f.clone()) + } } } } @@ -247,6 +252,63 @@ mod tests { })).unwrap() } + fn promql_filter(stream_name: &str) -> Filter { + serde_json::from_value(serde_json::json!({ + "version":"v2", "user_id":"owner", "filter_id":format!("promql-{stream_name}"), + "stream_name":stream_name, "filter_name":"Request rate", "tenant_id":null, + "query":{"filter_type":"promql","filter_query":"sum(rate(http_requests_total[5m]))","filter_builder":null}, + "time_filter":null + })).unwrap() + } + + // `rbac::map::init` needs `PARSEABLE`, so tests create the maps directly. + // A user without groups never reads `ROLES`, which other tests seed. + fn session_with(permissions: Vec) -> SessionKey { + use crate::rbac::map::{SESSIONS, Sessions, USER_GROUPS, USERS, UserGroups}; + USERS.get_or_init(|| parking_lot::RwLock::new(crate::rbac::map::Users::default())); + SESSIONS.get_or_init(|| parking_lot::RwLock::new(Sessions::default())); + USER_GROUPS.get_or_init(|| parking_lot::RwLock::new(UserGroups::default())); + let key = SessionKey::SessionId(ulid::Ulid::new()); + crate::rbac::map::mut_sessions().track_new( + format!("user-{key:?}"), + key.clone(), + chrono::DateTime::::MAX_UTC, + permissions, + &None, + ); + key + } + + #[test] + fn promql_filter_type_round_trips_as_lowercase() { + let filter = promql_filter("otel_metrics"); + assert_eq!(filter.query.filter_type, FilterType::Promql); + assert_eq!(filter.query.filter_type.to_str(), "promql"); + let value = serde_json::to_value(&filter).unwrap(); + assert_eq!(value["query"]["filter_type"], "promql"); + assert_eq!(serde_json::from_value::(value).unwrap(), filter); + } + + #[tokio::test] + async fn promql_filters_are_authorized_by_dataset_not_parsed_as_sql() { + use crate::rbac::role::{Action, Permission}; + let cache = Filters::default(); + let filter = promql_filter("otel_metrics"); + cache.update(&filter, &None).await; + + // PromQL is not SQL: routing it through SQL table extraction would + // fail and silently hide the filter from authorized users. + let reader = session_with(vec![Permission::Resource(Action::Query, None)]); + assert_eq!(cache.list_filters(&reader).await, vec![filter]); + + let unauthorized = session_with(vec![]); + assert!(cache.list_filters(&unauthorized).await.is_empty()); + + for key in [reader, unauthorized] { + crate::rbac::map::mut_sessions().remove_session(&key); + } + } + #[tokio::test] async fn saved_filter_cache_prefers_administrators_own_copy_of_shared_id() { let cache = Filters::default();