From 0248a393d7ac07d7d0a68bd0d91306d15c0e3a96 Mon Sep 17 00:00:00 2001 From: Costa Alexoglou Date: Wed, 17 Sep 2025 11:49:15 +0200 Subject: [PATCH] fix: dashboard upsert with empty ref (#111190) * fix: dashboard upsert with empty ref * chore: cleanup * fix: branches in other git providers and linter --- .../pkg/apis/provisioning/v0alpha1/types.go | 31 +++++++++++++++++ pkg/registry/apis/provisioning/files.go | 1 + .../apis/provisioning/resources/dualwriter.go | 34 ++++++++++++++----- pkg/registry/apis/provisioning/routes.go | 6 ++-- .../SaveProvisionedDashboardForm.tsx | 5 +-- 5 files changed, 63 insertions(+), 14 deletions(-) diff --git a/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go b/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go index 7ee2bcb4c95..9ddfcd3897f 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go @@ -128,6 +128,37 @@ func (r RepositoryType) IsGit() bool { return r == GitRepositoryType || r == GitHubRepositoryType || r == BitbucketRepositoryType || r == GitLabRepositoryType } +// Branch returns the branch for git-based repositories +// or an empty string for local repositories +func (r *Repository) Branch() string { + if !r.Spec.Type.IsGit() { + return "" + } + + switch r.Spec.Type { + case GitHubRepositoryType: + if r.Spec.GitHub != nil { + return r.Spec.GitHub.Branch + } + case GitRepositoryType: + if r.Spec.Git != nil { + return r.Spec.Git.Branch + } + case BitbucketRepositoryType: + if r.Spec.Bitbucket != nil { + return r.Spec.Bitbucket.Branch + } + case GitLabRepositoryType: + if r.Spec.GitLab != nil { + return r.Spec.GitLab.Branch + } + default: + return "" + } + + return "" +} + type RepositorySpec struct { // The repository display name (shown in the UI) Title string `json:"title"` diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index 0360113f76b..ad9bc4bc472 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -112,6 +112,7 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. Message: query.Get("message"), SkipDryRun: query.Get("skipDryRun") == "true", OriginalPath: query.Get("originalPath"), + Branch: repo.Config().Branch(), } logger := logger.With("url", r.URL.Path, "ref", opts.Ref, "message", opts.Message) ctx := logging.Context(r.Context(), logger) diff --git a/pkg/registry/apis/provisioning/resources/dualwriter.go b/pkg/registry/apis/provisioning/resources/dualwriter.go index e75f3870ee4..673370b3820 100644 --- a/pkg/registry/apis/provisioning/resources/dualwriter.go +++ b/pkg/registry/apis/provisioning/resources/dualwriter.go @@ -28,12 +28,16 @@ type DualReadWriter struct { } type DualWriteOptions struct { - Path string + Path string + // Ref is the target branch + // Local repositories do not use this, all other repository types do. + // Empty ref means to target the configured default branch Ref string Message string Data []byte SkipDryRun bool OriginalPath string // Used for move operations + Branch string // Configured default branch } func NewDualReadWriter(repo repository.ReaderWriter, parser Parser, folders *FolderManager, access authlib.AccessChecker) *DualReadWriter { @@ -89,7 +93,7 @@ func (r *DualReadWriter) Delete(ctx context.Context, opts DualWriteOptions) (*Pa } // HACK: manual set to the provided branch so that the parser can possible read the file - if opts.Ref != "" { + if r.shouldUpdateGrafanaDB(opts, nil) { file.Ref = opts.Ref } @@ -119,7 +123,7 @@ func (r *DualReadWriter) Delete(ctx context.Context, opts DualWriteOptions) (*Pa } // Delete the file in the grafana database using the parser's Run method - if opts.Ref == "" { + if r.shouldUpdateGrafanaDB(opts, nil) { err = parsed.Run(ctx) if err != nil { return nil, fmt.Errorf("delete resource from storage: %w", err) @@ -170,7 +174,7 @@ func (r *DualReadWriter) CreateFolder(ctx context.Context, opts DualWriteOptions } wrap.URLs = urls - if opts.Ref == "" { + if r.shouldUpdateGrafanaDB(opts, nil) { folderName, err := r.folders.EnsureFolderPathExist(ctx, opts.Path) if err != nil { return nil, err @@ -264,7 +268,7 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts D // Behaves the same running sync after writing // FIXME: to make sure if behaves in the same way as in sync, we should // we should refactor the code to use the same function. - if opts.Ref == "" && parsed.Client != nil { + if r.shouldUpdateGrafanaDB(opts, parsed) { if _, err := r.folders.EnsureFolderPathExist(ctx, opts.Path); err != nil { return nil, fmt.Errorf("ensure folder path exists: %w", err) } @@ -339,7 +343,7 @@ func (r *DualReadWriter) moveDirectory(ctx context.Context, opts DualWriteOption } // Handle folder management for main branch - if opts.Ref == "" { + if r.shouldUpdateGrafanaDB(opts, nil) { // Ensure destination folder path exists if _, err := r.folders.EnsureFolderPathExist(ctx, opts.Path); err != nil { return nil, fmt.Errorf("ensure destination folder path exists: %w", err) @@ -461,7 +465,7 @@ func (r *DualReadWriter) moveFile(ctx context.Context, opts DualWriteOptions) (* } // Update the grafana database if this is the main branch - if opts.Ref == "" && newParsed.Client != nil { + if r.shouldUpdateGrafanaDB(opts, newParsed) { if _, err := r.folders.EnsureFolderPathExist(ctx, opts.Path); err != nil { return nil, fmt.Errorf("ensure folder path exists: %w", err) } @@ -535,7 +539,7 @@ func (r *DualReadWriter) authorizeCreateFolder(ctx context.Context, _ string) er func (r *DualReadWriter) deleteFolder(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { // if the ref is set, it is not the active branch, so just delete the files from the branch // and do not delete the items from grafana itself - if opts.Ref != "" { + if r.shouldUpdateGrafanaDB(opts, nil) { err := r.repo.Delete(ctx, opts.Path, opts.Ref, opts.Message) if err != nil { return nil, fmt.Errorf("error deleting folder from repository: %w", err) @@ -677,3 +681,17 @@ func (r *DualReadWriter) deleteChildren(ctx context.Context, childrenResources [ return nil } + +// shouldUpdateGrafanaDB returns true if we have an empty ref (targeting the configured branch) +// or if the ref matches the configured branch +func (r *DualReadWriter) shouldUpdateGrafanaDB(opts DualWriteOptions, parsed *ParsedResource) bool { + if parsed != nil && parsed.Client == nil { + return false + } + + if opts.Ref != "" && opts.Ref != opts.Branch { + return false + } + + return true +} diff --git a/pkg/registry/apis/provisioning/routes.go b/pkg/registry/apis/provisioning/routes.go index 36d88433cc7..ab971dfb179 100644 --- a/pkg/registry/apis/provisioning/routes.go +++ b/pkg/registry/apis/provisioning/routes.go @@ -169,10 +169,8 @@ func (b *APIBuilder) handleSettings(w http.ResponseWriter, r *http.Request) { } for i, val := range all { - branch := "" - if val.Spec.GitHub != nil { - branch = val.Spec.GitHub.Branch - } + branch := val.Branch() + settings.Items[i] = provisioning.RepositoryView{ Name: val.Name, Title: val.Spec.Title, diff --git a/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.tsx b/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.tsx index 122c7fd99ae..a3a2b79f9c9 100644 --- a/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.tsx +++ b/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.tsx @@ -115,7 +115,7 @@ export function SaveProvisionedDashboardForm({ panelEditor?.onDiscard(); const model = dashboard.getSaveModel(); - const resourceData = request?.data?.resource.dryRun; + const resourceData = request?.data?.resource.upsert || request?.data?.resource.dryRun; const saveResponse = createSaveResponseFromResource(resourceData); dashboard.saveCompleted(model, saveResponse, defaultValues.folder?.uid); @@ -158,7 +158,8 @@ export function SaveProvisionedDashboardForm({ }); createOrUpdateFile({ - ref, + // Skip adding ref to the default branch request + ref: ref === repository?.branch ? undefined : ref, name: repo, path, message,