Alerting: Escape namespace and group path parameters (#80504)

Co-authored-by: Jean-Philippe Quéméner <JohnnyQQQQ@users.noreply.github.com>
This commit is contained in:
Sven Kirschbaum 2024-02-16 09:43:47 +01:00 committed by GitHub
parent 69f604f7fa
commit 86c618a6d6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 286 additions and 14 deletions

View File

@ -3,6 +3,7 @@ package api
import (
"bytes"
"fmt"
"io"
"net/http"
"net/url"
@ -43,15 +44,22 @@ var subtypeToPrefix = map[string]string{
Mimir: mimirPrefix,
}
// The requester is primarily used for testing purposes, allowing us to inject a different implementation of withReq.
type requester interface {
withReq(ctx *contextmodel.ReqContext, method string, u *url.URL, body io.Reader, extractor func(*response.NormalResponse) (any, error), headers map[string]string) response.Response
}
type LotexRuler struct {
log log.Logger
*AlertingProxy
requester requester
}
func NewLotexRuler(proxy *AlertingProxy, log log.Logger) *LotexRuler {
return &LotexRuler{
log: log,
AlertingProxy: proxy,
requester: proxy,
}
}
@ -60,12 +68,12 @@ func (r *LotexRuler) RouteDeleteNamespaceRulesConfig(ctx *contextmodel.ReqContex
if err != nil {
return ErrResp(500, err, "")
}
return r.withReq(
return r.requester.withReq(
ctx,
http.MethodDelete,
withPath(
*ctx.Req.URL,
fmt.Sprintf("%s/%s", legacyRulerPrefix, namespace),
fmt.Sprintf("%s/%s", legacyRulerPrefix, url.PathEscape(namespace)),
),
nil,
messageExtractor,
@ -78,7 +86,7 @@ func (r *LotexRuler) RouteDeleteRuleGroupConfig(ctx *contextmodel.ReqContext, na
if err != nil {
return ErrResp(500, err, "")
}
return r.withReq(
return r.requester.withReq(
ctx,
http.MethodDelete,
withPath(
@ -86,8 +94,8 @@ func (r *LotexRuler) RouteDeleteRuleGroupConfig(ctx *contextmodel.ReqContext, na
fmt.Sprintf(
"%s/%s/%s",
legacyRulerPrefix,
namespace,
group,
url.PathEscape(namespace),
url.PathEscape(group),
),
),
nil,
@ -101,7 +109,7 @@ func (r *LotexRuler) RouteGetNamespaceRulesConfig(ctx *contextmodel.ReqContext,
if err != nil {
return ErrResp(500, err, "")
}
return r.withReq(
return r.requester.withReq(
ctx,
http.MethodGet,
withPath(
@ -109,7 +117,7 @@ func (r *LotexRuler) RouteGetNamespaceRulesConfig(ctx *contextmodel.ReqContext,
fmt.Sprintf(
"%s/%s",
legacyRulerPrefix,
namespace,
url.PathEscape(namespace),
),
),
nil,
@ -123,7 +131,7 @@ func (r *LotexRuler) RouteGetRulegGroupConfig(ctx *contextmodel.ReqContext, name
if err != nil {
return ErrResp(500, err, "")
}
return r.withReq(
return r.requester.withReq(
ctx,
http.MethodGet,
withPath(
@ -131,8 +139,8 @@ func (r *LotexRuler) RouteGetRulegGroupConfig(ctx *contextmodel.ReqContext, name
fmt.Sprintf(
"%s/%s/%s",
legacyRulerPrefix,
namespace,
group,
url.PathEscape(namespace),
url.PathEscape(group),
),
),
nil,
@ -147,7 +155,7 @@ func (r *LotexRuler) RouteGetRulesConfig(ctx *contextmodel.ReqContext) response.
return ErrResp(500, err, "")
}
return r.withReq(
return r.requester.withReq(
ctx,
http.MethodGet,
withPath(
@ -170,7 +178,7 @@ func (r *LotexRuler) RoutePostNameRulesConfig(ctx *contextmodel.ReqContext, conf
return ErrResp(500, err, "Failed marshal rule group")
}
u := withPath(*ctx.Req.URL, fmt.Sprintf("%s/%s", legacyRulerPrefix, ns))
return r.withReq(ctx, http.MethodPost, u, bytes.NewBuffer(yml), jsonExtractor(nil), nil)
return r.requester.withReq(ctx, http.MethodPost, u, bytes.NewBuffer(yml), jsonExtractor(nil), nil)
}
func (r *LotexRuler) validateAndGetPrefix(ctx *contextmodel.ReqContext) (string, error) {
@ -216,7 +224,8 @@ func (r *LotexRuler) validateAndGetPrefix(ctx *contextmodel.ReqContext) (string,
}
func withPath(u url.URL, newPath string) *url.URL {
// TODO: handle path escaping
u.Path = newPath
u.Path, _ = url.PathUnescape(newPath)
u.RawPath = newPath
return &u
}

View File

@ -3,11 +3,15 @@ package api
import (
"context"
"errors"
"io"
"net/http"
"net/url"
"testing"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/auth/identity"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
@ -129,3 +133,262 @@ func (f fakeCacheService) GetDatasourceByUID(ctx context.Context, datasourceUID
return f.datasource, nil
}
func TestLotexRuler_RouteDeleteNamespaceRulesConfig(t *testing.T) {
tc := []struct {
name string
namespace string
expected string
urlParams string
namedParams map[string]string
datasource *datasources.DataSource
}{
{
name: "with a namespace that has to be escaped",
namespace: "namespace/with/slashes",
expected: "http://mimir.com/config/v1/rules/namespace%2Fwith%2Fslashes?subtype=mimir",
urlParams: "?subtype=mimir",
namedParams: map[string]string{":DatasourceUID": "d164"},
datasource: &datasources.DataSource{URL: "http://mimir.com", Type: PrometheusDatasourceType},
},
{
name: "with a namespace that does not need to be escaped",
namespace: "namespace_without_slashes",
expected: "http://mimir.com/config/v1/rules/namespace_without_slashes?subtype=mimir",
urlParams: "?subtype=mimir",
namedParams: map[string]string{":DatasourceUID": "d164"},
datasource: &datasources.DataSource{URL: "http://mimir.com", Type: PrometheusDatasourceType},
},
}
for _, tt := range tc {
t.Run(tt.name, func(t *testing.T) {
requestMock := RequestMock{}
defer requestMock.AssertExpectations(t)
requestMock.On(
"withReq",
mock.Anything,
mock.Anything,
mock.AnythingOfType("*url.URL"),
mock.Anything,
mock.Anything,
mock.Anything,
).Return(response.Empty(200)).Run(func(args mock.Arguments) {
// Validate that the full url as string is equal to the expected value
require.Equal(t, tt.expected, args.Get(2).(*url.URL).String())
})
// Setup Proxy.
proxy := &AlertingProxy{DataProxy: &datasourceproxy.DataSourceProxyService{DataSourceCache: fakeCacheService{datasource: tt.datasource}}}
ruler := &LotexRuler{AlertingProxy: proxy, log: log.NewNopLogger(), requester: &requestMock}
// Setup request context.
httpReq, err := http.NewRequest(http.MethodGet, tt.datasource.URL+tt.urlParams, nil)
require.NoError(t, err)
ctx := &contextmodel.ReqContext{Context: &web.Context{Req: web.SetURLParams(httpReq, tt.namedParams)}}
ruler.RouteDeleteNamespaceRulesConfig(ctx, tt.namespace)
})
}
}
func TestLotexRuler_RouteDeleteRuleGroupConfig(t *testing.T) {
tc := []struct {
name string
namespace string
group string
expected string
urlParams string
namedParams map[string]string
datasource *datasources.DataSource
}{
{
name: "with a namespace that has to be escaped",
namespace: "namespace/with/slashes",
group: "group/with/slashes",
expected: "http://mimir.com/config/v1/rules/namespace%2Fwith%2Fslashes/group%2Fwith%2Fslashes?subtype=mimir",
urlParams: "?subtype=mimir",
namedParams: map[string]string{":DatasourceUID": "d164"},
datasource: &datasources.DataSource{URL: "http://mimir.com", Type: PrometheusDatasourceType},
},
{
name: "with a namespace that does not need to be escaped",
namespace: "namespace_without_slashes",
group: "group_without_slashes",
expected: "http://mimir.com/config/v1/rules/namespace_without_slashes/group_without_slashes?subtype=mimir",
urlParams: "?subtype=mimir",
namedParams: map[string]string{":DatasourceUID": "d164"},
datasource: &datasources.DataSource{URL: "http://mimir.com", Type: PrometheusDatasourceType},
},
}
for _, tt := range tc {
t.Run(tt.name, func(t *testing.T) {
requestMock := RequestMock{}
defer requestMock.AssertExpectations(t)
requestMock.On(
"withReq",
mock.Anything,
mock.Anything,
mock.AnythingOfType("*url.URL"),
mock.Anything,
mock.Anything,
mock.Anything,
).Return(response.Empty(200)).Run(func(args mock.Arguments) {
// Validate that the full url as string is equal to the expected value
require.Equal(t, tt.expected, args.Get(2).(*url.URL).String())
})
// Setup Proxy.
proxy := &AlertingProxy{DataProxy: &datasourceproxy.DataSourceProxyService{DataSourceCache: fakeCacheService{datasource: tt.datasource}}}
ruler := &LotexRuler{AlertingProxy: proxy, log: log.NewNopLogger(), requester: &requestMock}
// Setup request context.
httpReq, err := http.NewRequest(http.MethodGet, tt.datasource.URL+tt.urlParams, nil)
require.NoError(t, err)
ctx := &contextmodel.ReqContext{Context: &web.Context{Req: web.SetURLParams(httpReq, tt.namedParams)}}
ruler.RouteDeleteRuleGroupConfig(ctx, tt.namespace, tt.group)
})
}
}
func TestLotexRuler_RouteGetNamespaceRulesConfig(t *testing.T) {
tc := []struct {
name string
namespace string
group string
expected string
urlParams string
namedParams map[string]string
datasource *datasources.DataSource
}{
{
name: "with a namespace that has to be escaped",
namespace: "namespace/with/slashes",
expected: "http://mimir.com/config/v1/rules/namespace%2Fwith%2Fslashes?subtype=mimir",
urlParams: "?subtype=mimir",
namedParams: map[string]string{":DatasourceUID": "d164"},
datasource: &datasources.DataSource{URL: "http://mimir.com", Type: PrometheusDatasourceType},
},
{
name: "with a namespace that does not need to be escaped",
namespace: "namespace_without_slashes",
expected: "http://mimir.com/config/v1/rules/namespace_without_slashes?subtype=mimir",
urlParams: "?subtype=mimir",
namedParams: map[string]string{":DatasourceUID": "d164"},
datasource: &datasources.DataSource{URL: "http://mimir.com", Type: PrometheusDatasourceType},
},
}
for _, tt := range tc {
t.Run(tt.name, func(t *testing.T) {
requestMock := RequestMock{}
defer requestMock.AssertExpectations(t)
requestMock.On(
"withReq",
mock.Anything,
mock.Anything,
mock.AnythingOfType("*url.URL"),
mock.Anything,
mock.Anything,
mock.Anything,
).Return(response.Empty(200)).Run(func(args mock.Arguments) {
// Validate that the full url as string is equal to the expected value
require.Equal(t, tt.expected, args.Get(2).(*url.URL).String())
})
// Setup Proxy.
proxy := &AlertingProxy{DataProxy: &datasourceproxy.DataSourceProxyService{DataSourceCache: fakeCacheService{datasource: tt.datasource}}}
ruler := &LotexRuler{AlertingProxy: proxy, log: log.NewNopLogger(), requester: &requestMock}
// Setup request context.
httpReq, err := http.NewRequest(http.MethodGet, tt.datasource.URL+tt.urlParams, nil)
require.NoError(t, err)
ctx := &contextmodel.ReqContext{Context: &web.Context{Req: web.SetURLParams(httpReq, tt.namedParams)}}
ruler.RouteGetNamespaceRulesConfig(ctx, tt.namespace)
})
}
}
func TestLotexRuler_RouteGetRulegGroupConfig(t *testing.T) {
tc := []struct {
name string
namespace string
group string
expected string
urlParams string
namedParams map[string]string
datasource *datasources.DataSource
}{
{
name: "with a namespace that has to be escaped",
namespace: "namespace/with/slashes",
group: "group/with/slashes",
expected: "http://mimir.com/config/v1/rules/namespace%2Fwith%2Fslashes/group%2Fwith%2Fslashes?subtype=mimir",
urlParams: "?subtype=mimir",
namedParams: map[string]string{":DatasourceUID": "d164"},
datasource: &datasources.DataSource{URL: "http://mimir.com", Type: PrometheusDatasourceType},
},
{
name: "with a namespace that does not need to be escaped",
namespace: "namespace_without_slashes",
group: "group_without_slashes",
expected: "http://mimir.com/config/v1/rules/namespace_without_slashes/group_without_slashes?subtype=mimir",
urlParams: "?subtype=mimir",
namedParams: map[string]string{":DatasourceUID": "d164"},
datasource: &datasources.DataSource{URL: "http://mimir.com", Type: PrometheusDatasourceType},
},
}
for _, tt := range tc {
t.Run(tt.name, func(t *testing.T) {
requestMock := RequestMock{}
defer requestMock.AssertExpectations(t)
requestMock.On(
"withReq",
mock.Anything,
mock.Anything,
mock.AnythingOfType("*url.URL"),
mock.Anything,
mock.Anything,
mock.Anything,
).Return(response.Empty(200)).Run(func(args mock.Arguments) {
// Validate that the full url as string is equal to the expected value
require.Equal(t, tt.expected, args.Get(2).(*url.URL).String())
})
// Setup Proxy.
proxy := &AlertingProxy{DataProxy: &datasourceproxy.DataSourceProxyService{DataSourceCache: fakeCacheService{datasource: tt.datasource}}}
ruler := &LotexRuler{AlertingProxy: proxy, log: log.NewNopLogger(), requester: &requestMock}
// Setup request context.
httpReq, err := http.NewRequest(http.MethodGet, tt.datasource.URL+tt.urlParams, nil)
require.NoError(t, err)
ctx := &contextmodel.ReqContext{Context: &web.Context{Req: web.SetURLParams(httpReq, tt.namedParams)}}
ruler.RouteGetRulegGroupConfig(ctx, tt.namespace, tt.group)
})
}
}
type RequestMock struct {
mock.Mock
}
func (a *RequestMock) withReq(
ctx *contextmodel.ReqContext,
method string,
u *url.URL,
body io.Reader,
extractor func(*response.NormalResponse) (any, error),
headers map[string]string,
) response.Response {
args := a.Called(ctx, method, u, body, extractor, headers)
return args.Get(0).(response.Response)
}