diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 15c6d42a9b6..27aca213e35 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -85,8 +85,14 @@ func (srv RulerSrv) RouteDeleteAlertRules(c *contextmodel.ReqContext, namespaceU "namespaceUid", namespace.UID, } - if group != "" { - loggerCtx = append(loggerCtx, "group", group) + + finalGroup, err := getRulesGroupParam(c, group) + if err != nil { + return ErrResp(http.StatusBadRequest, err, "") + } + + if finalGroup != "" { + loggerCtx = append(loggerCtx, "group", finalGroup) } logger := srv.log.New(loggerCtx...) @@ -97,11 +103,11 @@ func (srv RulerSrv) RouteDeleteAlertRules(c *contextmodel.ReqContext, namespaceU err = srv.xactManager.InTransaction(c.Req.Context(), func(ctx context.Context) error { deletionCandidates := map[ngmodels.AlertRuleGroupKey]ngmodels.RulesGroup{} - if group != "" { + if finalGroup != "" { key := ngmodels.AlertRuleGroupKey{ OrgID: c.SignedInUser.GetOrgID(), NamespaceUID: namespace.UID, - RuleGroup: group, + RuleGroup: finalGroup, } rules, err := srv.getAuthorizedRuleGroup(ctx, c, key) if err != nil { @@ -218,9 +224,14 @@ func (srv RulerSrv) RouteGetRulesGroupConfig(c *contextmodel.ReqContext, namespa return toNamespaceErrorResponse(err) } + finalRuleGroup, err := getRulesGroupParam(c, ruleGroup) + if err != nil { + return ErrResp(http.StatusBadRequest, err, "") + } + rules, err := srv.getAuthorizedRuleGroup(c.Req.Context(), c, ngmodels.AlertRuleGroupKey{ OrgID: c.SignedInUser.GetOrgID(), - RuleGroup: ruleGroup, + RuleGroup: finalRuleGroup, NamespaceUID: namespace.UID, }) if err != nil { @@ -234,7 +245,7 @@ func (srv RulerSrv) RouteGetRulesGroupConfig(c *contextmodel.ReqContext, namespa result := apimodels.RuleGroupConfigResponse{ // nolint:staticcheck - GettableRuleGroupConfig: toGettableRuleGroupConfig(ruleGroup, rules, provenanceRecords), + GettableRuleGroupConfig: toGettableRuleGroupConfig(finalRuleGroup, rules, provenanceRecords), } return response.JSON(http.StatusAccepted, result) } diff --git a/pkg/services/ngalert/api/lotex_ruler.go b/pkg/services/ngalert/api/lotex_ruler.go index bcffe81c63f..7e920dbc136 100644 --- a/pkg/services/ngalert/api/lotex_ruler.go +++ b/pkg/services/ngalert/api/lotex_ruler.go @@ -68,12 +68,18 @@ func (r *LotexRuler) RouteDeleteNamespaceRulesConfig(ctx *contextmodel.ReqContex if err != nil { return ErrResp(500, err, "") } + + finalNamespace, err := getRulesNamespaceParam(ctx, namespace) + if err != nil { + return ErrResp(http.StatusBadRequest, err, "") + } + return r.requester.withReq( ctx, http.MethodDelete, withPath( *ctx.Req.URL, - fmt.Sprintf("%s/%s", legacyRulerPrefix, url.PathEscape(namespace)), + fmt.Sprintf("%s/%s", legacyRulerPrefix, url.PathEscape(finalNamespace)), ), nil, messageExtractor, @@ -86,6 +92,17 @@ func (r *LotexRuler) RouteDeleteRuleGroupConfig(ctx *contextmodel.ReqContext, na if err != nil { return ErrResp(500, err, "") } + + finalNamespace, err := getRulesNamespaceParam(ctx, namespace) + if err != nil { + return ErrResp(http.StatusBadRequest, err, "") + } + + finalGroup, err := getRulesGroupParam(ctx, group) + if err != nil { + return ErrResp(http.StatusBadRequest, err, "") + } + return r.requester.withReq( ctx, http.MethodDelete, @@ -94,8 +111,8 @@ func (r *LotexRuler) RouteDeleteRuleGroupConfig(ctx *contextmodel.ReqContext, na fmt.Sprintf( "%s/%s/%s", legacyRulerPrefix, - url.PathEscape(namespace), - url.PathEscape(group), + url.PathEscape(finalNamespace), + url.PathEscape(finalGroup), ), ), nil, @@ -109,6 +126,12 @@ func (r *LotexRuler) RouteGetNamespaceRulesConfig(ctx *contextmodel.ReqContext, if err != nil { return ErrResp(500, err, "") } + + finalNamespace, err := getRulesNamespaceParam(ctx, namespace) + if err != nil { + return ErrResp(http.StatusBadRequest, err, "") + } + return r.requester.withReq( ctx, http.MethodGet, @@ -117,7 +140,7 @@ func (r *LotexRuler) RouteGetNamespaceRulesConfig(ctx *contextmodel.ReqContext, fmt.Sprintf( "%s/%s", legacyRulerPrefix, - url.PathEscape(namespace), + url.PathEscape(finalNamespace), ), ), nil, @@ -131,6 +154,17 @@ func (r *LotexRuler) RouteGetRulegGroupConfig(ctx *contextmodel.ReqContext, name if err != nil { return ErrResp(500, err, "") } + + finalNamespace, err := getRulesNamespaceParam(ctx, namespace) + if err != nil { + return ErrResp(http.StatusBadRequest, err, "") + } + + finalGroup, err := getRulesGroupParam(ctx, group) + if err != nil { + return ErrResp(http.StatusBadRequest, err, "") + } + return r.requester.withReq( ctx, http.MethodGet, @@ -139,8 +173,8 @@ func (r *LotexRuler) RouteGetRulegGroupConfig(ctx *contextmodel.ReqContext, name fmt.Sprintf( "%s/%s/%s", legacyRulerPrefix, - url.PathEscape(namespace), - url.PathEscape(group), + url.PathEscape(finalNamespace), + url.PathEscape(finalGroup), ), ), nil, @@ -177,7 +211,13 @@ func (r *LotexRuler) RoutePostNameRulesConfig(ctx *contextmodel.ReqContext, conf if err != nil { return ErrResp(500, err, "Failed marshal rule group") } - u := withPath(*ctx.Req.URL, fmt.Sprintf("%s/%s", legacyRulerPrefix, ns)) + + finalNamespace, err := getRulesNamespaceParam(ctx, ns) + if err != nil { + return ErrResp(http.StatusBadRequest, err, "") + } + + u := withPath(*ctx.Req.URL, fmt.Sprintf("%s/%s", legacyRulerPrefix, url.PathEscape(finalNamespace))) return r.requester.withReq(ctx, http.MethodPost, u, bytes.NewBuffer(yml), jsonExtractor(nil), nil) } diff --git a/pkg/services/ngalert/api/util.go b/pkg/services/ngalert/api/util.go index 5b81f27b30e..6c320715d19 100644 --- a/pkg/services/ngalert/api/util.go +++ b/pkg/services/ngalert/api/util.go @@ -26,6 +26,11 @@ import ( "github.com/grafana/grafana/pkg/web" ) +const ( + namespaceQueryTag = "QUERY_NAMESPACE" + groupQueryTag = "QUERY_GROUP" +) + var searchRegex = regexp.MustCompile(`\{(\w+)\}`) func toMacaronPath(path string) string { @@ -240,3 +245,29 @@ func getHash(hashSlice []string) uint64 { hash := sum.Sum64() return hash } + +func getRulesGroupParam(ctx *contextmodel.ReqContext, pathGroup string) (string, error) { + if pathGroup == groupQueryTag { + group := ctx.Query("group") + if group == "" { + return "", fmt.Errorf("group query parameter is empty") + } + + return group, nil + } + + return pathGroup, nil +} + +func getRulesNamespaceParam(ctx *contextmodel.ReqContext, pathNamespace string) (string, error) { + if pathNamespace == namespaceQueryTag { + namespace := ctx.Query("namespace") + if namespace == "" { + return "", fmt.Errorf("namespace query parameter is empty") + } + + return namespace, nil + } + + return pathNamespace, nil +} diff --git a/public/app/features/alerting/unified/api/alertRuleApi.ts b/public/app/features/alerting/unified/api/alertRuleApi.ts index fd8d5726ac0..3441dde01f9 100644 --- a/public/app/features/alerting/unified/api/alertRuleApi.ts +++ b/public/app/features/alerting/unified/api/alertRuleApi.ts @@ -27,7 +27,7 @@ import { FetchPromRulesFilter, groupRulesByFileName, paramsWithMatcherAndState, - prepareRulesFilterQueryParams, + getRulesFilterSearchParams, } from './prometheus'; import { FetchRulerRulesFilter, rulerUrlBuilder } from './ruler'; @@ -153,7 +153,8 @@ export const alertRuleApi = alertingApi.injectEndpoints({ searchParams.set(PrometheusAPIFilters.RuleGroup, identifier.groupName); } - const params = prepareRulesFilterQueryParams(searchParams, filter); + const filterParams = getRulesFilterSearchParams(filter); + const params = { ...filterParams, ...Object.fromEntries(searchParams) }; return { url: PROM_RULES_URL, params: paramsWithMatcherAndState(params, state, matcher) }; }, diff --git a/public/app/features/alerting/unified/api/prometheus.ts b/public/app/features/alerting/unified/api/prometheus.ts index 9a5c20bcd04..a18e1aa9294 100644 --- a/public/app/features/alerting/unified/api/prometheus.ts +++ b/public/app/features/alerting/unified/api/prometheus.ts @@ -37,8 +37,9 @@ export function prometheusUrlBuilder(dataSourceConfig: PrometheusDataSourceConfi searchParams.set('rule_group', identifier.groupName); } - const params = prepareRulesFilterQueryParams(searchParams, filter); + const filterParams = getRulesFilterSearchParams(filter); + const params = { ...filterParams, ...Object.fromEntries(searchParams) }; return { url: `/api/prometheus/${getDatasourceAPIUid(dataSourceName)}/api/v1/rules`, params: paramsWithMatcherAndState(params, state, matcher), @@ -47,18 +48,17 @@ export function prometheusUrlBuilder(dataSourceConfig: PrometheusDataSourceConfi }; } -export function prepareRulesFilterQueryParams( - params: URLSearchParams, - filter?: FetchPromRulesFilter -): Record { +export function getRulesFilterSearchParams(filter?: FetchPromRulesFilter): Record { + const filterParams: Record = {}; + if (filter?.dashboardUID) { - params.set('dashboard_uid', filter.dashboardUID); + filterParams.dashboard_uid = filter.dashboardUID; if (filter?.panelId) { - params.set('panel_id', String(filter.panelId)); + filterParams.panel_id = String(filter.panelId); } } - return Object.fromEntries(params); + return filterParams; } export function paramsWithMatcherAndState( diff --git a/public/app/features/alerting/unified/api/ruler.test.ts b/public/app/features/alerting/unified/api/ruler.test.ts index b31cdd3efd2..a4f69e49ee4 100644 --- a/public/app/features/alerting/unified/api/ruler.test.ts +++ b/public/app/features/alerting/unified/api/ruler.test.ts @@ -1,15 +1,28 @@ import { RulerDataSourceConfig } from 'app/types/unified-alerting'; -import { getDatasourceAPIUid } from '../utils/datasource'; +import { mockDataSource } from '../mocks'; +import { setupDataSources } from '../testSetup/datasources'; +import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; import { rulerUrlBuilder } from './ruler'; -jest.mock('../utils/datasource'); - -const mocks = { - getDatasourceAPIUId: jest.mocked(getDatasourceAPIUid), +const grafanaConfig: RulerDataSourceConfig = { + dataSourceName: GRAFANA_RULES_SOURCE_NAME, + apiVersion: 'legacy', }; +const mimirConfig: RulerDataSourceConfig = { + dataSourceName: 'Mimir-cloud', + apiVersion: 'config', +}; + +beforeAll(() => { + setupDataSources( + mockDataSource({ type: DataSourceType.Prometheus, name: 'Mimir-cloud', uid: 'mimir-1' }), + mockDataSource({ type: DataSourceType.Prometheus, name: 'Cortex', uid: 'cortex-1' }) + ); +}); + describe('rulerUrlBuilder', () => { it('Should use /api/v1/rules endpoint with subtype = cortex param for legacy api version', () => { // Arrange @@ -18,8 +31,6 @@ describe('rulerUrlBuilder', () => { apiVersion: 'legacy', }; - mocks.getDatasourceAPIUId.mockReturnValue('ds-uid'); - // Act const builder = rulerUrlBuilder(config); @@ -28,54 +39,38 @@ describe('rulerUrlBuilder', () => { const group = builder.namespaceGroup('test-ns', 'test-gr'); // Assert - expect(rules.path).toBe('/api/ruler/ds-uid/api/v1/rules'); + expect(rules.path).toBe('/api/ruler/cortex-1/api/v1/rules'); expect(rules.params).toMatchObject({ subtype: 'cortex' }); - expect(namespace.path).toBe('/api/ruler/ds-uid/api/v1/rules/test-ns'); + expect(namespace.path).toBe('/api/ruler/cortex-1/api/v1/rules/test-ns'); expect(namespace.params).toMatchObject({ subtype: 'cortex' }); - expect(group.path).toBe('/api/ruler/ds-uid/api/v1/rules/test-ns/test-gr'); + expect(group.path).toBe('/api/ruler/cortex-1/api/v1/rules/test-ns/test-gr'); expect(group.params).toMatchObject({ subtype: 'cortex' }); }); it('Should use /api/v1/rules endpoint with subtype = mimir parameter for config api version', () => { - // Arrange - const config: RulerDataSourceConfig = { - dataSourceName: 'Cortex v2', - apiVersion: 'config', - }; - - mocks.getDatasourceAPIUId.mockReturnValue('ds-uid'); - // Act - const builder = rulerUrlBuilder(config); + const builder = rulerUrlBuilder(mimirConfig); const rules = builder.rules(); const namespace = builder.namespace('test-ns'); const group = builder.namespaceGroup('test-ns', 'test-gr'); // Assert - expect(rules.path).toBe('/api/ruler/ds-uid/api/v1/rules'); + expect(rules.path).toBe('/api/ruler/mimir-1/api/v1/rules'); expect(rules.params).toMatchObject({ subtype: 'mimir' }); - expect(namespace.path).toBe('/api/ruler/ds-uid/api/v1/rules/test-ns'); + expect(namespace.path).toBe('/api/ruler/mimir-1/api/v1/rules/test-ns'); expect(namespace.params).toMatchObject({ subtype: 'mimir' }); - expect(group.path).toBe('/api/ruler/ds-uid/api/v1/rules/test-ns/test-gr'); + expect(group.path).toBe('/api/ruler/mimir-1/api/v1/rules/test-ns/test-gr'); expect(group.params).toMatchObject({ subtype: 'mimir' }); }); - it('Should append source=rules parameter when custom ruler enabled', () => { - // Arrange - const config: RulerDataSourceConfig = { - dataSourceName: 'Cortex v2', - apiVersion: 'config', - }; - - mocks.getDatasourceAPIUId.mockReturnValue('ds-uid'); - + it('Should append subtype parameter when custom ruler enabled', () => { // Act - const builder = rulerUrlBuilder(config); + const builder = rulerUrlBuilder(mimirConfig); const rules = builder.rules(); const namespace = builder.namespace('test-ns'); @@ -88,19 +83,52 @@ describe('rulerUrlBuilder', () => { }); it('Should append dashboard_uid and panel_id for rules endpoint when specified', () => { - // Arrange - const config: RulerDataSourceConfig = { - dataSourceName: 'Cortex v2', - apiVersion: 'config', - }; - - mocks.getDatasourceAPIUId.mockReturnValue('ds-uid'); - // Act - const builder = rulerUrlBuilder(config); + const builder = rulerUrlBuilder(mimirConfig); const rules = builder.rules({ dashboardUID: 'dashboard-uid', panelId: 1234 }); // Assert expect(rules.params).toMatchObject({ dashboard_uid: 'dashboard-uid', panel_id: '1234', subtype: 'mimir' }); }); + + describe('When slash in namespace or group', () => { + it('Should use QUERY_NAMESPACE and QUERY_GROUP path placeholders and include names in query string params', () => { + // Act + const builder = rulerUrlBuilder(mimirConfig); + + const namespace = builder.namespace('test/ns'); + const group = builder.namespaceGroup('test/ns', 'test/gr'); + + // Assert + expect(namespace.path).toBe('/api/ruler/mimir-1/api/v1/rules/QUERY_NAMESPACE'); + expect(namespace.params).toMatchObject({ subtype: 'mimir', namespace: 'test/ns' }); + + expect(group.path).toBe('/api/ruler/mimir-1/api/v1/rules/QUERY_NAMESPACE/QUERY_GROUP'); + expect(group.params).toMatchObject({ subtype: 'mimir', namespace: 'test/ns', group: 'test/gr' }); + }); + + it('Should use the tag replacement only when the slash is present', () => { + // Act + const builder = rulerUrlBuilder(mimirConfig); + + const group = builder.namespaceGroup('test-ns', 'test/gr'); + + // Assert + expect(group.path).toBe('/api/ruler/mimir-1/api/v1/rules/test-ns/QUERY_GROUP'); + expect(group.params).toMatchObject({ subtype: 'mimir', group: 'test/gr' }); + }); + + // GMA uses folderUIDs as namespaces and they should never contain slashes + it('Should only replace the group segment for Grafana-managed rules', () => { + // Act + const builder = rulerUrlBuilder(grafanaConfig); + + const group = builder.namespaceGroup('test/ns', 'test/gr'); + + // Assert + expect(group.path).toBe(`/api/ruler/grafana/api/v1/rules/${encodeURIComponent('test/ns')}/QUERY_GROUP`); + expect(group.params).toHaveProperty('group'); + expect(group.params).not.toHaveProperty('namespace'); + }); + }); }); diff --git a/public/app/features/alerting/unified/api/ruler.ts b/public/app/features/alerting/unified/api/ruler.ts index 6426b828db5..331d45154d0 100644 --- a/public/app/features/alerting/unified/api/ruler.ts +++ b/public/app/features/alerting/unified/api/ruler.ts @@ -5,10 +5,11 @@ import { FetchResponse, getBackendSrv } from '@grafana/runtime'; import { RulerDataSourceConfig } from 'app/types/unified-alerting'; import { PostableRulerRuleGroupDTO, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; +import { checkForPathSeparator } from '../components/rule-editor/util'; import { RULER_NOT_SUPPORTED_MSG } from '../utils/constants'; import { getDatasourceAPIUid, GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; -import { prepareRulesFilterQueryParams } from './prometheus'; +import { getRulesFilterSearchParams } from './prometheus'; interface ErrorResponseMessage { message?: string; @@ -20,34 +21,92 @@ export interface RulerRequestUrl { params?: Record; } +const QUERY_NAMESPACE_TAG = 'QUERY_NAMESPACE'; +const QUERY_GROUP_TAG = 'QUERY_GROUP'; + export function rulerUrlBuilder(rulerConfig: RulerDataSourceConfig) { - const grafanaServerPath = `/api/ruler/${getDatasourceAPIUid(rulerConfig.dataSourceName)}`; + const rulerPath = getRulerPath(rulerConfig); + const queryDetailsProvider = getQueryDetailsProvider(rulerConfig); - const rulerPath = `${grafanaServerPath}/api/v1/rules`; - const rulerSearchParams = new URLSearchParams(); - - rulerSearchParams.set('subtype', rulerConfig.apiVersion === 'legacy' ? 'cortex' : 'mimir'); + const subtype = rulerConfig.apiVersion === 'legacy' ? 'cortex' : 'mimir'; return { - rules: (filter?: FetchRulerRulesFilter): RulerRequestUrl => { - const params = prepareRulesFilterQueryParams(rulerSearchParams, filter); + rules: (filter?: FetchRulerRulesFilter): RulerRequestUrl => ({ + path: rulerPath, + params: { subtype, ...getRulesFilterSearchParams(filter) }, + }), + + namespace: (namespace: string): RulerRequestUrl => { + // To handle slashes we need to convert namespace to a query parameter + const { namespace: finalNs, searchParams: nsParams } = queryDetailsProvider.namespace(namespace); return { - path: `${rulerPath}`, - params: params, + path: `${rulerPath}/${encodeURIComponent(finalNs)}`, + params: { subtype, ...nsParams }, + }; + }, + + namespaceGroup: (namespaceUID: string, group: string): RulerRequestUrl => { + const { namespace: finalNs, searchParams: nsParams } = queryDetailsProvider.namespace(namespaceUID); + const { group: finalGroup, searchParams: groupParams } = queryDetailsProvider.group(group); + + return { + path: `${rulerPath}/${encodeURIComponent(finalNs)}/${encodeURIComponent(finalGroup)}`, + params: { subtype, ...nsParams, ...groupParams }, }; }, - namespace: (namespace: string): RulerRequestUrl => ({ - path: `${rulerPath}/${encodeURIComponent(namespace)}`, - params: Object.fromEntries(rulerSearchParams), - }), - namespaceGroup: (namespaceUID: string, group: string): RulerRequestUrl => ({ - path: `${rulerPath}/${encodeURIComponent(namespaceUID)}/${encodeURIComponent(group)}`, - params: Object.fromEntries(rulerSearchParams), - }), }; } +interface NamespaceUrlParams { + namespace: string; + searchParams: Record; +} + +interface GroupUrlParams { + group: string; + searchParams: Record; +} + +interface RulerQueryDetailsProvider { + namespace: (namespace: string) => NamespaceUrlParams; + group: (group: string) => GroupUrlParams; +} + +function getQueryDetailsProvider(rulerConfig: RulerDataSourceConfig): RulerQueryDetailsProvider { + const isGrafanaDatasource = rulerConfig.dataSourceName === GRAFANA_RULES_SOURCE_NAME; + + const groupParamRewrite = (group: string): GroupUrlParams => { + if (checkForPathSeparator(group) !== true) { + return { group: QUERY_GROUP_TAG, searchParams: { group } }; + } + return { group, searchParams: {} }; + }; + + // GMA uses folderUID as namespace identifiers so we need to rewrite them + if (isGrafanaDatasource) { + return { + namespace: (namespace: string) => ({ namespace, searchParams: {} }), + group: groupParamRewrite, + }; + } + + return { + namespace: (namespace: string): NamespaceUrlParams => { + if (checkForPathSeparator(namespace) !== true) { + return { namespace: QUERY_NAMESPACE_TAG, searchParams: { namespace } }; + } + return { namespace, searchParams: {} }; + }, + group: groupParamRewrite, + }; +} + +function getRulerPath(rulerConfig: RulerDataSourceConfig) { + const grafanaServerPath = `/api/ruler/${getDatasourceAPIUid(rulerConfig.dataSourceName)}`; + return `${grafanaServerPath}/api/v1/rules`; +} + // upsert a rule group. use this to update rule export async function setRulerRuleGroup( rulerConfig: RulerDataSourceConfig,