Service accounts: Grafana service accounts are enabled by default (#51402)

* Remove feature flag for service accounts

* Fix failing tests and remove remaining usage

* Fix failing tests and remove remaining usage

packages/grafana-data/src/types/featureToggles.gen.ts

@@ -18,7 +18,6 @@ export interface FeatureToggles {
 
   trimDefaults?: boolean;
   disableEnvelopeEncryption?: boolean;
-  serviceAccounts?: boolean;
   database_metrics?: boolean;
   dashboardPreviews?: boolean;
   dashboardPreviewsAdmin?: boolean;

pkg/api/index.go

@@ -151,9 +151,6 @@ func (hs *HTTPServer) getAppLinks(c *models.ReqContext) ([]*dtos.NavLink, error)
 }
 
 func enableServiceAccount(hs *HTTPServer, c *models.ReqContext) bool {
-	if !hs.Features.IsEnabled(featuremgmt.FlagServiceAccounts) {
-		return false
-	}
 	hasAccess := ac.HasAccess(hs.AccessControl, c)
 	return hasAccess(ac.ReqOrgAdmin, serviceAccountAccessEvaluator)
 }
@@ -298,7 +295,6 @@ func (hs *HTTPServer) getNavTree(c *models.ReqContext, hasEditPerm bool, prefs *
 			Url:         hs.Cfg.AppSubURL + "/org/apikeys",
 		})
 	}
