Merge branch 'pks-hook-refactor-prereceive' into 'master'

hook: Pull primary-only pre-receive logic into its own function

See merge request gitlab-org/gitaly!2593

1 files changed, 47 insertions, 39 deletions

diff --git a/internal/gitaly/hook/prereceive.go b/internal/gitaly/hook/prereceive.go
index e3bee9eb9..8f8fa79cf 100644
--- a/internal/gitaly/hook/prereceive.go
+++ b/internal/gitaly/hook/prereceive.go
@@ -40,6 +40,27 @@ func getRelativeObjectDirs(repoPath, gitObjectDir, gitAlternateObjectDirs string
 }
 
 func (m *GitLabHookManager) PreReceiveHook(ctx context.Context, repo *gitalypb.Repository, env []string, stdin io.Reader, stdout, stderr io.Writer) error {
+	changes, err := ioutil.ReadAll(stdin)
+	if err != nil {
+		return helper.ErrInternalf("reading stdin from request: %w", err)
+	}
+
+	primary, err := isPrimary(env)
+	if err != nil {
+		return helper.ErrInternalf("could not check role: %w", err)
+	}
+
+	// Only the primary should execute hooks and increment reference counters.
+	if primary {
+		if err := m.preReceiveHook(ctx, repo, env, changes, stdout, stderr); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (m *GitLabHookManager) preReceiveHook(ctx context.Context, repo *gitalypb.Repository, env []string, changes []byte, stdout, stderr io.Writer) error {
 	if gitObjDir, gitAltObjDirs := getEnvVar("GIT_OBJECT_DIRECTORY", env), getEnvVar("GIT_ALTERNATE_OBJECT_DIRECTORIES", env); gitObjDir != "" && gitAltObjDirs != "" {
 		repoPath, err := helper.GetRepoPath(repo)
 		if err != nil {
@@ -55,54 +76,41 @@ func (m *GitLabHookManager) PreReceiveHook(ctx context.Context, repo *gitalypb.R
 		repo.GitAlternateObjectDirectories = gitAltObjectDirRel
 	}
 
-	changes, err := ioutil.ReadAll(stdin)
-	if err != nil {
-		return helper.ErrInternalf("reading stdin from request: %w", err)
-	}
-
 	glID, glRepo, glProtocol := getEnvVar("GL_ID", env), getEnvVar("GL_REPOSITORY", env), getEnvVar("GL_PROTOCOL", env)
 
-	primary, err := isPrimary(env)
+	allowed, message, err := m.gitlabAPI.Allowed(ctx, repo, glRepo, glID, glProtocol, string(changes))
 	if err != nil {
-		return helper.ErrInternalf("could not check role: %w", err)
+		return fmt.Errorf("GitLab: %v", err)
 	}
 
-	// Only the primary should execute hooks and increment reference counters.
-	if primary {
-		allowed, message, err := m.gitlabAPI.Allowed(ctx, repo, glRepo, glID, glProtocol, string(changes))
-		if err != nil {
-			return fmt.Errorf("GitLab: %v", err)
-		}
-
-		if !allowed {
-			return errors.New(message)
-		}
+	if !allowed {
+		return errors.New(message)
+	}
 
-		executor, err := m.newCustomHooksExecutor(repo, "pre-receive")
-		if err != nil {
-			return fmt.Errorf("creating custom hooks executor: %w", err)
-		}
+	executor, err := m.newCustomHooksExecutor(repo, "pre-receive")
+	if err != nil {
+		return fmt.Errorf("creating custom hooks executor: %w", err)
+	}
 
-		if err = executor(
-			ctx,
-			nil,
-			env,
-			bytes.NewReader(changes),
-			stdout,
-			stderr,
-		); err != nil {
-			return fmt.Errorf("executing custom hooks: %w", err)
-		}
+	if err = executor(
+		ctx,
+		nil,
+		env,
+		bytes.NewReader(changes),
+		stdout,
+		stderr,
+	); err != nil {
+		return fmt.Errorf("executing custom hooks: %w", err)
+	}
 
-		// reference counter
-		ok, err := m.gitlabAPI.PreReceive(ctx, glRepo)
-		if err != nil {
-			return helper.ErrInternalf("calling pre_receive endpoint: %v", err)
-		}
+	// reference counter
+	ok, err := m.gitlabAPI.PreReceive(ctx, glRepo)
+	if err != nil {
+		return helper.ErrInternalf("calling pre_receive endpoint: %v", err)
+	}
 
-		if !ok {
-			return errors.New("")
-		}
+	if !ok {
+		return errors.New("")
 	}
 
 	return nil