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
diff options
context:
space:
mode:
Diffstat (limited to 'internal')
-rw-r--r--internal/gitaly/service/ssh/server.go39
-rw-r--r--internal/gitaly/service/ssh/upload_archive.go3
-rw-r--r--internal/gitaly/service/ssh/upload_archive_test.go25
-rw-r--r--internal/gitaly/service/ssh/upload_pack.go2
-rw-r--r--internal/gitaly/service/ssh/upload_pack_test.go26
5 files changed, 72 insertions, 23 deletions
diff --git a/internal/gitaly/service/ssh/server.go b/internal/gitaly/service/ssh/server.go
index 12bc965cd..95188c11e 100644
--- a/internal/gitaly/service/ssh/server.go
+++ b/internal/gitaly/service/ssh/server.go
@@ -7,6 +7,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v15/internal/git"
"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction"
+ "gitlab.com/gitlab-org/gitaly/v15/internal/helper"
"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
)
@@ -17,12 +18,12 @@ var (
type server struct {
gitalypb.UnimplementedSSHServiceServer
- locator storage.Locator
- gitCmdFactory git.CommandFactory
- txManager transaction.Manager
- uploadPackRequestTimeout time.Duration
- uploadArchiveRequestTimeout time.Duration
- packfileNegotiationMetrics *prometheus.CounterVec
+ locator storage.Locator
+ gitCmdFactory git.CommandFactory
+ txManager transaction.Manager
+ uploadPackRequestTimeoutTickerFactory func() helper.Ticker
+ uploadArchiveRequestTimeoutTickerFactory func() helper.Ticker
+ packfileNegotiationMetrics *prometheus.CounterVec
}
// NewServer creates a new instance of a grpc SSHServer
@@ -33,11 +34,15 @@ func NewServer(
serverOpts ...ServerOpt,
) gitalypb.SSHServiceServer {
s := &server{
- locator: locator,
- gitCmdFactory: gitCmdFactory,
- txManager: txManager,
- uploadPackRequestTimeout: defaultUploadPackRequestTimeout,
- uploadArchiveRequestTimeout: defaultUploadArchiveRequestTimeout,
+ locator: locator,
+ gitCmdFactory: gitCmdFactory,
+ txManager: txManager,
+ uploadPackRequestTimeoutTickerFactory: func() helper.Ticker {
+ return helper.NewTimerTicker(defaultUploadPackRequestTimeout)
+ },
+ uploadArchiveRequestTimeoutTickerFactory: func() helper.Ticker {
+ return helper.NewTimerTicker(defaultUploadArchiveRequestTimeout)
+ },
packfileNegotiationMetrics: prometheus.NewCounterVec(
prometheus.CounterOpts{},
[]string{"git_negotiation_feature"},
@@ -54,17 +59,17 @@ func NewServer(
// ServerOpt is a self referential option for server
type ServerOpt func(s *server)
-// WithUploadPackRequestTimeout sets the upload pack request timeout
-func WithUploadPackRequestTimeout(d time.Duration) ServerOpt {
+// WithUploadPackRequestTimeoutTickerFactory sets the upload pack request timeout ticker factory.
+func WithUploadPackRequestTimeoutTickerFactory(factory func() helper.Ticker) ServerOpt {
return func(s *server) {
- s.uploadPackRequestTimeout = d
+ s.uploadPackRequestTimeoutTickerFactory = factory
}
}
-// WithArchiveRequestTimeout sets the upload pack request timeout
-func WithArchiveRequestTimeout(d time.Duration) ServerOpt {
+// WithArchiveRequestTimeoutTickerFactory sets the upload pack request timeout ticker factory.
+func WithArchiveRequestTimeoutTickerFactory(factory func() helper.Ticker) ServerOpt {
return func(s *server) {
- s.uploadArchiveRequestTimeout = d
+ s.uploadArchiveRequestTimeoutTickerFactory = factory
}
}
diff --git a/internal/gitaly/service/ssh/upload_archive.go b/internal/gitaly/service/ssh/upload_archive.go
index 8f087bb0b..610778943 100644
--- a/internal/gitaly/service/ssh/upload_archive.go
+++ b/internal/gitaly/service/ssh/upload_archive.go
@@ -10,7 +10,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v15/internal/git"
"gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline"
"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service"
- "gitlab.com/gitlab-org/gitaly/v15/internal/helper"
"gitlab.com/gitlab-org/gitaly/v15/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitaly/v15/streamio"
@@ -62,7 +61,7 @@ func (s *server) sshUploadArchive(stream gitalypb.SSHService_SSHUploadArchiveSer
return err
}
- timeoutTicker := helper.NewTimerTicker(s.uploadArchiveRequestTimeout)
+ timeoutTicker := s.uploadArchiveRequestTimeoutTickerFactory()
// upload-archive expects a list of options terminated by a flush packet:
// https://github.com/git/git/blob/v2.22.0/builtin/upload-archive.c#L38
diff --git a/internal/gitaly/service/ssh/upload_archive_test.go b/internal/gitaly/service/ssh/upload_archive_test.go
index 87be3beaa..350a972c9 100644
--- a/internal/gitaly/service/ssh/upload_archive_test.go
+++ b/internal/gitaly/service/ssh/upload_archive_test.go
@@ -4,11 +4,11 @@ import (
"fmt"
"os"
"testing"
- "time"
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitaly/v15/internal/git"
"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
+ "gitlab.com/gitlab-org/gitaly/v15/internal/helper"
"gitlab.com/gitlab-org/gitaly/v15/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
@@ -23,7 +23,23 @@ func TestFailedUploadArchiveRequestDueToTimeout(t *testing.T) {
cfg := testcfg.Build(t)
- cfg.SocketPath = runSSHServerWithOptions(t, cfg, []ServerOpt{WithArchiveRequestTimeout(100 * time.Microsecond)})
+ // Use a ticker channel so that we can observe that the ticker is being created. The channel
+ // is unbuffered on purpose so that we can assert that it is getting created exactly at the
+ // time we expect it to be.
+ tickerCh := make(chan *helper.ManualTicker)
+
+ cfg.SocketPath = runSSHServerWithOptions(t, cfg, []ServerOpt{
+ WithArchiveRequestTimeoutTickerFactory(func() helper.Ticker {
+ // Create a ticker that will immediately tick when getting reset so that the
+ // server-side can observe this as an emulated timeout.
+ ticker := helper.NewManualTicker()
+ ticker.ResetFunc = func() {
+ ticker.Tick()
+ }
+ tickerCh <- ticker
+ return ticker
+ }),
+ })
ctx := testhelper.Context(t)
repo, _ := gittest.CreateRepository(t, ctx, cfg)
@@ -36,6 +52,11 @@ func TestFailedUploadArchiveRequestDueToTimeout(t *testing.T) {
// The first request is not limited by timeout, but also not under attacker control
require.NoError(t, stream.Send(&gitalypb.SSHUploadArchiveRequest{Repository: repo}))
+ // We should now see that the ticker limiting the request is being created. We don't need to
+ // use the ticker, but this statement is only there in order to verify that the ticker is
+ // indeed getting created at the expected point in time.
+ <-tickerCh
+
// Because the client says nothing, the server would block. Because of
// the timeout, it won't block forever, and return with a non-zero exit
// code instead.
diff --git a/internal/gitaly/service/ssh/upload_pack.go b/internal/gitaly/service/ssh/upload_pack.go
index 7542c3a2a..f08482d39 100644
--- a/internal/gitaly/service/ssh/upload_pack.go
+++ b/internal/gitaly/service/ssh/upload_pack.go
@@ -137,7 +137,7 @@ func (s *server) sshUploadPack(rpcContext context.Context, req sshUploadPackRequ
return 0, err
}
- timeoutTicker := helper.NewTimerTicker(s.uploadPackRequestTimeout)
+ timeoutTicker := s.uploadPackRequestTimeoutTickerFactory()
// upload-pack negotiation is terminated by either a flush, or the "done"
// packet: https://github.com/git/git/blob/v2.20.0/Documentation/technical/pack-protocol.txt#L335
diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go
index e6376e256..a40ddfb51 100644
--- a/internal/gitaly/service/ssh/upload_pack_test.go
+++ b/internal/gitaly/service/ssh/upload_pack_test.go
@@ -19,6 +19,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v15/internal/git"
"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
+ "gitlab.com/gitlab-org/gitaly/v15/internal/helper"
"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
@@ -97,10 +98,28 @@ func TestUploadPack_timeout(t *testing.T) {
}
func testUploadPackTimeout(t *testing.T, opts ...testcfg.Option) {
+ t.Parallel()
+
ctx := testhelper.Context(t)
cfg := testcfg.Build(t, opts...)
- cfg.SocketPath = runSSHServerWithOptions(t, cfg, []ServerOpt{WithUploadPackRequestTimeout(1)})
+ // Use a ticker channel so that we can observe that the ticker is being created. The channel
+ // is unbuffered on purpose so that we can assert that it is getting created exactly at the
+ // time we expect it to be.
+ tickerCh := make(chan *helper.ManualTicker)
+
+ cfg.SocketPath = runSSHServerWithOptions(t, cfg, []ServerOpt{
+ WithUploadPackRequestTimeoutTickerFactory(func() helper.Ticker {
+ // Create a ticker that will immediately tick when getting reset so that the
+ // server-side can observe this as an emulated timeout.
+ ticker := helper.NewManualTicker()
+ ticker.ResetFunc = func() {
+ ticker.Tick()
+ }
+ tickerCh <- ticker
+ return ticker
+ }),
+ })
repo, repoPath := gittest.CreateRepository(t, testhelper.Context(t), cfg)
gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
@@ -113,6 +132,11 @@ func testUploadPackTimeout(t *testing.T, opts ...testcfg.Option) {
// The first request is not limited by timeout, but also not under attacker control
require.NoError(t, stream.Send(&gitalypb.SSHUploadPackRequest{Repository: repo}))
+ // We should now see that the ticker limiting the request is being created. We don't need to
+ // use the ticker, but this statement is only there in order to verify that the ticker is
+ // indeed getting created at the expected point in time.
+ <-tickerCh
+
// Because the client says nothing, the server would block. Because of
// the timeout, it won't block forever, and return with a non-zero exit
// code instead.