diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go index 85c2e732cb9..8f07f37658b 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go @@ -342,7 +342,10 @@ func (a *dashboardSqlAccess) scanRow(rows *sql.Rows, history bool) (*dashboardRo return row, fmt.Errorf("JSON unmarshal error for: %s // %w", dash.Name, err) } } - dash.Spec.Remove("id") + // Ignore any saved values for id/version/uid + delete(dash.Spec.Object, "id") + delete(dash.Spec.Object, "version") + delete(dash.Spec.Object, "uid") } return row, err } diff --git a/pkg/registry/apis/dashboard/mutate.go b/pkg/registry/apis/dashboard/mutate.go index 57b25078848..a8df79270e9 100644 --- a/pkg/registry/apis/dashboard/mutate.go +++ b/pkg/registry/apis/dashboard/mutate.go @@ -28,11 +28,15 @@ func (b *DashboardsAPIBuilder) Mutate(ctx context.Context, a admission.Attribute switch v := obj.(type) { case *dashboardV0.Dashboard: + delete(v.Spec.Object, "uid") + delete(v.Spec.Object, "version") if id, ok := v.Spec.Object["id"].(float64); ok { delete(v.Spec.Object, "id") internalID = int64(id) } case *dashboardV1.Dashboard: + delete(v.Spec.Object, "uid") + delete(v.Spec.Object, "version") if id, ok := v.Spec.Object["id"].(float64); ok { delete(v.Spec.Object, "id") internalID = int64(id) diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 924a435cd2d..cd55eb029e2 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -1946,10 +1946,8 @@ func (dr *DashboardServiceImpl) UnstructuredToLegacyDashboard(ctx context.Contex uid := obj.GetName() spec["uid"] = uid - dashVersion := 0 - if version, ok := spec["version"].(int64); ok { - dashVersion = int(version) - } + dashVersion := obj.GetGeneration() + spec["version"] = dashVersion out := dashboards.Dashboard{ OrgID: orgID, @@ -1957,7 +1955,7 @@ func (dr *DashboardServiceImpl) UnstructuredToLegacyDashboard(ctx context.Contex UID: uid, Slug: obj.GetSlug(), FolderUID: obj.GetFolder(), - Version: dashVersion, + Version: int(dashVersion), Data: simplejson.NewFromAny(spec), APIVersion: strings.TrimPrefix(item.GetAPIVersion(), dashboardv0alpha1.GROUP+"/"), } diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 005d8409886..1f5f5d5f9ba 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -299,12 +299,12 @@ func TestGetDashboard(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) dashboardUnstructured := unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ - "name": "uid", + "name": "uid", + "generation": int64(1), }, "spec": map[string]any{ - "test": "test", - "version": int64(1), - "title": "testing slugify", + "test": "test", + "title": "testing slugify", }, }} @@ -337,12 +337,12 @@ func TestGetDashboard(t *testing.T) { k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") dashboardUnstructured := unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ - "name": "uid", + "name": "uid", + "generation": int64(2), }, "spec": map[string]any{ - "test": "test", - "version": int64(1), - "title": "testing slugify", + "test": "test", + "title": "testing slugify", }, }} @@ -351,8 +351,8 @@ func TestGetDashboard(t *testing.T) { Title: "testing slugify", Slug: "testing-slugify", // slug is taken from title OrgID: 1, // orgID is populated from the query - Version: 1, - Data: simplejson.NewFromAny(map[string]any{"test": "test", "title": "testing slugify", "uid": "uid", "version": int64(1)}), + Version: 2, + Data: simplejson.NewFromAny(map[string]any{"test": "test", "title": "testing slugify", "uid": "uid", "version": int64(2)}), } k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil).Once() k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) @@ -436,12 +436,12 @@ func TestGetAllDashboards(t *testing.T) { dashboardUnstructured := unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ - "name": "uid", + "name": "uid", + "generation": int64(1), }, "spec": map[string]any{ - "test": "test", - "version": int64(1), - "title": "testing slugify", + "test": "test", + "title": "testing slugify", }, }} @@ -488,12 +488,12 @@ func TestGetAllDashboardsByOrgId(t *testing.T) { dashboardUnstructured := unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ - "name": "uid", + "name": "uid", + "generation": int64(1), }, "spec": map[string]any{ - "test": "test", - "version": int64(1), - "title": "testing slugify", + "test": "test", + "title": "testing slugify", }, }} @@ -1573,23 +1573,26 @@ func TestGetDashboards(t *testing.T) { expectedResult := []*dashboards.Dashboard{ { - UID: "uid1", - Slug: "dashboard-1", - OrgID: 1, - Title: "Dashboard 1", - Data: simplejson.NewFromAny(map[string]any{"title": "Dashboard 1", "uid": "uid1"}), + UID: "uid1", + Slug: "dashboard-1", + OrgID: 1, + Title: "Dashboard 1", + Version: 1, + Data: simplejson.NewFromAny(map[string]any{"title": "Dashboard 1", "uid": "uid1", "version": int64(1)}), }, { - UID: "uid2", - Slug: "dashboard-2", - OrgID: 1, - Title: "Dashboard 2", - Data: simplejson.NewFromAny(map[string]any{"title": "Dashboard 2", "uid": "uid2"}), + UID: "uid2", + Slug: "dashboard-2", + OrgID: 1, + Title: "Dashboard 2", + Version: 1, + Data: simplejson.NewFromAny(map[string]any{"title": "Dashboard 2", "uid": "uid2", "version": int64(1)}), }, } uid1Unstructured := &unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ - "name": "uid1", + "name": "uid1", + "generation": int64(1), }, "spec": map[string]any{ "title": "Dashboard 1", @@ -1597,7 +1600,8 @@ func TestGetDashboards(t *testing.T) { }} uid2Unstructured := &unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ - "name": "uid2", + "name": "uid2", + "generation": int64(1), }, "spec": map[string]any{ "title": "Dashboard 2", diff --git a/pkg/services/dashboardversion/dashverimpl/dashver.go b/pkg/services/dashboardversion/dashverimpl/dashver.go index 12120bc9188..31103e1821a 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver.go @@ -286,11 +286,13 @@ func (s *Service) UnstructuredToLegacyDashboardVersion(ctx context.Context, item uid := obj.GetName() spec["uid"] = uid - dashVersion := 0 - parentVersion := 0 - if version, ok := spec["version"].(int64); ok { - dashVersion = int(version) - parentVersion = dashVersion - 1 + dashVersion := obj.GetGeneration() + parentVersion := dashVersion - 1 + if parentVersion < 0 { + parentVersion = 0 + } + if dashVersion > 0 { + spec["version"] = dashVersion } createdBy, err := s.k8sclient.GetUserFromMeta(ctx, obj.GetCreatedBy()) @@ -325,8 +327,8 @@ func (s *Service) UnstructuredToLegacyDashboardVersion(ctx context.Context, item CreatedBy: createdBy.ID, Message: obj.GetMessage(), RestoredFrom: restoreVer, - Version: dashVersion, - ParentVersion: parentVersion, + Version: int(dashVersion), + ParentVersion: int(parentVersion), Data: simplejson.NewFromAny(spec), } diff --git a/pkg/services/dashboardversion/dashverimpl/dashver_test.go b/pkg/services/dashboardversion/dashverimpl/dashver_test.go index 03b2c2302c8..bd022a00640 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver_test.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver_test.go @@ -55,6 +55,7 @@ func TestDashboardVersionService(t *testing.T) { "metadata": map[string]any{ "name": "uid", "resourceVersion": "12", + "generation": int64(10), "labels": map[string]any{ utils.LabelKeyDeprecatedInternalID: "42", // nolint:staticcheck }, @@ -63,7 +64,7 @@ func TestDashboardVersionService(t *testing.T) { }, }, "spec": map[string]any{ - "version": int64(10), + "hello": "world", }, }}, nil).Once() res, err := dashboardVersionService.Get(context.Background(), &dashver.GetDashboardVersionQuery{ @@ -79,7 +80,7 @@ func TestDashboardVersionService(t *testing.T) { DashboardID: 42, DashboardUID: "uid", CreatedBy: 1, - Data: simplejson.NewFromAny(map[string]any{"uid": "uid", "version": int64(10)}), + Data: simplejson.NewFromAny(map[string]any{"uid": "uid", "version": int64(10), "hello": "world"}), }) mockCli.On("GetUserFromMeta", mock.Anything, "user:2").Return(&user.User{ID: 2}, nil) @@ -88,6 +89,7 @@ func TestDashboardVersionService(t *testing.T) { "metadata": map[string]any{ "name": "uid", "resourceVersion": "11", + "generation": int64(11), "labels": map[string]any{ utils.LabelKeyDeprecatedInternalID: "42", // nolint:staticcheck }, @@ -96,9 +98,7 @@ func TestDashboardVersionService(t *testing.T) { utils.AnnoKeyUpdatedBy: "user:2", // if updated by is set, that is the version creator }, }, - "spec": map[string]any{ - "version": int64(11), - }, + "spec": map[string]any{}, }}, nil).Once() res, err = dashboardVersionService.Get(context.Background(), &dashver.GetDashboardVersionQuery{ DashboardID: 42, @@ -264,13 +264,12 @@ func TestListDashboardVersions(t *testing.T) { "metadata": map[string]any{ "name": "uid", "resourceVersion": "12", + "generation": int64(5), "labels": map[string]any{ utils.LabelKeyDeprecatedInternalID: "42", // nolint:staticcheck }, }, - "spec": map[string]any{ - "version": int64(5), - }, + "spec": map[string]any{}, }}}}, nil).Once() res, err := dashboardVersionService.List(context.Background(), &query) require.Nil(t, err) diff --git a/pkg/tests/apis/dashboard/dashboards_test.go b/pkg/tests/apis/dashboard/dashboards_test.go index d0e48414fd6..f6ffedfc601 100644 --- a/pkg/tests/apis/dashboard/dashboards_test.go +++ b/pkg/tests/apis/dashboard/dashboards_test.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tests/apis" "github.com/grafana/grafana/pkg/tests/testinfra" @@ -313,6 +314,22 @@ func TestIntegrationLegacySupport(t *testing.T) { obj, err = client.Get(ctx, name, metav1.GetOptions{}, "dto") require.NoError(t, err) require.Equal(t, name, obj.GetName()) + + if obj.Object["spec"] == nil { + continue // missing conversions + } + + // This should have been moved to metadata + spec, _, err := unstructured.NestedMap(obj.Object, "spec") + require.NoError(t, err) + + require.Nil(t, spec["id"]) + require.Nil(t, spec["uid"]) + require.Nil(t, spec["version"]) + + access, _, err := unstructured.NestedMap(obj.Object, "access") + require.NoError(t, err) + require.Equal(t, slugify.Slugify(spec["title"].(string)), access["slug"]) } } diff --git a/pkg/tests/apis/dashboard/testdata/dashboard-test-v0.yaml b/pkg/tests/apis/dashboard/testdata/dashboard-test-v0.yaml index e1f66d12a34..2d4cf82b060 100644 --- a/pkg/tests/apis/dashboard/testdata/dashboard-test-v0.yaml +++ b/pkg/tests/apis/dashboard/testdata/dashboard-test-v0.yaml @@ -4,3 +4,5 @@ metadata: name: test-v0 spec: title: Test dashboard. Created at v0 + uid: test-v0 # will be removed by mutation hook + version: 1234567 # will be removed by mutation hook diff --git a/pkg/tests/apis/dashboard/testdata/dashboard-test-v1.yaml b/pkg/tests/apis/dashboard/testdata/dashboard-test-v1.yaml index 4a093f8a5cb..1e17651de60 100644 --- a/pkg/tests/apis/dashboard/testdata/dashboard-test-v1.yaml +++ b/pkg/tests/apis/dashboard/testdata/dashboard-test-v1.yaml @@ -3,4 +3,6 @@ kind: Dashboard metadata: name: test-v1 spec: - title: Test dashboard. Created at v1 XXX + title: Test dashboard. Created at v1 + uid: test-v1 # will be removed by mutation hook + version: 1234567 # will be removed by mutation hook