From 082f020b7d8d7a4c6d3d9d90810aa9216265989a Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Tue, 13 Feb 2024 13:44:08 +0100 Subject: [PATCH] Elasticsearch: Fix resource calls for paths that include `:` (#82327) * Elasticsearch: Fix resource calls for paths that include : * Add tests * Add test case and comment * Remove redundant comment --- pkg/tsdb/elasticsearch/elasticsearch.go | 24 ++++++++-------- pkg/tsdb/elasticsearch/elasticsearch_test.go | 30 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/pkg/tsdb/elasticsearch/elasticsearch.go b/pkg/tsdb/elasticsearch/elasticsearch.go index a60af5d9793..7a064154876 100644 --- a/pkg/tsdb/elasticsearch/elasticsearch.go +++ b/pkg/tsdb/elasticsearch/elasticsearch.go @@ -205,21 +205,11 @@ func (s *Service) CallResource(ctx context.Context, req *backend.CallResourceReq return err } - esUrl, err := url.Parse(ds.URL) + esUrl, err := createElasticsearchURL(req, ds) if err != nil { - logger.Error("Failed to parse data source URL", "error", err, "url", ds.URL) - return err + logger.Error("Failed to create request url", "error", err, "url", ds.URL, "path", req.Path) } - resourcePath, err := url.Parse(req.Path) - if err != nil { - logger.Error("Failed to parse data source path", "error", err, "url", req.Path) - return err - } - - // We take the path and the query-string only - esUrl.RawQuery = resourcePath.RawQuery - esUrl.Path = path.Join(esUrl.Path, resourcePath.Path) request, err := http.NewRequestWithContext(ctx, req.Method, esUrl.String(), bytes.NewBuffer(req.Body)) if err != nil { logger.Error("Failed to create request", "error", err, "url", esUrl.String()) @@ -269,3 +259,13 @@ func (s *Service) CallResource(ctx context.Context, req *backend.CallResourceReq Body: body, }) } + +func createElasticsearchURL(req *backend.CallResourceRequest, ds *es.DatasourceInfo) (*url.URL, error) { + esUrl, err := url.Parse(ds.URL) + if err != nil { + return nil, fmt.Errorf("failed to parse data source URL: %s, error: %w", ds.URL, err) + } + + esUrl.Path = path.Join(esUrl.Path, req.Path) + return esUrl, nil +} diff --git a/pkg/tsdb/elasticsearch/elasticsearch_test.go b/pkg/tsdb/elasticsearch/elasticsearch_test.go index 0434a79ae10..13a6aa79c71 100644 --- a/pkg/tsdb/elasticsearch/elasticsearch_test.go +++ b/pkg/tsdb/elasticsearch/elasticsearch_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/httpclient" + es "github.com/grafana/grafana/pkg/tsdb/elasticsearch/client" ) type datasourceInfo struct { @@ -71,3 +72,32 @@ func TestNewInstanceSettings(t *testing.T) { }) }) } + +func TestCreateElasticsearchURL(t *testing.T) { + tt := []struct { + name string + settings es.DatasourceInfo + req backend.CallResourceRequest + expected string + }{ + {name: "with /_msearch path and valid url", settings: es.DatasourceInfo{URL: "http://localhost:9200"}, req: backend.CallResourceRequest{Path: "_msearch"}, expected: "http://localhost:9200/_msearch"}, + {name: "with _msearch path and valid url", settings: es.DatasourceInfo{URL: "http://localhost:9200"}, req: backend.CallResourceRequest{Path: "_msearch"}, expected: "http://localhost:9200/_msearch"}, + {name: "with _msearch path and valid url with /", settings: es.DatasourceInfo{URL: "http://localhost:9200/"}, req: backend.CallResourceRequest{Path: "_msearch"}, expected: "http://localhost:9200/_msearch"}, + {name: "with _mapping path and valid url", settings: es.DatasourceInfo{URL: "http://localhost:9200"}, req: backend.CallResourceRequest{Path: "/_mapping"}, expected: "http://localhost:9200/_mapping"}, + {name: "with /_mapping path and valid url", settings: es.DatasourceInfo{URL: "http://localhost:9200"}, req: backend.CallResourceRequest{Path: "/_mapping"}, expected: "http://localhost:9200/_mapping"}, + {name: "with /_mapping path and valid url with /", settings: es.DatasourceInfo{URL: "http://localhost:9200/"}, req: backend.CallResourceRequest{Path: "/_mapping"}, expected: "http://localhost:9200/_mapping"}, + {name: "with abc/_mapping path and valid url", settings: es.DatasourceInfo{URL: "http://localhost:9200"}, req: backend.CallResourceRequest{Path: "abc/_mapping"}, expected: "http://localhost:9200/abc/_mapping"}, + {name: "with /abc/_mapping path and valid url", settings: es.DatasourceInfo{URL: "http://localhost:9200"}, req: backend.CallResourceRequest{Path: "abc/_mapping"}, expected: "http://localhost:9200/abc/_mapping"}, + {name: "with /abc/_mapping path and valid url", settings: es.DatasourceInfo{URL: "http://localhost:9200/"}, req: backend.CallResourceRequest{Path: "abc/_mapping"}, expected: "http://localhost:9200/abc/_mapping"}, + // This is to support mappings to cross cluster search that includes ":" + {name: "with path including :", settings: es.DatasourceInfo{URL: "http://localhost:9200/"}, req: backend.CallResourceRequest{Path: "ab:c/_mapping"}, expected: "http://localhost:9200/ab:c/_mapping"}, + } + + for _, test := range tt { + t.Run(test.name, func(t *testing.T) { + url, err := createElasticsearchURL(&test.req, &test.settings) + require.NoError(t, err) + require.Equal(t, test.expected, url.String()) + }) + } +}