From 2123099e882c7cff6f914ce81ab089fa7e1e56ac Mon Sep 17 00:00:00 2001 From: Gonzalo Trigueros Manzanas <242162051+gttrigger@users.noreply.github.com> Date: Thu, 18 Dec 2025 11:28:10 +0100 Subject: [PATCH] Provisioning: escape URLs in PR comments to avoid malformed markdown. (#115486) provisioning: escape URLs in webhook changes to allow for proper markdown. --- .../webhooks/pullrequest/changes.go | 15 ++-- .../webhooks/pullrequest/changes_test.go | 74 ++++++++++++++++++- 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/pkg/registry/apis/provisioning/webhooks/pullrequest/changes.go b/pkg/registry/apis/provisioning/webhooks/pullrequest/changes.go index f8e485e8fe5..d6fec09bc96 100644 --- a/pkg/registry/apis/provisioning/webhooks/pullrequest/changes.go +++ b/pkg/registry/apis/provisioning/webhooks/pullrequest/changes.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/url" - "path" "strings" "time" @@ -141,14 +140,20 @@ func (e *evaluator) evaluateFile(ctx context.Context, repo repository.Reader, ba if info.Parsed.GVK.Kind == dashboardKind { // FIXME: extract the logic out of a dashboard URL builder/injector or similar // for testability and decoupling + urlBuilder, err := url.Parse(baseURL) + if err != nil { + info.Error = err.Error() + return info + } + if info.Parsed.Existing != nil { - info.GrafanaURL = fmt.Sprintf("%sd/%s/%s", baseURL, obj.GetName(), - slugify.Slugify(info.Title)) + grafanaURL := urlBuilder.JoinPath("d", obj.GetName(), slugify.Slugify(info.Title)) + info.GrafanaURL = grafanaURL.String() } // Load this file directly - info.PreviewURL = baseURL + path.Join("admin/provisioning", - info.Parsed.Repo.Name, "dashboard/preview", info.Parsed.Info.Path) + previewURL := urlBuilder.JoinPath("admin/provisioning", info.Parsed.Repo.Name, "dashboard/preview", info.Parsed.Info.Path) + info.PreviewURL = previewURL.String() query := url.Values{} query.Set("ref", info.Parsed.Info.Ref) diff --git a/pkg/registry/apis/provisioning/webhooks/pullrequest/changes_test.go b/pkg/registry/apis/provisioning/webhooks/pullrequest/changes_test.go index c8f0c33e92a..6c513830d29 100644 --- a/pkg/registry/apis/provisioning/webhooks/pullrequest/changes_test.go +++ b/pkg/registry/apis/provisioning/webhooks/pullrequest/changes_test.go @@ -737,8 +737,78 @@ func TestCalculateChanges(t *testing.T) { Path: "path/to/file.json", Ref: "ref", }, - GrafanaURL: "ht tp://bad url/d/the-uid/hello-world", // Malformed URL - PreviewURL: "ht tp://bad url/admin/provisioning/y/dashboard/preview/path/to/file.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", + Error: "parse \"ht tp://bad url/\": first path segment in URL cannot contain colon", + }}, + }, + }, + { + name: "path with spaces", + setupMocks: func(parser *resources.MockParser, reader *repository.MockReader, progress *jobs.MockJobProgressRecorder, renderer *MockScreenshotRenderer, parserFactory *resources.MockParserFactory) { + finfo := &repository.FileInfo{ + Path: "path/to/file with spaces.json", + Ref: "ref", + Data: []byte("xxxx"), + } + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": resources.DashboardResource.GroupVersion().String(), + "kind": dashboardKind, + "metadata": map[string]interface{}{ + "name": "the-uid", + }, + "spec": map[string]interface{}{ + "title": "hello world", + }, + }, + } + meta, _ := utils.MetaAccessor(obj) + + progress.On("SetMessage", mock.Anything, "process path/to/file with spaces.json").Return() + reader.On("Read", mock.Anything, "path/to/file with spaces.json", "ref").Return(finfo, nil) + reader.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "x", + }, + Spec: provisioning.RepositorySpec{ + GitHub: &provisioning.GitHubRepositoryConfig{ + GenerateDashboardPreviews: true, + }, + }, + }) + parser.On("Parse", mock.Anything, finfo).Return(&resources.ParsedResource{ + Info: finfo, + Repo: provisioning.ResourceRepositoryInfo{ + Namespace: "x", + Name: "y", + }, + GVK: schema.GroupVersionKind{ + Kind: dashboardKind, + }, + Obj: obj, + Existing: obj, + Meta: meta, + DryRunResponse: obj, + }, nil) + renderer.On("IsAvailable", mock.Anything, mock.Anything).Return(false) + parserFactory.On("GetParser", mock.Anything, mock.Anything).Return(parser, nil) + }, + changes: []repository.VersionedFileChange{{ + Action: repository.FileActionCreated, + Path: "path/to/file with spaces.json", + Ref: "ref", + }}, + expectedInfo: changeInfo{ + Changes: []fileChangeInfo{{ + Change: repository.VersionedFileChange{ + Action: repository.FileActionCreated, + Path: "path/to/file with spaces.json", + Ref: "ref", + }, + GrafanaURL: "http://host/d/the-uid/hello-world", + PreviewURL: "http://host/admin/provisioning/y/dashboard/preview/path/to/file%20with%20spaces.json?pull_request_url=http%253A%252F%252Fgithub.com%252Fpr%252F&ref=ref", + GrafanaScreenshotURL: "", + PreviewScreenshotURL: "", }}, }, },