diff --git a/pkg/cmd/grafana-cli/commands/datamigrations/to_unified_storage.go b/pkg/cmd/grafana-cli/commands/datamigrations/to_unified_storage.go index 2e8a9892ab1..a884c8f16cf 100644 --- a/pkg/cmd/grafana-cli/commands/datamigrations/to_unified_storage.go +++ b/pkg/cmd/grafana-cli/commands/datamigrations/to_unified_storage.go @@ -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") { diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go index 81ed308cdcc..8c465ef7b24 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go @@ -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 diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go index b142b144800..e95ee0db6aa 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go @@ -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) diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index ce8345d2b09..eb448ba41a8 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -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)