mirror of
https://github.com/grafana/grafana.git
synced 2025-02-25 18:55:37 -06:00
Migrate wrong datasource UIDs (#86598)
This commit is contained in:
parent
b25f193be0
commit
b6f899d953
@ -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
|
||||
|
@ -181,5 +181,6 @@ export interface FeatureToggles {
|
||||
disableNumericMetricsSortingInExpressions?: boolean;
|
||||
grafanaManagedRecordingRules?: boolean;
|
||||
queryLibrary?: boolean;
|
||||
autofixDSUID?: boolean;
|
||||
logsExploreTableDefaultVisualization?: boolean;
|
||||
}
|
||||
|
@ -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,
|
||||
|
@ -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,
|
||||
)
|
||||
}
|
||||
|
@ -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) {
|
||||
|
@ -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",
|
||||
|
@ -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
|
||||
|
|
@ -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"
|
||||
|
@ -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",
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user