Skip to content

Commit

Permalink
feat(log): sending audit log concurrently (#140)
Browse files Browse the repository at this point in the history 
* feat(log): sending audit log concurrently

* test: update test

* chore: update ctx

* test: fix unit test temporaryly with sleep for go routine

* test: update test

* test: fix param

---------

Co-authored-by: Muhammad Idil Haq Amir <[email protected]>
  • Loading branch information
@idilhaq
idilhaq and Muhammad Idil Haq Amir authored Jun 4, 2024
1 parent 457de70 commit 133300f
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 66 deletions.
49 changes: 32 additions & 17 deletions core/appeal/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,12 @@ func (s *Service) Create(ctx context.Context, appeals []*domain.Appeal, opts ...
return fmt.Errorf("inserting appeals into db: %w", err)
}

if err := s.auditLogger.Log(ctx, AuditKeyBulkInsert, appeals); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, AuditKeyBulkInsert, appeals); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()

for _, a := range appeals {
if a.Status == domain.AppealStatusRejected {
Expand Down Expand Up @@ -647,9 +650,12 @@ func (s *Service) UpdateApproval(ctx context.Context, approvalAction domain.Appr
auditKey = AuditKeyApprove
}
if auditKey != "" {
if err := s.auditLogger.Log(ctx, auditKey, approvalAction); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, auditKey, approvalAction); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()
}

return appeal, nil
Expand Down Expand Up @@ -687,11 +693,14 @@ func (s *Service) Cancel(ctx context.Context, id string) (*domain.Appeal, error)
return nil, err
}

if err := s.auditLogger.Log(ctx, AuditKeyCancel, map[string]interface{}{
"appeal_id": id,
}); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, AuditKeyCancel, map[string]interface{}{
"appeal_id": id,
}); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()

return appeal, nil
}
Expand Down Expand Up @@ -738,9 +747,12 @@ func (s *Service) AddApprover(ctx context.Context, appealID, approvalID, email s
return nil, fmt.Errorf("converting approval to map: %w", err)
}
auditData["affected_approver"] = email
if err := s.auditLogger.Log(ctx, AuditKeyAddApprover, auditData); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, AuditKeyAddApprover, auditData); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()

duration := domain.PermanentDurationLabel
if !appeal.IsDurationEmpty() {
Expand Down Expand Up @@ -836,9 +848,12 @@ func (s *Service) DeleteApprover(ctx context.Context, appealID, approvalID, emai
return nil, fmt.Errorf("converting approval to map: %w", err)
}
auditData["affected_approver"] = email
if err := s.auditLogger.Log(ctx, AuditKeyDeleteApprover, auditData); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, AuditKeyDeleteApprover, auditData); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()

