Feature Management: Define HideFromAdminPage and AllowSelfServe configs (#77580)

* Feature Management: Define HideFromAdminPage and AllowSelfServe configs

* update tests

* add constraint for self-serve

* Update pkg/services/featuremgmt/models.go

Co-authored-by: Michael Mandrus <41969079+mmandrus@users.noreply.github.com>

---------

Co-authored-by: Michael Mandrus <41969079+mmandrus@users.noreply.github.com>
This commit is contained in:
João Calisto 2023-11-03 15:59:07 +00:00 committed by GitHub
parent 3dac37380d
commit ade140c161
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 259 additions and 193 deletions

View File

@ -98,7 +98,7 @@ func isFeatureHidden(flag featuremgmt.FeatureFlag, hideCfg map[string]struct{})
if _, ok := hideCfg[flag.Name]; ok { if _, ok := hideCfg[flag.Name]; ok {
return true return true
} }
return flag.Stage == featuremgmt.FeatureStageUnknown || flag.Stage == featuremgmt.FeatureStageExperimental || flag.Stage == featuremgmt.FeatureStagePrivatePreview return flag.Stage == featuremgmt.FeatureStageUnknown || flag.Stage == featuremgmt.FeatureStageExperimental || flag.Stage == featuremgmt.FeatureStagePrivatePreview || flag.HideFromAdminPage
} }
// isFeatureWriteable returns whether a toggle on the admin page can be updated by the user. // isFeatureWriteable returns whether a toggle on the admin page can be updated by the user.
@ -110,7 +110,8 @@ func isFeatureWriteable(flag featuremgmt.FeatureFlag, readOnlyCfg map[string]str
if flag.Name == featuremgmt.FlagFeatureToggleAdminPage { if flag.Name == featuremgmt.FlagFeatureToggleAdminPage {
return false return false
} }
return flag.Stage == featuremgmt.FeatureStageGeneralAvailability || flag.Stage == featuremgmt.FeatureStageDeprecated allowSelfServe := flag.AllowSelfServe != nil && *flag.AllowSelfServe
return flag.Stage == featuremgmt.FeatureStageGeneralAvailability && allowSelfServe || flag.Stage == featuremgmt.FeatureStageDeprecated
} }
// isFeatureEditingAllowed checks if the backend is properly configured to allow feature toggle changes from the UI // isFeatureEditingAllowed checks if the backend is properly configured to allow feature toggle changes from the UI

View File

