FeatureFlags: Use interface rather than manager (#80000)

This commit is contained in:
Ryan McKinley
2024-01-09 10:38:06 -08:00
committed by GitHub
parent e550829dae
commit 1caaa56de0
48 changed files with 125 additions and 143 deletions
+3 -3
View File
@@ -41,7 +41,7 @@ var SharedWithMeFolderPermission = accesscontrol.Permission{
}
func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService,
accessControl accesscontrol.AccessControl, features *featuremgmt.FeatureManager) (*Service, error) {
accessControl accesscontrol.AccessControl, features featuremgmt.FeatureToggles) (*Service, error) {
service := ProvideOSSService(cfg, database.ProvideService(db), cache, features)
api.NewAccessControlAPI(routeRegister, accessControl, service, features).RegisterAPIEndpoints()
@@ -62,7 +62,7 @@ func ProvideService(cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegis
return service, nil
}
func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheService, features *featuremgmt.FeatureManager) *Service {
func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheService, features featuremgmt.FeatureToggles) *Service {
s := &Service{
cfg: cfg,
store: store,
@@ -93,7 +93,7 @@ type Service struct {
cache *localcache.CacheService
registrations accesscontrol.RegistrationList
roles map[string]*accesscontrol.RoleDTO
features *featuremgmt.FeatureManager
features featuremgmt.FeatureToggles
}
func (s *Service) GetUsageStats(_ context.Context) map[string]any {
+2 -2
View File
@@ -15,7 +15,7 @@ import (
)
func NewAccessControlAPI(router routing.RouteRegister, accesscontrol ac.AccessControl, service ac.Service,
features *featuremgmt.FeatureManager) *AccessControlAPI {
features featuremgmt.FeatureToggles) *AccessControlAPI {
return &AccessControlAPI{
RouteRegister: router,
Service: service,
@@ -28,7 +28,7 @@ type AccessControlAPI struct {
Service ac.Service
AccessControl ac.AccessControl
RouteRegister routing.RouteRegister
features *featuremgmt.FeatureManager
features featuremgmt.FeatureToggles
}
func (api *AccessControlAPI) RegisterAPIEndpoints() {
+2 -2
View File
@@ -19,7 +19,7 @@ var _ authn.HookClient = new(Session)
var _ authn.ContextAwareClient = new(Session)
func ProvideSession(cfg *setting.Cfg, sessionService auth.UserTokenService,
features *featuremgmt.FeatureManager) *Session {
features featuremgmt.FeatureToggles) *Session {
return &Session{
cfg: cfg,
features: features,
@@ -30,7 +30,7 @@ func ProvideSession(cfg *setting.Cfg, sessionService auth.UserTokenService,
type Session struct {
cfg *setting.Cfg
features *featuremgmt.FeatureManager
features featuremgmt.FeatureToggles
sessionService auth.UserTokenService
log log.Logger
}
+1 -1
View File
@@ -64,7 +64,7 @@ func TestSession_Authenticate(t *testing.T) {
type fields struct {
sessionService auth.UserTokenService
features *featuremgmt.FeatureManager
features featuremgmt.FeatureToggles
}
type args struct {
r *authn.Request
@@ -25,7 +25,7 @@ import (
"github.com/grafana/grafana/pkg/web"
)
func ProvideService(cfg *setting.Cfg, tracer tracing.Tracer, features *featuremgmt.FeatureManager, authnService authn.Service,
func ProvideService(cfg *setting.Cfg, tracer tracing.Tracer, features featuremgmt.FeatureToggles, authnService authn.Service,
) *ContextHandler {
return &ContextHandler{
Cfg: cfg,
@@ -39,7 +39,7 @@ func ProvideService(cfg *setting.Cfg, tracer tracing.Tracer, features *featuremg
type ContextHandler struct {
Cfg *setting.Cfg
tracer tracing.Tracer
features *featuremgmt.FeatureManager
features featuremgmt.FeatureToggles
authnService authn.Service
}
+5 -1
View File
@@ -174,10 +174,14 @@ func (fm *FeatureManager) LookupFlag(name string) (FeatureFlag, bool) {
// ############# Test Functions #############
func WithFeatures(spec ...any) FeatureToggles {
return WithManager(spec...)
}
// WithFeatures is used to define feature toggles for testing.
// The arguments are a list of strings that are optionally followed by a boolean value for example:
// WithFeatures([]any{"my_feature", "other_feature"}) or WithFeatures([]any{"my_feature", true})
func WithFeatures(spec ...any) *FeatureManager {
func WithManager(spec ...any) *FeatureManager {
count := len(spec)
features := make(map[string]*FeatureFlag, count)
enabled := make(map[string]bool, count)
+2 -2
View File
@@ -9,7 +9,7 @@ import (
func TestFeatureManager(t *testing.T) {
t.Run("check testing stubs", func(t *testing.T) {
ft := WithFeatures("a", "b", "c")
ft := WithManager("a", "b", "c")
require.True(t, ft.IsEnabledGlobally("a"))
require.True(t, ft.IsEnabledGlobally("b"))
require.True(t, ft.IsEnabledGlobally("c"))
@@ -18,7 +18,7 @@ func TestFeatureManager(t *testing.T) {
require.Equal(t, map[string]bool{"a": true, "b": true, "c": true}, ft.GetEnabled(context.Background()))
// Explicit values
ft = WithFeatures("a", true, "b", false)
ft = WithManager("a", true, "b", false)
require.True(t, ft.IsEnabledGlobally("a"))
require.False(t, ft.IsEnabledGlobally("b"))
require.Equal(t, map[string]bool{"a": true}, ft.GetEnabled(context.Background()))
+4
View File
@@ -18,6 +18,10 @@ type FeatureToggles interface {
// Use of global feature flags should be limited and careful as they require
// a full server restart for a change to take place.
IsEnabledGlobally(flag string) bool
// Get the enabled flags -- this *may* also include disabled flags (with value false)
// but it is guaranteed to have the enabled ones listed
GetEnabled(ctx context.Context) map[string]bool
}
// FeatureFlagStage indicates the quality level
+1 -1
View File
@@ -8,7 +8,7 @@ import (
)
func TestFeatureUsageStats(t *testing.T) {
featureManagerWithAllFeatures := WithFeatures(
featureManagerWithAllFeatures := WithManager(
"database_metrics",
"live-config",
"UPPER_SNAKE_CASE",
@@ -587,7 +587,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
testCases := []struct {
service *Service
featuresFlag *featuremgmt.FeatureManager
featuresFlag featuremgmt.FeatureToggles
prefix string
depth int
forceDelete bool
+2 -2
View File
@@ -33,7 +33,7 @@ type ServiceImpl struct {
pluginStore pluginstore.Store
pluginSettings pluginsettings.Service
starService star.Service
features *featuremgmt.FeatureManager
features featuremgmt.FeatureToggles
dashboardService dashboards.DashboardService
accesscontrolService ac.Service
kvStore kvstore.KVStore
@@ -52,7 +52,7 @@ type NavigationAppConfig struct {
Icon string
}
func ProvideService(cfg *setting.Cfg, accessControl ac.AccessControl, pluginStore pluginstore.Store, pluginSettings pluginsettings.Service, starService star.Service, features *featuremgmt.FeatureManager, dashboardService dashboards.DashboardService, accesscontrolService ac.Service, kvStore kvstore.KVStore, apiKeyService apikey.Service, license licensing.Licensing) navtree.Service {
func ProvideService(cfg *setting.Cfg, accessControl ac.AccessControl, pluginStore pluginstore.Store, pluginSettings pluginsettings.Service, starService star.Service, features featuremgmt.FeatureToggles, dashboardService dashboards.DashboardService, accesscontrolService ac.Service, kvStore kvstore.KVStore, apiKeyService apikey.Service, license licensing.Licensing) navtree.Service {
service := &ServiceImpl{
cfg: cfg,
log: log.New("navtree service"),
@@ -27,7 +27,7 @@ func NewCachingMiddleware(cachingService caching.CachingService) plugins.ClientM
// NewCachingMiddlewareWithFeatureManager creates a new plugins.ClientMiddleware that will
// attempt to read and write query results to the cache with a feature manager
func NewCachingMiddlewareWithFeatureManager(cachingService caching.CachingService, features *featuremgmt.FeatureManager) plugins.ClientMiddleware {
func NewCachingMiddlewareWithFeatureManager(cachingService caching.CachingService, features featuremgmt.FeatureToggles) plugins.ClientMiddleware {
log := log.New("caching_middleware")
if err := prometheus.Register(QueryCachingRequestHistogram); err != nil {
log.Error("Error registering prometheus collector 'QueryRequestHistogram'", "error", err)
@@ -49,7 +49,7 @@ type CachingMiddleware struct {
next plugins.Client
caching caching.CachingService
log log.Logger
features *featuremgmt.FeatureManager
features featuremgmt.FeatureToggles
}
// QueryData receives a data request and attempts to access results already stored in the cache for that request.
@@ -9,7 +9,7 @@ import (
"github.com/grafana/grafana/pkg/setting"
)
func ProvideConfig(settingProvider setting.Provider, grafanaCfg *setting.Cfg, features *featuremgmt.FeatureManager) (*pCfg.Cfg, error) {
func ProvideConfig(settingProvider setting.Provider, grafanaCfg *setting.Cfg, features featuremgmt.FeatureToggles) (*pCfg.Cfg, error) {
plugins := settingProvider.Section("plugins")
allowedUnsigned := grafanaCfg.PluginsAllowUnsigned
if len(plugins.KeyValue("allow_loading_unsigned_plugins").Value()) > 0 {
@@ -507,7 +507,7 @@ func TestLoader_Load_ExternalRegistration(t *testing.T) {
t.Run("Load a plugin with oauth client registration", func(t *testing.T) {
cfg := &config.Cfg{
Features: fakes.NewFakeFeatureToggles(featuremgmt.FlagExternalServiceAuth),
Features: featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAuth),
PluginsAllowUnsigned: []string{"grafana-test-datasource"},
}
pluginPaths := []string{filepath.Join(testDataDir(t), "oauth-external-registration")}
@@ -608,7 +608,7 @@ func TestLoader_Load_ExternalRegistration(t *testing.T) {
t.Run("Load a plugin with service account registration", func(t *testing.T) {
cfg := &config.Cfg{
Features: fakes.NewFakeFeatureToggles(featuremgmt.FlagExternalServiceAuth),
Features: featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAuth),
PluginsAllowUnsigned: []string{"grafana-test-datasource"},
}
pluginPaths := []string{filepath.Join(testDataDir(t), "external-registration")}
@@ -3,12 +3,13 @@ package pipeline
import (
"testing"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/config"
"github.com/grafana/grafana/pkg/plugins/log"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/require"
)
func TestSkipPlugins(t *testing.T) {
+3 -3
View File
@@ -27,7 +27,7 @@ type Api struct {
accessControl accesscontrol.AccessControl
cfg *setting.Cfg
features *featuremgmt.FeatureManager
features featuremgmt.FeatureToggles
log log.Logger
routeRegister routing.RouteRegister
}
@@ -36,7 +36,7 @@ func ProvideApi(
pd publicdashboards.Service,
rr routing.RouteRegister,
ac accesscontrol.AccessControl,
features *featuremgmt.FeatureManager,
features featuremgmt.FeatureToggles,
md publicdashboards.Middleware,
cfg *setting.Cfg,
) *Api {
@@ -297,7 +297,7 @@ func (api *Api) DeletePublicDashboard(c *contextmodel.ReqContext) response.Respo
}
// Copied from pkg/api/metrics.go
func toJsonStreamingResponse(ctx context.Context, features *featuremgmt.FeatureManager, qdr *backend.QueryDataResponse) response.Response {
func toJsonStreamingResponse(ctx context.Context, features featuremgmt.FeatureToggles, qdr *backend.QueryDataResponse) response.Response {
statusWhenError := http.StatusBadRequest
if features.IsEnabled(ctx, featuremgmt.FlagDatasourceQueryMultiStatus) {
statusWhenError = http.StatusMultiStatus
@@ -155,6 +155,10 @@ func (f fakeFeatureToggles) IsEnabled(ctx context.Context, feature string) bool
return f.returnValue
}
func (f fakeFeatureToggles) GetEnabled(ctx context.Context) map[string]bool {
return map[string]bool{}
}
// Fake grpc secrets plugin impl
type fakeGRPCSecretsPlugin struct {
kv map[Key]string
+1 -1
View File
@@ -23,7 +23,7 @@ func SetupDisabledTestService(tb testing.TB, store secrets.Store) *SecretsServic
return setupTestService(tb, store, featuremgmt.WithFeatures(featuremgmt.FlagDisableEnvelopeEncryption))
}
func setupTestService(tb testing.TB, store secrets.Store, features *featuremgmt.FeatureManager) *SecretsService {
func setupTestService(tb testing.TB, store secrets.Store, features featuremgmt.FeatureToggles) *SecretsService {
tb.Helper()
defaultKey := "SdlklWklckeLS"
if len(setting.SecretKey) > 0 {
@@ -26,7 +26,7 @@ import (
type ExtSvcAccountsService struct {
acSvc ac.Service
features *featuremgmt.FeatureManager
features featuremgmt.FeatureToggles
logger log.Logger
metrics *metrics
saSvc sa.Service
@@ -4,6 +4,9 @@ import (
"context"
"testing"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/tracing"
@@ -18,8 +21,6 @@ import (
sa "github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)
type TestEnv struct {
@@ -195,7 +195,7 @@ func TestIntegration_DashboardPermissionFilter(t *testing.T) {
usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}
for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
m := features.GetEnabled(context.Background())
keys := make([]string, 0, len(m))
for k := range m {
@@ -394,7 +394,7 @@ func TestIntegration_DashboardPermissionFilter_WithSelfContainedPermissions(t *t
usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.signedInUserPermissions)}}
for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
m := features.GetEnabled(context.Background())
keys := make([]string, 0, len(m))
for k := range m {
@@ -545,7 +545,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
})
usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(tc.permissions)}}
for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} {
for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} {
m := features.GetEnabled(context.Background())
keys := make([]string, 0, len(m))
for k := range m {
@@ -716,7 +716,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
}),
},
}
for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} {
for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} {
m := features.GetEnabled(context.Background())
keys := make([]string, 0, len(m))
for k := range m {
@@ -34,7 +34,7 @@ type SSOSettingsService struct {
}
func ProvideService(cfg *setting.Cfg, sqlStore db.DB, ac ac.AccessControl,
routeRegister routing.RouteRegister, features *featuremgmt.FeatureManager,
routeRegister routing.RouteRegister, features featuremgmt.FeatureToggles,
secrets secrets.Service) *SSOSettingsService {
strategies := []ssosettings.FallbackStrategy{
strategies.NewOAuthStrategy(cfg),