Add latest changes from gitlab-org/gitlab@master

GITALY_SERVER_VERSION

@@ -1 +1 @@
-6ca31a17e3225737d5cf9deba18c9b18951120a8
+20fe02e33885f459cddfb32717e02141d01ab12f

app/mailers/emails/profile.rb

@@ -65,9 +65,7 @@ module Emails
       @target_url = profile_personal_access_tokens_url
       @token_name = token_name
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("A new personal access token has been created")))
-        mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("A new personal access token has been created")))
-      end
     end
 
     def access_token_about_to_expire_email(user, token_names)
@@ -78,9 +76,7 @@ module Emails
       @target_url = profile_personal_access_tokens_url
       @days_to_expire = PersonalAccessToken::DAYS_TO_EXPIRE
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Your personal access tokens will expire in %{days_to_expire} days or less") % { days_to_expire: @days_to_expire }))
-        mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Your personal access tokens will expire in %{days_to_expire} days or less") % { days_to_expire: @days_to_expire }))
-      end
     end
 
     def access_token_expired_email(user, token_names = [])
@@ -90,9 +86,7 @@ module Emails
       @token_names = token_names
       @target_url = profile_personal_access_tokens_url
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Your personal access tokens have expired")))
-        mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Your personal access tokens have expired")))
-      end
     end
 
     def access_token_revoked_email(user, token_name, source = nil)
@@ -103,9 +97,7 @@ module Emails
       @target_url = profile_personal_access_tokens_url
       @source = source
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("A personal access token has been revoked")))
-        mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("A personal access token has been revoked")))
-      end
     end
 
     def ssh_key_expired_email(user, fingerprints)
@@ -115,9 +107,7 @@ module Emails
       @fingerprints = fingerprints
       @target_url = profile_keys_url
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Your SSH key has expired")))
-        mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Your SSH key has expired")))
-      end
     end
 
     def ssh_key_expiring_soon_email(user, fingerprints)
@@ -127,9 +117,7 @@ module Emails
       @fingerprints = fingerprints
       @target_url = profile_keys_url
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Your SSH key is expiring soon.")))
-        mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Your SSH key is expiring soon.")))
-      end
     end
 
     def unknown_sign_in_email(user, ip, time)
@@ -138,11 +126,9 @@ module Emails
       @time = time
       @target_url = edit_profile_password_url
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      email_with_layout(
-        email_with_layout(
+        to: @user.notification_email_or_default,
-          to: @user.notification_email_or_default,
+        subject: subject(_("%{host} sign-in from new location") % { host: Gitlab.config.gitlab.host }))
-          subject: subject(_("%{host} sign-in from new location") % { host: Gitlab.config.gitlab.host }))
-      end
     end
 
     def two_factor_otp_attempt_failed_email(user, ip, time = Time.current)
@@ -150,11 +136,9 @@ module Emails
       @ip = ip
       @time = time
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      email_with_layout(
-        email_with_layout(
+        to: @user.notification_email_or_default,
-          to: @user.notification_email_or_default,
+        subject: subject(_("Attempted sign in to %{host} using an incorrect verification code") % { host: Gitlab.config.gitlab.host }))
-          subject: subject(_("Attempted sign in to %{host} using an incorrect verification code") % { host: Gitlab.config.gitlab.host }))
-      end
     end
 
     def disabled_two_factor_email(user)
@@ -162,9 +146,7 @@ module Emails
 
       @user = user
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Two-factor authentication disabled")))
-        mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("Two-factor authentication disabled")))
-      end
     end
 
     def new_email_address_added_email(user, email)
@@ -173,9 +155,7 @@ module Emails
       @user = user
       @email = email
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("New email address added")))
-        mail_with_locale(to: @user.notification_email_or_default, subject: subject(_("New email address added")))
-      end
     end
 
     def new_achievement_email(user, achievement)
@@ -184,11 +164,9 @@ module Emails
       @user = user
       @achievement = achievement
 
-      Gitlab::I18n.with_locale(@user.preferred_language) do
+      email_with_layout(
-        email_with_layout(
+        to: @user.notification_email_or_default,
-          to: @user.notification_email_or_default,
+        subject: subject(s_("Achievements|%{namespace_full_path} awarded you the %{achievement_name} achievement") % { namespace_full_path: @achievement.namespace.full_path, achievement_name: @achievement.name }))
-          subject: subject(s_("Achievements|%{namespace_full_path} awarded you the %{achievement_name} achievement") % { namespace_full_path: @achievement.namespace.full_path, achievement_name: @achievement.name }))
-      end
     end
   end
 end

config/application.rb

@@ -30,13 +30,6 @@ module Gitlab
 
     # Rails 7.0
     config.action_controller.raise_on_open_redirects = false
