Add DisconnectGitAlternates RPC

5 files changed, 395 insertions, 8 deletions

diff --git a/changelogs/unreleased/has-gitalternates.yml b/changelogs/unreleased/has-gitalternates.yml
new file mode 100644
index 000000000..6d94d750d
--- /dev/null
+++ b/changelogs/unreleased/has-gitalternates.yml
@@ -0,0 +1,5 @@
+---
+title: Add DisconnectGitAlternates RPC
+merge_request: 1141
+author:
+type: added
diff --git a/internal/service/objectpool/alternates.go b/internal/service/objectpool/alternates.go
new file mode 100644
index 000000000..09d2a3453
--- /dev/null
+++ b/internal/service/objectpool/alternates.go
@@ -0,0 +1,241 @@
+package objectpool
+
+import (
+	"context"
+	"crypto/rand"
+	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+	"time"
+
+	grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
+	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+)
+
+// DisconnectGitAlternates is a slightly dangerous RPC. It optimistically
+// hard-links all alternate objects we might need, and then temporarily
+// removes (renames) objects/info/alternates and runs 'git fsck'. If we
+// are unlucky that leaves the repository in a broken state during 'git
+// fsck'. If we are very unlucky and Gitaly crashes, the repository stays
+// in a broken state until an administrator intervenes and restores the
+// backed-up copy of objects/info/alternates.
+func (s *server) DisconnectGitAlternates(ctx context.Context, req *gitalypb.DisconnectGitAlternatesRequest) (*gitalypb.DisconnectGitAlternatesResponse, error) {
+	repo := req.Repository
+
+	if repo == nil {
+		return nil, helper.ErrInvalidArgument(errors.New("no repository"))
+	}
+
+	if err := disconnectAlternates(ctx, repo); err != nil {
+		return nil, helper.ErrInternal(err)
+	}
+
+	return &gitalypb.DisconnectGitAlternatesResponse{}, nil
+}
+
+func disconnectAlternates(ctx context.Context, repo *gitalypb.Repository) error {
+	repoPath, err := helper.GetRepoPath(repo)
+	if err != nil {
+		return err
+	}
+
+	altFile, err := git.InfoAlternatesPath(repo)
+	if err != nil {
+		return err
+	}
+
+	altContents, err := ioutil.ReadFile(altFile)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+
+		return err
+	}
+
+	altDir := strings.TrimSpace(string(altContents))
+	if strings.Contains(altDir, "\n") {
+		return fmt.Errorf("found multiple lines in %s", altFile)
+	}
+
+	if !filepath.IsAbs(altDir) {
+		altDir = filepath.Join(repoPath, "objects", altDir)
+	}
+
+	backupFile, err := newBackupFile(altFile)
+	if err != nil {
+		return err
+	}
+
+	stat, err := os.Stat(altDir)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+
+		return err
+	}
+
+	if !stat.IsDir() {
+		// The existing alternates file is bogus. There can be no objects on the
+		// other end that the current repository relies on, so we could just
+		// delete objects/info/alternates. To be a little more careful, instead of
+		// deleting it we rename it.
+		return os.Rename(altFile, backupFile)
+	}
+
+	objectFiles, err := findObjectFiles(altDir)
+	if err != nil {
+		return err
+	}
+
+	for _, path := range objectFiles {
+		source := filepath.Join(altDir, path)
+		target := filepath.Join(repoPath, "objects", path)
+
+		if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil {
+			return err
+		}
+
+		if err := os.Link(source, target); err != nil {
+			if os.IsExist(err) {
+				continue
+			}
+
+			return err
+		}
+	}
+
+	return removeAlternatesIfOk(ctx, repo, altFile, backupFile)
+}
+
+func newBackupFile(altFile string) (string, error) {
+	randSuffix := make([]byte, 6)
+	if _, err := io.ReadFull(rand.Reader, randSuffix); err != nil {
+		return "", err
+	}
+
+	return fmt.Sprintf("%s.%d.%x", altFile, time.Now().Unix(), randSuffix), nil
+}
+
+func findObjectFiles(altDir string) ([]string, error) {
+	var objectFiles []string
+	if walkErr := filepath.Walk(altDir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		rel, err := filepath.Rel(altDir, path)
+		if err != nil {
+			return err
+		}
+
+		if strings.HasPrefix(rel, "info/") {
+			return nil
+		}
+
+		if info.IsDir() {
+			return nil
+		}
+
+		objectFiles = append(objectFiles, rel)
+
+		return nil
+	}); walkErr != nil {
+		return nil, walkErr
+	}
+
+	sort.Sort(objectPaths(objectFiles))
+
+	return objectFiles, nil
+}
+
+type fsckError struct{ error }
+
+func (fe *fsckError) Error() string {
+	return fmt.Sprintf("git fsck error while disconnected: %v", fe.error)
+}
+
+// removeAlternatesIfOk is dangerous. We optimistically temporarily
+// rename objects/info/alternates, and run `git fsck` to see if the
+// resulting repo is connected. If this fails we restore
+// objects/info/alternates. If the repo is not connected for whatever
+// reason, then until this function returns, probably **all concurrent
+// RPC calls to the repo will fail**. Also, if Gitaly crashes in the
+// middle of this function, the repo is left in a broken state. We do
+// take care to leave a copy of the alternates file, so that it can be
+// manually restored by an administrator if needed.
+func removeAlternatesIfOk(ctx context.Context, repo *gitalypb.Repository, altFile, backupFile string) error {
+	if err := os.Rename(altFile, backupFile); err != nil {
+		return err
+	}
+
+	rollback := true
+	defer func() {
+		if !rollback {
+			return
+		}
+
+		logger := grpc_logrus.Extract(ctx)
+
+		// If we would do a os.Rename, and then someone else comes and clobbers
+		// our file, it's gone forever. This trick with os.Link and os.Rename
+		// is equivalent to "cp $backupFile $altFile", meaning backupFile is
+		// preserved for possible forensic use.
+		tmp := backupFile + ".2"
+
+		if err := os.Link(backupFile, tmp); err != nil {
+			logger.WithError(err).Error("copy backup alternates file")
+			return
+		}
+
+		if err := os.Rename(tmp, altFile); err != nil {
+			logger.WithError(err).Error("restore backup alternates file")
+		}
+	}()
+
+	cmd, err := git.Command(ctx, repo, "fsck", "--connectivity-only")
+	if err != nil {
+		return err
+	}
+
+	if err := cmd.Wait(); err != nil {
+		return &fsckError{error: err}
+	}
+
+	rollback = false
+	return nil
+}
+
+type objectPaths []string
+
+func (o objectPaths) Len() int      { return len(o) }
+func (o objectPaths) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
+
+func (o objectPaths) Less(i, j int) bool {
+	return objectPriority(o[i]) <= objectPriority(o[j])
+}
+
+// Based on pack_copy_priority in git/tmp-objdir.c
+func objectPriority(name string) int {
+	if !strings.HasPrefix(name, "pack") {
+		return 0
+	}
+	if strings.HasSuffix(name, ".keep") {
+		return 1
+	}
+	if strings.HasSuffix(name, ".pack") {
+		return 2
+	}
+	if strings.HasSuffix(name, ".idx") {
+		return 3
+	}
+	return 4
+}
diff --git a/internal/service/objectpool/alternates_test.go b/internal/service/objectpool/alternates_test.go
new file mode 100644
index 000000000..a8269f037
--- /dev/null
+++ b/internal/service/objectpool/alternates_test.go
@@ -0,0 +1,148 @@
+package objectpool
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/objectpool"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestDisconnectGitAlternates(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, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := objectpool.NewObjectPool(testRepo.GetStorageName(), testhelper.NewTestObjectPoolName(t))
+	require.NoError(t, err)
+	defer pool.Remove(ctx)
+
+	require.NoError(t, pool.Create(ctx, testRepo))
+	require.NoError(t, pool.Link(ctx, testRepo))
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "gc")
+
+	existingObjectID := "55bc176024cfa3baaceb71db584c7e5df900ea65"
+
+	// Corrupt the repository to check that existingObjectID can no longer be found
+	altPath, err := git.InfoAlternatesPath(testRepo)
+	require.NoError(t, err, "find info/alternates")
+	require.NoError(t, os.RemoveAll(altPath))
+
+	cmd, err := git.Command(ctx, testRepo, "cat-file", "-e", existingObjectID)
+	require.NoError(t, err)
+	require.Error(t, cmd.Wait(), "expect cat-file to fail because object cannot be found")
+
+	require.NoError(t, pool.Link(ctx, testRepo))
+	require.FileExists(t, altPath, "objects/info/alternates should be back")
+
+	// At this point we know that the repository has access to
+	// existingObjectID, but only if objects/info/alternates is in place.
+
+	_, err = client.DisconnectGitAlternates(ctx, &gitalypb.DisconnectGitAlternatesRequest{Repository: testRepo})
+	require.NoError(t, err, "call DisconnectGitAlternates")
+
+	// Check that the object can still be found, even though
+	// objects/info/alternates is gone. This is the purpose of
+	// DisconnectGitAlternates.
+	testhelper.AssertFileNotExists(t, altPath)
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "cat-file", "-e", existingObjectID)
+}
+
+func TestDisconnectGitAlternatesNoAlternates(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, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	altPath, err := git.InfoAlternatesPath(testRepo)
+	require.NoError(t, err, "find info/alternates")
+	testhelper.AssertFileNotExists(t, altPath)
+
+	_, err = client.DisconnectGitAlternates(ctx, &gitalypb.DisconnectGitAlternatesRequest{Repository: testRepo})
+	require.NoError(t, err, "call DisconnectGitAlternates on repository without alternates")
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "fsck")
+}
+
+func TestDisconnectGitAlternatesMultipleAlternates(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()
+
+	altPath, err := git.InfoAlternatesPath(testRepo)
+	require.NoError(t, err, "find info/alternates")
+
+	altContent := "/foo/bar\n/qux/baz\n"
+	require.NoError(t, ioutil.WriteFile(altPath, []byte(altContent), 0644))
+
+	_, err = client.DisconnectGitAlternates(ctx, &gitalypb.DisconnectGitAlternatesRequest{Repository: testRepo})
+	require.Error(t, err, "call DisconnectGitAlternates on repository with multiple alternates")
+
+	contentAfterRPC, err := ioutil.ReadFile(altPath)
+	require.NoError(t, err, "read back objects/info/alternates")
+	require.Equal(t, altContent, string(contentAfterRPC), "objects/info/alternates content should not have changed")
+}
+
+func TestRemoveAlternatesIfOk(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	altPath, err := git.InfoAlternatesPath(testRepo)
+	require.NoError(t, err, "find info/alternates")
+	altContent := "/var/empty\n"
+	require.NoError(t, ioutil.WriteFile(altPath, []byte(altContent), 0644), "write alternates file")
+
+	// Intentionally break the repository, so that 'git fsck' will fail later.
+	testhelper.MustRunCommand(t, nil, "sh", "-c", fmt.Sprintf("rm %s/objects/pack/*.pack", testRepoPath))
+
+	altBackup := altPath + ".backup"
+
+	err = removeAlternatesIfOk(ctx, testRepo, altPath, altBackup)
+	require.Error(t, err, "removeAlternatesIfOk should fail")
+	require.IsType(t, &fsckError{}, err, "error must be because of fsck")
+
+	// We expect objects/info/alternates to have been restored when
+	// removeAlternatesIfOk returned.
+	assertAlternates(t, altPath, altContent)
+
+	// We expect the backup alternates file to still exist.
+	assertAlternates(t, altBackup, altContent)
+}
+
+func assertAlternates(t *testing.T, altPath string, altContent string) {
+	actualContent, err := ioutil.ReadFile(altPath)
+	require.NoError(t, err, "read %s after fsck failure", altPath)
+
+	require.Equal(t, altContent, string(actualContent), "%s content after fsck failure", altPath)
+}
diff --git a/internal/service/objectpool/server.go b/internal/service/objectpool/server.go
index d3cab887c..e6dbb1c6b 100644
--- a/internal/service/objectpool/server.go
+++ b/internal/service/objectpool/server.go
@@ -1,10 +1,7 @@
 package objectpool
 
 import (
-	"context"
-
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 type server struct{}
@@ -13,7 +10,3 @@ type server struct{}
 func NewServer() gitalypb.ObjectPoolServiceServer {
 	return &server{}
 }
-
-func (s *server) DisconnectGitAlternates(ctx context.Context, req *gitalypb.DisconnectGitAlternatesRequest) (*gitalypb.DisconnectGitAlternatesResponse, error) {
-	return nil, helper.Unimplemented
-}
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index d4d5dbe11..71242fa0c 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -460,7 +460,7 @@ func GetRepositoryRefs(t *testing.T, repoPath string) string {
 // AssertFileNotExists asserts true if the file doesn't exist, false otherwise
 func AssertFileNotExists(t *testing.T, path string) {
 	_, err := os.Stat(path)
-	assert.True(t, os.IsNotExist(err))
+	assert.True(t, os.IsNotExist(err), "file should not exist: %s", path)
 }
 
 // NewTestObjectPoolName returns a random pool repository name.