public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore: remove CreateUser from sqlstore & replace with userService.CreateUserForTests (#59910)

Browse Source
This commit is contained in:
Kristin Laemmert
2022-12-07 17:03:22 +01:00
committed by GitHub
parent d036225f7b
commit 70fbf47022
44 changed files with 943 additions and 758 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/serviceaccounts/api/api_test.go
+42 -32
View File
@@ -23,6 +23,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
"github.com/grafana/grafana/pkg/services/apikey"
"github.com/grafana/grafana/pkg/services/apikey/apikeyimpl"
"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
"github.com/grafana/grafana/pkg/services/licensing"
@@ -32,6 +33,7 @@ import (
"github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/team/teamimpl"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/userimpl"
@@ -46,14 +48,7 @@ var (
func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) {
store := db.InitTestDB(t)
quotaService := quotatest.New(false, nil)
apiKeyService, err := apikeyimpl.ProvideService(store, store.Cfg, quotaService)
require.NoError(t, err)
kvStore := kvstore.ProvideService(store)
orgService, err := orgimpl.ProvideService(store, setting.NewCfg(), quotaService)
require.NoError(t, err)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, orgService)
svcmock := tests.ServiceAccountMock{}
services := setupTestServices(t, store)
autoAssignOrg := store.Cfg.AutoAssignOrg
store.Cfg.AutoAssignOrg = true
@@ -62,7 +57,7 @@ func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) {
}()
orgCmd := &org.CreateOrgCommand{Name: "Some Test Org"}
_, err = orgService.CreateWithMember(context.Background(), orgCmd)
_, err := services.OrgService.CreateWithMember(context.Background(), orgCmd)
require.Nil(t, err)
type testCreateSATestCase struct {
@@ -167,7 +162,7 @@ func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
serviceAccountRequestScenario(t, http.MethodPost, serviceAccountPath, testUser, func(httpmethod string, endpoint string, usr *tests.TestUser) {
server, api := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, saStore)
server, api := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore)
marshalled, err := json.Marshal(tc.body)
require.NoError(t, err)
@@ -216,12 +211,7 @@ func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) {
// with permissions and without permissions
func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
store := db.InitTestDB(t)
kvStore := kvstore.ProvideService(store)
quotaService := quotatest.New(false, nil)
apiKeyService, err := apikeyimpl.ProvideService(store, store.Cfg, quotaService)
require.NoError(t, err)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
svcmock := tests.ServiceAccountMock{}
services := setupTestServices(t, store)
var requestResponse = func(server *web.Mux, httpMethod, requestpath string) *httptest.ResponseRecorder {
req, err := http.NewRequest(httpMethod, requestpath, nil)
@@ -249,7 +239,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
}
serviceAccountRequestScenario(t, http.MethodDelete, serviceAccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, saStore)
server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), testcase.acmock, store, services.SAStore)
actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, fmt.Sprint(createduser.ID))).Code
require.Equal(t, testcase.expectedCode, actual)
})
@@ -273,7 +263,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
}
serviceAccountRequestScenario(t, http.MethodDelete, serviceAccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, saStore)
server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), testcase.acmock, store, services.SAStore)
actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, createduser.ID)).Code
require.Equal(t, testcase.expectedCode, actual)
})
@@ -326,12 +316,7 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock,
func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) {
store := db.InitTestDB(t)
quotaService := quotatest.New(false, nil)
apiKeyService, err := apikeyimpl.ProvideService(store, store.Cfg, quotaService)
require.NoError(t, err)
kvStore := kvstore.ProvideService(store)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
svcmock := tests.ServiceAccountMock{}
services := setupTestServices(t, store)
type testRetrieveSATestCase struct {
desc string
user *tests.TestUser
@@ -395,7 +380,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) {
createdUser := tests.SetupUserServiceAccount(t, store, *tc.user)
scopeID = int(createdUser.ID)
}
server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, saStore)
server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore)
actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, scopeID))
@@ -420,12 +405,7 @@ func newString(s string) *string {
func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
store := db.InitTestDB(t)
quotaService := quotatest.New(false, nil)
apiKeyService, err := apikeyimpl.ProvideService(store, store.Cfg, quotaService)
require.NoError(t, err)
kvStore := kvstore.ProvideService(store)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
svcmock := tests.ServiceAccountMock{}
services := setupTestServices(t, store)
type testUpdateSATestCase struct {
desc string
user *tests.TestUser
@@ -518,7 +498,7 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
server, saAPI := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, saStore)
server, saAPI := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore)
scopeID := tc.Id
if tc.user != nil {
createdUser := tests.SetupUserServiceAccount(t, store, *tc.user)
@@ -556,3 +536,33 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
})
}
}
type services struct {
OrgService org.Service
UserService user.Service
SAStore serviceaccounts.Store
SAService tests.ServiceAccountMock
APIKeyService apikey.Service
}
func setupTestServices(t *testing.T, db *sqlstore.SQLStore) services {
kvStore := kvstore.ProvideService(db)
quotaService := quotatest.New(false, nil)
apiKeyService, err := apikeyimpl.ProvideService(db, db.Cfg, quotaService)
require.NoError(t, err)
orgService, err := orgimpl.ProvideService(db, setting.NewCfg(), quotaService)
require.NoError(t, err)
userSvc, err := userimpl.ProvideService(db, orgService, db.Cfg, nil, nil, quotaService)
require.NoError(t, err)
saStore := database.ProvideServiceAccountsStore(db, apiKeyService, kvStore, userSvc, orgService)
svcmock := tests.ServiceAccountMock{}
return services{
OrgService: orgService,
UserService: userSvc,
SAStore: saStore,
SAService: svcmock,
APIKeyService: apiKeyService,
}
}
pkg/services/serviceaccounts/api/token_test.go
+7 -21
View File
@@ -18,14 +18,10 @@ import (
"github.com/grafana/grafana/pkg/components/apikeygen"
apikeygenprefix "github.com/grafana/grafana/pkg/components/apikeygenprefixed"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/kvstore"
"github.com/grafana/grafana/pkg/services/accesscontrol"
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/apikey"
"github.com/grafana/grafana/pkg/services/apikey/apikeyimpl"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/web"
@@ -55,12 +51,7 @@ func createTokenforSA(t *testing.T, store serviceaccounts.Store, keyName string,
func TestServiceAccountsAPI_CreateToken(t *testing.T) {
store := db.InitTestDB(t)
quotaService := quotatest.New(false, nil)
apiKeyService, err := apikeyimpl.ProvideService(store, store.Cfg, quotaService)
require.NoError(t, err)
kvStore := kvstore.ProvideService(store)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
svcmock := tests.ServiceAccountMock{}
services := setupTestServices(t, store)
sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true})
type testCreateSAToken struct {
@@ -140,7 +131,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) {
bodyString = string(b)
}
server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, saStore)
server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore)
actual := requestResponse(server, http.MethodPost, endpoint, strings.NewReader(bodyString))
actualCode := actual.Code
@@ -154,7 +145,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) {
assert.Equal(t, tc.body["name"], actualBody["name"])
query := apikey.GetByNameQuery{KeyName: tc.body["name"].(string), OrgId: sa.OrgID}
err = apiKeyService.GetApiKeyByName(context.Background(), &query)
err = services.APIKeyService.GetApiKeyByName(context.Background(), &query)
require.NoError(t, err)
assert.Equal(t, sa.ID, *query.Result.ServiceAccountId)
@@ -174,12 +165,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) {
func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
store := db.InitTestDB(t)
quotaService := quotatest.New(false, nil)
apiKeyService, err := apikeyimpl.ProvideService(store, store.Cfg, quotaService)
require.NoError(t, err)
kvStore := kvstore.ProvideService(store)
svcMock := &tests.ServiceAccountMock{}
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
services := setupTestServices(t, store)
sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true})
type testCreateSAToken struct {
@@ -239,11 +225,11 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
token := createTokenforSA(t, saStore, tc.keyName, sa.OrgID, sa.ID, 1)
token := createTokenforSA(t, services.SAStore, tc.keyName, sa.OrgID, sa.ID, 1)
endpoint := fmt.Sprintf(serviceaccountIDTokensDetailPath, sa.ID, token.Id)
bodyString := ""
server, _ := setupTestServer(t, svcMock, routing.NewRouteRegister(), tc.acmock, store, saStore)
server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store, services.SAStore)
actual := requestResponse(server, http.MethodDelete, endpoint, strings.NewReader(bodyString))
actualCode := actual.Code
@@ -253,7 +239,7 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody)
query := apikey.GetByNameQuery{KeyName: tc.keyName, OrgId: sa.OrgID}
err := apiKeyService.GetApiKeyByName(context.Background(), &query)
err := services.APIKeyService.GetApiKeyByName(context.Background(), &query)
if actualCode == http.StatusOK {
require.Error(t, err)
} else {
pkg/services/serviceaccounts/database/database.go
+4 -3
View File
@@ -30,13 +30,14 @@ type ServiceAccountsStoreImpl struct {
}
func ProvideServiceAccountsStore(store *sqlstore.SQLStore, apiKeyService apikey.Service,
kvStore kvstore.KVStore, orgService org.Service) *ServiceAccountsStoreImpl {
kvStore kvstore.KVStore, userService user.Service, orgService org.Service) *ServiceAccountsStoreImpl {
return &ServiceAccountsStoreImpl{
sqlStore: store,
apiKeyService: apiKeyService,
kvStore: kvStore,
log: log.New("serviceaccounts.store"),
orgService: orgService,
userService: userService,
}
}
@@ -55,7 +56,7 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, org
var newSA *user.User
createErr := s.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) (err error) {
var errUser error
newSA, errUser = s.sqlStore.CreateUser(ctx, user.CreateUserCommand{
newSA, errUser = s.userService.CreateServiceAccount(ctx, &user.CreateUserCommand{
Login: generatedLogin,
OrgID: orgId,
Name: saForm.Name,
@@ -461,7 +462,7 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Co
}
return s.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
newSA, errCreateSA := s.sqlStore.CreateUser(ctx, cmd)
newSA, errCreateSA := s.userService.CreateServiceAccount(ctx, &cmd)
if errCreateSA != nil {
return fmt.Errorf("failed to create service account: %w", errCreateSA)
}
pkg/services/serviceaccounts/database/database_test.go
+4 -1
View File
@@ -18,6 +18,7 @@ import (
"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/userimpl"
"github.com/grafana/grafana/pkg/setting"
)
@@ -118,7 +119,9 @@ func setupTestDatabase(t *testing.T) (*sqlstore.SQLStore, *ServiceAccountsStoreI
kvStore := kvstore.ProvideService(db)
orgService, err := orgimpl.ProvideService(db, setting.NewCfg(), quotaService)
require.NoError(t, err)
return db, ProvideServiceAccountsStore(db, apiKeyService, kvStore, orgService)
userSvc, err := userimpl.ProvideService(db, orgService, db.Cfg, nil, nil, quotaService)
require.NoError(t, err)
return db, ProvideServiceAccountsStore(db, apiKeyService, kvStore, userSvc, orgService)
}
func TestStore_RetrieveServiceAccount(t *testing.T) {
pkg/services/serviceaccounts/tests/common.go
+10 -1
View File
@@ -12,10 +12,13 @@ import (
"github.com/grafana/grafana/pkg/services/apikey"
"github.com/grafana/grafana/pkg/services/apikey/apikeyimpl"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/userimpl"
)
type TestUser struct {
@@ -41,7 +44,13 @@ func SetupUserServiceAccount(t *testing.T, sqlStore *sqlstore.SQLStore, testUser
role = testUser.Role
}
u1, err := sqlStore.CreateUser(context.Background(), user.CreateUserCommand{
quotaService := quotaimpl.ProvideService(sqlStore, sqlStore.Cfg)
orgService, err := orgimpl.ProvideService(sqlStore, sqlStore.Cfg, quotaService)
require.NoError(t, err)
usrSvc, err := userimpl.ProvideService(sqlStore, orgService, sqlStore.Cfg, nil, nil, quotaService)
require.NoError(t, err)
u1, err := usrSvc.CreateUserForTests(context.Background(), &user.CreateUserCommand{
Login: testUser.Login,
IsServiceAccount: testUser.IsServiceAccount,
DefaultOrgRole: role,