return appeal, nil
}
Expand Down
20 changes: 13 additions & 7 deletions core/appeal/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1099,9 +1099,10 @@ func (s *ServiceTestSuite) TestCreate() {
h.mockNotifier.EXPECT().
Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()
h.mockAuditLogger.EXPECT().
Log(mock.Anything, appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once()
Log(h.ctxMatcher, appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once()

actualError := h.service.Create(context.Background(), appeals)
time.Sleep(time.Millisecond)

s.Nil(actualError)
s.Equal(expectedResult, appeals)
Expand Down Expand Up @@ -1564,7 +1565,7 @@ func (s *ServiceTestSuite) TestCreate() {
h.mockNotifier.EXPECT().
Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()
h.mockAuditLogger.EXPECT().
Log(mock.Anything, appeal.AuditKeyBulkInsert, mock.Anything).
Log(h.ctxMatcher, appeal.AuditKeyBulkInsert, mock.Anything).
Return(nil).Once()

actualError := h.service.Create(context.Background(), appeals)
Expand Down Expand Up @@ -1657,7 +1658,7 @@ func (s *ServiceTestSuite) TestCreate() {
BulkUpsert(h.ctxMatcher, mock.Anything).
Return(nil).Once()
h.mockNotifier.EXPECT().Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()
h.mockAuditLogger.EXPECT().Log(mock.Anything, appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once()
h.mockAuditLogger.EXPECT().Log(h.ctxMatcher, appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once()
h.mockProviderService.EXPECT().
IsExclusiveRoleAssignment(mock.Anything, mock.Anything, mock.Anything).
Return(false).Once()
Expand Down Expand Up @@ -1942,9 +1943,10 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithExistingAppealAndWithAutoApprov
}
}).Once()
h.mockNotifier.EXPECT().Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()
h.mockAuditLogger.EXPECT().Log(mock.Anything, appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once()
h.mockAuditLogger.EXPECT().Log(h.ctxMatcher, appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once()

actualError := h.service.Create(context.Background(), appeals)
time.Sleep(time.Millisecond)

s.Nil(actualError)
s.Equal(expectedResult, appeals)
Expand Down Expand Up @@ -2126,7 +2128,7 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithAdditionalAppeals() {
appeal := appeals[0]
s.Equal(targetResource.ID, appeal.Resource.ID)
})
h.mockAuditLogger.EXPECT().Log(mock.AnythingOfType("*context.cancelCtx"), appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once()
h.mockAuditLogger.EXPECT().Log(h.ctxMatcher, appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once()
h.mockNotifier.EXPECT().Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()

// 1.b grant access for the main appeal
Expand All @@ -2143,6 +2145,7 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithAdditionalAppeals() {
h.mockNotifier.EXPECT().Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()

err := h.service.Create(context.Background(), appealsPayload)
time.Sleep(time.Millisecond)

s.NoError(err)

Expand Down Expand Up @@ -2775,10 +2778,11 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
h.mockProviderService.EXPECT().GrantAccess(mock.Anything, mock.Anything).Return(nil).Once()
h.mockRepository.EXPECT().Update(h.ctxMatcher, appealDetails).Return(nil).Once()
h.mockNotifier.EXPECT().Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()
h.mockAuditLogger.EXPECT().Log(mock.Anything, mock.Anything, mock.Anything).
h.mockAuditLogger.EXPECT().Log(h.ctxMatcher, mock.Anything, mock.Anything).
Return(nil).Once()

_, actualError := h.service.UpdateApproval(context.Background(), action)
time.Sleep(time.Millisecond)

s.Nil(actualError)

Expand Down Expand Up @@ -3178,7 +3182,7 @@ func (s *ServiceTestSuite) TestUpdateApproval() {

h.mockRepository.EXPECT().Update(h.ctxMatcher, tc.expectedResult).Return(nil).Once()
h.mockNotifier.EXPECT().Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()
h.mockAuditLogger.EXPECT().Log(mock.Anything, mock.Anything, mock.Anything).
h.mockAuditLogger.EXPECT().Log(h.ctxMatcher, mock.Anything, mock.Anything).
Return(nil).Once()

actualResult, actualError := h.service.UpdateApproval(context.Background(), tc.expectedApprovalAction)
Expand Down Expand Up @@ -3389,6 +3393,7 @@ func (s *ServiceTestSuite) TestAddApprover() {
Return(nil).Once()

actualAppeal, actualError := h.service.AddApprover(context.Background(), appealID, approvalID, newApprover)
time.Sleep(time.Millisecond)

s.NoError(actualError)
s.Equal(expectedApproval, actualAppeal.Approvals[0])
Expand Down Expand Up @@ -3670,6 +3675,7 @@ func (s *ServiceTestSuite) TestDeleteApprover() {
})).Return(nil).Once()

actualAppeal, actualError := h.service.DeleteApprover(context.Background(), appealID, approvalID, approverEmail)
time.Sleep(time.Millisecond)

s.NoError(actualError)
s.Equal(expectedApproval, actualAppeal.Approvals[0])
Expand Down
32 changes: 19 additions & 13 deletions core/grant/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,16 @@ func (s *Service) Update(ctx context.Context, payload *domain.Grant) error {
*payload = *grantDetails
s.logger.Info(ctx, "grant updated", "grant_id", grantDetails.ID, "updatedGrant", updatedGrant)

if err := s.auditLogger.Log(ctx, AuditKeyUpdate, map[string]interface{}{
"grant_id": grantDetails.ID,
"payload": updatedGrant,
"updated_grant": payload,
}); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, AuditKeyUpdate, map[string]interface{}{
"grant_id": grantDetails.ID,
"payload": updatedGrant,
"updated_grant": payload,
}); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()

if previousOwner != updatedGrant.Owner {
go func() {
Expand Down Expand Up @@ -251,12 +254,15 @@ func (s *Service) Revoke(ctx context.Context, id, actor, reason string, opts ...

s.logger.Info(ctx, "grant revoked", "grant_id", id)

if err := s.auditLogger.Log(ctx, AuditKeyRevoke, map[string]interface{}{
"grant_id": id,
"reason": reason,
}); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, AuditKeyRevoke, map[string]interface{}{
"grant_id": id,
"reason": reason,
}); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()

return grant, nil
}
Expand Down
16 changes: 10 additions & 6 deletions core/policy/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ func (s *Service) Create(ctx context.Context, p *domain.Policy) error {
return err
}

if err := s.auditLogger.Log(ctx, AuditKeyPolicyCreate, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
if err := s.auditLogger.Log(ctx, AuditKeyPolicyCreate, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()
}

if p.HasIAMConfig() {
Expand Down Expand Up @@ -229,9 +231,11 @@ func (s *Service) Update(ctx context.Context, p *domain.Policy) error {
return err
}

if err := s.auditLogger.Log(ctx, AuditKeyPolicyUpdate, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
if err := s.auditLogger.Log(ctx, AuditKeyPolicyUpdate, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()
}

if p.HasIAMConfig() {
Expand Down
4 changes: 3 additions & 1 deletion core/policy/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"testing"
"time"

"github.com/go-playground/validator/v10"
"github.com/google/uuid"
Expand Down Expand Up @@ -401,6 +402,7 @@ func (s *ServiceTestSuite) TestCreate() {
s.Nil(actualError)
s.Equal(expectedVersion, validPolicy.Version)
s.mockPolicyRepository.AssertExpectations(s.T())
time.Sleep(time.Millisecond)
s.mockAuditLogger.AssertExpectations(s.T())
s.mockCrypto.AssertExpectations(s.T())
})
Expand All @@ -418,7 +420,6 @@ func (s *ServiceTestSuite) TestCreate() {

s.Nil(actualError)
s.mockPolicyRepository.AssertNotCalled(s.T(), "Create")
s.mockAuditLogger.AssertNotCalled(s.T(), "Log")
})
})
}
Expand Down Expand Up @@ -868,6 +869,7 @@ func (s *ServiceTestSuite) TestUpdate() {

s.Equal(expectedNewVersion, p.Version)
s.mockPolicyRepository.AssertExpectations(s.T())
time.Sleep(time.Millisecond)
s.mockAuditLogger.AssertExpectations(s.T())
s.mockCrypto.AssertExpectations(s.T())
})
Expand Down
27 changes: 18 additions & 9 deletions core/provider/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,12 @@ func (s *Service) Create(ctx context.Context, p *domain.Provider) error {
return err
}

if err := s.auditLogger.Log(ctx, AuditKeyCreate, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, AuditKeyCreate, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()
} else {
s.logger.Info(ctx, "dry run enabled, skipping provider creation", "provider_urn", p.URN)
}
Expand Down Expand Up @@ -223,9 +226,12 @@ func (s *Service) Update(ctx context.Context, p *domain.Provider) error {
return err
}

if err := s.auditLogger.Log(ctx, AuditKeyUpdate, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, AuditKeyUpdate, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()
} else {
s.logger.Info(ctx, "dry run enabled, skipping provider update", "provider_urn", p.URN)
}
Expand Down Expand Up @@ -470,9 +476,12 @@ func (s *Service) Delete(ctx context.Context, id string) error {
}
s.logger.Info(ctx, "provider deleted", "provider", id)

if err := s.auditLogger.Log(ctx, AuditKeyDelete, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
go func() {
ctx := context.WithoutCancel(ctx)
if err := s.auditLogger.Log(ctx, AuditKeyDelete, p); err != nil {
s.logger.Error(ctx, "failed to record audit log", "error", err)
}
}()

return nil
}
Expand Down
1 change: 0 additions & 1 deletion core/provider/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ func (s *ServiceTestSuite) TestCreate() {

s.Nil(actualError)
s.mockProviderRepository.AssertExpectations(s.T())
s.mockAuditLogger.AssertExpectations(s.T())
})

s.Run("with dryRun true", func() {
Expand Down
Loading

0 comments on commit 133300f

Please sign in to comment.