Commit 32c4f70a authored by Ahmad Hassan's avatar Ahmad Hassan

Followups on review

parent c1ed498f
......@@ -2,4 +2,4 @@
title: Support tls communication in gitaly
merge_request: 22602
author:
type: changed
type: added
......@@ -228,8 +228,8 @@ Omnibus installations:
```ruby
# /etc/gitlab/gitlab.rb
git_data_dirs({
'default' => { 'path' => '/mnt/gitlab/default', 'gitaly_address' => 'tls://gitaly.internal:8075' },
'storage1' => { 'path' => '/mnt/gitlab/storage1', 'gitaly_address' => 'tls://gitaly.internal:8075' },
'default' => { 'path' => '/mnt/gitlab/default', 'gitaly_address' => 'tls://gitaly.internal:9999' },
'storage1' => { 'path' => '/mnt/gitlab/storage1', 'gitaly_address' => 'tls://gitaly.internal:9999' },
})
gitlab_rails['gitaly_token'] = 'abc123secret'
......@@ -244,10 +244,10 @@ gitlab:
storages:
default:
path: /mnt/gitlab/default/repositories
gitaly_address: tls://gitaly.internal:8075
gitaly_address: tls://gitaly.internal:9999
storage1:
path: /mnt/gitlab/storage1/repositories
gitaly_address: tls://gitaly.internal:8075
gitaly_address: tls://gitaly.internal:9999
gitaly:
token: 'abc123secret'
......
......@@ -26,7 +26,7 @@ module Gitlab
end
end
PEM_REXP = /[-]+BEGIN CERTIFICATE[-]+.+?[-]+END CERTIFICATE[-]+/m
PEM_REGEX = /\-+BEGIN CERTIFICATE\-+.+?\-+END CERTIFICATE\-+/m
SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'
MAXIMUM_GITALY_CALLS = 35
CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze
......@@ -57,29 +57,27 @@ module Gitlab
end
end
def self.certs
def self.stub_certs
return @certs if @certs
cert_paths = Dir["#{OpenSSL::X509::DEFAULT_CERT_DIR}/*"]
cert_paths << OpenSSL::X509::DEFAULT_CERT_FILE if File.exist? OpenSSL::X509::DEFAULT_CERT_FILE
@certs = []
cert_paths.each do |cert_file|
begin
File.read(cert_file).scan(PEM_REXP).each do |cert|
pem = OpenSSL::X509::Certificate.new(cert).to_pem
@certs << pem
@certs = cert_paths.flat_map do |cert_file|
File.read(cert_file).scan(PEM_REGEX).map do |cert|
begin
OpenSSL::X509::Certificate.new(cert).to_pem
rescue OpenSSL::OpenSSLError => e
Rails.logger.error "Could not load certificate #{cert_file} #{e}"
nil
end
rescue StandardError => e
Rails.logger.error "Could not load certificate #{e}"
end
end
@certs = @certs.uniq.join "\n"
end.compact
end.uniq.join("\n")
end
def self.stub_creds(storage)
if URI(address(storage)).scheme == 'tls'
GRPC::Core::ChannelCredentials.new certs
GRPC::Core::ChannelCredentials.new stub_certs
else
:this_channel_is_insecure
end
......@@ -94,9 +92,7 @@ module Gitlab
end
def self.stub_address(storage)
addr = address(storage)
addr = addr.sub(%r{^tcp://|^tls://}, '') if %w(tcp tls).include? URI(addr).scheme
addr
address(storage).sub(%r{^tcp://|^tls://}, '')
end
def self.clear_stubs!
......
......@@ -3,6 +3,12 @@ require 'spec_helper'
# We stub Gitaly in `spec/support/gitaly.rb` for other tests. We don't want
# those stubs while testing the GitalyClient itself.
describe Gitlab::GitalyClient do
def stub_repos_storages(address)
allow(Gitlab.config.repositories).to receive(:storages).and_return({
'default' => { 'gitaly_address' => address }
})
end
describe '.stub_class' do
it 'returns the gRPC health check stub' do
expect(described_class.stub_class(:health_check)).to eq(::Grpc::Health::V1::Health::Stub)
......@@ -15,12 +21,8 @@ describe Gitlab::GitalyClient do
describe '.stub_address' do
it 'returns the same result after being called multiple times' do
address = 'localhost:9876'
prefixed_address = "tcp://#{address}"
allow(Gitlab.config.repositories).to receive(:storages).and_return({
'default' => { 'gitaly_address' => prefixed_address }
})
address = 'tcp://localhost:9876'
stub_repos_storages address
2.times do
expect(described_class.stub_address('default')).to eq('localhost:9876')
......@@ -29,19 +31,24 @@ describe Gitlab::GitalyClient do
end
describe '.stub_creds' do
it 'returns :this_channel_is_insecure if unix' do
address = 'unix:/tmp/gitaly.sock'
stub_repos_storages address
expect(described_class.stub_creds('default')).to eq(:this_channel_is_insecure)
end
it 'returns :this_channel_is_insecure if tcp' do
address = 'tcp://localhost:9876'
allow(Gitlab.config.repositories).to receive(:storages).and_return({
'default' => { 'gitaly_address' => address }
})
stub_repos_storages address
expect(described_class.stub_creds('default')).to eq(:this_channel_is_insecure)
end
it 'returns Credentials object if tls' do
address = 'tls://localhost:9876'
allow(Gitlab.config.repositories).to receive(:storages).and_return({
'default' => { 'gitaly_address' => address }
})
stub_repos_storages address
expect(described_class.stub_creds('default')).to be_a(GRPC::Core::ChannelCredentials)
end
end
......@@ -55,9 +62,7 @@ describe Gitlab::GitalyClient do
context 'when passed a UNIX socket address' do
it 'passes the address as-is to GRPC' do
address = 'unix:/tmp/gitaly.sock'
allow(Gitlab.config.repositories).to receive(:storages).and_return({
'default' => { 'gitaly_address' => address }
})
stub_repos_storages address
expect(Gitaly::CommitService::Stub).to receive(:new).with(address, any_args)
......@@ -69,10 +74,7 @@ describe Gitlab::GitalyClient do
it 'strips tls:// prefix before passing it to GRPC::Core::Channel initializer' do
address = 'localhost:9876'
prefixed_address = "tls://#{address}"
allow(Gitlab.config.repositories).to receive(:storages).and_return({
'default' => { 'gitaly_address' => prefixed_address }
})
stub_repos_storages prefixed_address
expect(Gitaly::CommitService::Stub).to receive(:new).with(address, any_args)
......@@ -84,10 +86,7 @@ describe Gitlab::GitalyClient do
it 'strips tcp:// prefix before passing it to GRPC::Core::Channel initializer' do
address = 'localhost:9876'
prefixed_address = "tcp://#{address}"
allow(Gitlab.config.repositories).to receive(:storages).and_return({
'default' => { 'gitaly_address' => prefixed_address }
})
stub_repos_storages prefixed_address
expect(Gitaly::CommitService::Stub).to receive(:new).with(address, any_args)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment