repository: Generalize setting custom hooks

Currently the `SetCustomHooks` RPC is set up in such a way that requires
a tar archive to be provided as input. In a future commit the
`ReplicateRepository` RPC will be updated to replicate hooks and
leverage the logic from `SetCustomHooks` to set hooks for the
repository. Since some repositories may not contain any custom hooks,
the logic needs to be updated to support this additional use case.

This change updates `extractHooks()` to be able to support receiving an
empty `io.Reader` as input without producing an error. Since the version
of tar used at runtime is system dependent, the code is generalized to
support both GNU and BSD tar.

2 files changed, 134 insertions, 8 deletions

diff --git a/internal/gitaly/service/repository/set_custom_hooks.go b/internal/gitaly/service/repository/set_custom_hooks.go
index 5e231a6f9..6c8d47444 100644
--- a/internal/gitaly/service/repository/set_custom_hooks.go
+++ b/internal/gitaly/service/repository/set_custom_hooks.go
@@ -1,6 +1,7 @@
 package repository
 
 import (
+	"bufio"
 	"context"
 	"encoding/binary"
 	"errors"
@@ -9,6 +10,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/command"
@@ -34,12 +36,12 @@ func (s *server) SetCustomHooks(stream gitalypb.RepositoryService_SetCustomHooks
 
 	firstRequest, err := stream.Recv()
 	if err != nil {
-		return structerr.NewInternal("first request failed %w", err)
+		return structerr.NewInternal("getting first request: %w", err)
 	}
 
 	repo := firstRequest.GetRepository()
 	if err := service.ValidateRepository(repo); err != nil {
-		return structerr.NewInvalidArgument("%w", err)
+		return structerr.NewInvalidArgument("validating repo: %w", err)
 	}
 
 	reader := streamio.NewReader(func() ([]byte, error) {
@@ -68,12 +70,12 @@ func (s *server) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCus
 
 	firstRequest, err := stream.Recv()
 	if err != nil {
-		return structerr.NewInternal("first request failed %w", err)
+		return structerr.NewInternal("getting first request: %w", err)
 	}
 
 	repo := firstRequest.GetRepository()
 	if err := service.ValidateRepository(repo); err != nil {
-		return structerr.NewInvalidArgument("%w", err)
+		return structerr.NewInvalidArgument("validating repo: %w", err)
 	}
 
 	reader := streamio.NewReader(func() ([]byte, error) {
@@ -97,7 +99,7 @@ func (s *server) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCus
 func (s *server) setCustomHooks(ctx context.Context, reader io.Reader, repo repository.GitRepo) error {
 	if featureflag.TransactionalRestoreCustomHooks.IsEnabled(ctx) {
 		if err := s.setCustomHooksTransaction(ctx, reader, repo); err != nil {
-			return fmt.Errorf("writing custom hooks: %w", err)
+			return fmt.Errorf("setting custom hooks: %w", err)
 		}
 
 		return nil
@@ -105,7 +107,7 @@ func (s *server) setCustomHooks(ctx context.Context, reader io.Reader, repo repo
 
 	repoPath, err := s.locator.GetPath(repo)
 	if err != nil {
-		return fmt.Errorf("getting repo path failed %w", err)
+		return fmt.Errorf("getting repo path: %w", err)
 	}
 
 	if err := extractHooks(ctx, reader, repoPath); err != nil {
@@ -291,14 +293,34 @@ func voteCustomHooks(
 // extractHooks unpacks a tar file containing custom hooks into a `custom_hooks`
 // directory at the specified path.
 func extractHooks(ctx context.Context, reader io.Reader, path string) error {
+	// GNU tar does not accept an empty file as a valid tar archive and produces
+	// an error. Since an empty hooks tar is symbolic of a repository having no
+	// hooks, the reader is peeked to check if there is any data present.
+	buf := bufio.NewReader(reader)
+	if _, err := buf.Peek(1); err == io.EOF {
+		return nil
+	}
+
 	cmdArgs := []string{"-xf", "-", "-C", path, customHooksDir}
 
-	cmd, err := command.New(ctx, append([]string{"tar"}, cmdArgs...), command.WithStdin(reader))
+	var stderrBuilder strings.Builder
+	cmd, err := command.New(ctx, append([]string{"tar"}, cmdArgs...),
+		command.WithStdin(buf),
+		command.WithStderr(&stderrBuilder))
 	if err != nil {
 		return fmt.Errorf("executing tar command: %w", err)
 	}
 
 	if err := cmd.Wait(); err != nil {
+		// GNU and BSD tar versions have differing errors when attempting to
+		// extract specified members from a valid tar archive. If the tar
+		// archive is valid the errors for GNU and BSD tar should have the
+		// same prefix, which can be checked to validate whether the expected
+		// content is present in the archive for extraction.
+		if strings.HasPrefix(stderrBuilder.String(), "tar: custom_hooks: Not found in archive") {
+			return nil
+		}
+
 		return fmt.Errorf("waiting for tar command completion: %w", err)
 	}
 
diff --git a/internal/gitaly/service/repository/set_custom_hooks_test.go b/internal/gitaly/service/repository/set_custom_hooks_test.go
index 6302f2fdb..040906765 100644
--- a/internal/gitaly/service/repository/set_custom_hooks_test.go
+++ b/internal/gitaly/service/repository/set_custom_hooks_test.go
@@ -5,6 +5,7 @@ package repository
 import (
 	"context"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"syscall"
@@ -192,7 +193,7 @@ func testFailedSetCustomHooksDueToValidations(t *testing.T, ctx context.Context)
 
 			err := tc.streamSender(t, ctx, client)
 			testhelper.RequireGrpcError(t, err, status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefect(
-				"empty Repository",
+				"validating repo: empty Repository",
 				"repo scoped: empty Repository",
 			)))
 		})
@@ -398,3 +399,106 @@ func mustCreateCorruptHooksArchive(t *testing.T) string {
 
 	return archivePath
 }
+
+func TestExtractHooks(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	hooksFiles := []testFile{
+		{name: "pre-commit.sample", content: "foo", mode: 0o755},
+		{name: "pre-push.sample", content: "bar", mode: 0o755},
+	}
+
+	tmpDir := testhelper.TempDir(t)
+	emptyTar := filepath.Join(tmpDir, "empty_hooks.tar")
+	err := os.WriteFile(emptyTar, nil, 0o644)
+	require.NoError(t, err)
+
+	for _, tc := range []struct {
+		desc          string
+		path          string
+		expectedFiles []testFile
+		expectedErr   string
+	}{
+		{
+			desc: "custom hooks tar",
+			path: mustCreateCustomHooksArchive(t, ctx, hooksFiles, customHooksDir),
+			expectedFiles: []testFile{
+				{name: "custom_hooks"},
+				{name: "custom_hooks/pre-commit.sample", content: "foo"},
+				{name: "custom_hooks/pre-push.sample", content: "bar"},
+			},
+		},
+		{
+			desc:          "empty custom hooks tar",
+			path:          mustCreateCustomHooksArchive(t, ctx, []testFile{}, customHooksDir),
+			expectedFiles: []testFile{{name: "custom_hooks"}},
+		},
+		{
+			desc: "no hooks tar",
+			path: mustCreateCustomHooksArchive(t, ctx, hooksFiles, "no_hooks"),
+		},
+		{
+			desc:        "corrupt tar",
+			path:        mustCreateCorruptHooksArchive(t),
+			expectedErr: "waiting for tar command completion: exit status",
+		},
+		{
+			desc: "empty tar",
+			path: emptyTar,
+		},
+	} {
+		tc := tc
+
+		t.Run(tc.desc, func(t *testing.T) {
+			t.Parallel()
+
+			_, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+			})
+
+			tarball, err := os.Open(tc.path)
+			require.NoError(t, err)
+			defer tarball.Close()
+
+			err = extractHooks(ctx, tarball, repoPath)
+			if tc.expectedErr == "" {
+				require.NoError(t, err)
+			} else {
+				require.ErrorContains(t, err, tc.expectedErr)
+			}
+
+			if len(tc.expectedFiles) == 0 {
+				require.NoDirExists(t, filepath.Join(repoPath, customHooksDir))
+				return
+			}
+
+			var extractedFiles []testFile
+
+			hooksPath := filepath.Join(repoPath, customHooksDir)
+			err = filepath.WalkDir(hooksPath, func(path string, d fs.DirEntry, err error) error {
+				require.NoError(t, err)
+
+				relPath, err := filepath.Rel(repoPath, path)
+				require.NoError(t, err)
+
+				var hookData []byte
+				if !d.IsDir() {
+					hookData, err = os.ReadFile(path)
+					require.NoError(t, err)
+				}
+
+				extractedFiles = append(extractedFiles, testFile{
+					name:    relPath,
+					content: string(hookData),
+				})
+
+				return nil
+			})
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedFiles, extractedFiles)
+		})
+	}
+}