From 9785e573aa9ea71b03c7461e4cf3e4aac05c1557 Mon Sep 17 00:00:00 2001 From: Costa Alexoglou Date: Wed, 27 Aug 2025 12:20:57 +0200 Subject: [PATCH 1/3] Provisioning: Fix Dashboard Creation For First-Level Repository Folders (#109962) --- .../apis/provisioning/resources/parser.go | 5 ++ .../provisioning/resources/parser_test.go | 50 +++++++++++++++++++ pkg/tests/apis/provisioning/files_test.go | 20 ++++++++ pkg/tests/apis/provisioning/helper_test.go | 28 +++++++++++ 4 files changed, 103 insertions(+) diff --git a/pkg/registry/apis/provisioning/resources/parser.go b/pkg/registry/apis/provisioning/resources/parser.go index 35b94007390..447fb3eda11 100644 --- a/pkg/registry/apis/provisioning/resources/parser.go +++ b/pkg/registry/apis/provisioning/resources/parser.go @@ -65,6 +65,7 @@ func (f *parserFactory) GetParser(ctx context.Context, repo repository.Reader) ( }, urls: urls, clients: clients, + config: config, }, nil } @@ -75,6 +76,8 @@ type parser struct { // for repositories that have URL support urls repository.RepositoryWithURLs + config *provisioning.Repository + // ResourceClients give access to k8s apis clients ResourceClients } @@ -192,6 +195,8 @@ func (r *parser) Parse(ctx context.Context, info *repository.FileInfo) (parsed * dirPath := safepath.Dir(info.Path) if dirPath != "" { parsed.Meta.SetFolder(ParseFolder(dirPath, r.repo.Name).ID) + } else { + parsed.Meta.SetFolder(RootFolder(r.config)) } } obj.SetUID("") // clear identifiers diff --git a/pkg/registry/apis/provisioning/resources/parser_test.go b/pkg/registry/apis/provisioning/resources/parser_test.go index 61f374a8e05..c3aa679be84 100644 --- a/pkg/registry/apis/provisioning/resources/parser_test.go +++ b/pkg/registry/apis/provisioning/resources/parser_test.go @@ -11,6 +11,7 @@ import ( dashboardV1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestParser(t *testing.T) { @@ -27,6 +28,16 @@ func TestParser(t *testing.T) { Name: "repo", }, clients: clients, + config: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "repo", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.LocalRepositoryType, + Sync: provisioning.SyncOptions{Target: provisioning.SyncTargetTypeFolder}, + }, + }, } t.Run("invalid input", func(t *testing.T) { @@ -93,4 +104,43 @@ spec: require.Equal(t, "dashboard.grafana.app", dash.GVR.Group) require.Equal(t, "v0alpha1", dash.GVR.Version) }) + + t.Run("validate proper folder metadata is set", func(t *testing.T) { + testCases := []struct { + name string + filePath string + expectedFolder string + }{ + { + name: "file in subdirectory should use parsed folder ID", + filePath: "team-a/testing-valid-dashboard.json", + expectedFolder: ParseFolder("team-a/", "repo").ID, + }, + { + name: "file in first-level directory should use parent folder id", + filePath: "testing-valid-dashboard.json", + expectedFolder: parser.repo.Name, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dash, err := parser.Parse(context.Background(), &repository.FileInfo{ + Path: tc.filePath, + Data: []byte(`apiVersion: dashboard.grafana.app/v0alpha1 +kind: Dashboard +metadata: + name: test-dashboard +spec: + title: Test dashboard +`), + }) + require.NoError(t, err) + require.Equal(t, tc.expectedFolder, dash.Meta.GetFolder(), "folder should match expected") + annotations := dash.Obj.GetAnnotations() + require.NotNil(t, annotations, "annotations should not be nil") + require.Equal(t, tc.expectedFolder, annotations["grafana.app/folder"], "folder annotation should match expected") + }) + } + }) } diff --git a/pkg/tests/apis/provisioning/files_test.go b/pkg/tests/apis/provisioning/files_test.go index 7fa121e00f5..24a25849ecf 100644 --- a/pkg/tests/apis/provisioning/files_test.go +++ b/pkg/tests/apis/provisioning/files_test.go @@ -43,6 +43,8 @@ func TestIntegrationProvisioning_DeleteResources(t *testing.T) { require.NoError(t, err) require.Equal(t, 3, len(dashboards.Items)) + helper.validateManagedDashboardsFolderMetadata(t, ctx, repo, dashboards.Items) + folders, err := helper.Folders.Resource.List(ctx, metav1.ListOptions{}) require.NoError(t, err) require.Equal(t, 2, len(folders.Items)) @@ -126,6 +128,13 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) { ExpectedFolders: 0, }) + // Validate the dashboard metadata + dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, 1, len(dashboards.Items)) + + helper.validateManagedDashboardsFolderMetadata(t, ctx, repo, dashboards.Items) + // Verify the original dashboard exists in Grafana (using the UID from all-panels.json) const allPanelsUID = "n1jR8vnnz" // This is the UID from the all-panels.json file obj, err := helper.DashboardsV1.Resource.Get(ctx, allPanelsUID, metav1.GetOptions{}) @@ -420,6 +429,17 @@ func TestIntegrationProvisioning_FilesOwnershipProtection(t *testing.T) { ExpectedFolders: 2, // Total across both repos }) + allDashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + for _, dashboard := range allDashboards.Items { + annotations := dashboard.GetAnnotations() + // Expect to be managed by repo1 or repo2 + managerID := annotations["grafana.app/managerId"] + if managerID != repo1 && managerID != repo2 { + t.Fatalf("dashboard %s is not managed by repo1 or repo2", dashboard.GetName()) + } + } + t.Run("CREATE file with UID already owned by different repository - should fail", func(t *testing.T) { // Try to create a dashboard in repo2 that has the same UID as the one in repo1 // The all-panels.json has UID "n1jR8vnnz" which is already owned by repo1 diff --git a/pkg/tests/apis/provisioning/helper_test.go b/pkg/tests/apis/provisioning/helper_test.go index 1d03cbe1d02..3ba1c7e7604 100644 --- a/pkg/tests/apis/provisioning/helper_test.go +++ b/pkg/tests/apis/provisioning/helper_test.go @@ -460,6 +460,34 @@ func (h *provisioningTestHelper) logRepositoryObject(t *testing.T, obj map[strin } } +// validateManagedDashboardsFolderMetadata validates the folder metadata +// of the managed dashboards. +// If folder is nested, folder annotations should not be empty. +// Also checks that the managerId property exists. +func (h *provisioningTestHelper) validateManagedDashboardsFolderMetadata(t *testing.T, + ctx context.Context, repoName string, dashboards []unstructured.Unstructured) { + t.Helper() + + // Check if folder is nested or not. + // If not, folder annotations should be empty as we have an "instance" sync target + for _, d := range dashboards { + sourcePath, _, _ := unstructured.NestedString(d.Object, "metadata", "annotations", "grafana.app/sourcePath") + isNested := strings.Contains(sourcePath, "/") + + folder, found, _ := unstructured.NestedString(d.Object, "metadata", "annotations", "grafana.app/folder") + if isNested { + require.True(t, found, "dashboard should have a folder annotation") + require.NotEmpty(t, folder, "dashboard should be in a non-empty folder") + } else { + require.False(t, found, "dashboard should not have a folder annotation") + } + + managerID, _, _ := unstructured.NestedString(d.Object, "metadata", "annotations", "grafana.app/managerId") + // require.Equal(t, repoName, managerID, "dashboard should be managed by gitsync repo") + require.Equal(t, repoName, managerID, "dashboard should be managed by gitsync repo") + } +} + type TestRepo struct { Name string Target string From f31560534a71bb2e0f3304f96e6315e8d2a7b88d Mon Sep 17 00:00:00 2001 From: Stijn Van Hoey Date: Wed, 27 Aug 2025 12:47:02 +0200 Subject: [PATCH 2/3] Emails: Fix template brackets in passwordless_verify_ templates (#107108) --- .../passwordless_verify_existing_user.txt | 2 +- .../passwordless_verify_new_user.txt | 2 +- public/emails/invited_to_org.html | 16 ++--- public/emails/new_user_invite.html | 16 ++--- public/emails/ng_alert_notification.html | 64 ++++++++----------- .../passwordless_verify_existing_user.html | 54 ++++++++-------- .../passwordless_verify_existing_user.txt | 13 ++-- .../emails/passwordless_verify_new_user.html | 54 ++++++++-------- .../emails/passwordless_verify_new_user.txt | 13 ++-- public/emails/reset_password.html | 16 ++--- public/emails/signup_started.html | 16 ++--- public/emails/welcome_on_signup.html | 16 ++--- 12 files changed, 127 insertions(+), 155 deletions(-) diff --git a/emails/templates/passwordless_verify_existing_user.txt b/emails/templates/passwordless_verify_existing_user.txt index ccc8f1f3d1d..900f8618b0e 100644 --- a/emails/templates/passwordless_verify_existing_user.txt +++ b/emails/templates/passwordless_verify_existing_user.txt @@ -4,7 +4,7 @@ Hi, Copy and paste the email verification code: [[.ConfirmationCode]] -into the login form to verify your email address. This confirmation code will expire in {{ .Expire }} minutes. +into the login form to verify your email address. This confirmation code will expire in [[.Expire]] minutes. Alternatively, you can use the button below to verify your email address. [[.AppUrl]]login/?code=[[.Code]]&confirmationCode=[[.ConfirmationCode]] diff --git a/emails/templates/passwordless_verify_new_user.txt b/emails/templates/passwordless_verify_new_user.txt index 938d1dab934..4bf017ea63d 100644 --- a/emails/templates/passwordless_verify_new_user.txt +++ b/emails/templates/passwordless_verify_new_user.txt @@ -4,7 +4,7 @@ Hi, Copy and paste the email verification code: [[.ConfirmationCode]] -into the sign up form to verify your email address. This confirmation code will expire in {{ .Expire }} minutes. +into the sign up form to verify your email address. This confirmation code will expire in [[.Expire]] minutes. Alternatively, you can use the button below to verify your email address. [[.AppUrl]]login/?code=[[.Code]]&confirmationCode=[[.ConfirmationCode]] diff --git a/public/emails/invited_to_org.html b/public/emails/invited_to_org.html index 504a5c84ea8..6bc06b6e7fe 100644 --- a/public/emails/invited_to_org.html +++ b/public/emails/invited_to_org.html @@ -1,10 +1,8 @@ - + - - {{ Subject .Subject .TemplateData "{{ .InvitedBy }} has added you to the {{ .OrgName }} organization" }} - + {{ Subject .Subject .TemplateData "{{ .InvitedBy }} has added you to the {{ .OrgName }} organization" }} {{ __dangerouslyInjectHTML `` }} {{ __dangerouslyInjectHTML `` }} @@ -83,7 +81,7 @@ - -
+
{{ __dangerouslyInjectHTML `` }}
@@ -116,7 +112,7 @@ @@ -161,7 +157,7 @@ -
- +
+ diff --git a/public/emails/new_user_invite.html b/public/emails/new_user_invite.html index c43ade4bf98..2bf3897d289 100644 --- a/public/emails/new_user_invite.html +++ b/public/emails/new_user_invite.html @@ -1,10 +1,8 @@ - + - - {{ Subject .Subject .TemplateData "{{ .InvitedBy }} has invited you to join Grafana" }} - + {{ Subject .Subject .TemplateData "{{ .InvitedBy }} has invited you to join Grafana" }} {{ __dangerouslyInjectHTML `` }} {{ __dangerouslyInjectHTML `` }} @@ -83,7 +81,7 @@ - -
+
{{ __dangerouslyInjectHTML `` }}
@@ -116,7 +112,7 @@ @@ -155,7 +151,7 @@ -
- +
+ diff --git a/public/emails/ng_alert_notification.html b/public/emails/ng_alert_notification.html index 43fabdb6787..12fdcecc3ec 100644 --- a/public/emails/ng_alert_notification.html +++ b/public/emails/ng_alert_notification.html @@ -1,10 +1,8 @@ - + - - {{ Subject .Subject .TemplateData "{{ .Title }}" }} - + {{ Subject .Subject .TemplateData "{{ .Title }}" }} {{ __dangerouslyInjectHTML `` }} {{ __dangerouslyInjectHTML `` }} @@ -103,7 +101,7 @@ - {{ $numberOfFiringInstance := (len .Alerts.Firing) }} {{ $numberOfResolvedAlerts := (len .Alerts.Resolved) }} -
- +
{{ if $numberOfFiringInstance }} {{ $numberOfFiringInstance }} firing alert {{ $numberOfFiringInstance| plural "instance" "instances" }} @@ -142,7 +137,7 @@ {{ end }}
-
+
{{ __dangerouslyInjectHTML `` }}
@@ -159,7 +154,7 @@ @@ -238,8 +233,7 @@
- +
-
- +
{{ range $line := (splitList "\n" .Message) }} {{ $line }}
@@ -311,7 +305,7 @@ -
+ @@ -345,7 +339,7 @@
-
+ @@ -387,7 +381,7 @@ @@ -422,7 +416,7 @@ @@ -469,8 +463,7 @@ @@ -624,7 +616,7 @@
- +
- +
-
- {{ range $line := (splitList "\n" .Annotations.description) }} +
{{ range $line := (splitList "\n" .Annotations.description) }} {{ $line }}
{{ end }}
@@ -528,8 +521,7 @@
-
- {{ range $refID, $value := .Values }} +
{{ range $refID, $value := .Values }} {{ $refID }}={{ $value }}  {{ end }}
-
+ @@ -646,7 +638,7 @@
-
+ @@ -668,7 +660,7 @@
-
+ @@ -690,7 +682,7 @@
-
+ @@ -802,7 +794,7 @@
-
+ @@ -836,7 +828,7 @@
-
+ @@ -878,7 +870,7 @@ @@ -913,7 +905,7 @@ @@ -960,8 +952,7 @@ @@ -1115,7 +1105,7 @@
- +
- +
-
- {{ range $line := (splitList "\n" .Annotations.description) }} +
{{ range $line := (splitList "\n" .Annotations.description) }} {{ $line }}
{{ end }}
@@ -1019,8 +1010,7 @@
-
- {{ range $refID, $value := .Values }} +
{{ range $refID, $value := .Values }} {{ $refID }}={{ $value }}  {{ end }}
-
+ @@ -1137,7 +1127,7 @@
-
+ @@ -1159,7 +1149,7 @@
-
+ @@ -1181,7 +1171,7 @@
-
+ diff --git a/public/emails/passwordless_verify_existing_user.html b/public/emails/passwordless_verify_existing_user.html index fca20f91c71..d2eeaef0afe 100644 --- a/public/emails/passwordless_verify_existing_user.html +++ b/public/emails/passwordless_verify_existing_user.html @@ -3,9 +3,9 @@ {{ Subject .Subject .TemplateData "Verify your email" }} - + {{ __dangerouslyInjectHTML `` }} - + {{ __dangerouslyInjectHTML `` }} - - ` }} + {{ __dangerouslyInjectHTML ` - + ` }} + {{ __dangerouslyInjectHTML `` }} - + {{ __dangerouslyInjectHTML `` }} - - ` }} + {{ __dangerouslyInjectHTML ` - + ` }} + {{ __dangerouslyInjectHTML `` }} - + {{ __dangerouslyInjectHTML `` }} - -
+
{{ __dangerouslyInjectHTML `` }}
@@ -116,7 +112,7 @@ @@ -155,7 +151,7 @@ -
- +
+ diff --git a/public/emails/signup_started.html b/public/emails/signup_started.html index 47363a9f5c5..a0c40119805 100644 --- a/public/emails/signup_started.html +++ b/public/emails/signup_started.html @@ -1,10 +1,8 @@ - + - - {{ Subject .Subject .TemplateData "Welcome to Grafana, please complete your sign up!" }} - + {{ Subject .Subject .TemplateData "Welcome to Grafana, please complete your sign up!" }} {{ __dangerouslyInjectHTML `` }} {{ __dangerouslyInjectHTML `` }} @@ -83,7 +81,7 @@ - -
+
{{ __dangerouslyInjectHTML `` }}
@@ -116,7 +112,7 @@ @@ -204,7 +200,7 @@
- +
-
+ diff --git a/public/emails/welcome_on_signup.html b/public/emails/welcome_on_signup.html index 34620adaa3d..a3ca3aa3c74 100644 --- a/public/emails/welcome_on_signup.html +++ b/public/emails/welcome_on_signup.html @@ -1,10 +1,8 @@ - + - - {{ Subject .Subject .TemplateData "Welcome to Grafana" }} - + {{ Subject .Subject .TemplateData "Welcome to Grafana" }} {{ __dangerouslyInjectHTML `` }} {{ __dangerouslyInjectHTML `` }} @@ -83,7 +81,7 @@ - -
+
{{ __dangerouslyInjectHTML `` }}
@@ -116,7 +112,7 @@ @@ -160,7 +156,7 @@ -
- +
+ From 86c7f96fcb98fb0c2b4339657cab65ae684a7c31 Mon Sep 17 00:00:00 2001 From: Victor Marin <36818606+mdvictor@users.noreply.github.com> Date: Wed, 27 Aug 2025 13:59:07 +0300 Subject: [PATCH 3/3] TextBoxVariable: Fix change detection for `query` prop (#109895) * fix textbox variable model save on change * better test name * betterer * fix test * fix test --- .betterer.results | 4 +- .../saving/getDashboardChanges.test.ts | 58 +++++++++++++++++++ .../saving/getDashboardChanges.ts | 16 ++++- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/.betterer.results b/.betterer.results index ca0b5eeff6f..398edbbc345 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1765,7 +1765,9 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "3"], [0, 0, 0, "Do not use any type assertions.", "4"], [0, 0, 0, "Do not use any type assertions.", "5"], - [0, 0, 0, "Unexpected any. Specify a different type.", "6"] + [0, 0, 0, "Do not use any type assertions.", "6"], + [0, 0, 0, "Do not use any type assertions.", "7"], + [0, 0, 0, "Unexpected any. Specify a different type.", "8"] ], "public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"] diff --git a/public/app/features/dashboard-scene/saving/getDashboardChanges.test.ts b/public/app/features/dashboard-scene/saving/getDashboardChanges.test.ts index ae184d6aecf..a683f551952 100644 --- a/public/app/features/dashboard-scene/saving/getDashboardChanges.test.ts +++ b/public/app/features/dashboard-scene/saving/getDashboardChanges.test.ts @@ -326,6 +326,64 @@ describe('getDashboardChanges', () => { expect(result).toEqual(expectedChanges); }); + it('should not see any changes on modified textbox var when we do not update variable values', () => { + const newDashboard: Dashboard = { + ...initial, + templating: { + list: [ + { + name: 'var1', + type: 'textbox', + query: '', + current: { + value: 'value1', + text: 'text1', + }, + options: [], + }, + ], + }, + }; + + const changedDashboard: Dashboard = { + ...newDashboard, + templating: { + list: [ + { + name: 'var1', + type: 'textbox', + query: 'query', + current: { + value: 'value1', + text: 'text1', + }, + options: [], + }, + ], + }, + }; + + const expectedChanges = { + initialSaveModel: { + ...newDashboard, + }, + changedSaveModel: { + ...changedDashboard, + }, + diffs: {}, + diffCount: 0, + hasChanges: false, + hasTimeChanges: false, + isNew: false, + hasVariableValueChanges: false, + hasRefreshChange: false, + }; + + const result = getRawDashboardChanges(newDashboard, changedDashboard, false, false, false); + + expect(result).toEqual(expectedChanges); + }); + it('should return the correct result when the variable value changes', () => { const changed = { ...initial, diff --git a/public/app/features/dashboard-scene/saving/getDashboardChanges.ts b/public/app/features/dashboard-scene/saving/getDashboardChanges.ts index 33c046e84b9..1a1617029fd 100644 --- a/public/app/features/dashboard-scene/saving/getDashboardChanges.ts +++ b/public/app/features/dashboard-scene/saving/getDashboardChanges.ts @@ -1,11 +1,12 @@ // @ts-ignore -import type { AdHocVariableModel, TypedVariableModel } from '@grafana/data'; +import type { AdHocVariableModel, TextBoxVariableModel, TypedVariableModel } from '@grafana/data'; import { Dashboard, Panel, VariableOption } from '@grafana/schema'; import { AdHocFilterWithLabels, AdhocVariableSpec, Spec as DashboardV2Spec, + TextVariableSpec, VariableKind, } from '@grafana/schema/dist/esm/schema/dashboard/v2'; import { ResponseTransformers } from 'app/features/dashboard/api/ResponseTransformers'; @@ -217,7 +218,11 @@ export function applyVariableChangesV2( if (!saveVariables) { if (variable.kind === 'AdhocVariable') { variable.spec.filters = (original.spec as AdhocVariableSpec).filters; - } else { + } else if (variable.kind === 'TextVariable') { + variable.spec.query = (original.spec as TextVariableSpec).query; + } + + if (variable.kind !== 'AdhocVariable') { if (hasCurrentValueToSave(variable) && hasCurrentValueToSave(original)) { variable.spec.current = original.spec.current; } @@ -262,9 +267,14 @@ export function applyVariableChanges(saveModel: Dashboard, originalSaveModel: Da if (!saveVariables) { const typed = variable as TypedVariableModel; + if (typed.type === 'adhoc') { typed.filters = (original as AdHocVariableModel).filters; - } else { + } else if (typed.type === 'textbox') { + typed.query = (original as TextBoxVariableModel).query; + } + + if (typed.type !== 'adhoc') { variable.current = original.current; variable.options = original.options; }