Skip to content

Commit

Permalink
perf: improve get environment api key latency (#1380)
Browse files Browse the repository at this point in the history
Signed-off-by: Alessandro Yuichi Okimoto <[email protected]>
  • Loading branch information
cre8ivejp authored Dec 13, 2024
1 parent 9788305 commit d9d5895
Show file tree
Hide file tree
Showing 28 changed files with 1,388 additions and 1,482 deletions.
30 changes: 11 additions & 19 deletions api-description/web-api.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -725,15 +725,15 @@ paths:
tags:
- API Key
/v1/account/get_environment_api_key:
post:
get:
summary: Get Environment API Key
description: Get an environment API Key.
operationId: web.v1.account.get_environment_api_key
responses:
"200":
description: A successful response.
schema:
$ref: '#/definitions/accountGetAPIKeyBySearchingAllEnvironmentsResponse'
$ref: '#/definitions/accountGetEnvironmentAPIKeyResponse'
"400":
description: Returned for bad requests that may have failed validation.
schema:
Expand Down Expand Up @@ -766,11 +766,10 @@ paths:
schema:
$ref: '#/definitions/rpcStatus'
parameters:
- name: body
in: body
required: true
schema:
$ref: '#/definitions/accountGetAPIKeyBySearchingAllEnvironmentsRequest'
- name: apiKey
in: query
required: false
type: string
tags:
- API Key
/v1/account/get_me:
Expand Down Expand Up @@ -3309,18 +3308,6 @@ definitions:
- UNKNOWN
- FEATURE_FLAG
default: UNKNOWN
accountGetAPIKeyBySearchingAllEnvironmentsRequest:
type: object
properties:
id:
type: string
apiKey:
type: string
accountGetAPIKeyBySearchingAllEnvironmentsResponse:
type: object
properties:
environmentApiKey:
$ref: '#/definitions/accountEnvironmentAPIKey'
accountGetAPIKeyResponse:
type: object
properties:
Expand Down Expand Up @@ -3350,6 +3337,11 @@ definitions:
properties:
account:
$ref: '#/definitions/accountAccountV2'
accountGetEnvironmentAPIKeyResponse:
type: object
properties:
environmentApiKey:
$ref: '#/definitions/accountEnvironmentAPIKey'
accountGetMeRequest:
type: object
properties:
Expand Down
2 changes: 1 addition & 1 deletion manifests/bucketeer/charts/api/values.yaml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion manifests/bucketeer/charts/web/values.yaml

Large diffs are not rendered by default.

41 changes: 0 additions & 41 deletions pkg/account/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,47 +106,6 @@ func (s *AccountService) makeProjectSet(projects []*environmentproto.Project) ma
return projectSet
}

func (s *AccountService) listProjects(ctx context.Context) ([]*environmentproto.Project, error) {
projects := []*environmentproto.Project{}
cursor := ""
for {
resp, err := s.environmentClient.ListProjects(ctx, &environmentproto.ListProjectsRequest{
PageSize: listRequestPageSize,
Cursor: cursor,
})
if err != nil {
return nil, err
}
projects = append(projects, resp.Projects...)
projectSize := len(resp.Projects)
if projectSize == 0 || projectSize < listRequestPageSize {
return projects, nil
}
cursor = resp.Cursor
}
}

func (s *AccountService) listEnvironments(ctx context.Context) ([]*environmentproto.EnvironmentV2, error) {
var environments []*environmentproto.EnvironmentV2
cursor := ""
for {
resp, err := s.environmentClient.ListEnvironmentsV2(ctx, &environmentproto.ListEnvironmentsV2Request{
PageSize: listRequestPageSize,
Cursor: cursor,
Archived: wrapperspb.Bool(false),
})
if err != nil {
return nil, err
}
environments = append(environments, resp.Environments...)
environmentSize := len(resp.Environments)
if environmentSize == 0 || environmentSize < listRequestPageSize {
return environments, nil
}
cursor = resp.Cursor
}
}

