Skip to content

Commit 6cebe94

Browse files
committed
Fix reconfigure failing when Sentinel TLS is only enabled
If the unencrypted Sentinel port is disabled and only TLS enabled, `gitlab-ctl reconfigure` previously failed because it attempted to talk to port 0, which is an invalid port. This commit fixes this problem by switching to TLS if is the only option. In addition, server and client certificates need to be provided to `redis-cli` if required. Relates to https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/6627 Changelog: fixed
1 parent 9ebd500 commit 6cebe94

File tree

2 files changed

+104
-1
lines changed

2 files changed

+104
-1
lines changed

files/gitlab-cookbooks/gitlab-ee/libraries/sentinel_helper.rb

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ def use_hostnames
2929
def running_version
3030
return unless OmnibusHelper.new(@node).service_up?('sentinel')
3131

32-
command = "/opt/gitlab/embedded/bin/redis-cli -h #{sentinel['bind']} -p #{sentinel['port']} INFO"
32+
command = "/opt/gitlab/embedded/bin/redis-cli #{redis_cli_connect_options} INFO"
3333
env =
3434
if sentinel['password']
3535
{ 'REDISCLI_AUTH' => sentinel['password'] }
@@ -109,4 +109,44 @@ def save_to_file(data)
109109
def generate_myid
110110
SecureRandom.hex(20) # size will be n*2 -> 40 characters
111111
end
112+
113+
def redis_cli_connect_options
114+
args = ["-h #{sentinel['bind']}"]
115+
port = sentinel['port'].to_i
116+
117+
if port.zero?
118+
redis_cli_tls_options(args)
119+
else
120+
args << "-p #{port}"
121+
end
122+
123+
args.join(' ')
124+
end
125+
126+
def redis_cli_tls_options(args)
127+
tls_port = sentinel['tls_port'].to_i
128+
129+
raise "No Sentinel port available: sentinel['port'] or sentinel['tls_port'] must be non-zero" if tls_port.zero?
130+
131+
args << "--tls"
132+
args << "-p #{tls_port}"
133+
args << "--cacert '#{sentinel['tls_ca_cert_file']}'" if sentinel['tls_ca_cert_file']
134+
args << "--cacertdir '#{sentinel['tls_ca_cert_dir']}'" if sentinel['tls_ca_cert_dir']
135+
136+
return unless client_certs_required?
137+
138+
raise "Sentinel TLS client authentication requires sentinel['tls_cert_file'] and sentinel['tls_key_file'] options" unless client_cert_and_key_available?
139+
140+
args << "--cert '#{sentinel['tls_cert_file']}'"
141+
args << "--key '#{sentinel['tls_key_file']}'"
142+
end
143+
144+
def client_certs_required?
145+
sentinel['tls_auth_clients'] == 'yes'
146+
end
147+
148+
def client_cert_and_key_available?
149+
sentinel['tls_cert_file'] && !sentinel['tls_cert_file'].empty? &&
150+
sentinel['tls_key_file'] && !sentinel['tls_key_file'].empty?
151+
end
112152
end

spec/chef/cookbooks/gitlab-ee/libraries/sentinel_helper_spec.rb

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,69 @@
4545
expect(subject.running_version).to eq('1.2.3')
4646
end
4747
end
48+
49+
context 'with both TLS and unencrypted ports disabled' do
50+
let(:gitlab_rb) do
51+
{
52+
sentinel: {
53+
port: 0,
54+
tls_port: 0
55+
}
56+
}
57+
end
58+
59+
it 'raises an error' do
60+
expect { subject.running_version }.to raise_error("No Sentinel port available: sentinel['port'] or sentinel['tls_port'] must be non-zero")
61+
end
62+
end
63+
64+
context 'with TLS enabled' do
65+
let(:cmd) { "/opt/gitlab/embedded/bin/redis-cli -h 0.0.0.0 --tls -p 6380 --cacert '#{tls_ca_cert_file}' --cacertdir '#{tls_ca_cert_dir}' INFO" }
66+
let(:tls_auth_clients) { 'no' }
67+
let(:tls_cert_file) { '/etc/gitlab/ssl/redis.crt' }
68+
let(:tls_key_file) { '/etc/gitlab/ssl/redis.key' }
69+
let(:tls_ca_cert_file) { '/etc/gitlab/ssl/redis-ca.crt' }
70+
let(:tls_ca_cert_dir) { '/opt/gitlab/embedded/ssl/certs' }
71+
let(:gitlab_rb) do
72+
{
73+
sentinel: {
74+
port: 0,
75+
tls_port: 6380,
76+
tls_cert_file: tls_cert_file,
77+
tls_key_file: tls_key_file,
78+
tls_dh_params_file: '/etc/gitlab/ssl/redis-dhparams',
79+
tls_ca_cert_file: tls_ca_cert_file,
80+
tls_ca_cert_dir: tls_ca_cert_dir,
81+
tls_auth_clients: tls_auth_clients
82+
}
83+
}
84+
end
85+
86+
it 'connects with TLS server certificates' do
87+
expect(VersionHelper).to receive(:do_shell_out).with(cmd, env: {}).and_return(status_success)
88+
89+
expect(subject.running_version).to eq('1.2.3')
90+
end
91+
92+
context 'with TLS client certs required' do
93+
let(:tls_auth_clients) { 'yes' }
94+
let(:cmd) { "/opt/gitlab/embedded/bin/redis-cli -h 0.0.0.0 --tls -p 6380 --cacert '#{tls_ca_cert_file}' --cacertdir '#{tls_ca_cert_dir}' --cert '#{tls_cert_file}' --key '#{tls_key_file}' INFO" }
95+
96+
it 'connects with TLS server and client certificates' do
97+
expect(VersionHelper).to receive(:do_shell_out).with(cmd, env: {}).and_return(status_success)
98+
99+
expect(subject.running_version).to eq('1.2.3')
100+
end
101+
102+
context 'with missing TLS client cert' do
103+
let(:tls_key_file) { nil }
104+
105+
it 'raises an error' do
106+
expect { subject.running_version }.to raise_error("Sentinel TLS client authentication requires sentinel['tls_cert_file'] and sentinel['tls_key_file'] options")
107+
end
108+
end
109+
end
110+
end
48111
end
49112

50113
context '#myid' do

0 commit comments

Comments
 (0)