@ -20,6 +20,15 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
func boolPtr(b bool) *bool {
return &b
}
var (
truePtr = boolPtr(true)
falsePtr = boolPtr(false)
)
func TestGetFeatureToggles(t *testing.T) { func TestGetFeatureToggles(t *testing.T) {
readPermissions := []accesscontrol.Permission{{Action: accesscontrol.ActionFeatureManagementRead}} readPermissions := []accesscontrol.Permission{{Action: accesscontrol.ActionFeatureManagementRead}}
@ -109,18 +118,25 @@ func TestGetFeatureToggles(t *testing.T) {
}, { }, {
Name: "toggle4", Name: "toggle4",
Stage: featuremgmt.FeatureStagePublicPreview, Stage: featuremgmt.FeatureStagePublicPreview,
AllowSelfServe: truePtr,
}, { }, {
Name: "toggle5", Name: "toggle5",
Stage: featuremgmt.FeatureStageGeneralAvailability, Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: truePtr,
}, { }, {
Name: "toggle6", Name: "toggle6",
Stage: featuremgmt.FeatureStageDeprecated, Stage: featuremgmt.FeatureStageDeprecated,
AllowSelfServe: truePtr,
}, {
Name: "toggle7",
Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: falsePtr,
}, },
} }
t.Run("unknown, experimental, and private preview toggles are hidden by default", func(t *testing.T) { t.Run("unknown, experimental, and private preview toggles are hidden by default", func(t *testing.T) {
result := runGetScenario(t, features, setting.FeatureMgmtSettings{}, readPermissions, http.StatusOK) result := runGetScenario(t, features, setting.FeatureMgmtSettings{}, readPermissions, http.StatusOK)
assert.Len(t, result, 3) assert.Len(t, result, 4)
_, ok := findResult(t, result, "toggle1") _, ok := findResult(t, result, "toggle1")
assert.False(t, ok) assert.False(t, ok)
@ -130,13 +146,13 @@ func TestGetFeatureToggles(t *testing.T) {
assert.False(t, ok) assert.False(t, ok)
}) })
t.Run("only public preview and GA are writeable by default", func(t *testing.T) { t.Run("only public preview and GA with AllowSelfServe are writeable", func(t *testing.T) {
settings := setting.FeatureMgmtSettings{ settings := setting.FeatureMgmtSettings{
AllowEditing: true, AllowEditing: true,
UpdateWebhook: "bogus", UpdateWebhook: "bogus",
} }
result := runGetScenario(t, features, settings, readPermissions, http.StatusOK) result := runGetScenario(t, features, settings, readPermissions, http.StatusOK)
assert.Len(t, result, 3) assert.Len(t, result, 4)
t4, ok := findResult(t, result, "toggle4") t4, ok := findResult(t, result, "toggle4")
assert.True(t, ok) assert.True(t, ok)
@ -155,7 +171,7 @@ func TestGetFeatureToggles(t *testing.T) {
UpdateWebhook: "", UpdateWebhook: "",
} }
result := runGetScenario(t, features, settings, readPermissions, http.StatusOK) result := runGetScenario(t, features, settings, readPermissions, http.StatusOK)
assert.Len(t, result, 3) assert.Len(t, result, 4)
t4, ok := findResult(t, result, "toggle4") t4, ok := findResult(t, result, "toggle4")
assert.True(t, ok) assert.True(t, ok)
@ -308,10 +324,12 @@ func TestSetFeatureToggles(t *testing.T) {
Name: "toggle4", Name: "toggle4",
Enabled: false, Enabled: false,
Stage: featuremgmt.FeatureStageGeneralAvailability, Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: truePtr,
}, { }, {
Name: "toggle5", Name: "toggle5",
Enabled: false, Enabled: false,
Stage: featuremgmt.FeatureStageDeprecated, Stage: featuremgmt.FeatureStageDeprecated,
AllowSelfServe: truePtr,
}, },
} }

View File

@ -114,6 +114,8 @@ type FeatureFlag struct {
RequiresLicense bool `json:"requiresLicense,omitempty"` // Must be enabled in the license RequiresLicense bool `json:"requiresLicense,omitempty"` // Must be enabled in the license
FrontendOnly bool `json:"frontend,omitempty"` // change is only seen in the frontend FrontendOnly bool `json:"frontend,omitempty"` // change is only seen in the frontend
HideFromDocs bool `json:"hideFromDocs,omitempty"` // don't add the values to docs HideFromDocs bool `json:"hideFromDocs,omitempty"` // don't add the values to docs
HideFromAdminPage bool `json:"hideFromAdminPage,omitempty"` // don't display the feature in the admin page - add a comment with the reasoning
AllowSelfServe *bool `json:"allowSelfServe,omitempty"` // allow admin users to toggle the feature state from the admin page; this is required for GA toggles only
// This field is only for the feature management API. To enable your feature toggle by default, use `Expression`. // This field is only for the feature management API. To enable your feature toggle by default, use `Expression`.
Enabled bool `json:"enabled,omitempty"` Enabled bool `json:"enabled,omitempty"`

View File

@ -7,6 +7,8 @@
package featuremgmt package featuremgmt
var ( var (
falsePtr = boolPtr(false)
truePtr = boolPtr(true)
// Register each toggle here // Register each toggle here
standardFeatureFlags = []FeatureFlag{ standardFeatureFlags = []FeatureFlag{
{ {
@ -14,6 +16,7 @@ var (
Description: "Disable envelope encryption (emergency only)", Description: "Disable envelope encryption (emergency only)",
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: grafanaAsCodeSquad, Owner: grafanaAsCodeSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "live-service-web-worker", Name: "live-service-web-worker",
@ -41,6 +44,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: grafanaSharingSquad, Owner: grafanaSharingSquad,
Expression: "true", // enabled by default Expression: "true", // enabled by default
AllowSelfServe: truePtr,
}, },
{ {
Name: "publicDashboardsEmailSharing", Name: "publicDashboardsEmailSharing",
@ -61,6 +65,7 @@ var (
Description: "Highlight Grafana Enterprise features", Description: "Highlight Grafana Enterprise features",
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: grafanaAsCodeSquad, Owner: grafanaAsCodeSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "migrationLocking", Name: "migrationLocking",
@ -87,6 +92,7 @@ var (
Owner: grafanaExploreSquad, Owner: grafanaExploreSquad,
Expression: "true", // enabled by default Expression: "true", // enabled by default
FrontendOnly: true, FrontendOnly: true,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "datasourceQueryMultiStatus", Name: "datasourceQueryMultiStatus",
@ -154,6 +160,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Expression: "true", // turned on by default Expression: "true", // turned on by default
Owner: grafanaPluginsPlatformSquad, Owner: grafanaPluginsPlatformSquad,
AllowSelfServe: falsePtr,
}, },
{ {
// Some plugins rely on topnav feature flag being enabled, so we cannot remove this until we // Some plugins rely on topnav feature flag being enabled, so we cannot remove this until we
@ -190,6 +197,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Expression: "true", // enabled by default Expression: "true", // enabled by default
Owner: awsDatasourcesSquad, Owner: awsDatasourcesSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "redshiftAsyncQueryDataSupport", Name: "redshiftAsyncQueryDataSupport",
@ -197,6 +205,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Expression: "true", // enabled by default Expression: "true", // enabled by default
Owner: awsDatasourcesSquad, Owner: awsDatasourcesSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "athenaAsyncQueryDataSupport", Name: "athenaAsyncQueryDataSupport",
@ -205,6 +214,7 @@ var (
Expression: "true", // enabled by default Expression: "true", // enabled by default
FrontendOnly: true, FrontendOnly: true,
Owner: awsDatasourcesSquad, Owner: awsDatasourcesSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "cloudwatchNewRegionsHandler", Name: "cloudwatchNewRegionsHandler",
@ -243,12 +253,14 @@ var (
Owner: grafanaFrontendPlatformSquad, Owner: grafanaFrontendPlatformSquad,
FrontendOnly: true, FrontendOnly: true,
Expression: "true", // enabled by default Expression: "true", // enabled by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "accessTokenExpirationCheck", Name: "accessTokenExpirationCheck",
Description: "Enable OAuth access_token expiration check and token refresh using the refresh_token", Description: "Enable OAuth access_token expiration check and token refresh using the refresh_token",
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: identityAccessTeam, Owner: identityAccessTeam,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "emptyDashboardPage", Name: "emptyDashboardPage",
@ -257,12 +269,14 @@ var (
FrontendOnly: true, FrontendOnly: true,
Expression: "true", // enabled by default Expression: "true", // enabled by default
Owner: grafanaDashboardsSquad, Owner: grafanaDashboardsSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "disablePrometheusExemplarSampling", Name: "disablePrometheusExemplarSampling",
Description: "Disable Prometheus exemplar sampling", Description: "Disable Prometheus exemplar sampling",
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: grafanaObservabilityMetricsSquad, Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "alertingBacktesting", Name: "alertingBacktesting",
@ -291,6 +305,7 @@ var (
FrontendOnly: true, FrontendOnly: true,
Owner: grafanaObservabilityLogsSquad, Owner: grafanaObservabilityLogsSquad,
Expression: "true", // turned on by default Expression: "true", // turned on by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "lokiQuerySplitting", Name: "lokiQuerySplitting",
@ -299,6 +314,7 @@ var (
FrontendOnly: true, FrontendOnly: true,
Owner: grafanaObservabilityLogsSquad, Owner: grafanaObservabilityLogsSquad,
Expression: "true", // turned on by default Expression: "true", // turned on by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "lokiQuerySplittingConfig", Name: "lokiQuerySplittingConfig",
@ -318,6 +334,7 @@ var (
Description: "Prohibits a user from changing organization roles synced with Grafana Cloud auth provider", Description: "Prohibits a user from changing organization roles synced with Grafana Cloud auth provider",
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: identityAccessTeam, Owner: identityAccessTeam,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "prometheusMetricEncyclopedia", Name: "prometheusMetricEncyclopedia",
@ -326,6 +343,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
FrontendOnly: true, FrontendOnly: true,
Owner: grafanaObservabilityMetricsSquad, Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "influxdbBackendMigration", Name: "influxdbBackendMigration",
@ -334,6 +352,7 @@ var (
FrontendOnly: true, FrontendOnly: true,
Owner: grafanaObservabilityMetricsSquad, Owner: grafanaObservabilityMetricsSquad,
Expression: "true", // enabled by default Expression: "true", // enabled by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "clientTokenRotation", Name: "clientTokenRotation",
@ -347,6 +366,7 @@ var (
Expression: "true", Expression: "true",
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: grafanaObservabilityMetricsSquad, Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "lokiMetricDataplane", Name: "lokiMetricDataplane",
@ -354,6 +374,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Expression: "true", Expression: "true",
Owner: grafanaObservabilityLogsSquad, Owner: grafanaObservabilityLogsSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "lokiLogsDataplane", Name: "lokiLogsDataplane",
@ -368,6 +389,7 @@ var (
FrontendOnly: true, FrontendOnly: true,
Expression: "true", Expression: "true",
Owner: grafanaObservabilityMetricsSquad, Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "disableSSEDataplane", Name: "disableSSEDataplane",
@ -388,6 +410,7 @@ var (
FrontendOnly: true, FrontendOnly: true,
Expression: "true", // enabled by default Expression: "true", // enabled by default
Owner: grafanaAlertingSquad, Owner: grafanaAlertingSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "alertStateHistoryLokiPrimary", Name: "alertStateHistoryLokiPrimary",
@ -433,6 +456,7 @@ var (
Owner: grafanaOperatorExperienceSquad, Owner: grafanaOperatorExperienceSquad,
RequiresRestart: true, RequiresRestart: true,
Expression: "true", // enabled by default Expression: "true", // enabled by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "enableElasticsearchBackendQuerying", Name: "enableElasticsearchBackendQuerying",
@ -440,6 +464,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: grafanaObservabilityLogsSquad, Owner: grafanaObservabilityLogsSquad,
Expression: "true", // enabled by default Expression: "true", // enabled by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "advancedDataSourcePicker", Name: "advancedDataSourcePicker",
@ -448,6 +473,7 @@ var (
FrontendOnly: true, FrontendOnly: true,
Expression: "true", // enabled by default Expression: "true", // enabled by default
Owner: grafanaDashboardsSquad, Owner: grafanaDashboardsSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "faroDatasourceSelector", Name: "faroDatasourceSelector",
@ -526,6 +552,7 @@ var (
FrontendOnly: true, FrontendOnly: true,
Expression: "true", // enabled by default Expression: "true", // enabled by default
Owner: awsDatasourcesSquad, Owner: awsDatasourcesSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "exploreScrollableLogsContainer", Name: "exploreScrollableLogsContainer",
@ -540,6 +567,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Expression: "true", Expression: "true",
Owner: grafanaObservabilityMetricsSquad, Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "pluginsDynamicAngularDetectionPatterns", Name: "pluginsDynamicAngularDetectionPatterns",
@ -582,6 +610,7 @@ var (
FrontendOnly: true, FrontendOnly: true,
Expression: "true", // enabled by default Expression: "true", // enabled by default
Owner: grafanaObservabilityMetricsSquad, Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "mlExpressions", Name: "mlExpressions",
@ -646,6 +675,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: grafanaPartnerPluginsSquad, Owner: grafanaPartnerPluginsSquad,
Expression: "true", // on by default Expression: "true", // on by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "traceToProfiles", Name: "traceToProfiles",
@ -666,6 +696,7 @@ var (
Owner: grafanaObservabilityMetricsSquad, Owner: grafanaObservabilityMetricsSquad,
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Expression: "true", // on by default Expression: "true", // on by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "configurableSchedulerTick", Name: "configurableSchedulerTick",
@ -707,6 +738,7 @@ var (
FrontendOnly: true, FrontendOnly: true,
Owner: grafanaDashboardsSquad, Owner: grafanaDashboardsSquad,
Expression: "true", // on by default Expression: "true", // on by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "reportingRetries", Name: "reportingRetries",
@ -723,6 +755,7 @@ var (
Owner: grafanaFrontendPlatformSquad, Owner: grafanaFrontendPlatformSquad,
FrontendOnly: true, FrontendOnly: true,
Expression: "true", // on by default Expression: "true", // on by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "sseGroupByDatasource", Name: "sseGroupByDatasource",
@ -766,6 +799,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Owner: grafanaAlertingSquad, Owner: grafanaAlertingSquad,
Expression: "true", // enabled by default Expression: "true", // enabled by default
AllowSelfServe: falsePtr,
}, },
{ {
Name: "alertingContactPointsV2", Name: "alertingContactPointsV2",
@ -808,6 +842,7 @@ var (
Stage: FeatureStageGeneralAvailability, Stage: FeatureStageGeneralAvailability,
Expression: "true", // enabled by default Expression: "true", // enabled by default
Owner: awsDatasourcesSquad, Owner: awsDatasourcesSquad,
AllowSelfServe: falsePtr,
}, },
{ {
Name: "externalServiceAccounts", Name: "externalServiceAccounts",
@ -991,3 +1026,7 @@ var (
}, },
} }
) )
func boolPtr(b bool) *bool {
return &b
}

View File

@ -45,6 +45,12 @@ func TestFeatureToggleFiles(t *testing.T) {
if flag.Name != strings.TrimSpace(flag.Name) { if flag.Name != strings.TrimSpace(flag.Name) {
t.Errorf("flag Name should not start/end with spaces. See: %s", flag.Name) t.Errorf("flag Name should not start/end with spaces. See: %s", flag.Name)
} }
if flag.Stage == FeatureStageGeneralAvailability && flag.AllowSelfServe == nil {
t.Errorf("feature stage FeatureStageGeneralAvailability should have the AllowSelfServe field defined")
}
if flag.AllowSelfServe != nil && flag.Stage != FeatureStageGeneralAvailability {
t.Errorf("only allow self-serving GA toggles")
}
} }
}) })