From 410bb1cf7431a2a1ee8c858d729a56be85ed0df6 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Wed, 9 Apr 2025 10:33:55 +0300 Subject: [PATCH] Provisioning: Fix URL sanitization errors (#103640) --- pkg/registry/apis/provisioning/files.go | 3 +- pkg/registry/apis/provisioning/history.go | 2 +- pkg/registry/apis/provisioning/register.go | 3 +- .../apis/provisioning/provisioning_test.go | 53 ++++++++++++++++++- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index e28c36547c1..81b958a66df 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -59,7 +59,7 @@ func (*filesConnector) NewConnectOptions() (runtime.Object, bool, string) { func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { logger := logging.FromContext(ctx).With("logger", "files-connector", "repository_name", name) ctx = logging.Context(ctx, logger) - repo, err := s.getter.GetRepository(ctx, name) + repo, err := s.getter.GetHealthyRepository(ctx, name) if err != nil { logger.Debug("failed to find repository", "error", err) return nil, err @@ -110,6 +110,7 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime. files, err := s.listFolderFiles(ctx, filePath, ref, readWriter) if err != nil { responder.Error(err) + return } responder.Object(http.StatusOK, files) diff --git a/pkg/registry/apis/provisioning/history.go b/pkg/registry/apis/provisioning/history.go index 775dbdacc65..2dce3cdb7e8 100644 --- a/pkg/registry/apis/provisioning/history.go +++ b/pkg/registry/apis/provisioning/history.go @@ -54,7 +54,7 @@ func (h *historySubresource) NewConnectOptions() (runtime.Object, bool, string) func (h *historySubresource) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { logger := logging.FromContext(ctx).With("logger", "history-subresource") ctx = logging.Context(ctx, logger) - repo, err := h.repoGetter.GetRepository(ctx, name) + repo, err := h.repoGetter.GetHealthyRepository(ctx, name) if err != nil { logger.Debug("failed to find repository", "error", err) return nil, err diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index ebf6fcdb277..81615c0f18e 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -425,7 +425,8 @@ func (b *APIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admis // Trim trailing slash or .git if len(r.Spec.GitHub.URL) > 5 { - r.Spec.GitHub.URL = strings.TrimRight(strings.TrimRight(r.Spec.GitHub.URL, "/"), ".git") + r.Spec.GitHub.URL = strings.TrimSuffix(r.Spec.GitHub.URL, ".git") + r.Spec.GitHub.URL = strings.TrimSuffix(r.Spec.GitHub.URL, "/") } } diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index c642c366cd4..051049af119 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -173,14 +173,14 @@ func TestIntegrationProvisioning_CreatingGitHubRepository(t *testing.T) { ) const repo = "github-create-test" - _, err := helper.Repositories.Resource.Update(ctx, + _, err := helper.Repositories.Resource.Create(ctx, helper.RenderObject(t, "testdata/github-readonly.json.tmpl", map[string]any{ "Name": repo, "SyncEnabled": true, "SyncTarget": "instance", "Path": "grafana/", }), - metav1.UpdateOptions{}, + metav1.CreateOptions{}, ) require.NoError(t, err) @@ -197,6 +197,55 @@ func TestIntegrationProvisioning_CreatingGitHubRepository(t *testing.T) { } assert.Contains(t, names, "n1jR8vnnz", "should contain dashboard.json's contents") assert.Contains(t, names, "WZ7AhQiVz", "should contain dashboard2.yaml's contents") + + err = helper.Repositories.Resource.Delete(ctx, repo, metav1.DeleteOptions{}) + require.NoError(t, err, "should delete values") + + t.Run("github url cleanup", func(t *testing.T) { + tests := []struct { + name string + input string + output string + }{ + { + name: "simple-url", + input: "https://github.com/dprokop/grafana-git-sync-test", + output: "https://github.com/dprokop/grafana-git-sync-test", + }, + { + name: "trim-dot-git", + input: "https://github.com/dprokop/grafana-git-sync-test.git", + output: "https://github.com/dprokop/grafana-git-sync-test", + }, + { + name: "trim-slash", + input: "https://github.com/dprokop/grafana-git-sync-test/", + output: "https://github.com/dprokop/grafana-git-sync-test", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + input := helper.RenderObject(t, "testdata/github-readonly.json.tmpl", map[string]any{ + "Name": test.name, + "URL": test.input, + }) + + _, err := helper.Repositories.Resource.Create(ctx, input, metav1.CreateOptions{}) + require.NoError(t, err, "failed to create resource") + + obj, err := helper.Repositories.Resource.Get(ctx, test.name, metav1.GetOptions{}) + require.NoError(t, err, "failed to read back resource") + + url, _, err := unstructured.NestedString(obj.Object, "spec", "github", "url") + require.NoError(t, err, "failed to read URL") + require.Equal(t, test.output, url) + + err = helper.Repositories.Resource.Delete(ctx, test.name, metav1.DeleteOptions{}) + require.NoError(t, err, "failed to delete") + }) + } + }) } func TestIntegrationProvisioning_RunLocalRepository(t *testing.T) {