From c9ab4e3a9e1af7c4353e3eebc431169a0f1af130 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 2 Apr 2024 20:09:18 +0200 Subject: [PATCH] DS apiserver: Fix resource path (#85494) --- pkg/registry/apis/datasource/sub_resource.go | 39 ++++++---- .../apis/datasource/sub_resource_test.go | 71 +++++++++++++++++++ 2 files changed, 95 insertions(+), 15 deletions(-) create mode 100644 pkg/registry/apis/datasource/sub_resource_test.go diff --git a/pkg/registry/apis/datasource/sub_resource.go b/pkg/registry/apis/datasource/sub_resource.go index a9b1e259a7b..93027767158 100644 --- a/pkg/registry/apis/datasource/sub_resource.go +++ b/pkg/registry/apis/datasource/sub_resource.go @@ -55,31 +55,23 @@ func (r *subResourceREST) Connect(ctx context.Context, name string, opts runtime ctx = contextualMiddlewares(ctx) return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + clonedReq, err := resourceRequest(req) + if err != nil { + responder.Error(err) + return + } + body, err := io.ReadAll(req.Body) if err != nil { responder.Error(err) return } - idx := strings.LastIndex(req.URL.Path, "/resource") - if idx < 0 { - responder.Error(fmt.Errorf("expected resource path")) // 400? - return - } - - clonedReq := req.Clone(req.Context()) - rawURL := req.URL.Path[idx+len("/resource"):] - - clonedReq.URL = &url.URL{ - Path: rawURL, - RawQuery: clonedReq.URL.RawQuery, - } - err = r.builder.client.CallResource(ctx, &backend.CallResourceRequest{ PluginContext: pluginCtx, Path: clonedReq.URL.Path, Method: req.Method, - URL: req.URL.String(), + URL: clonedReq.URL.String(), Body: body, Headers: req.Header, }, httpresponsesender.New(w)) @@ -89,3 +81,20 @@ func (r *subResourceREST) Connect(ctx context.Context, name string, opts runtime } }), nil } + +func resourceRequest(req *http.Request) (*http.Request, error) { + idx := strings.LastIndex(req.URL.Path, "/resource") + if idx < 0 { + return nil, fmt.Errorf("expected resource path") // 400? + } + + clonedReq := req.Clone(req.Context()) + rawURL := strings.TrimLeft(req.URL.Path[idx+len("/resource"):], "/") + + clonedReq.URL = &url.URL{ + Path: rawURL, + RawQuery: clonedReq.URL.RawQuery, + } + + return clonedReq, nil +} diff --git a/pkg/registry/apis/datasource/sub_resource_test.go b/pkg/registry/apis/datasource/sub_resource_test.go new file mode 100644 index 00000000000..3900a34a732 --- /dev/null +++ b/pkg/registry/apis/datasource/sub_resource_test.go @@ -0,0 +1,71 @@ +package datasource + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestResourceRequest(t *testing.T) { + testCases := []struct { + desc string + url string + error bool + expectedPath string + expectedURL string + }{ + { + desc: "no resource path", + url: "http://localhost:6443/apis/test.datasource.grafana.app/v0alpha1/namespaces/default/connections/abc", + error: true, + }, + { + desc: "root resource path", + url: "http://localhost:6443/apis/test.datasource.grafana.app/v0alpha1/namespaces/default/connections/abc/resource", + expectedPath: "", + expectedURL: "", + }, + { + desc: "root resource path", + url: "http://localhost:6443/apis/test.datasource.grafana.app/v0alpha1/namespaces/default/connections/abc/resource/", + expectedPath: "", + expectedURL: "", + }, + { + desc: "resource sub path", + url: "http://localhost:6443/apis/test.datasource.grafana.app/v0alpha1/namespaces/default/connections/abc/resource/test", + expectedPath: "test", + expectedURL: "test", + }, + { + desc: "resource sub path with colon", + url: "http://localhost:6443/apis/test.datasource.grafana.app/v0alpha1/namespaces/default/connections/abc/resource/test-*,*:test-*/_mapping", + expectedPath: "test-*,*:test-*/_mapping", + expectedURL: "./test-%2A,%2A:test-%2A/_mapping", + }, + { + desc: "resource sub path with query params", + url: "http://localhost:6443/apis/test.datasource.grafana.app/v0alpha1/namespaces/default/connections/abc/resource/test?k1=v1&k2=v2", + expectedPath: "test", + expectedURL: "test?k1=v1&k2=v2", + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, tc.url, nil) + clonedReq, err := resourceRequest(req) + + if tc.error { + require.Error(t, err) + require.Nil(t, clonedReq) + } else { + require.NoError(t, err) + require.NotNil(t, clonedReq) + require.Equal(t, tc.expectedPath, clonedReq.URL.Path) + require.Equal(t, tc.expectedURL, clonedReq.URL.String()) + } + }) + } +}