Merge branch...

Merge branch '322991-asset_proxy_allowlist-errors-in-13-9-for-geo-with-downtime-upgrades-or-restoring-from-backup' into 'master'

Rename asset_proxy_allowlist column

See merge request gitlab-org/gitlab!55419

app/models/application_setting.rb

@@ -53,8 +53,9 @@ class ApplicationSetting < ApplicationRecord
   serialize :domain_allowlist, Array # rubocop:disable Cop/ActiveRecordSerialize
   serialize :domain_denylist, Array # rubocop:disable Cop/ActiveRecordSerialize
   serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize
-  serialize :asset_proxy_allowlist, Array # rubocop:disable Cop/ActiveRecordSerialize
+
   # See https://gitlab.com/gitlab-org/gitlab/-/issues/300916
+  serialize :asset_proxy_allowlist, Array # rubocop:disable Cop/ActiveRecordSerialize
   serialize :asset_proxy_whitelist, Array # rubocop:disable Cop/ActiveRecordSerialize
 
   cache_markdown_field :sign_in_text

app/models/application_setting_implementation.rb

@@ -280,13 +280,18 @@ module ApplicationSettingImplementation
     self.notes_create_limit_allowlist = strings_to_array(values).map(&:downcase)
   end
 
-  def asset_proxy_allowlist=(values)
+  def asset_proxy_whitelist=(values)
     values = strings_to_array(values) if values.is_a?(String)
 
     # make sure we always allow the running host
     values << Gitlab.config.gitlab.host unless values.include?(Gitlab.config.gitlab.host)
 
-    self[:asset_proxy_allowlist] = values
+    self[:asset_proxy_whitelist] = values
+  end
+  alias_method :asset_proxy_allowlist=, :asset_proxy_whitelist=
+
+  def asset_proxy_allowlist
+    read_attribute(:asset_proxy_whitelist)
   end
 
   def repository_storages

app/services/application_settings/update_service.rb

@@ -6,7 +6,7 @@ module ApplicationSettings
 
     attr_reader :params, :application_setting
 
-    MARKDOWN_CACHE_INVALIDATING_PARAMS = %w(asset_proxy_enabled asset_proxy_url asset_proxy_secret_key asset_proxy_allowlist).freeze
+    MARKDOWN_CACHE_INVALIDATING_PARAMS = %w(asset_proxy_enabled asset_proxy_url asset_proxy_secret_key asset_proxy_whitelist).freeze
 
     def execute
       result = update_settings

changelogs/unreleased/322991-asset_proxy_allowlist-errors-in-13-9-for-geo-with-downtime-upgrade.yml

+---
+title: Rename asset_proxy_allowlist column
+merge_request: 55419
+author:
+type: fixed

db/migrate/20210301200601_rename_asset_proxy_allowlist_on_application_settings.rb

+# frozen_string_literal: true
+
+class RenameAssetProxyAllowlistOnApplicationSettings < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers::V2
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    rename_column_concurrently :application_settings,
+      :asset_proxy_allowlist,
+      :asset_proxy_whitelist
+  end
+
+  def down
+    undo_rename_column_concurrently :application_settings,
+      :asset_proxy_allowlist,
+      :asset_proxy_whitelist
+  end
+end

db/post_migrate/20210301200959_clean_up_asset_proxy_allowlist_rename_on_application_settings.rb

+# frozen_string_literal: true
+
+class CleanUpAssetProxyAllowlistRenameOnApplicationSettings < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers::V2
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    cleanup_concurrent_column_rename :application_settings,
+      :asset_proxy_allowlist,
+      :asset_proxy_whitelist
+  end
+
+  def down
+    undo_cleanup_concurrent_column_rename :application_settings,
+      :asset_proxy_allowlist,
+      :asset_proxy_whitelist
+  end
+end

db/schema_migrations/20210301200601

+21ae7ea7cbf1d34c7b9dc300a641eaf975ed1e33f5bc519494cd37c4a661bec8
\ No newline at end of file

db/schema_migrations/20210301200959

+28b90c9b7c2e4f2e2b12088f5aee85c16dfb567f89ed6a8e771f2c5d91c818d9
\ No newline at end of file

db/structure.sql

@@ -9394,12 +9394,12 @@ CREATE TABLE application_settings (
     invisible_captcha_enabled boolean DEFAULT false NOT NULL,
     enforce_ssh_key_expiration boolean DEFAULT false NOT NULL,
     git_two_factor_session_expiry integer DEFAULT 15 NOT NULL,
-    asset_proxy_allowlist text,
     keep_latest_artifact boolean DEFAULT true NOT NULL,
     notes_create_limit integer DEFAULT 300 NOT NULL,
     notes_create_limit_allowlist text[] DEFAULT '{}'::text[] NOT NULL,
     kroki_formats jsonb DEFAULT '{}'::jsonb NOT NULL,
     in_product_marketing_emails_enabled boolean DEFAULT true NOT NULL,
+    asset_proxy_whitelist text,
     CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
     CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
     CONSTRAINT check_17d9558205 CHECK ((char_length(kroki_url) <= 1024)),

spec/models/application_setting_spec.rb

@@ -650,6 +650,32 @@ RSpec.describe ApplicationSetting do
         end
       end
 
+      describe '#asset_proxy_whitelist' do
+        context 'when given an Array' do
+          it 'sets the domains and adds current running host' do
+            setting.asset_proxy_whitelist = ['example.com', 'assets.example.com']
+            expect(setting.asset_proxy_whitelist).to eq(['example.com', 'assets.example.com', 'localhost'])
+          end
+        end
+
+        context 'when given a String' do
+          it 'sets multiple domains with spaces' do
+            setting.asset_proxy_whitelist = 'example.com *.example.com'
+            expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost'])
+          end
+
+          it 'sets multiple domains with newlines and a space' do
+            setting.asset_proxy_whitelist = "example.com\n *.example.com"
+            expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost'])
+          end
+
+          it 'sets multiple domains with commas' do
+            setting.asset_proxy_whitelist = "example.com, *.example.com"
+            expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost'])
+          end
+        end
+      end
+
       describe '#asset_proxy_allowlist' do
         context 'when given an Array' do
           it 'sets the domains and adds current running host' do

spec/services/application_settings/update_service_spec.rb

@@ -123,6 +123,7 @@ RSpec.describe ApplicationSettings::UpdateService do
     it_behaves_like 'invalidates markdown cache', { asset_proxy_url: 'http://test.com' }
     it_behaves_like 'invalidates markdown cache', { asset_proxy_secret_key: 'another secret' }
     it_behaves_like 'invalidates markdown cache', { asset_proxy_allowlist: ['domain.com'] }
+    it_behaves_like 'invalidates markdown cache', { asset_proxy_whitelist: ['domain.com'] }
 
     context 'when also setting the local_markdown_version' do
       let(:params) { { asset_proxy_enabled: true, local_markdown_version: 12 } }