IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2026-07-30 00:08:10 -05:00
Code Issues Packages Projects Releases Wiki Activity

FeatureFlags: Remove enabled from FeatureFlag model (#79960)

Browse Source
This commit is contained in:
Ryan McKinley
2024-01-10 21:34:18 -08:00
committed by GitHub
parent 48b5ac779b
commit 85d68b88cf
8 changed files with 95 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md
+1
View File
@@ -52,6 +52,7 @@ Some features are enabled by default. You can disable these feature by setting t
| `splitScopes` | Support faster dashboard and folder search by splitting permission scopes into parts | Yes |
| `prometheusConfigOverhaulAuth` | Update the Prometheus configuration page with the new auth component | Yes |
| `influxdbSqlSupport` | Enable InfluxDB SQL query language support with new querying UI | Yes |
| `alertingNoDataErrorExecution` | Changes how Alerting state manager handles execution of NoData/Error | Yes |
| `alertingInsights` | Show the new alerting insights landing page | Yes |
| `cloudWatchWildCardDimensionValues` | Fetches dimension values from CloudWatch to correctly label wildcard dimensions | Yes |
| `recoveryThreshold` | Enables feature recovery threshold (aka hysteresis) for threshold server-side expression | Yes |
pkg/api/featuremgmt_test.go
+59 -71
View File
@@ -25,24 +25,23 @@ func TestGetFeatureToggles(t *testing.T) {
readPermissions := []accesscontrol.Permission{{Action: accesscontrol.ActionFeatureManagementRead}}
t.Run("should not be able to get feature toggles without permissions", func(t *testing.T) {
result := runGetScenario(t, []*featuremgmt.FeatureFlag{}, setting.FeatureMgmtSettings{}, []accesscontrol.Permission{}, http.StatusForbidden)
result := runGetScenario(t, []*featuremgmt.FeatureFlag{}, []string{}, setting.FeatureMgmtSettings{}, []accesscontrol.Permission{}, http.StatusForbidden)
assert.Len(t, result, 0)
})
t.Run("should be able to get feature toggles with correct permissions", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: "toggle1",
Enabled: true,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle1",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Enabled: false,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle2",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
disabled := []string{"toggle2"}
result := runGetScenario(t, features, setting.FeatureMgmtSettings{}, readPermissions, http.StatusOK)
result := runGetScenario(t, features, disabled, setting.FeatureMgmtSettings{}, readPermissions, http.StatusOK)
assert.Len(t, result, 2)
t1, _ := findResult(t, result, "toggle1")
assert.True(t, t1.Enabled)
@@ -53,20 +52,18 @@ func TestGetFeatureToggles(t *testing.T) {
t.Run("toggles hidden by config are not present in the response", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: "toggle1",
Enabled: true,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle1",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Enabled: false,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle2",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
settings := setting.FeatureMgmtSettings{
HiddenToggles: map[string]struct{}{"toggle1": {}},
}
result := runGetScenario(t, features, settings, readPermissions, http.StatusOK)
result := runGetScenario(t, features, []string{}, settings, readPermissions, http.StatusOK)
assert.Len(t, result, 1)
assert.Equal(t, "toggle2", result[0].Name)
})
@@ -74,15 +71,14 @@ func TestGetFeatureToggles(t *testing.T) {
t.Run("toggles that are read-only by config have the readOnly field set", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: "toggle1",
Enabled: true,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle1",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Enabled: false,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle2",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
disabled := []string{"toggle2"}
settings := setting.FeatureMgmtSettings{
HiddenToggles: map[string]struct{}{"toggle1": {}},
ReadOnlyToggles: map[string]struct{}{"toggle2": {}},
@@ -90,7 +86,7 @@ func TestGetFeatureToggles(t *testing.T) {
UpdateWebhook: "bogus",
}
result := runGetScenario(t, features, settings, readPermissions, http.StatusOK)
result := runGetScenario(t, features, disabled, settings, readPermissions, http.StatusOK)
assert.Len(t, result, 1)
assert.Equal(t, "toggle2", result[0].Name)
assert.True(t, result[0].ReadOnly)
@@ -127,7 +123,7 @@ func TestGetFeatureToggles(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, []string{}, setting.FeatureMgmtSettings{}, readPermissions, http.StatusOK)
assert.Len(t, result, 4)
_, ok := findResult(t, result, "toggle1")
@@ -143,7 +139,7 @@ func TestGetFeatureToggles(t *testing.T) {
AllowEditing: true,
UpdateWebhook: "bogus",
}
result := runGetScenario(t, features, settings, readPermissions, http.StatusOK)
result := runGetScenario(t, features, []string{}, settings, readPermissions, http.StatusOK)
assert.Len(t, result, 4)
t4, ok := findResult(t, result, "toggle4")
@@ -162,7 +158,7 @@ func TestGetFeatureToggles(t *testing.T) {
AllowEditing: false,
UpdateWebhook: "",
}
result := runGetScenario(t, features, settings, readPermissions, http.StatusOK)
result := runGetScenario(t, features, []string{}, settings, readPermissions, http.StatusOK)
assert.Len(t, result, 4)
t4, ok := findResult(t, result, "toggle4")
@@ -182,12 +178,12 @@ func TestSetFeatureToggles(t *testing.T) {
writePermissions := []accesscontrol.Permission{{Action: accesscontrol.ActionFeatureManagementWrite}}
t.Run("fails without adequate permissions", func(t *testing.T) {
res := runSetScenario(t, nil, nil, setting.FeatureMgmtSettings{}, []accesscontrol.Permission{}, http.StatusForbidden)
res := runSetScenario(t, nil, []string{}, nil, setting.FeatureMgmtSettings{}, []accesscontrol.Permission{}, http.StatusForbidden)
defer func() { require.NoError(t, res.Body.Close()) }()
})
t.Run("fails when toggle editing is not enabled", func(t *testing.T) {
res := runSetScenario(t, nil, nil, setting.FeatureMgmtSettings{}, writePermissions, http.StatusForbidden)
res := runSetScenario(t, nil, []string{}, nil, setting.FeatureMgmtSettings{}, writePermissions, http.StatusForbidden)
defer func() { require.NoError(t, res.Body.Close()) }()
p := readBody(t, res.Body)
assert.Equal(t, "feature toggles are read-only", p["message"])
@@ -197,7 +193,7 @@ func TestSetFeatureToggles(t *testing.T) {
s := setting.FeatureMgmtSettings{
AllowEditing: true,
}
res := runSetScenario(t, nil, nil, s, writePermissions, http.StatusInternalServerError)
res := runSetScenario(t, nil, []string{}, nil, s, writePermissions, http.StatusInternalServerError)
defer func() { require.NoError(t, res.Body.Close()) }()
p := readBody(t, res.Body)
assert.Equal(t, "feature toggles service is misconfigured", p["message"])
@@ -206,16 +202,14 @@ func TestSetFeatureToggles(t *testing.T) {
t.Run("fails with non-existent toggle", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: "toggle1",
Enabled: true,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle1",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Enabled: false,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle2",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
disabled := []string{"toggle2"}
updates := []featuremgmt.FeatureToggleDTO{
{
Name: "toggle3",
@@ -227,7 +221,7 @@ func TestSetFeatureToggles(t *testing.T) {
AllowEditing: true,
UpdateWebhook: "random",
}
res := runSetScenario(t, features, updates, s, writePermissions, http.StatusBadRequest)
res := runSetScenario(t, features, disabled, updates, s, writePermissions, http.StatusBadRequest)
defer func() { require.NoError(t, res.Body.Close()) }()
p := readBody(t, res.Body)
assert.Equal(t, "invalid toggle passed in", p["message"])
@@ -236,19 +230,17 @@ func TestSetFeatureToggles(t *testing.T) {
t.Run("fails with read-only toggles", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: featuremgmt.FlagFeatureToggleAdminPage,
Enabled: true,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: featuremgmt.FlagFeatureToggleAdminPage,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Enabled: false,
Stage: featuremgmt.FeatureStagePublicPreview,
Name: "toggle2",
Stage: featuremgmt.FeatureStagePublicPreview,
}, {
Name: "toggle3",
Enabled: false,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle3",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
disabled := []string{"toggle2", "toggle3"}
s := setting.FeatureMgmtSettings{
AllowEditing: true,
@@ -265,7 +257,7 @@ func TestSetFeatureToggles(t *testing.T) {
Enabled: true,
},
}
res := runSetScenario(t, features, updates, s, writePermissions, http.StatusBadRequest)
res := runSetScenario(t, features, disabled, updates, s, writePermissions, http.StatusBadRequest)
defer func() { require.NoError(t, res.Body.Close()) }()
p := readBody(t, res.Body)
assert.Equal(t, "invalid toggle passed in", p["message"])
@@ -278,7 +270,7 @@ func TestSetFeatureToggles(t *testing.T) {
Enabled: true,
},
}
res := runSetScenario(t, features, updates, s, writePermissions, http.StatusBadRequest)
res := runSetScenario(t, features, disabled, updates, s, writePermissions, http.StatusBadRequest)
defer func() { require.NoError(t, res.Body.Close()) }()
p := readBody(t, res.Body)
assert.Equal(t, "invalid toggle passed in", p["message"])
@@ -291,7 +283,7 @@ func TestSetFeatureToggles(t *testing.T) {
Enabled: true,
},
}
res := runSetScenario(t, features, updates, s, writePermissions, http.StatusBadRequest)
res := runSetScenario(t, features, disabled, updates, s, writePermissions, http.StatusBadRequest)
defer func() { require.NoError(t, res.Body.Close()) }()
p := readBody(t, res.Body)
assert.Equal(t, "invalid toggle passed in", p["message"])
@@ -301,29 +293,25 @@ func TestSetFeatureToggles(t *testing.T) {
t.Run("when all conditions met", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: featuremgmt.FlagFeatureToggleAdminPage,
Enabled: true,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: featuremgmt.FlagFeatureToggleAdminPage,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Enabled: false,
Stage: featuremgmt.FeatureStagePublicPreview,
Name: "toggle2",
Stage: featuremgmt.FeatureStagePublicPreview,
}, {
Name: "toggle3",
Enabled: false,
Stage: featuremgmt.FeatureStageGeneralAvailability,
Name: "toggle3",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle4",
Enabled: false,
Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: true,
}, {
Name: "toggle5",
Enabled: false,
Stage: featuremgmt.FeatureStageDeprecated,
AllowSelfServe: true,
},
}
disabled := []string{"toggle2", "toggle3", "toggle4", "toggle5"}
s := setting.FeatureMgmtSettings{
AllowEditing: true,
@@ -349,7 +337,7 @@ func TestSetFeatureToggles(t *testing.T) {
}))
defer webhookServer.Close()
s.UpdateWebhook = webhookServer.URL
res := runSetScenario(t, features, updates, s, writePermissions, http.StatusBadRequest)
res := runSetScenario(t, features, disabled, updates, s, writePermissions, http.StatusBadRequest)
defer func() { require.NoError(t, res.Body.Close()) }()
assert.Equal(t, http.StatusBadRequest, res.StatusCode)
})
@@ -367,7 +355,7 @@ func TestSetFeatureToggles(t *testing.T) {
}))
defer webhookServer.Close()
s.UpdateWebhook = webhookServer.URL
res := runSetScenario(t, features, updates, s, writePermissions, http.StatusOK)
res := runSetScenario(t, features, disabled, updates, s, writePermissions, http.StatusOK)
defer func() { require.NoError(t, res.Body.Close()) }()
assert.Equal(t, http.StatusOK, res.StatusCode)
})
@@ -398,6 +386,7 @@ func readBody(t *testing.T, rc io.ReadCloser) map[string]any {
func runGetScenario(
t *testing.T,
features []*featuremgmt.FeatureFlag,
disabled []string, // the flags that are disabled
settings setting.FeatureMgmtSettings,
permissions []accesscontrol.Permission,
expectedCode int,
@@ -406,11 +395,10 @@ func runGetScenario(
cfg := setting.NewCfg()
cfg.FeatureManagement = settings
fm := featuremgmt.WithFeatureFlags(append([]*featuremgmt.FeatureFlag{{
Name: featuremgmt.FlagFeatureToggleAdminPage,
Enabled: true,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}}, features...))
fm := featuremgmt.WithFeatureManager(append([]*featuremgmt.FeatureFlag{{
Name: featuremgmt.FlagFeatureToggleAdminPage,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}}, features...), disabled...)
server := SetupAPITestServer(t, func(hs *HTTPServer) {
hs.Cfg = cfg
@@ -464,6 +452,7 @@ func runGetScenario(
func runSetScenario(
t *testing.T,
serverFeatures []*featuremgmt.FeatureFlag,
disabled []string, // the flags that are disabled
updateFeatures []featuremgmt.FeatureToggleDTO,
settings setting.FeatureMgmtSettings,
permissions []accesscontrol.Permission,
@@ -473,11 +462,10 @@ func runSetScenario(
cfg := setting.NewCfg()
cfg.FeatureManagement = settings
features := featuremgmt.WithFeatureFlags(append([]*featuremgmt.FeatureFlag{{
Name: featuremgmt.FlagFeatureToggleAdminPage,
Enabled: true,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}}, serverFeatures...))
features := featuremgmt.WithFeatureManager(append([]*featuremgmt.FeatureFlag{{
Name: featuremgmt.FlagFeatureToggleAdminPage,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}}, serverFeatures...), disabled...)
server := SetupAPITestServer(t, func(hs *HTTPServer) {
hs.Cfg = cfg
pkg/services/featuremgmt/manager.go
+22 -13
View File
@@ -22,6 +22,8 @@ type FeatureManager struct {
enabled map[string]bool // only the "on" values
config string // path to config file
vars map[string]any
startup map[string]bool // the explicit values registered at startup
warnings map[string]string // potential warnings about the flag
log log.Logger
}
@@ -73,16 +75,16 @@ func (fm *FeatureManager) registerFlags(flags ...FeatureFlag) {
}
// meetsRequirements checks if grafana is able to run the given feature due to dev mode or licensing requirements
func (fm *FeatureManager) meetsRequirements(ff *FeatureFlag) bool {
func (fm *FeatureManager) meetsRequirements(ff *FeatureFlag) (bool, string) {
if ff.RequiresDevMode && !fm.isDevMod {
return false
return false, "requires dev mode"
}
if ff.RequiresLicense && (fm.licensing == nil || !fm.licensing.FeatureEnabled(ff.Name)) {
return false
return false, "license requirement"
}
return true
return true, ""
}
// Update
@@ -90,14 +92,16 @@ func (fm *FeatureManager) update() {
enabled := make(map[string]bool)
for _, flag := range fm.flags {
// if grafana cannot run the feature, omit metrics around it
if !fm.meetsRequirements(flag) {
ok, reason := fm.meetsRequirements(flag)
if !ok {
fm.warnings[flag.Name] = reason
continue
}
// Update the registry
track := 0.0
// TODO: CEL - expression
if flag.Expression == "true" {
if flag.Expression == "true" || (fm.startup[flag.Name]) {
track = 1
enabled[flag.Name] = true
}
@@ -196,30 +200,35 @@ func WithManager(spec ...any) *FeatureManager {
idx++
}
features[key] = &FeatureFlag{Name: key, Enabled: val}
features[key] = &FeatureFlag{Name: key}
if val {
enabled[key] = true
}
}
return &FeatureManager{enabled: enabled, flags: features}
return &FeatureManager{enabled: enabled, flags: features, startup: enabled, warnings: map[string]string{}}
}
// WithFeatureFlags is used to define feature toggles for testing.
// WithFeatureManager is used to define feature toggle manager for testing.
// It should be used when your test feature toggles require metadata beyond `Name` and `Enabled`.
// You should provide a feature toggle Name at a minimum.
func WithFeatureFlags(flags []*FeatureFlag) *FeatureManager {
func WithFeatureManager(flags []*FeatureFlag, disabled ...string) *FeatureManager {
count := len(flags)
features := make(map[string]*FeatureFlag, count)
enabled := make(map[string]bool, count)
dis := make(map[string]bool)
for _, v := range disabled {
dis[v] = true
}
for _, f := range flags {
if f.Name == "" {
continue
}
features[f.Name] = f
enabled[f.Name] = f.Enabled
enabled[f.Name] = !dis[f.Name]
}
return &FeatureManager{enabled: enabled, flags: features}
return &FeatureManager{enabled: enabled, flags: features, startup: enabled, warnings: map[string]string{}}
}
pkg/services/featuremgmt/manager_test.go
+2 -1
View File
@@ -26,7 +26,8 @@ func TestFeatureManager(t *testing.T) {
t.Run("check license validation", func(t *testing.T) {
ft := FeatureManager{
flags: map[string]*FeatureFlag{},
flags: map[string]*FeatureFlag{},
warnings: map[string]string{},
}
ft.registerFlags(FeatureFlag{
Name: "a",
pkg/services/featuremgmt/models.go
+1 -4
View File
@@ -110,8 +110,8 @@ func (s *FeatureFlagStage) UnmarshalJSON(b []byte) error {
return nil
}
// These are properties about the feature, but not the current state or value for it
type FeatureFlag struct {
// Required properties
Name string `json:"name" yaml:"name"` // Unique name
Description string `json:"description"`
Stage FeatureFlagStage `json:"stage,omitempty"`
@@ -131,9 +131,6 @@ type FeatureFlag struct {
FrontendOnly bool `json:"frontend,omitempty"` // change is only seen in the frontend
HideFromDocs bool `json:"hideFromDocs,omitempty"` // don't add the values to docs
// This field is only for the feature management API. To enable your feature toggle by default, use `Expression`.
Enabled bool `json:"enabled,omitempty"`
// These are currently unused
DocsURL string `json:"docsURL,omitempty"`
RequiresRestart bool `json:"requiresRestart,omitempty"` // The server must be initialized with the value
pkg/services/featuremgmt/registry.go
+2 -2
View File
@@ -826,11 +826,11 @@ var (
{
Name: "alertingNoDataErrorExecution",
Description: "Changes how Alerting state manager handles execution of NoData/Error",
Stage: FeatureStagePrivatePreview,
Stage: FeatureStageGeneralAvailability,
FrontendOnly: false,
Owner: grafanaAlertingSquad,
RequiresRestart: true,
Enabled: true,
Expression: "true", // enabled by default
Created: time.Date(2023, time.August, 15, 12, 0, 0, 0, time.UTC),
},
{
pkg/services/featuremgmt/service.go
+7 -6
View File
@@ -1,7 +1,6 @@
package featuremgmt
import (
"fmt"
"os"
"path/filepath"
@@ -28,6 +27,8 @@ func ProvideManagerService(cfg *setting.Cfg, licensing licensing.Licensing) (*Fe
licensing: licensing,
flags: make(map[string]*FeatureFlag, 30),
enabled: make(map[string]bool),
startup: make(map[string]bool),
warnings: make(map[string]string),
allowEditing: cfg.FeatureManagement.AllowEditing && cfg.FeatureManagement.UpdateWebhook != "",
log: log.New("featuremgmt"),
}
@@ -41,21 +42,21 @@ func ProvideManagerService(cfg *setting.Cfg, licensing licensing.Licensing) (*Fe
return mgmt, err
}
for key, val := range flags {
flag, ok := mgmt.flags[key]
_, ok := mgmt.flags[key]
if !ok {
switch key {
// renamed the flag so it supports more panels
case "autoMigrateGraphPanels":
flag = mgmt.flags[FlagAutoMigrateOldPanels]
key = FlagAutoMigrateOldPanels
default:
flag = &FeatureFlag{
mgmt.flags[key] = &FeatureFlag{
Name: key,
Stage: FeatureStageUnknown,
}
mgmt.flags[key] = flag
mgmt.warnings[key] = "unknown flag in config"
}
}
flag.Expression = fmt.Sprintf("%t", val) // true | false
mgmt.startup[key] = val
}
// Load config settings
pkg/services/featuremgmt/toggles_gen.csv
+1 -1
View File
@@ -95,7 +95,7 @@ permissionsFilterRemoveSubquery,experimental,@grafana/backend-platform,2023-08-0
prometheusConfigOverhaulAuth,GA,@grafana/observability-metrics,2023-07-21,false,false,false,false
configurableSchedulerTick,experimental,@grafana/alerting-squad,2023-07-26,false,false,true,false
influxdbSqlSupport,GA,@grafana/observability-metrics,2023-08-02,false,false,true,false
alertingNoDataErrorExecution,privatePreview,@grafana/alerting-squad,2023-08-15,false,false,true,false
alertingNoDataErrorExecution,GA,@grafana/alerting-squad,2023-08-15,false,false,true,false
angularDeprecationUI,experimental,@grafana/plugins-platform-backend,2023-08-29,false,false,false,true
dashgpt,preview,@grafana/dashboards-squad,2023-11-17,false,false,false,true
reportingRetries,preview,@grafana/sharing-squad,2023-08-31,false,false,true,false
1 Name Stage Owner Created requiresDevMode RequiresLicense RequiresRestart FrontendOnly
95 prometheusConfigOverhaulAuth GA @grafana/observability-metrics 2023-07-21 false false false false
96 configurableSchedulerTick experimental @grafana/alerting-squad 2023-07-26 false false true false
97 influxdbSqlSupport GA @grafana/observability-metrics 2023-08-02 false false true false
98 alertingNoDataErrorExecution privatePreview GA @grafana/alerting-squad 2023-08-15 false false true false
99 angularDeprecationUI experimental @grafana/plugins-platform-backend 2023-08-29 false false false true
100 dashgpt preview @grafana/dashboards-squad 2023-11-17 false false false true
101 reportingRetries preview @grafana/sharing-squad 2023-08-31 false false true false