simplify code

gravitational · Apr 1, 2024 · 5cc5dfb · 5cc5dfb
1 parent 18cce10
commit 5cc5dfb
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 39 deletions.
diff --git a/lib/services/local/access_list.go b/lib/services/local/access_list.go
@@ -148,46 +148,21 @@ func (a *AccessListService) GetAccessListsToReview(ctx context.Context) ([]*acce
 
 // UpsertAccessList creates or updates an access list resource.
 func (a *AccessListService) UpsertAccessList(ctx context.Context, accessList *accesslist.AccessList) (*accesslist.AccessList, error) {
-	var upserted *accesslist.AccessList
-	upsertWithLockFn := func() error {
-		return a.service.RunWhileLocked(ctx, lockName(accessList.GetName()), accessListLockTTL, func(ctx context.Context, _ backend.Backend) error {
-			ownerMap := make(map[string]struct{}, len(accessList.Spec.Owners))
-			for _, owner := range accessList.Spec.Owners {
-				if _, ok := ownerMap[owner.Name]; ok {
-					return trace.AlreadyExists("owner %s already exists in the owner list", owner.Name)
-				}
-				ownerMap[owner.Name] = struct{}{}
-			}
-
-			var err error
-			upserted, err = a.service.UpsertResource(ctx, accessList)
-			return trace.Wrap(err)
-		})
-	}
-
-	var err error
-	if feature := modules.GetModules().Features(); !feature.IGSEnabled() {
-		err = a.service.RunWhileLocked(ctx, "createAccessListLimitLock", accessListLockTTL, func(ctx context.Context, _ backend.Backend) error {
-			if err := a.VerifyAccessListCreateLimit(ctx, accessList.GetName()); err != nil {
-				return trace.Wrap(err)
-			}
-			return trace.Wrap(upsertWithLockFn())
-		})
-	} else {
-		err = upsertWithLockFn()
-	}
-
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-
-	return upserted, nil
+	op := a.service.UpsertResource
+	return a.runOpWithLock(ctx, accessList, op)
 }
 
 // UpdateAccessList updates an access list resource.
 func (a *AccessListService) UpdateAccessList(ctx context.Context, accessList *accesslist.AccessList) (*accesslist.AccessList, error) {
+	op := a.service.ConditionalUpdateResource
+	return a.runOpWithLock(ctx, accessList, op)
+}
+
+type opFunc func(context.Context, *accesslist.AccessList) (*accesslist.AccessList, error)
+
+func (a *AccessListService) runOpWithLock(ctx context.Context, accessList *accesslist.AccessList, op opFunc) (*accesslist.AccessList, error) {
 	var upserted *accesslist.AccessList
-	updateWithLockFn := func() error {
+	upsertWithLockFn := func() error {
 		return a.service.RunWhileLocked(ctx, lockName(accessList.GetName()), accessListLockTTL, func(ctx context.Context, _ backend.Backend) error {
 			ownerMap := make(map[string]struct{}, len(accessList.Spec.Owners))
 			for _, owner := range accessList.Spec.Owners {
@@ -198,7 +173,7 @@ func (a *AccessListService) UpdateAccessList(ctx context.Context, accessList *ac
 			}
 
 			var err error
-			upserted, err = a.service.ConditionalUpdateResource(ctx, accessList)
+			upserted, err = op(ctx, accessList)
 			return trace.Wrap(err)
 		})
 	}
@@ -209,10 +184,10 @@ func (a *AccessListService) UpdateAccessList(ctx context.Context, accessList *ac
 			if err := a.VerifyAccessListCreateLimit(ctx, accessList.GetName()); err != nil {
 				return trace.Wrap(err)
 			}
-			return trace.Wrap(updateWithLockFn())
+			return trace.Wrap(upsertWithLockFn())
 		})
 	} else {
-		err = updateWithLockFn()
+		err = upsertWithLockFn()
 	}
 
 	if err != nil {

diff --git a/lib/services/simple/access_list.go b/lib/services/simple/access_list.go
@@ -186,6 +186,6 @@ func (a *AccessListService) DeleteAllAccessListReviews(ctx context.Context) erro
 
 // ListAllAccessListMembers returns a paginated list of all access list members for all access lists.
 func (a *AccessListService) ListAllAccessListMembers(ctx context.Context, pageSize int, pageToken string) (members []*accesslist.AccessListMember, nextToken string, err error) {
-	members, nextToken, err =  a.memberService.ListResources(ctx, pageSize, nextToken)
+	members, nextToken, err = a.memberService.ListResources(ctx, pageSize, nextToken)
 	return members, nextToken, trace.Wrap(err)
 }