Turn on go prereceive and go update by default

15 files changed, 37 insertions, 139 deletions

diff --git a/changelogs/unreleased/jc-go-update-default-on.yml b/changelogs/unreleased/jc-go-update-default-on.yml
new file mode 100644
index 000000000..9cf0b9ea5
--- /dev/null
+++ b/changelogs/unreleased/jc-go-update-default-on.yml
@@ -0,0 +1,5 @@
+---
+title: Turn on go prereceive and go update by default
+merge_request: 2329
+author:
+type: changed
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index 6f84cc18e..2b075bc91 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -9,7 +9,7 @@ var (
 	// LinguistFileCountStats will invoke an additional git-linguist command to get the number of files per language
 	LinguistFileCountStats = FeatureFlag{Name: "linguist_file_count_stats", OnByDefault: false}
 	// GoUpdateHook will bypass the ruby update hook and use the go implementation of custom hooks
-	GoUpdateHook = FeatureFlag{Name: "go_update_hook", OnByDefault: false}
+	GoUpdateHook = FeatureFlag{Name: "go_update_hook", OnByDefault: true}
 	// RemoteBranchesLsRemote will use `ls-remote` for remote branches
 	RemoteBranchesLsRemote = FeatureFlag{Name: "ruby_remote_branches_ls_remote", OnByDefault: true}
 	// GoFetchSourceBranch enables a go implementation of FetchSourceBranch
@@ -19,7 +19,7 @@ var (
 	// DistributedReads allows praefect to redirect accessor operations to up-to-date secondaries
 	DistributedReads = FeatureFlag{Name: "distributed_reads", OnByDefault: false}
 	// GoPreReceiveHook will bypass the ruby pre-receive hook and use the go implementation
-	GoPreReceiveHook = FeatureFlag{Name: "go_prereceive_hook", OnByDefault: false}
+	GoPreReceiveHook = FeatureFlag{Name: "go_prereceive_hook", OnByDefault: true}
 )
 
 const (
diff --git a/internal/service/operations/apply_patch_test.go b/internal/service/operations/apply_patch_test.go
index 309bc2b0d..1b24b2391 100644
--- a/internal/service/operations/apply_patch_test.go
+++ b/internal/service/operations/apply_patch_test.go
@@ -12,7 +12,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/streamio"
@@ -20,17 +19,10 @@ import (
 )
 
 func TestSuccessfulUserApplyPatch(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulUserApplyPatch(t, ctx)
-		})
-	}
+	testSuccessfulUserApplyPatch(t, ctx)
 }
 
 func testSuccessfulUserApplyPatch(t *testing.T, ctx context.Context) {
diff --git a/internal/service/operations/branches_test.go b/internal/service/operations/branches_test.go
index 0d8b94ccf..e290e3948 100644
--- a/internal/service/operations/branches_test.go
+++ b/internal/service/operations/branches_test.go
@@ -7,7 +7,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -76,17 +75,10 @@ func TestSuccessfulCreateBranchRequest(t *testing.T) {
 }
 
 func TestSuccessfulGitHooksForUserCreateBranchRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulGitHooksForUserCreateBranchRequest(t, ctx)
-		})
-	}
+	testSuccessfulGitHooksForUserCreateBranchRequest(t, ctx)
 }
 
 func testSuccessfulGitHooksForUserCreateBranchRequest(t *testing.T, ctx context.Context) {
diff --git a/internal/service/operations/cherry_pick_test.go b/internal/service/operations/cherry_pick_test.go
index 3a30688b7..6bbb68acb 100644
--- a/internal/service/operations/cherry_pick_test.go
+++ b/internal/service/operations/cherry_pick_test.go
@@ -6,7 +6,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -117,17 +116,10 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) {
 }
 
 func TestSuccessfulGitHooksForUserCherryPickRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulGitHooksForUserCherryPickRequest(t, ctx)
-		})
-	}
+	testSuccessfulGitHooksForUserCherryPickRequest(t, ctx)
 }
 
 func testSuccessfulGitHooksForUserCherryPickRequest(t *testing.T, ctxOuter context.Context) {
diff --git a/internal/service/operations/commit_files_test.go b/internal/service/operations/commit_files_test.go
index 2aceef1ba..391ba9c24 100644
--- a/internal/service/operations/commit_files_test.go
+++ b/internal/service/operations/commit_files_test.go
@@ -9,7 +9,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -127,17 +126,10 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctxWithFeatureFlags cont
 }
 
 func TestSuccessfulUserCommitFilesRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulUserCommitFilesRequest(t, ctx)
