Feature Toggles: Remove use of boolPtr in FeatureFlag struct (#79550)

* remove bool ptr and update docs

* fix silly thing

* merge main

* maybe this time

---------

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
This commit is contained in:
Michael Mandrus
2023-12-18 13:55:21 -05:00
committed by GitHub
parent 9c3c49e48e
commit 456939bac4
5 changed files with 52 additions and 70 deletions

View File

@@ -110,8 +110,7 @@ func isFeatureWriteable(flag featuremgmt.FeatureFlag, readOnlyCfg map[string]str
if flag.Name == featuremgmt.FlagFeatureToggleAdminPage {
return false
}
allowSelfServe := flag.AllowSelfServe != nil && *flag.AllowSelfServe
return flag.Stage == featuremgmt.FeatureStageGeneralAvailability && allowSelfServe || flag.Stage == featuremgmt.FeatureStageDeprecated
return (flag.Stage == featuremgmt.FeatureStageGeneralAvailability || flag.Stage == featuremgmt.FeatureStageDeprecated) && flag.AllowSelfServe
}
// isFeatureEditingAllowed checks if the backend is properly configured to allow feature toggle changes from the UI

View File

@@ -20,15 +20,6 @@ import (
"github.com/stretchr/testify/require"
)
func boolPtr(b bool) *bool {
return &b
}
var (
truePtr = boolPtr(true)
falsePtr = boolPtr(false)
)
func TestGetFeatureToggles(t *testing.T) {
readPermissions := []accesscontrol.Permission{{Action: accesscontrol.ActionFeatureManagementRead}}
@@ -118,19 +109,19 @@ func TestGetFeatureToggles(t *testing.T) {
}, {
Name: "toggle4",
Stage: featuremgmt.FeatureStagePublicPreview,
AllowSelfServe: truePtr,
AllowSelfServe: true,
}, {
Name: "toggle5",
Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: truePtr,
AllowSelfServe: true,
}, {
Name: "toggle6",
Stage: featuremgmt.FeatureStageDeprecated,
AllowSelfServe: truePtr,
AllowSelfServe: true,
}, {
Name: "toggle7",
Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: falsePtr,
AllowSelfServe: false,
},
}
@@ -324,12 +315,12 @@ func TestSetFeatureToggles(t *testing.T) {
Name: "toggle4",
Enabled: false,
Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: truePtr,
AllowSelfServe: true,
}, {
Name: "toggle5",
Enabled: false,
Stage: featuremgmt.FeatureStageDeprecated,
AllowSelfServe: truePtr,
AllowSelfServe: true,
},
}

View File

