From 28956e9c90f627c2e284c8fa0c4007295dbb5ede Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Sun, 11 Nov 2018 12:40:59 +0200 Subject: [PATCH 1/8] Initial testing --- cmd/gitaly-ssh/auth_test.go | 100 ++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 cmd/gitaly-ssh/auth_test.go diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go new file mode 100644 index 00000000000..8f0a0bf43ca --- /dev/null +++ b/cmd/gitaly-ssh/auth_test.go @@ -0,0 +1,100 @@ +package main + +import ( + "fmt" + "net" + "os" + "os/exec" + "path" + "strings" + "testing" + + "github.com/golang/protobuf/jsonpb" + "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" + "gitlab.com/gitlab-org/gitaly/internal/server" + "gitlab.com/gitlab-org/gitaly/internal/testhelper" + "google.golang.org/grpc" +) + +var ( + testRepo *gitalypb.Repository + gitalySSHPath string +) + +func TestMain(m *testing.M) { + os.Exit(testMain(m)) +} + +func testMain(m *testing.M) int { + defer testhelper.MustHaveNoChildProcess() + + cwd, _ := os.Getwd() + + testRepo = testhelper.TestRepository() + + // Build the test-binary that we need + os.Remove("gitaly-ssh") + + testhelper.MustRunCommand(nil, nil, "go", "build", "gitlab.com/gitlab-org/gitaly/cmd/gitaly-ssh") + defer os.Remove("gitaly-ssh") + gitalySSHPath = path.Join(cwd, "gitaly-ssh") + + return m.Run() +} + +func TestConnectivity(t *testing.T) { + testCases := []struct { + addr string + }{ + { + addr: "unix:///tmp/foo.socket", + }, + { + addr: "tcp://localhost:9999", + }, + } + + pbMarshaler := &jsonpb.Marshaler{} + payload, err := pbMarshaler.MarshalToString(&gitalypb.SSHReceivePackRequest{ + Repository: testRepo, + GlRepository: testRepo.GetRelativePath(), + GlId: "1", + }) + + require.NoError(t, err) + for _, testcase := range testCases { + srv := runServer(t, testcase.addr) + defer srv.Stop() + + cmd := exec.Command("git", "ls-remote", "git@localhost:test/test.git") + + cmd.Env = []string{ + fmt.Sprintf("GITALY_PAYLOAD=%s", payload), + fmt.Sprintf("GITALY_ADDRESS=%s", testcase.addr), + fmt.Sprintf("PATH=.:%s", os.Getenv("PATH")), + fmt.Sprintf("GIT_SSH_COMMAND=%s receive-pack", gitalySSHPath), + } + + err := cmd.Run() + require.NoError(t, err) + } +} + +func runServer(t *testing.T, addr string) *grpc.Server { + srv := server.New(nil) + + connectionType := "tcp" + addr = strings.TrimPrefix(addr, "tcp://") + if strings.HasPrefix(addr, "unix://") { + connectionType = "unix" + addr = strings.TrimPrefix(addr, "unix://") + } + + listener, err := net.Listen(connectionType, addr) + require.NoError(t, err) + + go srv.Serve(listener) + + return srv +} -- GitLab From 1c74d18ebe2c780f9ad754d5a7a6b45e717bde2a Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Wed, 14 Nov 2018 15:45:03 +0200 Subject: [PATCH 2/8] Add RemoteRepositoryClient tests --- .../git/remote_repository_client_spec.rb | 40 +++++++++++++++++++ .../support/helpers/integration_helper.rb | 16 +++++++- 2 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb diff --git a/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb b/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb new file mode 100644 index 00000000000..54e9ac34227 --- /dev/null +++ b/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Gitlab::Git::GitalyRemoteRepository do + include TestRepo + let(:repository) { gitlab_git_from_gitaly_with_gitlab_projects(new_mutable_test_repo) } + def get_client(type) + addr = case type + when 'tcp' + "tcp://#{TCP_ADDR}" + when 'unix' + "unix:#{File.join(TMP_DIR_NAME, SOCKET_PATH)}" + end + servers = Base64.strict_encode64({ + default: { + address: addr, + token: 'the-secret-token' + } + }.to_json) + + call = double({ + metadata: { + 'gitaly-servers': servers + } + }.with_indifferent_access) + + described_class.new(repository.gitaly_repository, call) + end + + describe 'Connectivity' do + it 'Should connect over tcp' do + client = get_client('tcp') + expect(client.empty?).to eq false + end + + it 'Should connect over unix' do + client = get_client('unix') + expect(client.empty?).to eq false + end + end +end diff --git a/ruby/spec/support/helpers/integration_helper.rb b/ruby/spec/support/helpers/integration_helper.rb index 8402ee1e4ca..546ca5118d6 100644 --- a/ruby/spec/support/helpers/integration_helper.rb +++ b/ruby/spec/support/helpers/integration_helper.rb @@ -4,11 +4,22 @@ require 'gitaly' require 'spec_helper' SOCKET_PATH = 'gitaly.socket'.freeze +TCP_ADDR = 'localhost:9999'.freeze module IntegrationClient - def gitaly_stub(service) + def gitaly_stub(service, type = 'unix') klass = Gitaly.const_get(service).const_get(:Stub) - klass.new("unix:#{File.join(TMP_DIR_NAME, SOCKET_PATH)}", :this_channel_is_insecure) + addr = case type + when 'unix' + "unix:#{File.join(TMP_DIR_NAME, SOCKET_PATH)}" + when 'tcp' + "tcp://#{TCP_ADDR}" + end + klass.new(addr, creds) + end + + def creds + :this_channel_is_insecure end def gitaly_repo(storage, relative_path) @@ -22,6 +33,7 @@ def start_gitaly config_toml = <<~CONFIG socket_path = "#{SOCKET_PATH}" + listen_addr = "#{TCP_ADDR}" bin_dir = "#{build_dir}/bin" [gitlab-shell] -- GitLab From 9cbb27f7a175c92b8fe792ac78cf82affda242e9 Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Thu, 15 Nov 2018 12:05:11 +0200 Subject: [PATCH 3/8] Add changelog entry --- changelogs/unreleased/test-authentication.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/test-authentication.yml diff --git a/changelogs/unreleased/test-authentication.yml b/changelogs/unreleased/test-authentication.yml new file mode 100644 index 00000000000..c68f1092c2b --- /dev/null +++ b/changelogs/unreleased/test-authentication.yml @@ -0,0 +1,5 @@ +--- +title: Add connectivity tests +merge_request: 968 +author: +type: other -- GitLab From b04d67eb0e15f023fea81fcde886aaff5866200b Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Fri, 23 Nov 2018 11:52:59 +0200 Subject: [PATCH 4/8] Use a dynamic gitaly port in ruby tests, use upload-pack instead of receive-pack in gitaly-ssh tests --- cmd/gitaly-ssh/auth_test.go | 8 +++----- .../gitlab/git/remote_repository_client_spec.rb | 4 +++- ruby/spec/support/helpers/integration_helper.rb | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go index ca1fd4caf0d..6a03886ca71 100644 --- a/cmd/gitaly-ssh/auth_test.go +++ b/cmd/gitaly-ssh/auth_test.go @@ -43,10 +43,8 @@ func TestConnectivity(t *testing.T) { } pbMarshaler := &jsonpb.Marshaler{} - payload, err := pbMarshaler.MarshalToString(&gitalypb.SSHReceivePackRequest{ - Repository: testRepo, - GlRepository: testRepo.GetRelativePath(), - GlId: "1", + payload, err := pbMarshaler.MarshalToString(&gitalypb.SSHUploadPackRequest{ + Repository: testRepo, }) require.NoError(t, err) @@ -60,7 +58,7 @@ func TestConnectivity(t *testing.T) { fmt.Sprintf("GITALY_PAYLOAD=%s", payload), fmt.Sprintf("GITALY_ADDRESS=%s", addr), fmt.Sprintf("PATH=.:%s", os.Getenv("PATH")), - fmt.Sprintf("GIT_SSH_COMMAND=%s receive-pack", gitalySSHPath), + fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", gitalySSHPath), } output, err := cmd.Output() diff --git a/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb b/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb index f4f1a4d7a05..312acf50f65 100644 --- a/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb +++ b/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb @@ -8,8 +8,9 @@ describe Gitlab::Git::GitalyRemoteRepository do describe 'Connectivity' do context 'tcp' do let(:client) do - get_client("tcp://#{TCP_ADDR}") + get_client("tcp://localhost:#{dynamic_port}") end + it 'Should connect over tcp' do expect(client).not_to be_empty end @@ -17,6 +18,7 @@ describe Gitlab::Git::GitalyRemoteRepository do context 'unix' do let(:client) { get_client("unix:#{File.join(TMP_DIR_NAME, SOCKET_PATH)}") } + it 'Should connect over unix' do expect(client).not_to be_empty end diff --git a/ruby/spec/support/helpers/integration_helper.rb b/ruby/spec/support/helpers/integration_helper.rb index b035e698228..444e8aea041 100644 --- a/ruby/spec/support/helpers/integration_helper.rb +++ b/ruby/spec/support/helpers/integration_helper.rb @@ -4,7 +4,6 @@ require 'gitaly' require 'spec_helper' SOCKET_PATH = 'gitaly.socket'.freeze -TCP_ADDR = 'localhost:9999'.freeze module IntegrationClient def gitaly_stub(service, type = 'unix') @@ -13,7 +12,7 @@ module IntegrationClient when 'unix' "unix:#{File.join(TMP_DIR_NAME, SOCKET_PATH)}" when 'tcp' - "tcp://#{TCP_ADDR}" + "tcp://localhost:#{dynamic_port}" end klass.new(addr, creds) end @@ -44,13 +43,23 @@ module IntegrationClient end end +def dynamic_port + $gitaly_port ||= begin + sock = Socket.new(:INET, :STREAM) + sock.bind(Addrinfo.tcp('127.0.0.1', 0)) + sock.local_address.ip_port + ensure + sock.close + end +end + def start_gitaly build_dir = File.expand_path(File.join(GITALY_RUBY_DIR, '../_build')) GitlabShellHelper.setup_gitlab_shell config_toml = <<~CONFIG socket_path = "#{SOCKET_PATH}" - listen_addr = "#{TCP_ADDR}" + listen_addr = "localhost:#{dynamic_port}" bin_dir = "#{build_dir}/bin" [gitlab-shell] -- GitLab From cc869adb463e5cc368af621fcd828ed6a29132a6 Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Fri, 23 Nov 2018 14:09:33 +0200 Subject: [PATCH 5/8] Refactor gitaly dynamic port --- cmd/gitaly-ssh/auth_test.go | 15 ++++++----- .../git/remote_repository_client_spec.rb | 2 +- .../support/helpers/integration_helper.rb | 26 ++++++++++--------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go index 6a03886ca71..ad6590cef68 100644 --- a/cmd/gitaly-ssh/auth_test.go +++ b/cmd/gitaly-ssh/auth_test.go @@ -31,14 +31,20 @@ func TestConnectivity(t *testing.T) { require.NoError(t, err) gitalySSHPath := path.Join(cwd, "gitaly-ssh") + tcpServer, tcpAddress := runServer(t, "tcp://localhost:0") + defer tcpServer.Stop() + + unixServer, unixAdddress := runServer(t, fmt.Sprintf("unix://%s", testhelper.GetTemporaryGitalySocketFileName())) + defer unixServer.Stop() + testCases := []struct { addr string }{ { - addr: fmt.Sprintf("unix://%s", testhelper.GetTemporaryGitalySocketFileName()), + addr: tcpAddress, }, { - addr: "tcp://localhost:0", + addr: unixAdddress, }, } @@ -49,14 +55,11 @@ func TestConnectivity(t *testing.T) { require.NoError(t, err) for _, testcase := range testCases { - srv, addr := runServer(t, testcase.addr) - defer srv.Stop() - cmd := exec.Command("git", "ls-remote", "git@localhost:test/test.git", "refs/heads/master") cmd.Env = []string{ fmt.Sprintf("GITALY_PAYLOAD=%s", payload), - fmt.Sprintf("GITALY_ADDRESS=%s", addr), + fmt.Sprintf("GITALY_ADDRESS=%s", testcase.addr), fmt.Sprintf("PATH=.:%s", os.Getenv("PATH")), fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", gitalySSHPath), } diff --git a/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb b/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb index 312acf50f65..dddb21c00f9 100644 --- a/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb +++ b/ruby/spec/lib/gitlab/git/remote_repository_client_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Git::GitalyRemoteRepository do describe 'Connectivity' do context 'tcp' do let(:client) do - get_client("tcp://localhost:#{dynamic_port}") + get_client("tcp://localhost:#{GitalyConfig.dynamic_port}") end it 'Should connect over tcp' do diff --git a/ruby/spec/support/helpers/integration_helper.rb b/ruby/spec/support/helpers/integration_helper.rb index 444e8aea041..29616716bdd 100644 --- a/ruby/spec/support/helpers/integration_helper.rb +++ b/ruby/spec/support/helpers/integration_helper.rb @@ -5,6 +5,18 @@ require 'spec_helper' SOCKET_PATH = 'gitaly.socket'.freeze +module GitalyConfig + def self.dynamic_port + @dynamic_port ||= begin + sock = Socket.new(:INET, :STREAM) + sock.bind(Addrinfo.tcp('127.0.0.1', 0)) + sock.local_address.ip_port + ensure + sock.close + end + end +end + module IntegrationClient def gitaly_stub(service, type = 'unix') klass = Gitaly.const_get(service).const_get(:Stub) @@ -12,7 +24,7 @@ module IntegrationClient when 'unix' "unix:#{File.join(TMP_DIR_NAME, SOCKET_PATH)}" when 'tcp' - "tcp://localhost:#{dynamic_port}" + "tcp://localhost:#{GitalyConfig.dynamic_port}" end klass.new(addr, creds) end @@ -43,23 +55,13 @@ module IntegrationClient end end -def dynamic_port - $gitaly_port ||= begin - sock = Socket.new(:INET, :STREAM) - sock.bind(Addrinfo.tcp('127.0.0.1', 0)) - sock.local_address.ip_port - ensure - sock.close - end -end - def start_gitaly build_dir = File.expand_path(File.join(GITALY_RUBY_DIR, '../_build')) GitlabShellHelper.setup_gitlab_shell config_toml = <<~CONFIG socket_path = "#{SOCKET_PATH}" - listen_addr = "localhost:#{dynamic_port}" + listen_addr = "localhost:#{GitalyConfig.dynamic_port}" bin_dir = "#{build_dir}/bin" [gitlab-shell] -- GitLab From b5d4f786d147b81a8fc52bd70ec5efbaf86bce35 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 23 Nov 2018 16:06:18 +0100 Subject: [PATCH 6/8] Refactor TestConnectivity --- cmd/gitaly-ssh/auth_test.go | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go index ad6590cef68..58ff89d9a57 100644 --- a/cmd/gitaly-ssh/auth_test.go +++ b/cmd/gitaly-ssh/auth_test.go @@ -6,12 +6,14 @@ import ( "os" "os/exec" "path" + "strconv" "strings" "testing" "github.com/golang/protobuf/jsonpb" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" + "gitlab.com/gitlab-org/gitaly/internal/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/server" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "google.golang.org/grpc" @@ -31,20 +33,22 @@ func TestConnectivity(t *testing.T) { require.NoError(t, err) gitalySSHPath := path.Join(cwd, "gitaly-ssh") - tcpServer, tcpAddress := runServer(t, "tcp://localhost:0") + socketPath := testhelper.GetTemporaryGitalySocketFileName() + + tcpServer, tcpPort := runServer(t, server.New, "tcp", "localhost:0") defer tcpServer.Stop() - unixServer, unixAdddress := runServer(t, fmt.Sprintf("unix://%s", testhelper.GetTemporaryGitalySocketFileName())) + unixServer, _ := runServer(t, server.New, "unix", socketPath) defer unixServer.Stop() testCases := []struct { addr string }{ { - addr: tcpAddress, + addr: fmt.Sprintf("tcp://localhost:%d", tcpPort), }, { - addr: unixAdddress, + addr: fmt.Sprintf("unix://%s", socketPath), }, } @@ -71,20 +75,22 @@ func TestConnectivity(t *testing.T) { } } -func runServer(t *testing.T, addr string) (*grpc.Server, string) { - srv := server.New(nil) - - connectionType := "tcp" - addr = strings.TrimPrefix(addr, "tcp://") - if strings.HasPrefix(addr, "unix://") { - connectionType = "unix" - addr = strings.TrimPrefix(addr, "unix://") - } +func runServer(t *testing.T, newServer func(rubyServer *rubyserver.Server) *grpc.Server, connectionType string, addr string) (*grpc.Server, int) { + srv := newServer(nil) listener, err := net.Listen(connectionType, addr) require.NoError(t, err) go srv.Serve(listener) - return srv, fmt.Sprintf("%s://%s", connectionType, listener.Addr().String()) + port := 0 + if connectionType != "unix" { + addrSplit := strings.Split(listener.Addr().String(), ":") + portString := addrSplit[len(addrSplit)-1] + + port, err = strconv.Atoi(portString) + require.NoError(t, err) + } + + return srv, port } -- GitLab From 6fb0e986282792d26de72ebeac637b2056a85b59 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 23 Nov 2018 16:56:29 +0100 Subject: [PATCH 7/8] Remove indifferent access --- ruby/spec/support/helpers/integration_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ruby/spec/support/helpers/integration_helper.rb b/ruby/spec/support/helpers/integration_helper.rb index 29616716bdd..ca3a83b7256 100644 --- a/ruby/spec/support/helpers/integration_helper.rb +++ b/ruby/spec/support/helpers/integration_helper.rb @@ -47,9 +47,9 @@ module IntegrationClient call = double({ metadata: { - 'gitaly-servers': servers + 'gitaly-servers' => servers } - }.with_indifferent_access) + }) Gitlab::Git::GitalyRemoteRepository.new(repository.gitaly_repository, call) end -- GitLab From 2475f2461118dbc47dfb58a5e45f02966b2f88a2 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 23 Nov 2018 17:38:13 +0100 Subject: [PATCH 8/8] rubocop --- ruby/spec/support/helpers/integration_helper.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/ruby/spec/support/helpers/integration_helper.rb b/ruby/spec/support/helpers/integration_helper.rb index ca3a83b7256..9c717261f94 100644 --- a/ruby/spec/support/helpers/integration_helper.rb +++ b/ruby/spec/support/helpers/integration_helper.rb @@ -45,12 +45,7 @@ module IntegrationClient } }.to_json) - call = double({ - metadata: { - 'gitaly-servers' => servers - } - }) - + call = double(metadata: { 'gitaly-servers' => servers }) Gitlab::Git::GitalyRemoteRepository.new(repository.gitaly_repository, call) end end -- GitLab