From 6f245121d09cc5cfb048334316c6e8bf60f8b635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Giedrius=20Statkevi=C4=8Dius?= Date: Mon, 29 Jan 2024 11:31:49 +0200 Subject: [PATCH] Plugins: Fix colon in CallResource URL returning an error when creating plugin resource request (#79746) * Plugin: handle colon character in path url.Parse() does not handle the given input correctly when the input contains a colon character. The user will see the following error message when trying to use remote cluster in Elasticsearch: ``` level=warn msg="Failed for create plugin resource request" error="parse \"foo-*,*:foo-*/_mapping\": first path segment in URL cannot contain colon" traceID= ``` As far as I can tell, we only want to set the path here + rawquery so avoid url.Parse() altogether. * Add more tests --------- Co-authored-by: Giuseppe Guerra --- pkg/api/plugin_resource.go | 11 ++-- pkg/api/plugin_resource_test.go | 89 ++++++++++++++++++++++++++++++--- 2 files changed, 87 insertions(+), 13 deletions(-) diff --git a/pkg/api/plugin_resource.go b/pkg/api/plugin_resource.go index 814f0329f8f..6f0530effcd 100644 --- a/pkg/api/plugin_resource.go +++ b/pkg/api/plugin_resource.go @@ -90,14 +90,11 @@ func (hs *HTTPServer) callPluginResourceWithDataSource(c *contextmodel.ReqContex func (hs *HTTPServer) pluginResourceRequest(c *contextmodel.ReqContext) (*http.Request, error) { clonedReq := c.Req.Clone(c.Req.Context()) rawURL := web.Params(c.Req)["*"] - if clonedReq.URL.RawQuery != "" { - rawURL += "?" + clonedReq.URL.RawQuery + + clonedReq.URL = &url.URL{ + Path: rawURL, + RawQuery: clonedReq.URL.RawQuery, } - urlPath, err := url.Parse(rawURL) - if err != nil { - return nil, err - } - clonedReq.URL = urlPath return clonedReq, nil } diff --git a/pkg/api/plugin_resource_test.go b/pkg/api/plugin_resource_test.go index 0d7d7fbe96b..9fb4532019c 100644 --- a/pkg/api/plugin_resource_test.go +++ b/pkg/api/plugin_resource_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "io" + "net/url" "path/filepath" "strings" "testing" @@ -53,22 +54,22 @@ func TestCallResource(t *testing.T) { coreRegistry := coreplugin.ProvideCoreRegistry(tracing.InitializeTracerForTest(), nil, &cloudwatch.CloudWatchService{}, nil, nil, nil, nil, nil, nil, nil, nil, testdatasource.ProvideService(), nil, nil, nil, nil, nil, nil) - textCtx := pluginsintegration.CreateIntegrationTestCtx(t, cfg, coreRegistry) + testCtx := pluginsintegration.CreateIntegrationTestCtx(t, cfg, coreRegistry) - pcp := plugincontext.ProvideService(cfg, localcache.ProvideService(), textCtx.PluginStore, &datasources.FakeCacheService{}, + pcp := plugincontext.ProvideService(cfg, localcache.ProvideService(), testCtx.PluginStore, &datasources.FakeCacheService{}, &datasources.FakeDataSourceService{}, pluginSettings.ProvideService(db.InitTestDB(t), fakeSecrets.NewFakeSecretsService()), nil, &pCfg) srv := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = cfg hs.pluginContextProvider = pcp hs.QuotaService = quotatest.New(false, nil) - hs.pluginStore = textCtx.PluginStore - hs.pluginClient = textCtx.PluginClient + hs.pluginStore = testCtx.PluginStore + hs.pluginClient = testCtx.PluginClient hs.log = log.New("test") }) t.Run("Test successful response is received for valid request", func(t *testing.T) { - req := srv.NewPostRequest("/api/plugins/grafana-testdata-datasource/resources/test", strings.NewReader("{ \"test\": true }")) + req := srv.NewPostRequest("/api/plugins/grafana-testdata-datasource/resources/test", strings.NewReader(`{"test": "true"}`)) webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{ 1: accesscontrol.GroupScopesByAction([]accesscontrol.Permission{ {Action: pluginaccesscontrol.ActionAppAccess, Scope: pluginaccesscontrol.ScopeProvider.GetResourceAllScope()}, @@ -88,6 +89,82 @@ func TestCallResource(t *testing.T) { require.NoError(t, resp.Body.Close()) require.Equal(t, 200, resp.StatusCode) }) + + t.Run("Test successful response is received for valid request with the colon character", func(t *testing.T) { + req := srv.NewPostRequest("/api/plugins/grafana-testdata-datasource/resources/test-*,*:test-*/_mapping", strings.NewReader(`{"test": "true"}`)) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{ + 1: accesscontrol.GroupScopesByAction([]accesscontrol.Permission{ + {Action: pluginaccesscontrol.ActionAppAccess, Scope: pluginaccesscontrol.ScopeProvider.GetResourceAllScope()}, + }), + }}) + resp, err := srv.SendJSON(req) + require.NoError(t, err) + + require.NoError(t, resp.Body.Close()) + require.Equal(t, 200, resp.StatusCode) + }) + + t.Run("CallResource plugin resource request is created correctly", func(t *testing.T) { + type testdataCallResourceTestResponse struct { + Message string `json:"message"` + Request struct { + URL url.URL + Body map[string]any `json:"body"` + } `json:"request"` + } + + for _, tc := range []struct { + name string + url string + exp func(t *testing.T, resp testdataCallResourceTestResponse) + }{ + { + name: "Simple URL", + url: "/api/plugins/grafana-testdata-datasource/resources/test", + exp: func(t *testing.T, resp testdataCallResourceTestResponse) { + require.Equal(t, "Hello world from test datasource!", resp.Message) + require.Equal(t, "/test", resp.Request.URL.Path) + require.Equal(t, "true", resp.Request.Body["test"]) + require.Len(t, resp.Request.Body, 1) + require.Empty(t, resp.Request.URL.RawQuery) + require.Empty(t, resp.Request.URL.Query()) + }, + }, + { + name: "URL with query params", + url: "/api/plugins/grafana-testdata-datasource/resources/test?test=true&a=b", + exp: func(t *testing.T, resp testdataCallResourceTestResponse) { + require.Equal(t, "Hello world from test datasource!", resp.Message) + require.Equal(t, "/test", resp.Request.URL.Path) + require.Equal(t, "test=true&a=b", resp.Request.URL.RawQuery) + query := resp.Request.URL.Query() + require.Equal(t, "true", query.Get("test")) + require.Equal(t, "b", query.Get("a")) + require.Len(t, query, 2) + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + req := srv.NewPostRequest(tc.url, strings.NewReader(`{"test": "true"}`)) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{ + 1: accesscontrol.GroupScopesByAction([]accesscontrol.Permission{ + {Action: pluginaccesscontrol.ActionAppAccess, Scope: pluginaccesscontrol.ScopeProvider.GetResourceAllScope()}, + }), + }}) + resp, err := srv.SendJSON(req) + require.NoError(t, err) + + var body testdataCallResourceTestResponse + require.NoError(t, json.NewDecoder(resp.Body).Decode(&body)) + + tc.exp(t, body) + + require.NoError(t, resp.Body.Close()) + require.Equal(t, 200, resp.StatusCode) + }) + } + }) + pluginRegistry := fakes.NewFakePluginRegistry() require.NoError(t, pluginRegistry.Add(context.Background(), &plugins.Plugin{ JSONData: plugins.JSONData{ @@ -108,7 +185,7 @@ func TestCallResource(t *testing.T) { hs.Cfg = cfg hs.pluginContextProvider = pcp hs.QuotaService = quotatest.New(false, nil) - hs.pluginStore = textCtx.PluginStore + hs.pluginStore = testCtx.PluginStore hs.pluginClient = pc hs.log = log.New("test") })