Merge branch 'pks-remove-reftx-hook-workaround' into 'master'

hooks: Remove reftx arguments workaround

Closes #3250

See merge request gitlab-org/gitaly!2736

2 files changed, 7 insertions, 123 deletions

diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go
index 3c44b80ba..c4cbf3bef 100644
--- a/cmd/gitaly-hooks/hooks.go
+++ b/cmd/gitaly-hooks/hooks.go
@@ -27,32 +27,6 @@ import (
 	"google.golang.org/grpc"
 )
 
-// fixupReftxHook fixes up the subcommand in case case the wrong hook was executed instead of the
-// reference-transaction hook. This bug was introduced together with the reference-transaction hook
-// in Git v2.28.0. As pre-receive and post-receive don't have any arguments and update has the fully
-// qualified reference as first argument, none should ever be invoked with the verbs that the
-// reference-transaction hook receives.
-func fixupReftxHook(args []string) []string {
-	if len(args) != 3 {
-		return args
-	}
-
-	name := args[1]
-	if name != "pre-receive" && name != "post-receive" && name != "update" {
-		return args
-	}
-
-	arg := args[2]
-	if arg != "prepared" && arg != "committed" && arg != "aborted" {
-		return args
-	}
-
-	return append(
-		[]string{args[0], "reference-transaction"},
-		args[2:]...,
-	)
-}
-
 func main() {
 	var logger = gitalylog.NewHookLogger()
 
@@ -60,17 +34,15 @@ func main() {
 		logger.Fatalf("requires hook name. args: %v", os.Args)
 	}
 
-	fixedArgs := fixupReftxHook(os.Args)
-
-	subCmd := fixedArgs[1]
+	subCmd := os.Args[1]
 
 	if subCmd == "check" {
 		logrus.SetLevel(logrus.ErrorLevel)
-		if len(fixedArgs) != 3 {
+		if len(os.Args) != 3 {
 			logger.Fatal(errors.New("no configuration file path provided invoke with: gitaly-hooks check <config_path>"))
 		}
 
-		configPath := fixedArgs[2]
+		configPath := os.Args[2]
 		fmt.Print("Checking GitLab API access: ")
 
 		info, err := check(configPath)
@@ -113,7 +85,7 @@ func main() {
 
 	switch subCmd {
 	case "update":
-		args := fixedArgs[2:]
+		args := os.Args[2:]
 		if len(args) != 3 {
 			logger.Fatalf("hook %q expects exactly three arguments", subCmd)
 		}
@@ -205,12 +177,12 @@ func main() {
 			os.Exit(0)
 		}
 
-		if len(fixedArgs) != 3 {
+		if len(os.Args) != 3 {
 			logger.Fatalf("hook %q is missing required arguments", subCmd)
 		}
 
 		var state gitalypb.ReferenceTransactionHookRequest_State
-		switch fixedArgs[2] {
+		switch os.Args[2] {
 		case "prepared":
 			state = gitalypb.ReferenceTransactionHookRequest_PREPARED
 		case "committed":
@@ -218,7 +190,7 @@ func main() {
 		case "aborted":
 			state = gitalypb.ReferenceTransactionHookRequest_ABORTED
 		default:
-			logger.Fatalf("hook %q has invalid state %s", subCmd, fixedArgs[2])
+			logger.Fatalf("hook %q has invalid state %s", subCmd, os.Args[2])
 		}
 
 		referenceTransactionHookStream, err := hookClient.ReferenceTransactionHook(ctx)
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index 91f73073a..f2c570708 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -50,94 +50,6 @@ func testMain(m *testing.M) int {
 	return m.Run()
 }
 
-func TestReftxHookFixup(t *testing.T) {
-	testcases := []struct {
-		desc  string
-		args  []string
-		fixed bool
-	}{
-		{
-			desc:  "empty arguments",
-			args:  []string{},
-			fixed: false,
-		},
-		{
-			desc:  "single argument",
-			args:  []string{"gitaly-hooks"},
-			fixed: false,
-		},
-		{
-			desc:  "check",
-			args:  []string{"gitaly-hooks", "check"},
-			fixed: false,
-		},
-		{
-			desc:  "check with arguments",
-			args:  []string{"gitaly-hooks", "check", "foo", "bar"},
-			fixed: false,
-		},
-		{
-			desc:  "pre-receive hook without arguments",
-			args:  []string{"gitaly-hooks", "pre-receive"},
-			fixed: false,
-		},
-		{
-			desc:  "post-receive hook without arguments",
-			args:  []string{"gitaly-hooks", "post-receive"},
-			fixed: false,
-		},
-		{
-			desc:  "update hook without arguments",
-			args:  []string{"gitaly-hooks", "update"},
-			fixed: false,
-		},
-		{
-			desc:  "update with typical arguments",
-			args:  []string{"gitaly-hooks", "update", "refs/heads/master", "foo", "bar"},
-			fixed: false,
-		},
-		{
-			desc:  "update with prepared argument",
-			args:  []string{"gitaly-hooks", "update", "prepared", "foo"},
-			fixed: false,
-		},
-		{
-			desc:  "wrongly called update hook",
-			args:  []string{"gitaly-hooks", "update", "prepared"},
-			fixed: true,
-		},
-		{
-			desc:  "wrongly called pre-receive hook",
-			args:  []string{"gitaly-hooks", "pre-receive", "committed"},
-			fixed: true,
-		},
-		{
-			desc:  "wrongly called post-receive hook",
-			args:  []string{"gitaly-hooks", "post-receive", "aborted"},
-			fixed: true,
-		},
-		{
-			desc:  "unrelated hook",
-			args:  []string{"gitaly-hooks", "pre-commit", "prepared"},
-			fixed: false,
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.desc, func(t *testing.T) {
-			actual := fixupReftxHook(tc.args)
-
-			expected := make([]string, len(tc.args))
-			copy(expected, tc.args)
-			if tc.fixed {
-				expected[1] = "reference-transaction"
-			}
-
-			require.Equal(t, actual, expected)
-		})
-	}
-}
-
 func TestHooksPrePostWithSymlinkedStoragePath(t *testing.T) {
 	defer func(cfg config.Cfg) {
 		config.Config = cfg