Fix encrypted application settings not working with pending migrations

In GitLab 12.3.7, a number of fields were encrypted with
attr_encrypted. Unfortunately, `FakeApplicationSetting` doesn't
implement the `attr_encrypted` fields, so any encrypted value stored in
the database would be empty while pending migrations exist.

Originally `FakeApplicationSetting` existed when mixed versions of
GitLab in a no-downtime deployment used the same Redis key, causing new
attributes to be present when they were not known by older
deployments. `FakeApplicationSetting` is really only needed in a Rake
task where certain application settings may be expected to be available
but haven't been applied via a migration. Since the cache key is now
based on the GitLab and Rails version, this should no longer be an
issue.

We restrict the use of `FakeApplicationSetting` to Rake tasks. Web and
Sidekiq workers should always have a real `ApplicationSetting` object.

Closes https://gitlab.com/gitlab-org/gitlab/issues/197988

changelogs/unreleased/sh-fix-current-settings.yml

+---
+title: Fix application settings not working with pending migrations
+merge_request:
+author:
+type: fixed

lib/gitlab/current_settings.rb

@@ -50,7 +50,7 @@ module Gitlab
         # need to be added to the application settings. To prevent Rake tasks
         # and other callers from failing, use any loaded settings and return
         # defaults for missing columns.
-        if ActiveRecord::Base.connection.migration_context.needs_migration?
+        if Gitlab::Runtime.rake? && ActiveRecord::Base.connection.migration_context.needs_migration?
           db_attributes = current_settings&.attributes || {}
           fake_application_settings(db_attributes)
         elsif current_settings.present?

spec/lib/gitlab/current_settings_spec.rb

@@ -120,17 +120,13 @@ describe Gitlab::CurrentSettings do
         end
 
         context 'with pending migrations' do
+          let(:current_settings) { described_class.current_application_settings }
+
           before do
-            expect_any_instance_of(ActiveRecord::MigrationContext).to receive(:needs_migration?).and_return(true)
+            allow(Gitlab::Runtime).to receive(:rake?).and_return(false)
           end
 
           shared_examples 'a non-persisted ApplicationSetting object' do
-            let(:current_settings) { described_class.current_application_settings }
-
-            it 'returns a FakeApplicationSettings object' do
-              expect(current_settings).to be_a(Gitlab::FakeApplicationSettings)
-            end
-
             it 'uses the default value from ApplicationSetting.defaults' do
               expect(current_settings.signup_enabled).to eq(ApplicationSetting.defaults[:signup_enabled])
             end
@@ -144,18 +140,16 @@ describe Gitlab::CurrentSettings do
             end
           end
 
-          context 'with no ApplicationSetting DB record' do
-            it_behaves_like 'a non-persisted ApplicationSetting object'
-          end
-
-          context 'with an existing ApplicationSetting DB record' do
-            let!(:db_settings) { ApplicationSetting.build_from_defaults(home_page_url: 'http://mydomain.com').save! && ApplicationSetting.last }
-            let(:current_settings) { described_class.current_application_settings }
+          context 'in a Rake task' do
+            before do
+              allow(Gitlab::Runtime).to receive(:rake?).and_return(true)
+              expect_any_instance_of(ActiveRecord::MigrationContext).to receive(:needs_migration?).and_return(true)
+            end
 
             it_behaves_like 'a non-persisted ApplicationSetting object'
 
-            it 'uses the value from the DB attribute if present and not overridden by an accessor' do
-              expect(current_settings.home_page_url).to eq(db_settings.home_page_url)
+            it 'returns a FakeApplicationSettings object' do
+              expect(current_settings).to be_a(Gitlab::FakeApplicationSettings)
             end
 
             context 'when a new column is used before being migrated' do
@@ -168,6 +162,20 @@ describe Gitlab::CurrentSettings do
               end
             end
           end
+
+          context 'with no ApplicationSetting DB record' do
+            it_behaves_like 'a non-persisted ApplicationSetting object'
+          end
+
+          context 'with an existing ApplicationSetting DB record' do
+            let!(:db_settings) { ApplicationSetting.build_from_defaults(home_page_url: 'http://mydomain.com').save! && ApplicationSetting.last }
+
+            it_behaves_like 'a non-persisted ApplicationSetting object'
+
+            it 'uses the value from the DB attribute if present and not overridden by an accessor' do
+              expect(current_settings.home_page_url).to eq(db_settings.home_page_url)
+            end
+          end
         end
 
         context 'when ApplicationSettings.current is present' do