Mitigate racing condition between create repos and delete space (#1178)

app/api/controller/check/check_list.go

@@ -33,7 +33,7 @@ func (c *Controller) ListChecks(
 ) ([]types.Check, int, error) {
 	repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView)
 	if err != nil {
-		return nil, 0, fmt.Errorf("failed to acquire access access to repo: %w", err)
+		return nil, 0, fmt.Errorf("failed to acquire access to repo: %w", err)
 	}
 
 	var checks []types.Check

app/api/controller/check/check_recent.go

@@ -33,7 +33,7 @@ func (c *Controller) ListRecentChecks(
 ) ([]string, error) {
 	repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView)
 	if err != nil {
-		return nil, fmt.Errorf("failed to acquire access access to repo: %w", err)
+		return nil, fmt.Errorf("failed to acquire access to repo: %w", err)
 	}
 
 	if opts.Since == 0 {

app/api/controller/check/check_report.go

@@ -128,7 +128,7 @@ func (c *Controller) Report(
 ) (*types.Check, error) {
 	repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoReportCommitCheck)
 	if err != nil {
-		return nil, fmt.Errorf("failed to acquire access access to repo: %w", err)
+		return nil, fmt.Errorf("failed to acquire access to repo: %w", err)
 	}
 
 	if errValidate := in.Sanitize(c.sanitizers, session); errValidate != nil {

app/api/controller/pullreq/check_list.go

@@ -36,7 +36,7 @@ func (c *Controller) ListChecks(
 ) (types.PullReqChecks, error) {
 	repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView)
 	if err != nil {
-		return types.PullReqChecks{}, fmt.Errorf("failed to acquire access access to repo: %w", err)
+		return types.PullReqChecks{}, fmt.Errorf("failed to acquire access to repo: %w", err)
 	}
 
 	pr, err := c.pullreqStore.FindByNumber(ctx, repo.ID, prNum)

app/api/controller/pullreq/pr_create.go

@@ -55,14 +55,14 @@ func (c *Controller) Create(
 
 	targetRepo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoPush)
 	if err != nil {
-		return nil, fmt.Errorf("failed to acquire access access to target repo: %w", err)
+		return nil, fmt.Errorf("failed to acquire access to target repo: %w", err)
 	}
 
 	sourceRepo := targetRepo
 	if in.SourceRepoRef != "" {
 		sourceRepo, err = c.getRepoCheckAccess(ctx, session, in.SourceRepoRef, enum.PermissionRepoView)
 		if err != nil {
-			return nil, fmt.Errorf("failed to acquire access access to source repo: %w", err)
+			return nil, fmt.Errorf("failed to acquire access to source repo: %w", err)
 		}
 	}
 

app/api/controller/repo/create.go

@@ -81,6 +81,12 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea
 			return fmt.Errorf("resource limit exceeded: %w", limiter.ErrMaxNumReposReached)
 		}
 
+		// lock the space for update during repo creation to prevent racing conditions with space soft delete.
+		parentSpace, err = c.spaceStore.FindForUpdate(ctx, parentSpace.ID)
+		if err != nil {
+			return fmt.Errorf("failed to find the parent space: %w", err)
+		}
+
 		gitResp, isEmpty, err := c.createGitRepository(ctx, session, in)
 		if err != nil {
 			return fmt.Errorf("error creating repository on git: %w", err)

app/api/controller/repo/import.go

@@ -66,6 +66,12 @@ func (c *Controller) Import(ctx context.Context, session *auth.Session, in *Impo
 			c.publicResourceCreationEnabled,
 		)
 
+		// lock the space for update during repo creation to prevent racing conditions with space soft delete.
+		parentSpace, err = c.spaceStore.FindForUpdate(ctx, parentSpace.ID)
+		if err != nil {
+			return fmt.Errorf("failed to find the parent space: %w", err)
+		}
+
 		err = c.repoStore.Create(ctx, repo)
 		if err != nil {
 			return fmt.Errorf("failed to create repository in storage: %w", err)

app/api/controller/space/import_repositories.go

@@ -96,6 +96,12 @@ func (c *Controller) ImportRepositories(
 	duplicateRepos := make([]*types.Repository, 0, len(remoteRepositories))
 
 	err = c.tx.WithTx(ctx, func(ctx context.Context) error {
+		// lock the space for update during repo creation to prevent racing conditions with space soft delete.
+		space, err = c.spaceStore.FindForUpdate(ctx, space.ID)
+		if err != nil {
+			return fmt.Errorf("failed to find the parent space: %w", err)
+		}
+
 		if err := c.resourceLimiter.RepoCount(
 			ctx, space.ID, len(remoteRepositories)); err != nil {
 			return fmt.Errorf("resource limit exceeded: %w", limiter.ErrMaxNumReposReached)

app/api/controller/space/soft_delete.go

@@ -81,6 +81,10 @@ func (c *Controller) softDeleteInnerInTx(
 	session *auth.Session,
 	space *types.Space,
 ) (*SoftDeleteResponse, error) {
+	_, err := c.spaceStore.FindForUpdate(ctx, space.ID)
+	if err != nil {
+		return nil, fmt.Errorf("failed to lock the space for update: %w", err)
+	}
 	filter := &types.SpaceFilter{
 		Page:              1,
 		Size:              math.MaxInt,
@@ -98,6 +102,11 @@ func (c *Controller) softDeleteInnerInTx(
 	now := time.Now().UnixMilli()
 
 	for _, space := range subSpaces {
+		_, err := c.spaceStore.FindForUpdate(ctx, space.ID)
+		if err != nil {
+			return nil, fmt.Errorf("failed to lock the space for update: %w", err)
+		}
+
 		if err := c.spaceStore.SoftDelete(ctx, space, now); err != nil {
 			return nil, fmt.Errorf("failed to soft delete subspace: %w", err)
 		}

app/store/database.go

@@ -176,6 +176,9 @@ type (
 		UpdateOptLock(ctx context.Context, space *types.Space,
 			mutateFn func(space *types.Space) error) (*types.Space, error)
 
+		// FindForUpdate finds the space and locks it for an update.
+		FindForUpdate(ctx context.Context, id int64) (*types.Space, error)
+
 		// SoftDelete deletes the space.
 		SoftDelete(ctx context.Context, space *types.Space, deletedAt int64) error
 

app/store/database/space.go

@@ -386,6 +386,32 @@ func (s *SpaceStore) updateDeletedOptLock(
 	)
 }
 
+// FindForUpdate finds the space and locks it for an update (should be called in a tx).
+func (s *SpaceStore) FindForUpdate(ctx context.Context, id int64) (*types.Space, error) {
+	// sqlite allows at most one write to proceed (no need to lock)
+	if strings.HasPrefix(s.db.DriverName(), "sqlite") {
+		return s.find(ctx, id, nil)
+	}
+
+	stmt := database.Builder.Select("space_id").
+		From("spaces").
+		Where("space_id = ? AND space_deleted IS NULL", id).
+		Suffix("FOR UPDATE")
+
+	sqlQuery, params, err := stmt.ToSql()
+	if err != nil {
+		return nil, database.ProcessSQLErrorf(ctx, err, "failed to generate lock on spaces")
+	}
+
+	dst := new(space)
+	db := dbtx.GetAccessor(ctx, s.db)
+	if err = db.GetContext(ctx, dst, sqlQuery, params...); err != nil {
+		return nil, database.ProcessSQLErrorf(ctx, err, "Failed to find space")
+	}
+
+	return mapToSpace(ctx, s.db, s.spacePathStore, dst)
+}
+
 // SoftDelete deletes a space softly.
 func (s *SpaceStore) SoftDelete(
 	ctx context.Context,