-    config.action_dispatch.default_headers = { "X-Frame-Options" => "SAMEORIGIN",
-      "X-XSS-Protection" => "1; mode=block",
-      "X-Content-Type-Options" => "nosniff",
-      "X-Download-Options" => "noopen",
-      "X-Permitted-Cross-Domain-Policies" => "none",
-      "Referrer-Policy" => "strict-origin-when-cross-origin" }
-
     config.action_dispatch.return_only_request_media_type_on_content_type = true
     config.action_mailer.smtp_timeout = nil # New default is 5
     config.action_view.button_to_generates_button_tag = nil # New default is true

doc/api/graphql/reference/index.md

@@ -4890,7 +4890,7 @@ Input type: `MergeRequestUpdateApprovalRuleInput`
 | <a id="mutationmergerequestupdateapprovalruleiid"></a>`iid` | [`String!`](#string) | IID of the merge request to mutate. |
 | <a id="mutationmergerequestupdateapprovalrulename"></a>`name` | [`String!`](#string) | Name of the approval rule. |
 | <a id="mutationmergerequestupdateapprovalruleprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the merge request to mutate is in. |
-| <a id="mutationmergerequestupdateapprovalruleremovehiddengroups"></a>`removeHiddenGroups` | [`[Boolean!]`](#boolean) | Whether hidden groups should be removed. |
+| <a id="mutationmergerequestupdateapprovalruleremovehiddengroups"></a>`removeHiddenGroups` | [`Boolean`](#boolean) | Whether hidden groups should be removed. |
 | <a id="mutationmergerequestupdateapprovalruleuserids"></a>`userIds` | [`[String!]`](#string) | IDs of users as approvers. |
 
 #### Fields

lib/gitlab/database/load_balancing/load_balancer.rb

@@ -12,6 +12,8 @@ module Gitlab
 
         REPLICA_SUFFIX = '_replica'
 
+        attr_accessor :service_discovery
+
         attr_reader :host_list, :configuration
 
         # configuration - An instance of `LoadBalancing::Configuration` that
@@ -45,6 +47,8 @@ module Gitlab
         # If no secondaries were available this method will use the primary
         # instead.
         def read(&block)
+          service_discovery&.log_refresh_thread_interruption
+
           conflict_retried = 0
 
           while host
@@ -103,6 +107,8 @@ module Gitlab
 
         # Yields a connection that can be used for both reads and writes.
         def read_write
+          service_discovery&.log_refresh_thread_interruption
+
           connection = nil
           transaction_open = nil
 

lib/gitlab/database/load_balancing/service_discovery.rb

@@ -15,12 +15,14 @@ module Gitlab
       class ServiceDiscovery
         EmptyDnsResponse = Class.new(StandardError)
 
+        attr_accessor :refresh_thread, :refresh_thread_last_run, :refresh_thread_interruption_logged
+
         attr_reader :interval, :record, :record_type, :disconnect_timeout,
                     :load_balancer
 
         MAX_SLEEP_ADJUSTMENT = 10
-
         MAX_DISCOVERY_RETRIES = 3
+        DISCOVERY_THREAD_REFRESH_DELTA = 3
 
         RETRY_DELAY_RANGE = (0.1..0.2).freeze
 
@@ -74,8 +76,10 @@ module Gitlab
         # rubocop:enable Metrics/ParameterLists
 
         def start
-          Thread.new do
+          self.refresh_thread = Thread.new do
             loop do
+              self.refresh_thread_last_run = Time.current
+
               next_sleep_duration = perform_service_discovery
 
               # We slightly randomize the sleep() interval. This should reduce
@@ -103,15 +107,6 @@ module Gitlab
             # Slightly randomize the retry delay so that, in the case of a total
             # dns outage, all starting services do not pressure the dns server at the same time.
             sleep(rand(RETRY_DELAY_RANGE))
-          rescue Exception => error # rubocop:disable Lint/RescueException
-            # All exceptions are logged to find any pattern and solve https://gitlab.com/gitlab-org/gitlab/-/issues/364370
-            # This will be removed in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120173
-            Gitlab::Database::LoadBalancing::Logger.error(
-              event: :service_discovery_unexpected_exception,
-              message: "Service discovery encountered an uncaught error: #{error.message}"
-            )
-
-            raise
           end
 
           interval
@@ -214,6 +209,20 @@ module Gitlab
           )
         end
 
+        def log_refresh_thread_interruption
+          return if refresh_thread_last_run.blank? || refresh_thread_interruption_logged ||
+            (refresh_thread_last_run + DISCOVERY_THREAD_REFRESH_DELTA.minutes).future?
+
+          Gitlab::Database::LoadBalancing::Logger.error(
+            event: :service_discovery_refresh_thread_interrupt,
+            refresh_thread_last_run: refresh_thread_last_run,
+            thread_status: refresh_thread&.status&.to_s,
+            thread_backtrace: refresh_thread&.backtrace&.join('\n')
+          )
+
+          self.refresh_thread_interruption_logged = true
+        end
+
         private
 
         def record_type_for(type)

lib/gitlab/database/load_balancing/setup.rb

@@ -55,6 +55,8 @@ module Gitlab
 
           sv = ServiceDiscovery.new(load_balancer, **configuration.service_discovery)
 
+          load_balancer.service_discovery = sv
+
           sv.perform_service_discovery
 
           sv.start if @start_service_discovery

spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
+RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store, feature_category: :database do
   let(:conflict_error) { Class.new(RuntimeError) }
   let(:model) { ActiveRecord::Base }
   let(:db_host) { model.connection_pool.db_config.host }
@@ -71,7 +71,30 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
     end
   end
 
+  shared_examples 'logs service discovery thread interruption' do |lb_method|
+    context 'with service discovery' do
+      let(:service_discovery) do
+        instance_double(
+          Gitlab::Database::LoadBalancing::ServiceDiscovery,
+          log_refresh_thread_interruption: true
+        )
+      end
+
+      before do
+        allow(lb).to receive(:service_discovery).and_return(service_discovery)
+      end
+
+      it 'calls logs service discovery thread interruption' do
+        expect(service_discovery).to receive(:log_refresh_thread_interruption)
+
+        lb.public_send(lb_method) {}
+      end
+    end
+  end
+
   describe '#read' do
+    it_behaves_like 'logs service discovery thread interruption', :read
+
     it 'yields a connection for a read' do
       connection = double(:connection)
       host = double(:host)
@@ -203,6 +226,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
   end
 
   describe '#read_write' do
+    it_behaves_like 'logs service discovery thread interruption', :read_write
+
     it 'yields a connection for a write' do
       connection = ActiveRecord::Base.connection_pool.connection
 

spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb

@@ -58,14 +58,14 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery, feature_catego
     end
   end
 
-  describe '#start' do
+  describe '#start', :freeze_time do
     before do
       allow(service)
         .to receive(:loop)
         .and_yield
     end
 
-    it 'starts service discovery in a new thread' do
+    it 'starts service discovery in a new thread with proper assignments' do
       expect(Thread).to receive(:new).ordered.and_call_original # Thread starts
 
       expect(service).to receive(:perform_service_discovery).ordered.and_return(5)
@@ -73,6 +73,9 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery, feature_catego
       expect(service).to receive(:sleep).ordered.with(7) # Sleep runs after thread starts
 
       service.start.join
+
+      expect(service.refresh_thread_last_run).to eq(Time.current)
+      expect(service.refresh_thread).to be_present
     end
   end
 
@@ -142,21 +145,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery, feature_catego
         service.perform_service_discovery
       end
     end
-
-    context 'with Exception' do
-      it 'logs error and re-raises the exception' do
-        error = Exception.new('uncaught-test-error')
-
-        expect(service).to receive(:refresh_if_necessary).and_raise(error)
-
-        expect(Gitlab::Database::LoadBalancing::Logger).to receive(:error).with(
-          event: :service_discovery_unexpected_exception,
-          message: "Service discovery encountered an uncaught error: uncaught-test-error"
-        )
-
-        expect { service.perform_service_discovery }.to raise_error(Exception, error.message)
-      end
-    end
   end
 
   describe '#refresh_if_necessary' do
@@ -427,4 +415,64 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery, feature_catego
       end
     end
   end
+
+  describe '#log_refresh_thread_interruption' do
+    before do
+      service.refresh_thread = refresh_thread
+      service.refresh_thread_last_run = last_run_timestamp
+    end
+
+    let(:refresh_thread) { nil }
+    let(:last_run_timestamp) { nil }
+
+    subject { service.log_refresh_thread_interruption }
+
+    context 'without refresh thread timestamp' do
+      it 'does not log any interruption' do
+        expect(service.refresh_thread_last_run).to be_nil
+
+        expect(Gitlab::Database::LoadBalancing::Logger).not_to receive(:error)
+
+        subject
+      end
+    end
+
+    context 'with refresh thread timestamp' do
+      let(:last_run_timestamp) { Time.current }
+
+      it 'does not log if last run time plus delta is in future' do
+        expect(Gitlab::Database::LoadBalancing::Logger).not_to receive(:error)
+
+        subject
+      end
+
+      context 'with way past last run timestamp' do
+        let(:refresh_thread) { instance_double(Thread, status: :run, backtrace: %w[backtrace foo]) }
+        let(:last_run_timestamp) { 20.minutes.before + described_class::DISCOVERY_THREAD_REFRESH_DELTA.minutes }
+
+        it 'does not log if the interruption is already logged' do
+          service.refresh_thread_interruption_logged = true
+
+          expect(Gitlab::Database::LoadBalancing::Logger).not_to receive(:error)
+
+          subject
+        end
+
+        it 'logs the error if the interruption was not logged before' do
+          expect(service.refresh_thread_interruption_logged).not_to be_present
+
+          expect(Gitlab::Database::LoadBalancing::Logger).to receive(:error).with(
+            event: :service_discovery_refresh_thread_interrupt,
+            refresh_thread_last_run: last_run_timestamp,
+            thread_status: refresh_thread.status.to_s,
+            thread_backtrace: 'backtrace\nfoo'
+          )
+
+          subject
+
+          expect(service.refresh_thread_interruption_logged).to be_truthy
+        end
+      end
+    end
+  end
 end