Fix crash when LDAP CA file set outside tls_options
What does this MR do and why?
Previously if ldap_servers.ca_file
were set instead of
ldap_servers.tls_options.ca_file
, the changes in
!122888 (merged) would
flag the call to Gitlab::Options#delete
.
Unfortunately, the introduction of
Gitlab::ErrorTracking.track_and_raise_for_dev_exception
in
!124396 (merged) caused
the process to crash with an undefined devise
method in the User
module. This happened because the method attempted to load the User
for the application context, but since Devise is initialized in
8_devise.rb
, this came too late for the LDAP config parsing in
1_settings.rb
.
To avoid this error, avoid loading the ApplicationContext
by using
Gitlab::AppJsonLogger
.
Relates to #419485 (closed)
How to set up and validate locally
- In
config/gitlab.yml
insert aldap.servers.ca_file
option:
# Enables SSL certificate verification if encryption method is
# "start_tls" or "simple_tls". Defaults to true.
verify_certificates: true
ca_file: 'asdf'
- Run
bin/rails console
. - In the GDK, this should crash.
- Disable the
raise
and repeat the test. This should NOT crash:
diff --git a/lib/gitlab_settings/options.rb b/lib/gitlab_settings/options.rb
index 3338c2d216ba..eed5013b52dd 100644
--- a/lib/gitlab_settings/options.rb
+++ b/lib/gitlab_settings/options.rb
@@ -172,7 +172,7 @@ def respond_to_missing?(name, include_all = false)
# fail to load User since the Devise is not yet set up in
# `config/initialiers/8_devise.rb`.
def log_and_raise_dev_exception(message, extra = {})
- raise message unless Rails.env.production?
+# raise message unless Rails.env.production?
# Gitlab::BacktraceCleaner drops config/initializers, so we just limit the
# backtrace to the first 10 lines.
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.