@@ -107,30 +107,32 @@ func (s *FeatureFlagStage) UnmarshalJSON(b []byte) error {
}
type FeatureFlag struct {
// Required properties
Name string `json:"name" yaml:"name"` // Unique name
Description string `json:"description"`
Stage FeatureFlagStage `json:"stage,omitempty"`
DocsURL string `json:"docsURL,omitempty"`
Created time.Time `json:"created,omitempty"` // when the flag was introduced
Owner codeowner `json:"-"` // Owner person or team that owns this feature flag
// Owner person or team that owns this feature flag
Owner codeowner `json:"-"`
// Recommended properties - control behavior of the feature toggle management page in the UI
AllowSelfServe bool `json:"allowSelfServe,omitempty"` // allow users with the right privileges to toggle this from the UI (GeneralAvailability and Deprecated toggles only)
HideFromAdminPage bool `json:"hideFromAdminPage,omitempty"` // GA, Deprecated, and PublicPreview toggles only: don't display this feature in the UI; if this is a GA toggle, add a comment with the reasoning
// CEL-GO expression. Using the value "true" will mean this is on by default
Expression string `json:"expression,omitempty"`
// Special behavior flags
// Special behavior properties
RequiresDevMode bool `json:"requiresDevMode,omitempty"` // can not be enabled in production
// This flag is currently unused.
RequiresRestart bool `json:"requiresRestart,omitempty"` // The server must be initialized with the value
RequiresLicense bool `json:"requiresLicense,omitempty"` // Must be enabled in the license
FrontendOnly bool `json:"frontend,omitempty"` // change is only seen in the frontend
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
RequiresLicense bool `json:"requiresLicense,omitempty"` // Must be enabled in the license
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
}
type UpdateFeatureTogglesCommand struct {

View File

@@ -11,8 +11,6 @@ import (
)
var (
falsePtr = boolPtr(false)
truePtr = boolPtr(true)
// Register each toggle here
standardFeatureFlags = []FeatureFlag{
{
@@ -21,7 +19,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Owner: grafanaAsCodeSquad,
HideFromAdminPage: true,
AllowSelfServe: falsePtr,
AllowSelfServe: false,
Created: time.Date(2022, time.May, 24, 12, 0, 0, 0, time.UTC),
},
{
@@ -54,7 +52,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Owner: grafanaSharingSquad,
Expression: "true", // enabled by default
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2022, time.April, 7, 12, 0, 0, 0, time.UTC),
},
{
@@ -79,7 +77,7 @@ var (
Description: "Highlight Grafana Enterprise features",
Stage: FeatureStageGeneralAvailability,
Owner: grafanaAsCodeSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2022, time.February, 3, 12, 0, 0, 0, time.UTC),
},
{
@@ -110,7 +108,7 @@ var (
Owner: grafanaExploreSquad,
Expression: "true", // enabled by default
FrontendOnly: true,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.November, 3, 12, 0, 0, 0, time.UTC),
},
{
@@ -160,7 +158,7 @@ var (
Stage: FeatureStageGeneralAvailability,
FrontendOnly: true,
Owner: grafanaDatavizSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.November, 3, 12, 0, 0, 0, time.UTC),
},
{
@@ -192,7 +190,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Expression: "true", // turned on by default
Owner: grafanaPluginsPlatformSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2022, time.June, 1, 12, 0, 0, 0, time.UTC),
},
{
@@ -236,7 +234,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Expression: "true", // enabled by default
Owner: awsDatasourcesSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2022, time.November, 28, 12, 0, 0, 0, time.UTC),
},
{
@@ -245,7 +243,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Expression: "true", // enabled by default
Owner: awsDatasourcesSquad,
AllowSelfServe: falsePtr,
AllowSelfServe: false,
Created: time.Date(2022, time.August, 27, 12, 0, 0, 0, time.UTC),
},
{
@@ -255,7 +253,7 @@ var (
Expression: "true", // enabled by default
FrontendOnly: true,
Owner: awsDatasourcesSquad,
AllowSelfServe: falsePtr,
AllowSelfServe: false,
Created: time.Date(2022, time.August, 27, 12, 0, 0, 0, time.UTC),
},
{
@@ -264,7 +262,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Expression: "true", // enabled by default
Owner: awsDatasourcesSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.September, 25, 12, 0, 0, 0, time.UTC),
},
{
@@ -303,7 +301,7 @@ var (
Owner: grafanaFrontendPlatformSquad,
FrontendOnly: true,
Expression: "true", // enabled by default
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.July, 24, 12, 0, 0, 0, time.UTC),
},
{
@@ -313,7 +311,7 @@ var (
FrontendOnly: true,
Expression: "true", // enabled by default
Owner: grafanaDashboardsSquad,
AllowSelfServe: falsePtr,
AllowSelfServe: false,
HideFromAdminPage: true,
Created: time.Date(2023, time.March, 28, 12, 0, 0, 0, time.UTC),
},
@@ -322,7 +320,7 @@ var (
Description: "Disable Prometheus exemplar sampling",
Stage: FeatureStageGeneralAvailability,
Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2022, time.December, 19, 12, 0, 0, 0, time.UTC),
},
{
@@ -356,7 +354,7 @@ var (
FrontendOnly: true,
Owner: grafanaObservabilityLogsSquad,
Expression: "true", // turned on by default
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.January, 27, 12, 0, 0, 0, time.UTC),
},
{
@@ -366,7 +364,7 @@ var (
FrontendOnly: true,
Owner: grafanaObservabilityLogsSquad,
Expression: "true", // turned on by default
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.February, 9, 12, 0, 0, 0, time.UTC),
},
{
@@ -391,7 +389,7 @@ var (
Stage: FeatureStageGeneralAvailability,
FrontendOnly: true,
Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.March, 7, 12, 0, 0, 0, time.UTC),
},
{
@@ -401,7 +399,7 @@ var (
FrontendOnly: true,
Owner: grafanaObservabilityMetricsSquad,
Expression: "true", // enabled by default
AllowSelfServe: falsePtr,
AllowSelfServe: false,
Created: time.Date(2023, time.March, 15, 12, 0, 0, 0, time.UTC),
},
{
@@ -417,7 +415,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Expression: "true",
Owner: identityAccessTeam,
AllowSelfServe: falsePtr,
AllowSelfServe: false,
Created: time.Date(2023, time.March, 23, 12, 0, 0, 0, time.UTC),
},
{
@@ -426,7 +424,7 @@ var (
Expression: "true",
Stage: FeatureStageGeneralAvailability,
Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.March, 29, 12, 0, 0, 0, time.UTC),
},
{
@@ -435,7 +433,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Expression: "true",
Owner: grafanaObservabilityLogsSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.April, 13, 12, 0, 0, 0, time.UTC),
},
{
@@ -452,7 +450,7 @@ var (
FrontendOnly: true,
Expression: "true",
Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.April, 24, 12, 0, 0, 0, time.UTC),
},
{
@@ -521,7 +519,7 @@ var (
Owner: grafanaOperatorExperienceSquad,
RequiresRestart: true,
Expression: "true", // enabled by default
AllowSelfServe: falsePtr,
AllowSelfServe: false,
Created: time.Date(2023, time.April, 12, 12, 0, 0, 0, time.UTC),
},
{
@@ -530,7 +528,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Owner: grafanaObservabilityLogsSquad,
Expression: "true", // enabled by default
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.April, 14, 12, 0, 0, 0, time.UTC),
},
{
@@ -540,7 +538,7 @@ var (
FrontendOnly: true,
Expression: "true", // enabled by default
Owner: grafanaDashboardsSquad,
AllowSelfServe: falsePtr,
AllowSelfServe: false,
HideFromAdminPage: true,
Created: time.Date(2023, time.April, 14, 12, 0, 0, 0, time.UTC),
},
@@ -624,7 +622,7 @@ var (
FrontendOnly: true,
Expression: "true", // enabled by default
Owner: awsDatasourcesSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.June, 12, 12, 0, 0, 0, time.UTC),
},
{
@@ -641,7 +639,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Expression: "true",
Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: falsePtr,
AllowSelfServe: false,
Created: time.Date(2023, time.June, 14, 12, 0, 0, 0, time.UTC),
},
{
@@ -690,7 +688,7 @@ var (
FrontendOnly: true,
Expression: "true", // enabled by default
Owner: grafanaObservabilityMetricsSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.July, 12, 12, 0, 0, 0, time.UTC),
},
{
@@ -764,7 +762,6 @@ var (
Stage: FeatureStageGeneralAvailability,
FrontendOnly: false,
Expression: "true", // enabled by default
AllowSelfServe: falsePtr,
Owner: identityAccessTeam,
RequiresRestart: true,
HideFromAdminPage: true, // This is internal work to speed up dashboard search, and is not ready for wider use
@@ -799,7 +796,7 @@ var (
Owner: grafanaObservabilityMetricsSquad,
Stage: FeatureStageGeneralAvailability,
Expression: "true", // on by default
AllowSelfServe: falsePtr,
AllowSelfServe: false,
Created: time.Date(2023, time.July, 21, 12, 0, 0, 0, time.UTC),
},
{
@@ -903,7 +900,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Owner: grafanaAlertingSquad,
Expression: "true", // enabled by default
AllowSelfServe: falsePtr,
AllowSelfServe: false,
HideFromAdminPage: true, // This is moving away from being a feature toggle.
Created: time.Date(2023, time.September, 14, 12, 0, 0, 0, time.UTC),
},
@@ -944,7 +941,7 @@ var (
Stage: FeatureStageGeneralAvailability,
Expression: "true", // enabled by default
Owner: awsDatasourcesSquad,
AllowSelfServe: truePtr,
AllowSelfServe: true,
Created: time.Date(2023, time.September, 27, 12, 0, 0, 0, time.UTC),
},
{
@@ -1260,7 +1257,7 @@ var (
FrontendOnly: true,
Owner: identityAccessTeam,
Created: time.Date(2023, time.November, 29, 12, 0, 0, 0, time.UTC),
AllowSelfServe: falsePtr,
AllowSelfServe: false,
Expression: "true", // enabled by default
},
{
@@ -1275,7 +1272,3 @@ var (
},
}
)
func boolPtr(b bool) *bool {
return &b
}

View File

@@ -42,10 +42,7 @@ func TestFeatureToggleFiles(t *testing.T) {
if flag.Name != strings.TrimSpace(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 {
if flag.AllowSelfServe && flag.Stage != FeatureStageGeneralAvailability {
t.Errorf("only allow self-serving GA toggles")
}
if flag.Created.Year() < 2021 {