From 30c2a11a06dbcdcd65f29bfbe7ab5b8a5dde9102 Mon Sep 17 00:00:00 2001 From: Dominik Prokop Date: Thu, 15 May 2025 14:00:07 +0200 Subject: [PATCH] Dashboards V2 API: Ignore conversion status when the /dto response is V2 spec (#105381) * Dashboards V2 API: Ignore conversion status when the /dto response is V2 spec * fix lint --- .../dashboard/api/UnifiedDashboardAPI.test.ts | 53 ++++++++++++++++++- public/app/features/dashboard/api/v2.test.ts | 50 ++++++++++++++++- public/app/features/dashboard/api/v2.ts | 4 ++ 3 files changed, 104 insertions(+), 3 deletions(-) diff --git a/public/app/features/dashboard/api/UnifiedDashboardAPI.test.ts b/public/app/features/dashboard/api/UnifiedDashboardAPI.test.ts index 238d21f74c8..36dddf6000d 100644 --- a/public/app/features/dashboard/api/UnifiedDashboardAPI.test.ts +++ b/public/app/features/dashboard/api/UnifiedDashboardAPI.test.ts @@ -1,5 +1,8 @@ import { Dashboard } from '@grafana/schema/dist/esm/index'; -import { Spec as DashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha1/types.spec.gen'; +import { + Spec as DashboardV2Spec, + defaultSpec as defaultDashboardV2Spec, +} from '@grafana/schema/dist/esm/schema/dashboard/v2alpha1/types.spec.gen'; import { DashboardDTO } from 'app/types'; import { SaveDashboardCommand } from '../components/SaveDashboard/types'; @@ -13,6 +16,27 @@ import { K8sDashboardV2API } from './v2'; jest.mock('./v1'); jest.mock('./v2'); +let mockBackendSrvGet = {}; + +// Mocking just for the sake of not importing the entire universe. +// backendSrv.getFolderByUid is used in the v2 client +jest.mock('app/core/services/backend_srv', () => { + return { + backendSrv: {}, + }; +}); + +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + getBackendSrv: () => ({ + get: jest.fn(() => { + return new Promise((resolve) => { + resolve(mockBackendSrvGet); + }); + }), + }), +})); + describe('UnifiedDashboardAPI', () => { let api: UnifiedDashboardAPI; let v1Client: jest.Mocked; @@ -47,6 +71,33 @@ describe('UnifiedDashboardAPI', () => { expect(result).toBe(mockV2Response); expect(v2Client.getDashboardDTO).toHaveBeenCalledWith('123'); }); + + it('should return v2 even if there is a conversion error', async () => { + const mockV2Response = { + spec: defaultDashboardV2Spec(), + status: { + conversion: { + failed: true, + storedVersion: 'v0alpha1', + error: 'backend conversion not yet implemented', + }, + }, + metadata: { + annotations: {}, + }, + }; + + v1Client.getDashboardDTO.mockRejectedValue(new DashboardVersionError('v2alpha1', 'Dashboard is V1 format')); + v2Client.getDashboardDTO.mockImplementation((params) => { + const actualClient = jest.requireActual('./v2').K8sDashboardV2API; + const client = new actualClient(); + return client.getDashboardDTO(params); + }); + mockBackendSrvGet = mockV2Response; + const result = await api.getDashboardDTO('123'); + expect(result).toEqual(mockV2Response); + expect(v2Client.getDashboardDTO).toHaveBeenCalledWith('123'); + }); }); describe('saveDashboard', () => { diff --git a/public/app/features/dashboard/api/v2.test.ts b/public/app/features/dashboard/api/v2.test.ts index 79459ba28de..2ac3cb361fe 100644 --- a/public/app/features/dashboard/api/v2.test.ts +++ b/public/app/features/dashboard/api/v2.test.ts @@ -223,7 +223,7 @@ describe('v2 dashboard API', () => { }); describe('version error handling', () => { - it('should throw DashboardVersionError for v0alpha1 conversion error', async () => { + it('should not throw DashboardVersionError for v0alpha1 conversion error and v2 spec', async () => { const mockDashboardWithError = { ...mockDashboardDto, status: { @@ -237,11 +237,34 @@ describe('v2 dashboard API', () => { mockGet.mockResolvedValueOnce(mockDashboardWithError); + const api = new K8sDashboardV2API(); + await expect(api.getDashboardDTO('test')).resolves.toBe(mockDashboardWithError); + }); + + it('should throw DashboardVersionError for v0alpha1 conversion error and v1 spec', async () => { + const mockDashboardWithError = { + ...mockDashboardDto, + spec: { + // this is a v1 dashboard + title: 'test-dashboard', + panels: [], + }, + 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 v1beta1 conversion error', async () => { + it('should not throw DashboardVersionError for v1beta1 conversion error and v2 spec', async () => { const mockDashboardWithError = { ...mockDashboardDto, status: { @@ -255,6 +278,29 @@ describe('v2 dashboard API', () => { mockGet.mockResolvedValueOnce(mockDashboardWithError); + const api = new K8sDashboardV2API(); + await expect(api.getDashboardDTO('test')).resolves.toBe(mockDashboardWithError); + }); + + it('should throw DashboardVersionError for v1beta1 conversion error and v1 spec', async () => { + const mockDashboardWithError = { + ...mockDashboardDto, + spec: { + // this is a v1 dashboard + title: 'test-dashboard', + panels: [], + }, + status: { + conversion: { + failed: true, + error: 'backend conversion not yet implemented', + storedVersion: 'v1beta1', + }, + }, + }; + + mockGet.mockResolvedValueOnce(mockDashboardWithError); + const api = new K8sDashboardV2API(); await expect(api.getDashboardDTO('test')).rejects.toThrow('backend conversion not yet implemented'); }); diff --git a/public/app/features/dashboard/api/v2.ts b/public/app/features/dashboard/api/v2.ts index 523b15c89bf..577eb4d2501 100644 --- a/public/app/features/dashboard/api/v2.ts +++ b/public/app/features/dashboard/api/v2.ts @@ -22,6 +22,7 @@ import { DashboardDTO, SaveDashboardResponseDTO } from 'app/types'; import { SaveDashboardCommand } from '../components/SaveDashboard/types'; import { DashboardAPI, DashboardVersionError, DashboardWithAccessInfo } from './types'; +import { isDashboardV2Spec } from './utils'; export const K8S_V2_DASHBOARD_API_CONFIG = { group: 'dashboard.grafana.app', @@ -42,7 +43,10 @@ export class K8sDashboardV2API try { const dashboard = await this.client.subresource>(uid, 'dto'); + // FOR /dto calls returning v2 spec we are ignoring the conversion status to avoid runtime errors caused by the status + // being saved for v2 resources that's been client-side converted to v2 and then PUT to the API server. if ( + !isDashboardV2Spec(dashboard.spec) && dashboard.status?.conversion?.failed && (dashboard.status.conversion.storedVersion === 'v1alpha1' || dashboard.status.conversion.storedVersion === 'v1beta1' ||