-		})
-	}
+	testSuccessfulUserCommitFilesRequest(t, ctx)
 }
 
 func TestSuccessfulUserCommitFilesRequestMove(t *testing.T) {
diff --git a/internal/service/operations/merge_test.go b/internal/service/operations/merge_test.go
index ea2a5997e..2c60c3718 100644
--- a/internal/service/operations/merge_test.go
+++ b/internal/service/operations/merge_test.go
@@ -12,7 +12,6 @@ import (
 	"github.com/stretchr/testify/require"
 	gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -25,17 +24,10 @@ var (
 )
 
 func TestSuccessfulMerge(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulMerge(t, ctx)
-		})
-	}
+	testSuccessfulMerge(t, ctx)
 }
 
 func testSuccessfulMerge(t *testing.T, ctx context.Context) {
diff --git a/internal/service/operations/rebase_test.go b/internal/service/operations/rebase_test.go
index a175c4324..a7d0aeb46 100644
--- a/internal/service/operations/rebase_test.go
+++ b/internal/service/operations/rebase_test.go
@@ -11,7 +11,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -24,8 +23,6 @@ var (
 )
 
 func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoPreReceiveHook, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
@@ -41,12 +38,7 @@ func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) {
 	serverSocketPath, stop := runOperationServiceServerWithRubyServer(t, &ruby)
 	defer stop()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulUserRebaseConfirmableRequest(t, ctx, serverSocketPath, pushOptions)
-		})
-	}
+	testSuccessfulUserRebaseConfirmableRequest(t, ctx, serverSocketPath, pushOptions)
 }
 
 func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctxOuter context.Context, serverSocketPath string, pushOptions []string) {
diff --git a/internal/service/operations/revert_test.go b/internal/service/operations/revert_test.go
index 4f251ffe7..392255087 100644
--- a/internal/service/operations/revert_test.go
+++ b/internal/service/operations/revert_test.go
@@ -2,12 +2,10 @@ package operations
 
 import (
 	"context"
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -118,17 +116,10 @@ func TestSuccessfulUserRevertRequest(t *testing.T) {
 }
 
 func TestSuccessfulGitHooksForUserRevertRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulGitHooksForUserRevertRequest(t, ctx)
-		})
-	}
+	testSuccessfulGitHooksForUserRevertRequest(t, ctx)
 }
 
 func testSuccessfulGitHooksForUserRevertRequest(t *testing.T, ctxOuter context.Context) {
@@ -284,24 +275,19 @@ func TestFailedUserRevertRequestDueToPreReceiveError(t *testing.T) {
 
 	hookContent := []byte("#!/bin/sh\necho GL_ID=$GL_ID\nexit 1")
 
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoPreReceiveHook, featureflag.GoUpdateHook)
-	require.NoError(t, err)
-
 	for _, hookName := range GitlabPreHooks {
-		for _, features := range featureSet {
-			t.Run(fmt.Sprintf("%s, features: %v", hookName, features.String()), func(t *testing.T) {
-				remove, err := testhelper.WriteCustomHook(testRepoPath, hookName, hookContent)
-				require.NoError(t, err)
-				defer remove()
-
-				md := testhelper.GitalyServersMetadata(t, serverSocketPath)
-				ctx := metadata.NewOutgoingContext(ctxOuter, md)
-
-				response, err := client.UserRevert(features.WithParent(ctx), request)
-				require.NoError(t, err)
-				require.Contains(t, response.PreReceiveError, "GL_ID="+testhelper.TestUser.GlId)
-			})
-		}
+		t.Run(hookName, func(t *testing.T) {
+			remove, err := testhelper.WriteCustomHook(testRepoPath, hookName, hookContent)
+			require.NoError(t, err)
+			defer remove()
+
+			md := testhelper.GitalyServersMetadata(t, serverSocketPath)
+			ctx := metadata.NewOutgoingContext(ctxOuter, md)
+
+			response, err := client.UserRevert(ctx, request)
+			require.NoError(t, err)
+			require.Contains(t, response.PreReceiveError, "GL_ID="+testhelper.TestUser.GlId)
+		})
 	}
 }
 
diff --git a/internal/service/operations/squash_test.go b/internal/service/operations/squash_test.go
index 6a363ec9a..7164f8dc5 100644
--- a/internal/service/operations/squash_test.go
+++ b/internal/service/operations/squash_test.go
@@ -11,7 +11,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -29,17 +28,10 @@ var (
 )
 
 func TestSuccessfulUserSquashRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulUserSquashRequest(t, ctx)
