public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: make graceful handle default for the malformed dashboard jsons during unified migration (#114295)

Browse Source 
* fix: graceful handling by default

* fix: make fallback the default behavior
This commit is contained in:
Mustafa Sencer Özcan
2025-11-24 14:12:52 +01:00
committed by GitHub
parent 67ad43a784
commit 2f6836e78a
4 changed files with 18 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/cmd/grafana-cli/commands/datamigrations/to_unified_storage.go
-1
View File
@@ -83,7 +83,6 @@ func ToUnifiedStorage(c utils.CommandLine, cfg *setting.Cfg, sqlStore db.DB) err
legacysql.NewDatabaseProvider(sqlStore),
provisioning,
acimpl.ProvideAccessControl(featuremgmt.WithFeatures()),
featureToggles,
)
if c.Bool("non-interactive") {
pkg/registry/apis/dashboard/legacy/sql_dashboards.go
+5 -19
View File
@@ -83,9 +83,6 @@ type dashboardSqlAccess struct {
namespacer request.NamespaceMapper
provisioning provisioning.StubProvisioningService
// TODO: consider enabling this by default for on-prem migrations
invalidDashboardParseFallbackEnabled bool
// Use for writing (not reading)
dashStore dashboards.Store
dashboardSearchClient legacysearcher.DashboardSearchClient
@@ -106,7 +103,6 @@ func ProvideMigratorDashboardAccessor(
sql legacysql.LegacyDatabaseProvider,
provisioning provisioning.StubProvisioningService,
accessControl accesscontrol.AccessControl,
features featuremgmt.FeatureToggles,
) MigrationDashboardAccessor {
return &dashboardSqlAccess{
sql: sql,
@@ -116,8 +112,6 @@ func ProvideMigratorDashboardAccessor(
dashboardPermissionSvc: nil, // not needed for migration
libraryPanelSvc: nil, // not needed for migration
accessControl: accessControl,
//nolint:staticcheck // not yet migrated to OpenFeature
invalidDashboardParseFallbackEnabled: features.IsEnabled(context.Background(), featuremgmt.FlagScanRowInvalidDashboardParseFallbackEnabled),
}
}
@@ -141,8 +135,6 @@ func NewDashboardSQLAccess(sql legacysql.LegacyDatabaseProvider,
dashboardPermissionSvc: dashboardPermissionSvc,
libraryPanelSvc: libraryPanelSvc,
accessControl: accessControl,
//nolint:staticcheck // not yet migrated to OpenFeature
invalidDashboardParseFallbackEnabled: features.IsEnabled(context.Background(), featuremgmt.FlagScanRowInvalidDashboardParseFallbackEnabled),
}
}
@@ -593,17 +585,17 @@ func generateFallbackDashboard(data []byte, title, uid string) ([]byte, error) {
func (a *dashboardSqlAccess) parseDashboard(dash *dashboardV1.Dashboard, data []byte, id int64, title string) error {
if err := dash.Spec.UnmarshalJSON(data); err != nil {
a.log.Warn("error unmarshalling dashboard spec. Generating fallback dashboard data", "error", err, "uid", dash.UID, "name", dash.Name)
a.log.Warn("error unmarshalling dashboard spec. Generating fallback dashboard data", "error", err, "uid", dash.UID, "id", id, "name", dash.Name)
dash.Spec = *dashboardV0.NewDashboardSpec()
dashboardData, err := generateFallbackDashboard(data, title, string(dash.UID))
if err != nil {
a.log.Warn("error generating fallback dashboard data", "error", err, "uid", dash.UID, "name", dash.Name)
a.log.Warn("error generating fallback dashboard data", "error", err, "uid", dash.UID, "id", id, "name", dash.Name)
return err
}
if err = dash.Spec.UnmarshalJSON(dashboardData); err != nil {
a.log.Warn("error unmarshalling fallback dashboard data", "error", err, "uid", dash.UID, "name", dash.Name)
a.log.Warn("error unmarshalling fallback dashboard data", "error", err, "uid", dash.UID, "id", id, "name", dash.Name)
return err
}
}
@@ -715,14 +707,8 @@ func (a *dashboardSqlAccess) scanRow(rows *sql.Rows, history bool) (*dashboardRo
}
if len(data) > 0 {
if a.invalidDashboardParseFallbackEnabled {
if err := a.parseDashboard(dash, data, dashboard_id, title); err != nil {
return row, err
}
} else {
if err := dash.Spec.UnmarshalJSON(data); err != nil {
return row, fmt.Errorf("JSON unmarshal error for: %s // %w", dash.Name, err)
}
if err := a.parseDashboard(dash, data, dashboard_id, title); err != nil {
return row, err
}
}
// Ignore any saved values for id/version/uid
pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go
+11 -26
View File
@@ -35,10 +35,9 @@ func TestScanRow(t *testing.T) {
provisioner := provisioning.NewProvisioningServiceMock(context.Background())
provisioner.GetDashboardProvisionerResolvedPathFunc = func(name string) string { return "provisioner" }
store := &dashboardSqlAccess{
namespacer: func(_ int64) string { return "default" },
provisioning: provisioner,
log: log.New("test"),
invalidDashboardParseFallbackEnabled: false,
namespacer: func(_ int64) string { return "default" },
provisioning: provisioner,
log: log.New("test"),
}
columns := []string{"orgId", "dashboard_id", "name", "title", "folder_uid", "deleted", "plugin_id", "origin_name", "origin_path", "origin_hash", "origin_ts", "created", "createdBy", "createdByID", "updated", "updatedBy", "updatedByID", "version", "message", "data", "api_version"}
@@ -194,39 +193,25 @@ func TestScanRow(t *testing.T) {
require.Equal(t, "dashboard.grafana.app/"+migrationAPIVersion, row.Dash.APIVersion)
})
t.Run("should follow dashboard template when failing to unmarshal dashboard if feature flag X is enabled", func(t *testing.T) {
t.Run("should follow dashboard template when failing to unmarshal dashboard", func(t *testing.T) {
// row with bad data
badData := []byte(`{"rows":[{"panels":[{"targets":[{"refId":"A","target":"aliasSub(alias, '^(.{27}).+', '\1...')"}]}]}]}`)
rows := sqlmock.NewRows(columns).AddRow(1, id, uid, title, folderUID, nil, "", "", "", "", 0, timestamp, createdUser, 0, timestamp, updatedUser, 0, version, message, badData, "vXyz")
mock.ExpectQuery("SELECT *").WillReturnRows(rows)
resultRows, err := mockDB.Query("SELECT *")
require.NoError(t, err)
defer resultRows.Close() // nolint:errcheck
defer func() {
_ = resultRows.Close()
}()
resultRows.Next()
row, err := store.scanRow(resultRows, false)
require.Error(t, err, "JSON unmarshal error for: Test Dashboard // invalid character '1' in string escape code")
require.NotNil(t, row)
// correctly scans these
require.Equal(t, uid, row.Dash.Name)
require.Equal(t, version, row.RV)
require.Equal(t, "default", row.Dash.Namespace)
require.Equal(t, &continueToken{orgId: int64(1), id: id}, row.token)
// failure case: does NOT parse the dashboard itself
require.Equal(t, common.Unstructured{
Object: nil,
}, row.Dash.Spec)
// store with feature flag enabled
store = &dashboardSqlAccess{
namespacer: func(_ int64) string { return "default" },
provisioning: provisioner,
log: log.New("test"),
invalidDashboardParseFallbackEnabled: true,
namespacer: func(_ int64) string { return "default" },
provisioning: provisioner,
log: log.New("test"),
}
row, err = store.scanRow(resultRows, false)
row, err := store.scanRow(resultRows, false)
require.NoError(t, err)
require.NotNil(t, row)
require.Equal(t, uid, row.Dash.Name)
pkg/server/wire_gen.go
Generated
+2 -2
View File
@@ -532,7 +532,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api
if err != nil {
return nil, err
}
migrationDashboardAccessor := legacy.ProvideMigratorDashboardAccessor(legacyDatabaseProvider, stubProvisioningService, accessControl, featureToggles)
migrationDashboardAccessor := legacy.ProvideMigratorDashboardAccessor(legacyDatabaseProvider, stubProvisioningService, accessControl)
unifiedMigrator := migrations2.ProvideUnifiedMigrator(migrationDashboardAccessor, resourceClient)
unifiedStorageMigrationService := migrations2.ProvideUnifiedStorageMigrationService(unifiedMigrator, cfg, sqlStore, kvStore, resourceClient)
dualwriteService, err := dualwrite.ProvideService(featureToggles, kvStore, cfg, unifiedStorageMigrationService)
@@ -1179,7 +1179,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac
if err != nil {
return nil, err
}
migrationDashboardAccessor := legacy.ProvideMigratorDashboardAccessor(legacyDatabaseProvider, stubProvisioningService, accessControl, featureToggles)
migrationDashboardAccessor := legacy.ProvideMigratorDashboardAccessor(legacyDatabaseProvider, stubProvisioningService, accessControl)
unifiedMigrator := migrations2.ProvideUnifiedMigrator(migrationDashboardAccessor, resourceClient)
unifiedStorageMigrationService := migrations2.ProvideUnifiedStorageMigrationService(unifiedMigrator, cfg, sqlStore, kvStore, resourceClient)
dualwriteService, err := dualwrite.ProvideService(featureToggles, kvStore, cfg, unifiedStorageMigrationService)