From 51bef166c289fef9fcd58c52d54d01a0696317cc Mon Sep 17 00:00:00 2001 From: suntala <33031346+suntala@users.noreply.github.com> Date: Tue, 31 Jan 2023 09:51:55 +0100 Subject: [PATCH] Chore: Remove Result field from serviceaccounts, ualert (#62476) * Chore: Remove Result field from serviceaccounts * Chore: Remove Result field from ualert --- pkg/services/serviceaccounts/api/api.go | 2 +- pkg/services/serviceaccounts/api/api_test.go | 7 +++---- pkg/services/serviceaccounts/api/token.go | 7 ++++--- pkg/services/serviceaccounts/api/token_test.go | 6 +++--- .../serviceaccounts/database/stats_test.go | 2 +- .../serviceaccounts/database/token_store.go | 8 +++++--- .../serviceaccounts/database/token_store_test.go | 16 ++++------------ pkg/services/serviceaccounts/manager/service.go | 4 ++-- .../serviceaccounts/manager/service_test.go | 9 +++++---- pkg/services/serviceaccounts/manager/store.go | 2 +- pkg/services/serviceaccounts/models.go | 10 ++++------ pkg/services/serviceaccounts/serviceaccounts.go | 5 ++++- .../sqlstore/migrations/ualert/dashboard.go | 2 -- 13 files changed, 37 insertions(+), 43 deletions(-) diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index d48ee7efb1c..b11665388ca 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -47,7 +47,7 @@ type service interface { MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error RevertApiKey(ctx context.Context, saId int64, keyId int64) error // Service account tokens - AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error + AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error } diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index cdaad1e2696..f6f97f40928 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -260,7 +260,7 @@ var _ service = new(fakeService) type fakeService struct { service ExpectedErr error - ExpectedApiKey *apikey.APIKey + ExpectedAPIKey *apikey.APIKey ExpectedServiceAccountTokens []apikey.APIKey ExpectedServiceAccount *serviceaccounts.ServiceAccountDTO ExpectedServiceAccountProfile *serviceaccounts.ServiceAccountProfileDTO @@ -286,9 +286,8 @@ func (f *fakeService) UpdateServiceAccount(ctx context.Context, orgID, id int64, return f.ExpectedServiceAccountProfile, f.ExpectedErr } -func (f *fakeService) AddServiceAccountToken(ctx context.Context, id int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error { - cmd.Result = f.ExpectedApiKey - return f.ExpectedErr +func (f *fakeService) AddServiceAccountToken(ctx context.Context, id int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) { + return f.ExpectedAPIKey, f.ExpectedErr } func (f *fakeService) DeleteServiceAccountToken(ctx context.Context, orgID, id, tokenID int64) error { diff --git a/pkg/services/serviceaccounts/api/token.go b/pkg/services/serviceaccounts/api/token.go index 5867cc7734f..b67d85b3402 100644 --- a/pkg/services/serviceaccounts/api/token.go +++ b/pkg/services/serviceaccounts/api/token.go @@ -175,7 +175,8 @@ func (api *ServiceAccountsAPI) CreateToken(c *contextmodel.ReqContext) response. cmd.Key = newKeyInfo.HashedKey - if err := api.service.AddServiceAccountToken(c.Req.Context(), saID, &cmd); err != nil { + apiKey, err := api.service.AddServiceAccountToken(c.Req.Context(), saID, &cmd) + if err != nil { if errors.Is(err, database.ErrInvalidTokenExpiration) { return response.Error(http.StatusBadRequest, err.Error(), nil) } @@ -186,8 +187,8 @@ func (api *ServiceAccountsAPI) CreateToken(c *contextmodel.ReqContext) response. } result := &dtos.NewApiKeyResult{ - ID: cmd.Result.Id, - Name: cmd.Result.Name, + ID: apiKey.Id, + Name: apiKey.Name, Key: newKeyInfo.ClientSecret, } diff --git a/pkg/services/serviceaccounts/api/token_test.go b/pkg/services/serviceaccounts/api/token_test.go index 0f91a88a1a0..79ce0ef1bb7 100644 --- a/pkg/services/serviceaccounts/api/token_test.go +++ b/pkg/services/serviceaccounts/api/token_test.go @@ -64,7 +64,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) { permissions []accesscontrol.Permission tokenTTL int64 expectedErr error - expectedApiKey *apikey.APIKey + expectedAPIKey *apikey.APIKey expectedCode int } @@ -75,7 +75,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) { body: `{"name": "test"}`, tokenTTL: -1, permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, - expectedApiKey: &apikey.APIKey{}, + expectedAPIKey: &apikey.APIKey{}, expectedCode: http.StatusOK, }, { @@ -111,7 +111,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) { a.cfg.ApiKeyMaxSecondsToLive = tt.tokenTTL a.service = &fakeService{ ExpectedErr: tt.expectedErr, - ExpectedApiKey: tt.expectedApiKey, + ExpectedAPIKey: tt.expectedAPIKey, } }) req := server.NewRequest(http.MethodPost, fmt.Sprintf("/api/serviceaccounts/%d/tokens", tt.id), strings.NewReader(tt.body)) diff --git a/pkg/services/serviceaccounts/database/stats_test.go b/pkg/services/serviceaccounts/database/stats_test.go index 15f6983b138..558bb7cf246 100644 --- a/pkg/services/serviceaccounts/database/stats_test.go +++ b/pkg/services/serviceaccounts/database/stats_test.go @@ -28,7 +28,7 @@ func TestStore_UsageStats(t *testing.T) { SecondsToLive: 0, } - err = store.AddServiceAccountToken(context.Background(), sa.ID, &cmd) + _, err = store.AddServiceAccountToken(context.Background(), sa.ID, &cmd) require.NoError(t, err) stats, err := store.GetUsageMetrics(context.Background()) diff --git a/pkg/services/serviceaccounts/database/token_store.go b/pkg/services/serviceaccounts/database/token_store.go index 15b716c8d61..10e0b17ca6f 100644 --- a/pkg/services/serviceaccounts/database/token_store.go +++ b/pkg/services/serviceaccounts/database/token_store.go @@ -38,8 +38,10 @@ func (s *ServiceAccountsStoreImpl) ListTokens( return result, err } -func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, serviceAccountId int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error { - return s.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { +func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, serviceAccountId int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) { + var apiKey *apikey.APIKey + + return apiKey, s.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { if _, err := s.RetrieveServiceAccount(ctx, cmd.OrgId, serviceAccountId); err != nil { return err } @@ -64,7 +66,7 @@ func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, s return err } - cmd.Result = addKeyCmd.Result + apiKey = addKeyCmd.Result return nil }) } diff --git a/pkg/services/serviceaccounts/database/token_store_test.go b/pkg/services/serviceaccounts/database/token_store_test.go index c95cd1cd9ff..9dd02e5e80f 100644 --- a/pkg/services/serviceaccounts/database/token_store_test.go +++ b/pkg/services/serviceaccounts/database/token_store_test.go @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/components/apikeygen" - "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" ) @@ -35,17 +34,15 @@ func TestStore_AddServiceAccountToken(t *testing.T) { OrgId: user.OrgID, Key: key.HashedKey, SecondsToLive: tc.secondsToLive, - Result: &apikey.APIKey{}, } - err = store.AddServiceAccountToken(context.Background(), user.ID, &cmd) + newKey, err := store.AddServiceAccountToken(context.Background(), user.ID, &cmd) if tc.secondsToLive < 0 { require.Error(t, err) return } require.NoError(t, err) - newKey := cmd.Result require.Equal(t, t.Name(), newKey.Name) // Verify against DB @@ -90,10 +87,9 @@ func TestStore_AddServiceAccountToken_WrongServiceAccount(t *testing.T) { OrgId: sa.OrgID, Key: key.HashedKey, SecondsToLive: 0, - Result: &apikey.APIKey{}, } - err = store.AddServiceAccountToken(context.Background(), sa.ID+1, &cmd) + _, err = store.AddServiceAccountToken(context.Background(), sa.ID+1, &cmd) require.Error(t, err, "It should not be possible to add token to non-existing service account") } @@ -111,12 +107,10 @@ func TestStore_RevokeServiceAccountToken(t *testing.T) { OrgId: sa.OrgID, Key: key.HashedKey, SecondsToLive: 0, - Result: &apikey.APIKey{}, } - err = store.AddServiceAccountToken(context.Background(), sa.ID, &cmd) + newKey, err := store.AddServiceAccountToken(context.Background(), sa.ID, &cmd) require.NoError(t, err) - newKey := cmd.Result // Revoke SAT err = store.RevokeServiceAccountToken(context.Background(), sa.OrgID, sa.ID, newKey.Id) @@ -153,12 +147,10 @@ func TestStore_DeleteServiceAccountToken(t *testing.T) { OrgId: sa.OrgID, Key: key.HashedKey, SecondsToLive: 0, - Result: &apikey.APIKey{}, } - err = store.AddServiceAccountToken(context.Background(), sa.ID, &cmd) + newKey, err := store.AddServiceAccountToken(context.Background(), sa.ID, &cmd) require.NoError(t, err) - newKey := cmd.Result // Delete key from wrong service account err = store.DeleteServiceAccountToken(context.Background(), sa.OrgID, sa.ID+2, newKey.Id) diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index daf3ca637c7..74560bfb7f8 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -206,9 +206,9 @@ func (sa *ServiceAccountsService) ListTokens(ctx context.Context, query *service return sa.store.ListTokens(ctx, query) } -func (sa *ServiceAccountsService) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, query *serviceaccounts.AddServiceAccountTokenCommand) error { +func (sa *ServiceAccountsService) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, query *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) { if err := validServiceAccountID(serviceAccountID); err != nil { - return err + return nil, err } return sa.store.AddServiceAccountToken(ctx, serviceAccountID, query) } diff --git a/pkg/services/serviceaccounts/manager/service_test.go b/pkg/services/serviceaccounts/manager/service_test.go index a5425acd7ad..1e5d68a4b55 100644 --- a/pkg/services/serviceaccounts/manager/service_test.go +++ b/pkg/services/serviceaccounts/manager/service_test.go @@ -19,7 +19,8 @@ type FakeServiceAccountStore struct { ExpectedSearchServiceAccountQueryResult *serviceaccounts.SearchOrgServiceAccountsResult ExpectedServiceAccountMigrationStatus *serviceaccounts.APIKeysMigrationStatus ExpectedStats *serviceaccounts.Stats - ExpectedApiKeys []apikey.APIKey + ExpectedAPIKeys []apikey.APIKey + ExpectedAPIKey *apikey.APIKey ExpectedError error } @@ -85,7 +86,7 @@ func (f *FakeServiceAccountStore) RevertApiKey(ctx context.Context, saId int64, // ListTokens is a fake listing tokens. func (f *FakeServiceAccountStore) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { - return f.ExpectedApiKeys, f.ExpectedError + return f.ExpectedAPIKeys, f.ExpectedError } // RevokeServiceAccountToken is a fake revoking a service account token. @@ -94,8 +95,8 @@ func (f *FakeServiceAccountStore) RevokeServiceAccountToken(ctx context.Context, } // AddServiceAccountToken is a fake adding a service account token. -func (f *FakeServiceAccountStore) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error { - return f.ExpectedError +func (f *FakeServiceAccountStore) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) { + return f.ExpectedAPIKey, f.ExpectedError } // DeleteServiceAccountToken is a fake deleting a service account token. diff --git a/pkg/services/serviceaccounts/manager/store.go b/pkg/services/serviceaccounts/manager/store.go index 448820059b7..7ddb53ceb34 100644 --- a/pkg/services/serviceaccounts/manager/store.go +++ b/pkg/services/serviceaccounts/manager/store.go @@ -33,7 +33,7 @@ type store interface { RevertApiKey(ctx context.Context, saId int64, keyId int64) error ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) RevokeServiceAccountToken(ctx context.Context, orgId, serviceAccountId, tokenId int64) error - AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error + AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) (*apikey.APIKey, error) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error GetUsageMetrics(ctx context.Context) (*serviceaccounts.Stats, error) } diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index 5c74cda8b6e..68a7ab64411 100644 --- a/pkg/services/serviceaccounts/models.go +++ b/pkg/services/serviceaccounts/models.go @@ -4,7 +4,6 @@ import ( "time" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" ) @@ -72,11 +71,10 @@ type GetSATokensQuery struct { } type AddServiceAccountTokenCommand struct { - Name string `json:"name" binding:"Required"` - OrgId int64 `json:"-"` - Key string `json:"-"` - SecondsToLive int64 `json:"secondsToLive"` - Result *apikey.APIKey `json:"-"` + Name string `json:"name" binding:"Required"` + OrgId int64 `json:"-"` + Key string `json:"-"` + SecondsToLive int64 `json:"secondsToLive"` } type SearchOrgServiceAccountsQuery struct { diff --git a/pkg/services/serviceaccounts/serviceaccounts.go b/pkg/services/serviceaccounts/serviceaccounts.go index 1452aa10e9d..5ef93b0fc1e 100644 --- a/pkg/services/serviceaccounts/serviceaccounts.go +++ b/pkg/services/serviceaccounts/serviceaccounts.go @@ -2,6 +2,8 @@ package serviceaccounts import ( "context" + + "github.com/grafana/grafana/pkg/services/apikey" ) /* @@ -17,5 +19,6 @@ type Service interface { RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, saForm *UpdateServiceAccountForm) (*ServiceAccountProfileDTO, error) - AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *AddServiceAccountTokenCommand) error + AddServiceAccountToken(ctx context.Context, serviceAccountID int64, + cmd *AddServiceAccountTokenCommand) (*apikey.APIKey, error) } diff --git a/pkg/services/sqlstore/migrations/ualert/dashboard.go b/pkg/services/sqlstore/migrations/ualert/dashboard.go index aa6ff8be3ba..d7440f60b02 100644 --- a/pkg/services/sqlstore/migrations/ualert/dashboard.go +++ b/pkg/services/sqlstore/migrations/ualert/dashboard.go @@ -87,8 +87,6 @@ type saveFolderCommand struct { PluginId string `json:"-"` FolderId int64 `json:"folderId"` IsFolder bool `json:"isFolder"` - - Result *dashboard } // GetDashboardModel turns the command into the saveable model