From 35e3d269876c2df1cfed29b2d365855c7d4a27c7 Mon Sep 17 00:00:00 2001 From: Ivan Ortega Alba Date: Wed, 19 Mar 2025 08:57:05 +0100 Subject: [PATCH] Dashboards: Do not throw error if backend cannot migrate schemaVersion to latest (#102357) * Refactor migration error handling to use MinimumVersionError for schema version checks - Updated migration logic to return MinimumVersionError instead of MigrationError for outdated schema versions. - Enhanced MinimumVersionError message for clarity on migration constraints. - Added tests for version error handling in the dashboard API to ensure proper error throwing for specific conversion errors. * Fix tests and remove folder dependencies --- apps/dashboard/pkg/migration/migrate.go | 2 +- apps/dashboard/pkg/migration/migrate_test.go | 2 +- .../pkg/migration/schemaversion/errors.go | 2 +- public/app/features/dashboard/api/v1.test.ts | 68 ++++- public/app/features/dashboard/api/v1.ts | 2 +- public/app/features/dashboard/api/v2.test.ts | 250 +++++++++++------- public/app/features/dashboard/api/v2.ts | 6 +- 7 files changed, 232 insertions(+), 100 deletions(-) diff --git a/apps/dashboard/pkg/migration/migrate.go b/apps/dashboard/pkg/migration/migrate.go index 0f8992e6d2d..51d61772704 100644 --- a/apps/dashboard/pkg/migration/migrate.go +++ b/apps/dashboard/pkg/migration/migrate.go @@ -12,7 +12,7 @@ func Migrate(dash map[string]interface{}, targetVersion int) error { // If the schema version is older than the minimum version, with migration support, // we don't migrate the dashboard. if inputVersion < schemaversion.MIN_VERSION { - return schemaversion.NewMigrationError("schema version is too old", inputVersion, schemaversion.MIN_VERSION) + return schemaversion.NewMinimumVersionError(inputVersion) } for nextVersion := inputVersion + 1; nextVersion <= targetVersion; nextVersion++ { diff --git a/apps/dashboard/pkg/migration/migrate_test.go b/apps/dashboard/pkg/migration/migrate_test.go index ef2a3468ce0..8130ca80e17 100644 --- a/apps/dashboard/pkg/migration/migrate_test.go +++ b/apps/dashboard/pkg/migration/migrate_test.go @@ -27,7 +27,7 @@ func TestMigrate(t *testing.T) { "schemaVersion": schemaversion.MIN_VERSION - 1, }, schemaversion.MIN_VERSION) - var minVersionErr = schemaversion.NewMigrationError("schema version is too old", schemaversion.MIN_VERSION-1, schemaversion.MIN_VERSION) + var minVersionErr = schemaversion.NewMinimumVersionError(schemaversion.MIN_VERSION - 1) require.ErrorAs(t, err, &minVersionErr) }) diff --git a/apps/dashboard/pkg/migration/schemaversion/errors.go b/apps/dashboard/pkg/migration/schemaversion/errors.go index ee98593219d..ec01e229b8e 100644 --- a/apps/dashboard/pkg/migration/schemaversion/errors.go +++ b/apps/dashboard/pkg/migration/schemaversion/errors.go @@ -35,5 +35,5 @@ type MinimumVersionError struct { } func (e *MinimumVersionError) Error() string { - return fmt.Errorf("input schema version is below minimum version. input: %d minimum: %d", e.inputVersion, MIN_VERSION).Error() + return fmt.Errorf("dashboard schema version %d cannot be migrated to latest version %d - migration path only exists for versions greater than %d", e.inputVersion, LATEST_VERSION, MIN_VERSION).Error() } diff --git a/public/app/features/dashboard/api/v1.test.ts b/public/app/features/dashboard/api/v1.test.ts index 0f2eadedc5f..4a51041f140 100644 --- a/public/app/features/dashboard/api/v1.test.ts +++ b/public/app/features/dashboard/api/v1.test.ts @@ -14,9 +14,7 @@ const mockDashboardDto: DashboardWithAccessInfo = { name: 'dash-uid', resourceVersion: '1', creationTimestamp: '1', - annotations: { - [AnnoKeyFolder]: 'new-folder', - }, + annotations: {}, }, spec: { title: 'test', @@ -87,10 +85,13 @@ const saveDashboardResponse = { weekStart: '', }, }; + +const mockGet = jest.fn().mockResolvedValue(mockDashboardDto); + jest.mock('@grafana/runtime', () => ({ ...jest.requireActual('@grafana/runtime'), getBackendSrv: () => ({ - get: () => mockDashboardDto, + get: mockGet, put: jest.fn().mockResolvedValue(saveDashboardResponse), post: jest.fn().mockResolvedValue(saveDashboardResponse), }), @@ -108,7 +109,15 @@ jest.mock('app/features/live/dashboard/dashboardWatcher', () => ({ describe('v1 dashboard API', () => { it('should provide folder annotations', async () => { - jest.spyOn(backendSrv, 'getFolderByUid').mockResolvedValue({ + mockGet.mockResolvedValueOnce({ + ...mockDashboardDto, + metadata: { + ...mockDashboardDto.metadata, + annotations: { [AnnoKeyFolder]: 'new-folder' }, + }, + }); + + jest.spyOn(backendSrv, 'getFolderByUid').mockResolvedValueOnce({ id: 1, uid: 'new-folder', title: 'New Folder', @@ -134,7 +143,16 @@ describe('v1 dashboard API', () => { }); it('throws an error if folder is not found', async () => { - jest.spyOn(backendSrv, 'getFolderByUid').mockRejectedValue({ message: 'folder not found', status: 'not-found' }); + mockGet.mockResolvedValueOnce({ + ...mockDashboardDto, + metadata: { + ...mockDashboardDto.metadata, + annotations: { [AnnoKeyFolder]: 'new-folder' }, + }, + }); + jest + .spyOn(backendSrv, 'getFolderByUid') + .mockRejectedValueOnce({ message: 'folder not found', status: 'not-found' }); const api = new K8sDashboardAPI(); await expect(api.getDashboardDTO('test')).rejects.toThrow('Failed to load folder'); @@ -241,4 +259,42 @@ describe('v1 dashboard API', () => { }); }); }); + + describe('version error handling', () => { + it('should throw DashboardVersionError for v2alpha1 conversion error', async () => { + const mockDashboardWithError = { + ...mockDashboardDto, + status: { + conversion: { + failed: true, + error: 'backend conversion not yet implemented', + storedVersion: 'v2alpha1', + }, + }, + }; + + mockGet.mockResolvedValueOnce(mockDashboardWithError); + + const api = new K8sDashboardAPI(); + await expect(api.getDashboardDTO('test')).rejects.toThrow('backend conversion not yet implemented'); + }); + + it.each(['v0alpha1', 'v1alpha1'])('should not throw for %s conversion errors', async (correctStoredVersion) => { + const mockDashboardWithError = { + ...mockDashboardDto, + status: { + conversion: { + failed: true, + error: 'other-error', + storedVersion: correctStoredVersion, + }, + }, + }; + + jest.spyOn(backendSrv, 'get').mockResolvedValueOnce(mockDashboardWithError); + + const api = new K8sDashboardAPI(); + await expect(api.getDashboardDTO('test')).resolves.toBeDefined(); + }); + }); }); diff --git a/public/app/features/dashboard/api/v1.ts b/public/app/features/dashboard/api/v1.ts index 33cedae8709..89dd8bc8046 100644 --- a/public/app/features/dashboard/api/v1.ts +++ b/public/app/features/dashboard/api/v1.ts @@ -97,7 +97,7 @@ export class K8sDashboardAPI implements DashboardAPI { const dash = await this.client.subresource>(uid, 'dto'); // This could come as conversion error from v0 or v2 to V1. - if (dash.status?.conversion?.failed) { + if (dash.status?.conversion?.failed && dash.status.conversion.storedVersion === 'v2alpha1') { throw new DashboardVersionError(dash.status.conversion.storedVersion, dash.status.conversion.error); } diff --git a/public/app/features/dashboard/api/v2.test.ts b/public/app/features/dashboard/api/v2.test.ts index cfae61b6caf..a6eed33bcd2 100644 --- a/public/app/features/dashboard/api/v2.test.ts +++ b/public/app/features/dashboard/api/v2.test.ts @@ -19,9 +19,7 @@ const mockDashboardDto: DashboardWithAccessInfo = { name: 'dash-uid', resourceVersion: '1', creationTimestamp: '1', - annotations: { - [AnnoKeyFolder]: 'new-folder', - }, + annotations: {}, }, spec: { ...defaultDashboardV2Spec(), @@ -29,7 +27,9 @@ const mockDashboardDto: DashboardWithAccessInfo = { access: {}, }; -// Create a mock put function that we can spy on +// Create mock get and put functions that we can spy on +const mockGet = jest.fn().mockResolvedValue(mockDashboardDto); + const mockPut = jest.fn().mockImplementation((url, data) => { return { apiVersion: 'dashboard.grafana.app/v2alpha1', @@ -48,7 +48,7 @@ const mockPut = jest.fn().mockImplementation((url, data) => { jest.mock('@grafana/runtime', () => ({ ...jest.requireActual('@grafana/runtime'), getBackendSrv: () => ({ - get: () => mockDashboardDto, + get: mockGet, put: mockPut, }), config: { @@ -66,6 +66,14 @@ describe('v2 dashboard API', () => { }); it('should provide folder annotations', async () => { + mockGet.mockResolvedValueOnce({ + ...mockDashboardDto, + metadata: { + ...mockDashboardDto.metadata, + annotations: { [AnnoKeyFolder]: 'new-folder' }, + }, + }); + jest.spyOn(backendSrv, 'getFolderByUid').mockResolvedValue({ id: 1, uid: 'new-folder', @@ -94,104 +102,168 @@ describe('v2 dashboard API', () => { }); it('throws an error if folder is not found', async () => { - jest.spyOn(backendSrv, 'getFolderByUid').mockRejectedValue({ message: 'folder not found', status: 'not-found' }); + mockGet.mockResolvedValueOnce({ + ...mockDashboardDto, + metadata: { + ...mockDashboardDto.metadata, + annotations: { [AnnoKeyFolder]: 'new-folder' }, + }, + }); + jest + .spyOn(backendSrv, 'getFolderByUid') + .mockRejectedValueOnce({ message: 'folder not found', status: 'not-found' }); const api = new K8sDashboardV2API(); await expect(api.getDashboardDTO('test')).rejects.toThrow('Failed to load folder'); }); -}); - -describe('v2 dashboard API - Save', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - const defaultSaveCommand = { - dashboard: defaultDashboardV2Spec(), - message: 'test save', - folderUid: 'test-folder', - k8s: { - name: 'test-dash', - labels: { - [DeprecatedInternalId]: '123', - }, - - annotations: { - [AnnoKeyFolder]: 'new-folder', - }, - }, - }; - - it('should create new dashboard', async () => { - const api = new K8sDashboardV2API(); - const result = await api.saveDashboard({ - ...defaultSaveCommand, - dashboard: { - ...defaultSaveCommand.dashboard, - title: 'test-dashboard', - }, + describe('v2 dashboard API - Save', () => { + beforeEach(() => { + jest.clearAllMocks(); }); - expect(result).toEqual({ - id: 123, - uid: 'test-dash', - url: '/d/test-dash/testdashboard', - slug: '', - status: 'success', - version: 2, - }); - }); - - it('should update existing dashboard', async () => { - const api = new K8sDashboardV2API(); - - const result = await api.saveDashboard({ - ...defaultSaveCommand, - dashboard: { - ...defaultSaveCommand.dashboard, - title: 'chaing-title-dashboard', - }, + const defaultSaveCommand = { + dashboard: defaultDashboardV2Spec(), + message: 'test save', + folderUid: 'test-folder', k8s: { - ...defaultSaveCommand.k8s, - name: 'existing-dash', - }, - }); - expect(result.version).toBe(2); - }); + name: 'test-dash', + labels: { + [DeprecatedInternalId]: '123', + }, - it('should update existing dashboard that is store in a folder', async () => { - const api = new K8sDashboardV2API(); - await api.saveDashboard({ - dashboard: { - ...defaultSaveCommand.dashboard, - title: 'chaing-title-dashboard', - }, - folderUid: 'folderUidXyz', - k8s: { - name: 'existing-dash', annotations: { - [AnnoKeyFolder]: 'folderUidXyz', - [AnnoKeyFolderUrl]: 'url folder used in the client', - [AnnoKeyFolderId]: 42, - [AnnoKeyFolderTitle]: 'title folder used in the client', + [AnnoKeyFolder]: 'new-folder', }, }, - }); - expect(mockPut).toHaveBeenCalledTimes(1); - expect(mockPut).toHaveBeenCalledWith( - '/apis/dashboard.grafana.app/v2alpha1/namespaces/default/dashboards/existing-dash', - { - metadata: { - name: 'existing-dash', - annotations: { - [AnnoKeyFolder]: 'folderUidXyz', - }, + }; + + it('should create new dashboard', async () => { + const api = new K8sDashboardV2API(); + const result = await api.saveDashboard({ + ...defaultSaveCommand, + dashboard: { + ...defaultSaveCommand.dashboard, + title: 'test-dashboard', }, - spec: { + }); + + expect(result).toEqual({ + id: 123, + uid: 'test-dash', + url: '/d/test-dash/testdashboard', + slug: '', + status: 'success', + version: 2, + }); + }); + + it('should update existing dashboard', async () => { + const api = new K8sDashboardV2API(); + + const result = await api.saveDashboard({ + ...defaultSaveCommand, + dashboard: { ...defaultSaveCommand.dashboard, title: 'chaing-title-dashboard', }, - } - ); + k8s: { + ...defaultSaveCommand.k8s, + name: 'existing-dash', + }, + }); + expect(result.version).toBe(2); + }); + + it('should update existing dashboard that is store in a folder', async () => { + const api = new K8sDashboardV2API(); + await api.saveDashboard({ + dashboard: { + ...defaultSaveCommand.dashboard, + title: 'chaing-title-dashboard', + }, + folderUid: 'folderUidXyz', + k8s: { + name: 'existing-dash', + annotations: { + [AnnoKeyFolder]: 'folderUidXyz', + [AnnoKeyFolderUrl]: 'url folder used in the client', + [AnnoKeyFolderId]: 42, + [AnnoKeyFolderTitle]: 'title folder used in the client', + }, + }, + }); + expect(mockPut).toHaveBeenCalledTimes(1); + expect(mockPut).toHaveBeenCalledWith( + '/apis/dashboard.grafana.app/v2alpha1/namespaces/default/dashboards/existing-dash', + { + metadata: { + name: 'existing-dash', + annotations: { + [AnnoKeyFolder]: 'folderUidXyz', + }, + }, + spec: { + ...defaultSaveCommand.dashboard, + title: 'chaing-title-dashboard', + }, + } + ); + }); + }); + + describe('version error handling', () => { + it('should throw DashboardVersionError for v0alpha1 conversion error', async () => { + const mockDashboardWithError = { + ...mockDashboardDto, + status: { + conversion: { + failed: true, + error: 'backend conversion not yet implemented', + storedVersion: 'v0alpha1', + }, + }, + }; + + mockGet.mockResolvedValueOnce(mockDashboardWithError); + + const api = new K8sDashboardV2API(); + await expect(api.getDashboardDTO('test')).rejects.toThrow('backend conversion not yet implemented'); + }); + + it('should throw DashboardVersionError for v1alpha1 conversion error', async () => { + const mockDashboardWithError = { + ...mockDashboardDto, + status: { + conversion: { + failed: true, + error: 'backend conversion not yet implemented', + storedVersion: 'v1alpha1', + }, + }, + }; + + mockGet.mockResolvedValueOnce(mockDashboardWithError); + + const api = new K8sDashboardV2API(); + await expect(api.getDashboardDTO('test')).rejects.toThrow('backend conversion not yet implemented'); + }); + + it('should not throw for other conversion errors', async () => { + const mockDashboardWithError = { + ...mockDashboardDto, + status: { + conversion: { + failed: true, + error: 'other-error', + storedVersion: 'v2alpha1', + }, + }, + }; + + mockGet.mockResolvedValueOnce(mockDashboardWithError); + + const api = new K8sDashboardV2API(); + await expect(api.getDashboardDTO('test')).resolves.toBeDefined(); + }); }); }); diff --git a/public/app/features/dashboard/api/v2.ts b/public/app/features/dashboard/api/v2.ts index 21ceeef053f..72fb4501f08 100644 --- a/public/app/features/dashboard/api/v2.ts +++ b/public/app/features/dashboard/api/v2.ts @@ -40,7 +40,11 @@ export class K8sDashboardV2API try { const dashboard = await this.client.subresource>(uid, 'dto'); - if (dashboard.status?.conversion?.failed) { + if ( + dashboard.status?.conversion?.failed && + (dashboard.status.conversion.storedVersion === 'v1alpha1' || + dashboard.status.conversion.storedVersion === 'v0alpha1') + ) { throw new DashboardVersionError(dashboard.status.conversion.storedVersion, dashboard.status.conversion.error); }