-	// needs both feature flag and migration to be able to show service accounts
 	if enableServiceAccount(hs, c) {
 		configNodes = append(configNodes, &dtos.NavLink{
 			Text:        "Service accounts",

pkg/services/featuremgmt/registry.go

@@ -19,11 +19,6 @@ var (
 			Description: "Disable envelope encryption (emergency only)",
 			State:       FeatureStateStable,
 		},
-		{
-			Name:        "serviceAccounts",
-			Description: "support service accounts",
-			State:       FeatureStateBeta,
-		},
 		{
 			Name:        "database_metrics",
 			Description: "Add prometheus metrics for database tables",

pkg/services/featuremgmt/toggles_gen.go

@@ -15,10 +15,6 @@ const (
 	// Disable envelope encryption (emergency only)
 	FlagDisableEnvelopeEncryption = "disableEnvelopeEncryption"
 
-	// FlagServiceAccounts
-	// support service accounts
-	FlagServiceAccounts = "serviceAccounts"
-
 	// FlagDatabaseMetrics
 	// Add prometheus metrics for database tables
 	FlagDatabaseMetrics = "database_metrics"

pkg/services/featuremgmt/usage_stats_test.go

@@ -11,7 +11,6 @@ func TestFeatureUsageStats(t *testing.T) {
 	featureManagerWithAllFeatures := WithFeatures(
 		"trimDefaults",
 		"httpclientprovider_azure_auth",
-		"serviceAccounts",
 		"database_metrics",
 		"dashboardPreviews",
 		"live-config",
@@ -23,7 +22,6 @@ func TestFeatureUsageStats(t *testing.T) {
 	require.Equal(t, map[string]interface{}{
 		"stats.features.trim_defaults.count":                 1,
 		"stats.features.httpclientprovider_azure_auth.count": 1,
-		"stats.features.service_accounts.count":              1,
 		"stats.features.database_metrics.count":              1,
 		"stats.features.dashboard_previews.count":            1,
 		"stats.features.live_config.count":                   1,

pkg/services/serviceaccounts/api/api.go

@@ -12,7 +12,6 @@ import (
 	"github.com/grafana/grafana/pkg/middleware"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
 	"github.com/grafana/grafana/pkg/setting"
@@ -46,13 +45,7 @@ func NewServiceAccountsAPI(
 	}
 }
 
-func (api *ServiceAccountsAPI) RegisterAPIEndpoints(
+func (api *ServiceAccountsAPI) RegisterAPIEndpoints() {
-	features featuremgmt.FeatureToggles,
-) {
-	if !features.IsEnabled(featuremgmt.FlagServiceAccounts) {
-		return
-	}
-
 	auth := accesscontrol.Middleware(api.accesscontrol)
 	api.RouterRegister.Group("/api/serviceaccounts", func(serviceAccountsRoute routing.RouteRegister) {
 		serviceAccountsRoute.Get("/search", auth(middleware.ReqOrgAdmin,

pkg/services/serviceaccounts/api/api_test.go

@@ -17,7 +17,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
@@ -226,7 +225,7 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock,
 	acmock *accesscontrolmock.Mock,
 	sqlStore *sqlstore.SQLStore, saStore serviceaccounts.Store) (*web.Mux, *ServiceAccountsAPI) {
 	a := NewServiceAccountsAPI(setting.NewCfg(), svc, acmock, routerRegister, saStore)
-	a.RegisterAPIEndpoints(featuremgmt.WithFeatures(featuremgmt.FlagServiceAccounts))
+	a.RegisterAPIEndpoints()
 
 	a.cfg.ApiKeyMaxSecondsToLive = -1 // disable api key expiration
 

pkg/services/serviceaccounts/manager/service.go

@@ -8,7 +8,6 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/usagestats"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/api"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
@@ -21,14 +20,12 @@ var (
 )
 
 type ServiceAccountsService struct {
-	store    serviceaccounts.Store
+	store serviceaccounts.Store
-	features featuremgmt.FeatureToggles
+	log   log.Logger
-	log      log.Logger
 }
 
 func ProvideServiceAccountsService(
 	cfg *setting.Cfg,
-	features featuremgmt.FeatureToggles,
 	store *sqlstore.SQLStore,
 	kvStore kvstore.KVStore,
 	ac accesscontrol.AccessControl,
@@ -36,45 +33,30 @@ func ProvideServiceAccountsService(
 	usageStats usagestats.Service,
 ) (*ServiceAccountsService, error) {
 	s := &ServiceAccountsService{
-		features: features,
+		store: database.NewServiceAccountsStore(store, kvStore),
-		store:    database.NewServiceAccountsStore(store, kvStore),
+		log:   log.New("serviceaccounts"),
-		log:      log.New("serviceaccounts"),
 	}
 
-	if features.IsEnabled(featuremgmt.FlagServiceAccounts) {
+	if err := RegisterRoles(ac); err != nil {
-		if err := RegisterRoles(ac); err != nil {
+		s.log.Error("Failed to register roles", "error", err)
-			s.log.Error("Failed to register roles", "error", err)
-		}
-
-		usageStats.RegisterMetricsFunc(s.store.GetUsageMetrics)
 	}
 
+	usageStats.RegisterMetricsFunc(s.store.GetUsageMetrics)
+
 	serviceaccountsAPI := api.NewServiceAccountsAPI(cfg, s, ac, routeRegister, s.store)
-	serviceaccountsAPI.RegisterAPIEndpoints(features)
+	serviceaccountsAPI.RegisterAPIEndpoints()
 
 	return s, nil
 }
 
 func (sa *ServiceAccountsService) CreateServiceAccount(ctx context.Context, orgID int64, name string) (*serviceaccounts.ServiceAccountDTO, error) {
-	if !sa.features.IsEnabled(featuremgmt.FlagServiceAccounts) {
-		sa.log.Debug(ServiceAccountFeatureToggleNotFound)
-		return nil, nil
-	}
 	return sa.store.CreateServiceAccount(ctx, orgID, name)
 }
 
 func (sa *ServiceAccountsService) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error {
-	if !sa.features.IsEnabled(featuremgmt.FlagServiceAccounts) {
-		sa.log.Debug(ServiceAccountFeatureToggleNotFound)
-		return nil
-	}
 	return sa.store.DeleteServiceAccount(ctx, orgID, serviceAccountID)
 }
 
 func (sa *ServiceAccountsService) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) {
-	if !sa.features.IsEnabled(featuremgmt.FlagServiceAccounts) {
-		sa.log.Debug(ServiceAccountFeatureToggleNotFound)
-		return 0, nil
-	}
 	return sa.store.RetrieveServiceAccountIdByName(ctx, orgID, name)
 }

pkg/services/serviceaccounts/manager/service_test.go

@@ -4,33 +4,17 @@ import (
 	"context"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestProvideServiceAccount_DeleteServiceAccount(t *testing.T) {
-	t.Run("feature toggle present, should call store function", func(t *testing.T) {
+	t.Run("should call store function", func(t *testing.T) {
 		storeMock := &tests.ServiceAccountsStoreMock{Calls: tests.Calls{}}
-		svc := ServiceAccountsService{
+		svc := ServiceAccountsService{store: storeMock}
-			features: featuremgmt.WithFeatures("serviceAccounts", true),
-			store:    storeMock}
 		err := svc.DeleteServiceAccount(context.Background(), 1, 1)
 		require.NoError(t, err)
 		assert.Len(t, storeMock.Calls.DeleteServiceAccount, 1)
 	})
-
-	t.Run("no feature toggle present, should not call store function", func(t *testing.T) {
-		svcMock := &tests.ServiceAccountsStoreMock{Calls: tests.Calls{}}
-		svc := ServiceAccountsService{
-			features: featuremgmt.WithFeatures("serviceAccounts", false),
-			store:    svcMock,
-			log:      log.New("serviceaccounts-manager-test"),
-		}
-		err := svc.DeleteServiceAccount(context.Background(), 1, 1)
-		require.NoError(t, err)
-		assert.Len(t, svcMock.Calls.DeleteServiceAccount, 0)
-	})
 }