Skip to content

Commit 7493941

Browse files
authored
feat(policy): Create/Update scs to use transaction. (#2882)
### Proposed Changes 1.) Add transactions around subject-condition-set creation/update. >[!NOTE] >I believe this is the cause for all the intermittent/flaky otdfctl >test failures that have been occurring. ### Checklist - [ ] I have added or updated unit tests - [ ] I have added or updated integration tests (if appropriate) - [ ] I have added or updated documentation ### Testing Instructions
1 parent 25e7204 commit 7493941

File tree

1 file changed

+35
-14
lines changed

1 file changed

+35
-14
lines changed

service/policy/subjectmapping/subject_mapping.go

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -267,17 +267,27 @@ func (s SubjectMappingService) CreateSubjectConditionSet(ctx context.Context,
267267
ObjectType: audit.ObjectTypeConditionSet,
268268
}
269269

270-
conditionSet, err := s.dbClient.CreateSubjectConditionSet(ctx, req.Msg.GetSubjectConditionSet())
270+
var conditionSet *policy.SubjectConditionSet
271+
err := s.dbClient.RunInTx(ctx, func(txClient *policydb.PolicyDBClient) error {
272+
cs, err := txClient.CreateSubjectConditionSet(ctx, req.Msg.GetSubjectConditionSet())
273+
if err != nil {
274+
s.logger.Audit.PolicyCRUDFailure(ctx, auditParams)
275+
return db.StatusifyError(ctx, s.logger, err, db.ErrTextCreationFailed, slog.String("subjectConditionSet", req.Msg.String()))
276+
}
277+
278+
conditionSet = cs
279+
return nil
280+
})
271281
if err != nil {
272-
s.logger.Audit.PolicyCRUDFailure(ctx, auditParams)
273-
return nil, db.StatusifyError(ctx, s.logger, err, db.ErrTextCreationFailed, slog.String("subjectConditionSet", req.Msg.String()))
282+
return nil, err
274283
}
275284

276285
auditParams.ObjectID = conditionSet.GetId()
277286
auditParams.Original = conditionSet
278287
s.logger.Audit.PolicyCRUDSuccess(ctx, auditParams)
279288

280289
rsp.SubjectConditionSet = conditionSet
290+
281291
return connect.NewResponse(rsp), nil
282292
}
283293

@@ -294,25 +304,36 @@ func (s SubjectMappingService) UpdateSubjectConditionSet(ctx context.Context,
294304
ObjectID: subjectConditionSetID,
295305
}
296306

297-
original, err := s.dbClient.GetSubjectConditionSet(ctx, subjectConditionSetID)
298-
if err != nil {
299-
s.logger.Audit.PolicyCRUDFailure(ctx, auditParams)
300-
return nil, db.StatusifyError(ctx, s.logger, err, db.ErrTextGetRetrievalFailed, slog.String("id", subjectConditionSetID))
301-
}
307+
var original, updated *policy.SubjectConditionSet
308+
err := s.dbClient.RunInTx(ctx, func(txClient *policydb.PolicyDBClient) error {
309+
orig, err := txClient.GetSubjectConditionSet(ctx, subjectConditionSetID)
310+
if err != nil {
311+
s.logger.Audit.PolicyCRUDFailure(ctx, auditParams)
312+
return db.StatusifyError(ctx, s.logger, err, db.ErrTextGetRetrievalFailed, slog.String("id", subjectConditionSetID))
313+
}
314+
315+
upd, err := txClient.UpdateSubjectConditionSet(ctx, req.Msg)
316+
if err != nil {
317+
s.logger.Audit.PolicyCRUDFailure(ctx, auditParams)
318+
return db.StatusifyError(ctx, s.logger, err, db.ErrTextUpdateFailed, slog.String("id", req.Msg.GetId()), slog.String("subjectConditionSet fields", req.Msg.String()))
319+
}
320+
321+
original = orig
322+
updated = upd
323+
rsp.SubjectConditionSet = &policy.SubjectConditionSet{
324+
Id: subjectConditionSetID,
325+
}
302326

303-
updated, err := s.dbClient.UpdateSubjectConditionSet(ctx, req.Msg)
327+
return nil
328+
})
304329
if err != nil {
305-
s.logger.Audit.PolicyCRUDFailure(ctx, auditParams)
306-
return nil, db.StatusifyError(ctx, s.logger, err, db.ErrTextUpdateFailed, slog.String("id", req.Msg.GetId()), slog.String("subjectConditionSet fields", req.Msg.String()))
330+
return nil, err
307331
}
308332

309333
auditParams.Original = original
310334
auditParams.Updated = updated
311335
s.logger.Audit.PolicyCRUDSuccess(ctx, auditParams)
312336

313-
rsp.SubjectConditionSet = &policy.SubjectConditionSet{
314-
Id: subjectConditionSetID,
315-
}
316337
return connect.NewResponse(rsp), nil
317338
}
318339

0 commit comments

Comments
 (0)