-		})
-	}
+	testSuccessfulUserSquashRequest(t, ctx)
 }
 
 func testSuccessfulUserSquashRequest(t *testing.T, ctx context.Context) {
diff --git a/internal/service/operations/submodules_test.go b/internal/service/operations/submodules_test.go
index b59deb76d..e3de793b0 100644
--- a/internal/service/operations/submodules_test.go
+++ b/internal/service/operations/submodules_test.go
@@ -9,24 +9,16 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/git/lstree"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 )
 
 func TestSuccessfulUserUpdateSubmoduleRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulUserUpdateSubmoduleRequest(t, ctx)
-		})
-	}
+	testSuccessfulUserUpdateSubmoduleRequest(t, ctx)
 }
 
 func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context) {
diff --git a/internal/service/operations/tags_test.go b/internal/service/operations/tags_test.go
index 3def7c506..aeaa73d6a 100644
--- a/internal/service/operations/tags_test.go
+++ b/internal/service/operations/tags_test.go
@@ -11,7 +11,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -50,17 +49,10 @@ func TestSuccessfulUserDeleteTagRequest(t *testing.T) {
 }
 
 func TestSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulGitHooksForUserDeleteTagRequest(t, ctx)
-		})
-	}
+	testSuccessfulGitHooksForUserDeleteTagRequest(t, ctx)
 }
 
 func testSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T, ctx context.Context) {
@@ -195,17 +187,10 @@ func TestSuccessfulUserCreateTagRequest(t *testing.T) {
 }
 
 func TestSuccessfulGitHooksForUserCreateTagRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
-
 	ctx, cancel := testhelper.Context()
 	defer cancel()
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulGitHooksForUserCreateTagRequest(t, ctx)
-		})
-	}
+
+	testSuccessfulGitHooksForUserCreateTagRequest(t, ctx)
 }
 
 func testSuccessfulGitHooksForUserCreateTagRequest(t *testing.T, ctx context.Context) {
@@ -302,17 +287,10 @@ func TestFailedUserDeleteTagRequestDueToValidation(t *testing.T) {
 }
 
 func TestFailedUserDeleteTagDueToHooks(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
-
 	ctx, cancel := testhelper.Context()
 	defer cancel()
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testFailedUserDeleteTagDueToHooks(t, ctx)
-		})
-	}
+
+	testFailedUserDeleteTagDueToHooks(t, ctx)
 }
 
 func testFailedUserDeleteTagDueToHooks(t *testing.T, ctx context.Context) {
diff --git a/internal/service/operations/update_branches_test.go b/internal/service/operations/update_branches_test.go
index 978195c27..f151d9f69 100644
--- a/internal/service/operations/update_branches_test.go
+++ b/internal/service/operations/update_branches_test.go
@@ -53,17 +53,10 @@ func TestSuccessfulUserUpdateBranchRequest(t *testing.T) {
 }
 
 func TestSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T) {
-	featureSet, err := testhelper.NewFeatureSets(nil, featureflag.GoUpdateHook)
-	require.NoError(t, err)
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	for _, features := range featureSet {
-		t.Run(features.String(), func(t *testing.T) {
-			ctx = features.WithParent(ctx)
-			testSuccessfulGitHooksForUserUpdateBranchRequest(t, ctx)
-		})
-	}
+	testSuccessfulGitHooksForUserUpdateBranchRequest(t, ctx)
 }
 
 func testSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T, ctx context.Context) {
diff --git a/internal/service/smarthttp/receive_pack_test.go b/internal/service/smarthttp/receive_pack_test.go
index 713f9b419..17f3d7f14 100644
--- a/internal/service/smarthttp/receive_pack_test.go
+++ b/internal/service/smarthttp/receive_pack_test.go
@@ -467,7 +467,7 @@ func TestPostReceiveWithTransactions(t *testing.T) {
 	gitlabUser := "gitlab_user-1234"
 	gitlabPassword := "gitlabsecret9887"
 
-	featureSets, err := testhelper.NewFeatureSets([]featureflag.FeatureFlag{featureflag.ReferenceTransactions, featureflag.GoPreReceiveHook})
+	featureSets, err := testhelper.NewFeatureSets([]featureflag.FeatureFlag{featureflag.ReferenceTransactions})
 	require.NoError(t, err)
 
 	for _, features := range featureSets {
diff --git a/ruby/lib/gitlab/git/hook.rb b/ruby/lib/gitlab/git/hook.rb
index 7755ef925..78fa009fd 100644
--- a/ruby/lib/gitlab/git/hook.rb
+++ b/ruby/lib/gitlab/git/hook.rb
@@ -119,8 +119,8 @@ module Gitlab
           'GIT_DIR' => repo_path,
           'GITALY_REPO' => repository.gitaly_repository.to_json,
           'GITALY_SOCKET' => Gitlab.config.gitaly.internal_socket,
-          'GITALY_GO_UPDATE' => repository.feature_enabled?('go-update-hook').to_s,
-          'GITALY_GO_PRERECEIVE' => repository.feature_enabled?('go-prereceive-hook').to_s
+          'GITALY_GO_UPDATE' => repository.feature_enabled?('go-update-hook', on_by_default: true).to_s,
+          'GITALY_GO_PRERECEIVE' => repository.feature_enabled?('go-prereceive-hook', on_by_default: true).to_s
         }
       end
     end