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:
authorZeger-Jan van de Weg <git@zjvandeweg.nl>2021-01-18 15:02:09 +0300
committerZeger-Jan van de Weg <git@zjvandeweg.nl>2021-01-18 15:02:09 +0300
commitcebd5cc82779b53d67e528bf1f31816389475a88 (patch)
tree2506015f0bfe80c09bcd383ebd58599bc9d0b9a4
parent6dd2027292f0842be9accd7722ae8df53f5232e6 (diff)
parentadfa1692fd9dfde07b89609a05b8801a40c18fd2 (diff)
Merge branch 'master' into 'pks-revert-go-resolve-conflicts-default'
Conflicts: internal/metadata/featureflag/feature_flags.go
-rw-r--r--changelogs/unreleased/ps-cleanup-ruby-fetch-remote.yml5
-rw-r--r--internal/gitaly/service/conflicts/resolve_conflicts.go2
-rw-r--r--internal/gitaly/service/operations/commit_files.go2
-rw-r--r--internal/gitaly/service/register.go2
-rw-r--r--internal/gitaly/service/remote/fetch_internal_remote.go2
-rw-r--r--internal/gitaly/service/remote/server.go5
-rw-r--r--internal/gitaly/service/remote/testhelper_test.go2
-rw-r--r--internal/gitaly/service/repository/fetch.go2
-rw-r--r--internal/gitaly/service/repository/fetch_remote.go34
-rw-r--r--internal/gitaly/service/repository/fetch_remote_test.go459
-rw-r--r--internal/gitaly/service/repository/fork.go2
-rw-r--r--internal/gitalyssh/gitalyssh.go13
-rw-r--r--internal/gitalyssh/gitalyssh_test.go5
-rw-r--r--internal/metadata/featureflag/feature_flags.go1
-rw-r--r--internal/praefect/replicator_test.go2
-rw-r--r--ruby/lib/gitaly_server/repository_service.rb41
-rw-r--r--ruby/lib/gitlab/git/gitlab_projects.rb19
-rw-r--r--ruby/lib/gitlab/git/http_auth.rb33
-rw-r--r--ruby/spec/gitaly/repository_service_spec.rb41
-rw-r--r--ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb61
20 files changed, 231 insertions, 502 deletions
diff --git a/changelogs/unreleased/ps-cleanup-ruby-fetch-remote.yml b/changelogs/unreleased/ps-cleanup-ruby-fetch-remote.yml
new file mode 100644
index 000000000..e7a590c6f
--- /dev/null
+++ b/changelogs/unreleased/ps-cleanup-ruby-fetch-remote.yml
@@ -0,0 +1,5 @@
+---
+title: Removal of ruby implementation of the FetchRemote
+merge_request: 2967
+author:
+type: removed
diff --git a/internal/gitaly/service/conflicts/resolve_conflicts.go b/internal/gitaly/service/conflicts/resolve_conflicts.go
index 0d7d495a7..024e2df1a 100644
--- a/internal/gitaly/service/conflicts/resolve_conflicts.go
+++ b/internal/gitaly/service/conflicts/resolve_conflicts.go
@@ -279,7 +279,7 @@ func (s *server) repoWithBranchCommit(ctx context.Context, srcRepo, targetRepo *
return nil
}
- env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{Repository: targetRepo})
+ env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{Repository: targetRepo})
if err != nil {
return err
}
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index ee9922272..df2f90024 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -429,7 +429,7 @@ func (s *Server) fetchMissingCommit(ctx context.Context, local, remote *gitalypb
}
func (s *Server) fetchRemoteObject(ctx context.Context, local, remote *gitalypb.Repository, sha string) error {
- env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{
+ env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{
Repository: remote,
GitConfigOptions: []string{"uploadpack.allowAnySHA1InWant=true"},
})
diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go
index f7365d3fe..83f24ab49 100644
--- a/internal/gitaly/service/register.go
+++ b/internal/gitaly/service/register.go
@@ -87,7 +87,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver
))
gitalypb.RegisterWikiServiceServer(grpcServer, wiki.NewServer(rubyServer, locator))
gitalypb.RegisterConflictsServiceServer(grpcServer, conflicts.NewServer(rubyServer, cfg, locator))
- gitalypb.RegisterRemoteServiceServer(grpcServer, remote.NewServer(rubyServer, locator))
+ gitalypb.RegisterRemoteServiceServer(grpcServer, remote.NewServer(cfg, rubyServer, locator))
gitalypb.RegisterServerServiceServer(grpcServer, server.NewServer(cfg.Storages))
gitalypb.RegisterObjectPoolServiceServer(grpcServer, objectpool.NewServer(cfg, locator))
gitalypb.RegisterHookServiceServer(grpcServer, hook.NewServer(cfg, hookManager))
diff --git a/internal/gitaly/service/remote/fetch_internal_remote.go b/internal/gitaly/service/remote/fetch_internal_remote.go
index 72fe4765a..321be53b0 100644
--- a/internal/gitaly/service/remote/fetch_internal_remote.go
+++ b/internal/gitaly/service/remote/fetch_internal_remote.go
@@ -27,7 +27,7 @@ func (s *server) FetchInternalRemote(ctx context.Context, req *gitalypb.FetchInt
return nil, status.Errorf(codes.InvalidArgument, "FetchInternalRemote: %v", err)
}
- env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{Repository: req.RemoteRepository})
+ env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{Repository: req.RemoteRepository})
if err != nil {
return nil, fmt.Errorf("upload pack environment: %w", err)
}
diff --git a/internal/gitaly/service/remote/server.go b/internal/gitaly/service/remote/server.go
index 7da72b02d..ef48bbf9a 100644
--- a/internal/gitaly/service/remote/server.go
+++ b/internal/gitaly/service/remote/server.go
@@ -2,12 +2,14 @@ package remote
import (
"gitlab.com/gitlab-org/gitaly/client"
+ "gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
"gitlab.com/gitlab-org/gitaly/internal/storage"
"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
)
type server struct {
+ cfg config.Cfg
ruby *rubyserver.Server
locator storage.Locator
@@ -15,8 +17,9 @@ type server struct {
}
// NewServer creates a new instance of a grpc RemoteServiceServer
-func NewServer(rs *rubyserver.Server, locator storage.Locator) gitalypb.RemoteServiceServer {
+func NewServer(cfg config.Cfg, rs *rubyserver.Server, locator storage.Locator) gitalypb.RemoteServiceServer {
return &server{
+ cfg: cfg,
ruby: rs,
locator: locator,
conns: client.NewPoolWithOptions(
diff --git a/internal/gitaly/service/remote/testhelper_test.go b/internal/gitaly/service/remote/testhelper_test.go
index 44cd2bdaa..c90669e73 100644
--- a/internal/gitaly/service/remote/testhelper_test.go
+++ b/internal/gitaly/service/remote/testhelper_test.go
@@ -38,7 +38,7 @@ func testMain(m *testing.M) int {
func RunRemoteServiceServer(t *testing.T, opts ...testhelper.TestServerOpt) (string, func()) {
srv := testhelper.NewServer(t, nil, nil, opts...)
- gitalypb.RegisterRemoteServiceServer(srv.GrpcServer(), NewServer(RubyServer, config.NewLocator(config.Config)))
+ gitalypb.RegisterRemoteServiceServer(srv.GrpcServer(), NewServer(config.Config, RubyServer, config.NewLocator(config.Config)))
reflection.Register(srv.GrpcServer())
srv.Start(t)
diff --git a/internal/gitaly/service/repository/fetch.go b/internal/gitaly/service/repository/fetch.go
index 417e1f2f8..3d7d357a7 100644
--- a/internal/gitaly/service/repository/fetch.go
+++ b/internal/gitaly/service/repository/fetch.go
@@ -76,7 +76,7 @@ func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourc
// There's no need to perform the fetch if we already have the object
// available.
if !containsObject {
- env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{Repository: req.SourceRepository})
+ env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{Repository: req.SourceRepository})
if err != nil {
return nil, err
}
diff --git a/internal/gitaly/service/repository/fetch_remote.go b/internal/gitaly/service/repository/fetch_remote.go
index 486feb106..fc08616c4 100644
--- a/internal/gitaly/service/repository/fetch_remote.go
+++ b/internal/gitaly/service/repository/fetch_remote.go
@@ -13,51 +13,17 @@ import (
"time"
"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
- "github.com/prometheus/client_golang/prometheus"
"github.com/sirupsen/logrus"
"gitlab.com/gitlab-org/gitaly/internal/command"
"gitlab.com/gitlab-org/gitaly/internal/errors"
"gitlab.com/gitlab-org/gitaly/internal/git"
"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/status"
)
-var (
- fetchRemoteImplCounter = prometheus.NewCounterVec(
- prometheus.CounterOpts{
- Name: "gitaly_fetch_remote_counter",
- Help: "Number of calls to FetchRemote rpc for each implementation (ruby/go)",
- },
- []string{"impl"},
- )
-)
-
-func init() {
- prometheus.MustRegister(fetchRemoteImplCounter)
-}
-
func (s *server) FetchRemote(ctx context.Context, req *gitalypb.FetchRemoteRequest) (*gitalypb.FetchRemoteResponse, error) {
- if featureflag.IsDisabled(ctx, featureflag.GoFetchRemote) {
- fetchRemoteImplCounter.WithLabelValues("ruby").Inc()
-
- client, err := s.ruby.RepositoryServiceClient(ctx)
- if err != nil {
- return nil, err
- }
-
- clientCtx, err := rubyserver.SetHeaders(ctx, s.locator, req.GetRepository())
- if err != nil {
- return nil, err
- }
-
- return client.FetchRemote(clientCtx, req)
- }
-
- fetchRemoteImplCounter.WithLabelValues("go").Inc()
-
if err := s.validateFetchRemoteRequest(req); err != nil {
return nil, err
}
diff --git a/internal/gitaly/service/repository/fetch_remote_test.go b/internal/gitaly/service/repository/fetch_remote_test.go
index 1006fd59c..c0577df6a 100644
--- a/internal/gitaly/service/repository/fetch_remote_test.go
+++ b/internal/gitaly/service/repository/fetch_remote_test.go
@@ -1,7 +1,6 @@
package repository
import (
- "context"
"fmt"
"io/ioutil"
"net/http"
@@ -16,12 +15,10 @@ import (
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
"gitlab.com/gitlab-org/gitaly/internal/helper/text"
- "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
"gitlab.com/gitlab-org/gitaly/internal/storage"
"gitlab.com/gitlab-org/gitaly/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
"google.golang.org/grpc/codes"
- "google.golang.org/grpc/metadata"
)
func copyRepoWithNewRemote(t *testing.T, repo *gitalypb.Repository, locator storage.Locator, remote string) (*gitalypb.Repository, string) {
@@ -48,40 +45,32 @@ func TestFetchRemoteSuccess(t *testing.T) {
client, conn := newRepositoryClient(t, serverSocketPath)
defer conn.Close()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.GoFetchRemote,
- }).Run(t, func(t *testing.T, ctx context.Context) {
- testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
- defer cleanupFn()
-
- cloneRepo, cloneRepoPath := copyRepoWithNewRemote(t, testRepo, locator, "my-remote")
- defer func() {
- require.NoError(t, os.RemoveAll(cloneRepoPath))
- }()
-
- // Ensure there's a new tag to fetch
- testhelper.CreateTag(t, testRepoPath, "testtag", "master", nil)
-
- // On the first run, TagsChanged will be true for both implementations
- req := &gitalypb.FetchRemoteRequest{Repository: cloneRepo, Remote: "my-remote", Timeout: 120, CheckTagsChanged: true}
- resp, err := client.FetchRemote(ctx, req)
- require.NoError(t, err)
- require.NotNil(t, resp)
- require.Equal(t, resp.TagsChanged, true)
-
- // On the second run, TagsChanged will be false for the Go implementation only
- resp, err = client.FetchRemote(ctx, req)
- require.NoError(t, err)
- require.NotNil(t, resp)
- require.Equal(t, resp.TagsChanged, !isFeatureEnabled(ctx, featureflag.GoFetchRemote))
-
- // Finally, ensure that it returns true if we're asked not to check
- req.CheckTagsChanged = false
- resp, err = client.FetchRemote(ctx, req)
- require.NoError(t, err)
- require.NotNil(t, resp)
- require.Equal(t, resp.TagsChanged, true)
- })
+ ctx, cancel := testhelper.Context()
+ defer cancel()
+
+ testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+ defer cleanupFn()
+
+ cloneRepo, cloneRepoPath := copyRepoWithNewRemote(t, testRepo, locator, "my-remote")
+ defer func() {
+ require.NoError(t, os.RemoveAll(cloneRepoPath))
+ }()
+
+ // Ensure there's a new tag to fetch
+ testhelper.CreateTag(t, testRepoPath, "testtag", "master", nil)
+
+ req := &gitalypb.FetchRemoteRequest{Repository: cloneRepo, Remote: "my-remote", Timeout: 120, CheckTagsChanged: true}
+ resp, err := client.FetchRemote(ctx, req)
+ require.NoError(t, err)
+ require.NotNil(t, resp)
+ require.Equal(t, resp.TagsChanged, true)
+
+ // Ensure that it returns true if we're asked not to check
+ req.CheckTagsChanged = false
+ resp, err = client.FetchRemote(ctx, req)
+ require.NoError(t, err)
+ require.NotNil(t, resp)
+ require.Equal(t, resp.TagsChanged, true)
}
func TestFetchRemoteFailure(t *testing.T) {
@@ -98,139 +87,117 @@ func TestFetchRemoteFailure(t *testing.T) {
client, conn := newRepositoryClient(t, serverSocketPath)
defer conn.Close()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.GoFetchRemote,
- }).Run(t, func(t *testing.T, ctx context.Context) {
- tests := []struct {
- desc string
- req *gitalypb.FetchRemoteRequest
- goCode codes.Code
- goErrMsg string
- rubyCode codes.Code
- rubyErrMsg string
- }{
- {
- desc: "no repository",
- req: &gitalypb.FetchRemoteRequest{
- Repository: nil,
- Remote: remoteName,
- Timeout: 1000,
- },
- goCode: codes.InvalidArgument,
- goErrMsg: "empty Repository",
- rubyCode: codes.InvalidArgument,
- rubyErrMsg: "",
+ ctx, cancel := testhelper.Context()
+ defer cancel()
+
+ tests := []struct {
+ desc string
+ req *gitalypb.FetchRemoteRequest
+ code codes.Code
+ errMsg string
+ }{
+ {
+ desc: "no repository",
+ req: &gitalypb.FetchRemoteRequest{
+ Repository: nil,
+ Remote: remoteName,
+ Timeout: 1000,
},
- {
- desc: "invalid storage",
- req: &gitalypb.FetchRemoteRequest{
- Repository: &gitalypb.Repository{
- StorageName: "invalid",
- RelativePath: "foobar.git",
- },
- Remote: remoteName,
- Timeout: 1000,
+ code: codes.InvalidArgument,
+ errMsg: "empty Repository",
+ },
+ {
+ desc: "invalid storage",
+ req: &gitalypb.FetchRemoteRequest{
+ Repository: &gitalypb.Repository{
+ StorageName: "invalid",
+ RelativePath: "foobar.git",
},
- // the error text is shortened to only a single word as requests to gitaly done via praefect returns different error messages
- goCode: codes.InvalidArgument,
- goErrMsg: "invalid",
- rubyCode: codes.InvalidArgument,
- rubyErrMsg: "invalid",
+ Remote: remoteName,
+ Timeout: 1000,
},
- {
- desc: "invalid remote",
- req: &gitalypb.FetchRemoteRequest{
- Repository: repo,
- Remote: "",
- Timeout: 1000,
- },
- goCode: codes.InvalidArgument,
- goErrMsg: `blank or empty "remote"`,
- rubyCode: codes.Unknown,
- rubyErrMsg: `fatal: no path specified`,
+ // the error text is shortened to only a single word as requests to gitaly done via praefect returns different error messages
+ code: codes.InvalidArgument,
+ errMsg: "invalid",
+ },
+ {
+ desc: "invalid remote",
+ req: &gitalypb.FetchRemoteRequest{
+ Repository: repo,
+ Remote: "",
+ Timeout: 1000,
},
- {
- desc: "invalid remote url: bad format",
- req: &gitalypb.FetchRemoteRequest{
- Repository: repo,
- RemoteParams: &gitalypb.Remote{
- Url: "not a url",
- Name: remoteName,
- HttpAuthorizationHeader: httpToken,
- },
- Timeout: 1000,
+ code: codes.InvalidArgument,
+ errMsg: `blank or empty "remote"`,
+ },
+ {
+ desc: "invalid remote url: bad format",
+ req: &gitalypb.FetchRemoteRequest{
+ Repository: repo,
+ RemoteParams: &gitalypb.Remote{
+ Url: "not a url",
+ Name: remoteName,
+ HttpAuthorizationHeader: httpToken,
},
- goCode: codes.InvalidArgument,
- goErrMsg: `invalid "remote_params.url"`,
- rubyCode: codes.InvalidArgument,
- rubyErrMsg: "invalid remote url",
+ Timeout: 1000,
},
- {
- desc: "invalid remote url: no host",
- req: &gitalypb.FetchRemoteRequest{
- Repository: repo,
- RemoteParams: &gitalypb.Remote{
- Url: "/not/a/url",
- Name: remoteName,
- HttpAuthorizationHeader: httpToken,
- },
- Timeout: 1000,
+ code: codes.InvalidArgument,
+ errMsg: `invalid "remote_params.url"`,
+ },
+ {
+ desc: "invalid remote url: no host",
+ req: &gitalypb.FetchRemoteRequest{
+ Repository: repo,
+ RemoteParams: &gitalypb.Remote{
+ Url: "/not/a/url",
+ Name: remoteName,
+ HttpAuthorizationHeader: httpToken,
},
- goCode: codes.InvalidArgument,
- goErrMsg: `invalid "remote_params.url"`,
- rubyCode: codes.Unknown,
- rubyErrMsg: "fatal: '/not/a/url' does not appear to be a git repository",
+ Timeout: 1000,
},
- {
- desc: "no name",
- req: &gitalypb.FetchRemoteRequest{
- Repository: repo,
- RemoteParams: &gitalypb.Remote{
- Name: "",
- Url: url,
- HttpAuthorizationHeader: httpToken,
- },
- Timeout: 1000,
+ code: codes.InvalidArgument,
+ errMsg: `invalid "remote_params.url"`,
+ },
+ {
+ desc: "no name",
+ req: &gitalypb.FetchRemoteRequest{
+ Repository: repo,
+ RemoteParams: &gitalypb.Remote{
+ Name: "",
+ Url: url,
+ HttpAuthorizationHeader: httpToken,
},
- goCode: codes.InvalidArgument,
- goErrMsg: `blank or empty "remote_params.name"`,
- rubyCode: codes.Unknown,
- rubyErrMsg: "Rugged::ConfigError: '' is not a valid remote name",
+ Timeout: 1000,
},
- {
- desc: "not existing repo via http",
- req: &gitalypb.FetchRemoteRequest{
- Repository: repo,
- RemoteParams: &gitalypb.Remote{
- Url: httpSrv.URL + "/invalid/repo/path.git",
- Name: remoteName,
- HttpAuthorizationHeader: httpToken,
- MirrorRefmaps: []string{"all_refs"},
- },
- Timeout: 1000,
+ code: codes.InvalidArgument,
+ errMsg: `blank or empty "remote_params.name"`,
+ },
+ {
+ desc: "not existing repo via http",
+ req: &gitalypb.FetchRemoteRequest{
+ Repository: repo,
+ RemoteParams: &gitalypb.Remote{
+ Url: httpSrv.URL + "/invalid/repo/path.git",
+ Name: remoteName,
+ HttpAuthorizationHeader: httpToken,
+ MirrorRefmaps: []string{"all_refs"},
},
- goCode: codes.Unknown,
- goErrMsg: "invalid/repo/path.git/' not found",
- rubyCode: codes.Unknown,
- rubyErrMsg: "/invalid/repo/path.git/' not found",
+ Timeout: 1000,
},
- }
- for _, tc := range tests {
- t.Run(tc.desc, func(t *testing.T) {
- resp, err := client.FetchRemote(ctx, tc.req)
- require.Error(t, err)
- require.Nil(t, resp)
-
- if isFeatureEnabled(ctx, featureflag.GoFetchRemote) {
- require.Contains(t, err.Error(), tc.goErrMsg)
- testhelper.RequireGrpcError(t, err, tc.goCode)
- } else {
- require.Contains(t, err.Error(), tc.rubyErrMsg)
- testhelper.RequireGrpcError(t, err, tc.rubyCode)
- }
- })
- }
- })
+ code: codes.Unknown,
+ errMsg: "invalid/repo/path.git/' not found",
+ },
+ }
+ for _, tc := range tests {
+ t.Run(tc.desc, func(t *testing.T) {
+ resp, err := client.FetchRemote(ctx, tc.req)
+ require.Error(t, err)
+ require.Nil(t, resp)
+
+ require.Contains(t, err.Error(), tc.errMsg)
+ testhelper.RequireGrpcError(t, err, tc.code)
+ })
+ }
}
const (
@@ -275,59 +242,58 @@ func TestFetchRemoteOverHTTP(t *testing.T) {
client, conn := newRepositoryClient(t, serverSocketPath)
defer conn.Close()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.GoFetchRemote,
- }).Run(t, func(t *testing.T, ctx context.Context) {
- testCases := []struct {
- description string
- httpToken string
- remoteURL string
- }{
- {
- description: "with http token",
- httpToken: httpToken,
- },
- {
- description: "without http token",
- httpToken: "",
- },
- }
-
- for _, tc := range testCases {
- t.Run(tc.description, func(t *testing.T) {
- forkedRepo, forkedRepoPath, forkedRepoCleanup := testhelper.NewTestRepo(t)
- defer forkedRepoCleanup()
-
- s, remoteURL := remoteHTTPServer(t, "my-repo", tc.httpToken)
- defer s.Close()
-
- req := &gitalypb.FetchRemoteRequest{
- Repository: forkedRepo,
- RemoteParams: &gitalypb.Remote{
- Url: remoteURL,
- Name: "geo",
- HttpAuthorizationHeader: tc.httpToken,
- MirrorRefmaps: []string{"all_refs"},
- },
- Timeout: 1000,
- }
- if tc.remoteURL != "" {
- req.RemoteParams.Url = s.URL + tc.remoteURL
- }
-
- refs := getRefnames(t, forkedRepoPath)
- require.True(t, len(refs) > 1, "the advertisement.txt should have deleted all refs except for master")
-
- _, err := client.FetchRemote(ctx, req)
- require.NoError(t, err)
-
- refs = getRefnames(t, forkedRepoPath)
-
- require.Len(t, refs, 1)
- assert.Equal(t, "master", refs[0])
- })
- }
- })
+ ctx, cancel := testhelper.Context()
+ defer cancel()
+
+ testCases := []struct {
+ description string
+ httpToken string
+ remoteURL string
+ }{
+ {
+ description: "with http token",
+ httpToken: httpToken,
+ },
+ {
+ description: "without http token",
+ httpToken: "",
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.description, func(t *testing.T) {
+ forkedRepo, forkedRepoPath, forkedRepoCleanup := testhelper.NewTestRepo(t)
+ defer forkedRepoCleanup()
+
+ s, remoteURL := remoteHTTPServer(t, "my-repo", tc.httpToken)
+ defer s.Close()
+
+ req := &gitalypb.FetchRemoteRequest{
+ Repository: forkedRepo,
+ RemoteParams: &gitalypb.Remote{
+ Url: remoteURL,
+ Name: "geo",
+ HttpAuthorizationHeader: tc.httpToken,
+ MirrorRefmaps: []string{"all_refs"},
+ },
+ Timeout: 1000,
+ }
+ if tc.remoteURL != "" {
+ req.RemoteParams.Url = s.URL + tc.remoteURL
+ }
+
+ refs := getRefnames(t, forkedRepoPath)
+ require.True(t, len(refs) > 1, "the advertisement.txt should have deleted all refs except for master")
+
+ _, err := client.FetchRemote(ctx, req)
+ require.NoError(t, err)
+
+ refs = getRefnames(t, forkedRepoPath)
+
+ require.Len(t, refs, 1)
+ assert.Equal(t, "master", refs[0])
+ })
+ }
}
func TestFetchRemoteOverHTTPWithRedirect(t *testing.T) {
@@ -345,22 +311,21 @@ func TestFetchRemoteOverHTTPWithRedirect(t *testing.T) {
)
defer s.Close()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.GoFetchRemote,
- }).Run(t, func(t *testing.T, ctx context.Context) {
- testRepo, _, cleanup := testhelper.NewTestRepo(t)
- defer cleanup()
-
- req := &gitalypb.FetchRemoteRequest{
- Repository: testRepo,
- RemoteParams: &gitalypb.Remote{Url: s.URL, Name: "geo"},
- Timeout: 1000,
- }
-
- _, err := client.FetchRemote(ctx, req)
- require.Error(t, err)
- require.Contains(t, err.Error(), "The requested URL returned error: 303")
- })
+ ctx, cancel := testhelper.Context()
+ defer cancel()
+
+ testRepo, _, cleanup := testhelper.NewTestRepo(t)
+ defer cleanup()
+
+ req := &gitalypb.FetchRemoteRequest{
+ Repository: testRepo,
+ RemoteParams: &gitalypb.Remote{Url: s.URL, Name: "geo"},
+ Timeout: 1000,
+ }
+
+ _, err := client.FetchRemote(ctx, req)
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "The requested URL returned error: 303")
}
func TestFetchRemoteOverHTTPWithTimeout(t *testing.T) {
@@ -379,30 +344,20 @@ func TestFetchRemoteOverHTTPWithTimeout(t *testing.T) {
)
defer s.Close()
- testhelper.NewFeatureSets([]featureflag.FeatureFlag{
- featureflag.GoFetchRemote,
- }).Run(t, func(t *testing.T, ctx context.Context) {
- testRepo, _, cleanup := testhelper.NewTestRepo(t)
- defer cleanup()
-
- req := &gitalypb.FetchRemoteRequest{
- Repository: testRepo,
- RemoteParams: &gitalypb.Remote{Url: s.URL, Name: "geo"},
- Timeout: 1,
- }
-
- _, err := client.FetchRemote(ctx, req)
- require.Error(t, err)
-
- if isFeatureEnabled(ctx, featureflag.GoFetchRemote) {
- require.Contains(t, err.Error(), "fetch remote: signal: terminated")
- } else {
- require.Contains(t, err.Error(), "failed: Timed out")
- }
- })
-}
+ ctx, cancel := testhelper.Context()
+ defer cancel()
+
+ testRepo, _, cleanup := testhelper.NewTestRepo(t)
+ defer cleanup()
+
+ req := &gitalypb.FetchRemoteRequest{
+ Repository: testRepo,
+ RemoteParams: &gitalypb.Remote{Url: s.URL, Name: "geo"},
+ Timeout: 1,
+ }
+
+ _, err := client.FetchRemote(ctx, req)
+ require.Error(t, err)
-func isFeatureEnabled(ctx context.Context, flag featureflag.FeatureFlag) bool {
- md, _ := metadata.FromOutgoingContext(ctx)
- return featureflag.IsEnabled(metadata.NewIncomingContext(ctx, md), flag)
+ require.Contains(t, err.Error(), "fetch remote: signal: terminated")
}
diff --git a/internal/gitaly/service/repository/fork.go b/internal/gitaly/service/repository/fork.go
index 721b07c2f..46c96b7fb 100644
--- a/internal/gitaly/service/repository/fork.go
+++ b/internal/gitaly/service/repository/fork.go
@@ -48,7 +48,7 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest
return nil, status.Errorf(codes.Internal, "CreateFork: create dest dir: %v", err)
}
- env, err := gitalyssh.UploadPackEnv(ctx, &gitalypb.SSHUploadPackRequest{Repository: sourceRepository})
+ env, err := gitalyssh.UploadPackEnv(ctx, s.cfg, &gitalypb.SSHUploadPackRequest{Repository: sourceRepository})
if err != nil {
return nil, err
}
diff --git a/internal/gitalyssh/gitalyssh.go b/internal/gitalyssh/gitalyssh.go
index 4b57f6168..26ccaad9f 100644
--- a/internal/gitalyssh/gitalyssh.go
+++ b/internal/gitalyssh/gitalyssh.go
@@ -30,15 +30,16 @@ var (
envInjector = tracing.NewEnvInjector()
)
-func UploadPackEnv(ctx context.Context, req *gitalypb.SSHUploadPackRequest) ([]string, error) {
- env, err := commandEnv(ctx, req.Repository.StorageName, "upload-pack", req)
+// UploadPackEnv returns a list of the key=val pairs required to set proper configuration options for upload-pack command.
+func UploadPackEnv(ctx context.Context, cfg config.Cfg, req *gitalypb.SSHUploadPackRequest) ([]string, error) {
+ env, err := commandEnv(ctx, cfg, req.Repository.StorageName, "upload-pack", req)
if err != nil {
return nil, err
}
return envInjector(ctx, env), nil
}
-func commandEnv(ctx context.Context, storageName, command string, message proto.Message) ([]string, error) {
+func commandEnv(ctx context.Context, cfg config.Cfg, storageName, command string, message proto.Message) ([]string, error) {
var pbMarshaler jsonpb.Marshaler
payload, err := pbMarshaler.MarshalToString(message)
if err != nil {
@@ -63,7 +64,7 @@ func commandEnv(ctx context.Context, storageName, command string, message proto.
return []string{
fmt.Sprintf("GITALY_PAYLOAD=%s", payload),
- fmt.Sprintf("GIT_SSH_COMMAND=%s %s", gitalySSHPath(), command),
+ fmt.Sprintf("GIT_SSH_COMMAND=%s %s", filepath.Join(cfg.BinDir, "gitaly-ssh"), command),
fmt.Sprintf("GITALY_ADDRESS=%s", storageInfo.Address),
fmt.Sprintf("GITALY_TOKEN=%s", storageInfo.Token),
fmt.Sprintf("GITALY_FEATUREFLAGS=%s", strings.Join(featureFlagPairs, ",")),
@@ -76,7 +77,3 @@ func commandEnv(ctx context.Context, storageName, command string, message proto.
fmt.Sprintf("%s=%s", gitaly_x509.SSLCertFile, os.Getenv(gitaly_x509.SSLCertFile)),
}, nil
}
-
-func gitalySSHPath() string {
- return filepath.Join(config.Config.BinDir, "gitaly-ssh")
-}
diff --git a/internal/gitalyssh/gitalyssh_test.go b/internal/gitalyssh/gitalyssh_test.go
index 7c7fe9dc9..17173bdaa 100644
--- a/internal/gitalyssh/gitalyssh_test.go
+++ b/internal/gitalyssh/gitalyssh_test.go
@@ -3,7 +3,6 @@ package gitalyssh
import (
"encoding/base64"
"fmt"
- "path/filepath"
"testing"
"github.com/golang/protobuf/jsonpb"
@@ -34,11 +33,11 @@ func TestUploadPackEnv(t *testing.T) {
expectedPayload, err := pbMarshaler.MarshalToString(&req)
require.NoError(t, err)
- env, err := UploadPackEnv(ctx, &req)
+ env, err := UploadPackEnv(ctx, config.Cfg{BinDir: "/path/bin"}, &req)
require.NoError(t, err)
require.Subset(t, env, []string{
- fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", filepath.Join(config.Config.BinDir, "gitaly-ssh")),
+ "GIT_SSH_COMMAND=/path/bin/gitaly-ssh upload-pack",
fmt.Sprintf("GITALY_PAYLOAD=%s", expectedPayload),
"CORRELATION_ID=correlation-id-1",
"GIT_SSH_VARIANT=simple",
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index 18a987a6b..3b002ad35 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -117,7 +117,6 @@ var All = []FeatureFlag{
GoUserSquash,
GoUserCommitFiles,
GoResolveConflicts,
- GoFetchRemote,
GoUserDeleteTag,
GoUserCreateTag,
GoUserRevert,
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index 121a2142e..6bab2c2a4 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -1034,7 +1034,7 @@ func newReplicationService(tb testing.TB) (*grpc.Server, string) {
locator := gitaly_config.NewLocator(gitaly_config.Config)
gitalypb.RegisterRepositoryServiceServer(svr, repository.NewServer(gitaly_config.Config, RubyServer, locator))
gitalypb.RegisterObjectPoolServiceServer(svr, objectpoolservice.NewServer(gitaly_config.Config, locator))
- gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(RubyServer, locator))
+ gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(gitaly_config.Config, RubyServer, locator))
gitalypb.RegisterSSHServiceServer(svr, ssh.NewServer(locator))
gitalypb.RegisterRefServiceServer(svr, ref.NewServer(locator))
healthpb.RegisterHealthServer(svr, health.NewServer())
diff --git a/ruby/lib/gitaly_server/repository_service.rb b/ruby/lib/gitaly_server/repository_service.rb
index 80f9098d1..9707862b6 100644
--- a/ruby/lib/gitaly_server/repository_service.rb
+++ b/ruby/lib/gitaly_server/repository_service.rb
@@ -12,47 +12,6 @@ module GitalyServer
Gitaly::FetchSourceBranchResponse.new(result: result)
end
- def fetch_remote(request, call)
- gitlab_projects = Gitlab::Git::GitlabProjects.from_gitaly(request.repository, call)
-
- remote_name = request.remote
-
- repository = nil
-
- if request.remote_params
- params = request.remote_params
- repository = Gitlab::Git::Repository.from_gitaly(request.repository, call)
-
- mirror_refmaps = parse_refmaps(params.mirror_refmaps)
-
- repository.add_remote(params.name, params.url, mirror_refmap: mirror_refmaps)
- remote_name = params.name
- end
-
- success = Gitlab::Git::SshAuth.from_gitaly(request).setup do |env|
- Gitlab::Git::HttpAuth.from_gitaly(request, call) do
- gitlab_projects.fetch_remote(
- remote_name,
- request.timeout,
- force: request.force,
- tags: !request.no_tags,
- env: env
- )
- end
- end
-
- raise GRPC::Unknown.new("Fetching remote #{request.remote} failed: #{gitlab_projects.output}") unless success
-
- # This implementation is being replaced with a Go implementation, which
- # correctly implements the tags_changed parameter. Pretending the tags
- # have always changed here retains the semantics of this implementation,
- # which will be removed once the rollout is complete:
- # https://gitlab.com/gitlab-org/gitaly/-/issues/3307
- Gitaly::FetchRemoteResponse.new(tags_changed: true)
- ensure
- repository&.remove_remote(remote_name)
- end
-
def set_config(request, call)
repo = Gitlab::Git::Repository.from_gitaly(request.repository, call)
diff --git a/ruby/lib/gitlab/git/gitlab_projects.rb b/ruby/lib/gitlab/git/gitlab_projects.rb
index a23cdfa91..9c11e4815 100644
--- a/ruby/lib/gitlab/git/gitlab_projects.rb
+++ b/ruby/lib/gitlab/git/gitlab_projects.rb
@@ -59,15 +59,6 @@ module Gitlab
end
end
- def fetch_remote(name, timeout, force:, tags:, env: {}, prune: true)
- logger.info "Fetching remote #{name} for repository #{repository_absolute_path}."
- cmd = fetch_remote_command(name, tags, prune, force)
-
- run_with_timeout(cmd, timeout, repository_absolute_path, env).tap do |success|
- logger.error "Fetching remote #{name} for repository #{repository_absolute_path} failed." unless success
- end
- end
-
def push_branches(remote_name, timeout, force, branch_names, env: {})
branch_names.each_slice(BRANCHES_PER_PUSH) do |branches|
logger.info "Pushing #{branches.count} branches from #{repository_absolute_path} to remote #{remote_name}"
@@ -125,16 +116,6 @@ module Gitlab
cmd = %W(#{Gitlab.config.git.bin_path} remote rm origin)
run(cmd, repository_absolute_path)
end
-
- private
-
- def fetch_remote_command(name, tags, prune, force)
- %W(#{Gitlab.config.git.bin_path} -c http.followRedirects=false fetch #{name} --quiet).tap do |cmd|
- cmd << '--prune' if prune
- cmd << '--force' if force
- cmd << (tags ? '--tags' : '--no-tags')
- end
- end
end
end
end
diff --git a/ruby/lib/gitlab/git/http_auth.rb b/ruby/lib/gitlab/git/http_auth.rb
deleted file mode 100644
index de4e7287e..000000000
--- a/ruby/lib/gitlab/git/http_auth.rb
+++ /dev/null
@@ -1,33 +0,0 @@
-module Gitlab
- module Git
- class HttpAuth
- def self.from_gitaly(request, call)
- params = request.remote_params
-
- return yield unless params.present?
- return yield unless params.http_authorization_header.present?
-
- repo = Gitlab::Git::Repository.from_gitaly(request.repository, call)
-
- validate_remote_params(params)
-
- key = "http.#{params.url}.extraHeader"
- repo.rugged.config[key] = "Authorization: #{params.http_authorization_header}"
-
- begin
- yield
- ensure
- repo.rugged.config.delete(key)
- end
- end
-
- def self.validate_remote_params(remote_params)
- begin
- URI.parse(remote_params.url)
- rescue URI::Error
- raise GRPC::InvalidArgument, 'invalid remote url'
- end
- end
- end
- end
-end
diff --git a/ruby/spec/gitaly/repository_service_spec.rb b/ruby/spec/gitaly/repository_service_spec.rb
index e4c9bcf95..5c41e9780 100644
--- a/ruby/spec/gitaly/repository_service_spec.rb
+++ b/ruby/spec/gitaly/repository_service_spec.rb
@@ -19,45 +19,4 @@ describe Gitaly::RepositoryService do
expect(response.exists).to eq(true)
end
end
-
- describe 'FetchRemote' do
- let(:call) { double(metadata: { 'gitaly-storage-path' => '/path/to/storage' }) }
- let(:repo) { gitaly_repo('default', 'foobar.git') }
- let(:remote) { 'my-remote' }
-
- let(:gl_projects) { double('Gitlab::Git::GitlabProjects') }
-
- before do
- allow(Gitlab::Git::GitlabProjects).to receive(:from_gitaly).and_return(gl_projects)
- end
-
- context 'request does not have ssh_key and known_hosts set' do
- it 'calls GitlabProjects#fetch_remote with an empty environment' do
- request = Gitaly::FetchRemoteRequest.new(repository: repo, remote: remote)
-
- expect(gl_projects).to receive(:fetch_remote)
- .with(remote, 0, force: false, tags: true, env: {})
- .and_return(true)
-
- GitalyServer::RepositoryService.new.fetch_remote(request, call)
- end
- end
-
- context 'request has ssh_key and known_hosts set' do
- it 'calls GitlabProjects#fetch_remote with a custom GIT_SSH_COMMAND' do
- request = Gitaly::FetchRemoteRequest.new(
- repository: repo,
- remote: remote,
- ssh_key: 'SSH KEY',
- known_hosts: 'KNOWN HOSTS'
- )
-
- expect(gl_projects).to receive(:fetch_remote)
- .with(remote, 0, force: false, tags: true, env: { 'GIT_SSH_COMMAND' => /ssh/ })
- .and_return(true)
-
- GitalyServer::RepositoryService.new.fetch_remote(request, call)
- end
- end
- end
end
diff --git a/ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb b/ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb
index cd1a6c2e4..ac1ae4d67 100644
--- a/ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb
+++ b/ruby/spec/lib/gitlab/git/gitlab_projects_spec.rb
@@ -95,67 +95,6 @@ describe Gitlab::Git::GitlabProjects do
end
end
- describe '#fetch_remote' do
- let(:remote_name) { 'remote-name' }
- let(:branch_name) { 'master' }
- let(:force) { false }
- let(:tags) { true }
- let(:env) { { 'GIT_SSH_COMMAND' => 'foo-command bar' } }
- let(:prune) { true }
- let(:follow_redirects) { false }
- let(:args) { { force: force, tags: tags, env: env, prune: prune } }
- let(:cmd) { %W(#{Gitlab.config.git.bin_path} -c http.followRedirects=false fetch #{remote_name} --quiet --prune --tags) }
-
- subject { gl_projects.fetch_remote(remote_name, 600, **args) }
-
- context 'with default args' do
- it 'executes the command' do
- stub_spawn(cmd, 600, tmp_repo_path, env, success: true)
-
- is_expected.to be_truthy
- end
-
- it 'returns false if the command fails' do
- stub_spawn(cmd, 600, tmp_repo_path, env, success: false)
-
- is_expected.to be_falsy
- end
- end
-
- context 'with --force' do
- let(:force) { true }
- let(:cmd) { %W(#{Gitlab.config.git.bin_path} -c http.followRedirects=false fetch #{remote_name} --quiet --prune --force --tags) }
-
- it 'executes the command with forced option' do
- stub_spawn(cmd, 600, tmp_repo_path, env, success: true)
-
- is_expected.to be_truthy
- end
- end
-
- context 'with --no-tags' do
- let(:tags) { false }
- let(:cmd) { %W(#{Gitlab.config.git.bin_path} -c http.followRedirects=false fetch #{remote_name} --quiet --prune --no-tags) }
-
- it 'executes the command' do
- stub_spawn(cmd, 600, tmp_repo_path, env, success: true)
-
- is_expected.to be_truthy
- end
- end
-
- context 'with no prune' do
- let(:prune) { false }
- let(:cmd) { %W(#{Gitlab.config.git.bin_path} -c http.followRedirects=false fetch #{remote_name} --quiet --tags) }
-
- it 'executes the command' do
- stub_spawn(cmd, 600, tmp_repo_path, env, success: true)
-
- is_expected.to be_truthy
- end
- end
- end
-
describe '#delete_remote_branches' do
let(:remote_name) { 'remote-name' }
let(:branch_names) { 20.times.map { |i| "branch#{i}" } }