diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 5d75d2a3628..b1710745b66 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -178,6 +178,7 @@ Experimental features might be changed or removed without prior notice. | `accessActionSets` | Introduces action sets for resource permissions | | `disableNumericMetricsSortingInExpressions` | In server-side expressions, disable the sorting of numeric-kind metrics by their metric name or labels. | | `queryLibrary` | Enables Query Library feature in Explore | +| `autofixDSUID` | Automatically migrates invalid datasource UIDs | | `logsExploreTableDefaultVisualization` | Sets the logs table as default visualisation in logs explore | ## Development feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 7b1da776335..a1f6325cd6e 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -181,5 +181,6 @@ export interface FeatureToggles { disableNumericMetricsSortingInExpressions?: boolean; grafanaManagedRecordingRules?: boolean; queryLibrary?: boolean; + autofixDSUID?: boolean; logsExploreTableDefaultVisualization?: boolean; } diff --git a/pkg/services/datasources/service/datasource.go b/pkg/services/datasources/service/datasource.go index ac2cb09a6f6..5f643ca893d 100644 --- a/pkg/services/datasources/service/datasource.go +++ b/pkg/services/datasources/service/datasource.go @@ -64,7 +64,7 @@ func ProvideService( quotaService quota.Service, pluginStore pluginstore.Store, ) (*Service, error) { dslogger := log.New("datasources") - store := &SqlStore{db: db, logger: dslogger} + store := &SqlStore{db: db, logger: dslogger, features: features} s := &Service{ SQLStore: store, SecretsStore: secretsStore, diff --git a/pkg/services/datasources/service/store.go b/pkg/services/datasources/service/store.go index 70c0c21f289..e32810453a7 100644 --- a/pkg/services/datasources/service/store.go +++ b/pkg/services/datasources/service/store.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/grafana/pkg/infra/metrics" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/util" @@ -36,8 +37,9 @@ type Store interface { } type SqlStore struct { - db db.DB - logger log.Logger + db db.DB + logger log.Logger + features featuremgmt.FeatureToggles } func CreateStore(db db.DB, logger log.Logger) *SqlStore { @@ -59,11 +61,17 @@ func (ss *SqlStore) GetDataSource(ctx context.Context, query *datasources.GetDat }) } -func (ss *SqlStore) getDataSource(ctx context.Context, query *datasources.GetDataSourceQuery, sess *db.Session) (*datasources.DataSource, error) { +func (ss *SqlStore) getDataSource(_ context.Context, query *datasources.GetDataSourceQuery, sess *db.Session) (*datasources.DataSource, error) { if query.OrgID == 0 || (query.ID == 0 && len(query.Name) == 0 && len(query.UID) == 0) { return nil, datasources.ErrDataSourceIdentifierNotSet } + if len(query.UID) > 0 { + if err := util.ValidateUID(query.UID); err != nil { + logDeprecatedInvalidDsUid(ss.logger, query.UID, query.Name, "read", fmt.Errorf("invalid UID")) + } + } + datasource := &datasources.DataSource{Name: query.Name, OrgID: query.OrgID, ID: query.ID, UID: query.UID} has, err := sess.Get(datasource) @@ -250,7 +258,10 @@ func (ss *SqlStore) AddDataSource(ctx context.Context, cmd *datasources.AddDataS } cmd.UID = uid } else if err := util.ValidateUID(cmd.UID); err != nil { - logDeprecatedInvalidDsUid(ss.logger, cmd.UID, cmd.Name, err) + logDeprecatedInvalidDsUid(ss.logger, cmd.UID, cmd.Name, "create", err) + if ss.features != nil && ss.features.IsEnabled(ctx, featuremgmt.FlagAutofixDSUID) { + return fmt.Errorf("invalid UID for datasource %s: %w", cmd.Name, err) + } } ds = &datasources.DataSource{ @@ -321,6 +332,15 @@ func (ss *SqlStore) UpdateDataSource(ctx context.Context, cmd *datasources.Updat cmd.JsonData = simplejson.New() } + if cmd.UID != "" { + if err := util.ValidateUID(cmd.UID); err != nil { + logDeprecatedInvalidDsUid(ss.logger, cmd.UID, cmd.Name, "update", err) + if ss.features != nil && ss.features.IsEnabled(ctx, featuremgmt.FlagAutofixDSUID) { + cmd.UID = util.AutofixUID(cmd.UID) + } + } + } + ds = &datasources.DataSource{ ID: cmd.ID, OrgID: cmd.OrgID, @@ -387,12 +407,6 @@ func (ss *SqlStore) UpdateDataSource(ctx context.Context, cmd *datasources.Updat } } - if cmd.UID != "" { - if err := util.ValidateUID(cmd.UID); err != nil { - logDeprecatedInvalidDsUid(ss.logger, cmd.UID, cmd.Name, err) - } - } - return err }) } @@ -416,11 +430,10 @@ func generateNewDatasourceUid(sess *db.Session, orgId int64) (string, error) { var generateNewUid func() string = util.GenerateShortUID -func logDeprecatedInvalidDsUid(logger log.Logger, uid string, name string, err error) { +func logDeprecatedInvalidDsUid(logger log.Logger, uid string, name string, action string, err error) { logger.Warn( - "Invalid datasource uid. The use of invalid uids is deprecated and this operation will fail in a future "+ - "version of Grafana. A valid uid is a combination of a-z, A-Z, 0-9 (alphanumeric), - (dash) and _ "+ - "(underscore) characters, maximum length 40", - "uid", uid, "name", name, "error", err, + "Invalid datasource uid. A valid uid is a combination of a-z, A-Z, 0-9 (alphanumeric), - (dash) and _ "+ + "(underscore) characters, maximum length 40. Invalid characters will be replaced by dashes.", + "uid", uid, "action", action, "name", name, "error", err, ) } diff --git a/pkg/services/datasources/service/store_test.go b/pkg/services/datasources/service/store_test.go index 70352aef450..f942d18c0c0 100644 --- a/pkg/services/datasources/service/store_test.go +++ b/pkg/services/datasources/service/store_test.go @@ -11,8 +11,10 @@ import ( "github.com/grafana/grafana/pkg/events" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/log" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/featuremgmt" ) func TestIntegrationDataAccess(t *testing.T) { @@ -96,6 +98,19 @@ func TestIntegrationDataAccess(t *testing.T) { require.IsType(t, datasources.ErrDataSourceUidExists, err) }) + t.Run("fails to create a datasource with an invalid uid", func(t *testing.T) { + db := db.InitTestDB(t) + ss := SqlStore{ + db: db, + logger: log.NewNopLogger(), + features: featuremgmt.WithFeatures(featuremgmt.FlagAutofixDSUID), + } + cmd := defaultAddDatasourceCommand + cmd.UID = "test/uid" + _, err := ss.AddDataSource(context.Background(), &cmd) + require.ErrorContains(t, err, "invalid format of UID") + }) + t.Run("fires an event when the datasource is added", func(t *testing.T) { db := db.InitTestDB(t) sqlStore := SqlStore{db: db} @@ -213,6 +228,30 @@ func TestIntegrationDataAccess(t *testing.T) { _, err := ss.UpdateDataSource(context.Background(), cmd) require.NoError(t, err) }) + + t.Run("updates UID with a valid one", func(t *testing.T) { + db := db.InitTestDB(t) + ds := initDatasource(db) + ss := SqlStore{ + db: db, + logger: log.NewNopLogger(), + features: featuremgmt.WithFeatures(featuremgmt.FlagAutofixDSUID), + } + require.NotEmpty(t, ds.UID) + + cmd := defaultUpdateDatasourceCommand + cmd.ID = ds.ID + cmd.UID = "new/uid" + res, err := ss.UpdateDataSource(context.Background(), &cmd) + require.NoError(t, err) + require.Equal(t, "new-uid", res.UID) + + // Return the datasource with the valid UID + query := datasources.GetDataSourceQuery{UID: "new-uid", OrgID: 10} + dataSource, err := ss.GetDataSource(context.Background(), &query) + require.NoError(t, err) + require.Equal(t, "new-uid", dataSource.UID) + }) }) t.Run("DeleteDataSourceById", func(t *testing.T) { diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 9e47518752a..8e8ef9cf285 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1220,6 +1220,12 @@ var ( FrontendOnly: false, AllowSelfServe: false, }, + { + Name: "autofixDSUID", + Description: "Automatically migrates invalid datasource UIDs", + Stage: FeatureStageExperimental, + Owner: grafanaPluginsPlatformSquad, + }, { Name: "logsExploreTableDefaultVisualization", Description: "Sets the logs table as default visualisation in logs explore", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 8d14f41e918..6b30739004e 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -162,4 +162,5 @@ accessActionSets,experimental,@grafana/identity-access-team,false,false,false disableNumericMetricsSortingInExpressions,experimental,@grafana/observability-metrics,false,true,false grafanaManagedRecordingRules,experimental,@grafana/alerting-squad,false,false,false queryLibrary,experimental,@grafana/explore-squad,false,false,false +autofixDSUID,experimental,@grafana/plugins-platform-backend,false,false,false logsExploreTableDefaultVisualization,experimental,@grafana/observability-logs,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index cd95d4af1ac..7cb51a9b304 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -659,6 +659,10 @@ const ( // Enables Query Library feature in Explore FlagQueryLibrary = "queryLibrary" + // FlagAutofixDSUID + // Automatically migrates invalid datasource UIDs + FlagAutofixDSUID = "autofixDSUID" + // FlagLogsExploreTableDefaultVisualization // Sets the logs table as default visualisation in logs explore FlagLogsExploreTableDefaultVisualization = "logsExploreTableDefaultVisualization" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 46beebfbc6c..b7148bc4148 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -2109,6 +2109,18 @@ "codeowner": "@grafana/explore-squad" } }, + { + "metadata": { + "name": "autofixDSUID", + "resourceVersion": "1714386506243", + "creationTimestamp": "2024-04-29T10:28:26Z" + }, + "spec": { + "description": "Automatically migrates invalid datasource UIDs", + "stage": "experimental", + "codeowner": "@grafana/plugins-platform-backend" + } + }, { "metadata": { "name": "logsExploreTableDefaultVisualization", diff --git a/pkg/util/shortid_generator.go b/pkg/util/shortid_generator.go index db216e622a6..b56aa89b633 100644 --- a/pkg/util/shortid_generator.go +++ b/pkg/util/shortid_generator.go @@ -30,7 +30,9 @@ var ( var mtx sync.Mutex // Legacy UID pattern -var validUIDPattern = regexp.MustCompile(`^[a-zA-Z0-9\-\_]*$`).MatchString +var validUIDCharPattern = `a-zA-Z0-9\-\_` +var validUIDPattern = regexp.MustCompile(`^[` + validUIDCharPattern + `]*$`).MatchString +var validUIDReplacer = regexp.MustCompile(`[^` + validUIDCharPattern + `]`).ReplaceAllString // IsValidShortUID checks if short unique identifier contains valid characters // NOTE: future Grafana UIDs will need conform to https://github.com/kubernetes/apimachinery/blob/master/pkg/util/validation/validation.go#L43 @@ -91,3 +93,13 @@ func ValidateUID(uid string) error { } return nil } + +func AutofixUID(uid string) string { + if IsShortUIDTooLong(uid) { + return uid[:MaxUIDLength] + } + if !IsValidShortUID(uid) { + uid = validUIDReplacer(uid, "-") + } + return uid +} diff --git a/pkg/util/shortid_generator_test.go b/pkg/util/shortid_generator_test.go index 6bc4bb05b5c..ea273522889 100644 --- a/pkg/util/shortid_generator_test.go +++ b/pkg/util/shortid_generator_test.go @@ -143,3 +143,32 @@ func TestValidateUID(t *testing.T) { }) } } + +func TestAutofixUID(t *testing.T) { + var tests = []struct { + name string + uid string + expected string + }{ + { + name: "return input when input is valid", + uid: "f8cc010c-ee72-4681-89d2-d46e1bd47d33", + expected: "f8cc010c-ee72-4681-89d2-d46e1bd47d33", + }, + { + name: "generate new uid when input is too long", + uid: strings.Repeat("1", MaxUIDLength+1), + expected: strings.Repeat("1", MaxUIDLength), + }, + { + name: "generate new uid when input has invalid characters", + uid: "f8cc010c.ee72.4681;89d2+d46e1bd47d33", + expected: "f8cc010c-ee72-4681-89d2-d46e1bd47d33", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.expected, AutofixUID(tt.uid)) + }) + } +}