Ensure pool exists before linking

4 files changed, 51 insertions, 1 deletions

diff --git a/changelogs/unreleased/jc-ensure-pool-exists-before-linking.yml b/changelogs/unreleased/jc-ensure-pool-exists-before-linking.yml
new file mode 100644
index 000000000..514e90a4d
--- /dev/null
+++ b/changelogs/unreleased/jc-ensure-pool-exists-before-linking.yml
@@ -0,0 +1,5 @@
+---
+title: Ensure pool exists in LinkRepositoryToObjectPool rpc
+merge_request: 1222
+author:
+type: other
diff --git a/internal/helper/security.go b/internal/helper/security.go
index afbcced9d..e66186580 100644
--- a/internal/helper/security.go
+++ b/internal/helper/security.go
@@ -1,6 +1,7 @@
 package helper
 
 import (
+	"errors"
 	"os"
 	"regexp"
 	"strings"
@@ -24,3 +25,8 @@ var hostPattern = regexp.MustCompile(`(?i)([a-z][a-z0-9+\-.]*://)([a-z0-9\-._~%!
 func SanitizeString(str string) string {
 	return hostPattern.ReplaceAllString(str, "$1[FILTERED]@$3$4")
 }
+
+// SanitizeError does the same thing as SanitizeString but for error types
+func SanitizeError(err error) error {
+	return errors.New(SanitizeString(err.Error()))
+}
diff --git a/internal/service/objectpool/link.go b/internal/service/objectpool/link.go
index 37e64ad07..3b97a1ee7 100644
--- a/internal/service/objectpool/link.go
+++ b/internal/service/objectpool/link.go
@@ -21,8 +21,12 @@ func (s *server) LinkRepositoryToObjectPool(ctx context.Context, req *gitalypb.L
 		return nil, err
 	}
 
+	if err := pool.Init(ctx); err != nil {
+		return nil, helper.ErrInternal(err)
+	}
+
 	if err := pool.Link(ctx, req.GetRepository()); err != nil {
-		return nil, status.Error(codes.Internal, helper.SanitizeString(err.Error()))
+		return nil, helper.ErrInternal(helper.SanitizeError(err))
 	}
 
 	return &gitalypb.LinkRepositoryToObjectPoolResponse{}, nil
diff --git a/internal/service/objectpool/link_test.go b/internal/service/objectpool/link_test.go
index 870fd27fe..28bf3e9f6 100644
--- a/internal/service/objectpool/link_test.go
+++ b/internal/service/objectpool/link_test.go
@@ -5,6 +5,9 @@ import (
 	"path/filepath"
 	"testing"
 
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
@@ -155,6 +158,38 @@ func TestLinkNoClobber(t *testing.T) {
 	require.Equal(t, contentBefore, string(contentAfter), "contents of existing alternates file should not have changed")
 }
 
+func TestLinkNoPool(t *testing.T) {
+	server, serverSocketPath := runObjectPoolServer(t)
+	defer server.Stop()
+
+	client, conn := newObjectPoolClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := objectpool.NewObjectPool(testRepo.GetStorageName(), t.Name())
+	require.NoError(t, err)
+	// intentionally do not call pool.Create
+	defer pool.Remove(ctx)
+
+	request := &gitalypb.LinkRepositoryToObjectPoolRequest{
+		Repository: testRepo,
+		ObjectPool: pool.ToProto(),
+	}
+
+	_, err = client.LinkRepositoryToObjectPool(ctx, request)
+	require.NoError(t, err)
+
+	poolRepoPath, err := helper.GetRepoPath(pool)
+	require.NoError(t, err)
+
+	assert.True(t, helper.IsGitDirectory(poolRepoPath))
+}
+
 func TestUnlink(t *testing.T) {
 	server, serverSocketPath := runObjectPoolServer(t)
 	defer server.Stop()