Repurpose wrong datasource UID feature flag (#89363)

This commit is contained in:
Andres Martinez Gotor
2024-06-20 12:56:39 +02:00
committed by GitHub
parent bf3a383489
commit 44fd13c742
11 changed files with 38 additions and 71 deletions

View File

@@ -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

View File

@@ -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;
}

View File

@@ -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."))
)

View File

@@ -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)
}
}
}

View File

@@ -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")
})
})

View File

@@ -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.",

View File

@@ -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
1 Name Stage Owner requiresDevMode RequiresRestart FrontendOnly
161 disableNumericMetricsSortingInExpressions experimental @grafana/observability-metrics false true false
162 grafanaManagedRecordingRules experimental @grafana/alerting-squad false false false
163 queryLibrary experimental @grafana/explore-squad false false false
autofixDSUID experimental @grafana/plugins-platform-backend false false false
164 logsExploreTableDefaultVisualization experimental @grafana/observability-logs false false true
165 newDashboardSharingComponent experimental @grafana/sharing-squad false false true
166 alertingListViewV2 experimental @grafana/alerting-squad false false true
176 authZGRPCServer experimental @grafana/identity-access-team false false false
177 openSearchBackendFlowEnabled preview @grafana/aws-datasources false false false
178 ssoSettingsLDAP experimental @grafana/identity-access-team false false false
179 failWrongDSUID experimental @grafana/plugins-platform-backend false false false
180 databaseReadReplica experimental @grafana/grafana-backend-services-squad false false false
181 zanzana experimental @grafana/identity-access-team false false false

View File

@@ -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"

View File

@@ -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",

View File

@@ -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
}

View File

@@ -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))
})
}
}