Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
path: root/internal/gitaly/service
diff options
context:
space:
mode:
Diffstat (limited to 'internal/gitaly/service')
-rw-r--r--internal/gitaly/service/operations/rebase.go45
-rw-r--r--internal/gitaly/service/operations/rebase_test.go60
-rw-r--r--internal/gitaly/service/repository/archive.go16
-rw-r--r--internal/gitaly/service/smarthttp/server.go3
-rw-r--r--internal/gitaly/service/ssh/receive_pack_test.go86
5 files changed, 140 insertions, 70 deletions
diff --git a/internal/gitaly/service/operations/rebase.go b/internal/gitaly/service/operations/rebase.go
index 30c4438cb..e23a18fe9 100644
--- a/internal/gitaly/service/operations/rebase.go
+++ b/internal/gitaly/service/operations/rebase.go
@@ -9,6 +9,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
+ "gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
@@ -67,6 +68,34 @@ func (s *Server) UserRebaseConfirmable(stream gitalypb.OperationService_UserReba
SkipEmptyCommits: true,
})
if err != nil {
+ if featureflag.UserRebaseConfirmableImprovedErrorHandling.IsEnabled(ctx) {
+ var conflictErr git2go.ConflictingFilesError
+ if errors.As(err, &conflictErr) {
+ conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFiles))
+ for _, conflictingFile := range conflictErr.ConflictingFiles {
+ conflictingFiles = append(conflictingFiles, []byte(conflictingFile))
+ }
+
+ detailedErr, err := helper.ErrWithDetails(
+ helper.ErrFailedPreconditionf("rebasing commits: %w", err),
+ &gitalypb.UserRebaseConfirmableError{
+ Error: &gitalypb.UserRebaseConfirmableError_RebaseConflict{
+ RebaseConflict: &gitalypb.MergeConflictError{
+ ConflictingFiles: conflictingFiles,
+ },
+ },
+ },
+ )
+ if err != nil {
+ return helper.ErrInternalf("error details: %w", err)
+ }
+
+ return detailedErr
+ }
+
+ return helper.ErrInternalf("rebasing commits: %w", err)
+ }
+
return stream.Send(&gitalypb.UserRebaseConfirmableResponse{
GitError: err.Error(),
})
@@ -100,6 +129,22 @@ func (s *Server) UserRebaseConfirmable(stream gitalypb.OperationService_UserReba
header.GitPushOptions...); err != nil {
switch {
case errors.As(err, &updateref.HookError{}):
+ if featureflag.UserRebaseConfirmableImprovedErrorHandling.IsEnabled(ctx) {
+ detailedErr, err := helper.ErrWithDetails(
+ helper.ErrPermissionDeniedf("access check: %q", err),
+ &gitalypb.UserRebaseConfirmableError{
+ Error: &gitalypb.UserRebaseConfirmableError_AccessCheck{
+ AccessCheck: &gitalypb.AccessCheckError{
+ ErrorMessage: err.Error(),
+ },
+ },
+ },
+ )
+ if err != nil {
+ return helper.ErrInternalf("error details: %w", err)
+ }
+ return detailedErr
+ }
return stream.Send(&gitalypb.UserRebaseConfirmableResponse{
PreReceiveError: err.Error(),
})
diff --git a/internal/gitaly/service/operations/rebase_test.go b/internal/gitaly/service/operations/rebase_test.go
index 1d388ded7..a43f084f3 100644
--- a/internal/gitaly/service/operations/rebase_test.go
+++ b/internal/gitaly/service/operations/rebase_test.go
@@ -1,6 +1,8 @@
package operations
import (
+ "context"
+ "errors"
"fmt"
"io"
"path/filepath"
@@ -14,7 +16,9 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
+ "gitlab.com/gitlab-org/gitaly/v14/internal/helper"
"gitlab.com/gitlab-org/gitaly/v14/internal/metadata"
+ "gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testserver"
"gitlab.com/gitlab-org/gitaly/v14/internal/transaction/txinfo"
@@ -507,8 +511,11 @@ func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) {
func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) {
t.Parallel()
- ctx := testhelper.Context(t)
+ testhelper.NewFeatureSets(featureflag.UserRebaseConfirmableImprovedErrorHandling).
+ Run(t, testFailedUserRebaseConfirmableRequestDueToPreReceiveError)
+}
+func testFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -541,11 +548,23 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) {
secondResponse, err := rebaseStream.Recv()
- require.NoError(t, err, "receive second response")
- require.Contains(t, secondResponse.PreReceiveError, "failure")
-
- _, err = rebaseStream.Recv()
- require.Equal(t, io.EOF, err)
+ if featureflag.UserRebaseConfirmableImprovedErrorHandling.IsEnabled(ctx) {
+ testhelper.RequireGrpcError(t, errWithDetails(t,
+ helper.ErrPermissionDeniedf(`access check: "failure\n"`),
+ &gitalypb.UserRebaseConfirmableError{
+ Error: &gitalypb.UserRebaseConfirmableError_AccessCheck{
+ AccessCheck: &gitalypb.AccessCheckError{
+ ErrorMessage: "failure\n",
+ },
+ },
+ },
+ ), err)
+ } else {
+ require.NoError(t, err, "receive second response")
+ require.Contains(t, secondResponse.PreReceiveError, "failure")
+ _, err = rebaseStream.Recv()
+ require.Equal(t, io.EOF, err)
+ }
_, err = repo.ReadCommit(ctx, git.Revision(firstResponse.GetRebaseSha()))
if hookName == "pre-receive" {
@@ -561,10 +580,13 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) {
}
}
-func TestFailedUserRebaseConfirmableDueToGitError(t *testing.T) {
+func TestUserRebaseConfirmable_gitError(t *testing.T) {
t.Parallel()
- ctx := testhelper.Context(t)
+ testhelper.NewFeatureSets(featureflag.UserRebaseConfirmableImprovedErrorHandling).
+ Run(t, testFailedUserRebaseConfirmableDueToGitError)
+}
+func testFailedUserRebaseConfirmableDueToGitError(t *testing.T, ctx context.Context) {
ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
repoCopyProto, _ := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
@@ -578,14 +600,28 @@ func TestFailedUserRebaseConfirmableDueToGitError(t *testing.T) {
require.NoError(t, err)
headerRequest := buildHeaderRequest(repoProto, gittest.TestUser, "1", failedBranchName, branchSha, repoCopyProto, "master")
+
require.NoError(t, rebaseStream.Send(headerRequest), "send header")
firstResponse, err := rebaseStream.Recv()
- require.NoError(t, err, "receive first response")
- require.Contains(t, firstResponse.GitError, "conflict")
+ if featureflag.UserRebaseConfirmableImprovedErrorHandling.IsEnabled(ctx) {
+ testhelper.RequireGrpcError(t, errWithDetails(t,
+ helper.ErrFailedPrecondition(errors.New(`rebasing commits: rebase: commit "eb8f5fb9523b868cef583e09d4bf70b99d2dd404": there are conflicting files`)),
+ &gitalypb.UserRebaseConfirmableError{
+ Error: &gitalypb.UserRebaseConfirmableError_RebaseConflict{
+ RebaseConflict: &gitalypb.MergeConflictError{
+ ConflictingFiles: [][]byte{[]byte("README.md")},
+ },
+ },
+ },
+ ), err)
+ } else {
+ require.NoError(t, err, "receive first response")
+ require.Contains(t, firstResponse.GitError, "conflict")
- _, err = rebaseStream.Recv()
- require.Equal(t, io.EOF, err)
+ _, err = rebaseStream.Recv()
+ require.Equal(t, io.EOF, err)
+ }
newBranchSha := getBranchSha(t, cfg, repoPath, failedBranchName)
require.Equal(t, branchSha, newBranchSha, "branch should not change when the rebase fails due to GitError")
diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go
index cffd4fb36..878572f91 100644
--- a/internal/gitaly/service/repository/archive.go
+++ b/internal/gitaly/service/repository/archive.go
@@ -2,6 +2,8 @@ package repository
import (
"context"
+ "crypto/sha256"
+ "encoding/hex"
"encoding/json"
"fmt"
"io"
@@ -10,6 +12,7 @@ import (
"path/filepath"
"strings"
+ "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
"gitlab.com/gitlab-org/gitaly/v14/internal/command"
"gitlab.com/gitlab-org/gitaly/v14/internal/git"
"gitlab.com/gitlab-org/gitaly/v14/internal/git/catfile"
@@ -19,6 +22,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitaly/v14/streamio"
"gitlab.com/gitlab-org/labkit/correlation"
+ "google.golang.org/protobuf/proto"
)
type archiveParams struct {
@@ -92,6 +96,8 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo
return err
}
+ ctxlogrus.Extract(ctx).WithField("request_hash", requestHash(in)).Info("request details")
+
return s.handleArchive(archiveParams{
ctx: ctx,
writer: writer,
@@ -243,3 +249,13 @@ func (s *server) handleArchive(p archiveParams) error {
return archiveCommand.Wait()
}
+
+func requestHash(req proto.Message) string {
+ reqBytes, err := proto.Marshal(req)
+ if err != nil {
+ return "failed to hash request"
+ }
+
+ hash := sha256.Sum256(reqBytes)
+ return hex.EncodeToString(hash[:])
+}
diff --git a/internal/gitaly/service/smarthttp/server.go b/internal/gitaly/service/smarthttp/server.go
index 62134032a..e1a3c60d0 100644
--- a/internal/gitaly/service/smarthttp/server.go
+++ b/internal/gitaly/service/smarthttp/server.go
@@ -18,7 +18,8 @@ type server struct {
// NewServer creates a new instance of a grpc SmartHTTPServer
func NewServer(locator storage.Locator, gitCmdFactory git.CommandFactory,
- cache cache.Streamer, serverOpts ...ServerOpt) gitalypb.SmartHTTPServiceServer {
+ cache cache.Streamer, serverOpts ...ServerOpt,
+) gitalypb.SmartHTTPServiceServer {
s := &server{
locator: locator,
gitCmdFactory: gitCmdFactory,
diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go
index 90797458d..a810d6766 100644
--- a/internal/gitaly/service/ssh/receive_pack_test.go
+++ b/internal/gitaly/service/ssh/receive_pack_test.go
@@ -2,6 +2,7 @@ package ssh
import (
"bytes"
+ "context"
"fmt"
"io"
"os"
@@ -118,18 +119,16 @@ func TestReceivePackPushSuccess(t *testing.T) {
// when deserializing the HooksPayload. By setting all flags to `true` explicitly, we both
// verify that gitaly-ssh picks up feature flags correctly and fix the test to behave the
// same with and without Praefect.
- featureFlags := map[featureflag.FeatureFlag]bool{}
for _, featureFlag := range featureflag.All {
- featureFlags[featureFlag] = true
+ ctx = featureflag.ContextWithFeatureFlag(ctx, featureFlag, true)
}
- lHead, rHead, err := testCloneAndPush(t, cfg, cfg.SocketPath, repo, repoPath, pushParams{
+ lHead, rHead, err := testCloneAndPush(ctx, t, cfg, cfg.SocketPath, repo, repoPath, pushParams{
storageName: cfg.Storages[0].Name,
glID: "123",
glUsername: "user",
glRepository: glRepository,
glProjectPath: glProjectPath,
- featureFlags: featureFlags,
})
require.NoError(t, err)
require.Equal(t, lHead, rHead, "local and remote head not equal. push failed")
@@ -188,7 +187,7 @@ func TestReceivePackPushSuccessWithGitProtocol(t *testing.T) {
Seed: gittest.SeedGitLabTest,
})
- lHead, rHead, err := testCloneAndPush(t, cfg, cfg.SocketPath, repo, repoPath, pushParams{
+ lHead, rHead, err := testCloneAndPush(ctx, t, cfg, cfg.SocketPath, repo, repoPath, pushParams{
storageName: testhelper.DefaultStorageName,
glRepository: "project-123",
glID: "1",
@@ -205,14 +204,16 @@ func TestReceivePackPushSuccessWithGitProtocol(t *testing.T) {
func TestReceivePackPushFailure(t *testing.T) {
t.Parallel()
+ ctx := testhelper.Context(t)
+
cfg, repo, repoPath := testcfg.BuildWithRepo(t)
serverSocketPath := runSSHServer(t, cfg)
- _, _, err := testCloneAndPush(t, cfg, serverSocketPath, repo, repoPath, pushParams{storageName: "foobar", glID: "1"})
+ _, _, err := testCloneAndPush(ctx, t, cfg, serverSocketPath, repo, repoPath, pushParams{storageName: "foobar", glID: "1"})
require.Error(t, err, "local and remote head equal. push did not fail")
- _, _, err = testCloneAndPush(t, cfg, serverSocketPath, repo, repoPath, pushParams{storageName: cfg.Storages[0].Name, glID: ""})
+ _, _, err = testCloneAndPush(ctx, t, cfg, serverSocketPath, repo, repoPath, pushParams{storageName: cfg.Storages[0].Name, glID: ""})
require.Error(t, err, "local and remote head equal. push did not fail")
}
@@ -234,7 +235,7 @@ func TestReceivePackPushHookFailure(t *testing.T) {
hookContent := []byte("#!/bin/sh\nexit 1")
require.NoError(t, os.WriteFile(filepath.Join(gitCmdFactory.HooksPath(ctx), "pre-receive"), hookContent, 0o755))
- _, _, err := testCloneAndPush(t, cfg, cfg.SocketPath, repo, repoPath, pushParams{storageName: cfg.Storages[0].Name, glID: "1"})
+ _, _, err := testCloneAndPush(ctx, t, cfg, cfg.SocketPath, repo, repoPath, pushParams{storageName: cfg.Storages[0].Name, glID: "1"})
require.Error(t, err)
require.Contains(t, err.Error(), "(pre-receive hook declined)")
}
@@ -550,7 +551,7 @@ func TestSSHReceivePackToHooks(t *testing.T) {
gittest.WriteCheckNewObjectExistsHook(t, cloneDetails.RemoteRepoPath)
- lHead, rHead, err := sshPush(t, cfg, cloneDetails, cfg.SocketPath, pushParams{
+ lHead, rHead, err := sshPush(ctx, t, cfg, cloneDetails, cfg.SocketPath, pushParams{
storageName: cfg.Storages[0].Name,
glID: glID,
glRepository: glRepository,
@@ -572,18 +573,23 @@ type SSHCloneDetails struct {
// setupSSHClone sets up a test clone
func setupSSHClone(t *testing.T, cfg config.Cfg, remoteRepo *gitalypb.Repository, remoteRepoPath string) (SSHCloneDetails, func()) {
- // Make a non-bare clone of the test repo to act as a local one
- _, localRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0], gittest.CloneRepoOpts{
- WithWorktree: true,
- })
-
- // We need git thinking we're pushing over SSH...
- oldHead, newHead, success := makeCommit(t, cfg, localRepoPath)
- require.True(t, success)
+ _, localRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
+
+ oldHead := text.ChompBytes(gittest.Exec(t, cfg, "-C", localRepoPath, "rev-parse", "HEAD"))
+ newHead := gittest.WriteCommit(t, cfg, localRepoPath,
+ gittest.WithMessage(fmt.Sprintf("Testing ReceivePack RPC around %d", time.Now().Unix())),
+ gittest.WithTreeEntries(gittest.TreeEntry{
+ Path: "foo.txt",
+ Mode: "100644",
+ Content: "foo bar",
+ }),
+ gittest.WithBranch("master"),
+ gittest.WithParents(git.ObjectID(oldHead)),
+ )
return SSHCloneDetails{
- OldHead: oldHead,
- NewHead: newHead,
+ OldHead: []byte(oldHead),
+ NewHead: []byte(newHead.String()),
LocalRepoPath: localRepoPath,
RemoteRepoPath: remoteRepoPath,
TempRepo: remoteRepo.GetRelativePath(),
@@ -593,7 +599,7 @@ func setupSSHClone(t *testing.T, cfg config.Cfg, remoteRepo *gitalypb.Repository
}
}
-func sshPush(t *testing.T, cfg config.Cfg, cloneDetails SSHCloneDetails, serverSocketPath string, params pushParams) (string, string, error) {
+func sshPush(ctx context.Context, t *testing.T, cfg config.Cfg, cloneDetails SSHCloneDetails, serverSocketPath string, params pushParams) (string, string, error) {
pbTempRepo := &gitalypb.Repository{
StorageName: params.storageName,
RelativePath: cloneDetails.TempRepo,
@@ -610,16 +616,11 @@ func sshPush(t *testing.T, cfg config.Cfg, cloneDetails SSHCloneDetails, serverS
})
require.NoError(t, err)
- featureFlags := []string{}
- for flag, value := range params.featureFlags {
- featureFlags = append(featureFlags, fmt.Sprintf("%s:%v", flag.Name, value))
- }
-
cmd := gittest.NewCommand(t, cfg, "-C", cloneDetails.LocalRepoPath, "push", "-v", "git@localhost:test/test.git", "master")
cmd.Env = []string{
fmt.Sprintf("GITALY_PAYLOAD=%s", payload),
fmt.Sprintf("GITALY_ADDRESS=%s", serverSocketPath),
- fmt.Sprintf("GITALY_FEATUREFLAGS=%s", strings.Join(featureFlags, ",")),
+ fmt.Sprintf("GITALY_FEATUREFLAGS=%s", strings.Join(featureflag.AllFlags(ctx), ",")),
fmt.Sprintf("GIT_SSH_COMMAND=%s receive-pack", filepath.Join(cfg.BinDir, "gitaly-ssh")),
}
@@ -638,39 +639,11 @@ func sshPush(t *testing.T, cfg config.Cfg, cloneDetails SSHCloneDetails, serverS
return string(localHead), string(remoteHead), nil
}
-func testCloneAndPush(t *testing.T, cfg config.Cfg, serverSocketPath string, remoteRepo *gitalypb.Repository, remoteRepoPath string, params pushParams) (string, string, error) {
+func testCloneAndPush(ctx context.Context, t *testing.T, cfg config.Cfg, serverSocketPath string, remoteRepo *gitalypb.Repository, remoteRepoPath string, params pushParams) (string, string, error) {
cloneDetails, cleanup := setupSSHClone(t, cfg, remoteRepo, remoteRepoPath)
defer cleanup()
- return sshPush(t, cfg, cloneDetails, serverSocketPath, params)
-}
-
-// makeCommit creates a new commit and returns oldHead, newHead, success
-func makeCommit(t *testing.T, cfg config.Cfg, localRepoPath string) ([]byte, []byte, bool) {
- commitMsg := fmt.Sprintf("Testing ReceivePack RPC around %d", time.Now().Unix())
- committerName := "Scrooge McDuck"
- committerEmail := "scrooge@mcduck.com"
- newFilePath := localRepoPath + "/foo.txt"
-
- // Create a tiny file and add it to the index
- require.NoError(t, os.WriteFile(newFilePath, []byte("foo bar"), 0o644))
- gittest.Exec(t, cfg, "-C", localRepoPath, "add", ".")
-
- // The latest commit ID on the remote repo
- oldHead := bytes.TrimSpace(gittest.Exec(t, cfg, "-C", localRepoPath, "rev-parse", "master"))
-
- gittest.Exec(t, cfg, "-C", localRepoPath,
- "-c", fmt.Sprintf("user.name=%s", committerName),
- "-c", fmt.Sprintf("user.email=%s", committerEmail),
- "commit", "-m", commitMsg)
- if t.Failed() {
- return nil, nil, false
- }
-
- // The commit ID we want to push to the remote repo
- newHead := bytes.TrimSpace(gittest.Exec(t, cfg, "-C", localRepoPath, "rev-parse", "master"))
-
- return oldHead, newHead, true
+ return sshPush(ctx, t, cfg, cloneDetails, serverSocketPath, params)
}
func drainPostReceivePackResponse(stream gitalypb.SSHService_SSHReceivePackClient) error {
@@ -689,5 +662,4 @@ type pushParams struct {
glProjectPath string
gitConfigOptions []string
gitProtocol string
- featureFlags map[featureflag.FeatureFlag]bool
}
generated by cgit v1.2.3 (git 2.25.1) at 2024-08-03 03:01:33 +0300