func (s *AccountService) listProjectsByOrganizationID(
ctx context.Context,
organizationID string,
Expand Down
126 changes: 24 additions & 102 deletions pkg/account/api/api_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,21 +649,15 @@ func (s *AccountService) newAPIKeyListOrders(
return []*mysql.Order{mysql.NewOrder(column, direction)}, nil
}

func (s *AccountService) GetAPIKeyBySearchingAllEnvironments(
func (s *AccountService) GetEnvironmentAPIKey(
ctx context.Context,
req *proto.GetAPIKeyBySearchingAllEnvironmentsRequest,
) (*proto.GetAPIKeyBySearchingAllEnvironmentsResponse, error) {
req *proto.GetEnvironmentAPIKeyRequest,
) (*proto.GetEnvironmentAPIKeyResponse, error) {
localizer := locale.NewLocalizer(ctx)
_, err := s.checkSystemAdminRole(ctx, localizer)
if err != nil {
return nil, err
}

// TODO: support both fields, when migration finished, remove this block
if req.ApiKey == "" {
req.ApiKey = req.Id
}

if req.ApiKey == "" {
dt, err := statusMissingAPIKeyID.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Expand All @@ -674,54 +668,24 @@ func (s *AccountService) GetAPIKeyBySearchingAllEnvironments(
}
return nil, dt.Err()
}
projects, err := s.listProjects(ctx)
envAPIKey, err := s.accountStorage.GetEnvironmentAPIKey(ctx, req.ApiKey)
if err != nil {
s.logger.Error(
"Failed to get project list",
log.FieldsFromImcomingContext(ctx).AddFields(zap.Error(err))...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
}
if len(projects) == 0 {
s.logger.Error(
"Could not find any projects",
log.FieldsFromImcomingContext(ctx).AddFields(zap.Error(err))...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
}
environments, err := s.listEnvironments(ctx)
if err != nil {
s.logger.Error(
"Failed to get environment list",
log.FieldsFromImcomingContext(ctx).AddFields(zap.Error(err))...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
if errors.Is(err, v2as.ErrAPIKeyNotFound) {
dt, err := statusNotFound.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.NotFoundError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
}
return nil, dt.Err()
}
if len(environments) == 0 {
s.logger.Error(
"Could not find any environments",
log.FieldsFromImcomingContext(ctx).AddFields(zap.Error(err))...,
"Failed to get environment api key",
log.FieldsFromImcomingContext(ctx).AddFields(
zap.Error(err),
zap.String("apiKey", req.ApiKey),
)...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Expand All @@ -732,55 +696,13 @@ func (s *AccountService) GetAPIKeyBySearchingAllEnvironments(
}
return nil, dt.Err()
}
projectSet := s.makeProjectSet(projects)
for _, e := range environments {
p, ok := projectSet[e.ProjectId]
if !ok || p.Disabled {
continue
}
apiKey, err := s.accountStorage.GetAPIKeyByAPIKey(ctx, req.ApiKey, e.Id)
if err != nil {
if errors.Is(err, v2as.ErrAPIKeyNotFound) {
continue
}
s.logger.Error(
"Failed to get api key",
log.FieldsFromImcomingContext(ctx).AddFields(
zap.Error(err),
zap.String("environmentId", e.Id),
)...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
}

// for security, obfuscate the returned key
shadowLen := int(float64(len(apiKey.ApiKey)) * apiKeyShadowPercentage)
apiKey.ApiKey = apiKey.ApiKey[shadowLen:]
// for security, obfuscate the returned key
shadowLen := int(float64(len(envAPIKey.ApiKey.ApiKey)) * apiKeyShadowPercentage)
envAPIKey.ApiKey.ApiKey = envAPIKey.ApiKey.ApiKey[shadowLen:]

return &proto.GetAPIKeyBySearchingAllEnvironmentsResponse{
EnvironmentApiKey: &proto.EnvironmentAPIKey{
ApiKey: apiKey.APIKey,
ProjectId: p.Id,
ProjectUrlCode: p.UrlCode,
Environment: e,
},
}, nil
}
dt, err := statusNotFound.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.NotFoundError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return &proto.GetEnvironmentAPIKeyResponse{
EnvironmentApiKey: envAPIKey.EnvironmentAPIKey,
}, nil
}

func (s *AccountService) UpdateAPIKey(
Expand Down
40 changes: 20 additions & 20 deletions pkg/account/client/mock/client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/account/domain/api_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ type APIKey struct {
*proto.APIKey
}

type EnvironmentAPIKey struct {
*proto.EnvironmentAPIKey
}

func NewAPIKey(
name string,
role proto.APIKey_Role,
Expand Down
Loading

0 comments on commit d9d5895

Please sign in to comment.