operations: Remove update submodule feature flag

The update submodule RPC has been enabled by default, and no known bugs
where spotted since. This change removes the feature flag so the Ruby
code can be removed in a few weeks.

4 files changed, 29 insertions, 96 deletions

diff --git a/internal/gitaly/service/operations/submodules.go b/internal/gitaly/service/operations/submodules.go
index fc5f79bf1..828569c9b 100644
--- a/internal/gitaly/service/operations/submodules.go
+++ b/internal/gitaly/service/operations/submodules.go
@@ -12,9 +12,7 @@ import (
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git2go"
-	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -27,21 +25,7 @@ func (s *Server) UserUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpda
 		return nil, status.Errorf(codes.InvalidArgument, userUpdateSubmoduleName+": %v", err)
 	}
 
-	if featureflag.IsEnabled(ctx, featureflag.GoUserUpdateSubmodule) {
-		return s.userUpdateSubmodule(ctx, req)
-	}
-
-	client, err := s.ruby.OperationServiceClient(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	clientCtx, err := rubyserver.SetHeaders(ctx, s.locator, req.GetRepository())
-	if err != nil {
-		return nil, err
-	}
-
-	return client.UserUpdateSubmodule(clientCtx, req)
+	return s.userUpdateSubmodule(ctx, req)
 }
 
 func validateUserUpdateSubmoduleRequest(req *gitalypb.UserUpdateSubmoduleRequest) error {
diff --git a/internal/gitaly/service/operations/submodules_test.go b/internal/gitaly/service/operations/submodules_test.go
index 454c40f46..c2d68c0f0 100644
--- a/internal/gitaly/service/operations/submodules_test.go
+++ b/internal/gitaly/service/operations/submodules_test.go
@@ -2,7 +2,6 @@ package operations
 
 import (
 	"bytes"
-	"context"
 	"fmt"
 	"testing"
 
@@ -12,24 +11,16 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/internal/git/lstree"
-	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
-	"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, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets(
-		[]featureflag.FeatureFlag{featureflag.GoUserUpdateSubmodule},
-	).Run(t, func(t *testing.T, ctx context.Context) {
-		testSuccessfulUserUpdateSubmoduleRequestFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+func TestSuccessfulUserUpdateSubmoduleRequest(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
-func testSuccessfulUserUpdateSubmoduleRequestFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, cfg, repoProto, repoPath, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
+	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
@@ -106,17 +97,11 @@ func testSuccessfulUserUpdateSubmoduleRequestFeatured(t *testing.T, ctx context.
 	}
 }
 
-func testUserUpdateSubmoduleStableID(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets(
-		[]featureflag.FeatureFlag{featureflag.GoUserUpdateSubmodule},
-	).Run(t, func(t *testing.T, ctx context.Context) {
-		testUserUpdateSubmoduleStableIDFeatured(t, ctx, cfg, rubySrv)
-	})
-}
-
-func testUserUpdateSubmoduleStableIDFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, cfg, repoProto, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
+func TestUserUpdateSubmoduleStableID(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
+	ctx, cfg, repoProto, _, client := setupOperationsService(t, ctx)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
 	response, err := client.UserUpdateSubmodule(ctx, &gitalypb.UserUpdateSubmoduleRequest{
@@ -158,16 +143,11 @@ func testUserUpdateSubmoduleStableIDFeatured(t *testing.T, ctx context.Context,
 	}, commit)
 }
 
-func testFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets(
-		[]featureflag.FeatureFlag{featureflag.GoUserUpdateSubmodule},
-	).Run(t, func(t *testing.T, ctx context.Context) {
-		testFailedUserUpdateSubmoduleRequestDueToValidationsFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
-func testFailedUserUpdateSubmoduleRequestDueToValidationsFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, _, repo, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
+	ctx, _, repo, _, client := setupOperationsService(t, ctx)
 
 	testCases := []struct {
 		desc    string
@@ -281,16 +261,11 @@ func testFailedUserUpdateSubmoduleRequestDueToValidationsFeatured(t *testing.T,
 	}
 }
 
-func testFailedUserUpdateSubmoduleRequestDueToInvalidBranch(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets(
-		[]featureflag.FeatureFlag{featureflag.GoUserUpdateSubmodule},
-	).Run(t, func(t *testing.T, ctx context.Context) {
-		testFailedUserUpdateSubmoduleRequestDueToInvalidBranchFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+func TestFailedUserUpdateSubmoduleRequestDueToInvalidBranch(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
-func testFailedUserUpdateSubmoduleRequestDueToInvalidBranchFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, _, repo, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
+	ctx, _, repo, _, client := setupOperationsService(t, ctx)
 
 	request := &gitalypb.UserUpdateSubmoduleRequest{
 		Repository:    repo,
@@ -306,16 +281,11 @@ func testFailedUserUpdateSubmoduleRequestDueToInvalidBranchFeatured(t *testing.T
 	require.Contains(t, err.Error(), "Cannot find branch")
 }
 
-func testFailedUserUpdateSubmoduleRequestDueToInvalidSubmodule(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets(
-		[]featureflag.FeatureFlag{featureflag.GoUserUpdateSubmodule},
-	).Run(t, func(t *testing.T, ctx context.Context) {
-		testFailedUserUpdateSubmoduleRequestDueToInvalidSubmoduleFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+func TestFailedUserUpdateSubmoduleRequestDueToInvalidSubmodule(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
-func testFailedUserUpdateSubmoduleRequestDueToInvalidSubmoduleFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, _, repo, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
+	ctx, _, repo, _, client := setupOperationsService(t, ctx)
 
 	request := &gitalypb.UserUpdateSubmoduleRequest{
 		Repository:    repo,
@@ -331,16 +301,11 @@ func testFailedUserUpdateSubmoduleRequestDueToInvalidSubmoduleFeatured(t *testin
 	require.Equal(t, response.CommitError, "Invalid submodule path")
 }
 
-func testFailedUserUpdateSubmoduleRequestDueToSameReference(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets(
-		[]featureflag.FeatureFlag{featureflag.GoUserUpdateSubmodule},
-	).Run(t, func(t *testing.T, ctx context.Context) {
-		testFailedUserUpdateSubmoduleRequestDueToSameReferenceFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+func TestFailedUserUpdateSubmoduleRequestDueToSameReference(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
-func testFailedUserUpdateSubmoduleRequestDueToSameReferenceFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, _, repo, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
+	ctx, _, repo, _, client := setupOperationsService(t, ctx)
 
 	request := &gitalypb.UserUpdateSubmoduleRequest{
 		Repository:    repo,
@@ -359,16 +324,11 @@ func testFailedUserUpdateSubmoduleRequestDueToSameReferenceFeatured(t *testing.T
 	require.Contains(t, response.CommitError, "is already at")
 }
 
-func testFailedUserUpdateSubmoduleRequestDueToRepositoryEmpty(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	testhelper.NewFeatureSets(
-		[]featureflag.FeatureFlag{featureflag.GoUserUpdateSubmodule},
-	).Run(t, func(t *testing.T, ctx context.Context) {
-		testFailedUserUpdateSubmoduleRequestDueToRepositoryEmptyFeatured(t, ctx, cfg, rubySrv)
-	})
-}
+func TestFailedUserUpdateSubmoduleRequestDueToRepositoryEmpty(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
-func testFailedUserUpdateSubmoduleRequestDueToRepositoryEmptyFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, _, _, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
+	ctx, cfg, _, _, client := setupOperationsService(t, ctx)
 
 	repo, _, cleanup := gittest.InitRepoWithWorktreeAtStorage(t, cfg, cfg.Storages[0])
 	t.Cleanup(cleanup)
diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go
index c2c3560ad..973d09ead 100644
--- a/internal/gitaly/service/operations/testhelper_test.go
+++ b/internal/gitaly/service/operations/testhelper_test.go
@@ -85,13 +85,6 @@ func TestWithRubySidecar(t *testing.T) {
 		testRebaseRequestWithDeletedFile,
 		testRebaseOntoRemoteBranch,
 		testRebaseFailedWithCode,
-		testSuccessfulUserUpdateSubmoduleRequest,
-		testUserUpdateSubmoduleStableID,
-		testFailedUserUpdateSubmoduleRequestDueToValidations,
-		testFailedUserUpdateSubmoduleRequestDueToInvalidBranch,
-		testFailedUserUpdateSubmoduleRequestDueToInvalidSubmodule,
-		testFailedUserUpdateSubmoduleRequestDueToSameReference,
-		testFailedUserUpdateSubmoduleRequestDueToRepositoryEmpty,
 		testServerUserRevertFailedDueToCreateTreeErrorEmpty,
 	}
 	for _, f := range fs {
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index a79c89f84..61da0e274 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -17,9 +17,6 @@ var (
 	GoUserUpdateBranch = FeatureFlag{Name: "go_user_update_branch", OnByDefault: true}
 	// UserRebaseConfirmable
 	GoUserRebaseConfirmable = FeatureFlag{Name: "go_user_rebase_confirmable", OnByDefault: false}
-	// GoUserUpdateSubmodule enables the Go implementation of
-	// UserUpdateSubmodules
-	GoUserUpdateSubmodule = FeatureFlag{Name: "go_user_update_submodule", OnByDefault: true}
 	// GoUserRevert enables the Go implementation of UserRevert
 	GoUserRevert = FeatureFlag{Name: "go_user_revert", OnByDefault: true}
 	// GoUpdateRemoteMirror enables the Go implementation of UpdateRemoteMirror
@@ -36,7 +33,6 @@ var All = []FeatureFlag{
 	ReferenceTransactions,
 	GoUserUpdateBranch,
 	GoUserRebaseConfirmable,
-	GoUserUpdateSubmodule,
 	GoUserRevert,
 	GrpcTreeEntryNotFound,
 	GoUpdateRemoteMirror,