diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go index f6f3e0e008bb59ad8fffb55a39197784208601a8..8b47e04ecdc2d1972129c67d17bd357bbe4c9cde 100644 --- a/internal/gitaly/service/operations/branches.go +++ b/internal/gitaly/service/operations/branches.go @@ -5,8 +5,6 @@ import ( "errors" "gitlab.com/gitlab-org/gitaly/internal/git" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" - "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" @@ -76,13 +74,6 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB }, nil } -func (s *Server) UserUpdateBranch(ctx context.Context, req *gitalypb.UserUpdateBranchRequest) (*gitalypb.UserUpdateBranchResponse, error) { - if featureflag.IsDisabled(ctx, featureflag.GoUserUpdateBranch) { - return s.userUpdateBranchRuby(ctx, req) - } - return s.userUpdateBranchGo(ctx, req) -} - func validateUserUpdateBranchGo(req *gitalypb.UserUpdateBranchRequest) error { if req.User == nil { return status.Errorf(codes.InvalidArgument, "empty user") @@ -103,7 +94,7 @@ func validateUserUpdateBranchGo(req *gitalypb.UserUpdateBranchRequest) error { return nil } -func (s *Server) userUpdateBranchGo(ctx context.Context, req *gitalypb.UserUpdateBranchRequest) (*gitalypb.UserUpdateBranchResponse, error) { +func (s *Server) UserUpdateBranch(ctx context.Context, req *gitalypb.UserUpdateBranchRequest) (*gitalypb.UserUpdateBranchResponse, error) { // Validate the request if err := validateUserUpdateBranchGo(req); err != nil { return nil, err @@ -141,20 +132,6 @@ func (s *Server) userUpdateBranchGo(ctx context.Context, req *gitalypb.UserUpdat return &gitalypb.UserUpdateBranchResponse{}, nil } -func (s *Server) userUpdateBranchRuby(ctx context.Context, req *gitalypb.UserUpdateBranchRequest) (*gitalypb.UserUpdateBranchResponse, error) { - 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.UserUpdateBranch(clientCtx, req) -} - func (s *Server) UserDeleteBranch(ctx context.Context, req *gitalypb.UserDeleteBranchRequest) (*gitalypb.UserDeleteBranchResponse, error) { // That we do the branch name & user check here first only in // UserDelete but not UserCreate is "intentional", i.e. it's diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go index db8338c066e4e147239bdea40a955ee50378ce54..ae8019253e1cbea74708fe442251267b824d6f7b 100644 --- a/internal/gitaly/service/operations/testhelper_test.go +++ b/internal/gitaly/service/operations/testhelper_test.go @@ -61,11 +61,6 @@ func TestWithRubySidecar(t *testing.T) { testSuccessfulUserApplyPatch, testUserApplyPatchStableID, testFailedPatchApplyPatch, - testSuccessfulUserUpdateBranchRequestToDelete, - testSuccessfulGitHooksForUserUpdateBranchRequest, - testFailedUserUpdateBranchDueToHooks, - testFailedUserUpdateBranchRequest, - testSuccessfulUserUpdateBranchRequest, testSuccessfulUserRebaseConfirmableRequest, testUserRebaseConfirmableTransaction, testUserRebaseConfirmableStableCommitIDs, diff --git a/internal/gitaly/service/operations/update_branches_test.go b/internal/gitaly/service/operations/update_branches_test.go index 32e89f0a319bfb070a323cc5d2e8d6d3ec4f9b85..6a27817533f4c00b7f1c922db924068ee2327b11 100644 --- a/internal/gitaly/service/operations/update_branches_test.go +++ b/internal/gitaly/service/operations/update_branches_test.go @@ -1,7 +1,6 @@ package operations import ( - "context" "crypto/sha1" "fmt" "testing" @@ -10,9 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" - "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" @@ -25,17 +21,11 @@ var ( oldrev = []byte("0b4bc9a49b562e85de7cc9e834518ea6828729b9") ) -func testSuccessfulUserUpdateBranchRequest(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) { - testhelper.NewFeatureSets([]featureflag.FeatureFlag{ - featureflag.ReferenceTransactions, - featureflag.GoUserUpdateBranch, - }).Run(t, func(t *testing.T, ctx context.Context) { - testSuccessfulUserUpdateBranchRequestFeatured(t, ctx, cfg, rubySrv) - }) -} +func TestSuccessfulUserUpdateBranchRequest(t *testing.T) { + ctx, cancel := testhelper.Context() + defer cancel() -func testSuccessfulUserUpdateBranchRequestFeatured(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,12 +96,11 @@ func testSuccessfulUserUpdateBranchRequestFeatured(t *testing.T, ctx context.Con } } -func testSuccessfulUserUpdateBranchRequestToDelete(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) { - testWithFeature(t, featureflag.GoUserUpdateBranch, cfg, rubySrv, testSuccessfulUserUpdateBranchRequestToDeleteFeatured) -} +func TestSuccessfulUserUpdateBranchRequestToDelete(t *testing.T) { + ctx, cancel := testhelper.Context() + defer cancel() -func testSuccessfulUserUpdateBranchRequestToDeleteFeatured(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) @@ -176,12 +165,11 @@ func testSuccessfulUserUpdateBranchRequestToDeleteFeatured(t *testing.T, ctx con } } -func testSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) { - testWithFeature(t, featureflag.GoUserUpdateBranch, cfg, rubySrv, testSuccessfulGitHooksForUserUpdateBranchRequestFeatured) -} +func TestSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T) { + ctx, cancel := testhelper.Context() + defer cancel() -func testSuccessfulGitHooksForUserUpdateBranchRequestFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) { - ctx, cfg, _, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv) + ctx, cfg, _, _, client := setupOperationsService(t, ctx) for _, hookName := range GitlabHooks { t.Run(hookName, func(t *testing.T) { @@ -210,12 +198,11 @@ func testSuccessfulGitHooksForUserUpdateBranchRequestFeatured(t *testing.T, ctx } } -func testFailedUserUpdateBranchDueToHooks(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) { - testWithFeature(t, featureflag.GoUserUpdateBranch, cfg, rubySrv, testFailedUserUpdateBranchDueToHooksFeatured) -} +func TestFailedUserUpdateBranchDueToHooks(t *testing.T) { + ctx, cancel := testhelper.Context() + defer cancel() -func testFailedUserUpdateBranchDueToHooksFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) { - ctx, _, repo, repoPath, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv) + ctx, _, repo, repoPath, client := setupOperationsService(t, ctx) request := &gitalypb.UserUpdateBranchRequest{ Repository: repo, @@ -243,12 +230,11 @@ func testFailedUserUpdateBranchDueToHooksFeatured(t *testing.T, ctx context.Cont } } -func testFailedUserUpdateBranchRequest(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) { - testWithFeature(t, featureflag.GoUserUpdateBranch, cfg, rubySrv, testFailedUserUpdateBranchRequestFeatured) -} +func TestFailedUserUpdateBranchRequest(t *testing.T) { + ctx, cancel := testhelper.Context() + defer cancel() -func testFailedUserUpdateBranchRequestFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) { - ctx, cfg, repoProto, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv) + ctx, cfg, repoProto, _, client := setupOperationsService(t, ctx) repo := localrepo.NewTestRepo(t, cfg, repoProto) diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go index f119aeade5795e820ffb7482dd9a0fd6f327e51b..88f3d69e1f42ee2859f55d5bc7c00045031960e8 100644 --- a/internal/metadata/featureflag/feature_flags.go +++ b/internal/metadata/featureflag/feature_flags.go @@ -11,8 +11,6 @@ type FeatureFlag struct { var ( // ReferenceTransactions will handle Git reference updates via the transaction service for strong consistency ReferenceTransactions = FeatureFlag{Name: "reference_transactions", OnByDefault: true} - // GoUserUpdateBranch enables the Go implementation of UserUpdateBranch - GoUserUpdateBranch = FeatureFlag{Name: "go_user_update_branch", OnByDefault: true} // UserRebaseConfirmable GoUserRebaseConfirmable = FeatureFlag{Name: "go_user_rebase_confirmable", OnByDefault: true} // GoUpdateRemoteMirror enables the Go implementation of UpdateRemoteMirror @@ -30,7 +28,6 @@ var ( // All includes all feature flags. var All = []FeatureFlag{ ReferenceTransactions, - GoUserUpdateBranch, GoUserRebaseConfirmable, GrpcTreeEntryNotFound, GoUpdateRemoteMirror,