Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions frontend/src/features/logs/SavedViews.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
7 changes: 5 additions & 2 deletions frontend/src/features/logs/SavedViews.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 }]
: [];
}
Expand Down
11 changes: 11 additions & 0 deletions frontend/src/features/sql/serverSavedQueries.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions frontend/src/lib/logsContract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/logsContract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
12 changes: 11 additions & 1 deletion src/handlers/http/users/filters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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();
Expand Down
94 changes: 78 additions & 16 deletions src/users/filters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ pub enum FilterType {
Filter,
SQL,
Search,
Promql,
}

impl FilterType {
Expand All @@ -89,6 +90,7 @@ impl FilterType {
FilterType::Filter => "filter",
FilterType::SQL => "sql",
FilterType::Search => "search",
FilterType::Promql => "promql",
}
}
}
Expand Down Expand Up @@ -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())
}
}
}
}
Expand Down Expand Up @@ -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<crate::rbac::role::Permission>) -> 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::<chrono::Utc>::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::<Filter>(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();
Expand Down
Loading