From a9ccd374e0b56d380ede756847412fca76cd4b28 Mon Sep 17 00:00:00 2001 From: Femi Novia Lina Date: Thu, 18 Apr 2024 18:28:38 +0700 Subject: [PATCH] feat: logging fail user fetch --- cmd/serve.go | 35 ++++++++----------- core/action/service.go | 14 ++++---- core/group/service.go | 12 +++---- core/namespace/service.go | 14 ++++---- core/organization/service.go | 19 +++++----- core/policy/service.go | 14 ++++---- core/project/service.go | 12 +++---- core/relation/service.go | 14 ++++---- core/resource/service.go | 12 +++---- core/role/service.go | 14 ++++---- core/user/service.go | 22 ++++++------ .../store/postgres/activity_repository.go | 10 +----- .../postgres/activity_repository_test.go | 17 --------- ...0240402042520_create_activity_table.up.sql | 2 +- .../testdata/configs/rules/rule.yaml | 2 +- 15 files changed, 93 insertions(+), 120 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index 75413894a..a6a8140f6 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -36,7 +36,6 @@ import ( "github.com/goto/shield/pkg/db" "github.com/goto/salt/log" - grpczap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" "github.com/pkg/profile" "google.golang.org/grpc/codes" ) @@ -94,26 +93,24 @@ func StartServer(logger *log.Zap, cfg *config.Shield) error { schemaMigrationConfig := schema.NewSchemaMigrationConfig(cfg.App.DefaultSystemEmail) // - serviceLogger := grpczap.Extract(ctx).Sugar() - activityRepository := postgres.NewActivityRepository(dbClient) activityService := activity.NewService(activityRepository) userRepository := postgres.NewUserRepository(dbClient) - userService := user.NewService(serviceLogger, userRepository, activityService) + userService := user.NewService(logger, userRepository, activityService) actionRepository := postgres.NewActionRepository(dbClient) - actionService := action.NewService(serviceLogger, actionRepository, userService, activityService) + actionService := action.NewService(logger, actionRepository, userService, activityService) roleRepository := postgres.NewRoleRepository(dbClient) - roleService := role.NewService(serviceLogger, roleRepository, userService, activityService) + roleService := role.NewService(logger, roleRepository, userService, activityService) policyPGRepository := postgres.NewPolicyRepository(dbClient) policySpiceRepository := spicedb.NewPolicyRepository(spiceDBClient) - policyService := policy.NewService(serviceLogger, policyPGRepository, userService, activityService) + policyService := policy.NewService(logger, policyPGRepository, userService, activityService) namespaceRepository := postgres.NewNamespaceRepository(dbClient) - namespaceService := namespace.NewService(serviceLogger, namespaceRepository, userService, activityService) + namespaceService := namespace.NewService(logger, namespaceRepository, userService, activityService) s := schema.NewSchemaMigrationService( blob.NewSchemaConfigRepository(resourceBlobFS), @@ -173,42 +170,40 @@ func BuildAPIDependencies( dbc *db.Client, sdb *spicedb.SpiceDB, ) (api.Deps, error) { - serviceLogger := grpczap.Extract(ctx).Sugar() - activityRepository := postgres.NewActivityRepository(dbc) activityService := activity.NewService(activityRepository) userRepository := postgres.NewUserRepository(dbc) - userService := user.NewService(serviceLogger, userRepository, activityService) + userService := user.NewService(logger, userRepository, activityService) actionRepository := postgres.NewActionRepository(dbc) - actionService := action.NewService(serviceLogger, actionRepository, userService, activityService) + actionService := action.NewService(logger, actionRepository, userService, activityService) namespaceRepository := postgres.NewNamespaceRepository(dbc) - namespaceService := namespace.NewService(serviceLogger, namespaceRepository, userService, activityService) + namespaceService := namespace.NewService(logger, namespaceRepository, userService, activityService) roleRepository := postgres.NewRoleRepository(dbc) - roleService := role.NewService(serviceLogger, roleRepository, userService, activityService) + roleService := role.NewService(logger, roleRepository, userService, activityService) relationPGRepository := postgres.NewRelationRepository(dbc) relationSpiceRepository := spicedb.NewRelationRepository(sdb) - relationService := relation.NewService(serviceLogger, relationPGRepository, relationSpiceRepository, userService, activityService) + relationService := relation.NewService(logger, relationPGRepository, relationSpiceRepository, userService, activityService) groupRepository := postgres.NewGroupRepository(dbc) - groupService := group.NewService(serviceLogger, groupRepository, relationService, userService, activityService) + groupService := group.NewService(logger, groupRepository, relationService, userService, activityService) organizationRepository := postgres.NewOrganizationRepository(dbc) - organizationService := organization.NewService(serviceLogger, organizationRepository, relationService, userService, activityService) + organizationService := organization.NewService(logger, organizationRepository, relationService, userService, activityService) projectRepository := postgres.NewProjectRepository(dbc) - projectService := project.NewService(serviceLogger, projectRepository, relationService, userService, activityService) + projectService := project.NewService(logger, projectRepository, relationService, userService, activityService) policyPGRepository := postgres.NewPolicyRepository(dbc) - policyService := policy.NewService(serviceLogger, policyPGRepository, userService, activityService) + policyService := policy.NewService(logger, policyPGRepository, userService, activityService) resourcePGRepository := postgres.NewResourceRepository(dbc) resourceService := resource.NewService( - serviceLogger, resourcePGRepository, resourceBlobRepository, relationService, userService, projectService, organizationService, groupService, activityService) + logger, resourcePGRepository, resourceBlobRepository, relationService, userService, projectService, organizationService, groupService, activityService) relationAdapter := adapter.NewRelation(groupService, userService, relationService) diff --git a/core/action/service.go b/core/action/service.go index 868674ad8..29a0c715e 100644 --- a/core/action/service.go +++ b/core/action/service.go @@ -4,9 +4,9 @@ import ( "context" "fmt" + "github.com/goto/salt/log" "github.com/goto/shield/core/user" pkgctx "github.com/goto/shield/pkg/context" - "go.uber.org/zap" ) const ( @@ -23,13 +23,13 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository userService UserService activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, userService UserService, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, userService UserService, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -45,7 +45,7 @@ func (s Service) Get(ctx context.Context, id string) (Action, error) { func (s Service) Create(ctx context.Context, action Action) (Action, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Action{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } newAction, err := s.repository.Create(ctx, action) @@ -57,7 +57,7 @@ func (s Service) Create(ctx context.Context, action Action) (Action, error) { ctx := pkgctx.WithoutCancel(ctx) actionLogData := newAction.ToActionLogData() if err := s.activityService.Log(ctx, auditKeyActionCreate, currentUser.ID, actionLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -71,7 +71,7 @@ func (s Service) List(ctx context.Context) ([]Action, error) { func (s Service) Update(ctx context.Context, id string, action Action) (Action, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Action{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } updatedAction, err := s.repository.Update(ctx, Action{ @@ -87,7 +87,7 @@ func (s Service) Update(ctx context.Context, id string, action Action) (Action, ctx := pkgctx.WithoutCancel(ctx) actionLogData := updatedAction.ToActionLogData() if err := s.activityService.Log(ctx, auditKeyActionUpdate, currentUser.ID, actionLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/core/group/service.go b/core/group/service.go index 29ea533ce..daf9a3088 100644 --- a/core/group/service.go +++ b/core/group/service.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/goto/salt/log" "github.com/goto/shield/core/action" "github.com/goto/shield/core/namespace" "github.com/goto/shield/core/relation" @@ -13,7 +14,6 @@ import ( pkgctx "github.com/goto/shield/pkg/context" "github.com/goto/shield/pkg/str" "github.com/goto/shield/pkg/uuid" - "go.uber.org/zap" ) const ( @@ -38,14 +38,14 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository relationService RelationService userService UserService activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, relationService RelationService, userService UserService, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, relationService RelationService, userService UserService, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -74,7 +74,7 @@ func (s Service) Create(ctx context.Context, grp Group) (Group, error) { ctx := pkgctx.WithoutCancel(ctx) groupLogData := newGroup.ToGroupLogData() if err := s.activityService.Log(ctx, auditKeyGroupCreate, currentUser.ID, groupLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -103,7 +103,7 @@ func (s Service) List(ctx context.Context, flt Filter) ([]Group, error) { func (s Service) Update(ctx context.Context, grp Group) (Group, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Group{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } if strings.TrimSpace(grp.ID) != "" { @@ -119,7 +119,7 @@ func (s Service) Update(ctx context.Context, grp Group) (Group, error) { ctx := pkgctx.WithoutCancel(ctx) groupLogData := updatedGroup.ToGroupLogData() if err := s.activityService.Log(ctx, auditKeyGroupUpdate, currentUser.ID, groupLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/core/namespace/service.go b/core/namespace/service.go index e8ed05226..439e53f89 100644 --- a/core/namespace/service.go +++ b/core/namespace/service.go @@ -4,9 +4,9 @@ import ( "context" "fmt" + "github.com/goto/salt/log" "github.com/goto/shield/core/user" pkgctx "github.com/goto/shield/pkg/context" - "go.uber.org/zap" ) const ( @@ -23,13 +23,13 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository userService UserService activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, userService UserService, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, userService UserService, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -45,7 +45,7 @@ func (s Service) Get(ctx context.Context, id string) (Namespace, error) { func (s Service) Create(ctx context.Context, ns Namespace) (Namespace, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Namespace{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } newNamespace, err := s.repository.Create(ctx, ns) @@ -57,7 +57,7 @@ func (s Service) Create(ctx context.Context, ns Namespace) (Namespace, error) { ctx := pkgctx.WithoutCancel(ctx) namespaceLogData := newNamespace.ToNameSpaceLogData() if err := s.activityService.Log(ctx, auditKeyNamespaceCreate, currentUser.ID, namespaceLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -71,7 +71,7 @@ func (s Service) List(ctx context.Context) ([]Namespace, error) { func (s Service) Update(ctx context.Context, ns Namespace) (Namespace, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Namespace{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } updatedNamespace, err := s.repository.Update(ctx, ns) @@ -83,7 +83,7 @@ func (s Service) Update(ctx context.Context, ns Namespace) (Namespace, error) { ctx := pkgctx.WithoutCancel(ctx) namespaceLogData := updatedNamespace.ToNameSpaceLogData() if err := s.activityService.Log(ctx, auditKeyNamespaceUpdate, currentUser.ID, namespaceLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/core/organization/service.go b/core/organization/service.go index 7436cf58b..a7d1c3482 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/goto/salt/log" "github.com/goto/shield/core/action" "github.com/goto/shield/core/namespace" "github.com/goto/shield/core/relation" @@ -11,7 +12,6 @@ import ( "github.com/goto/shield/internal/schema" pkgctx "github.com/goto/shield/pkg/context" "github.com/goto/shield/pkg/uuid" - "go.uber.org/zap" ) const ( @@ -36,14 +36,14 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository relationService RelationService userService UserService activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, relationService RelationService, userService UserService, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, relationService RelationService, userService UserService, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -83,7 +83,7 @@ func (s Service) Create(ctx context.Context, org Organization) (Organization, er ctx := pkgctx.WithoutCancel(ctx) organizationLogData := newOrg.ToOrganizationLogData() if err := s.activityService.Log(ctx, auditKeyOrganizationCreate, currentUser.ID, organizationLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -97,14 +97,17 @@ func (s Service) List(ctx context.Context) ([]Organization, error) { func (s Service) Update(ctx context.Context, org Organization) (Organization, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Organization{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } + var updatedOrg Organization + if org.ID != "" { - return s.repository.UpdateByID(ctx, org) + updatedOrg, err = s.repository.UpdateByID(ctx, org) + } else { + updatedOrg, err = s.repository.UpdateBySlug(ctx, org) } - updatedOrg, err := s.repository.UpdateBySlug(ctx, org) if err != nil { return Organization{}, err } @@ -113,7 +116,7 @@ func (s Service) Update(ctx context.Context, org Organization) (Organization, er ctx := pkgctx.WithoutCancel(ctx) organizationLogData := updatedOrg.ToOrganizationLogData() if err := s.activityService.Log(ctx, auditKeyOrganizationUpdate, currentUser.ID, organizationLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/core/policy/service.go b/core/policy/service.go index 955fc8849..30c7101cd 100644 --- a/core/policy/service.go +++ b/core/policy/service.go @@ -4,9 +4,9 @@ import ( "context" "fmt" + "github.com/goto/salt/log" "github.com/goto/shield/core/user" pkgctx "github.com/goto/shield/pkg/context" - "go.uber.org/zap" ) const ( @@ -23,13 +23,13 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository userService UserService activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, userService UserService, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, userService UserService, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -49,7 +49,7 @@ func (s Service) List(ctx context.Context) ([]Policy, error) { func (s Service) Create(ctx context.Context, policy Policy) ([]Policy, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return []Policy{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } policyId, err := s.repository.Create(ctx, policy) @@ -65,7 +65,7 @@ func (s Service) Create(ctx context.Context, policy Policy) ([]Policy, error) { ctx := pkgctx.WithoutCancel(ctx) policyLogData := policy.ToPolicyLogData(policyId) if err := s.activityService.Log(ctx, auditKeyPolicyCreate, currentUser.ID, policyLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -75,7 +75,7 @@ func (s Service) Create(ctx context.Context, policy Policy) ([]Policy, error) { func (s Service) Update(ctx context.Context, pol Policy) ([]Policy, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return []Policy{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } policyId, err := s.repository.Update(ctx, pol) @@ -92,7 +92,7 @@ func (s Service) Update(ctx context.Context, pol Policy) ([]Policy, error) { ctx := pkgctx.WithoutCancel(ctx) policyLogData := pol.ToPolicyLogData(policyId) if err := s.activityService.Log(ctx, auditKeyPolicyUpdate, currentUser.ID, policyLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/core/project/service.go b/core/project/service.go index bf9eb7bb9..ee1bc2e76 100644 --- a/core/project/service.go +++ b/core/project/service.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/goto/salt/log" "github.com/goto/shield/core/action" "github.com/goto/shield/core/namespace" "github.com/goto/shield/core/organization" @@ -12,7 +13,6 @@ import ( "github.com/goto/shield/internal/schema" pkgctx "github.com/goto/shield/pkg/context" "github.com/goto/shield/pkg/uuid" - "go.uber.org/zap" ) const ( @@ -37,14 +37,14 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository relationService RelationService userService UserService activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, relationService RelationService, userService UserService, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, relationService RelationService, userService UserService, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -85,7 +85,7 @@ func (s Service) Create(ctx context.Context, prj Project) (Project, error) { ctx := pkgctx.WithoutCancel(ctx) projectLogData := newProject.ToProjectLogData() if err := s.activityService.Log(ctx, auditKeyProjectCreate, currentUser.ID, projectLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -99,7 +99,7 @@ func (s Service) List(ctx context.Context) ([]Project, error) { func (s Service) Update(ctx context.Context, prj Project) (Project, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Project{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } if prj.ID != "" { @@ -115,7 +115,7 @@ func (s Service) Update(ctx context.Context, prj Project) (Project, error) { ctx := pkgctx.WithoutCancel(ctx) projectLogData := updatedProject.ToProjectLogData() if err := s.activityService.Log(ctx, auditKeyProjectUpdate, currentUser.ID, projectLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/core/relation/service.go b/core/relation/service.go index 46e907d76..5164aa7b6 100644 --- a/core/relation/service.go +++ b/core/relation/service.go @@ -4,11 +4,11 @@ import ( "context" "fmt" + "github.com/goto/salt/log" "github.com/goto/shield/core/action" "github.com/goto/shield/core/namespace" "github.com/goto/shield/core/user" pkgctx "github.com/goto/shield/pkg/context" - "go.uber.org/zap" ) const ( @@ -25,14 +25,14 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository authzRepository AuthzRepository userService UserService activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, authzRepository AuthzRepository, userService UserService, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, authzRepository AuthzRepository, userService UserService, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -49,7 +49,7 @@ func (s Service) Get(ctx context.Context, id string) (RelationV2, error) { func (s Service) Create(ctx context.Context, rel RelationV2) (RelationV2, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return RelationV2{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } createdRelation, err := s.repository.Create(ctx, rel) @@ -66,7 +66,7 @@ func (s Service) Create(ctx context.Context, rel RelationV2) (RelationV2, error) ctx := pkgctx.WithoutCancel(ctx) relationLogData := createdRelation.ToRelationLogData() if err := s.activityService.Log(ctx, auditKeyRelationCreate, currentUser.ID, relationLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -149,7 +149,7 @@ func (s Service) CheckPermission(ctx context.Context, usr user.User, resourceNS func (s Service) DeleteSubjectRelations(ctx context.Context, resourceType, optionalResourceID string) error { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } err = s.authzRepository.DeleteSubjectRelations(ctx, resourceType, optionalResourceID) @@ -161,7 +161,7 @@ func (s Service) DeleteSubjectRelations(ctx context.Context, resourceType, optio ctx := pkgctx.WithoutCancel(ctx) relationSubjectlogData := ToRelationSubjectLogData(resourceType, optionalResourceID) if err := s.activityService.Log(ctx, auditKeyRelationSubjectDelete, currentUser.ID, relationSubjectlogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/core/resource/service.go b/core/resource/service.go index 55c320a8b..fea1ad398 100644 --- a/core/resource/service.go +++ b/core/resource/service.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/goto/salt/log" "github.com/goto/shield/core/action" "github.com/goto/shield/core/group" "github.com/goto/shield/core/namespace" @@ -15,7 +16,6 @@ import ( "github.com/goto/shield/internal/schema" pkgctx "github.com/goto/shield/pkg/context" "github.com/goto/shield/pkg/uuid" - "go.uber.org/zap" ) const ( @@ -51,7 +51,7 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository configRepository ConfigRepository relationService RelationService @@ -62,7 +62,7 @@ type Service struct { activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, configRepository ConfigRepository, relationService RelationService, userService UserService, projectService ProjectService, organizationService OrganizationService, groupService GroupService, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, configRepository ConfigRepository, relationService RelationService, userService UserService, projectService ProjectService, organizationService OrganizationService, groupService GroupService, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -130,7 +130,7 @@ func (s Service) Create(ctx context.Context, res Resource) (Resource, error) { ctx := pkgctx.WithoutCancel(ctx) resourceLogData := newResource.ToResourceLogData() if err := s.activityService.Log(ctx, auditKeyResourceCreate, currentUser.ID, resourceLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -151,7 +151,7 @@ func (s Service) List(ctx context.Context, flt Filter) (PagedResources, error) { func (s Service) Update(ctx context.Context, id string, resource Resource) (Resource, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Resource{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } // TODO there should be an update logic like create here @@ -164,7 +164,7 @@ func (s Service) Update(ctx context.Context, id string, resource Resource) (Reso ctx := pkgctx.WithoutCancel(ctx) resourceLogData := updatedResource.ToResourceLogData() if err := s.activityService.Log(ctx, auditKeyResourceUpdate, currentUser.ID, resourceLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/core/role/service.go b/core/role/service.go index eb9db219c..d5f99c54e 100644 --- a/core/role/service.go +++ b/core/role/service.go @@ -4,9 +4,9 @@ import ( "context" "fmt" + "github.com/goto/salt/log" "github.com/goto/shield/core/user" pkgctx "github.com/goto/shield/pkg/context" - "go.uber.org/zap" ) const ( @@ -23,13 +23,13 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository userService UserService activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, userService UserService, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, userService UserService, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -41,7 +41,7 @@ func NewService(logger *zap.SugaredLogger, repository Repository, userService Us func (s Service) Create(ctx context.Context, toCreate Role) (Role, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Role{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } roleID, err := s.repository.Create(ctx, toCreate) @@ -58,7 +58,7 @@ func (s Service) Create(ctx context.Context, toCreate Role) (Role, error) { ctx := pkgctx.WithoutCancel(ctx) roleLogData := newRole.ToRoleLogData() if err := s.activityService.Log(ctx, auditKeyRoleCreate, currentUser.ID, roleLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -76,7 +76,7 @@ func (s Service) List(ctx context.Context) ([]Role, error) { func (s Service) Update(ctx context.Context, toUpdate Role) (Role, error) { currentUser, err := s.userService.FetchCurrentUser(ctx) if err != nil { - return Role{}, fmt.Errorf("%w: %s", user.ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", user.ErrInvalidEmail.Error(), err.Error())) } roleID, err := s.repository.Update(ctx, toUpdate) @@ -93,7 +93,7 @@ func (s Service) Update(ctx context.Context, toUpdate Role) (Role, error) { ctx := pkgctx.WithoutCancel(ctx) roleLogData := updatedRole.ToRoleLogData() if err := s.activityService.Log(ctx, auditKeyRoleUpdate, currentUser.ID, roleLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/core/user/service.go b/core/user/service.go index 9de1a48be..03a50853c 100644 --- a/core/user/service.go +++ b/core/user/service.go @@ -5,9 +5,9 @@ import ( "fmt" "strings" + "github.com/goto/salt/log" pkgctx "github.com/goto/shield/pkg/context" "github.com/goto/shield/pkg/uuid" - "go.uber.org/zap" ) const ( @@ -21,12 +21,12 @@ type ActivityService interface { } type Service struct { - logger *zap.SugaredLogger + logger log.Logger repository Repository activityService ActivityService } -func NewService(logger *zap.SugaredLogger, repository Repository, activityService ActivityService) *Service { +func NewService(logger log.Logger, repository Repository, activityService ActivityService) *Service { return &Service{ logger: logger, repository: repository, @@ -56,7 +56,7 @@ func (s Service) GetByEmail(ctx context.Context, email string) (User, error) { func (s Service) Create(ctx context.Context, user User) (User, error) { currentUser, err := s.FetchCurrentUser(ctx) if err != nil { - return User{}, fmt.Errorf("%w: %s", ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrInvalidEmail.Error(), err.Error())) } newUser, err := s.repository.Create(ctx, User{ @@ -72,7 +72,7 @@ func (s Service) Create(ctx context.Context, user User) (User, error) { ctx := pkgctx.WithoutCancel(ctx) userLogData := newUser.ToUserLogData() if err := s.activityService.Log(ctx, auditKeyUserCreate, currentUser.ID, userLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -82,7 +82,7 @@ func (s Service) Create(ctx context.Context, user User) (User, error) { func (s Service) CreateMetadataKey(ctx context.Context, key UserMetadataKey) (UserMetadataKey, error) { currentUser, err := s.FetchCurrentUser(ctx) if err != nil { - return UserMetadataKey{}, fmt.Errorf("%w: %s", ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrInvalidEmail.Error(), err.Error())) } newUserMetadataKey, err := s.repository.CreateMetadataKey(ctx, UserMetadataKey{ @@ -97,7 +97,7 @@ func (s Service) CreateMetadataKey(ctx context.Context, key UserMetadataKey) (Us ctx := pkgctx.WithoutCancel(ctx) userMetadataKeyLogData := newUserMetadataKey.ToUserMetadataKeyLogData() if err := s.activityService.Log(ctx, auditKeyUserMetadataKeyCreate, currentUser.ID, userMetadataKeyLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -119,7 +119,7 @@ func (s Service) List(ctx context.Context, flt Filter) (PagedUsers, error) { func (s Service) UpdateByID(ctx context.Context, toUpdate User) (User, error) { currentUser, err := s.FetchCurrentUser(ctx) if err != nil { - return User{}, fmt.Errorf("%w: %s", ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrInvalidEmail.Error(), err.Error())) } updatedUser, err := s.repository.UpdateByID(ctx, toUpdate) @@ -131,7 +131,7 @@ func (s Service) UpdateByID(ctx context.Context, toUpdate User) (User, error) { ctx := pkgctx.WithoutCancel(ctx) userLogData := updatedUser.ToUserLogData() if err := s.activityService.Log(ctx, auditKeyUserUpdate, currentUser.ID, userLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() @@ -141,7 +141,7 @@ func (s Service) UpdateByID(ctx context.Context, toUpdate User) (User, error) { func (s Service) UpdateByEmail(ctx context.Context, toUpdate User) (User, error) { currentUser, err := s.FetchCurrentUser(ctx) if err != nil { - return User{}, fmt.Errorf("%w: %s", ErrInvalidEmail, err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrInvalidEmail.Error(), err.Error())) } updatedUser, err := s.repository.UpdateByEmail(ctx, toUpdate) @@ -153,7 +153,7 @@ func (s Service) UpdateByEmail(ctx context.Context, toUpdate User) (User, error) ctx := pkgctx.WithoutCancel(ctx) userLogData := updatedUser.ToUserLogData() if err := s.activityService.Log(ctx, auditKeyUserUpdate, currentUser.ID, userLogData); err != nil { - s.logger.Errorf("%s: %s", ErrLogActivity.Error(), err.Error()) + s.logger.Error(fmt.Sprintf("%s: %s", ErrLogActivity.Error(), err.Error())) } }() diff --git a/internal/store/postgres/activity_repository.go b/internal/store/postgres/activity_repository.go index 83c631606..af0ca234a 100644 --- a/internal/store/postgres/activity_repository.go +++ b/internal/store/postgres/activity_repository.go @@ -3,12 +3,10 @@ package postgres import ( "context" "encoding/json" - "errors" "fmt" "github.com/doug-martin/goqu/v9" "github.com/goto/salt/audit" - "github.com/goto/shield/core/activity" "github.com/goto/shield/pkg/db" newrelic "github.com/newrelic/go-agent" ) @@ -60,13 +58,7 @@ func (r ActivityRepository) Insert(ctx context.Context, log *audit.Log) error { _, err = r.dbc.ExecContext(ctx, query, params...) return err }); err != nil { - err = checkPostgresError(err) - switch { - case errors.Is(err, errInvalidTexRepresentation): - return activity.ErrInvalidUUID - default: - return err - } + return checkPostgresError(err) } return nil diff --git a/internal/store/postgres/activity_repository_test.go b/internal/store/postgres/activity_repository_test.go index 254af4e1e..bc2d3523f 100644 --- a/internal/store/postgres/activity_repository_test.go +++ b/internal/store/postgres/activity_repository_test.go @@ -104,23 +104,6 @@ func (s *ActivityRepositoryTestSuite) TestInsert() { }, ErrString: "parsing error: json: unsupported type: chan int", }, - { - Description: "should return error if actor uuid is invalid", - LogToCreate: &audit.Log{ - Actor: "invalid-uuid", - Action: "group.update", - Data: map[string]string{ - "entity": "group", - "id": "1234-5678-1234", - }, - Metadata: map[string]string{ - "app_name": "shield", - "app_version": "v1.0", - }, - Timestamp: time.Now(), - }, - ErrString: "invalid syntax of uuid", - }, } for _, tc := range testCases { diff --git a/internal/store/postgres/migrations/20240402042520_create_activity_table.up.sql b/internal/store/postgres/migrations/20240402042520_create_activity_table.up.sql index b2c593afe..9df414ce5 100644 --- a/internal/store/postgres/migrations/20240402042520_create_activity_table.up.sql +++ b/internal/store/postgres/migrations/20240402042520_create_activity_table.up.sql @@ -1,7 +1,7 @@ CREATE TABLE IF NOT EXISTS activities ( timestamp timestamptz, action varchar, - actor uuid, + actor varchar, data jsonb, metadata jsonb ); \ No newline at end of file diff --git a/test/e2e_test/testbench/testdata/configs/rules/rule.yaml b/test/e2e_test/testbench/testdata/configs/rules/rule.yaml index f942e947f..ab98cea5e 100644 --- a/test/e2e_test/testbench/testdata/configs/rules/rule.yaml +++ b/test/e2e_test/testbench/testdata/configs/rules/rule.yaml @@ -1,7 +1,7 @@ rules: - backends: - name: entropy - target: "http://localhost:53624" + target: "http://localhost:62568" frontends: - name: ping path: "/api/ping"