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 fc6e4bd12ad..b2413585105 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -182,7 +182,6 @@ 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 | | `newDashboardSharingComponent` | Enables the new sharing drawer design | | `alertingListViewV2` | Enables the new alert list view design | @@ -191,6 +190,7 @@ Experimental features might be changed or removed without prior notice. | `alertingCentralAlertHistory` | Enables the new central alert history. | | `azureMonitorPrometheusExemplars` | Allows configuration of Azure Monitor as a data source that can provide Prometheus exemplars | | `pinNavItems` | Enables pinning of nav items | +| `failWrongDSUID` | Throws an error if a datasource has an invalid UIDs | | `databaseReadReplica` | Use a read replica for some database queries. | ## Development feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index a8ac9a8e1b0..a7b45c16968 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -180,7 +180,6 @@ export interface FeatureToggles { disableNumericMetricsSortingInExpressions?: boolean; grafanaManagedRecordingRules?: boolean; queryLibrary?: boolean; - autofixDSUID?: boolean; logsExploreTableDefaultVisualization?: boolean; newDashboardSharingComponent?: boolean; alertingListViewV2?: boolean; @@ -196,6 +195,7 @@ export interface FeatureToggles { authZGRPCServer?: boolean; openSearchBackendFlowEnabled?: boolean; ssoSettingsLDAP?: boolean; + failWrongDSUID?: boolean; databaseReadReplica?: boolean; zanzana?: boolean; } diff --git a/pkg/services/datasources/errors.go b/pkg/services/datasources/errors.go index e4db5a71d0f..8147b4a4e65 100644 --- a/pkg/services/datasources/errors.go +++ b/pkg/services/datasources/errors.go @@ -18,4 +18,5 @@ var ( ErrDataSourceNameInvalid = errutil.ValidationFailed("datasource.nameInvalid", errutil.WithPublicMessage("Invalid datasource name.")) ErrDataSourceURLInvalid = errutil.ValidationFailed("datasource.urlInvalid", errutil.WithPublicMessage("Invalid datasource url.")) ErrDataSourceAPIVersionInvalid = errutil.ValidationFailed("datasource.apiVersionInvalid", errutil.WithPublicMessage("Invalid datasource apiVersion.")) + ErrDataSourceUIDInvalid = errutil.ValidationFailed("datasource.uidInvalid", errutil.WithPublicMessage("Invalid datasource UID.")) ) diff --git a/pkg/services/datasources/service/store.go b/pkg/services/datasources/service/store.go index 27bbc95935c..601872aefc9 100644 --- a/pkg/services/datasources/service/store.go +++ b/pkg/services/datasources/service/store.go @@ -252,8 +252,8 @@ 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, "create", err) - if ss.features != nil && ss.features.IsEnabled(ctx, featuremgmt.FlagAutofixDSUID) { - return fmt.Errorf("invalid UID for datasource %s: %w", cmd.Name, err) + if ss.features != nil && ss.features.IsEnabled(ctx, featuremgmt.FlagFailWrongDSUID) { + return datasources.ErrDataSourceUIDInvalid.Errorf("invalid UID for datasource %s: %w", cmd.Name, err) } } @@ -329,8 +329,8 @@ 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, "update", err) - if ss.features != nil && ss.features.IsEnabled(ctx, featuremgmt.FlagAutofixDSUID) { - cmd.UID = util.AutofixUID(cmd.UID) + if ss.features != nil && ss.features.IsEnabled(ctx, featuremgmt.FlagFailWrongDSUID) { + return datasources.ErrDataSourceUIDInvalid.Errorf("invalid UID for datasource %s: %w", cmd.Name, err) } } } diff --git a/pkg/services/datasources/service/store_test.go b/pkg/services/datasources/service/store_test.go index e0cd2e06dc2..5ade7093422 100644 --- a/pkg/services/datasources/service/store_test.go +++ b/pkg/services/datasources/service/store_test.go @@ -104,7 +104,7 @@ func TestIntegrationDataAccess(t *testing.T) { ss := SqlStore{ db: db, logger: log.NewNopLogger(), - features: featuremgmt.WithFeatures(featuremgmt.FlagAutofixDSUID), + features: featuremgmt.WithFeatures(featuremgmt.FlagFailWrongDSUID), } cmd := defaultAddDatasourceCommand cmd.UID = "test/uid" @@ -232,28 +232,21 @@ func TestIntegrationDataAccess(t *testing.T) { require.NoError(t, err) }) - t.Run("updates UID with a valid one", func(t *testing.T) { + t.Run("fails to update a datasource with an invalid uid", func(t *testing.T) { db := db.InitTestDB(t) ds := initDatasource(db) ss := SqlStore{ db: db, logger: log.NewNopLogger(), - features: featuremgmt.WithFeatures(featuremgmt.FlagAutofixDSUID), + features: featuremgmt.WithFeatures(featuremgmt.FlagFailWrongDSUID), } 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) + _, err := ss.UpdateDataSource(context.Background(), &cmd) + require.ErrorContains(t, err, "invalid format of UID") }) }) diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index b769382e364..9117d55c666 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1222,12 +1222,6 @@ 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", @@ -1336,6 +1330,12 @@ var ( HideFromDocs: true, HideFromAdminPage: true, }, + { + Name: "failWrongDSUID", + Description: "Throws an error if a datasource has an invalid UIDs", + Stage: FeatureStageExperimental, + Owner: grafanaPluginsPlatformSquad, + }, { Name: "databaseReadReplica", Description: "Use a read replica for some database queries.", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 020c6443d5b..da88637ca63 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -161,7 +161,6 @@ 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 newDashboardSharingComponent,experimental,@grafana/sharing-squad,false,false,true alertingListViewV2,experimental,@grafana/alerting-squad,false,false,true @@ -177,5 +176,6 @@ pinNavItems,experimental,@grafana/grafana-frontend-platform,false,false,false authZGRPCServer,experimental,@grafana/identity-access-team,false,false,false openSearchBackendFlowEnabled,preview,@grafana/aws-datasources,false,false,false ssoSettingsLDAP,experimental,@grafana/identity-access-team,false,false,false +failWrongDSUID,experimental,@grafana/plugins-platform-backend,false,false,false databaseReadReplica,experimental,@grafana/grafana-backend-services-squad,false,false,false zanzana,experimental,@grafana/identity-access-team,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 0a4314ba04e..b56aa2c1e46 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -655,10 +655,6 @@ 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" @@ -719,6 +715,10 @@ const ( // Use the new SSO Settings API to configure LDAP FlagSsoSettingsLDAP = "ssoSettingsLDAP" + // FlagFailWrongDSUID + // Throws an error if a datasource has an invalid UIDs + FlagFailWrongDSUID = "failWrongDSUID" + // FlagDatabaseReadReplica // Use a read replica for some database queries. FlagDatabaseReadReplica = "databaseReadReplica" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 92a0a954eb4..e26f18d188b 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -401,8 +401,9 @@ { "metadata": { "name": "autofixDSUID", - "resourceVersion": "1718727528075", - "creationTimestamp": "2024-05-03T11:32:07Z" + "resourceVersion": "1717578796182", + "creationTimestamp": "2024-05-03T11:32:07Z", + "deletionTimestamp": "2024-06-18T14:28:32Z" }, "spec": { "description": "Automatically migrates invalid datasource UIDs", @@ -915,6 +916,18 @@ "frontend": true } }, + { + "metadata": { + "name": "failWrongDSUID", + "resourceVersion": "1718721033692", + "creationTimestamp": "2024-06-18T14:30:33Z" + }, + "spec": { + "description": "Throws an error if a datasource has an invalid UIDs", + "stage": "experimental", + "codeowner": "@grafana/plugins-platform-backend" + } + }, { "metadata": { "name": "faroDatasourceSelector", diff --git a/pkg/util/shortid_generator.go b/pkg/util/shortid_generator.go index b56aa89b633..152553f1824 100644 --- a/pkg/util/shortid_generator.go +++ b/pkg/util/shortid_generator.go @@ -32,7 +32,6 @@ var mtx sync.Mutex // Legacy UID pattern 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 @@ -93,13 +92,3 @@ 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 ea273522889..6bc4bb05b5c 100644 --- a/pkg/util/shortid_generator_test.go +++ b/pkg/util/shortid_generator_test.go @@ -143,32 +143,3 @@ 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)) - }) - } -}