Merge branch 'pages_consolidated_object_storage_settings' into 'master'

Add pages to consolidated object storage settings

See merge request gitlab-org/gitlab!44032

config/object_store_settings.rb

 # Set default values for object_store settings
 class ObjectStoreSettings
-  SUPPORTED_TYPES = %w(artifacts external_diffs lfs uploads packages dependency_proxy terraform_state).freeze
+  SUPPORTED_TYPES = %w(artifacts external_diffs lfs uploads packages dependency_proxy terraform_state pages).freeze
   ALLOWED_OBJECT_STORE_OVERRIDES = %w(bucket enabled proxy_download).freeze
 
+  # pages may be enabled but use legacy disk storage
+  # we don't need to raise an error in that case
+  ALLOWED_INCOMPLETE_TYPES = %w(pages).freeze
+
   attr_accessor :settings
 
   # Legacy parser
@@ -115,7 +119,9 @@ class ObjectStoreSettings
 
       next unless section
 
-      raise "Object storage for #{store_type} must have a bucket specified" if section['enabled'] && target_config['bucket'].blank?
+      if section['enabled'] && target_config['bucket'].blank?
+        missing_bucket_for(store_type)
+      end
 
       # Map bucket (external name) -> remote_directory (internal representation)
       target_config['remote_directory'] = target_config.delete('bucket')
@@ -152,4 +158,14 @@ class ObjectStoreSettings
 
     true
   end
+
+  def missing_bucket_for(store_type)
+    message = "Object storage for #{store_type} must have a bucket specified"
+
+    if ALLOWED_INCOMPLETE_TYPES.include?(store_type)
+      warn "[WARNING] #{message}"
+    else
+      raise message
+    end
+  end
 end

spec/config/object_store_settings_spec.rb

@@ -24,6 +24,7 @@ RSpec.describe ObjectStoreSettings do
           'lfs' => { 'enabled' => true },
           'artifacts' => { 'enabled' => true },
           'external_diffs' => { 'enabled' => false },
+          'pages' => { 'enabled' => true },
           'object_store' => {
             'enabled' => true,
             'connection' => connection,
@@ -39,6 +40,9 @@ RSpec.describe ObjectStoreSettings do
               'external_diffs' => {
                 'bucket' => 'external_diffs',
                 'enabled' => false
+              },
+              'pages' => {
+                'bucket' => 'pages'
               }
             }
           }
@@ -64,6 +68,11 @@ RSpec.describe ObjectStoreSettings do
         expect(settings.lfs['object_store']['proxy_download']).to be true
         expect(settings.lfs['object_store']['remote_directory']).to eq('lfs-objects')
 
+        expect(settings.pages['enabled']).to be true
+        expect(settings.pages['object_store']['enabled']).to be true
+        expect(settings.pages['object_store']['connection']).to eq(connection)
+        expect(settings.pages['object_store']['remote_directory']).to eq('pages')
+
         expect(settings.external_diffs['enabled']).to be false
         expect(settings.external_diffs['object_store']['enabled']).to be false
         expect(settings.external_diffs['object_store']['remote_directory']).to eq('external_diffs')
@@ -75,6 +84,12 @@ RSpec.describe ObjectStoreSettings do
         expect { subject }.to raise_error(/Object storage for lfs must have a bucket specified/)
       end
 
+      it 'does not raise error if pages bucket is missing' do
+        config['object_store']['objects']['pages'].delete('bucket')
+
+        expect { subject }.not_to raise_error
+      end
+
       context 'with legacy config' do
         let(:legacy_settings) do
           {